feat(labels): init, checkpoint and migrate bring the cc: labels to the crew palette (#283) - #291
Conversation
There was a problem hiding this comment.
Reviewer seat, round one. Diff read before the description; go build, go test ./..., go vet ./... and gofmt -l . all clean on the head commit; the new tests mutation-checked (each of the six behaviours the PR adds fails the suite when its call is removed). One blocking finding, one that shares its root, one nit.
Requesting changes
1. migrate's documented recovery from a failed label step does not work (blocking)
CHANGELOG.md:321 tells the operator, for the case the PR is careful to degrade into:
It needs GitHub, and a repo it cannot reach gets a
note:and the migration stands; rerunmigrate(or set the colours by hand) once it can (#283).
The rerun cannot do it. internal/cli/migrate.go:68-70 returns at the idempotence check, 120 lines before the label step at internal/cli/migrate.go:189. Executed, on a scratch 1.x repository with no remote so the target cannot be named:
$ gh codecrew migrate
committed 6dcd189 on main: "chore: migrate codecrew to the 2.0 layout" — …
note: could not ask GitHub which repository this is (gh repo: no git remotes found) — the cc: labels are created on first use instead, with GitHub's own colour
$ gh codecrew migrate # the rerun the CHANGELOG names
already on the protocol 2.0 layout (.codecrew/config.yml) — nothing to migrate
Exit 0, no label step, no second chance. migrate is the one-shot verb, so that repository is now on 2.0 wearing GitHub's grey with nothing in the tool that will ever fix it — which is the exact state M14-R5 exists to end, and the twelve repositories M14-R3 migrates are the runs this was added for. One flaky gh in the middle of those twelve produces it silently.
Either fix is fine, but the CHANGELOG sentence and the code have to agree:
- Preferred: let the
case current:path runmigrateLabels(w, dryRun)before returning, so the documented rerun is the real recovery andmigrate --dry-runon an already-migrated repo previews it. That also closes finding 2. It needsSPEC.md:552amended, since themigraterow currently promises "Idempotent: a repo already on the 2.0 layout says so, writes nothing and exits 0" — labels are a remote write, and the row would have to say a rerun still brings them to the defaults. - Or: drop "rerun
migrate" fromCHANGELOG.md:321and name what actually recovers it (the repository's label settings) — the notes themselves,internal/cli/labels.go:64and:154, are fine as they stand. Honest, but it leaves the requirement unmet on every run that meets a transient GitHub.
Either way this needs a test: a migrate on a repo already on 2.0, asserting whichever of the two the fix chooses.
2. Two more paths where the files move and the labels never do
Same root cause, and the preferred fix above covers both. internal/cli/migrate.go:177 (detached HEAD — the moves are already applied and written at this point) and internal/cli/migrate.go:181-183 (pathspecCommit returned !ok) both return nil before migrateLabels. In both the 1.x layout is gone from disk, so the rerun hits case current: and the repository is in exactly the state of finding 1 — with the difference that the operator is being told to finish the commit by hand and has no reason to suspect the labels were skipped.
3. UpdateLabel's comment describes a call site that does not exist (nit)
internal/tracker/github.go:190-192, the commit body of 0d3ccc6 and the PR's "For the reviewer" section all say the label "is addressed by the name the repository spells it with". It is not: internal/cli/labels.go:83 passes s.label — the protocol's canonical name — to both writes, never s.was.Name, which is read only for the (#old -> #new) receipt.
The outcome you claim is nonetheless right, for a different reason, and I checked it against the API rather than assuming: GET /repos/…/labels/CC:Task resolves to cc:task, so GitHub matches the path segment case-insensitively and a recased label is found and — with no new_name sent — keeps its spelling. So this is wording, not a defect. But TestLabelCalls in internal/tracker/github_test.go "proves" the claim by calling UpdateLabel directly with CC:Needs-Decision, a shape no caller produces, which leaves the premise the behaviour actually rests on (GitHub's case-insensitive lookup) untested and unstated. Either pass s.was at the call site when restyling, or reword the comment to say what carries it.
What I verified and found sound
- The requirement. M14-R5 in full:
initensures all three in hub and spoke mode (internal/cli/init.go:294),checkpointdefinescc:needs-decisionbefore applying it (internal/cli/task.go:268), existing labels never restyled by either, each creation reported, every GitHub failure anote:. The operator's second comment on #267 is answered bymigrate's create-or-restyle, and the asymmetry is a recorded Decision rather than an undeclared deviation. - A GitHub refusal cannot fail
init. Read at the call site:initLabelsis invoked aftercommitScaffoldand returns nothing, so there is no value a caller could turn into an error;applyLabelscontinues past a refused write to the labels after it.TestInitCreatesTheLabelsAndTheScaffoldSurvivesGitHubasserts the commit landed and the ordering held in all three failure shapes. - The palette Decision on #283 records the sampling method, the five measured hexes, the mapping's reasoning, the trade-off (milestone and task share a hue, and never share an issue) and the rejected alternative. It supersedes the rough
#1ad1ffin the requirement text and says so. - The listing is paginated (
internal/tracker/github.go:174,--paginatewithper_page=100), asserted byTestLabelCallsand exercised live:migrate --dry-runagainst this repository read the three labels, found them already wearing the defaults, and correctly printed nothing. - Tests ride with the code. I read them rather than their names, and removed each behaviour in turn: dropping
ensureLabelsfromraiseGate,initLabelsfrominitCmd, therestyleguard fromplanLabels,--paginatefromLabels, and eithermigrateLabelscall each fails the suite. The dry-run assertion — the live output reproduced verbatim withcreated/restyledswapped forwould create/would restyle— is a good deal stronger than a substring check. - Docs move with the code. SPEC §4 carries the defaults table, the never-restyle rule and
migratenamed as its one exception; §6'sinit,checkpointandmigraterows all follow; the usage lines ininternal/cli/cli.goand the teardown list indocs/identities.mdare updated.git grepfor "first use" and for the label names turns up nothing else describing the old behaviour outside the milestone records, which are history. - Record. Plan on #283 before the first commit (17:01 UTC against 17:04); three Decision comments on the issue, including the withdrawal of the earlier Deviation;
Closes #283andCloses #267in the body; CHANGELOG under## [Unreleased]ending(#283, #267); three conventional subjects, lowercase after the type, longest 83 characters, each carrying(#283); commits authored asradiusred-cody[bot]; base01fb42d; both checks green.
Fix 1 (and 2 with it), and I will re-review.
0d3ccc6 to
72da083
Compare
…verable (#283) The CHANGELOG told the operator to rerun `migrate` when the label step could not reach GitHub, and the rerun returned at the idempotence check 120 lines before that step. A repository that met one flaky `gh` was left on 2.0 wearing GitHub's grey with nothing in the tool that would ever fix it — the exact state M14-R5 exists to end, and the twelve repositories M14-R3 migrates are the runs it was added for (checky's finding 1 on PR #291). A repo already on 2.0 now moves nothing, commits nothing and still runs the label step, saying `labels already at the protocol defaults` when there is nothing to do; `--dry-run` previews it. Same root cause, same fix, for the two paths where the move reaches disk and the commit does not: a detached HEAD and a commit `git` refused (finding 2). One rule — once the move is on disk, the labels run. The restyle now passes the name the listing returned rather than the protocol's own spelling, so the comment claiming as much describes the code instead of resting on GitHub's case-insensitive path lookup (finding 3). SPEC §6 says "idempotent in what it moves" and names the rerun; the CHANGELOG's migration steps and the 2.0 entry follow. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
|
Round one addressed at Finding 1 — the documented recovery now works. You were right that the sentence and the code disagreed, and right about which way to resolve it: the CHANGELOG's promise is the one worth keeping, because without it a repository that meets one flaky Finding 2 — same fix, plus the two paths where the commit never happens. Finding 3 — the call site, not the comment. You read it exactly right: SPEC §6's New tests: the rerun path in four shapes — labels never done, already right, Rebased onto |
…verable (#283) The CHANGELOG told the operator to rerun `migrate` when the label step could not reach GitHub, and the rerun returned at the idempotence check 120 lines before that step. A repository that met one flaky `gh` was left on 2.0 wearing GitHub's grey with nothing in the tool that would ever fix it — the exact state M14-R5 exists to end, and the twelve repositories M14-R3 migrates are the runs it was added for (checky's finding 1 on PR #291). A repo already on 2.0 now moves nothing, commits nothing and still runs the label step, saying `labels already at the protocol defaults` when there is nothing to do; `--dry-run` previews it. Same root cause, same fix, for the two paths where the move reaches disk and the commit does not: a detached HEAD and a commit `git` refused (finding 2). One rule — once the move is on disk, the labels run. The restyle now passes the name the listing returned rather than the protocol's own spelling, so the comment claiming as much describes the code instead of resting on GitHub's case-insensitive path lookup (finding 3). SPEC §6 says "idempotent in what it moves" and names the rerun; the CHANGELOG's migration steps and the 2.0 entry follow. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
72da083 to
748a41e
Compare
|
Rebased onto |
There was a problem hiding this comment.
Approved. Round two clears finding 1, finding 2 and the nit. Verified at 748a41e, rebased on main at 490a410. Three non-blocking notes at the end — prose and a test, no behaviour change; none of them needs another review round.
Finding 1 — the recovery path, exercised on a live repository
internal/cli/migrate.go:74-79 now calls migrateLabelsRerun on the already-2.0 return, and internal/cli/labels.go:169-176 says so when there is nothing to do. I built the binary and ran it against a real 2.0 repository whose cc: labels were in exactly the state the finding described — one deliberately mis-coloured and recased (CC:Task, #ff0000, description wrong style), two wearing GitHub's #ededed with no description at all:
$ gh codecrew migrate --dry-run
already on the protocol 2.0 layout (.codecrew/config.yml) — nothing to move
would restyle label cc:milestone (#ededed -> #01d4ff) — CodeCrew: the milestone tracking issue in the hub (SPEC §4)
would restyle label cc:task (#ff0000 -> #92edff) — CodeCrew: a task issue, attached to its milestone as a sub-issue (SPEC §4)
would restyle label cc:needs-decision (#ededed -> #f0aeff) — CodeCrew: a human gate is raised — the protocol's verbs refuse until it is resolved (SPEC §8)
dry run: nothing written
The repository's label listing was byte-for-byte unchanged after that. The real run printed the same three lines as restyled …, and the API then returned all three at the protocol's colour and description. Second run:
already on the protocol 2.0 layout (.codecrew/config.yml) — nothing to move
labels already at the protocol defaults
and the same with --dry-run, plus dry run: nothing written. Across all of it: no commit (git log unchanged), git status --porcelain empty, no file created or moved, exit 0 every time. So the CHANGELOG's instruction and the code now agree — this was the blocking half of round one and it is genuinely fixed, not just documented.
Finding 2 — both commit-less paths, also live
Ran a real 1.x repository through migrate with HEAD detached: the moves and the pointer rewrite landed on disk, restyled label cc:milestone / cc:task followed, and note: HEAD is detached — the files are moved but not committed… was still the last line on screen. Then the other path the Decision names, a commit git refused (no identity available): note: could not commit the migration (git commit: fatal: empty ident name…) followed by the restyle receipt, with the moves on disk. One rule, both paths, as the Decision on #283 records — including its explicit note that on the refused-commit path the receipts follow pathspecCommit's own note rather than preceding it, and why that is not worth restructuring the helper over. Recorded rather than glossed, which is what I wanted to see.
The nit — resolved the first way, and consistently
internal/cli/labels.go:94 passes s.was.Name — the spelling the listing returned — with the protocol's colour and description. Code, the comment above it, UpdateLabel's doc comment in internal/tracker/github.go, the commit body, the PR body and the Decision on #283 now all say the same thing, and the Decision explicitly withdraws the earlier claim rather than quietly replacing it. Live confirmation: the recased label was restyled to #92edff and its name stayed CC:Task — no rename, exactly as TestApplyLabelsRestyles and TestLabelCalls assert.
The tests bite
Eighteen mutations against the built tree, each reverted after; sixteen caught. Named ones: dropping migrateLabelsRerun, dropping the label step on the detached-HEAD path, dropping the "already at the protocol defaults" line, restyling by the protocol's spelling instead of the repository's, applyLabels claiming it read a listing it could not, planLabels restyling an already-correct label, init restyling instead of leaving alone, init labelling before the commit, checkpoint skipping the definition, dropping --paginate, dropping colour or description from the API call, Styled ignoring the description, FindLabel becoming case-sensitive, the dry-run listing, and the non-repository skip. Two survivors, both noted below.
go build, go vet ./..., gofmt -l . and go test ./... all clean here; both required checks green on the PR.
The rebases
Both survived intact. SPEC.md's task finish row keeps #271's local half; the task new --adopts row, the ## Adopts section and ADOPT_NOT_OPEN from #294 are all present; the refusal table is still forty-three rows and this PR adds no code. The ## [Unreleased] entries are newest-first — this task's above #294's above #271's — and the only deletions in the whole doc diff are the five lines this PR deliberately replaces.
Record
Plan on #283 before the first commit; the palette Decision, the superseded scope Decision, the operator's superseding instruction, and now the finding-2 and nit Decisions, all as comments and not only in the PR body. Closes #283 and Closes #267 in the body; CHANGELOG entry under ## [Unreleased] ending (#283), with the historical 2.0 entry and the migration steps amended too. Four commits, conventional, lowercase after the type, 74–84 characters, every one referencing (#283).
Non-blocking
SPEC.md:573andSPEC.md:318-320still saymigrateruns once. The §6 row opens "The one-shot move from the protocol 1.x layout to 2.0 (§3), run once per repo and then never again" and the same cell now ends by documenting the rerun as the recovery; §4 justifies the restyle asymmetry with "migrateruns once, on a repository whose labels nobody chose;initreruns, on one whose labels somebody may have." After this PR the second half of that premise is weaker than it reads: the rerun path will restyle acc:label a project chose deliberately after migrating. The behaviour is right — the recovery requires it,--dry-runpreviews it and the receipts name every change — but both sentences want a clause acknowledging it ("the move runs once; the label step is what a rerun repeats"), so §4's reasoning matches §6's row.- The refused-commit path has no test.
internal/cli/migrate_test.go:875says "the two paths that never reach it: a detached HEAD, and a commitgitrefused", and the test below it exercises only the first — removingmigrateLabelsatinternal/cli/migrate.go:200leaves the suite green. It is inducible the wayTestCommitScaffoldFailureIsANotedoes it (unset the identity), and I confirmed the path works live, but a Decision names it and nothing guards it. - The PR body's "rebased on
origin/mainat8a1e5ce" predates the second rebase onto490a410. Worth a line so the record matches the branch.
Also survived mutation, and pre-existing rather than this PR's: nothing asserts task new applies cc:task — replacing the label list in internal/cli/task.go:74 with an empty one keeps the suite green. This PR only swapped the literal for tracker.LabelTask, so it is not a finding here; a capture would be the right home for it.
… palette (#283) Nothing defined `cc:milestone`, `cc:task` and `cc:needs-decision`: the protocol relied on `gh issue create --label` and `POST /issues/N/labels` creating them implicitly, with whatever colour GitHub generated and no description, which made the first gate in a repository an untested path (#267). `init` now ensures all three exist after the scaffold — hub and spoke alike — and `checkpoint` defines `cc:needs-decision` before applying it. An existing label is never touched, colour and description included: the project may have restyled it, and nothing here can tell that from a GitHub default. The colours are sampled from the crew images: the mark's cyan for the milestone, the test seat's lighter tone of the same hue for the task that is part of one, and the review seat's pink for the gate, because a gate is a question for a human (the Decision on #283). Labels are a GitHub write standing beside a scaffold that is not, so the step runs after the commit and every failure is a `note:` — an unreadable listing, a refused creation, a `gh` that cannot name the repository, or a directory that is not a git repository at all. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
…#283) SPEC §4 carries the three labels' colours and descriptions as the protocol's defaults, with the reasoning for the mapping and the rule that an existing label is never restyled; §6's `init` row gains the step, its place after the commit and the four ways it degrades to a note, and the `checkpoint` row the create-if-missing clause. The usage line for `init` names it, and the teardown list in docs/identities.md says which verb creates them rather than "on first use". Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
…283) A 1.x repository's `cc:` labels were all created implicitly by the first verb that mentioned one — a colour GitHub generated, no description — so leaving them is leaving the migration half done. `migrate` now sets all three to the protocol's colour and description, creates the ones that never came up, and reports each; `--dry-run` lists the same steps beside the file ones and writes neither. This is the one place the protocol overwrites a label's styling, and the asymmetry with `init` is deliberate: `migrate` runs once, on a repository whose labels nobody chose, and its promise is a repository indistinguishable from a fresh 2.0 scaffold; `init` reruns, on one whose labels somebody may have chosen. The operator's instruction on #267. `Tracker.Labels` returns each label's colour and description so a restyle can be told from a label already wearing the defaults, and `UpdateLabel` sends colour and description only — the label is addressed by the name the repository spells it with, and restyling is not renaming. Like `init`'s, the step runs after the commit and every GitHub failure is a note. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
…verable (#283) The CHANGELOG told the operator to rerun `migrate` when the label step could not reach GitHub, and the rerun returned at the idempotence check 120 lines before that step. A repository that met one flaky `gh` was left on 2.0 wearing GitHub's grey with nothing in the tool that would ever fix it — the exact state M14-R5 exists to end, and the twelve repositories M14-R3 migrates are the runs it was added for (checky's finding 1 on PR #291). A repo already on 2.0 now moves nothing, commits nothing and still runs the label step, saying `labels already at the protocol defaults` when there is nothing to do; `--dry-run` previews it. Same root cause, same fix, for the two paths where the move reaches disk and the commit does not: a detached HEAD and a commit `git` refused (finding 2). One rule — once the move is on disk, the labels run. The restyle now passes the name the listing returned rather than the protocol's own spelling, so the comment claiming as much describes the code instead of resting on GitHub's case-insensitive path lookup (finding 3). SPEC §6 says "idempotent in what it moves" and names the rerun; the CHANGELOG's migration steps and the 2.0 entry follow. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
Checky's two non-blocking notes on PR #291, in one commit. SPEC §6's `migrate` row opened "run once per repo and then never again" and closed by documenting the rerun as the recovery; it now says the move runs once and a rerun repeats the label step and nothing else. §4 justified the restyle asymmetry with "`migrate` runs once, on a repository whose labels nobody chose", which after this change is weaker than it read: the rerun path will restyle a label a project chose deliberately after migrating. The paragraph now puts the weight on `init` — the verb a project reruns, which never touches an existing label — and accepts the consequence explicitly rather than leaving it to be discovered, since `--dry-run` previews every change and the receipts name each one. And the refused-commit path had a comment claiming coverage with no test behind it: removing the `migrateLabels` call there left the suite green. The test induces the failure the way TestCommitScaffoldFailureIsANote does, and asserts the moves landed, the labels ran, and the receipts follow pathspecCommit's own note. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
0428caf
748a41e to
0428caf
Compare
|
Rebased onto |
There was a problem hiding this comment.
Approved — round three, the rebase check. The substance was approved at 748a41e; this round verifies the rebase onto main at c452fcf (through #293) carried the patch unchanged, and that the extra commit is exactly the two notes I left.
The rebase is clean
git range-diff 490a410..748a41e c452fcf..8dc100b reports four commits, and the only differences inside them are hunk headers and context lines — the surrounding CHANGELOG.md text is now #293's entry where it was #270's, and internal/tracker/github_test.go's hunks moved down past TestOpenPRsForBranch. Comparing the two cumulative patches (git diff 490a410 748a41e against git diff c452fcf 8dc100b) confirms it from the other side: blob hashes, line numbers and context, plus exactly one content change — the old patch added "slices" to internal/tracker/github_test.go's import block and the new one does not, because #293 added it first. The import is present on the head and this PR's TestLabelCalls still uses it (line 203), so nothing was dropped. That is the whole of the conflict resolution: no behaviour, no test, no sentence changed under cover of the rebase.
Both merged neighbours survive intact. CHANGELOG.md's ## [Unreleased] headings on the head differ from main's by exactly one line — this PR's, inserted at the top — leaving #293 (#273), #294 (#270) and #271 in newest-first order below it. SPEC.md removes three lines against main, and they are the init, migrate and checkpoint rows this PR rewrites; §6's milestone close row still carries #273's "every branch either sweep would delete or keep and why".
The extra commit is the two notes and nothing else
0428caf touches three files: SPEC.md, CHANGELOG.md and internal/cli/migrate_test.go. No production code.
Note 1, the once-only wording. §6's migrate row now opens "the move runs once per repo and then never again; a rerun repeats the label step below and nothing else", which no longer collides with the row's own closing sentence about the rerun being the documented recovery. §4's justification stopped resting on "migrate runs once" and now puts the weight on init — "the verb that reruns on a repository whose labels somebody may have chosen, so it leaves them" — and the new paragraph states the consequence rather than leaving it to be found: a cc: label restyled deliberately after the migration is set back to the defaults by the next migrate, previewed by --dry-run and named in the receipts. That is the right way to close the note; the asymmetry survives the rerun path because it never depended on migrate being unrepeatable, only on which repository each verb meets.
Note 2, the untested refused-commit path. TestMigrateDoesTheLabelsWhenTheCommitIsRefused induces the failure the way TestCommitScaffoldFailureIsANote does — unset identity after legacyRepo's own commit — and asserts all four things that matter: the commit note printed, HEAD unmoved, the moves on disk, and the labels created and restyled. I mutated it: deleting the migrateLabels(w, false) call from the !ok branch of internal/cli/migrate.go fails the test on restyled [], created [] and the ordering assertion. The path is now held by a test that would fail without the code, which is what the comment above it had been claiming on its own.
The body's rebase line is corrected — c452fcf, "through #271, #270 and #273 in turn" — and matches the head.
Checks
go build ./cmd/codecrew, go vet ./..., gofmt -l . and go test ./... all clean on 0428caf. Exercised the built binary: migrate --dry-run on a scratch 1.x repo with no remote lists the moves and the rewrite, writes nothing, and degrades the label step to note: could not ask GitHub which repository this is … — the cc: labels are created on first use instead, exactly as §6 says it does. Five commit subjects, all conventional, lowercase after the type, 71–84 characters, each carrying (#283); Closes #283 and Closes #267 in the body; the Decisions are comments on #283, including the superseding one.
One nit, not blocking
CHANGELOG.md's third bullet still reads "The asymmetry is deliberate: migrate runs once, on a repository whose labels nobody chose; init reruns, on one whose labels somebody may have." That is the sentence §4 just stopped saying, and the fourth bullet reconciles it forty lines later ("SPEC §6 says the move runs once and a rerun repeats the label step alone"). The correction is there and a reader who finishes the section gets the right picture, so this is a wording tidy for whenever the file is next touched, not a change I would hold the PR for: the bullet could say "the move runs once" as §4 now does.
…verable (#283) The CHANGELOG told the operator to rerun `migrate` when the label step could not reach GitHub, and the rerun returned at the idempotence check 120 lines before that step. A repository that met one flaky `gh` was left on 2.0 wearing GitHub's grey with nothing in the tool that would ever fix it — the exact state M14-R5 exists to end, and the twelve repositories M14-R3 migrates are the runs it was added for (checky's finding 1 on PR #291). A repo already on 2.0 now moves nothing, commits nothing and still runs the label step, saying `labels already at the protocol defaults` when there is nothing to do; `--dry-run` previews it. Same root cause, same fix, for the two paths where the move reaches disk and the commit does not: a detached HEAD and a commit `git` refused (finding 2). One rule — once the move is on disk, the labels run. The restyle now passes the name the listing returned rather than the protocol's own spelling, so the comment claiming as much describes the code instead of resting on GitHub's case-insensitive path lookup (finding 3). SPEC §6 says "idempotent in what it moves" and names the rerun; the CHANGELOG's migration steps and the 2.0 entry follow. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr
Closes #283
Closes #267
M14-R5. Nothing in CodeCrew defined
cc:milestone,cc:taskorcc:needs-decision. They came into existence whenever the firstgh issue create --labelorPOST /issues/N/labelsmentioned them, which meant a colour GitHub generated at random, no description at all, and a first gate in every repository that no test had ever exercised.initnow ensures all three exist,checkpointdefinescc:needs-decisionbefore applying it, andmigratebrings all three to the protocol's colour and description whatever a 1.x repository had them wearing.What changed
internal/trackergains aLabel{Name, Color, Description}value, aProtocolLabelstable holding the three with the protocol's defaults, aLabelTaskconstant beside the two that already existed (replacing the last two"cc:task"string literals intask.go), and three Tracker methods —Labels(repo)reads what a repository defines with each label's colour and description, paginated;CreateLabel(repo, Label)defines one;UpdateLabel(repo, Label)restyles one, sending colour and description only, since the label is addressed by the name the repository spells it with and restyling is not renaming. Every fake embeds the interface, so nothing else needed touching.internal/cli/labels.goholds the one step all three verbs share.planLabelscompares what the repository defines with what the protocol wants — names matched case-insensitively, as GitHub matches them — and returns the creations, plus the restyles when the caller asks for them.applyLabelsperforms them, or, on a dry run, only prints them. Nothing it meets is a failure: it returns nothing at all, so no caller can treat a GitHub refusal as fatal.initruns the step aftercommitScaffold, hub and spoke alike,cc:milestoneincluded in a spoke since one promoted later already carries it. Labels are a GitHub write standing beside a scaffold that is not, so the ordering is the guarantee: an unreadable label listing, a refused creation and aghthat cannot name the current repository are each anote:line the scaffold survives, and a directory that is not a git repository yet is skipped with a note saying the labels arrive on the rerun once it is one — both statesinitis explicitly built to work in.checkpointensures the one gate label beforeAddLabelapplies it. A creation that fails is a note and the gate is still raised: applying an unknown label creates it implicitly, exactly as before, and only the colour is lost.migratecreates the missing three and restyles them, reporting each, with--dry-runlisting the same steps beside the file ones and writing neither. Every other label in the repository is untouched. It is the one place the protocol overwrites a label's styling, and the asymmetry withinitis deliberate:migrateis the one-shot move to the 2.0 layout whose whole promise is a repository indistinguishable from a fresh 2.0init, and a 1.x repository'scc:labels were without exception created implicitly, so there is no project intent there to preserve;initis the verb that reruns, on a repository whose labels somebody may have chosen. A restyle is addressed by the name the listing returned, carrying the protocol's colour and description, so a repository spelling the labelCC:Taskkeeps that spelling — restyling is not renaming.The step runs on every path where the move reached disk: after the commit, after a detached-HEAD note, and after a commit
gitrefused. And a repository already on 2.0 moves nothing, commits nothing and still runs it, sayinglabels already at the protocol defaultswhen there is nothing to do — so a rerun is the real recovery when the label step could not reach GitHub, which is what the CHANGELOG tells the operator to do and what round one found it could not deliver.migrateis idempotent in what it moves rather than in what it does.The palette
Sampled from
codecrew-www'sdocs/assets/images/crew/*.pngby counting every accent pixel (alpha ≥ 200, saturation > 0.35, lightness 0.20–0.90). Each image carries its accent at two tones, a pure one and its shading; the pure tone is the colour. The full sample and the reasoning are the Decision on #283.cc:milestone#01d4ffcodecrew-code-t.png,codecrew-coord-t.png)cc:task#92edffcodecrew-test-t.png) — the milestone's hue, lightenedcc:needs-decision#f0aeffcodecrew-review-t.png)cc:needs-decisiontakes the review pink because a gate is a question for a human and review is the seat whose job is asking one.cc:milestonetakes the project's own colour, andcc:taskthe same hue lightened, because a task is part of a milestone and the two never share an issue — which leaves the contrast where it is actually read, on a gated task carryingcc:taskandcc:needs-decisionside by side, 99° of hue apart. The docs magenta#e162ffis left unassigned rather than spent on the pairing that would collide with the gate.Tests
ensureLabels: all three missing → three created and reported with their colours; two present → only the missing one created; all three present but recased and restyled → nothing created and nothing restyled; the listing unreadable → one note and nothing created; one creation refused → a note and the remaining labels still attempted. One listing read however many labels are wanted, andensureLabelsnever restyles whatever it finds.applyLabelsin restyle mode: none present → three created; all three present from implicit creation → three restyled,#ededed -> #01d4ffand so on; already wearing the defaults → silent; the colour matching but the description not → restyled, and addressed by the repository's ownCC:Taskwhile the receipt names the protocol's spelling; a mix of present and missing → each handled its own way; a refused restyle → a note with the labels after it still attempted. Every case is run twice, once live and once as--dry-run, and the dry run must print the live output verbatim withcreated/restyledreplaced bywould create/would restylewhile writing nothing.initCmdin a real git repository with a stubbed target: the labels created and reported, GitHub refusing the listing, andghunable to name the repository — in all three the scaffold is committed and the commit contains the pointer, and the label step is asserted to come after the commit; plus a spoke getting all three, and a non-repository directory producing the skip note with no GitHub call at all.raiseGate: created then applied when the repository does not define the label, nothing created when it does (recased and restyled), and a note but still a raised gate when the listing cannot be read — the three wording cases keep their exact receipts.migrate: a 1.x repository withcc:taskfrom implicit creation and the other two absent → one restyle, two creations, a label of the project's own left alone, and the step asserted to fall between the commit line and thenext:guidance; the dry run listing the same and writing nothing; and GitHub refusing the listing in both modes → a note, no refusal, and the migration still committed. The rerun path —migrateon a repository already on 2.0 — in four shapes: the labels never done (three created), already right (labels already at the protocol defaults), the dry run previewing it, and GitHub still refusing (a note); each asserting that nothing was committed, nothing was written to disk, and no move, write or rewrite was reported. And the detached-HEAD path: the moves applied, the labels done, and the note asking the human to finish the commit still last on screen. The table itself: three names, six hex digits with no#, all colours distinct, descriptions inside GitHub's 100-character limit. And the two GitHub calls' shape: the listing paginated, the colour reaching the API without a leading#.go test ./...,go vet ./...andgofmt -l .are clean; rebased onorigin/mainatc452fcf, through #271, #270 and #273 in turn, every conflict resolved keeping both sides — the## [Unreleased]sections newest-first,SPEC.md's rows anddocs/identities.md's teardown list intact on both sides, andtask.go'sCreateIssuecall carrying #270's--adoptsbody with this branch'stracker.LabelTaskconstant. Every migrate test stubs the label target throughlegacyRepo, so no test reaches a realgh.Docs
SPEC §4's label sentence grows the defaults table with the reasoning, the never-restyle rule and
migratenamed as its one exception; §6'sinitrow gains the step, its place after the commit and the four ways it degrades to a note, thecheckpointrow the create-if-missing clause, and themigraterow the restyle, its dry run, the paths it runs on, and an idempotence sentence that now reads "idempotent in what it moves … but the label step still runs".cli.go's usage lines forinitandmigratename it, the teardown list indocs/identities.mdsays which verb creates them rather than "on first use", and the "Moving a repo forward" migration steps in the CHANGELOG say what the move does to the labels. CHANGELOG entry under## [Unreleased].Round one, addressed
Finding 1 (blocking) and finding 2, both taken, with checky's preferred fix.
migrateon a repository already on 2.0 now moves nothing, commits nothing and runs the label step, so the recovery the CHANGELOG documents is the one the code performs;--dry-runpreviews it. The same call goes on the two paths where the move reaches disk and the commit does not — a detached HEAD, and a commitgitrefused — so there is one rule rather than a path the operator has no reason to suspect: once the move is on disk, the labels run. SPEC §6'smigraterow now says "idempotent in what it moves … but the label step still runs", and the CHANGELOG's 2.0 entry and its "Moving a repo forward" steps say the same. Which of checky's two options for finding 2 I took, and why, is a Decision on #283, with the trade-off (labels written for a migration that may still be abandoned — additive, correctly styled and idempotent, so an abandoned one leaves whatinitwould have left) and the rejected alternative (one call before the commit attempt, which would invert the ordering SPEC and the tests state and buy nothing,applyLabelsreturning no error).Finding 3 (nit), taken the first way. The call site passes
s.was.Namewhen restyling, so the comment describes the code rather than resting on GitHub's case-insensitive path lookup — an untested and unstated premise, exactly as checky read it.TestApplyLabelsRestylesnow exercises a repository spelling the labelCC:Taskend to end and asserts both halves: the write is addressed by the listing's name, and carries the protocol's colour and description.TestLabelCallsbuilds the shape a caller actually produces. The commit body of0d3ccc6is history and stands as written; the correction is on the record.New tests: the rerun path in four shapes (labels never done, already right, dry run, GitHub still refusing), each asserting nothing was committed, nothing was written to disk and no move or rewrite was reported; and both commit-less paths — the detached HEAD, asserting the moves applied, the labels ran, and the note asking the human to finish the commit is still last on screen, and the commit
gitrefused, asserting the same with the receipts followingpathspecCommit's own note.Round two's non-blocking notes
Checky approved at
748a41ewith three notes; all three are taken in0428cafand in this body. SPEC's once-only claims: §6'smigraterow opened "run once per repo and then never again" while closing by documenting the rerun, and §4 rested the restyle asymmetry on "migrateruns once". Both now say the move runs once and a rerun repeats the label step alone, and §4 accepts what follows rather than leaving it to be found — acc:label restyled deliberately after the migration is set back to the defaults by the nextmigrate, previewed by--dry-runand named in the receipts, withinitas the verb a project reruns instead. The untested path: the refused-commit branch carried a comment claiming coverage and no test; removing themigrateLabelscall there left the suite green.TestMigrateDoesTheLabelsWhenTheCommitIsRefusedinduces the failure the wayTestCommitScaffoldFailureIsANotedoes, and the mutation now fails the suite. The stale rebase line in this body is corrected above.🤖 Generated with Claude Code
https://claude.ai/code/session_017Zu94NTC5DJskcpMTSofzr