Skip to content

[#419] feat: mirrors:regenerate — local, deterministic mirror realignment - #476

Open
rucka wants to merge 14 commits into
mainfrom
feature/US-419-mirror-realign-command
Open

[#419] feat: mirrors:regenerate — local, deterministic mirror realignment#476
rucka wants to merge 14 commits into
mainfrom
feature/US-419-mirror-realign-command

Conversation

@rucka

@rucka rucka commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

PR Information

PR Title: [#419] feat: mirrors:regenerate — local, deterministic mirror realignment
Story/Epic: #419 — standalone tooling/process defect
Type: Feature
Priority: P1 (Should-Have)
Assignee: rucka
Labels: tech-debt, risk:yellow

Summary

What Changed

  • New pnpm mirrors:regenerate (scripts/regenerate-mirrors.sh) — wraps pair update --source <local dataset> --offline, no new generation logic. Fail-loud (no git tree / no dataset / no turbo / build failure / missing dist/cli.js ⇒ exit 1), no check mode.
  • Renamed the remedy everywhere it was named: PRE_PUSH_REMEDY, DEVELOPMENT.md, its docs-site twin, and every failure branch of both mirror guards — drifted, missing, and the orphan sweep's "add it to the dataset and regenerate" (new shared MIRROR_REGENERATE_COMMAND, tied to the root package.json script). The missing and orphan branches matter most: both are reached by ADDING a dataset file, which is the one case pair update provably cannot serve — the new file is in no release, so the guard stays red and every other local mirror is overwritten with released content.
  • /pair-capability-publish-pr runs the regeneration in Phase 1, before the gate, reading the command from adoption (mirror-realign-command) — absent ⇒ step skipped, zero-config default — and commits what it wrote separately from the story's own commits.
  • The staged set is what the run actually wrote, derived from a git status --porcelain -z --untracked-files=all before/after comparison paired with a git hash-object -w digest of the already-dirty paths — never a path glob. What the run removed (a contributor's draft under a mirror registry) leaves that set and is named with its recover sha; the set is git add-ed, checked with git diff --cached --quiet (empty ⇒ no-op, not a failed commit), then committed by pathspec, so nothing the contributor had already staged is swept in and a mirror the run just created still lands. Before any of it runs, an untracked file under the trees the command writes into HALTs the step (mirror tree ⇒ deleted; add tree ⇒ indexed into llms.txt). See "The staging rule" below: it is the load-bearing part of the wiring.
  • gate:composition extended (MIRROR_REMEDY_SCRIPT) so a two-step remedy naming a nonexistent script is dead-advice-checked like the first step already was.

Why This Change

pair update installs the latest published knowledge base; the mirror-equality guards need regeneration from the working tree's dataset. The wrong remedy is the most plausible reason three of seven recorded drift incidents were hand-ported mirrors instead of regenerated ones.

Story Context

User Story: As a contributor whose push was blocked by the mirror-equality guard, I want an explicit command that realigns the generated mirrors from the local dataset, named by the gate and run by /pair-capability-publish-pr, so the remedy is deterministic and one step.

Acceptance Criteria: AC-1…AC-8 (root script; remedy renamed in PRE_PUSH_REMEDY / DEVELOPMENT.md / docs twin / both guards; publish-pr wiring + separate commit; fail-loud, never a silent success over a no-op; no check mode; format/format:check untouched) — all covered.

The staging rule (read this first)

The skill cannot say "stage the paths the command owns": the adoption declares a command, not a path list, deliberately. Resolving ownership from globs is wrong here for a concrete reason — root .pair/** is where this repo's generated mirrors land and holds 117 tracked authored files under .pair/adoption/** (git ls-files .pair/adoption | wc -l → 117). A contributor who edits way-of-working.md, leaves it unstaged and runs the skill would get their prose committed under chore: regenerate mirrors from local dataset — a commit they never wrote.

Phase 1 step 3 therefore snapshots git status --porcelain -z --untracked-files=all before the command and takes a git hash-object -w digest of every dirty entry whose worktree file exists; step 4 re-reads both after. The staged set is every path whose porcelain entry appeared, disappeared or changed, plus every already-dirty path whose digest moved. Never git add -A, never a glob, and no adopter enumerates owned globs anywhere: the command's own effect is the declaration.

And the commit is made by pathspec, not from the index. git add <paths> followed by a plain git commit commits the WHOLE index — so the same harm the rule above prevents for unstaged prose re-enters through anything the contributor had already staged before the run. MEASURED: with M authored.md (staged prose) and M mirror.md (regenerated), git add mirror.md && git commit -m 'chore: regenerate mirrors from local dataset' produces a commit listing both. It is an ordinary state, not an exotic one — this skill is standalone, explicitly runs on a dirty tree, and a resumed or interrupted /pair-process-implement leaves a populated index; and the step-4 Verify would only notice after the commit exists, which is not a HALT condition, so the mislabelled commit would be pushed. git commit -m "…" -- <paths> commits the pathspec alone and leaves M authored.md staged and byte-identical.

But the pathspec replaces the index as the commit's scope, not the git add as its step. A pathspec resolves against paths git already knows (index or HEAD), and the single most common way this step produces a path at all is a contributor adding a file to the dataset — the one case a published-KB install provably cannot serve — so the run creates its mirror and the entry is ?? <path>, which git does not know. MEASURED, untracked brandnew.md: git commit -m 'chore: regenerate mirrors from local dataset' -- brandnew.mderror: pathspec 'brandnew.md' did not match any file(s) known to git, exit 1, and the commit aborts whole — so the mirror it just regenerated never lands, the branch is pushed without it, and its own skills:conformance job goes red: the exact drift this step exists to remove, now caused by the step. The reason a dropped git add is invisible is the other half of the table, measured on the same tree: unstaged M tracked.md and D gone.md do commit by pathspec (git show --name-statusM tracked.md, D gone.md), so the recipe works on every drifted or removed mirror and fails on the first NEW one. The skill therefore states both the step and the asymmetry: git add <paths>, then git commit -m "…" -- <paths> — and the git add costs the earlier property nothing, since the pathspec still scopes the commit (measured on the same fixture: A authored.md is still staged and byte-identical after).

Why the digest half is not redundant — a porcelain entry encodes status, not content. Concrete case: HEAD carries a drifted mirror, the contributor is holding an uncommitted hand-edit to that same file, the command regenerates it. The entry is the same unstaged-modified M <path> on both reads. Under a status-only comparison the agent concludes NO CHANGE: no commit, no Mirrors: row, silence — while (a) the uncommitted work is destroyed on disk with nothing reported, and (b) the regenerated content is never committed, so the branch still pushes the stale mirror and skills:conformance goes red on the very PR this step exists to keep green. So those paths are staged and committed like any other write, each named in the output with a way back (overwrote uncommitted changes in: <path> (recover: git cat-file -p <sha> > <path>)), and the step-4 Verify reads the digest of every pre-existing dirty path not in the staged set — never git status's listing, which an overwrite leaves in place and which would therefore certify the loss it is meant to catch.

Why each flag in that recipe is load-bearing — the recipe has to hold for the tree a real contributor is standing in, which also contains deletions and not-yet-committed directories. Measured in a scratch repo (rm gone.md; mkdir newdir && echo a > newdir/a.md; hand-edit tracked.md): git status --porcelain prints D gone.md / M tracked.md / ?? newdir/, and git hash-object over those paths gives fatal: could not open 'gone.md' for reading (exit 128), a valid sha, and fatal: Unable to hash newdir/ (exit 128).

  • -z — the default porcelain format is not a list of paths: it QUOTES and octal-escapes anything with a space or a non-ASCII byte. MEASURED over with space.md, caffè.md, new file.md, nuovo-caffè.md: git status --porcelain -uall prints M "with space.md", M "caff\303\250.md" and the ?? twins, and all four resolve to no such file — so the entry fails the exists test and is dropped from the digest in silence, which is the very status-vs-content blindness the digest exists to close, re-entering through the parser. Concrete loss: a generated docs/My Guide.md already carrying an uncommitted hand-edit is overwritten, the entry is unchanged on both reads, the digest was never taken ⇒ NO CHANGE — hand-edit destroyed with no recover: row, regenerated bytes never staged, stale mirror pushed and the branch's own conformance job red. The reverse shape costs the step outright: a NEW generated file with a space is caught by status, and then git add '"con spazio.md"'fatal: pathspec … did not match any files, mid-step. -z prints raw bytes, NUL-separated, never quoted — so the snapshot is split on NUL (a filename may contain a newline), and a rename entry's <old> arrives as a second field to be consumed rather than read as an entry (which is also what removes the default format's old -> new ambiguity).
  • --untracked-files=all — the default reports a not-yet-committed directory as ONE ?? dir/ entry: identical before and after however many files under it the run rewrote, and unhashable. A generated mirror living under such a directory would be regenerated, read as NO CHANGE, and never staged — exactly the status-vs-content blindness the digest exists to close, in the one shape the digest could not reach. Expanded per file, ?? dir/a.md hashes like anything else. (git diff is no substitute: it never reports untracked paths at all.)
  • Digest only entries whose file exists — a fatal inside step 3 meets the step's own non-zero exit → HALT and blocks the PR on a condition the snapshot pass itself created. Deletions are safe to skip, and that is a property rather than a concession: a deleted path the run recreates moves its porcelain entry (DM or gone), so status already catches it. The digest is only needed where status cannot move — which is also why the step-4 Verify's "still has a file on disk" qualifier is not a hole.
  • -w — plain git hash-object hashes the bytes and discards them, so after the overwrite the contributor's edit is in no HEAD, no index, no disk and no ODB: the report would name a path nobody can restore. -w writes the blob, and git cat-file -p <sha> prints it back. MEASURED: SHA=$(git hash-object -w tracked.md) → overwrite → git cat-file -p $SHA prints v2-handedit; the same sha taken without -wfatal: Not a valid object name.

Executed against the real script, not argued. regenerate-mirrors.test.ts"the documented before/after recipe survives every ordinary porcelain shape" puts all three shapes in one fixture (a deleted tracked file, an installed file inside an untracked directory, a committed-drifted mirror carrying an uncommitted hand-edit). After the run, after.entries === before.entries — status is blind to both writes — while the digests of .pair/knowledge/index.md and .pair/knowledge/sub/note.md have both moved and both files hold dataset content, and git cat-file -p <before sha> returns each hand-edit verbatim. Its sibling "the snapshot recipe sees a path with a space and a non-ASCII byte — the default parse does not" runs the same shape over .pair/knowledge/con spazio.md and .pair/knowledge/caffè.md: the pre-fix parse digests neither, the -z recipe digests both and hands the bytes back. And "the regeneration commit carries only the regenerated paths, never a pre-STAGED authored file" runs both commit forms on one fixture — index-based lists authored.md + the mirror, pathspec lists the mirror alone. Its sibling "stages a newly created mirror before committing it — a pathspec alone cannot name it" holds both rows in one fixture: a dataset file added after convergence (so the run creates ?? .pair/knowledge/new-guide.md) next to a drifted tracked mirror. The bare pathspec commit over both is refused and HEAD is asserted unmoved; over the tracked path alone it succeeds, which is the whole reason the omission stays silent; the documented git add + pathspec form commits exactly the two and leaves the pre-staged src/authored.ts untouched. Three more run the three paragraphs above end to end: "deletes an uncommitted file under a mirror registry — nothing to stage, only the -w blob survives" (both removed shapes plus a genuine regeneration in one set: the naive git add → 128, the naive pathspec commit → 1 with HEAD unmoved, the documented recipe lands new-guide.md alone, and git cat-file -p restores both drafts byte-for-byte); "a non-empty set whose cached diff is empty is a no-op, never a failed commit" (the three render-equals-HEAD shapes, then the mixed set whose commit lists a subset); "indexes an untracked adoption file into the generated llms.txt — stash it before the run" (the dangling link measured in HEAD:.pair/llms.txt, then the stash/run/pop remedy asserted to its postcondition).

What the run removes is not stageable, and has to be named — the knowledge, github and agents registries are behavior: "mirror" (apps/pair-cli/config.json): the target is made equal to the dataset, so a file only the target has is deleted, a contributor's draft included. MEASURED with the real script: untracked ?? .pair/knowledge/wip-draft.md → run exit 0 → file gone, entry gone; the entry "disappeared", so the rule above puts it in the set, and git add .pair/knowledge/wip-draft.mdfatal: pathspec '.pair/knowledge/wip-draft.md' did not match any files, exit 128. The staged-new shape is worse: A. .pair/knowledge/staged-draft.mdAD; git add exits 0 (it stages the removal, dropping the index's only copy) and the pathspec commit fails instead — error: pathspec '.pair/knowledge/staged-draft.md' did not match any file(s) known to git, exit 1, HEAD unmoved, taking the genuine new-guide.md in the same set down with it. Either way Phase 1 dies after the destructive run — regenerated mirrors uncommitted, branch pushed stale, its own conformance job red — and the draft is destroyed with no report row, because overwrote uncommitted changes in: fires on a digest that moved, never on an entry that vanished, though its -w blob exists (git cat-file -p <sha># my wip draft). So a before entry HEAD does not know (??/A., porcelain XY with . for the blank column) whose file is gone after the run goes neither in git add nor in the pathspec, and is named removed untracked: <path> (recover: git cat-file -p <sha> > <path>).

A non-empty set whose cached diff is empty is a no-op, not a failed commit — a path whose dataset render already equals HEAD moves its entry when the run rewrites it, so it is in the set, yet after git add its index entry equals HEAD. MEASURED with the real script, three shapes in one fixture: staged hand-edit M. a.mdMM a.md; staged deletion D. b.mdD. b.md + ?? b.md; unstaged hand-edit .M c.md → entry gone. git add a b c exit 0; git diff --cached --quiet -- a b c exit 0 (empty); git commit -m … -- a b cnothing to commit, working tree clean, exit 1 — and the two hand-edits are gone from disk and index, with no row, because they entered the set through the entry comparison. Mixed with a genuine d.md: cached-quiet exit 1, the pathspec commit succeeds and lists [d] — a subset of the set. The recipe now runs git diff --cached --quiet -- <paths> between the git add and the commit (empty ⇒ no commit, the no-op branch), drives the recover rows from the digest comparison alone, whether or not a commit was made, and the Verify compares the commit's file list to git diff --cached --name-only -- <paths> rather than to the set.

The writer reads the whole target tree, untracked files included — so the run must start with none under the trees it writes into. The adoption registry is behavior: "add" (a file only the target has survives) and generateLlmsTxt (apps/pair-cli/src/registry/llms-generation.ts) indexes everything it finds on disk under .pair/adoption/**. MEASURED with the real script: untracked ?? .pair/adoption/tech/wip-note.md → run exit 0 → .pair/llms.txt gains - [adoption note](.pair/adoption/tech/wip-note.md) while the note stays ??; under the staging rule llms.txt (entry appeared) commits and the note (entry unchanged) does not, so HEAD:.pair/llms.txt carries a dangling link and the contributor's private WIP filename, with HEAD:.pair/adoption/tech/wip-note.md absent. Bytes untouched — the story's edge case held on bytes only — derived output leaked. Phase 1 step 3 therefore reads the before snapshot's ??/A. entries against the trees the adoption's mirror-realign-command entry names as written (this repo's already listed them, descriptively) and HALTs before running the command — the one point where a HALT costs nothing — naming each path and the remedy git stash push -u -- <paths> / git stash pop after Phase 1 (or commit/move them). Measured to its postcondition: stashed, the run leaves the tree clean and llms.txt link-free; popped, the note is back as ??. The skill owns no globs, so when the adoption names no written trees the check is skipped and the removed untracked: row is the only net (a deleted draft is caught after the fact; an indexed one is not). HALT rather than auto-stash is deliberate: the skill's only write to the branch stays the regeneration commit.

Writer and checker are not the same scope, and the asymmetry is the writer's. The guards compare dataset-sourced mirrors (a target-tree file with no dataset counterpart is compared to nothing), while the command additionally rewrites /skill-name tokens and .skills/** link paths across the whole installed .pair/** tree. Evidence on this branch: 6655439d regenerated adr-021, adr-022, adr-023 and collaborative-workflow.context.md — four files with no dataset counterpart (find packages/knowledge-hub/dataset -name 'adr-021*' → nothing), which had sat drifted on a green main. That region accumulates drift undetected and then lands, unrelated, in whichever PR next runs the writer. Documented in way-of-working.md and in the script header rather than left to be rediscovered; the separate commit is what keeps it readable.

Changes Made

Implementation Details

  • Root script: scripts/regenerate-mirrors.sh + mirrors:regenerate — builds the CLI through turbo first (cached; a stale dist/ would regenerate with yesterday's transform), pins INIT_CWD to the repo root (it outranks both the positional target and the cwd), resolves the target tree from git rev-parse --show-toplevel. TOOLCHAIN_ROOT (the script's own tree) and TARGET_ROOT (the tree being realigned) are split, which is what makes the happy path exercisable against a throwaway fixture — the same split scripts/format-lib/run-format.sh uses. Consequence, stated in the header because it is intended: a freshly created linked worktree has no node_modules/, so the script exits 1 with run pnpm install first and publish-pr HALTs before creating the PR — that tree cannot pass the gate one step later either.
  • Remedy renamed in PRE_PUSH_REMEDY, DEVELOPMENT.md, the docs-site twin, and both guards' failure messages — a scope extension past the card's literal "three places", because the guard whose failure the contributor reads is literally AC-3's Given. Descriptive references to the pair update transform (what a mirror is compared against) are a different claim and were left alone.
  • /pair-capability-publish-pr wiring in Phase 1, before the gate — not Phase 2 as the card proposed: mirror drift is what turns the gate red and a red gate HALTs, so a step after it is unreachable in its only case; and a gate that ran first judged a tree the PR no longer contains.
  • Guards for the wiring: mirror-realignment.test.ts (26) pins the phase ordering to the Phase 1 span, the adoption-read (the skill must not name this repo's script), the skip-when-absent default, the digest-paired staging rule, the hashable-shape scoping + -w recoverability, the NUL-separated read, the stage-then-pathspec commit, the removed untracked: exclusion + row, the cached-empty no-op branch, the pre-run HALT on untracked files under the written trees (skill + way-of-working.md), and the content-based Verify.
  • turbo.json cache keys: #test / #test:coverage inputs widened to $TURBO_ROOT$/scripts/** + $TURBO_ROOT$/package.json, and @pair/dev-tools#test(:coverage) now dependsOn: ["build", "@pair/pair-cli#build"] — those tests build and run the real CLI, and dev-tools declares no dependency on it, so a regression in the regeneration transform replayed a local FULL TURBO PASS. A task edge covers the whole --filter=@pair/pair-cli... closure and, unlike a hand-listed set of trees, cannot go stale as that closure grows. Guarded by a requiredDependsOn case in web-cloud-environment.test.ts.

Files Changed

  • Added: scripts/regenerate-mirrors.sh, packages/dev-tools/src/quality-gates/regenerate-mirrors.test.ts, packages/knowledge-hub/src/conformance/mirror-realignment.test.ts, ADL 2026-09-01-publish-pr-realigns-mirrors-before-the-gate.md
  • Modified: root package.json (one line), turbo.json, scripts/regenerate-mirrors.sh header, DEVELOPMENT.md, apps/website/content/docs/contributing/development-setup.mdx, apps/website/content/docs/reference/skills-catalog.mdx, packages/dev-tools/src/quality-gates/pre-push-gate-composition.{ts,test.ts}, packages/knowledge-hub/src/tools/{mirror-guard,skill-md-mirror}.{ts,test.ts}, packages/knowledge-hub/src/conformance/web-cloud-environment.test.ts, packages/knowledge-hub/dataset/.skills/capability/publish-pr/SKILL.md + its .claude mirror, .pair/adoption/tech/way-of-working.md, two other decision-log files, .pair/llms.txt (generated)
  • Regenerated, in their own commit 6655439d: three ADRs + one subdomain context file — pre-existing drift this branch never authored (see above)

Testing

Test Coverage

  • Unit / behavioural: regenerate-mirrors.test.ts19, every one shelling out to the real script against a real fixture git repo, never a source-string regex. Seven of them run the skill's documented snapshot/commit recipe verbatim around that script: over all three ordinary porcelain shapes, over a path with a space and a non-ASCII byte, over both commit forms with a populated index, over a newly created mirror (the ?? row a pathspec cannot name), over the two removed shapes (?? and A. drafts under a mirror registry), over the three render-equals-HEAD shapes plus a mixed set, and over an untracked adoption file indexed into llms.txt with the stash remedy applied to its postcondition.
  • Conformance: mirror-realignment.test.ts26 over the skill prose and whole-file mirror reproducibility; plus 2 in web-cloud-environment.test.ts (turbo dependsOn), 2 in pre-push-gate-composition.test.ts, 2 in mirror-guard.test.ts (the drifted AND the missing branch), and the orphan-sweep remedy case moved off the 'pair update' literal.
  • Mutation-checked: flipping never git add -A in the skill prose fails the conformance suite; reverting the digest recipe to a bare git hash-object over every dirty path fails 3 of its cases; reverting the commit recipe to the pathspec without its git add fails the stage-first case (1 failed / 21 passed); the four round-6 pins were red against the pre-fix prose (4 failed / 22 passed) and green after; dropping @pair/pair-cli#build fails with @pair/dev-tools#test is missing dependsOn @pair/pair-cli#build.

Test Results

pnpm quality-gate: exit 0 at 95a11736 — 22/22 turbo tasks + gate:composition,
hygiene:check, smoke-modes:check, docs:staleness, skills:conformance, dup:check
Pre-merge tiering: disabled ⇒ the full suite IS the tier check set for 🟡 (CI parity)

Testing Strategy

  • Happy path: regeneration against a fixture repo, and idempotence (a second run leaves git status --porcelain empty).
  • The pre-dirty overwrite: converge the fixture → commit a drifted mirror to HEAD → uncommitted hand-edit on it, plus one unrelated dirty authored file → run the real script. git status --porcelain after is byte-identical to before (both list M .pair/knowledge/index.md and M src/authored.ts) while git hash-object on the mirror moved and the authored file's did not. That is the executed proof the staging rule needs a digest.
  • The recipe over every porcelain shape: the same fixture extended with a deleted tracked file and an installed file inside an untracked directory (see "The staging rule"), proving the snapshot pass neither fatals nor goes blind on either, and that the pre-overwrite blob is recoverable from the ODB.
  • The recipe over a quoted path: two generated mirrors named con spazio.md and caffè.md, drifted on HEAD and hand-edited in the worktree. The default parse digests neither (both quoted/escaped ⇒ no such file); the -z parse digests both, sees both overwrites while after.entries === before.entries, and recovers both hand-edits from the ODB. The pathspec failure is measured in the same case.
  • The commit form, both ways: with M authored.md staged before the run, the index-based git add <paths> && git commit produces a commit listing the contributor's prose alongside the mirror; the documented git commit -m … -- <paths> on the same state lists the mirror alone and leaves the staged entry untouched.
  • The commit form on a mirror that did not exist yet: a dataset file added after convergence, so the run creates ?? .pair/knowledge/new-guide.md. The pathspec commit without a git add is refused (did not match any file(s) known to git, HEAD unmoved); over the drifted tracked mirror alone it succeeds, which is why the omission is silent; git add + pathspec commits exactly the two paths and still leaves the pre-staged authored file alone.
  • What the run removes: converge → add ?? .pair/knowledge/wip-draft.md, staged-new A. .pair/knowledge/staged-draft.md, and a new dataset file → run. Both drafts gone (?? entry vanished, A.AD); naive git add → 128 / naive pathspec commit → 1, HEAD unmoved; documented recipe commits new-guide.md alone; git cat-file -p <before sha> > <path> restores both drafts.
  • The cached-empty set: three dataset files converged so every render equals HEAD; staged edit / staged git rm / unstaged edit → run → MM, D.+??, gone; git add + git diff --cached --quiet exit 0 + pathspec commit → nothing to commit, exit 1; hand-edits recoverable only from the -w blobs; then a mixed set whose commit lists the one real path.
  • The indexed untracked adoption file: ?? .pair/adoption/tech/wip-note.md → run → .pair/llms.txt links it; committed by the rule, the link is dangling in HEAD. git stash push -u -- <note> → run → clean tree, no link → git stash pop → note back.
  • Fail-loud, all four AC-7 branches: no git tree; no dataset; no turbo; turbo exits 1; turbo exits 0 without producing dist/cli.js. The last three run the real script copied into the fixture's own scripts/ (TOOLCHAIN_ROOT is derived from the script's dirname, so that is the only way to reach them) driven by a node_modules/.bin/turbo stub. Dropping the post-build -f "$CLI" check makes exec node "$CLI" regenerate from a stale dist/ and exit 0 — the silent success AC-7 forbids; that case is now red.
  • Interrupt: a SIGTERM to a real sleeping build asserts the EXIT HUP INT TERM trap left TMPDIR empty. The two explicit rm -f calls stay: the success path ends in exec, which replaces the process and runs no EXIT trap.
  • Cache-key probe (measured, not argued): appending a comment to apps/pair-cli/src/registry/skill-refs.ts and re-running turbo run test --filter @pair/dev-tools gave 1 cached, 125ms >>> FULL TURBO before the dependsOn edge and 2 cached / 4, 12.7s after; the same probe on packages/content-ops/src/index.ts gives 0 cached.

Quality Assurance

Review Areas

  • Blast radius on format: git diff origin/main -- package.json is exactly one added line. format, format:check, prettier:*, mdlint:* and quality-gate are byte-identical to main (AC-6).
  • AC-3 byte-equality: the remedy paragraph in DEVELOPMENT.md and the docs twin differ on exactly one line — the ADL link form (relative path vs GitHub blob URL) — as ADL 2026-07-31 requires.

Documentation

Knowledge Sharing

  • ADL 2026-09-01-publish-pr-realigns-mirrors-before-the-gate.md — Phase 1 vs Phase 2; command read from adoption, never named in the skill; the content-aware staging rule and why HALT-and-report was rejected for it; and the thin-script-in-vitest deviation from the "scripts are never unit-tested" rule (its bounded exception is recorded in way-of-working.md too).
  • ADL 2026-07-31-pre-push-gate-is-check-only.md — its Open Decision is now closed by this story, listing what shipped.

Reviewer Guide

Testing the Changes

git checkout feature/US-419-mirror-realign-command && pnpm install
pnpm mirrors:regenerate && git status --porcelain   # expect: empty (idempotent, already aligned)
pnpm --filter @pair/dev-tools test && pnpm --filter @pair/knowledge-hub test

Review Focus Areas

  1. The staging rule above — it is the contract the skill's Phase 1 now carries for every adopter, not just this repo. Two design calls in it: (a) an untracked file under the written trees HALTs before the run rather than being auto-stashed, and the check is scoped by the trees the adoption names (skipped when it names none); (b) the recover rows are emitted from the digest comparison even when nothing is committed, so the Mirrors: row can appear with no commit. This repository has no path with a space or a non-ASCII byte today (git ls-files | grep '[^ -~]\| ' is empty), so the -z half is proven by fixture, not by this tree. In particular: committing the regenerated bytes over an overwritten hand-edit (rather than HALTing) is a deliberate call, recorded in the ADL's Alternatives; the -w blob plus the recover: hint on the Mirrors: row is what makes that call survivable for the contributor.
  2. Whether the writer/checker scope asymmetry is acceptable as documented, or wants a guard of its own (out of scope here: nothing today can check a file with no dataset source).

Dependencies & Related Work

Follow-up Work

  • Technical Debt addressed: the mirror guards no longer name a remedy that installs a published release instead of regenerating the local tree.

Pre-Submission Checklist

  • All acceptance criteria implemented and tested
  • All tests passing — pnpm quality-gate exit 0
  • Documentation updated (DEVELOPMENT.md, docs site, way-of-working.md, two ADLs)

@rucka rucka added tech-debt Tracked technical debt (living backlog, R7.2 — never blocks a PR) risk:yellow Classification: medium risk tier labels Sep 1, 2026
@rucka rucka self-assigned this Sep 1, 2026
@rucka rucka added the pr-state:to-be-reviewed PR state: awaiting review / gate label Sep 1, 2026
@rucka

rucka commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict

risk:yellow · cost:greenCHANGES-REQUESTED — the command, its wiring and its docs are sound and every AC is materially met, but publish-pr's staging rule can sweep authored .pair/adoption/** edits into the "regenerate mirrors" commit, and three claims the PR makes about scope are not true of the writer as configured here.

Open findings: 6. (1 Major, 5 Minor. 4 Questions are informational and never blocking.)

PR: #476 · Author: rucka · Reviewer: independent reviewer agent · Date: 2026-09-01 · Story: US-419 · Type: feature

Classification matrix — per dimension
Dimension Tier Source Note
Service/domain criticality green tech/risk-matrix.md every touched deployable listed Low
Change/diff risk yellow diff footprint root script + 3 packages + a skill that now commits on the contributor's behalf
Business impact green supporting subdomain contributor tooling
Security relevance green /pair-capability-assess-security (inline) no new inputs, no credentials, no network (the CLI runs --offline)
Coupling balance not assessed /pair-capability-assess-coupling not assessed until #263

Tier = max(assessed) = yellow, confirming the story's refinement-time tier. No drift.

Assessments

Security — Input validation

Verdict: green — the only external input is the repo's own tree; git rev-parse, the dataset dir and the CLI path are all existence-checked before use, and every shell expansion in scripts/regenerate-mirrors.sh is quoted under set -eu.

Security — Output handling

Verdict: green — output is regenerated Markdown written by the pre-existing pair update --source path; no new rendering or encoding surface.

Security — Authentication

Verdict: green — no auth surface touched. --offline structurally removes the network fetch the old pair update remedy performed.

Security — Authorization

Verdict: green — no access-control path touched. The new write is a local git commit on the contributor's own branch, git add -A explicitly forbidden.

Security — Introduced vulnerabilities

Verdict: green — 0 introduced, 1 pre-existing (CI secret-scan, see Q3).

Details
Severity Category File:location Introduced / pre-existing Recommendation
P2 A05 Security misconfiguration (false positive) scripts/smoke-tests/scenarios/pr-state-flow.sh:430 (commit 5e99d538, branch feature/US-398-…) pre-existing allowlist the synthetic 40-hex fixture in .gitleaks.toml on the branch that owns it

Evidence: gitleaks detect --source . --config .gitleaks.toml reproduced locally → leaks found: 1, rule generic-api-key, in a commit that is not an ancestor of origin/main nor of this PR's head. The value is TOKEN_HEAD='cc1fba12…', a SHA-shaped smoke fixture, not a credential. CI run 33410499948 on main @ e7d375bf — this PR's exact merge base — fails secret-scan identically. Not introduced here.

Cost

Verdict: cost:green — no provider, runtime or storage signal; the only added cost is a turbo-cached local build on the contributor's machine.

Architecture (Coupling)

Verdict: not assessed — /pair-capability-assess-coupling not composed (until #263). Observation only: the new MIRROR_REGENERATE_COMMAND export makes mirror-guard.tsskill-md-mirror.ts carry a second symbol on an import that already existed (diffSkillMd), so no new edge.

Bug fix — Red test before fix

Verdict: not applicable — not a bug fix. Filed and labelled as a feature/tooling story; no defect reference requiring a reproduction test.

Details

Findings by severity

Critical (must fix before merge)

None.

Major (must fix before merge)

  • packages/knowledge-hub/dataset/.skills/capability/publish-pr/SKILL.md:67 (and its mirror .claude/skills/pair-capability-publish-pr/SKILL.md) — the staging rule is a path glob, not "what the command actually wrote", and in this repo that glob includes authored files. Phase 1 step 4 says "Read git status --porcelain for the generated paths the command owns … stage only those paths", and way-of-working.md:79 declares those owned paths as .claude/**, root .pair/**, AGENTS.md/CLAUDE.md, .github/**. Root .pair/** holds 117 tracked authored files under .pair/adoption/** (git ls-files .pair/adoption | wc -l → 117). Concrete failure: a contributor edits .pair/adoption/tech/way-of-working.md and does not stage it — exactly what this very PR did — then runs /pair-capability-publish-pr. The realign command rewrites .claude/skills/**, step 4 reads porcelain over the owned globs, sees the authored way-of-working.md as dirty inside root .pair/**, and commits it in chore: regenerate mirrors from local dataset. The contributor's authored prose lands under a regeneration message in a commit they never wrote, directly contradicting the same bullet's "unstaged authored changes … must survive the run untouched" and the ADL's "never git add -A, so unstaged authored changes in the working tree survive untouched". Fix: make the staged set the command's own effect, not a glob — snapshot git status --porcelain before running the command, snapshot it after, and stage exactly the paths whose status changed. That also removes the need for any repo to enumerate owned globs, and makes the rule correct for adopters whose generated and authored trees interleave.

Minor (must fix before merge — same bar as Major, just lower impact)

  • packages/knowledge-hub/dataset/.skills/capability/publish-pr/SKILL.md:229 (Notes) — "it does not modify source files" is now false in the same document that adds a step which writes and commits files. A skill whose behaviour is defined only by its prose now carries a normative Notes bullet contradicting its own Phase 1, and the version bump ships the contradiction to every adopter. A reader (or agent) reconciling the two can reasonably conclude the realignment write is out of contract. Fix: carve out the exception, e.g. "…it modifies source files only through the adoption-declared mirror-realign-command (Phase 1), never renders a review verdict, and never merges."

  • .pair/adoption/tech/way-of-working.md:79 and scripts/regenerate-mirrors.sh:17-18 — the "one writer, one checker — the mirror guards (skills:conformance) are the checker, this is the only writer" claim asserts a parity that does not hold: the writer's scope is strictly wider than the checkers'. Evidence from this PR's own history: commit 6655439d ("chore: regenerate mirrors from local dataset") rewrote adr-021, adr-022, adr-023 and collaborative-workflow.context.md — four files that exist only in the target tree (find packages/knowledge-hub/dataset/.pair/adoption -name 'adr-021*' → no match), i.e. no mirror guard compares them to anything. They were drifted on main while main's CI build job was green (run 33410499948). So drift in that region accumulates undetected and then lands, unrelated, in whichever PR next runs the writer — which is now every PR. Fix: state the asymmetry where the claim is made ("the guards check the dataset-sourced mirrors; the command additionally rewrites skill references across the installed tree, which no guard verifies"), so the next reader does not infer a check that does not exist.

  • packages/knowledge-hub/src/tools/skill-md-mirror.ts:81MIRROR_REGENERATE_COMMAND = 'pnpm mirrors:regenerate' is a sixth independent copy of the command name and the only one with no guard tying it to package.json. gate:composition covers MIRROR_REMEDY_SCRIPT, and mirror-realignment.test.ts:128 covers the way-of-working.md literal. Concrete failure: rename the script to mirrors:sync in package.json and update MIRROR_REMEDY_SCRIPT + way-of-working.md → both guards green, while every mirror-guard failure still prints Regenerate with 'pnpm mirrors:regenerate' — a dead command, which is precisely the dead-advice class this PR extends gate:composition to prevent. Fix: assert MIRROR_REGENERATE_COMMAND === \pnpm ${MIRROR_REMEDY_SCRIPT}`(or read the rootpackage.json` scripts in a conformance test) so the two packages cannot disagree.

  • packages/dev-tools/src/quality-gates/regenerate-mirrors.test.ts:186-209 — AC-7 is fail-loud on four conditions; only two are exercised (missing dataset, no git tree). The missing-turbo (regenerate-mirrors.sh:50), build-failure (:63) and missing-dist/cli.js (:72) branches have no test. Concrete regression this leaves open: soften if [ ! -x "$TURBO" ] to a warning, or drop the post-build [ ! -f "$CLI" ] check, and the script proceeds to exec node "$CLI" — with a stale dist/ it regenerates using yesterday's transform and exits 0, reporting success over output the guards will still reject, which is the exact "never a silent success over a no-op" AC-7 forbids. Nothing turns red. Fix: add a case pointing TOOLCHAIN_ROOT at a tree with no node_modules/.bin/turbo (or stub a failing turbo) and assert non-zero + the reason on stderr.

  • packages/dev-tools/src/quality-gates/regenerate-mirrors.test.ts:36execFileSync(REGENERATE, …) black-box-executes a shell script inside vitest, which ADL 2026-07-13-gate-tooling-code-in-tested-modules.md forbids in as many words: "no black-box spawnSync/exec of a script inside a vitest unit test … it uses the smoke-test suite (scripts/smoke-tests/), not vitest." run-format.test.ts already deviates the same way, so the practice is established — but the deviation is nowhere recorded, and CLAUDE.md requires decisions to be written down. As it stands the repo's adoption says one thing and two of its test files do another, with nothing telling the next author which wins. Fix: record the deviation (a line in the existing ADL, or in the new one) stating that a thin script whose behaviour is the deliverable is exercised from vitest against a throwaway fixture, and why the smoke suite is not the right home — or move the suite to scripts/smoke-tests/.

Questions (informational, never blocking — includes anything that only fires on a contrived scenario)

  • packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts:172-178 vs DEVELOPMENT.md:130-140 — AC-3 requires the load-bearing paragraph to stay byte-identical across three places, but PRE_PUSH_REMEDY is a materially shorter variant: it names only .claude/skills/** where both docs name .claude/skills/** and .pair/knowledge/**, and it omits the "pair update installs the PUBLISHED KB" clarification the docs now carry. The divergence pre-dates this PR and is harmless in effect (the one command regenerates everything), and the two docs are verifiably in lockstep — I diffed the paragraph blocks and they differ on exactly one line, the ADL link form, as the 2026-07-31 ADL requires. Question: is the docstring's "kept byte-identical (modulo the ADL link form) to the same paragraph in DEVELOPMENT.md and …development-setup.mdx" meant to describe the two docs only? As written it claims a three-way equality that does not hold for the constant it sits on.

  • DEVELOPMENT.mdapps/website/content/docs/contributing/development-setup.mdx — the byte-identical paragraph is hand-kept with no automated guard (I found no test referencing development-setup outside the website e2e navigation specs). This PR had to edit both copies by hand and got it right; the next one may not. Not raised as a finding because the ADL declares the copies hand-kept, but a #419-shaped drift class is exactly what a two-line conformance test would close.

  • CI secret-scan — FAILURE on this head — mechanically blocks merge, and is not this PR's doing: main @ e7d375bf (this PR's merge base) fails the same check in run 33410499948, and the single hit is a synthetic SHA-shaped fixture on the unrelated feature/US-398 branch (commit 5e99d538, scripts/smoke-tests/scenarios/pr-state-flow.sh:430, a file that exists on neither main nor this branch). Flagging so the merge gate is not read as a defect of [#419] feat: mirrors:regenerate — local, deterministic mirror realignment #476; it does need fixing on the base before anything merges.

  • packages/dev-tools/src/quality-gates/regenerate-mirrors.test.ts:202 and :42-47 — two nits in the same helper. expect(result.stderr).toContain('git') is satisfied by almost any failure message that mentions git, so it would not distinguish the "not inside a git working tree" path from an unrelated crash; and e.status is null when a child is killed by a signal, so expect(result.status).not.toBe(0) passes vacuously on a timeout kill. Consider asserting on 'not inside a git working tree' and on status !== 0 && status !== null. Also scripts/regenerate-mirrors.sh:59 leaks $BUILD_LOG if the script is interrupted between mktemp and rm — a trap 'rm -f "$BUILD_LOG"' EXIT costs one line.

Positive feedback
  • The phase-ordering argument is the strongest thing in the change and it is derived, not asserted: mirror drift is what turns the gate red, a red gate HALTs, so the card's Phase-2 placement would have been unreachable in its only case. The ADL records both that and the second, independent reason (a gate running first judges a tree the PR no longer contains).
  • Reading the command from adoption rather than naming it is the right call for a corpus that ships to other repos, and mirror-realignment.test.ts:57 enforces it negatively (expect(c).not.toContain('pnpm mirrors:regenerate')) — the assertion that actually prevents the regression.
  • regenerate-mirrors.test.ts runs the real script against a real fixture repo, and the "never fetches a published KB" test asserts the observable difference (~/.pair/kb absent under an isolated HOME) rather than trusting the flag. That is the difference between testing the story and testing the diff.
  • The scope extension to both guards' own failure messages was correct and correctly justified: AC-3's Given is a contributor reading a guard failure, and that message is what they read. Keeping the descriptive references to the pair update transform (what a mirror is compared against) is the right distinction, and mirror-guard.test.ts scopes its negative assertion to the remedy line so the distinction survives.
  • The 6655439d self-demonstration — running the new command on a clean main, finding real drift, committing it separately — is the strongest available evidence that the publish-pr step behaves as specified, and it is the shape the skill promises.
Functionality & requirements (AC coverage)
AC Verdict Evidence
AC-1 local regeneration, no published fetch met regenerate-mirrors.sh:83 (--source "$DATASET" --offline); tests at :117 and :132
AC-2 idempotent met test :150 — second run leaves git status --porcelain empty
AC-3 remedy renamed in all three places met (see Q1 on the byte-identical claim) PRE_PUSH_REMEDY, DEVELOPMENT.md:133, development-setup.mdx:85; plus both guards' messages — a justified scope extension
AC-4 publish-pr runs it, commits separately met, with the Major above on which paths get staged SKILL.md Phase 1 steps 3-4; mirror-realignment.test.ts:72
AC-5 no-op stays silent met SKILL.md Phase 1 step 4 first branch + conditional Mirrors: row; test :92
AC-6 pnpm format unchanged met git diff origin/main -- package.json = exactly one added line (:32); format/format:check byte-identical
AC-7 fail-loud met in the script, partially tested 4 guarded failure paths; 2 exercised (Minor #4)
AC-8 no check mode met test :205 asserts no --check/--dry-run in the script source

Edge cases: dirty tree covered by test :167; "drift in an untouched file" covered by prose + the real 6655439d commit; "hand-edited mirror is regenerated, not fixed" enforced by mirror-realignment.test.ts:80.

Testing & quality gates
  • Coverage: 7 script tests (real fixture repo), 13 conformance tests, 2 gate-composition tests, 1 mirror-guard test. Behaviour that lives in prose is guarded in prose; behaviour that lives in the script is guarded by executing the script. Appropriate split.
  • Edge + error scenarios: dirty tree, no-git, no-dataset, idempotence all tested. Gap: 3 of the script's fail-loud branches (Minor Workflow Customization #4).
  • Quality gates: PASS on CI build and smoke. secret-scan FAIL — pre-existing on the merge base, see Q3.
Adoption compliance
  • Degradation level: 1 (fully compliant surface) except the ADL 2026-07-13 script-testing rule — Minor Advanced Features Enablement #5.
  • No new dependency; no tech-stack.md or architecture.md delta. /pair-capability-assess-stack not required.
  • ADR/ADL present: the new ADL records both decisions (phase placement, adoption-declared command) with alternatives; the 2026-07-31 ADL's Open Decision is properly closed rather than left dangling. No missing decision record.
  • Conformance-test placement follows ADL 2026-07-18 (one file per target artifact).
Tech debt
  • The command literal exists in 6 places with 2 guarded (Minor Adoption Management & Guideline Linking #3) — the residual is one unguarded constant, not a systemic duplication.
  • packages/dev-tools' unit suite now transitively builds @pair/pair-cli on every run. Documented, turbo-cached, and matching run-format.test.ts; surfaced, not blocking.
Documentation
  • DEVELOPMENT.md + docs-site twin updated in lockstep, including a new troubleshooting row and both command tables.
  • way-of-working.md declares the adoption key with its blast radius.
  • skills-catalog.mdx bumped to match the skill's first sentence (skills:conformance requires it).
  • .pair/llms.txt indexes the new ADL.
  • The skill's own Notes bullet contradicts the new behaviour — Minor Setup & Project Management Integration #1.
Performance & deployment
  • No hot path touched. The added turbo build is cached after the first run.
  • Rollback path: the script is additive and the skill step is gated on an adoption key — deleting the mirror-realign-command line reverts publish-pr to its prior behaviour with no code change.

Bound re-review to immutable deltas and require a complete finite-state inventory before review or remediation.
rucka and others added 12 commits September 1, 2026 21:25
Require real boundary evidence for externally-defined state and repair claims.\n\nRefs: #416
…ment

- scripts/regenerate-mirrors.sh: thin wrapper over the CLI's existing
  `pair update --source <local dataset> --offline` path; no generation logic
- root script `mirrors:regenerate`; no check mode (the guards are the checker)
- fail-loud: non-zero + reason on no git tree, no dataset, no toolchain
- tests first: drift regenerated, idempotent, no published KB fetched,
  authored changes untouched, both failure paths
- Task: T-1 — Root script for local-source mirror regeneration

Refs: #419
…, not pair update

- PRE_PUSH_REMEDY names `pnpm mirrors:regenerate`; new MIRROR_REMEDY_SCRIPT is
  dead-advice-checked like REMEDY_SCRIPT (both remedy steps must exist)
- mirror-guard + skill-md-mirror print the same command, stated once as
  MIRROR_REGENERATE_COMMAND — the guard whose failure the contributor reads is
  the guard whose remedy AC-3 renames
- DEVELOPMENT.md + development-setup.mdx: paragraph stays byte-identical modulo
  the ADL link form; both command lists gain the script
- descriptive references to the `pair update` TRANSFORM are a different claim, untouched
- Task: T-2, T-3 — rename the remedy in the gate message and both docs

Refs: #419
- Phase 1 becomes "Realign Generated Mirrors, then Quality Gate": the step runs
  BEFORE the gate, because drift is what turns the gate red and a red gate HALTs
  — after it the remedy would be unreachable in its only case
- the command is read from adoption (`mirror-realign-command`), never named in
  the skill: publish-pr ships to every adopter, `mirrors:regenerate` is ours
- commits only the generated paths, alone, named a regeneration; no-op is silent
  (no commit, no output row); non-zero exit HALTs before any PR side effect
- way-of-working declares the key; ADL records the ordering + indirection
- conformance guard over the prose + whole-file mirror reproducibility
- Task: T-4, T-5 — wire the step into publish-pr and guard it

Refs: #419
Pre-existing drift this branch never touched: four adoption files still carry
bare `/skill-name` references the install-time rewriter resolves to their
prefixed form. Regenerated by `pnpm mirrors:regenerate`, committed separately —
exactly the shape /pair-capability-publish-pr now produces.

Refs: #419
… tests' timeout

- 2026-07-31-pre-push-gate-is-check-only.md: the Open Decision is closed by this
  story, with what actually shipped (incl. the two guard messages the card did
  not list) and a pointer to the phase-order ADL
- regenerate-mirrors.test.ts: explicit 120s per-test timeout — each case builds
  the CLI and runs a 7-registry regeneration, and vitest's 5s default is measured
  while turbo runs every other package in parallel (it flaked in the full gate)
- verified AC-6: `format`/`format:check`/`prettier:*`/`mdlint:*`/`quality-gate`
  are byte-identical to main; only `mirrors:regenerate` is added
- Task: T-6 — verify format unchanged and close the ADL

Refs: #419
…view round 1)

Major: publish-pr Phase 1 staged "the paths the command owns", resolved from
the adoption's owned-path globs. Root `.pair/**` is on that list and holds 117
tracked authored files under `.pair/adoption/**` — a contributor's unstaged
prose landed in `chore: regenerate mirrors from local dataset`. Now: snapshot
porcelain before and after, stage exactly what changed. No adopter enumerates
globs.

Also: Notes carve out the Phase-1 write instead of denying it; way-of-working
states the writer/checker scope asymmetry (guards check dataset-sourced
mirrors only) and marks its tree list descriptive; MIRROR_REGENERATE_COMMAND
now tied to root package.json; AC-7's missing-turbo / build-failure /
missing-dist branches tested, plus a TMPDIR-leak trap; refusal assertions no
longer pass vacuously on a signal kill; PRE_PUSH_REMEDY names both mirror
trees and its docblock scopes the byte-identity claim to the two documents;
ADL records the thin-script-in-vitest deviation; turbo inputs widened so none
of these guards can replay a stale PASS.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UQJzGMhRqBRRboxMrRqFPP
…ee rule (review round 1)

Minor findings 86783a8 did not reach.

mirror-guard.test.ts cast `find(...)` to string: reword the guard's remedy line and
the test dies with `TypeError: Cannot read properties of undefined` instead of naming
the broken contract. Asserted, not cast.

mirror-realignment.test.ts asserted `toContain('git add -A')` next to the negated
form — satisfied by the exact prose it exists to forbid ("stage everything with
`git add -A`"). Deleted; the regex proves presence and polarity.

ADL's rejected alternative said "Phase 2, after the push" — nothing is pushed at
Phase 2 (the PR is created in Phase 4), so a later reader is given a reason that is
not the one the same paragraph gives. Now "after the gate".

regenerate-mirrors.sh: TOOLCHAIN_ROOT is the script's own tree, so a fresh
`git worktree add` (no node_modules) exits 1 and publish-pr HALTs. Verified, and
intended — that tree fails the gate one step later anyway. Header says so.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UQJzGMhRqBRRboxMrRqFPP
… the CLI it runs (review round 2)

Minor 1 — publish-pr Phase 1 compared two `git status --porcelain` snapshots to
decide what the realign command wrote. A porcelain entry encodes STATUS, not
content: HEAD carries a drifted mirror, the contributor holds an uncommitted
edit to that same file, the command regenerates it — same unstaged-modified
entry on both reads. The agent read NO CHANGE: hand-edit destroyed with nothing
reported, stale mirror still pushed, `skills:conformance` red on the PR this
step exists to keep green. And the step-4 Verify ("git status still shows every
pre-existing unstaged authored change") PASSED on exactly that state. Now the
before snapshot carries a `git hash-object` digest of every dirty path; the
staged set adds the pre-dirty paths whose digest moved; those are named on the
`Mirrors:` row (`overwrote uncommitted changes in: <paths>`); the Verify reads
the digest, not the listing. Measured against the real script:
regenerate-mirrors.test.ts, 'overwrites a pre-dirty mirror while
`git status --porcelain` stays byte-identical'.

Minor 2 — regenerate-mirrors.test.ts AC1/AC2 build and run apps/pair-cli and
assert the real `pair update --source` transform, but @pair/dev-tools declares
no dependency on @pair/pair-cli, so no input or task edge covered that tree.
Measured at 0a6712e: a comment appended to
apps/pair-cli/src/registry/skill-refs.ts replayed `1 cached, 125ms >>> FULL
TURBO`. `dependsOn: ["build", "@pair/pair-cli#build"]` on #test and
#test:coverage — a task edge follows the whole closure and cannot go stale like
a hand-listed set of trees. After: 2 cached / 4; content-ops probe 0 cached.
web-cloud-environment.test.ts gains a requiredDependsOn guard, mutation-proven.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UQJzGMhRqBRRboxMrRqFPP
Phase 1's before-snapshot ran `git hash-object <path>` over every dirty porcelain
entry. Two of three ordinary shapes are fatal: ` D gone.md` -> "could not open …
for reading" (exit 128, which the step's own non-zero -> HALT turns into a PR
blocked by its own snapshot pass), `?? newdir/` -> "Unable to hash newdir/" — and
that entry is identical before and after however many files under it the run
rewrote, so the untracked subtree kept the status-vs-content blindness the digest
was added to close.

Now: `--untracked-files=all` (per-file `?? dir/a.md` entries, hashable), digest
only entries whose worktree file exists (a recreated deletion moves its porcelain
entry anyway), and `-w` so the overwritten bytes land in the ODB — the Mirrors:
row now carries `(recover: git cat-file -p <sha> > <path>)` instead of naming a
loss nobody can undo.

Executed against the real script (regenerate-mirrors.test.ts): all three shapes in
one fixture, snapshots byte-identical before/after while both digests move, and
`git cat-file -p` prints the hand-edits back.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UQJzGMhRqBRRboxMrRqFPP
…commit (review round 4)

- mirror-guard: the MISSING branch and the orphan-sweep remedy now name
  MIRROR_REGENERATE_COMMAND, like the drifted branch already did. Both are
  reached by ADDING a dataset file: `pair update` installs the published KB,
  so the new file is in no release and every other mirror is overwritten
  with released content.
- publish-pr Phase 1 snapshot is `git status --porcelain -z -uall`: the
  default format quotes/octal-escapes a path with a space or a non-ASCII
  byte, so the entry was silently dropped from the digest (and `git add`
  refused the quoted string as a pathspec).
- the regeneration commit is made BY PATHSPEC: a plain `git commit` after
  `git add <paths>` commits the whole index, sweeping in prose the
  contributor staged before the run.
- skill-md-mirror docblock + unreadable-path message name the constant.
…s reason (review round 5)

A pathspec resolves against paths git already knows, so the newly CREATED
mirror — the shape a contributor gets by adding a dataset file — aborts the
commit with 'did not match any file(s) known to git'. Stage first, always.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…f; HALT on untracked files under written trees (review round 6)

Three round-6 findings on PR #476, all measured against the real script in a fixture repo:

- Major: a mirror registry deletes a contributor's untracked/staged-new draft under it; its vanished
  `??` (or `A.`→`AD`) entry put it in the step-4 set and `git add`/the pathspec commit aborted Phase 1
  after the destructive run, with no report row. Such paths now leave `git add` and the pathspec and
  are named `removed untracked: <path> (recover: git cat-file -p <sha> > <path>)`.
- Minor: a path whose render equals HEAD moves its entry when rewritten (`M.`→`MM`, `D.`→`D.`+`??`,
  `.M`→gone) but equals HEAD after `git add`; `git commit -- <paths>` over only such paths exits 1
  (`nothing to commit`). Recipe now runs `git diff --cached --quiet -- <paths>` first; recover rows
  are driven by the digest comparison whether or not a commit was made; Verify compares against the
  cached name list.
- Minor: the writer indexes untracked `.pair/adoption/**` files into `.pair/llms.txt` (dangling link
  + WIP filename in history). Step 3 now HALTs on `??`/`A.` entries under the trees the adoption names
  as written, remedy `git stash push -u -- <paths>` / `git stash pop`; stated in way-of-working and
  the script header.

Tests: 3 real-script cases (regenerate-mirrors.test.ts), 4 conformance pins (mirror-realignment.test.ts).
.claude mirror regenerated via `pnpm mirrors:regenerate`, never hand-edited.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FfhvsS5rippi6aUbbGrf6F
@rucka
rucka force-pushed the main branch 2 times, most recently from 7b55900 to adb9627 Compare September 8, 2026 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-state:to-be-reviewed PR state: awaiting review / gate risk:yellow Classification: medium risk tier tech-debt Tracked technical debt (living backlog, R7.2 — never blocks a PR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant