[US-394] fix: pre-push gate checks formatting instead of applying it (a write-mode formatter cannot fix the commits being pushed) - #412
Conversation
Verdict
PR: [#412] · Author: rucka · Reviewer: independent reviewer (Claude) · Date: 2026-07-31 · Story: [US-394] · Type: config (repo tooling) Classification matrix — per dimension
Tier = max(assessed) = yellow — confirms the story's refinement-time AssessmentsSecurity — Input validationVerdict: green — the only new input is the repo's own Details
Security — Output handlingVerdict: green — output is Security — AuthenticationVerdict: green — no authentication path exists in or near the diff. Security — AuthorizationVerdict: 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 vulnerabilitiesVerdict: green — 0 introduced, 0 pre-existing in the touched surfaces. Details
No secret, credential, injection sink, deserialization of untrusted data or dependency addition. CostVerdict: Details
Architecture (Coupling)Verdict: yellow — one new integration, deliberately text-based, and it is under-specified for the regression it exists to prevent. Details
DetailsFindings by severityCritical (must fix before merge)
Major (should fix before merge)
Minor (consider)
Questions
Positive feedback
Functionality & requirements (AC coverage)
Testing & quality gates
Adoption compliance
Tech debt
Documentation
Performance & deployment
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 |
…ecked, ignores delegate to gitignore Review findings on PR #412, all resolved. Gate composition (the headline safeguard was defeatable): - the guard now expands `pnpm <script>` / `pnpm run <script>` references transitively against the root scripts (bounded depth + visited set) before scanning, so `quality-gate && pnpm format` and `format:check = turbo prettier:fix` — the two likeliest regressions, both one edit — fail. RED tests for each. - it also fails if the gate drops `pnpm gate:composition` itself, or if the remedy it advertises (`pnpm format`) stops existing. The unit test asserts the real root package.json and runs inside the gate, so unplugging the guard breaks the gate. - malformed root package.json returns `{ ok: false, 'not valid JSON' }` instead of a raw SyntaxError stack pointing at the guard. - `ROOT_PACKAGE_JSON` is exported from the module; the CLI and the test no longer each spell out the `../../../..` hop count. - ADR-014 shape restored: `main()` behind `require.main === module` in the module, `-cli.ts` deleted — same shape as the three siblings. Format coverage: - `format:check` = `pnpm prettier:check && pnpm mdlint:check`, `format` = `pnpm prettier:fix && pnpm mdlint:fix`. Both delegate instead of re-typing, so root-level *.md (reachable only via the root `mdlint:*` scripts, since turbo runs workspace members only) is CHECKED, not just fixed. Previously a markdownlint violation in README/CLAUDE/CONTRIBUTING passed the gate. Generated artifacts, declaratively: - both formatter wrappers now pass the repo's own .gitignore files (git root + the package cwd) as additional ignore sources — prettier 3.6 takes repeated --ignore-path; markdownlint-cli takes one, so the sources are concatenated into a temp file (it parses gitignore syntax). "gitignored => never checked" now holds by construction: `playwright-report/`, `test-results/`, `.source/`, a local `apps/pair-cli/.pair/` install tree can no longer block a push. The reactive enumeration in tools/prettier-config/.prettierignore is gone (that also drops the over-broad bare `test-results/`). Docs / decision record: - ADL 2026-07-31-pre-push-gate-is-check-only.md records context, decision, alternatives (incl. the rejected lint-staged / pre-commit auto-fix) and consequences; way-of-working keeps a one-line pointer instead of inline rationale. - way-of-working gate registry row 1: "formatting check (never fix)". - tech-stack: markdownlint wired into the gate via `mdlint:check` only. - DEVELOPMENT.md: gate step list corrected (was still claiming `prettier:fix`/`mdlint:fix`), `pnpm format` / `pnpm format:check` added to the root command table. - the stray trailing-newline hunk on the root .prettierignore is reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…onvention The routing rule lives in ONE file, so a change to the split model is a one-file change and no skill re-derives it: - `## Code-host resolution`: read `code-host` from `## Git Workflow`; absent => the PM tool (zero-config default, D21); same tool in both => as omitted; declared-but-unreachable/unauthenticated => HALT with a setup pointer, PM-side work NOT rolled back - `## Routing table`: 8 operation classes split across `pm-tool` (item CRUD, hierarchy/label/state reads, state transitions, close) and `code-host` (branches/pushes, PR CRUD, PR labels + required checks, review submission, merge, open-PR detection). Invariants: state transitions ALWAYS on the PM tool; PR state never mirrored onto the board; the pair review check registers on the code host only - `## Cross-linking convention`: `Refs: <issue-id>` in the PR body + PR URL posted back on the item; id-not-found => PR still created, warning with manual-link instruction; ids copied verbatim (#412 / ENG-412 / PROJ-412) - "What stays in the skill" extended: a skill names which SIDE it is on and points here — never restates the default or the convention - File retitled (PM-Tool + Code-Host Resolution); path unchanged so the 18 existing skill pointers keep resolving - Task: T2 — routing convention in one place + grep audit of the catalog Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…b + Linear reference case (#389) * [#236] test: conformance guard for code-host / PM-tool split (RED) - 32 assertions on the source-of-record dataset artifacts: WoW `code-host` schema (AC1), routing convention in one place (AC2/AC4), `Refs:` + PR-URL cross-link (AC3), grep-verifiable catalog audit (AC4), Linear guideline parity (AC5) - Data-driven PR-side skill list: a new skill needs no new test body - Asserts the invariant survives the copy to the root mirror - Task: T1-T5 — RED phase (31 failing / 1 passing before implementation) Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] feat: T1 — optional `code-host` in the WoW Git Workflow schema - New `## Git Workflow` section in the WoW template: `code-host` (optional, default = the PM tool) + `base-branch` (default `main`), the consolidation publish-pr's adoption inputs were waiting on - Resolution rule stated in the schema: omitted => code host = PM tool, behavior identical to a single-tool project (convention over configuration, D21); the same tool in both fields is treated exactly as omitted, no dual-write - Split example inline (Linear backlog + GitHub code); points at the single routing convention instead of restating the rule - pair's own adoption declares the single-tool default explicitly (`code-host` omitted, `base-branch: main`) — dogfooding the field without changing behavior - Task: T1 — WoW schema: `code-host` field + resolution rule + docs Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] feat: T2 — PM<->code-host routing table + `Refs:` cross-link convention The routing rule lives in ONE file, so a change to the split model is a one-file change and no skill re-derives it: - `## Code-host resolution`: read `code-host` from `## Git Workflow`; absent => the PM tool (zero-config default, D21); same tool in both => as omitted; declared-but-unreachable/unauthenticated => HALT with a setup pointer, PM-side work NOT rolled back - `## Routing table`: 8 operation classes split across `pm-tool` (item CRUD, hierarchy/label/state reads, state transitions, close) and `code-host` (branches/pushes, PR CRUD, PR labels + required checks, review submission, merge, open-PR detection). Invariants: state transitions ALWAYS on the PM tool; PR state never mirrored onto the board; the pair review check registers on the code host only - `## Cross-linking convention`: `Refs: <issue-id>` in the PR body + PR URL posted back on the item; id-not-found => PR still created, warning with manual-link instruction; ids copied verbatim (#412 / ENG-412 / PROJ-412) - "What stays in the skill" extended: a skill names which SIDE it is on and points here — never restates the default or the convention - File retitled (PM-Tool + Code-Host Resolution); path unchanged so the 18 existing skill pointers keep resolving - Task: T2 — routing convention in one place + grep audit of the catalog Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] feat: T3 — route PR/review ops by `code-host` across the catalog Grep audit of the whole catalog for PM-tool/code-host conflation, patched at every hit. Each skill now names which SIDE of the routing table an operation is on and points at the single convention — no rule re-derived locally: - publish-pr: reads `## Git Workflow` (the "#236's job" note is now done); new Phase-4 back-link step posts the PR URL onto the PM item via /write-issue (skipped when code host = PM tool), `Refs: <issue-id>` on the split path, `Cross-link:` line in the report; HALT extended to unauthenticated code host + PM-side-not-rolled-back - review: PR read + native verdict submission are code-host ops (was "from the PM tool"); verdict never mirrored onto the PM tool; story read stays PM-tool and resolves the id from `Refs:`; degradation split per side - review/merge-and-cascade: PR merge + branch delete on the code host, issue close + parent cascade on the PM tool - implement: branch cut from `base-branch` on the code host; post-review-merge merges on the code host, writes Done on the PM tool - verify-quality: PR labels come from the code host, story card from the PM tool (gh snippet is the single-tool case); precedence unchanged - write-issue: explicitly PM-tool-only, never code-host state - next: row 6 open-PR detection queries the code host and matches items via `Refs:`; declared-but-unreachable host skips row 6 rather than HALTing - Root mirror regenerated via the real `pair update` pipeline (#352 guard) - Task: T3 — patch conflating skills to route by field Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] docs: T4 — Linear PM guideline at parity + reachable from the skills - `linear-implementation.md`: full parity with the other implementation guides (Quick Setup, Prerequisites, Detection & HALT, Adoption Configuration, hierarchy mapping, state mapping, cross-topic navigation, Working with Issues, Code Review & PR Management, Troubleshooting, Related). Documents BOTH access paths (MCP Server, GraphQL API) so adoption picks one - Linear specifics: one issue type => types via labels; native `estimate` field + per-team scale; per-team workflow states with the default Backlog/Todo/In Progress/In Review/Done mapping and the Readiness-Fallback and no-Review-state caveats - Framed as the reference split case: Linear hosts no code, so PR/review work routes to the declared `code-host` and the two tools link via `Refs: <issue-id>` + a PR-URL comment - `issue-management/linear-issues.md`: issue lifecycle companion (creation, parent/sub-issue, priority map, estimates, states, filters, troubleshooting) - PM-tool README: Linear row + "PM tool != code host" framing; issue-management README + scope lists updated; llms.txt regenerated - Reachability fixed for BOTH previously-unlisted guides: write-issue, assess-pm and setup-pm now resolve `azure-devops` and `linear`; setup-pm Step 4 writes `code-host` iff the PM tool hosts no code - Task: T4 — Linear PM guideline (implementation + issue management) Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] docs: T5 — code-host concept page + Linear split walkthrough (docs site) - New `concepts/code-host.mdx`: zero-config default, `## Git Workflow` schema, the routing table, the `Refs:` + PR-URL cross-linking convention, the Linear+GitHub reference cycle (refine -> implement -> publish -> review), and the three failure modes (unreachable host, id not found, same tool twice) - `pm-tools/linear.mdx`: leads with "Linear hosts no code" + the required `code-host` declaration; status table split into PM-tool vs code-host columns showing review state is never mirrored; both access paths listed - Task: T5 — linking convention + dual-tool walkthrough + docs site Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] fix: review round 1 — executable back-link, AC4 audit gaps, ADR-018 Resolve all 15 findings from PR #389 review. - write-issue: new `$mode: comment` — non-destructive back-link path (Step 7c), no template/body/board write; not-found + PM error warn instead of HALT; /publish-pr Composition Interface entry. Fixes the Critical: the back-link through write-issue was either a no-op, a HALT, or a full-body overwrite. - publish-pr: Phase 4.5 names the composition explicitly (`$mode: comment $id: <id> $comment: "PR: <url>"`) with a direct implementation-guide fallback; Refs: now fills a template slot. - classify: routes by target (card ⇒ PM tool, PR ⇒ code host) in Steps 4/5, edge cases and degradation; added to PR_SIDE + the routing table. - verify-done: PR tier/approval reads marked code-host; routing table PR row. - code-host resolution: `filesystem` joins linear/jira in the hosts-no-code set (absent code-host there HALTs, never resolves to a non-PR tool) — mirrored in setup-pm, the WoW schema, code-host.mdx and filesystem.mdx. - implement: branch snippet parametrised to <base-branch> (was hardcoded main). - ADR-018 records the decision (optional code-host override, text-convention cross-link, review check on the code host only, no status mirroring). - how-to 11 "Load PR from PM tool" -> code host; pr-template gains a conditional `Refs:` slot; pair's own adoption declares `code-host: github` instead of the undefined literal "omitted"; 3 stale link labels retitled. - linear-implementation: `linear_gql` helper (endpoint+headers once, token via 0600 header file instead of argv/history) used by every example. - skill-conventions: version-bump rule codified; 11 skills bumped accordingly. Refs: #236 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] fix: review round 2 — idempotent back-link, documented comment mechanisms, plain Refs token Round-2 findings on PR #389 (15 items, all resolved): - Major: /publish-pr Phase 4 step 5 gains its Check→Skip half (existing back-link comment detected, not re-posted); /write-issue's idempotency Note scoped to write mode with comment mode as the documented exception (dedup is the caller's). - Major: the comment mechanism is now documented in every supported PM guide — gh issue comment (github), work-item comments REST (azure-devops), `## Activity Log` append (filesystem, incl. how the "body unchanged" guarantee reads for a file tracker), commentCreate hoisted to its own section (linear). Link-field branch scoped: no supported tool needs it. - Minor: pr-template emits the plain `Refs:` token (the literal the read-backs match); AC4 audit regex word-bounded + case-sensitive PR (no more "prepend" matches); filesystem concept-page assertion checks the claim, not a mention; canonical identifier-alias table (github ≡ github-projects); one-time upgrade step for existing hosts-no-code adoptions; verify-done back to a patch per the side-marker rule (now codified); classification row + upgrade/re-run rows on the docs table; Linear credential file hardened (subshell umask, cleanup, curl ≥ 7.55, rotation); website Linear API-key path aligned with the KB. - Questions: link-field branch owner named; frontmatter confirmed authoritative. Tests 570 (was 564). Quality gate exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [#236] fix: review round 3 — classify read side, catalog-wide AC4 grep, usable Linear setup - classify Step 3.3: refinement floor is a pm-tool read; id resolved from the PR's `Refs:` on a split project (standalone `$target: <PR>` could not reach the story ⇒ silent D17 never-lower degradation). Assertion added. - AC4 negative audit now iterates EVERY dataset skill doc (PR_SIDE stays the positive set); pattern widened to `PRs?` + `in` + the possessive shape, tempered on `code host` so contrastive prose is not flagged. setup-gates gains its side marker + pointer and joins PR_SIDE (0.5.1). - linear-implementation: `rm -f` out of the setup fence (`linear_gql_cleanup`, called when done) — the block left the documented path self-defeating; `--netrc` dropped for `curl -K -` (raw, non-Bearer Authorization). - linear-issues: snippets go through `linear_gql`, no inline key. - publish-pr: single-tool skip hoisted above the PM-item read (AC1 byte-identical); base-branch resolution names both sections + honors the legacy `## Merge Strategy` placement; filesystem back-link = `## Activity Log`. - write-issue: comment mode Output rows `n-a (comment mode)`, `$status`/`$labels` documented ignored, Activity-Log survival tied to the merged-full-body contract. - pm-resolution: `<issue-id>` table incl. filesystem (file stem, glob across status dirs); hosts-no-code HALT points at this convention, not a schema user-owned adoption cannot have. - pr-template: `Refs:` guidance moved from an inline HTML comment to a `> Conditional` blockquote (comment could leak into a real body and break the anchored read-back). - skill-conventions: version table row for adoption-parametrised commands ⇒ minor; implement 0.5.1→0.6.0. pair's own adoption drops both defaulted Git Workflow keys (delta-only, D21). Linear docs page notes the In Review state may not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * [US-236] fix: round-4 review — 2 Major + 7 Minor + label provisioning Major - pr-template Refs: slot no longer carries the markdown hard-break trailing spaces, AND the convention now specifies the anchored+trimmed extraction (^Refs:[ \t]*(.+?)[ \t]*$) so 'verbatim' means the id, not the line. - base-branch resolution order (incl. the legacy ## Merge Strategy placement) moved into way-of-working-pm-resolution.md; publish-pr and implement both point at it, so the two readers can no longer disagree on the PR target. Minor - publish-pr: board-state cross-reference renumbered (step 6 -> 7). - classify: rendering slot for the 'refinement floor unreadable' state. - linear-implementation: file-mode guard on truncation + keychain read before the export form. - way-of-working-pm-resolution: degradation path for a declared code host with no KB implementation guide (warn + best effort, never HALT). - development-process: branching procedure parametrised on <base-branch>. - verify-quality: distinguishes an unreachable PM tool from a genuinely untagged item instead of collapsing both into fail-safe red. - pm-tools/index.mdx: 'Hosts your code?' column + the split explanation. Question (F10) — resolved in-scope, not deferred: the Linear guide now names who provisions the chromatic risk:/cost: labels and what happens when one is missing. Guards: +81 assertions in code-host-routing.test.ts (67 total, all green). Two of them matched prose too literally (a code span and bold emphasis sat between the words); widened to tolerate the markdown, then injection-tested — both go red when the underlying claim is removed, so neither is vacuous. 585/585 knowledge-hub, 74/74 website, docs:staleness PASS, links valid. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
b1105b0 to
4dafdae
Compare
…ecked, ignores delegate to gitignore Review findings on PR #412, all resolved. Gate composition (the headline safeguard was defeatable): - the guard now expands `pnpm <script>` / `pnpm run <script>` references transitively against the root scripts (bounded depth + visited set) before scanning, so `quality-gate && pnpm format` and `format:check = turbo prettier:fix` — the two likeliest regressions, both one edit — fail. RED tests for each. - it also fails if the gate drops `pnpm gate:composition` itself, or if the remedy it advertises (`pnpm format`) stops existing. The unit test asserts the real root package.json and runs inside the gate, so unplugging the guard breaks the gate. - malformed root package.json returns `{ ok: false, 'not valid JSON' }` instead of a raw SyntaxError stack pointing at the guard. - `ROOT_PACKAGE_JSON` is exported from the module; the CLI and the test no longer each spell out the `../../../..` hop count. - ADR-014 shape restored: `main()` behind `require.main === module` in the module, `-cli.ts` deleted — same shape as the three siblings. Format coverage: - `format:check` = `pnpm prettier:check && pnpm mdlint:check`, `format` = `pnpm prettier:fix && pnpm mdlint:fix`. Both delegate instead of re-typing, so root-level *.md (reachable only via the root `mdlint:*` scripts, since turbo runs workspace members only) is CHECKED, not just fixed. Previously a markdownlint violation in README/CLAUDE/CONTRIBUTING passed the gate. Generated artifacts, declaratively: - both formatter wrappers now pass the repo's own .gitignore files (git root + the package cwd) as additional ignore sources — prettier 3.6 takes repeated --ignore-path; markdownlint-cli takes one, so the sources are concatenated into a temp file (it parses gitignore syntax). "gitignored => never checked" now holds by construction: `playwright-report/`, `test-results/`, `.source/`, a local `apps/pair-cli/.pair/` install tree can no longer block a push. The reactive enumeration in tools/prettier-config/.prettierignore is gone (that also drops the over-broad bare `test-results/`). Docs / decision record: - ADL 2026-07-31-pre-push-gate-is-check-only.md records context, decision, alternatives (incl. the rejected lint-staged / pre-commit auto-fix) and consequences; way-of-working keeps a one-line pointer instead of inline rationale. - way-of-working gate registry row 1: "formatting check (never fix)". - tech-stack: markdownlint wired into the gate via `mdlint:check` only. - DEVELOPMENT.md: gate step list corrected (was still claiming `prettier:fix`/`mdlint:fix`), `pnpm format` / `pnpm format:check` added to the root command table. - the stray trailing-newline hunk on the root .prettierignore is reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ssess-cost SKILL.md) Rebased onto origin/main (#389 + #391 merged during review). Current main carries markdown drift the new check-mode gate turns into a hard failure: 2x MD049/emphasis-style at dataset/.skills/capability/assess-cost/SKILL.md:155 (`*as of then*` where the file's style is underscore). Without this commit `pnpm format:check` exits 1 for every contributor the moment #412 merges, so the pre-push hook blocks EVERY push on a file nobody touched — the exact pathology #394 removes, and a miss of AC3. `pnpm format` fixes the dataset copy only; the generated twin .claude/skills/pair-capability-assess-cost/SKILL.md is NOT in format scope (.claude/ is not a workspace member), and skill-md-mirror asserts byte equality through the real `pair update` transform — so the format-only edit broke that guard until the twin was propagated. Both copies changed here. That trap is now documented as a two-step remedy (see next commit).
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 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, Round 6 re-review surfaced 6 more findings (all Minor). Per finding — what was attempted so far / current state:
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 Persistence note (unchanged from round 3): the working log is an untracked file that lives only in the persistent authoring worktree Quality gates: full → Escalating to human for a decision on how to proceed (continue the fix loop vs. accept as-is with follow-ups filed). |
…ecked, ignores delegate to gitignore Review findings on PR #412, all resolved. Gate composition (the headline safeguard was defeatable): - the guard now expands `pnpm <script>` / `pnpm run <script>` references transitively against the root scripts (bounded depth + visited set) before scanning, so `quality-gate && pnpm format` and `format:check = turbo prettier:fix` — the two likeliest regressions, both one edit — fail. RED tests for each. - it also fails if the gate drops `pnpm gate:composition` itself, or if the remedy it advertises (`pnpm format`) stops existing. The unit test asserts the real root package.json and runs inside the gate, so unplugging the guard breaks the gate. - malformed root package.json returns `{ ok: false, 'not valid JSON' }` instead of a raw SyntaxError stack pointing at the guard. - `ROOT_PACKAGE_JSON` is exported from the module; the CLI and the test no longer each spell out the `../../../..` hop count. - ADR-014 shape restored: `main()` behind `require.main === module` in the module, `-cli.ts` deleted — same shape as the three siblings. Format coverage: - `format:check` = `pnpm prettier:check && pnpm mdlint:check`, `format` = `pnpm prettier:fix && pnpm mdlint:fix`. Both delegate instead of re-typing, so root-level *.md (reachable only via the root `mdlint:*` scripts, since turbo runs workspace members only) is CHECKED, not just fixed. Previously a markdownlint violation in README/CLAUDE/CONTRIBUTING passed the gate. Generated artifacts, declaratively: - both formatter wrappers now pass the repo's own .gitignore files (git root + the package cwd) as additional ignore sources — prettier 3.6 takes repeated --ignore-path; markdownlint-cli takes one, so the sources are concatenated into a temp file (it parses gitignore syntax). "gitignored => never checked" now holds by construction: `playwright-report/`, `test-results/`, `.source/`, a local `apps/pair-cli/.pair/` install tree can no longer block a push. The reactive enumeration in tools/prettier-config/.prettierignore is gone (that also drops the over-broad bare `test-results/`). Docs / decision record: - ADL 2026-07-31-pre-push-gate-is-check-only.md records context, decision, alternatives (incl. the rejected lint-staged / pre-commit auto-fix) and consequences; way-of-working keeps a one-line pointer instead of inline rationale. - way-of-working gate registry row 1: "formatting check (never fix)". - tech-stack: markdownlint wired into the gate via `mdlint:check` only. - DEVELOPMENT.md: gate step list corrected (was still claiming `prettier:fix`/`mdlint:fix`), `pnpm format` / `pnpm format:check` added to the root command table. - the stray trailing-newline hunk on the root .prettierignore is reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4dafdae to
19ed5d0
Compare
Remediation — round 4
Head:
Also, not from the findings: the two |
Test-first: the guard was written and verified RED against the repo's own package.json before the gate changed. The gate ran `prettier:fix` and `mdlint:fix` REPO-WIDE in write mode, and the gate is the pre-push hook. The decisive problem is not noise, it is uselessness: at pre-push THE COMMITS ALREADY EXIST, so a write-mode formatter rewrites the working tree and cannot fix 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`, and once bypassing is routine the hook asserts nothing. Observed three times in two days: #388 was pushed with --no-verify after the hook reformatted two unrelated pair-cli test files; the same two files were swept into #411 by a `git add -A` and had to be reverted; and they were excluded by hand from #408. - `format` — the explicit fix command (was the gate's write-mode step) - `format:check` — what the gate runs now - `gate:composition` — a tested module that reads the root package.json and fails if a write-mode formatter is ever put back. The regression is a one-word edit away and its symptom looks like author error rather than tooling behaviour, which is why a comment would not have been enough. - `**/.source/` prettier-ignored: generated by fumadocs-mdx at postinstall and already gitignored. In write mode the gate silently rewrote it on every push; in check mode it would have BLOCKED every push. A build artifact must not be able to do either. Includes the one-time sweep check mode requires: the two pair-cli version-check test files are formatted here. 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. Guard logic in a tested module under packages/dev-tools/src/quality-gates/ with a thin CLI entrypoint, per ADL 2026-07-13. The offender list is explicit rather than a `/:fix/` pattern: `lint:fix` is an eslint autofix, a different concern, and is asserted NOT to trip the guard. `pnpm quality-gate` green, and it now prints "✓ pre-push gate composition: check-mode only". Closes #394 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ecked, ignores delegate to gitignore Review findings on PR #412, all resolved. Gate composition (the headline safeguard was defeatable): - the guard now expands `pnpm <script>` / `pnpm run <script>` references transitively against the root scripts (bounded depth + visited set) before scanning, so `quality-gate && pnpm format` and `format:check = turbo prettier:fix` — the two likeliest regressions, both one edit — fail. RED tests for each. - it also fails if the gate drops `pnpm gate:composition` itself, or if the remedy it advertises (`pnpm format`) stops existing. The unit test asserts the real root package.json and runs inside the gate, so unplugging the guard breaks the gate. - malformed root package.json returns `{ ok: false, 'not valid JSON' }` instead of a raw SyntaxError stack pointing at the guard. - `ROOT_PACKAGE_JSON` is exported from the module; the CLI and the test no longer each spell out the `../../../..` hop count. - ADR-014 shape restored: `main()` behind `require.main === module` in the module, `-cli.ts` deleted — same shape as the three siblings. Format coverage: - `format:check` = `pnpm prettier:check && pnpm mdlint:check`, `format` = `pnpm prettier:fix && pnpm mdlint:fix`. Both delegate instead of re-typing, so root-level *.md (reachable only via the root `mdlint:*` scripts, since turbo runs workspace members only) is CHECKED, not just fixed. Previously a markdownlint violation in README/CLAUDE/CONTRIBUTING passed the gate. Generated artifacts, declaratively: - both formatter wrappers now pass the repo's own .gitignore files (git root + the package cwd) as additional ignore sources — prettier 3.6 takes repeated --ignore-path; markdownlint-cli takes one, so the sources are concatenated into a temp file (it parses gitignore syntax). "gitignored => never checked" now holds by construction: `playwright-report/`, `test-results/`, `.source/`, a local `apps/pair-cli/.pair/` install tree can no longer block a push. The reactive enumeration in tools/prettier-config/.prettierignore is gone (that also drops the over-broad bare `test-results/`). Docs / decision record: - ADL 2026-07-31-pre-push-gate-is-check-only.md records context, decision, alternatives (incl. the rejected lint-staged / pre-commit auto-fix) and consequences; way-of-working keeps a one-line pointer instead of inline rationale. - way-of-working gate registry row 1: "formatting check (never fix)". - tech-stack: markdownlint wired into the gate via `mdlint:check` only. - DEVELOPMENT.md: gate step list corrected (was still claiming `prettier:fix`/`mdlint:fix`), `pnpm format` / `pnpm format:check` added to the root command table. - the stray trailing-newline hunk on the root .prettierignore is reverted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…es re-anchored, wrappers deduped Guard (pre-push-gate-composition.ts): - expansion tolerates runner flags and npm/yarn: `pnpm -s format`, `pnpm -w format`, `npm run format` all reached the write-mode formatter with a green guard, because the captured "script name" was the flag and the body was never scanned. - offender list made symmetric per tool: bin alias / .sh entrypoint / raw CLI write flag (`prettier-fix`, `markdownlint-fix`, `prettier --write`, `markdownlint --fix`) — the prettier `.sh` form used to walk straight past a list that named markdownlint's. - guard-present check requires RUNNING it (`referencesScript`), so `echo gate:composition` no longer satisfies it. - docstring: offenders come back in the offender list's order, not the gate's. Wrappers: - ignore assembly extracted per tool (`bin/_ignore-args.sh`, `bin/_ignore-file.sh`); check and fix now share one source of the invariant instead of four copies. - prettier args assembled positionally, so a repo path containing a space no longer word-splits into a bogus pattern (prettier exited 2 on every push for that contributor). - root .gitignore re-anchored to the cwd for markdownlint (`_reanchor-gitignore.awk`): it resolves patterns against the cwd, git against the ignore file's dir, so a path-anchored entry (`apps/website/gen/`) would have blocked every push from that package. - cwd/git-root de-dup compares canonical paths (`pwd -P`), same structure in both tools. Coverage + caching: - new smoke test `format-ignore-delegation.sh` (both tools, check and fix, gitignored vs not, plus a path with a space) — verified RED against the pre-fix wrappers; wired into the CI list. - turbo.json `globalDependencies`: the ignore sources are inputs of cacheable tasks, so a .gitignore edit no longer replays a stale PASS/FAIL. ADL: Context now cites the incidents actually observed (PRs #388, #408, #411 — three in two days, with story+branch each) and records the re-anchoring, the shared helpers, the smoke coverage and the globalDependencies. Refs: #394
… eslint autofix, smoke asserts exit status - apps/website development-setup.mdx: gate list, "never formats" note + ADL link, `pnpm format`/`format:check` in the command block, husky lines corrected (pre-commit = ts:check, pre-push = full gate) — DEVELOPMENT.md's copy was fixed in this PR and the published one had been left diverging. - guard: invariant widened to "no step reachable from the gate WRITES" — lint:fix / lint-fix(.sh) / `eslint … --fix` added to the offender list (eslint autofix is the AC1 failure mode too); the lint:fix test flipped to a positive assertion; failure message no longer says "formatter". - guard docstring: expansion boundary stated (root scripts only; package scripts are caught by name, not by expansion). - repo-root.ts: the `__dirname` hop count now exists once for the folder; all four gate tools import it. - smoke: check-mode half asserts exit status (1 = violations) + no tool-error output, probe repo made hermetic (own package.json + .prettierrc.json), and _reanchor-gitignore.awk gets a table-driven block (3 cwd positions, all branches). - .prettierignore comment reworded (the enumerated entries ARE the non-git floor). - ADL: #413 / #414 / #415 replace the untracked "candidate follow-up" offers. Refs #394
…rs the repo's write scripts 11 findings (2 Major, 9 Minor), all resolved. Major: - Rebased onto current origin/main (twice — #389/#391, then #405/#408 landed mid-round) and cleared main's format baseline in its own commit, so `format:check` is green on the tree this actually lands on (AC3). The sync-version-in-docs.ts conflict with #391 keeps BOTH docstrings. - The advertised remedy was a trap: `pnpm format` fixes the dataset SKILL.md and cannot reach its generated .claude twin (not a workspace member), while skill-md-mirror asserts byte equality — so format:check-green became skills:conformance-red later in the SAME gate. Reproduced end-to-end. The two-step remedy is now stated in all three places that advertise it (DEVELOPMENT.md, the mdx twin, PRE_PUSH_REMEDY) and unit-tested; the structural fix is noted on #414. Minor: - Guard widened to the repo's real write scripts: `sync-version` (→ sync-version-in-docs.ts, writeFileSync across every .md/.mdx it walks; `--check` dry-run spared, bounded to the same command segment) and `test:perf` (→ benchmark-update-link.ts, no dry-run, banned outright). 6 tests, verified RED. The ADL + way-of-working now state that what is enforced is the explicit list, not the invariant in general. - `_reanchor-gitignore.awk`: `[!abc]` → `[^abc]` (ERE negates with ^, gitignore with !) — unfixed it matched a literal `!`, i.e. the inverse set, silently dropping the pattern. Pinned by a row in the table-driven smoke block (`apps/[!x]ebsite/build/`), verified RED. Known-approximation note extended to cover bracket expressions and the `[]abc]` leading-`]` case. - `_ignore-file.sh` "Effect:" now warns the EXIT trap REPLACES the caller's. - `format:check` aggregates instead of short-circuiting, so one run names both prettier AND markdownlint drift. Verified: the `&&` form hid the markdown violation entirely. - turbo.json `//` comment records the globalDependencies blast radius as a deliberate over-approximation, and why the per-task `inputs` option was passed over. - DEVELOPMENT.md and the mdx twin are now byte-identical modulo the link form (a `diff` of the two paragraphs is a one-line diff when in sync); command blocks mirrored both ways (`mdlint:check` added, `(prettier only)` added). The two remaining PR-description findings (a false "were removed" parenthetical and the stale #389/#391 merge-order section) are fixed in the PR body, not here. NOTE: `pnpm turbo test` is red on this base for TWO reasons that pre-date and have nothing to do with #394 — both reproduced on a pristine origin/main worktree: skill-md-mirror on process/bootstrap/quick-mode-defaults.md (#408 merged an unregenerated .claude mirror — the very coupling this round documents) and code-host-routing AC4 on assess-cost/SKILL.md:106 (#388 prose + #389 audit merge-order collision). main's own CI is failing.
…on, temp-file trap
- The --check sparing was broken by script expansion (the inlined '{ body }' put a
brace between the name and its own flag), so a delegated 'pnpm sync-version --check'
was reported as a writer. Lookahead now crosses braces only; real separators still
bound it. Asserted at integration level, not just at pattern level.
- format:check preserves a non-1 status as exit 2 (broken wrapper vs drift) and both
doc copies say what 2 means; the ADL quote matches the shipped body.
- The byte-identical claim is scoped to the load-bearing paragraph — the command
blocks legitimately differ, so demanding equality there taught readers to ignore it.
- The enforced-list paragraph now sits inside the bullet it qualifies.
- _ignore-file.sh: mktemp failure exits 2, trap expands at exit, contract updated.
- Test fixture mirrors the shipped format:check body.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
19ed5d0 to
d1e279d
Compare
…:fix in the gate Round 3's finding #5 was reported as addressed, but on a different file: the one adopters actually receive — dataset/.pair/adoption/tech/way-of-working.md — still said the gate includes 'prettier:fix' and carried a Custom Gate Registry row running 'pnpm prettier:fix'. So every new pair project inherited the exact defect this story removes from pair itself, while pair's own copy was already check-mode. Now 'format:check' in both places, with the ADL cited and the write-mode command named as deliberately outside the gate. The two copies are not mirror-pinned (the adoption layer installs with 'add' behaviour), which is why the drift was invisible. Found while writing /pair-assistant, which must warn users that a freshly installed adoption layer is pair's defaults rather than their decisions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
check:links caught it: the adoption template I just corrected pointed at decision-log/2026-07-31-pre-push-gate-is-check-only.md, which exists in THIS repo and not in the dataset — so every adopter would have received a dangling link. Same class as the isolation rule for plugin-shipped content: what you ship cannot reference what the recipient does not have. The substance stays inline (why the gate checks instead of writing) plus a pointer to record the decision in the adopter's OWN log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Addendum — round 3's finding #5 was still open, on a file nobody looked atHead Round 3 escalated this as the highest-value of nine: "The DATASET bootstrap template still prescribes What was still shipping.
So pair's own gate became check-only while every new pair project inherited a way-of-working telling it the gate runs a write-mode formatter. Both are now Why no guard caught it. The adoption layer installs with A second defect, self-inflicted and caught by the gate. My first version of the correction linked the reason to this repo's ADL ( How it surfaced. Not by re-reviewing this PR — by writing |
Verdict
Scope of this review: the two new commits only — PR: #412 · Author: rucka · Reviewer: independent reviewer agent · Date: 2026-08-01 · Story: US-394 · Type: config (shipped adoption template) Classification matrix — per dimension
Tier = max(assessed) → risk:yellow, confirmed at the story's floor (D17: confirm or raise, never lower). No drift. AssessmentsSecurity — Input validationVerdict: green — no input surface in the diff (markdown prose in a shipped template). Security — Output handlingVerdict: green — no output rendering path touched. Security — AuthenticationVerdict: green — not applicable to the diff. Security — AuthorizationVerdict: green — not applicable to the diff. Security — Introduced vulnerabilitiesVerdict: green — 0 introduced, 0 pre-existing in scope. DetailsAssessed by direct read of the two commits rather than a full CostVerdict: Architecture (Coupling)Verdict: green — the shipped copy and pair's live copy are independent by construction, and that was verified, not assumed (see Positive feedback). DetailsFindings by severityCritical (must fix before merge)
Major (should fix before merge)
Minor (consider)
Questions
Deferred (reported for transparency, not blocking)
Positive feedback
Functionality & requirements (AC coverage)
Testing & quality gates
Adoption compliance
Tech debt
Documentation
Performance & deployment
Delivered as a PR comment rather than a native review action: this is an independent reviewer agent, so the same verdict-first body is posted here per the template's note and |
…hree claims were false Closes #415. Reviewer findings, all confirmed against the repo: 1. Major — the shipped Custom Gate Registry row flipped write->check but kept Required: No. Per quality-gates.md that means Advisory: failure is a WARNING, not a blocker. With an auto-fixer that was coherent; with a check it is STRICTLY WORSE than what it replaced — the violation is neither fixed nor blocked. Now Required: Yes, with the reason in the cell, matching pair's own row. 2. Major — the ADL's own Consequences still said the dataset template 'still shows prettier:fix … tracked as #415' and that this decision has no dataset reach. Both went false the moment the previous commit landed. Corrected, and the framework example in quality-gates.md is fixed here too (maintainer call), which closes the rest of #415 rather than leaving it half-done with a Problem section citing lines that no longer exist. 3. Minor — the template asserted 'the gate runs at pre-push' unconditionally, while the shipped KB blesses fixing staged files at pre-commit, where the objection inverts. Now conditional, with the invariant that survives either layout. 4. Minor — it advertised format:check/format without saying they are the adopter's scripts to provide; a missing script surfaces as a warning indistinguishable from a formatting warning, which compounds finding 1. 5. Minor — table alignment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Remediation — round 5
Head
Still open — the Question, and my answer to it. The shipped template does not carry pair's "no step reachable from the gate writes files" invariant verbatim. That was deliberate and stays deliberate: the sentence names a composition guard ( One thing worth flagging beyond the findings, because the reviewer raised it and it is true: no conformance test pins the shipped gate row, which is exactly why this drift survived a whole story — pair's copy was corrected while the template was not, and the adoption layer is deliberately not mirror-pinned ( |
PR Information
Story: #394 · Type: Bug fix (repo tooling) · Priority: High (P1) · Assignee: @rucka
Classification:
risk:yellow·cost:greenSummary
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):
quality-gaterunsformat:checkinstead ofprettier:fix/mdlint:fix(rootpackage.json), andpnpm gate:compositionfails if ANY write-mode step (both formatters, their.sh/bin forms, raw--write/--fixCLIs, and eslint's autofix) becomes reachable from the gate —packages/dev-tools/src/quality-gates/pre-push-gate-composition.ts+ 43 unit assertions.pnpm format:check=pnpm prettier:check && pnpm mdlint:check(delegating, so check and fix cover the same set, root*.mdincluded) exits 1 and the failure namespnpm formatas the remedy.--no-verifyis no longer needed for a normally formatted changepair-cliversion-check files) and generated trees can no longer block a push: both wrappers delegate to the repo's.gitignorefiles, covered byscripts/smoke-tests/scenarios/format-ignore-delegation.sh.What Changed
The
quality-gatescript — which the husky pre-push hook runs — executedprettier:fixandmdlint:fixrepo-wide in write mode. It now runs them in check mode, with the fix moved to an explicitpnpm format.formatpnpm prettier:fix && pnpm mdlint:fix— the explicit fix command (was the gate's write-mode step)format:checkpnpm prettier:check && pnpm mdlint:check— what the gate runs nowgate:compositionBoth
formatandformat:checkdelegate 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 rootmdlint:*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:
--no-verifyafter the hook reformatted two unrelatedpair-clitest files;git add -Aand had to be reverted;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.mdcarries a one-line pointer, per the sibling ADL's precedent.Changes Made
Implementation Details
package.jsonand 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.expandScriptReferencesinlines runner references transitively against the root scripts (bounded depth + visited set) before scanning, so both likely regressions fail: the gate callingpnpm format, andformat:checkredefined asturbo 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 asecho gate:compositiondoes not count — the check is a runner invocation, not a substring), or if the remedy it advertises (pnpm format) stops existing. A malformed rootpackage.jsonreturns a structured failure, not a rawSyntaxErrorstack.prettier:fix,mdlint:fix), the packagebinalias /.shentrypoint (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--writebelonging to another command cannot incriminateprettier: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,writeFileSyncacross every.md/.mdxit walks — its--checkdry-run is spared, bounded to the same command segment) andtest: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..gitignorefiles (git root + the package being checked) as additional ignore sources — prettier 3.6 accepts--ignore-pathrepeatedly;markdownlint-cliaccepts only one, so the sources are concatenated into a temp file (it parses gitignore syntax). A new generated dir no longer needs a line intools/prettier-config/.prettierignore(the enumerated entries there stay on purpose — they are the floor for non-git contexts, wheregit rev-parsefails 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 byapps/website/.gitignore:2and**/test-results/by the root.gitignore:25, neither of which had to be restated in a formatter ignore list).playwright-report/(produced by the documenteda11y:report:html),test-results/,.source/and a localapps/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.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, sincedup:checkscans 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/tmppath..gitignoreis 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.jsondeclares the ignore sources asglobalDependencies(root.gitignore, the shared ignore files, bothbin/dirs). The formatter tasks are cacheable, so without it a.gitignoreedit touches no package file and turbo replays a pre-change result — a stale PASS, or a stale FAIL a developer cannot clear.globalDependenciesis the widest lever available — it folds into turbo's GLOBAL hash, so the same edit also bustsbuild/test/ts:check/lint/test:coveragecaches that cannot be affected. That blast radius is accepted deliberately, and the//comment now says so and why the narrower per-taskinputsoption 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).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 tosync-versionandtest:perfper above. Recorded in the ADL's Decision and inway-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 toWRITE_MODE_FORMATTERS" is part of adding a write script to this repo. The previous wording overclaimed.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 namedprettier:fix/mdlint:fix/lint:fix/sync-version/benchmark-update-link, all matched.main()behindrequire.main === module— same shape as the three siblings inpackages/dev-tools/src/quality-gates/. The__dirnamehop 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 sameresolve(__dirname, '..', '..', '..', '..'); ADR-014 records a folder move breaking exactly that once already).DEVELOPMENT.mdand its published twinapps/website/content/docs/contributing/development-setup.mdxcarried the same (now wrong) gate list; the mdx also documented the husky hooks incorrectly (pre-commit runspnpm ts:check, pre-push runs the whole gate). Nothing enforces equality between the two —docs:stalenesschecks skill/CLI counts and dead/docslinks, andformat:checkdoes not readapps/website/contentprose — 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), sodiffing 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:checkwas missing fromDEVELOPMENT.mdentirely.)packages/knowledge-hub/dataset/.skills/<cat>/<name>/SKILL.mdis in format scope (workspace package); its generated twin.claude/skills/pair-<prefixed>/SKILL.mdis not (.claude/is not a workspace member —pnpm-workspace.yamllistsapps/*,packages/*,tools/*, and the rootmdlint:checkextra step globs a non-recursive'*.md'). Meanwhilepackages/knowledge-hub/src/tools/skill-md-mirror.tsasserts the two are byte-equal by regenerating the.claudecopy through the realpair updatetransform. So a format-only edit to the dataset copy turns a greenformat:checkinto a redskills:conformancelater in the same gate, and a contributor told only "runpnpm format" loops straight back to--no-verify. The mechanism pre-dates this PR (the old gate ranmdlint:fixin-gate, hiding it), but this PR is what promotespnpm formatto 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, thedevelopment-setup.mdxtwin, andPRE_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/**/*.jsonbut neither.claude/skills/**/*.mdnor the mirror coupling.format:checkaggregates instead of short-circuiting.pnpm prettier:check && pnpm mdlint:checkmeant 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). Nowpnpm 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.awkhandles 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.gitignoreentry in this repo uses brackets, so impact today is nil — but the next author was going to read bracket support as complete._ignore-file.shdocuments that its EXIT trap REPLACES the caller's. POSIXtrapassigns 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
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.shpackage.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 inpackages/dev-tools/src/quality-gates/(two lines each: they import the sharedREPO_ROOT), the twopair-cliversion-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.mdTesting
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 … --fixflagged (plus the negative: a--fixbelonging to another command leaves check-mode eslint alone), and the real gate with&& pnpm lint:fixappended now fails ·sync-versionflagged by script name and by module path, its--checkdry-run spared, and a--checkin the next command not laundering the write ·test:perfandbenchmark-update-linkflagged · the real gate with&& pnpm sync-version 0.4.2and with&& pnpm test:perfappended now fails · the remedy naming the dataset↔.claudemirror 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,.shentrypoint, 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 rootpackage.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.shbuilds a throwaway git repo whose.gitignoreholds a path-anchoredpkg/gen/, runs all four wrappers frompkg/, 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 ownpackage.json+.prettierrc.jsonso prettier's config discovery cannot escape the fixture. Finally,_reanchor-gitignore.awkis covered table-driven in the same scenario — one fixture.gitignorethrough 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 gitignoredgen/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 inrun-all.sh.Test Results
Re-run on the rebased tree (
8f8956bd+ this branch), not the pre-rebase one:turbo ts:check lint(14 tasks)@pair/dev-tools testpre-push-gate-composition.test.ts@pair/pair-cli test@pair/content-ops/@pair/brand/@pair/website test@pair/knowledge-hub testorigin/main, see the callout aboveformat:checkgate:composition✓ pre-push gate composition: check-mode onlyhygiene:check/docs:staleness/skills:conformancedup:checkformat-ignore-delegation.shsmokeThe only red is— resolved. Both failures were@pair/knowledge-hub testmain's, not this branch's (anassess-costword order the #389 guard read as a violation, and aquick-mode-defaults.mdmirror link); they are fixed onmainand this branch is rebased onto that fix. The full gate is green here: 22 + 6 + 6 tasks.Quality Assurance
Review Areas
lintand coverage but noformat:check, and--no-verifystill 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..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..pair/knowledge/.../quality-gates.md+ the bootstrap dataset template) still showsprettier:fixas 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) andtech-stack.md(markdownlint wired viamdlint:checkonly). 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 backingrisk:yellow · cost:greenis now written into the issue body with a note saying so, and therisk:yellowlabel is applied. Same handling as #407.Baseline cleared on the tree this lands on. The branch was rebased onto current
origin/maintwice during round 5 (#389/#391 merged, then #405/#408). Current main carried 2xMD049/emphasis-styleatpackages/knowledge-hub/dataset/.skills/capability/assess-cost/SKILL.md:155(*as of then*in a file whose established style is underscore) — insidemdlint:check's scope, so without clearing itpnpm format:checkwould 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.claudetwin propagated by hand — which is how the two-step-remedy trap above was found.One-time sweep. The two
pair-cliversion-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
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: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 theREPO_ROOT is resolved from this file's location…sentence). Resolved keeping BOTH: main's doc-author rule plus this PR'sREPO_ROOT comes from ./repo-rootsentence.scripts/smoke-tests/run-all.shand [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.apps/pair-cli/src/commands/kb-info/version-check.test.tsandversion-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-runpnpm formaton 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.way-of-working.md— a small conflict on the Quality Gates bullet, which this PR rewords again in round 5.tools/prettier-config/,tools/markdownlint-config/andturbo.jsonare untouched by all of them. The rootpackage.jsonwas shared with the changesets release PR chore: release v0.5.0 #417 (version + CHANGELOG hunks only,scriptsuntouched, so either merge order auto-merged) — chore: release v0.5.0 #417 is now closed and the pending changeset removed frommain, so that overlap is gone.Important
mainis red independently of this PR, andpnpm turbo testtherefore fails on this branch too. Both failures were reproduced on a pristineorigin/mainworktree (8f8956bd), in files #394 never touches: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.claudemirror. This is precisely the dataset↔.claudecoupling round 5 documents.code-host-routingAC4 ›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 updateregeneration, and a prose or regex adjustment respectively). Everything else in the gate is green on the rebased tree — see Quality Gates below.Closes #394
🤖 Generated with Claude Code