[US-394] fix: pre-push gate checks formatting instead of applying it (a write-mode formatter cannot fix the commits being pushed)

[rucka]

PR Information

Story: #394 · Type: Bug fix (repo tooling) · Priority: High (P1) · Assignee: @rucka
Classification: risk:yellow · cost:green

Summary

Story Context

As a contributor pushing a story branch I want the pre-push hook to run in check mode so that I stop having to git push --no-verify, and the hook provides assurance again (#394).

Acceptance criteria — #394 (recorded post-hoc together with the classification matrix: the story was implemented straight from Draft, like #407):

AC	Where it is satisfied
AC1 — pushing a branch never modifies a file the branch did not change	quality-gate runs format:check instead of prettier:fix/mdlint:fix (root package.json), and pnpm gate:composition fails if ANY write-mode step (both formatters, their .sh/bin forms, raw --write/--fix CLIs, and eslint's autofix) becomes reachable from the gate — packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts + 43 unit assertions.
AC2 — a formatting violation in a changed file still fails before push	pnpm format:check = pnpm prettier:check && pnpm mdlint:check (delegating, so check and fix cover the same set, root *.md included) exits 1 and the failure names pnpm format as the remedy.
AC3 — --no-verify is no longer needed for a normally formatted change	The baseline drift is cleared once (the two pair-cli version-check files) and generated trees can no longer block a push: both wrappers delegate to the repo's .gitignore files, covered by scripts/smoke-tests/scenarios/format-ignore-delegation.sh.

What Changed

The quality-gate script — which the husky pre-push hook runs — executed prettier:fix and mdlint:fix repo-wide in write mode. It now runs them in check mode, with the fix moved to an explicit pnpm format.

Script	Role
format	pnpm prettier:fix && pnpm mdlint:fix — the explicit fix command (was the gate's write-mode step)
format:check	pnpm prettier:check && pnpm mdlint:check — what the gate runs now
gate:composition	a tested guard that fails if any write-mode step (formatter or eslint autofix) is ever reachable from the gate again

Both format and format:check delegate to the existing root scripts rather than re-typing their bodies, so check and fix cover the same file set by construction — including root-level *.md, which only the root mdlint:* scripts reach (turbo <task> runs workspace members only, and the repo root is not one).

Why This Change

The story frames it as noise. The decisive problem is uselessness: at pre-push the commits already exist, so a write-mode formatter rewrites the working tree and cannot touch what is being pushed. Its output goes nowhere unless the author notices and amends — so the author either sweeps unrelated reformats into the next commit or pushes with --no-verify. Once bypassing is routine, the hook asserts nothing at all.

Observed three times in two days, on the same two files:

• [US-281] feat: assess-cost report mode — cost monitoring panel #388 was pushed with --no-verify after the hook reformatted two unrelated pair-cli test files;
• those same two files were swept into [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411 by a git add -A and had to be reverted;
• and excluded by hand from [US-278] feat: bootstrap quick mode — a second resolution depth, guided stays the default #408.

Recorded as ADL 2026-07-31-pre-push-gate-is-check-only.md — context, decision, alternatives (including the rejected lint-staged / pre-commit auto-fix the story offered) and consequences. way-of-working.md carries a one-line pointer, per the sibling ADL's precedent.

Changes Made

Implementation Details

• The guard reads the repo's own package.json and survives indirection. The gate no longer names a formatter — it delegates (pnpm format:check), so a string scan of the gate would be defeated by the very indirection this PR introduces. expandScriptReferences inlines runner references transitively against the root scripts (bounded depth + visited set) before scanning, so both likely regressions fail: the gate calling pnpm format, and format:check redefined as turbo prettier:fix. It tolerates runner flags and the npm/yarn spellings (pnpm -s format, pnpm -w format, npm run format, yarn format) — a single token between the runner and the script name would otherwise capture the flag, miss the lookup and leave the referenced body unscanned. The guard additionally fails if the gate stops running the guard itself (a mention such as echo gate:composition does not count — the check is a runner invocation, not a substring), or if the remedy it advertises (pnpm format) stops existing. A malformed root package.json returns a structured failure, not a raw SyntaxError stack.
• The offender list is explicit and symmetric across the two tools — for each: the turbo task (prettier:fix, mdlint:fix), the package bin alias / .sh entrypoint (prettier-fix, markdownlint-fix), and the raw CLI with its write flag (prettier --write, markdownlint --fix), the CLI patterns bounded by command separators so a --write belonging to another command cannot incriminate prettier:check. Since round 4 the list also carries eslint's autofix (lint:fix, lint-fix(.sh), eslint … --fix) and, since round 5, the two root scripts in this repo whose job IS to write: sync-version (→ sync-version-in-docs.ts, writeFileSync across every .md/.mdx it walks — its --check dry-run is spared, bounded to the same command segment) and test:perf (→ benchmark-update-link.ts, a scratch KB tree + reports/performance/benchmark-report.json, no dry-run, banned outright). Still not a /:fix/ pattern: a guard that fires on things it has no opinion about gets disabled; a guard covering one tool's shell form and not the other's leaves the regression open.
• Generated artifacts: gitignored ⇒ never checked, by construction. Both formatter wrappers now pass the repo's own .gitignore files (git root + the package being checked) as additional ignore sources — prettier 3.6 accepts --ignore-path repeatedly; markdownlint-cli accepts only one, so the sources are concatenated into a temp file (it parses gitignore syntax). A new generated dir no longer needs a line in tools/prettier-config/.prettierignore (the enumerated entries there stay on purpose — they are the floor for non-git contexts, where git rev-parse fails and the delegation no-ops; no pattern was removed from that file in this diff — the point is that the delegation is what now covers the generated trees: .source/ is carried by apps/website/.gitignore:2 and **/test-results/ by the root .gitignore:25, neither of which had to be restated in a formatter ignore list). playwright-report/ (produced by the documented a11y:report:html), test-results/, .source/ and a local apps/pair-cli/.pair/ install tree can no longer block a push. In write mode the gate silently rewrote such trees; in check mode they would have blocked every push — a generated file must be able to do neither, and enumerating them reactively only re-arms the failure.
• The ignore assembly lives in ONE sourced helper per tool (tools/prettier-config/bin/_ignore-args.sh, tools/markdownlint-config/bin/_ignore-file.sh), because check and fix seeing the same file set is the invariant and four copies is how it drifts — invisibly, since dup:check scans TypeScript only. The prettier args are assembled positionally, so a repo path containing a space no longer word-splits (unquoted, prettier exited 2 on every push for that contributor and the gitignore delegation silently stopped applying), and the cwd/git-root de-dup compares canonical paths (pwd -P), so it is not dead code through a symlinked clone or a macOS /tmp path.
• The root .gitignore is re-anchored to the cwd for markdownlint (tools/markdownlint-config/bin/_reanchor-gitignore.awk). The two tools resolve those patterns against different bases: prettier against the ignore file's own directory (git semantics), markdownlint-cli against the cwd — so a path-anchored entry (apps/website/gen/) matches nothing from inside that package and would block every push from it. The awk pass strips the cwd prefix from anchored patterns, drops those outside cwd, keeps **/-style and non-anchored ones verbatim, and preserves order (last-match-wins) and negations. It holds today only because the current root entries happen to be **/-style.
• turbo.json declares the ignore sources as globalDependencies (root .gitignore, the shared ignore files, both bin/ dirs). The formatter tasks are cacheable, so without it a .gitignore edit touches no package file and turbo replays a pre-change result — a stale PASS, or a stale FAIL a developer cannot clear. globalDependencies is the widest lever available — it folds into turbo's GLOBAL hash, so the same edit also busts build/test/ts:check/lint/test:coverage caches that cannot be affected. That blast radius is accepted deliberately, and the // comment now says so and why the narrower per-task inputs option was passed over (it hardcodes the ../../ depth four times and degrades silently to a stale cache when a fifth formatter task is added, whereas over-approximation only fails towards extra work).
• The enforced invariant is "no step reachable from the gate WRITES files" — not merely "no write-mode formatter". eslint's autofix is on the offender list too (lint:fix, lint-fix(.sh), eslint … --fix): it modifies files the branch never touched exactly like a formatter, i.e. AC1's failure mode, and nothing in the gate runs it today — which is when widening a guard is free. Round 5 extended it to sync-version and test:perf per above. Recorded in the ADL's Decision and in way-of-working.md — which now also state plainly that what is enforced is the explicit list, not the invariant in general: a newly added, differently named writer passes the guard green and is caught by review, so "add it to WRITE_MODE_FORMATTERS" is part of adding a write script to this repo. The previous wording overclaimed.
• Expansion boundary, stated in the docstring: root scripts only. A pnpm --filter <pkg> <script> body is never read, so a package-level write script is caught by NAME, not by expansion — every one in this repo is named prettier:fix / mdlint:fix / lint:fix / sync-version / benchmark-update-link, all matched.
• ADR-014 shape: logic in an importable, unit-tested module + main() behind require.main === module — same shape as the three siblings in packages/dev-tools/src/quality-gates/. The __dirname hop count to the repo root now lives in ONE module, packages/dev-tools/src/quality-gates/repo-root.ts, imported by all four gate tools in the folder (it had drifted into four copies of the same resolve(__dirname, '..', '..', '..', '..'); ADR-014 records a folder move breaking exactly that once already).
• Both copies of the contributor docs are corrected. DEVELOPMENT.md and its published twin apps/website/content/docs/contributing/development-setup.mdx carried the same (now wrong) gate list; the mdx also documented the husky hooks incorrectly (pre-commit runs pnpm ts:check, pre-push runs the whole gate). Nothing enforces equality between the two — docs:staleness checks skill/CLI counts and dead /docs links, and format:check does not read apps/website/content prose — so the ADL's Consequences now state that a future gate change has to touch both. Round 5 goes further: the load-bearing paragraph and the root-command block are byte-identical between the two files except for the ADL link form (relative path vs GitHub blob URL), so diffing the two paragraphs yields exactly one line when they are in sync — a hand-kept pair that starts life already unequal never gives you that signal. (They had diverged in two spots: "a write-mode step" vs "a write-mode step (formatter or eslint autofix)", and a (prettier only) annotation present in one command block only; pnpm mdlint:check was missing from DEVELOPMENT.md entirely.)
• The advertised remedy is TWO steps, because format scope and mirror scope disagree. packages/knowledge-hub/dataset/.skills/<cat>/<name>/SKILL.md is in format scope (workspace package); its generated twin .claude/skills/pair-<prefixed>/SKILL.md is not (.claude/ is not a workspace member — pnpm-workspace.yaml lists apps/*, packages/*, tools/*, and the root mdlint:check extra step globs a non-recursive '*.md'). Meanwhile packages/knowledge-hub/src/tools/skill-md-mirror.ts asserts the two are byte-equal by regenerating the .claude copy through the real pair update transform. So a format-only edit to the dataset copy turns a green format:check into a red skills:conformance later in the same gate, and a contributor told only "run pnpm format" loops straight back to --no-verify. The mechanism pre-dates this PR (the old gate ran mdlint:fix in-gate, hiding it), but this PR is what promotes pnpm format to the documented happy path, so the two-step remedy — re-sync the .claude/skills/** copies (pair update) in the same commit — is now stated in all three places that advertise the remedy: DEVELOPMENT.md, the development-setup.mdx twin, and PRE_PUSH_REMEDY (with a unit test pinning it). Hit for real here: clearing main's MD049 drift broke the mirror guard until the twin was propagated. The structural fix (one format scope for both copies) is noted on [tech-debt] format:check does not cover files outside workspace packages (.pair/**, scripts/**, root-level JSON) #414, whose body listed .claude/**/*.json but neither .claude/skills/**/*.md nor the mirror coupling.
• format:check aggregates instead of short-circuiting. pnpm prettier:check && pnpm mdlint:check meant one prettier violation hid every markdownlint violation in the tree, costing a second push cycle in exactly the situation the new gate creates (push fails → pnpm format → push → fails again on markdown). Now pnpm prettier:check; _p=$?; pnpm mdlint:check; _m=$?; exit $((_p || _m)) — one run names everything to fix. Verified with a deliberate violation of each kind: the && form reported only the prettier one.
• _reanchor-gitignore.awk handles gitignore's negated bracket form. [!abc] was passed through to ERE unchanged, where negation is ^ — so it became a class matching a literal !, a, b, c, i.e. the inverse set, and the pattern got silently dropped. Now translated to [^abc]. The "Known approximation" note previously covered only mid-pattern **; it now also records that bracket expressions pass through with that single fixup, so a class whose first character is a literal ] ([]abc]) still breaks. No root .gitignore entry in this repo uses brackets, so impact today is nil — but the next author was going to read bracket support as complete.
• _ignore-file.sh documents that its EXIT trap REPLACES the caller's. POSIX trap assigns rather than composes; the file's "Effect:" line advertised only that it registers cleanup. The two current callers set no trap, so nothing is broken — but a third would have lost its own cleanup silently and discovered it as a leaked temp file.

Files Changed

• Added: packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts + .test.ts, packages/dev-tools/src/quality-gates/repo-root.ts, .pair/adoption/decision-log/2026-07-31-pre-push-gate-is-check-only.md, tools/prettier-config/bin/_ignore-args.sh, tools/markdownlint-config/bin/_ignore-file.sh + _reanchor-gitignore.awk, scripts/smoke-tests/scenarios/format-ignore-delegation.sh
• Modified: root package.json, turbo.json, scripts/smoke-tests/run-all.sh, packages/dev-tools/package.json, tools/prettier-config/.prettierignore, tools/prettier-config/bin/prettier-{check,fix}.sh, tools/markdownlint-config/bin/markdownlint-{check,fix}.sh, .pair/adoption/tech/way-of-working.md, .pair/adoption/tech/tech-stack.md, DEVELOPMENT.md, apps/website/content/docs/contributing/development-setup.mdx, the three sibling gate tools in packages/dev-tools/src/quality-gates/ (two lines each: they import the shared REPO_ROOT), the two pair-cli version-check test files (the one-time sweep, see below), and — as its own baseline-clearing commit — packages/knowledge-hub/dataset/.skills/capability/assess-cost/SKILL.md + its generated twin .claude/skills/pair-capability-assess-cost/SKILL.md

Testing

Test Coverage

Test-first: the guard was written and verified RED against the repo's own package.json (quality-gate runs 3 formatter(s) in WRITE mode: prettier:fix, mdlint:fix, markdownlint-fix.sh) before the gate changed.

50 assertions: each offender individually · all offenders at once, so a partial fix cannot look clean · lint:fix / lint-fix.sh / eslint … --fix flagged (plus the negative: a --fix belonging to another command leaves check-mode eslint alone), and the real gate with && pnpm lint:fix appended now fails · sync-version flagged by script name and by module path, its --check dry-run spared, and a --check in the next command not laundering the write · test:perf and benchmark-update-link flagged · the real gate with && pnpm sync-version 0.4.2 and with && pnpm test:perf appended now fails · the remedy naming the dataset↔.claude mirror re-sync · the remedy named in the message · transitive expansion (one hop, multi-hop, pnpm run, self-reference, cycle, unknown reference) · transitive expansion through runner flags and the npm/yarn spellings (pnpm -s format, pnpm -w format, pnpm --silent format, pnpm -r run format, npm run format, yarn format, yarn run format) · every offender shape per tool (bin alias, .sh entrypoint, raw CLI write flag) plus the negative case where the write flag belongs to another command · the two indirection regressions (gate → pnpm format, format:check → turbo prettier:fix) · the gate MENTIONING the guard (echo gate:composition) instead of running it · the gate dropping the guard · the advertised remedy missing · malformed JSON · a missing gate failing loudly rather than passing vacuously · and the real root package.json (twice — via text and via the module's own reader, so unplugging the guard from the gate fails the suite the gate runs).

Wrapper behaviour is covered by a smoke test, per the gate-tooling ADL (2026-07-13: script/CLI surfaces go to smoke tests, not unit tests): scripts/smoke-tests/scenarios/format-ignore-delegation.sh builds a throwaway git repo whose .gitignore holds a path-anchored pkg/gen/, runs all four wrappers from pkg/, and asserts per tool and per mode that the gitignored file is neither reported nor rewritten while the same file outside the ignored path still is (so the mechanism cannot pass by ignoring everything), plus a repo path containing a space. It also asserts each wrapper's exit status on the violation path (1 = violations found, anything else = broken wrapper) and the absence of tool-error output: both tools print the offending path inside their own error text, so a text-only assertion passes on a wrapper that died — verified by stubbing a wrapper that exits 2 with [error] Cannot find package …, which now FAILS the check-mode half. The probe repo carries its own package.json + .prettierrc.json so prettier's config discovery cannot escape the fixture. Finally, _reanchor-gitignore.awk is covered table-driven in the same scenario — one fixture .gitignore through three cwd positions, diffed line-by-line, exercising anchored-inside-cwd, a glob path segment, ignored-dir-is-cwd, cwd-inside-the-ignored-tree, dropped-outside-cwd, **/-verbatim and a preserved negation (its branches fail as a silent OVER-ignore, not a loud error). Verified RED against the pre-fix wrappers on both mechanisms (markdownlint reported the gitignored gen/bad.md; prettier exited 2 on the spaced path) and against a deliberately broken awk (negation dropped). Round 5 adds a bracket-negation row to that table (apps/[!x]ebsite/build/), verified RED against the unfixed awk — the pattern was dropped rather than re-anchored, i.e. a silent over-check. 21 assertions total. Wired into the CI list in run-all.sh.

Test Results

Re-run on the rebased tree (8f8956bd + this branch), not the pre-rebase one:

Gate step	Result
turbo ts:check lint (14 tasks)	PASS
@pair/dev-tools test	PASS — 109 tests, 50 in pre-push-gate-composition.test.ts
@pair/pair-cli test	PASS — 999
@pair/content-ops / @pair/brand / @pair/website test	PASS — 633 / 78 / 74
@pair/knowledge-hub test	2 failed / 1005 passed — both pre-existing on pristine origin/main, see the callout above
format:check	PASS (exit 0) — was exit 1 before the baseline commit
gate:composition	PASS — ✓ pre-push gate composition: check-mode only
hygiene:check / docs:staleness / skills:conformance	PASS / PASS — 41 skills, 9 commands / PASS — 41 skills
dup:check	PASS — 18 pre-existing clones, none in touched files
format-ignore-delegation.sh smoke	PASS — 21 assertions

The only red is @pair/knowledge-hub test — resolved. Both failures were main's, not this branch's (an assess-cost word order the #389 guard read as a violation, and a quick-mode-defaults.md mirror link); they are fixed on main and this branch is rebased onto that fix. The full gate is green here: 22 + 6 + 6 tasks.

Quality Assurance

Review Areas

• Check mode vs lint-staged. The story offered both. Check mode wins on the argument above: at pre-push, scoping a write to staged files still cannot fix the commits being pushed. Alternatives and the reasons for rejecting them are now in the ADL rather than only here.
• Enforcement point. CI runs lint and coverage but no format:check, and --no-verify still skips the hook. Deliberately unchanged (CI workflow edits are out of scope for [tech-debt] Pre-push quality-gate runs prettier:fix/mdlint:fix repo-wide in WRITE mode — authors bypass the hook #394) — tracked as [tech-debt] CI does not run format:check — the check-only pre-push gate is the only enforcement point (--no-verify skips it) #413, and recorded in the ADL's Consequences.
• Format coverage still excludes .pair/**, scripts/** and root-level JSON (pre-existing): no workspace package's cwd contains them and the root markdown check globs a non-recursive '*.md'. Unchanged here — tracked as [tech-debt] format:check does not cover files outside workspace packages (.pair/**, scripts/**, root-level JSON) #414.
• Framework guidance (.pair/knowledge/.../quality-gates.md + the bootstrap dataset template) still shows prettier:fix as the example gate command, so a newly bootstrapped project inherits the anti-pattern. Framework-audience decision, deliberately not made here — tracked as [tech-debt] Framework guidance still shows a write-mode formatter as the pre-push gate command (bootstrapped projects inherit the #394 anti-pattern) #415.

Notes for reviewers

Adoption edits, declared. way-of-working.md › Quality Gates (gate contents, the "no step writes" bullet + ADL pointer, Custom Gate Registry row 1) and tech-stack.md (markdownlint wired via mdlint:check only). Conflict expectations are in Dependencies & Related Work below.

Story classification recorded post-hoc. #394 was implemented straight from Draft (no refinement pass), so it carried no risk:* label: the classification matrix backing risk:yellow · cost:green is now written into the issue body with a note saying so, and the risk:yellow label is applied. Same handling as #407.

Baseline cleared on the tree this lands on. The branch was rebased onto current origin/main twice during round 5 (#389/#391 merged, then #405/#408). Current main carried 2x MD049/emphasis-style at packages/knowledge-hub/dataset/.skills/capability/assess-cost/SKILL.md:155 (*as of then* in a file whose established style is underscore) — inside mdlint:check's scope, so without clearing it pnpm format:check would exit 1 for every contributor the moment this merged, and the pre-push hook would block every push on a file nobody touched: the exact pathology #394 exists to remove, and a direct miss of AC3. Cleared in its own commit (1a4dde8e), with the generated .claude twin propagated by hand — which is how the two-step-remedy trap above was found.

One-time sweep. The two pair-cli version-check files are formatted in this PR. Sweeping unrelated files in the PR that forbids sweeping them is deliberate: the drift has to be cleared once for check mode to be viable, and after this the gate cannot produce such sweeps.

Dependencies & Related Work

• Already merged — resolved in the rebase onto 8f8956bd (4dafdaef): [US-236] feat: code host separate from PM tool (WoW override) — GitHub + Linear reference case #389 and [US-279] feat: pair-cli scaffold-kb — external KB repo + release script (reuses pair package) #391 merged 2026-07-31, then [US-372] feat: coverage baseline auto commit-back — monotonic ratchet as a bot PR, opt-in and off by default #405 and [US-278] feat: bootstrap quick mode — a second resolution depth, guided stays the default #408. What that rebase settled:
  • [US-279] feat: pair-cli scaffold-kb — external KB repo + release script (reuses pair package) #391 › packages/dev-tools/src/quality-gates/sync-version-in-docs.ts — a real content conflict (main inserted a "RULE FOR DOC AUTHORS" paragraph exactly where this PR replaces the REPO_ROOT is resolved from this file's location… sentence). Resolved keeping BOTH: main's doc-author rule plus this PR's REPO_ROOT comes from ./repo-root sentence.
  • [US-234] feat: PR state flow (gate≠review) + pair review as required check #390/[US-279] feat: pair-cli scaffold-kb — external KB repo + release script (reuses pair package) #391 › scripts/smoke-tests/run-all.sh and [US-236] feat: code host separate from PM tool (WoW override) — GitHub + Linear reference case #389/[US-372] feat: coverage baseline auto commit-back — monotonic ratchet as a bot PR, opt-in and off by default #405 › way-of-working.md — auto-merged, as expected.
• Still open, still to sequence against (whichever merges second resolves it):
  • [US-277] feat: .claude-plugin marketplace + plugin manifests with dataset catalog guard #386 touches the same two files this PR sweeps for formatting — apps/pair-cli/src/commands/kb-info/version-check.test.ts and version-check-formatter.test.ts (the exact pair that had to be reverted from [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411). If [US-277] feat: .claude-plugin marketplace + plugin manifests with dataset catalog guard #386 merges first, re-run pnpm format on them here; if this PR merges first, [US-277] feat: .claude-plugin marketplace + plugin manifests with dataset catalog guard #386 rebases onto the formatted version.
  • [US-234] feat: PR state flow (gate≠review) + pair review as required check #390 (and [US-402] docs: PM-tool adapters document membership + assignee — an item can no longer be written and stay invisible #404) carry live hunks on way-of-working.md — a small conflict on the Quality Gates bullet, which this PR rewords again in round 5.
• Otherwise file-disjoint from the open set ([US-277] feat: .claude-plugin marketplace + plugin manifests with dataset catalog guard #386, [US-234] feat: PR state flow (gate≠review) + pair review as required check #390, [US-402] docs: PM-tool adapters document membership + assignee — an item can no longer be written and stay invisible #404, [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411): husky, tools/prettier-config/, tools/markdownlint-config/ and turbo.json are untouched by all of them. The root package.json was shared with the changesets release PR chore: release v0.5.0 #417 (version + CHANGELOG hunks only, scripts untouched, so either merge order auto-merged) — chore: release v0.5.0 #417 is now closed and the pending changeset removed from main, so that overlap is gone.

Important

main is red independently of this PR, and pnpm turbo test therefore fails on this branch too. Both failures were reproduced on a pristine origin/main worktree (8f8956bd), in files #394 never touches:

1. skill-md-mirror › process/bootstrap/quick-mode-defaults.md — [US-278] feat: bootstrap quick mode — a second resolution depth, guided stays the default #408 merged with an unregenerated .claude mirror. This is precisely the dataset↔.claude coupling round 5 documents.
2. code-host-routing AC4 › dataset/.skills/capability/assess-cost/SKILL.md:106 — [US-281] feat: assess-cost report mode — cost monitoring panel #388 prose ("read the window's merged PRs … from the PM tool / code host") against [US-236] feat: code host separate from PM tool (WoW override) — GitHub + Linear reference case #389's audit regex, a merge-order collision between two already-merged PRs.

main's own CI run for #408 is failing. Neither is in scope here; both need their own fix (a pair update regeneration, and a prose or regex adjustment respectively). Everything else in the gate is green on the rebased tree — see Quality Gates below.

• Removes the cause of a class of defect seen in [US-281] feat: assess-cost report mode — cost monitoring panel #388, [US-278] feat: bootstrap quick mode — a second resolution depth, guided stays the default #408 and [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411.

Closes #394

🤖 Generated with Claude Code

[rucka]

Verdict

risk:yellow · cost:green — CHANGES-REQUESTED — the direction is right and well argued, but check mode landed with a hole in coverage (root-level *.md is no longer checked at all), a guard that the PR's own indirection defeats in one word, and one generated-artifact dir still able to block every push — so AC2/AC3 are not fully met.

PR: [#412] · Author: rucka · Reviewer: independent reviewer (Claude) · Date: 2026-07-31 · Story: [US-394] · Type: config (repo tooling)

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	yellow	KB default	Integration & Process Standardization — supporting subdomain; the gate is on every contributor's push path
Change/diff risk	yellow	diff footprint	10 files, +195/-9; root package.json gate string + shared ignore files are repo-wide blast radius
Business impact	green	subdomain class	No product surface, no runtime code path
Security relevance	green	inline security read	No untrusted input, no secrets, no network/auth surface (raise-only, D17)
Coupling balance	yellow	inline coupling read	New text-contract coupling between the root gate string and a hardcoded offender list — see Major #2

Tier = max(assessed) = yellow — confirms the story's refinement-time risk:yellow; no drift. Cost = green. Review value is a floor (D17): confirmed, not raised.

Assessments

Security — Input validation

Verdict: green — the only new input is the repo's own package.json, read from a __dirname-relative path; no user/network input enters the diff.

Details

• pre-push-gate-composition-cli.ts:10-11 reads a fixed local file and passes the text to a pure function; no argv, env or stdin is consumed.
• checkRootGate does JSON.parse on that text — a local, trusted, developer-owned file. Not a validation risk, but it is unguarded (Minor chore: update and reorganize guides, knowledge base, and documentation structure #8: a malformed file surfaces as a raw SyntaxError, not as a gate message).

Security — Output handling

Verdict: green — output is console.log/console.error of a template string built from a hardcoded allow-list plus a filename; no encoding surface.

Security — Authentication

Verdict: green — no authentication path exists in or near the diff.

Security — Authorization

Verdict: green — no access-control path exists in or near the diff. Worth noting positively: the change removes a filesystem-write side effect from the pre-push hook, i.e. it reduces, not increases, what the hook is allowed to do.

Security — Introduced vulnerabilities

Verdict: green — 0 introduced, 0 pre-existing in the touched surfaces.

Details

Severity	Category	File:location	Introduced / pre-existing	Recommendation
—	—	—	—	none

No secret, credential, injection sink, deserialization of untrusted data or dependency addition. process.exit(1) in a dev CLI is intended behaviour.

Cost

Verdict: cost:green — no paid service, no infra, no new dependency; net compute goes down (check mode is cheaper than write mode, and CI is untouched).

Details

Signal	Class	Provider	Note
New dependency	green	—	None added; ts-node, vitest already present in @pair/dev-tools
CI/CD minutes	green	GitHub	.github/workflows/ci.yml untouched
Local gate runtime	green	—	One extra ts-node invocation (~1s) offset by dropping two write-mode passes
Managed service / storage	green	—	None

Architecture (Coupling)

Verdict: yellow — one new integration, deliberately text-based, and it is under-specified for the regression it exists to prevent.

Details

• Integration point: packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts ↔ the text of the root package.json quality-gate script.
• Integration strength: intrusive — a string-inclusion contract over another file's script body, with no shared schema. That is the right shape for this job (the alternative is a comment, which enforces nothing), but it means the contract is only as wide as the strings it enumerates.
• Socio-technical distance: minimal — same repo, same team, both files in this PR.
• Volatility: the root quality-gate string is edited by many stories (pair-capability-verify-quality integrated with the gate matrix (local = CI) #259, [#312] docs: website IA restructuring (Option A) — hub, dissolve guides/, dedupe, extend staleness gate #315, Skill corpus effectiveness: trigger descriptions, shared references, progressive disclosure, sharpened criteria, router fixes, authoring standard + eval #313 all touched it) — high volatility against an intrusive contract.
• Balance: intrusive + volatile is the combination the model flags. Here it is acceptable because the coupling is the feature — but it must be complete to be balanced, and today it is not (Major Collaborative Knowledge Base #2: the gate can delegate to pnpm format and the guard sees nothing).

Details

Findings by severity

Critical (must fix before merge)

• none

Major (should fix before merge)

• package.json:35 — "format:check": "turbo prettier:check mdlint:check" drops the root-level markdown check entirely. turbo <task> only runs tasks in workspace members (pnpm-workspace.yaml = apps/*, packages/*, tools/*); the repo root is not one, and turbo.json declares no //#mdlint:check. Root-level *.md was previously covered by the gate's own ./tools/markdownlint-config/bin/markdownlint-fix.sh '*.md' — which the new format script keeps but format:check omits. Net effect on the six root docs (README.md, CLAUDE.md, AGENTS.md, CONTRIBUTING.md, DEVELOPMENT.md, RELEASE.md): a markdownlint violation there now passes the gate, and pnpm format will rewrite it later inside an unrelated diff — the precise failure loop this story exists to close, so AC2 ("a formatting violation in a changed file still fails before push") is unmet for root markdown. Fix: "format:check": "pnpm prettier:check && pnpm mdlint:check" — the root mdlint:check script already is turbo mdlint:check && ./tools/markdownlint-config/bin/markdownlint-check.sh '*.md', so delegating removes the asymmetry instead of re-typing it.
• packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts:36-38,49-56 — the guard is defeated by the indirection this PR introduces. findWriteModeFormatters matches only inside the value of the root quality-gate script, but the gate no longer contains any formatter — it now delegates (pnpm format:check). Changing quality-gate to ... && pnpm format && ... (literally the one-word edit the PR description says the guard prevents) restores repo-wide write-mode formatting with the guard green; the same holds for redefining format:check as turbo prettier:fix. Fix: resolve pnpm <script> / pnpm run <script> references transitively against pkg.scripts (bounded depth + visited-set for cycles) before scanning, and add the RED cases quality-gate → pnpm format and format:check → turbo prettier:fix. Two cheap hardenings while in there: assert the gate still invokes gate:composition (so deleting the guard from the gate fails the unit test, which runs inside the gate), and assert the remedy script named in PRE_PUSH_REMEDY (format) actually exists — otherwise the failure message can point at a script someone renamed.
• tools/prettier-config/.prettierignore:10-16 — the generated-artifact ignore set is enumerated reactively (only the two dirs that happened to break the author's push), so the same class of failure is still armed. **/playwright-report/ is gitignored, is produced by the repo's own pnpm --filter @pair/website a11y:report:html (--reporter=html → apps/website/playwright-report/index.html), and html is in the wrapper's glob {**/*,*}.{ts,tsx,js,jsx,json,html} — so after running that documented script, every push is blocked until the dir is deleted, i.e. --no-verify pressure is back (AC3). Likely second instance: apps/pair-cli/.pair/ (gitignored local install tree) — prettier does traverse dot-directories here (the author's own .source/index.ts hit proves it) and markdownlint runs with --dot, so its *.json/*.md would block too. Fix: add **/playwright-report/ (and apps/pair-cli/.pair/) and mirror the generated set into tools/markdownlint-config/.markdownlintignore; better, stop enumerating — prettier 3.6 accepts repeated --ignore-path, so the wrapper can pass the repo .gitignore alongside the shared ignore, the same way .jscpd.json already solves this with "gitignore": true. A check-mode gate makes "gitignored ⇒ never checked" a correctness requirement, not a nicety.
• .pair/adoption/decision-log/ (missing file) — the decision is not recorded as an ADL, and the repo's own precedent says it must be. This is a Convention Adoption change with a rejected alternative that the story explicitly offered (lint-staged / pre-commit auto-fix), argued only in the PR description. The sibling ADL 2026-07-13-gate-tooling-code-in-tested-modules.md shows the intended split — ADL holds Context/Decision/Alternatives/Consequences, and its "Adoption Impact" says way-of-working gains a short bullet pointing to the ADL. This PR inverts it: a ~5-line rationale inlined into way-of-working.md:39 and no ADL, so the rejected alternatives and consequences survive only in a PR body (CLAUDE.md: "architectural/project decisions MUST be recorded as ADR or ADL"). Fix: /pair-capability-record-decision $type: non-architectural for "pre-push gate is check-only; formatting is applied deliberately", then shorten the way-of-working bullet to a pointer.
• DEVELOPMENT.md:121 (also :65, :73) — the contributor-facing doc of record now states the opposite of the shipped behaviour: "This runs (in order): ts:check, test, lint, prettier:fix, mdlint:fix, hygiene:check, docs:staleness", and the Root-Level Commands block (:65, :73) never mentions pnpm format / pnpm format:check. The gate's own failure message tells a developer to run pnpm format — a command documented nowhere. way-of-working.md was updated for exactly this reason, so the omission is an inconsistency inside the PR, not a pre-existing gap. Fix: correct the gate step list, add pnpm format / pnpm format:check to the script table, and drop "prettier + mdlint" from the :65 one-liner in favour of "format check".

Minor (consider)

• .pair/adoption/tech/way-of-working.md:50 — the Custom Gate Registry row for gate 1 still reads "build test and formatting check&fix", now false, in the same file and section the PR edits (line 38 was fixed, line 50 was not). One-word fix: "formatting check (never fix)".
• .pair/adoption/tech/tech-stack.md:90 — "markdownlint-cli v0.47.0 … wired into pnpm quality-gate via mdlint:check / mdlint:fix" is stale: the gate now reaches mdlint:check only; mdlint:fix is reachable via pnpm format. Same for the phrasing around line 91's neighbours if they enumerate the gate.
• package.json:34 — "format" re-types the body of the root "mdlint:fix" (turbo mdlint:fix && ./tools/markdownlint-config/bin/markdownlint-fix.sh '*.md') instead of delegating. Two places now define "how root markdown is formatted", and that duplication is the direct mechanism of Major Setup & Project Management Integration #1 — the fix half was copied, the check half was not. Prefer "format": "pnpm prettier:fix && pnpm mdlint:fix".
• packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts:50 — JSON.parse is unguarded, so a malformed root package.json makes the gate-composition step die with a raw SyntaxError stack, pointing the developer at the guard rather than at their broken JSON. The function already returns a structured { ok, message } for the "no gate" case; do the same here ('root package.json is not valid JSON: …') plus a test.
• packages/dev-tools/src/quality-gates/pre-push-gate-composition-cli.ts:1-17 — a separate -cli.ts entrypoint diverges from the shape this repo documents. ADR-014 states "All four tools across the two packages share the same shape: an importable, unit-tested module plus a thin CLI wrapper (main() behind a require.main === module guard)", and the three siblings in this very directory (code-hygiene-check.ts:…main(), sync-version-in-docs.ts, benchmark-update-link.ts) all do it that way. The new shape does not violate the ADL's rules (logic is in a tested module; the script is not unit-tested) and is arguably cleaner — but it makes ADR-014's consequence statement false and leaves src/quality-gates/ with two conventions. Either conform, or record the shape change (one line in the ADL/ADR) so the next tool author knows which to copy.
• packages/dev-tools/src/quality-gates/pre-push-gate-composition-cli.ts:10 + pre-push-gate-composition.test.ts:88 — the '../../../../package.json' hop count is duplicated in the CLI and in the test; the sibling module centralizes exactly this (code-hygiene-check.ts:16, const REPO_ROOT = resolve(__dirname, '..','..','..','..'), with the hop count explained in the header comment). ADR-014's own "Consequences" records that a folder move already broke every __dirname-relative constant once. Export ROOT_PACKAGE_JSON (or a readRootGate()) from the module and use it in both places.
• .prettierignore:1 — a trailing-newline-only edit to the root .prettierignore, absent from the PR's own "Files Changed" list and unexplained. Per the PR's own analysis this file is not read by the gate (the wrappers pass --ignore-path tools/prettier-config/.prettierignore, and nothing invokes prettier at the repo root), so it is a stray byte in a file the PR argues is inert. Either drop the hunk, or say what still reads it (IDE-level prettier does) so the next reader is not misled.

Questions

• .github/workflows/ci.yml:80-97 — CI runs pnpm lint and coverage but no format:check, so after this PR formatting is asserted only by a local hook that --no-verify still skips (and that fork PRs never run). This is not a regression — write mode never asserted anything in CI either — and the story lists CI workflow changes as out of scope. Is a format:check CI step wanted as a follow-up story, or is the local hook deliberately the only enforcement point?
• tools/prettier-config/.prettierignore:16 — your own review area: **/test-results/ is a bare directory name, so a source directory called test-results anywhere in the repo would be silently unformatted forever. Scoping to the two real producers (apps/website/test-results/, packages/brand/test-results/) costs one line and removes the ambiguity — or is the breadth a deliberate bet that nobody will author such a dir?
• package.json:35 (pre-existing, informational) — the gate's format coverage excludes .pair/** entirely: no workspace package's cwd contains it, and the root markdown check globs '*.md' (non-recursive). So this PR's own .pair/adoption/tech/way-of-working.md edit was never format-checked by the gate, in either mode. Pre-existing and unchanged here — flagging it only because "check mode is the assurance" now depends on what the check actually reaches. Worth its own story?
• .pair/knowledge/guidelines/quality-assurance/quality-standards/quality-gates.md:160 — the shipped framework KB still uses pnpm prettier:fix as its example gate command, i.e. it models the anti-pattern this project just banned. Deliberately not proposed as a fix in this PR: [tech-debt] Pre-push quality-gate runs prettier:fix/mdlint:fix repo-wide in WRITE mode — authors bypass the hook #394 is a project-level decision and promoting it to framework guidance is a framework-scope decision. Should the KB gain a note that gate formatters run in check mode?

Positive feedback

• The reframing is the best part of this PR. "A write-mode formatter at pre-push cannot touch the commits being pushed" is a strictly stronger argument than the story's noise framing, and it is what makes check mode obviously right over lint-staged. It is recorded in the module header where the next reader will find it.
• Enforced rather than documented. A comment would not have survived; a guard that reads the real root package.json (pre-push-gate-composition.test.ts:85-93) is the right instinct, and the test-first RED against the repo's actual gate is exactly the discipline CLAUDE.md asks for.
• The offender list is explicit, with a test proving lint:fix is not flagged (pre-push-gate-composition.test.ts:56-61). A guard that fires on things it has no opinion about gets disabled; this one won't.
• findWriteModeFormatters returns every offender, not the first, with a test that a partial fix cannot look clean. Small design choice, real value.
• The one-time sweep was declared, not hidden, and it is correct: arrowParens: "avoid" and printWidth: 100 in tools/prettier-config/.prettierrc.json confirm both reformats are what prettier actually produces.
• AC1 verified end-to-end by inspection, not just claimed: no remaining gate step writes a tracked file — lint uses tools/eslint-config/bin/lint.sh (no --fix), hygiene:check / docs:staleness / skills:conformance contain no writeFileSync, and dup:check is "reporters": ["console"].
• Honest self-reporting of the two blocked push attempts. That failure mode is the strongest available evidence that check mode changes behaviour, and surfacing it is what let me find the third artifact dir.

Functionality & requirements (AC coverage)

• AC1 — "Pushing a branch never modifies a file the branch did not change": MET. Verified per gate step (see Positive feedback); the only writes left are untracked/gitignored test output.
• AC2 — "A formatting violation in a changed file still fails/fixes before push": PARTIALLY MET. Holds for every workspace package; fails for the six root-level *.md files (Major Setup & Project Management Integration #1) — the gate lost that check.
• AC3 — "--no-verify is no longer needed for a normally formatted change": PARTIALLY MET. Holds on the default path; re-armed by playwright-report/ (Major Adoption Management & Guideline Linking #3), which forces exactly the bypass the story removes.
• Business logic + edge cases correct — the guard's own logic is sound for the inputs it is given; the gap is which inputs it looks at (Major Collaborative Knowledge Base #2).
• Integrates with existing systems — root gate delegates via pnpm --filter @pair/dev-tools pre-push-gate:check (ADL 2026-07-13), turbo.json needs no change.
• Error handling appropriate — except the unguarded JSON.parse (Minor chore: update and reorganize guides, knowledge base, and documentation structure #8).

Testing & quality gates

• Adequate coverage for what is tested: 11 white-box assertions, test-first, RED against the real root package.json before the fix.
• Edge + error scenarios tested — gaps: no case for the delegated/indirect gate (quality-gate → pnpm format, Major Collaborative Knowledge Base #2), none for malformed JSON (Minor chore: update and reorganize guides, knowledge base, and documentation structure #8), none asserting the gate still invokes gate:composition, none asserting the remedy script format exists.
• Quality gates: PARTIAL — CI green (build pass, secret-scan pass), and the author reports pnpm quality-gate green locally. I could not re-run the gate from a read-only detached worktree (no installed node_modules); the reported result is consistent with CI. Note the guard's unit test is what keeps the property enforced if the CLI is ever dropped from the gate — worth strengthening as above.
• Coverage guardrail is not at risk: coverage-baseline.md feeds only @pair/website and @pair/brand, so the untested CLI file (by ADL design) does not move a measured baseline.

Adoption compliance

• Degradation level: 2 (/pair-capability-verify-adoption reasoning applied inline; no unlisted dependency, so /pair-capability-assess-stack was not needed — nothing was added to tech-stack.md's surface).
• ADL 2026-07-13 (gate tooling in tested modules): conforms — logic in packages/dev-tools/src/quality-gates/, white-box unit tests, root gate is a one-line pnpm --filter delegation, script not unit-tested.
• ADR-014 (tool package boundary by bounded context): conforms on placement (new folder-mate in an existing package, no new package) — diverges on the documented tool shape (Minor: separate -cli.ts vs main() behind require.main === module).
• way-of-working.md Quality Gates: updated but internally inconsistent — prose bullet fixed (line 39), Custom Gate Registry row not (line 50, Minor).
• Decision record: missing — no ADL for the check-mode convention change (Major Workflow Customization #4). tech-stack.md:90 left stale (Minor).
• No dependency, framework or infrastructure change; nothing else in adoption/tech/ is contradicted.

Tech debt

• Reduces debt overall — this PR retires the tracked item [tech-debt] Pre-push quality-gate runs prettier:fix/mdlint:fix repo-wide in WRITE mode — authors bypass the hook #394 and removes the cause of the [US-281] feat: assess-cost report mode — cost monitoring panel #388 / [US-278] feat: bootstrap quick mode — a second resolution depth, guided stays the default #408 / [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411 defect class.
• New debt introduced (small, all listed above): an incomplete guard contract (Major Collaborative Knowledge Base #2) and a reactively-maintained ignore list (Major Adoption Management & Guideline Linking #3) are the two that will re-surface as "why did my push break / why did unrelated files change" if not closed now.
• Pre-existing debt surfaced, not owned by this PR: the gate's format coverage never reaches .pair/** or root-level non-markdown files (root package.json, turbo.json, tsconfig*.json are outside every package's prettier cwd); CI asserts no formatting at all. Candidates for a follow-up story, not blockers.
• Nothing here justifies a TECH-DEBT verdict instead of CHANGES-REQUESTED: the blocking items are correctness/AC gaps in the change itself, not debt.

Documentation

• Code docs — module and test headers are unusually good: they record why, not what.
• Contributor docs — DEVELOPMENT.md now contradicts the gate and never mentions pnpm format (Major Advanced Features Enablement #5).
• Adoption docs — way-of-working.md:50 and tech-stack.md:90 stale (Minor); no ADL (Major Workflow Customization #4).
• PR description — excellent: declares the sweep, names the rejected alternative, flags its own risky choices. Only omission: root .prettierignore is missing from the Files Changed list.

Performance & deployment

• No performance regression — the gate gets faster (two write passes → two read passes); one extra ts-node start (~1s).
• Deployment / rollback — no runtime artifact; rollback is reverting the root package.json scripts. Note the intended one-way ratchet: after this lands, restoring the old gate string fails gate:composition by design, which is the point.

---

Reviewer note on method: reviewed independently from the story's AC, the PR diff/description and the branch code only, in a detached read-only worktree pinned to cfb5208d; .pair/working/ was not read. The five Major items are all small, local fixes (three one-liners, one test-plus-helper, one ADL) — the design is sound and I expect a fast turnaround.

[rucka]

In reply to the first-review comment.

Escalating to human — round 6 findings still open (supersedes the round-3 escalation)

This comment supersedes the earlier escalation comment (round 3), which is now minimized as outdated — it only covered rounds 1-3. Rounds 1, 2, 4 and 5 all fully converged (16 + 10 + 11 + 11 findings, all fixed or answered, none escalated) with no PR comment posted for those rounds (per orchestrator instruction — the working log was the record, and the PR description was updated instead, since the reviewer cannot see that file). Full per-finding detail for every round is in the working log .pair/working/reviews/394.md.

Round 3's 9 findings (all Minor) were the CI-scope / robustness items — remedy-at-failure-point, mention-vs-invocation scanning, space-separated runner flags, per-package blast radius, bootstrap template drift, smoke-scenario coverage gaps, turbo.json scoping, awk approximation docs, temp-file trap nits. Status: not restated by round 4's review, and the deferral-shaped ones are now tracked as follow-up issues (#413/#414/#415) referenced from the ADL — they remain open as filed follow-ups, not as unaddressed findings on this PR.

Round 6 re-review surfaced 6 more findings (all Minor). Per finding — what was attempted so far / current state:

1. [Minor] sync-version --check sparing doesn't hold on the real repo shape (pre-push-gate-composition.ts:100 + .test.ts:125-130) — not yet attempted. The lookahead is evaluated on the EXPANDED gate, and expandScriptReferences inlines the script body right after the match, so pnpm sync-version --check becomes pnpm sync-version { ... } --check — the { blocks the same-segment lookahead and the inlined body's own bare sync-version gets flagged, so the legitimate dry-run is reported as an offender. Reproduced directly against the real root package.json. The existing test only asserts the pattern in isolation, never through checkRootGate with sync-version defined as a root script, and the pkg() fixture's format:check body is also stale (pre-round-5 && form).
2. [Minor] ADL "byte-identical" claim is only true for the paragraph, not the command block (ADL 2026-07-31-pre-push-gate-is-check-only.md:54 vs DEVELOPMENT.md:65,67 and development-setup.mdx:118,120) — not yet attempted. The load-bearing paragraph is verified byte-identical (one link-form line). The command-block half is not: two comment-annotation lines differ (format check + hygiene vs never writes — formatting is checked only; missing ; never writes on format:check). A diff on the pair that's supposed to be in sync yields three lines, undermining the "diff and expect one line" drift signal the ADL itself prescribes.
3. [Minor] PR description's "Dependencies & Related Work" open set is stale again (last bullet) — not yet attempted. chore: release v0.5.0 #417 (chore: release v0.5.0) is now open and touches root package.json (version/CHANGELOG only, scripts untouched — no real hazard), contradicting the "untouched by all of them" blanket claim. Rest of the section re-verified accurate ([US-277] feat: .claude-plugin marketplace + plugin manifests with dataset catalog guard #386, [US-234] feat: PR state flow (gate≠review) + pair review as required check #390, [US-402] docs: PM-tool adapters document membership + assignee — an item can no longer be written and stay invisible #404, [US-407] fix: a skill's nested references/ installs inside it — bounded flatten + link rebase + config wiring #411).
4. [Minor] format:check aggregation collapses a broken-wrapper exit code into the same code as a formatting violation (package.json:33) — not yet attempted. _p=$?; _m=$?; exit $((_p || _m)) maps ANY non-zero (e.g. prettier exiting 2 on a broken install) to exit 1, indistinguishable from "there are unformatted files," which every doc tells the contributor to fix with pnpm format — a loop with no exit for exactly the distinction the new smoke test preserves at the wrapper level.
5. [Minor] _ignore-file.sh:47-49 EXIT trap interpolates the mktemp path into a single-quoted string at registration time — not yet attempted. A TMPDIR containing a single quote breaks out of the quoting (nil impact today, but the SC2064 suppression + "expand now" comment overstate the need — late expansion via trap 'rm -f "$_ignore_combined"' EXIT works and is strictly safer since the variable is still set in-scope when the trap fires).
6. [Minor] ADL residual-gap paragraph renders outside the bullet list it qualifies (.pair/adoption/decision-log/2026-07-31-pre-push-gate-is-check-only.md:37-40) — not yet attempted. The "What is actually enforced is that explicit list…" paragraph sits at column 0 between two - bullets, so markdown renders it as a break in the list rather than as part of the guard bullet it qualifies — the single most load-bearing sentence in the Decision, and markdownlint doesn't flag it (clean run confirmed).

Not changed (escalated): none of the above were rejected as a design disagreement — they simply haven't been attempted yet in this round. Listed here per orchestrator instruction to escalate this round rather than loop again locally.

Convention for what happens next: any further rework or re-review — including manual out-of-band rounds — should be appended to the working log .pair/working/reviews/394.md, not posted as a new standalone PR comment. The next orchestrated run on this story continues the same review<->fix cycle; its eventual convergence will synthesize ONE final remediation comment and minimize this one (and the working log stays the audit trail), per the established flow.

Persistence note (unchanged from round 3): the working log is an untracked file that lives only in the persistent authoring worktree ../pair-worktrees/394. That worktree must be preserved until merge — if it is pruned or recreated, the per-finding audit trail (rounds 1-6 detail) is lost. This comment and the first-review comment remain on the PR regardless (so the first-review signal still prevents a duplicate first review on a future run).

Quality gates: full pnpm quality-gate green as of commit 4dafdaef (round-5 verification — see the working log); no code changes made this round — round 6 is escalation only, no fixes attempted yet.

→ Escalating to human for a decision on how to proceed (continue the fix loop vs. accept as-is with follow-ups filed).

[rucka]

Remediation — round 4

Open findings: 0. Every finding from round 3's escalation is resolved; nothing on this PR is waiting on anyone.

Head: 19ed5d02 (rebased onto main — see the last item). Gate: 22 + 6 + 6 tasks green, working tree unmodified by the run.

#	Finding	What changed
1	The sync-version --check sparing did not hold through script expansion	Reproduced first at integration level (checkRootGate with sync-version defined as a root script → offenders: [sync-version]), then fixed: the lookahead now crosses {/}, which is what expansion inserts between the name and its own flag. Real separators (&& | ; newline) still bound it, so a --check next door cannot launder a write. Two new cases: the delegated dry-run passes, the delegated write still fails. The pkg() fixture also now mirrors the shipped format:check body.
2	The byte-identical claim was false for the command block	Scoped to the load-bearing paragraph, which does hold — verified: the two copies differ by exactly the ADL link line. The ADL now says why the command blocks are excluded: they legitimately list different commands, so demanding equality there produces a three-line diff on a pair that is in sync, which is how a reader learns to ignore the signal.
3	The declared open set omitted #417	#417 is closed and the pending changeset removed from main (dd1ff524), so the root package.json overlap no longer exists. The blanket "untouched by all of them" is gone from the description either way.
4	format:check collapsed a non-1 status to 1	Now exit $((_p > 1 || _m > 1 ? 2 : (_p || _m))) — 0 clean, 1 drift, 2 a broken wrapper. Verified in POSIX sh for all four combinations. Both doc copies gained the same clause: exit 2 means read the output, not run pnpm format. The ADL's quoted body matches the shipped script.
5	Two temp-file nits in _ignore-file.sh	mktemp failure now exits 2 (same "broken wrapper" code as above, deliberately), and the trap uses deferred expansion — the SC2064 suppression is gone. The Effect: contract states the new obligation: a caller must not reassign _ignore_combined after sourcing.
6	A paragraph escaped the bullet it qualifies	Indented back inside the gate:composition bullet.

Also, not from the findings: the two @pair/knowledge-hub failures this PR reported as pre-existing were exactly that — main's, not this branch's. They are fixed on main (d7195bbf) and this branch is rebased onto that fix, so the "only red" note in the description is now struck through with the explanation.