feat(adf): doctor, score and bootstrap read the vendor mode record (#318 3/6)

[stackbilt-admin]

Refs #318

PR 3 of 6. doctor, score and bootstrap now read the VENDORS mode record from PR1 instead of guessing from markers. This PR also adds charter adf vendor restore <file> (CP-5), and adf migrate now respects a recorded unmanaged. It builds on main @ 9ce61b4 (#350, #352).

Before → after

Command / case	main @ 9ce61b4	this PR
doctor, no VENDORS section	no vendor-mode check	+ one INFO line: "N vendor file(s) have no recorded mode … Record them with: charter adf vendor init". Exit code unchanged
doctor, recorded pointer, file is compiled	nothing	WARN recorded-drift, which fails --ci and the pre-commit hook. The remedy (adf vendor restore) clears it
doctor, recorded file deleted	nothing	WARN recorded-absent, naming a recreate command for its mode
doctor, malformed / unknown / not-allowed VENDORS line	silent	WARN (a duplicate line is INFO)
doctor, recorded-unmanaged hand-authored CLAUDE.md	WARN "contains stack rules"	no WARN
doctor, agent config PASS	"N … are thin pointers" (counted compiled files, and on Windows counted AGENTS.md and agents.md twice)	"P thin pointer(s) and C compiled file(s) route to .ai/", one entry per physical file
score, thin pointer with a populated .ai/	not substantive, −5, "Expand CLAUDE.md"	substantive via ai-dir (in-memory compile), no recommendation
score, compiled or unmanaged file	—	unchanged
bootstrap, compiled file (any record)	--force turned it into a pointer	skipped, with the vendor set + vendor restore commands
bootstrap --force, hand-authored file	overwritten before migrate ran, so its rules were lost	skipped with the custom-content warning; with --yes, phase 4 migrates it
bootstrap or bare adf migrate, recorded unmanaged	migrated / overwritten	left alone with a note
adf migrate --source F, F recorded unmanaged	migrated; record kept as unmanaged (drift)	migrated; record upserted to pointer before the write, reverted if the write fails
adf compile --check pointer remedies	git checkout -- F	charter adf vendor restore F
adf vendor restore F	n/a	rewrites one pointer and refuses on any content loss

Operator sign-offs (Gate 2)

• CP-1 / CP-2: new doctor WARNs fail --ci and the pre-commit hook. This only affects repos with a VENDORS record. A repo without one gets a single INFO line and no new WARN. On this repo, doctor --adf-only --ci still exits 0 (main: PASS; this PR: PASS plus one INFO).
• CP-3: bootstrap --force no longer overwrites hand-authored or compiled vendor files. This changes what a documented flag does. It is listed under CHANGELOG Changed, and the docs/cli-reference bootstrap --force line and the global --force row are updated.
• CP-4: score values rise for pointer repos. The CHANGELOG notes that score JSON numbers change, only upward. On this repo the score is 94 → 94.
• CP-5: adf vendor restore ships in this PR.
• CP-6: an explicit migrate --source F upserts pointer.
• CP-7: doctor JSON has rows on the adf vendor mode check only.

Data-loss fix

On main, bootstrap --force wrote the template over every existing pointer-file name in phase 3, before phase 4 (migrate) could read it. Vendor files are not backed up (only .ai/*.adf go to .ai/.backup/), so a hand-authored CLAUDE.md's rules were gone and no .pre-adf-migrate.bak was written. Reproduced end to end: on main the rule is not in .ai/ and there is no backup. With this PR the rule lands in .ai/core.adf, the backup exists, and CLAUDE.md: pointer is recorded after the migration.

A related fix found while testing: --force also re-scaffolds manifest.adf, which dropped the VENDORS record before the pointer and migrate phases read it. Valid entries are now carried over to the new manifest through the shared writer (add-if-absent, re-parse guard).

Invariants and where they are tested

All tests below are in packages/cli/src/__tests__/adf-vendor-doctor-score.test.ts unless another file is named.

• INV-P3-1 doctor and score never write. Covered by the snapshot tests "doctor writes neither…" and "VENDORS stays out of path grounding, and score writes nothing".
• INV-P3-2 no VENDORS means one INFO and no new WARN, and every other check keeps its status. Covered by "no VENDORS section gives one INFO line…" and "every other check keeps its status".
• INV-P3-3 WARN only for an explicit record. Covered by the ROW_STATUS pin and a mixed no-record fixture that stays INFO.
• INV-P3-4 / INV-P3-5 bootstrap never writes over compiled or hand-authored files. Covered by the decision-table test (every row, with and without --force) and full bootstrap --force --yes runs.
• INV-P3-6 recorded unmanaged is never migrated by bootstrap or bare migrate, while explicit --source migrates it and upserts. Covered by the bootstrap test, the bare-discovery test, the CP-6 test (doctor shows no drift afterwards), and the revert-on-failure test.
• INV-P3-7 score is monotonic. Five fixtures (thin pointer, hybrid pointer, compiled, unmanaged, empty .ai/) are scored against the text-only rule main used, and the thin-pointer fixture gains exactly +8.
• INV-P3-8 score never recommends hand-editing a pointer or compiled CLAUDE.md. This also drops "or CLAUDE.md" from two generic recommendations when CLAUDE.md is a pointer or compiled.
• INV-P3-9 restore never discards content. Covered by each refusal test, a no-bytes-lost sweep across refusal fixtures, retained sections kept, unchanged meaning byte-equal with no write, and record revert on a read-only file.
• INV-P3-10 adf.ts stays at 1027/1041. The vendor help line was edited in place. adf-migrate.ts is 780/792.
• INV-P3-11 git diff origin/main -- .githooks packages/cli/src/commands/hook.ts is empty.
• INV-P3-12 additive JSON only. Covered by key-order tests for the doctor checks and rows and for the score signals.

Superseded PR1 tests that pinned behaviour "until PR3" were updated in adf-vendor.test.ts, and adf-vendor-consumers.test.ts now expects the restore remedies.

Deviations from the addendum (deliberate)

• doctorDriftCompiled lists the content-preserving fix (vendor set F <inferred>) first, then regenerating. When regenerating would lose something, it says so (discards N item(s) / discards its hand-authored content) and adds --force for an unmanaged file. This mirrors PR2's compile --check drift line. The literal template put regenerating first and omitted --force, so for a hand-authored file its remedy would not have worked.
• No adf vendor mode entry when there are no vendor files (manifest present, no files, no record), to avoid an "All 0 vendor file(s)…" line.
• The restore revert after a failed file write puts back the manifest's previous bytes, and only if nothing else changed the manifest. The record add itself goes through recordVendorModes with the re-parse guard.

NOT in this PR

• Hook scope and the --source loop (PR4). The hooks are byte-identical to main.
• Compile retained-section preservation (PR5).
• The agents.md rename and record-key move (PR6). This PR only reports them as the INFO legacy-spelling row.
• Context/snapshot vendor map, sentinel marker, and doctor --fix.

Verification

• pnpm build, pnpm run typecheck and pnpm run verify:adf pass.
• pnpm test: 1228 passed, 5 failed. The failures are the 5 known Windows-only ones (bootstrap bootstrap: phases 6/7 collect warnings without printing them; absoluteAiDirArg misses Windows paths on POSIX #307 fixture, context-refresh "captures git signals", serve-context "returns existing snapshot", precommit-hook "auto-tidies bloated CLAUDE.md" and "re-stages modified .adf modules").
• E2E was run in temp git repos against both main and this branch (bootstrap, doctor rows, score, restore refusals and revert).

Implemented via constellation-loop against a locked blueprint.

🤖 Generated with Claude Code

[stackbilt-admin]

CHANGES_REQUESTED

Adversarial review of #353 (#318 PR 3/6) at bf890e6, against main at 9ce61b4. I built both in temp worktrees and ran E2E fixtures with the built CLI binaries (Windows, Git Bash).

The existing-user CI story holds, and so do the bootstrap data-loss fix, the score monotonicity and migrate CP-6. Three things need fixing first. Two are in adf vendor restore, which is supposed to refuse whenever content would be lost and can silently delete content instead. The third is a doctor remedy line that points at the wrong VENDORS entry.

Required changes

1. adf vendor restore silently deletes user content in a pointer, and exits 0. In packages/cli/src/commands/adf-vendor.ts:845-860 (restoreVendorFile), the only checks on an inferred or recorded pointer are countExtractable (bloat) and retained sections. Any line that tidy wouldn't extract and that doesn't sit under a retained heading gets dropped. This breaks INV-P3-9 and the CHANGELOG/cli-reference claim "refuses (exit 1, nothing written) whenever content would be lost".

Repro: start from a clean main-bootstrapped repo, make one edit to the hybrid CLAUDE.md, then run charter adf vendor restore CLAUDE.md. Every case below printed [ok] Restored CLAUDE.md as a thin pointer, exited 0, and lost the line:

• title changed to # Acme Payments — agent notes → reset to # CLAUDE.md
• an extra blockquote line > Escalate prod incidents to #oncall-payments. → deleted
• text inside ## Session Start replaced with Our CI token lives in 1Password vault Eng-Prod. → reverted to the template
• a prose line after <!-- charter:module-index:end --> → deleted (see item 2)

Suggested fix: after building next, refuse when any non-blank line of current is missing from next. Skip lines that belong to a known Charter template: pointerTemplatesFor(file), the generic migrate pointer, and the module-index block between its markers. The contract enum is locked, so reuse hand-authored (remedy adf migrate --source F) or pointer-bloated. Add these four repros to the INV-P3-9 sweep. Today that sweep only covers fixtures that already refuse, and has no test that a successful restore keeps every user byte.

2. ## Module Index is reported as kept, then dropped (adf-vendor.ts:774 withoutModuleIndexSection, and :858 keptSections). When the user has content under ## Module Index, divergentRetainedHeadings lists that heading, so the output says Restored CLAUDE.md as a thin pointer (kept ## Module Index). But restoredPointer strips the whole Module Index section from the retained block, so the content goes. Either carry over the non-index lines of that section, or refuse under item 1. In both cases keptSections must only list what actually survives.

3. Doctor's duplicate-entry line names the ignored entry as the winner (packages/cli/src/commands/doctor.ts:153 entryDuplicate, filled at :276-277).

Repro: put - AGENTS.md: pointer and then - AGENTS.md: unmanaged under VENDORS. The resolved mode is unmanaged (last wins; the row shows recordedMode: 'unmanaged', and agent config says "1 file(s) recorded unmanaged"). Doctor still prints "the last entry ('AGENTS.md: pointer') wins. Delete the others." p.raw is the ignored earlier entry, as PR1's vendor status wording ("Ignoring VENDORS entry '{raw}' … The last entry for {file} wins") says. A user who follows the doctor line deletes the entry that actually wins, which flips the file's mode. Reword to something like "VENDORS lists {file} more than once in {manifestPath}; '{raw}' is ignored because a later entry wins. Delete that line." Then update the pin at packages/cli/src/__tests__/adf-vendor-doctor-score.test.ts:258, which currently asserts the wrong text.

Verified, no change needed

• Existing-user CI. 11 fixtures, each run on its own copy with both binaries: no VENDORS section (thin, hybrid, compiled, unmanaged), scaffold-only .ai/, .ai/ with just a manifest, broken .ai/ (garbage core.adf plus a missing DEFAULT_LOAD module), a monorepo with packages/{a,b}/CLAUDE.md, no .ai/, the base repo, and a repo with a recorded VENDORS section.
  • Exit codes for doctor --adf-only --ci, doctor --ci and doctor --format json are identical to main.
  • The overall JSON status is identical.
  • Every other check keeps its name, status and key set.
  • The only differences are the one adf vendor mode INFO row (it never affects hasWarn) and the agent-config/bloat PASS wording and counts. Main counted agents.md and AGENTS.md twice on Windows (6 files, now 5).
  • This repo: doctor --adf-only --ci exits 0 on both (PR adds the INFO line). score is 93/A on both.
• Score only rises. In 8 of the 11 fixtures it went from 66 to 69 (or 65 to 68 for compiled); the other 3 were unchanged. None decreased. The rise comes from .cursorrules becoming substantive via ai-dir.
  • The broken .ai/ does not crash score (exit 0, 57 on both); the compile error is caught.
  • Score writes nothing. git status after the run matches main; a stray .charter/ appears only in repos without one, on both sides, so it is pre-existing.
  • Compile runs at most once per pointer file whose text is not substantive (≤3 per run), not once per module.
• bootstrap --force.
  • Data-loss fix confirmed. On main a hand-authored CLAUDE.md (with no record) is overwritten in phase 3; HANDRULE is gone with no backup.
  • With the PR it is skipped with the warning, phase 4 migrates it, HANDRULE lands in .ai/core.adf, .pre-adf-migrate.bak exists, and CLAUDE.md: pointer is appended.
  • A GEMINI.md recorded compiled survives (on main it is turned into a pointer and its record is flipped).
  • The VENDORS carry-over keeps the original entry order and goes through recordVendorModes add-if-absent.
• restore, the parts that work. Clean pointers come back byte-equal (unchanged, nothing written). ## Environment content is kept. Bloat refuses. The read-only-file revert puts the manifest back byte-for-byte (including removing the VENDORS section it had just created). Absent and unrecorded files record pointer and restore with the right GEMINI template. The recorded-pointer-plus-compiled drift remedy round-trips to doctor PASS.
• The revert logic itself is sound.
  • The read/compare/write is not atomic, but the TOCTOU window is CLI-scale and acceptable.
  • When the record add was unchanged, after stays null and no revert is attempted, which is correct because nothing changed.
  • A recorded pointer gets no record write.
• Doctor rows. All 9 kinds were exercised through the CLI with the expected statuses. Worst-row aggregation works (a duplicate alone exits 0). JSON rows appear on this check only, with exactly the schema keys.
  • Ruling on the driftCompiled deviation: accepted. Putting the content-preserving vendor set first, naming what regenerating discards, and adding --force for an unmanaged file matches PR2's compile --check line. The literal template's remedy would not have worked for a hand-authored file.
  • Leaving the check out when there are no vendor files is also fine.
• migrate. Bare discovery skips files recorded unmanaged (text and JSON). --source CLAUDE.md migrates, upserts pointer, and doctor afterwards shows "All 5 … match" with exit 0.
• Repo invariants.
  • Hooks: git diff origin/main -- .githooks packages/cli/src/commands/hook.ts is empty.
  • LOC: adf_commands_loc 1027/1041 (14 lines of headroom), adf_migrate_loc 780/792 (12 lines). PR4–PR6 need to budget for this. A PR6 rename that touches migrate has very little room.
  • verify:adf and typecheck pass. pnpm test gives 1228 passed and 5 failed; main gives 1179 passed with the same 5 Windows-only failures.
• Superseded PR1 tests. They now pin the new behaviour rather than being loosened: GEMINI recorded compiled is skipped and absent, --source on a recorded-unmanaged alias upserts to AGENTS.md: pointer, and the git checkout remedies become vendor restore. The deleted "doctor/score unchanged" INV-1 tests are replaced by the new file.

Non-blocking notes

1. Hand-edited compiled file. For a recorded pointer whose file is compiled output, restore discards any hand-edit in the compiled body. I tested HAND EDIT … call Bob before deploying and it was lost with exit 0. The contract allows this (only retained-divergent refuses), and the file says DO NOT EDIT, but a cheap guard would be to refuse when the body differs from a fresh in-memory compileAdf of that target.
2. Write failures are reported as record-write. adf-vendor.ts:903 uses that refusal for a failed file write, so JSON consumers cannot tell "record skipped, nothing written" from "file write failed". The enum is locked, so at least make the text distinct (it already is) and document it.
   • restoreNotReverted (:157) says to delete the line by hand, which is the exact inverse. Consider adding that for an absent file the leftover pointer entry will show as recorded-absent in doctor.
3. CHANGELOG overclaims phase 4. CHANGELOG.md:15 says that with --yes "a hand-authored file is then migrated into .ai/ by phase 4 as it would be without --force". That only holds without a VENDORS entry.
   • Repro: bootstrap on main (which records CLAUDE.md: pointer), replace CLAUDE.md with hand-written rules, then run bootstrap --force --yes. The file is preserved but not migrated (phase 4: "no migratable files").
   • cli-reference states the rule correctly ("Phase 4 migrates only hand-authored files with no VENDORS entry"). Align the CHANGELOG with it.
   • Under --force --yes with no record, the phase-3 warning also says "Run 'charter adf migrate --source CLAUDE.md' … first", even though phase 4 does exactly that a moment later.
4. Score wording.
   • score.ts:1570: "in README.md, or your ADF files" has a stray comma when CLAUDE.md is dropped from the list.
   • A scaffold-only .ai/ (fresh bootstrap, no project content) already compiles to "substantive", so scorePopulateAiDir will rarely fire. That is consistent with compile parity; just noting it.
5. Help text. adf.ts:977 dropped "adf compile and adf tidy act on it" from the vendor help line.
6. Monorepo coverage. The deleted PR1 test "score is unchanged, including non-unique vendor basenames" (monorepo packages/a/CLAUDE.md) has no replacement. Add a monorepo fixture to the INV-P3-7 sweep. In my E2E it goes 66 → 69 with no regression.

Follow-up issues to file (pre-existing on main; PR3 does not make them worse)

A. bootstrap --force replaces a hand-edited manifest.adf (and core.adf/state.adf) with the template. The old files are backed up to .ai/.backup/<name>.<ts>, but the live manifest loses its customisations. Repro on main and on the PR (identical except VENDORS):

1. Bootstrap a repo.
2. Add .ai/payments.adf and register it with (Triggers on: stripe, payment).
3. Append , sprint-custom to planning's triggers.
4. Add a manifest RULE Custom manifest rule XYZ.
5. Add a 📊 METRICS: section (custom_loc: 10 / 100 [lines]).
6. Run charter bootstrap --force --yes --skip-install --skip-doctor.

Afterwards the planning trigger is back to the template, payments is re-registered by orphan detection as (Triggers on: payments) (custom triggers lost), and the custom RULE and the whole METRICS section are gone, along with any LOC ceilings a repo relies on. Any module registered by #348's migrate loses its custom triggers the same way. Custom content in core.adf/state.adf is also replaced (backed up). PR3 carries only VENDORS over. Suggested fix: under --force, merge the manifest (keep unknown sections, ON_DEMAND entries and METRICS) instead of replacing it, or skip manifest.adf unless an explicit --reset-manifest is given.

B. A circular import makes adf migrate write the generic pointer instead of the named template. adf-migrate.ts:102 builds POINTER_TEMPLATES when the module loads. When anything loads adf.js before adf-migrate.js, every POINTER_* is still undefined at that moment. The CLI's own order does this: index.js → doctor.js → adf.js → adf-migrate.js. migrateSource then falls back to the generic "This project uses [ADF]…" pointer.

I checked load orders on both main and the PR: loading ./adf, ./bootstrap, ./doctor, ./adf-tidy or ../index first gives GENERIC, and loading ./adf-migrate first gives the proper template. Results are identical on main.

User-visible repro: put hand-written rules in GEMINI.md (or AGENTS.md), run charter adf migrate --source GEMINI.md --yes, and the file starts with > This project uses [ADF]… instead of > **DO NOT add rules or context to this file.**. Doctor and tidy still treat it as a pointer, so there is no drift WARN; the effect is lost template wording.

Fix: resolve the template at call time (for example function pointerTemplateFor(name)). PR3's restoreTemplate already does this and so is unaffected.

[stackbilt-admin]

Review round 1 addressed in 1a6d900:

• adf vendor restore now refuses (exit 1, hand-authored, first lines named) when any non-blank line of a pointer would not survive. Exempt: lines from any Charter pointer template, current or earlier (pre-feat(adf): strengthen thin pointer language with agent-specific write-prohibition #25 wording, # agents.md title, the generic migrate/tidy pointers), plus the generated module-index block. This covers all four repros. A pristine old-template pointer still restores. keptSections never lists ## Module Index.
• On a compiled file, restore now refuses with the new compiled-differs refusal when the file differs from a fresh in-memory compile. The message names compile --target X --check and vendor set X compiled.
• A failed file write is now refusal write-failed with a writeError field. This extends the locked RestoreRefusal enum and the result schema, which the round-1 instruction asked for.
• doctor duplicate line: "'{raw}' is ignored because a later entry wins. Delete that line." There is a test showing that following the advice keeps the resolved mode.
• CHANGELOG phase-4 claim narrowed to files with no VENDORS entry. The --force --yes phase-3 warning now says phase 4 migrates the file in this run. Fixed the score comma, restored the adf.ts help text (still 1027 lines), and added a monorepo fixture to the INV-P3-7 sweep.

Tests: 1237 passed, 5 failed. The 5 failures are the known Windows-only ones.