Rework migrating-fleet-context: amend the spec, rewrite the skill, add the missing gate - #41
Merged
Merged
Conversation
…s drift Two classifications told the migration skill to do things the repository forbids. docs/archive/ is strictly immutable per .agents/AGENTS.md, but the misplaced-docs walk covered all of docs/, so an archived *-plan.md was reported as misplaced -- and a misplaced file is one the skill is instructed to git mv. The classifier and the skill disagreed about what "misplaced" means; the archive rule wins. .agents/skills/ is where design section 2 says repository-specific skills belong, yet every unrecognised directory there was classified customized, which isDriftClean treats as dirty. A repository using the feature as designed could never report clean and no migration could clear it, because there was nothing to fix. The tool now judges only the skills it embeds; the rest are listed in a new local_skills field and never classified. Both existing assertions encoded the old behaviour and are updated deliberately; see Amendment 1 of the 2026-08-29 two-tier design.
The design's check table specifies warn for scaffold:skill-migrating when the skill is "missing or outdated", but the implementation reported a diverged copy as ok with the detail "carries repository customizations". migrating-fleet-context is 100% agents-owned (design section 5.1). Unlike recording-what-you-learn there is no customization to respect: a copy that does not match the embedded asset is stale, and reporting it ok let a skill carrying obsolete migration instructions pass its own health check. That is the same skill an agent then follows to migrate the fleet.
The skill was the one deliverable of the two-tier work that is prose rather than code, and it shipped contradicting the tooling it drives: single-repo only, one procedure for four router states, an unconditional CLAUDE.md replacement, no traceability for its own zero-rule-dropping invariant, and a commit with no approval gate. The rewrite follows the amended design section 7: a self-currency check first, the two JSON shapes named, a state table with one action per router state, the three root-file topologies including the destructive inverted case, explicitly named merge sources, retired-store triage, a traceability table, and a stop before anything is staged. TestMigrationSkillCoversItsSpecifiedProtocol is the gate that did not exist. Against the old skill it failed nine times, once per real defect. TestLivingDocumentsNameOnlyRealCommands already scanned this file but only catches commands that do not exist, never required commands that are absent -- which is why the omissions survived review. Two further tests bind the repository copy to the embedded asset and the pasted router to scaffold.DefaultAgentsMD, since both pairs ship in the same binary and nothing else stopped them diverging. All three are registered in the docs job, which fails if a named test stops existing.
…wrong Amendment 1 records what section 7 said before the corrections, because the spec is the design still in force and a body edited silently would leave the next author of the skill with no reason for any of it. Four defects traced to this spec rather than to the plan or the skill: the archive contradiction between 7.1 and 7.2, "3-way merge" used four times with its sources never named, no preservation step for CLAUDE.md, and an invariant asserted with no mechanism. New sections cover the router state table and named merge sources (7.3), the three root-file topologies including the inverted symlink measured on playground/desktop_pet (7.4), the traceability requirement (7.5), fleet mode and the two JSON shapes (7.6), and retired-store triage (7.7). The journal entry carries the attribution analysis: which defects entered at the spec, which at the plan, and which at the skill, and why the plan is its own control for the claim that writing-plans has no shape for a prose deliverable. The plan itself is deliberately unedited -- it is a dated record of what was intended and executed, and this entry is its correction.
…iles has The skill is embedded in the binary and scaffolded into arbitrary repositories, including on machines with no copy of this checkout. Its closing section listed four dated design and Q&A paths from this repository, which read as a working reference and resolve to nothing anywhere else -- worse than no pointer, because an agent will try to follow it. It now says where the skill is owned, that the tool rather than the document is the authority on state, and that the architecture rationale lives upstream -- deliberately without paths. The one dated example filename in a git mv sample is now a placeholder. TestMigrationSkillNamesNoRepoLocalDocuments rejects any dated markdown filename. Bare dates describing an era are still allowed; a dated filename is a path into this repository.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #40. The Go side of the two-tier work was sound; the one deliverable
that is prose — the
migrating-fleet-contextskill — shipped contradicting thetooling it drives. This amends the spec, rewrites the skill, and adds the gate
that would have caught it.
The gate that did not exist
TestMigrationSkillCoversItsSpecifiedProtocolfails nine times against theold skill, once per real defect.
TestLivingDocumentsNameOnlyRealCommandsalready scanned that file, but only catches commands that do not exist — never
required commands that are absent. That asymmetry is why the omissions survived
review.
Two further tests bind the pairs that ship in the same binary and had nothing
holding them together: the repository copy against the embedded asset, and the
router pasted into the skill against
scaffold.DefaultAgentsMD. All three areregistered in the
docsjob.Two defects found while fixing
The inverted symlink.
playground/desktop_petis on the pre-2026-08-19topology:
AGENTS.md -> CLAUDE.md, with the only real content inCLAUDE.md.The old skill's unconditional replacement would have deleted it and left
AGENTS.md -> CLAUDE.md -> AGENTS.md— a symlink loop with every line of contextgone. Now design §7.4, with a three-row topology table.
The tool owned a directory it did not own.
drift.goclassified everydirectory under
.agents/skills/, reporting unrecognised ones ascustomized,which
isDriftCleantreats as dirty. But §2 says that is exactly whererepository-specific skills belong — so a repository using the feature as designed
could never report clean, and no migration could fix it.
playground/autogo-mlxcarries two such skills and was permanently dirty. The tool now judges only what
it embeds; the rest is listed in a new
local_skillsfield.Also
docs/archive/is no longer reported asmisplaced_docs; the classifier andthe skill now agree with the immutability rule.
scaffold:skill-migratingwarns on divergence instead of reportingok. Theskill is
agents-owned, so a diverged copy is stale, not customized — and astale migration skill was passing its own health check.
Records
Amendment 1 of the design says what §7 said before each correction. The
implementation plan is deliberately unedited: it is a dated record of what
was intended and executed, and
docs/journal/2026-09-01-why-the-migration-skill-shipped-hollow.mdis itscorrection, including which defects entered at the spec, which at the plan, and
which at the skill.
Every fix went red before green. One control appeared to pass while sabotaged —
go testserved a cached result for a test reading a file outside its package;-count=1showed it failing correctly, andverify.ymlalready runs that way.