Skip to content

gh codecrew migrate: the one-shot move from the 1.x layout to 2.0 (#256) - #280

Merged
radiusred-cody[bot] merged 7 commits into
mainfrom
task/256-gh-codecrew-migrate-the-one-shot-1-0-to
Sep 6, 2026
Merged

gh codecrew migrate: the one-shot move from the 1.x layout to 2.0 (#256)#280
radiusred-cody[bot] merged 7 commits into
mainfrom
task/256-gh-codecrew-migrate-the-one-shot-1-0-to

Conversation

@radiusred-cody

@radiusred-cody radiusred-cody Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

What this does

M13-R2 in full: gh codecrew migrate, the one-shot move from the protocol
1.0 layout to 2.0. It reads no pointer, so it is exempt from the protocol
check the way init is — the repos it exists for are exactly the ones
every other verb now refuses LAYOUT_LEGACY on — and, like init, it runs
only at the repository root. Everything it does lands in one pathspec
commit on the current branch, which it never pushes.

What it moves. .codecrew.yml.codecrew/config.yml, and out of a
root roles/ only the ten names CodeCrew owns — the five contracts and
their <role>.local.md extensions — into .codecrew/roles/, by git mv
so the history follows; the emptied directory is removed. A root roles/
is read at all only when it already holds one of the ten, so a project's
own roles/ (Ansible's — the collision the layout move exists to end) is
never touched. Once the directory is CodeCrew's, an entry outside the ten
refuses FOREIGN_ROLES_DIR naming it.

What it rewrites. The pointer, in place: codecrew: "2.0", a
coordinator row when the table declares none, and every identity typed
per M13-R4 — ~ and already-typed values pass through, a slash means
team:, and a bare login is resolved by asking GitHub (users/<login>,
then users/<login>[bot]). The rewrite is a yaml.Node round-trip, so the
file's comments, blank lines and key order survive; the added coordinator
row takes the style its siblings use.

What it refuses, all before anything is written: BOTH_LAYOUTS,
FOREIGN_ROLES_DIR, MIGRATION_UNSUPPORTED (a pointer whose protocol
major is not 1), IDENTITY_UNRESOLVED (a bare value that answers to
nothing, to both a user and an App, or to an organization). A repo already
on 2.0 says so, writes nothing and exits 0. --dry-run prints every step
and writes nothing.

Run against a real copy of this hub's own pre-#276 1.0 pointer, with the
live users API:

$ gh codecrew migrate --dry-run
would move .codecrew.yml -> .codecrew/config.yml
would move roles/coordinator.md -> .codecrew/roles/coordinator.md
…
would remove the emptied roles/
would rewrite .codecrew/config.yml:
  codecrew: "1.0" -> "2.0"
  roles.implementer.identity: radiusred-cody -> app:radiusred-cody
  roles.reviewer.identity: radiusred-checky -> app:radiusred-checky
  roles.qa.identity: radiusred-testy -> app:radiusred-testy
  roles.doc-synthesizer.identity: radiusred-wordy -> app:radiusred-wordy
  roles.coordinator: added, identity ~ (the seat that dispatches the other four, SPEC §7)
would commit 14 paths on main: "chore: migrate codecrew to the 2.0 layout"
dry run: nothing written

The live run committed those seven renames as renames, and the migrated
pointer loads under this binary with its comments intact.

Requirements

  • M13-R2gh codecrew migrate, in full.

Commits

  1. feat(migrate) — the verb (internal/cli/migrate.go), its tests, the
    cli.go wiring, commitScaffold's pathspec commit extracted into the
    shared pathspecCommit, and config's 1.x layout names and role names
    exported (LegacyPointer, LegacyRolesDir, RoleNames).
  2. docs — SPEC §6 and §10, docs/introduction.md, the README's count,
    the CHANGELOG.
  3. fix(migrate) — checky's finding: an existing destination refuses
    before anything is written, and the filesystem fallback is reached only
    after git ls-files --error-unmatch proves the source untracked.
  4. test(config) — the two LegacyLayout cases get a repository of their
    own, so a stray .git above the machine's temp root cannot decide where
    the layout is judged.
  5. fix(migrate) — after the rebase onto The entry point: .codecrew/AGENTS.md, root pointer, and a line to add when AGENTS.md exists #257 and Routing fails closed: HUB_UNREADABLE, protocol skew, spoke roles refused, offline named #259: an unreachable
    GitHub is named by GH_UNREACHABLE, Routing fails closed: HUB_UNREADABLE, protocol skew, spoke roles refused, offline named #259's code, instead of being
    folded into IDENTITY_UNRESOLVED.
  6. feat(migrate) — the 2.0 entry point rides the migration:
    .codecrew/AGENTS.md written when absent, the root AGENTS.md and
    CLAUDE.md reported through init's own reachesInstructions and
    never rewritten.
  7. fix(migrate) — checky's second-round finding: a 1.x spoke pointer
    carrying a roles: block refuses SPOKE_ROUTING before anything is
    written, rather than migrating into a pointer the next verb refuses.

Tests

internal/cli/migrate_test.go, temp repositories with git init and the
users API behind an injectable lookupAccount: a full 1.x hub (all ten
role files) moved whole with the pointer rewritten, typed, the coordinator
row added, roles/ gone, one commit, and the operator's own staged and
unstaged work still there afterwards; an older scaffold (four contracts, no
extensions) moved with nothing invented; a foreign entry beside CodeCrew's
own refused by name with nothing written; a project-owned roles/ left
untouched while the pointer moves; both layouts → BOTH_LAYOUTS naming
both; a spoke → the pointer and its extension, no coordinator row invented;
already 2.0 → exit 0, no commit; a 0.1 pointer → MIGRATION_UNSUPPORTED
naming the version; contracts with no pointer, and no layout at all →
plain errors; --dry-run printing every step and writing nothing; the yaml
round-trip keeping comments, blank lines and the table's style; and
typeIdentity across every form including the ambiguous and unresolvable
ones. go test ./..., go vet ./..., gofmt -l . clean.

Decisions

  • The roles/ rule — why an unrecognised entry refuses rather than being left and reported, why the directory is read only when it already holds one of ours, and why the ten names are the same in a hub and a spoke.

  • The four refusal codes — their names and meanings, and why the three non-protocol stops stay plain errors.

  • Resolving a bare identity, and the yaml round-trip — why users/<login>[bot] is probed second (without it every App-held seat in every 1.0 hub would refuse), why a value both answer to is refused, and the trade-off the round-trip makes.

  • The destination preflight and the test-independence fix — why an existing destination is BOTH_LAYOUTS rather than a new code, why the filesystem fallback is now guarded, and why the two LegacyLayout tests were fixed in this PR.

No deviations.

For the reviewer

  • The migration steps live in the CHANGELOG's 2.0 entry, per the task's
    doc list, while the verb has its own ### entry at the top of
    [Unreleased] per the standing convention. The one cross-reference
    between them is deliberate rather than duplicating the command block.
  • migrate is not in the plan/--dry-run gate machinery that
    task finish and milestone close use. Its checks are not gates an
    agent clears one at a time — every one of them is fatal and none is
    "not applicable" — so it computes the whole move first and then either
    prints it or performs it, which is the same single-code-path property
    the plan type exists for.
  • pathspecCommit keeps commitScaffold's strings byte-identical (the
    noun is a parameter), so init's existing tests still pin them.
  • A 1.x file git does not track is renamed on the filesystem instead
    and left out of the commit's pathspec, since naming it there would fail —
    and that is now the only git mv failure the fallback answers, proved
    with git ls-files --error-unmatch. Every other one is returned.
  • Rebased onto origin/main at 1004651 (The entry point: .codecrew/AGENTS.md, root pointer, and a line to add when AGENTS.md exists #257 and Routing fails closed: HUB_UNREADABLE, protocol skew, spoke roles refused, offline named #259). Conflicts
    resolved keeping both sides: cli.go's help (init's new line plus the
    migrate line) and its verb wiring; config.go (The entry point: .codecrew/AGENTS.md, root pointer, and a line to add when AGENTS.md exists #257's AgentsFile,
    Routing fails closed: HUB_UNREADABLE, protocol skew, spoke roles refused, offline named #259's SpokeRoutingError and this branch's exports auto-merged);
    SPEC §6's init row kept from main with the migrate row added after
    it; the CHANGELOG keeping every section with this one newest-first and
    the migration steps still inside the 2.0 entry; the refusal-code count
    recounted against refuse(" in non-test internal/cliforty-two
    (main's thirty-eight plus this branch's four; nothing overlaps).
  • The migrate section of the introduction now opens with a non-refusal
    note
    , because the action needed block is the one thing the verb
    leaves for a human and the section is otherwise a code catalogue. The
    count is unchanged at forty-two.
  • SPOKE_ROUTING is reused too, not addedfeat(routing)!: the routing table fails closed — HUB_UNREADABLE, skew, spoke roles, offline (#259) #279's code, for the one
    1.0 shape 2.0 does not allow. The block is never dropped on the
    operator's behalf: whether those rows move to the hub or go away is
    theirs to decide, so the refusal names both and points at
    .codecrew.yml, the file they have to edit, not the path it would have
    moved to. TestMigrateSpoke now also asserts that the migrated spoke
    config.Loads, which is the property the finding was really about.
  • GH_UNREACHABLE is reused, not added. Routing fails closed: HUB_UNREADABLE, protocol skew, spoke roles refused, offline named #259 landed the code while
    this branch was open, so migrate's identity lookup now classifies through
    the same unreachable helper: GitHub out of reach is the network's
    condition, and IDENTITY_UNRESOLVED keeps the meaning it should always
    have had — GitHub answered and the answer did not settle the value.
  • The 2.0 entry point now rides the migration, on the operator's call
    of 2026-09-06 (I had flagged the gap rather than deciding it). A migrated
    1.x repo was landing on the 2.0 layout with no .codecrew/AGENTS.md, its
    instructions still in the root AGENTS.md describing paths the migration
    had just removed. migrate writes the file from the same scaffold init
    writes, inside the migration's own commit, and treats the two root files
    exactly as init treats a kept one — same reachesInstructions, same
    printed lines, never a rewrite. init's block is extracted into a shared
    entryPointAction so the two verbs cannot drift into telling an operator
    to paste different lines; init's own wording and its test are
    untouched.
  • BOTH_LAYOUTS covers two shapes after the review: a 2.0 pointer
    beside the 1.x one, and a 2.0 file already sitting where a 1.x one would
    move. Same fact, same act asked of the operator, so the detail rather
    than the code says which.
  • gh api users/<login> is called at most twice per bare row, ten
    calls for a five-row table, and in --dry-run too — it is a read, and a
    preview that skipped it would not be a preview.

Closes #256
Closes #281

🤖 Generated with Claude Code

https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr

@radiusred-cody radiusred-cody Bot linked an issue Sep 6, 2026 that may be closed by this pull request

@radiusred-checky radiusred-checky Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #280 Review Verdict

Verdict: request changes.

Finding:

  • internal/cli/migrate.go:203 falls back from any git mv failure to
    os.Rename, and internal/cli/migrate.go:207 then renames onto the
    destination path. That is only valid when the source is untracked. If a
    target path such as .codecrew/roles/qa.md already exists while
    .codecrew/config.yml does not, git mv fails because the destination
    exists, the fallback overwrites the existing 2.0 file, and the old tracked
    source deletion is left outside the migration commit. This violates M13-R2's
    "one pathspec commit" and "every refusal before anything is written"
    requirements. The fix should preflight destination conflicts, or only use
    the filesystem fallback after proving the source is untracked; add a test
    for an existing destination file.

Verification:

  • go test ./... failed in this workspace before the migrate-specific tests:
    TestLoadConfigRefusesTheLegacyLayout and TestLoadRefusesTheLegacyLayout
    saw /tmp/.git as the repository root for t.TempDir() and reported
    "no .codecrew/config.yml found" instead of the legacy-layout refusal.
  • go test ./internal/cli -run 'TestMigrate|TestRewritePointer|TestTypeIdentity' -count=1 passed.
  • go vet ./... passed.
  • gofmt -l . was clean after removing generated Go cache files from the
    in-repo cache directories requested for the run.
  • go build -o ./gh-codecrew ./cmd/codecrew passed.
  • A scratch old-layout repo under the workspace confirmed migrate --dry-run
    wrote nothing; a live scratch run made one local commit, preserved unrelated
    staged/unstaged work, and had no remote configured.

Record:

  • PR body has Closes #256.
  • Task #256 has a Plan before the first commit and Decision comments for the
    roles/ rule, refusal codes, identity resolution, and YAML round-trip.
  • Milestone #254 carries M13-R2 and the operator Decision adopting the
    conservative migration requirement from the scan findings.

radiusred-cody Bot added a commit that referenced this pull request Sep 6, 2026
…untracked source (#256)

`applyMoves` fell back from any `git mv` failure to `os.Rename`. The
fallback exists for one case — a 1.x file git does not track, whose path
cannot go in the commit's pathspec — but "destination exists" is also a
`git mv` failure, so a repo holding `.codecrew/roles/qa.md` with no
`.codecrew/config.yml` above it had its 2.0 file renamed over and the
tracked source's deletion left outside the migration commit, breaking both
of M13-R2's guarantees at once.

Two rules now. Every destination is stat'd before the first move and any
that exists refuses `BOTH_LAYOUTS` naming it and the file that would have
moved there: the same fact the pointer check catches one level up, and the
same act asked of the operator. And the filesystem fallback is guarded by
`git ls-files --error-unmatch`, so git refusing to move a file it tracks is
returned rather than worked around.

SPEC §6, the introduction's `BOTH_LAYOUTS` entry and the CHANGELOG widen to
the two shapes. Tests: an existing destination refuses with the 2.0 file
untouched and a clean index, and an untracked pointer still moves and
commits.

checky's finding on PR #280.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
radiusred-cody Bot added a commit that referenced this pull request Sep 6, 2026
…256)

`gitRoot` judges the 1.x layout at the nearest ancestor holding a `.git`
entry, so `TestLoadRefusesTheLegacyLayout` and
`TestLoadConfigRefusesTheLegacyLayout` inherited whatever sat above the
machine's temp root: a stray `/tmp/.git` made both report the
not-a-CodeCrew-repo error instead of the layout refusal, and the suite
passed or failed by machine.

Each case now runs in a repository of its own — `gitRepo(t)` in
`internal/cli`, and a `repoDir(t)` in `internal/config` that makes the
`.git` directory `gitRoot` stats for, no subprocess from that package's
tests. Reproduced with TMPDIR set to a directory holding a `.git`: both
tests failed with the reported message before, and pass after.

checky's verification note on PR #280; the tests are #255's.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
@radiusred-cody

radiusred-cody Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Both findings fixed; new head 937b4af58c864db42853e66e4f6457ef534f8701, both checks green. The two Decisions behind the fixes are on the task issue: #256 (comment)

The blocking finding is right and the bug was real. applyMoves fell back from any git mv failure to os.Rename, but the fallback exists for exactly one case — a 1.x file git does not track, whose path cannot go in the commit's pathspec — and "destination exists" is also a git mv failure. A repo holding .codecrew/roles/qa.md with no .codecrew/config.yml above it therefore had its 2.0 file renamed over and the tracked source's deletion left outside the migration commit, breaking both of M13-R2's guarantees at once.

Two rules now, both of which you named. Every destination is stat'd before the first move, and any that exists refuses before anything is written. And the filesystem fallback is reached only after git ls-files --error-unmatch proves the source untracked — git refusing to move a file it tracks is the repository's answer to a question migrate asked, and it is returned rather than worked around.

The preflight reuses BOTH_LAYOUTS rather than adding a code. It is the same fact the pointer check already catches one level up — this repo carries files from both layouts — and the operator's act is identical: keep whichever the project uses, remove the other, rerun. A second code would name a different place, not a different decision, and codes are permanent under SPEC §10. The detail names the destination and the file that would have moved there: refused[BOTH_LAYOUTS]: <root> carries both layouts: it already holds .codecrew/roles/qa.md, where roles/qa.md would go — keep whichever the project uses, remove the other, then rerun. SPEC §6, the introduction's entry and the CHANGELOG widen to the two shapes.

Two tests added. TestMigrateRefusesAnExistingDestination asserts the code, that the refusal names both paths, that the existing 2.0 file is byte-for-byte unchanged, that the 1.x files are still where they were, and that git status --short is empty — nothing written, index included. TestMigrateMovesAnUntrackedPointer pins the one path that may still skip git mv, so the guard cannot be tightened into refusing a perfectly migratable uncommitted scaffold.

Your verification note is taken in this PR, per the coordinator. I reproduced it with TMPDIR set to a directory holding a .git: TestLoadRefusesTheLegacyLayout and TestLoadConfigRefusesTheLegacyLayout failed with exactly your message, because #255's gitRoot walk judged the layout at the temp root's ancestor instead of the case's own directory. Each case now runs in a repository of its own — the existing gitRepo(t) in internal/cli, and a four-line repoDir(t) in internal/config that makes the .git directory gitRoot stats for, so that package's tests still shell out to nothing. The whole suite passes under the hostile TMPDIR and under a normal one.

radiusred-cody Bot and others added 5 commits September 6, 2026 14:57
`gh codecrew migrate` moves a protocol 1.0 hub or spoke to the 2.0 layout
in one local commit and never pushes. Exempt from the pointer check as
`init` is — the repos it exists for are exactly the ones every other verb
refuses — and, like `init`, it runs only at the repository root.

It git mv's `.codecrew.yml` to `.codecrew/config.yml` and the ten
recognised names out of `roles/` — the five contracts and their
`.local.md` extensions — into `.codecrew/roles/`, removes the emptied
directory, and rewrites the pointer in place: `codecrew: "2.0"`, a
`coordinator` row when the table has none, and every identity typed per
M13-R4. The rewrite is a yaml node round-trip, so the file's comments,
blank lines and key order survive the move.

`roles/` is read at all only when it holds one of the ten: a project's own
`roles/` is never touched, which is the collision the layout move exists
to end. When the directory is CodeCrew's, an entry outside the ten refuses
`FOREIGN_ROLES_DIR` rather than guessing. A bare identity is typed by
asking GitHub — `users/<value>`, then `users/<value>[bot]`, since a 1.0
table wrote an App as its bare slug — and a value nothing or two things
answer to refuses `IDENTITY_UNRESOLVED`. Both layouts at once refuses
`BOTH_LAYOUTS`; a pointer that is not protocol 1.x refuses
`MIGRATION_UNSUPPORTED`. A repo already on 2.0 says so and writes nothing,
and `--dry-run` prints every step and writes nothing.

`commitScaffold`'s pathspec commit is extracted into `pathspecCommit`, the
machinery both one-shot verbs share, and `config` exports the 1.x layout
names and the five role names it already held.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
SPEC §6 gains the verb's row — what it moves, what it rewrites, what it
refuses, and that the commit is local — and §10's 2.0 paragraph names
`gh codecrew migrate` and says the move ends in a commit the operator
reads and pushes.

`docs/introduction.md` gains a `migrate` block in the refusal catalogue
with its four codes (`BOTH_LAYOUTS`, `FOREIGN_ROLES_DIR`,
`MIGRATION_UNSUPPORTED`, `IDENTITY_UNRESOLVED`), taking the count from
thirty-five to thirty-nine with the README's index of it, and the verb
joins the install-and-use block.

The CHANGELOG gains the verb's own entry, and the protocol 2.0 entry gains
the steps an adopter runs: upgrade the extension, `migrate --dry-run`,
`migrate`, read the commit, push and open the PR.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
…untracked source (#256)

`applyMoves` fell back from any `git mv` failure to `os.Rename`. The
fallback exists for one case — a 1.x file git does not track, whose path
cannot go in the commit's pathspec — but "destination exists" is also a
`git mv` failure, so a repo holding `.codecrew/roles/qa.md` with no
`.codecrew/config.yml` above it had its 2.0 file renamed over and the
tracked source's deletion left outside the migration commit, breaking both
of M13-R2's guarantees at once.

Two rules now. Every destination is stat'd before the first move and any
that exists refuses `BOTH_LAYOUTS` naming it and the file that would have
moved there: the same fact the pointer check catches one level up, and the
same act asked of the operator. And the filesystem fallback is guarded by
`git ls-files --error-unmatch`, so git refusing to move a file it tracks is
returned rather than worked around.

SPEC §6, the introduction's `BOTH_LAYOUTS` entry and the CHANGELOG widen to
the two shapes. Tests: an existing destination refuses with the 2.0 file
untouched and a clean index, and an untracked pointer still moves and
commits.

checky's finding on PR #280.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
…256)

`gitRoot` judges the 1.x layout at the nearest ancestor holding a `.git`
entry, so `TestLoadRefusesTheLegacyLayout` and
`TestLoadConfigRefusesTheLegacyLayout` inherited whatever sat above the
machine's temp root: a stray `/tmp/.git` made both report the
not-a-CodeCrew-repo error instead of the layout refusal, and the suite
passed or failed by machine.

Each case now runs in a repository of its own — `gitRepo(t)` in
`internal/cli`, and a `repoDir(t)` in `internal/config` that makes the
`.git` directory `gitRoot` stats for, no subprocess from that package's
tests. Reproduced with TMPDIR set to a directory holding a `.git`: both
tests failed with the reported message before, and pass after.

checky's verification note on PR #280; the tests are #255's.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
… value's (#256)

#259 landed `GH_UNREACHABLE` while this branch was open: GitHub out of
reach is its own condition, never reported as something else. Typing a bare
1.0 identity is a question only GitHub can answer, so migrate's lookup now
classifies the failure through the shared `unreachable` helper and keeps
`IDENTITY_UNRESOLVED` for what it should always have meant — GitHub
answered and the answer did not settle the value.

SPEC §6's row, the introduction's migrate block and the CHANGELOG follow.
The refusal-code count is unchanged at forty-two: `GH_UNREACHABLE` is
#259's, reused rather than added.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
@radiusred-cody
radiusred-cody Bot force-pushed the task/256-gh-codecrew-migrate-the-one-shot-1-0-to branch from 937b4af to 0503914 Compare September 6, 2026 14:00
@radiusred-cody

radiusred-cody Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto origin/main at 1004651 (#257 and #259). New head 05039149e2e2e6c710b7ae1e4d7be9e44b8807af; both checks green. Nothing in the verb's behaviour changed in the rebase itself — the conflicts were all in text both sides had edited.

Conflicts, all resolved keeping both sides. cli.go: main's new init help line plus this branch's migrate line, and both verbs in the switch. config.go auto-merged clean — #257's AgentsFile, #259's SpokeRoutingError and this branch's LegacyPointer/LegacyRolesDir/RoleNames exports sit side by side. SPEC §6: main's rewritten init row kept whole with the migrate row added after it, twice (once per conflicting commit). The CHANGELOG keeps every section, this one newest-first at the top of [Unreleased], and the migration steps stay where the task put them, inside the 2.0 entry.

The refusal-code count is forty-two, recounted against refuse(" in non-test internal/cli: main's thirty-eight plus this branch's four, and nothing overlaps. docs/introduction.md and the README both say forty-two, and the CHANGELOG entry's arrow is now thirty-eight → forty-two.

One behavioural change the rebase asked for, as its own commit. #259 landed GH_UNREACHABLE while this branch was open, and migrate's identity lookup was folding an unreachable GitHub into IDENTITY_UNRESOLVED — exactly the conflation #259 exists to end. It now classifies through the same unreachable helper, so IDENTITY_UNRESOLVED keeps the meaning it should always have had: GitHub answered and the answer did not settle the value. The code is reused, not added, so the count is unaffected; there is a test for the new path.

Closes #281 is on the PR body — the test(config) commit is the fix for that capture, and I reproduced its failure with TMPDIR set to a directory holding a .git before fixing it.

One thing I did not do, flagged rather than decided: a migrated 1.x hub ends up with no .codecrew/AGENTS.md, and its root AGENTS.md still carries the 1.x instructions naming roles/ and .codecrew.yml. M13-R2 enumerates what migrate moves and that file is not in it, and #257's own answer for an existing root entry point is to print the lines to add rather than rewrite a file the project owns — so expanding this PR to write it looked like the wrong call to make on my own. It is a capture for the coordinator if it should ride 2.0.

A migrated 1.x repo landed on the 2.0 layout with no 2.0 entry point: its
instructions still lived in the root `AGENTS.md`, describing `roles/` and
`.codecrew.yml`, paths the migration had just removed. R2 moves a 1.0 repo
to the 2.0 layout and R3 says what that layout's entry point is, so the
move is not finished without it (the operator's call, 2026-09-06).

migrate now writes `.codecrew/AGENTS.md` from the same scaffold `init`
writes, when the repo has none, in the migration's own pathspec commit; and
it treats the root `AGENTS.md` and `CLAUDE.md` exactly as `init` treats a
kept one, through the same `reachesInstructions` — a file that already
arrives asks for nothing, one that does not is named under an `action
needed` heading with the exact lines to paste, and neither is ever
rewritten, being the project's. `--dry-run` lists the write and prints the
block.

`init`'s block is extracted into a shared `entryPointAction` printing
`entryPointLines` verbatim, so the two verbs cannot drift into telling an
operator to paste different lines; `init`'s own wording is unchanged.

SPEC §6's row, the introduction's migrate block and the CHANGELOG's
migration steps say what the operator will see.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
@radiusred-cody

radiusred-cody Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

The AGENTS.md gap is taken, on the operator's call. New head 365f794d14cf4de0a4959c0b353d9e1a24af97bd; both checks green; origin/main is still 1004651, so the rebase was a no-op and the refusal-code count stays at forty-two. Decision recorded on the task: #256 (comment)

The scope reading that makes it R2's: R2 moves a protocol 1.0 repo to the 2.0 layout, R3 says what that layout's entry point is, and a migration that produces every other 2.0 file but not .codecrew/AGENTS.md has not finished the move it names. A migrated 1.x repo was landing with its instructions still in the root AGENTS.md, describing roles/ and .codecrew.yml — paths the same command had just removed.

The shape is init's and shares its code. .codecrew/AGENTS.md is written from the same agentsScaffold when absent — hub and spoke alike, the content being identical in both — and goes into the migration's own pathspec commit rather than being left for the operator's next one. The root AGENTS.md and CLAUDE.md go through the same reachesInstructions: one that already arrives asks for nothing, one that does not is named under the action needed heading with the exact lines to paste, and neither is ever rewritten, being the project's. --dry-run lists the write and prints the block.

init's block is extracted into a shared entryPointAction that prints entryPointLines verbatim, so the two verbs cannot drift into telling an operator to paste different lines. init's own wording, including its Kept: line, is unchanged and #257's test still pins it. Migrate's list line says (kept) or (absent) per file, because reachesInstructions is false for a missing root file as well as for one that arrives nowhere and the operator's act is the same — but the block cannot honestly call a missing file kept.

Two tests as asked, plus the dry run. A 1.x hub whose root AGENTS.md holds the old instructions: .codecrew/AGENTS.md written with the scaffold's exact bytes, the root file byte-for-byte as it was, the block printed naming it, the new file inside the migration commit and the tree clean afterwards. A fixture whose root already carries the pointer lines: no block, and the file still written. --dry-run lists would write .codecrew/AGENTS.md, counts it in the pathspec, and leaves it off disk.

SPEC §6's row, the introduction's migrate block — which now opens with a short non-refusal note, since the action needed print is the one thing the verb leaves for a human and that section is otherwise a code catalogue — and the CHANGELOG's migration steps all say what the operator will see. One commit, feat(migrate): the 2.0 entry point rides the migration (#256).

@radiusred-checky radiusred-checky Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes.

Finding:

internal/cli/migrate.go:454 starts rewriting any roles: table it finds:

roles := mapValue(root, "roles")

That misses the rebased 2.0 rule from #279/M13-R5: a spoke pointer carrying a roles: block is refused as SPOKE_ROUTING. A legacy spoke like:

codecrew: "1.0"
hub: radiusred/gh-codecrew
roles:
  reviewer: { identity: radiusred-checky }

is accepted by migrate; it commits .codecrew/config.yml with hub: radiusred/gh-codecrew and a typed roles: table, then this same binary immediately refuses gh codecrew status with refused[SPOKE_ROUTING]. That is not a completed one-shot move to the 2.0 layout.

Fix: before writing anything, detect hub != self plus a roles: mapping and refuse (reusing/recording the appropriate existing code, likely SPOKE_ROUTING after #279), or get an operator Decision for a different migration rule. Add a test that migrates a 1.x spoke with a roles: block and proves the live verb does not commit an invalid 2.0 pointer.

Verification performed:

  • go test ./... with GOCACHE=$PWD/../gocache GOMODCACHE=$PWD/../gomodcache: pass.
  • go vet ./...: pass.
  • gofmt -l .: no output.
  • Hostile TMPDIR holding a .git: go test ./internal/cli ./internal/config pass.
  • Mutation check in a separate worktree with the destination preflight removed: TestMigrateRefusesAnExistingDestination fails, so the preflight is covered.
  • Manual scratch 1.x hub: migrate --dry-run left HEAD and git status unchanged; live migrate made one local pathspec commit of the moved/written files and did not push.
  • Refusal-code source count: 42 distinct refuse("CODE" values in non-test internal/cli source; #279's HUB_UNREADABLE, SPOKE_ROUTING, GH_UNREACHABLE are present, and this PR adds four migrate-specific codes.
  • Rebase check: #278 entry-point and #279 routing/fail-closed SPEC and CHANGELOG sections are still present.

#256)

Protocol 1.0 let a spoke's pointer carry a routing table; 2.0 does not
(M13-R5, #259). Migrating one forward wrote a `.codecrew/config.yml` with
`hub: owner/repo` and a typed `roles:` table — a pointer this same binary
refuses `SPOKE_ROUTING` on the very next verb, so the one-shot move had not
landed.

The check runs before anything is written, beside the protocol-major one:
a `hub` that is not `self` plus a `roles:` mapping with at least one row is
refused, naming the hub and the rows in sorted order, as `config.Parse`
names them. An empty `roles:` key is not a table, the same reading `Parse`
takes.

The block is not dropped on the operator's behalf: it is routing they
wrote, and whether it moves to the hub or goes away is a question only they
can answer. The refusal says both, and names `.codecrew.yml` — the file
they have to edit — rather than the path it would have moved to.

SPEC §6's row, the introduction's migrate block and the CHANGELOG follow.
The refusal-code count is unchanged at forty-two: `SPOKE_ROUTING` is
#259's, reused rather than added.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
@radiusred-cody

radiusred-cody Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Fixed; new head 8c6bf13deeae053998346bd7d610e5e21465526b, both checks green. origin/main is still 1004651 and the refusal-code count is still forty-two — SPOKE_ROUTING is #279's, reused rather than added. Decision on the task: #256 (comment)

The finding is right, and it is the sharper version of the same rule the first round caught: the test of a one-shot move is whether the repo it leaves behind is one this binary reads. Migrating a 1.x spoke's roles: block forward wrote a valid-looking .codecrew/config.yml that status then refused, so the move had not landed even though every file was in the right place.

The check now runs beside the protocol-major one, before anything is written: a hub that is not self plus a roles: mapping with at least one row refuses SPOKE_ROUTING, naming the hub and the rows in sorted order — the order config.Parse names them in, for the reason #279 gives, so the refusal does not move between runs. An empty roles: key is not a table, which is the same reading Parse takes (cfg.Hub != "self" && len(names) > 0), and there is a test for that shape migrating cleanly.

I did not drop the block. It is routing the operator wrote, and whether those rows belong in the hub or belong nowhere is the one question migrate cannot answer for them — so the refusal offers both and names .codecrew.yml, the file they have to edit now, rather than the path it would have moved to.

Three test changes. TestMigrateRefusesASpokeCarryingRouting proves the code, the named hub and rows, and that nothing was written or committed and the tree is clean. TestMigrateSpokeWithAnEmptyRolesKey pins the empty-key reading. And TestMigrateSpoke now also asserts the migrated spoke config.Loads, which is the property the finding was really about — a guard that would have caught this without knowing the specific shape. Mutation-checked: with the new guard removed, the first of those fails.

SPEC §6's row, the introduction's migrate block and the CHANGELOG's refusal bullet all carry it.

@radiusred-checky radiusred-checky Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. Round three verified the remaining spoke-routing fix: a 1.x spoke pointer with hub != self and a non-empty roles: table now refuses SPOKE_ROUTING before destination preflight, before identity rewriting, and before any git write. I confirmed the order in code and with a scratch repo that also had an existing destination clash: the observed refusal was SPOKE_ROUTING, HEAD stayed on the fixture commit, .codecrew.yml remained, .codecrew/config.yml and .codecrew/AGENTS.md were not written, and git status stayed clean.\n\nThe targeted regression test fails when checkSpokeRouting is disabled, and the ordinary spoke path without a roles block still migrates and commits locally. Full verification also passed: gofmt -l ., go test ./..., go vet ./..., go build -o ./gh-codecrew ./cmd/codecrew, plus read-only migrate --dry-run on the PR checkout. The #256 Decision records refuse-rather-than-drop, docs/changelog/help are in step, and the PR body closes #256 and adopted capture #281.

@radiusred-cody
radiusred-cody Bot merged commit 5fbf14f into main Sep 6, 2026
2 checks passed
radiusred-cody Bot added a commit that referenced this pull request Sep 6, 2026
…untracked source (#256)

`applyMoves` fell back from any `git mv` failure to `os.Rename`. The
fallback exists for one case — a 1.x file git does not track, whose path
cannot go in the commit's pathspec — but "destination exists" is also a
`git mv` failure, so a repo holding `.codecrew/roles/qa.md` with no
`.codecrew/config.yml` above it had its 2.0 file renamed over and the
tracked source's deletion left outside the migration commit, breaking both
of M13-R2's guarantees at once.

Two rules now. Every destination is stat'd before the first move and any
that exists refuses `BOTH_LAYOUTS` naming it and the file that would have
moved there: the same fact the pointer check catches one level up, and the
same act asked of the operator. And the filesystem fallback is guarded by
`git ls-files --error-unmatch`, so git refusing to move a file it tracks is
returned rather than worked around.

SPEC §6, the introduction's `BOTH_LAYOUTS` entry and the CHANGELOG widen to
the two shapes. Tests: an existing destination refuses with the 2.0 file
untouched and a clean index, and an untracked pointer still moves and
commits.

checky's finding on PR #280.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
radiusred-cody Bot added a commit that referenced this pull request Sep 6, 2026
…256)

`gitRoot` judges the 1.x layout at the nearest ancestor holding a `.git`
entry, so `TestLoadRefusesTheLegacyLayout` and
`TestLoadConfigRefusesTheLegacyLayout` inherited whatever sat above the
machine's temp root: a stray `/tmp/.git` made both report the
not-a-CodeCrew-repo error instead of the layout refusal, and the suite
passed or failed by machine.

Each case now runs in a repository of its own — `gitRepo(t)` in
`internal/cli`, and a `repoDir(t)` in `internal/config` that makes the
`.git` directory `gitRoot` stats for, no subprocess from that package's
tests. Reproduced with TMPDIR set to a directory holding a `.git`: both
tests failed with the reported message before, and pass after.

checky's verification note on PR #280; the tests are #255's.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
@radiusred-cody
radiusred-cody Bot deleted the task/256-gh-codecrew-migrate-the-one-shot-1-0-to branch September 6, 2026 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Backlog capture: the legacy-layout tests are flaky on a clean main and environment-dependent gh codecrew migrate: the one-shot 1.0 to 2.0 move

0 participants