[US-400] fix: coverage-gate.sh was committed 644 (unrunnable) — smoke suite now runs pre-merge in CI - #424
Open
rucka wants to merge 16 commits into
Open
[US-400] fix: coverage-gate.sh was committed 644 (unrunnable) — smoke suite now runs pre-merge in CI#424rucka wants to merge 16 commits into
rucka wants to merge 16 commits into
Conversation
…runnable since #368) - Guard reads `git ls-files -s`, not the filesystem: the mode is a property of the commit, so a locally +x file with a 644 commit still fails - Scope: scenarios/** and run-all.sh must be 100755; lib/utils.sh (sourced) and fixtures/README exempt; empty match fails loudly (a renamed folder must not silently disable the guard) - Check-only (ADL 2026-07-31): names file, mode and the chmod command, never fixes - RED first: the repo assertion failed on coverage-gate.sh (100644) — 19/20 green, then 20/20 after `git update-index --chmod=+x` - Task: T-1 — git-mode guard + the live defect it catches Refs: #400
…list carries its exclusions - scenario_state/file_mode (lib/utils.sh): the state is decided BEFORE execution, so "cannot run" is never reported as "ran and failed" again - NOT EXECUTABLE names the file, its mode and the COMMIT-level remedy; check-only, the runner never chmods (ADL 2026-07-31) - CI_TESTS moves to lib/ci-tests.sh with CI_EXCLUDED reasons — one copy, audited: every scenario is in the CI list or excluded WITH a reason (AC7 executable) - Rule: CI runs every offline-safe scenario. Added registry-exclude.sh (0.8s) and no-dataset-in-artifacts.sh (12s), both measured passing offline; removed default-resolution.sh, which declares OFFLINE_SAFE=false yet sat in the CI list (same unchecked-claim shape as the 644 mode) — it still runs in the local suite - scenarios/runner-outcomes.sh asserts the three states, the mode in the message, check-only, and the list invariants (shell surface -> smoke test, ADL 2026-07-13) - Demonstrated end-to-end on fixtures: PASS / NOT EXECUTABLE (mode 644) / FAIL / MISSING, four distinct rows, runner exit 1 - Task: T-2 — runner outcome vocabulary + documented CI list Refs: #400
…lready replaced - First execution ever of this scenario (born mode 644 in #368) found two stale assertions, both about the ratchet's command plan: 1. it demanded `config --add remote.origin.fetch`, while the module deliberately uses the TRANSIENT `git -c remote.origin.fetch=<refspec>` and its unit tests assert `config --add` is NOT used — the scenario asserted the opposite of the decided behavior 2. it refuted the bare substring `git checkout`, which also matches the restore step's `git checkout -- <configPath>` (reverts one file, switches nothing) - Realigned to the decided behavior, with sharper refutations: no `git checkout chore/`, no `git switch`, no `git branch`, and the refspec must not be persisted - Evidence for the story: an assertion nobody executes is not an assertion - Task: T-3 — the scenario the mode fix brought back to life Refs: #400
…build - New `smoke` job (checkout -> pnpm -> install -> run-all.sh --ci --cleanup), its own status check: a red here means a real-CLI scenario broke, a different diagnosis from a red unit/lint gate - Measured cost recorded NEXT TO the job with its revisit threshold (AC6): 1m58s for 18 scenarios vs ~4 min for `build` => runner time, zero wall-clock; if it approaches `build` it gets scoped or scheduled, deliberately - Check-only (AC8): no chmod, no commit from CI; runner-outcomes.sh asserts the workflow runs the suite, records the cost/threshold, and never chmods - No secrets used => a fork PR is a full-strength run - Task: T-4 — the CI job Refs: #400
…orkflow runs it" claims - ADL 2026-08-11-smoke-suite-runs-in-ci-pre-merge: nightly rejected on measured data, four outcomes, git-mode guard, check-only, CI_TESTS as a rule with recorded exceptions (+ the alternatives that were rejected and why) - assert_pinned_bug's reach note: a pinned assertion in the CI list is now a CI guarantee, manual only outside it - marketplace ADL (#277): dated update — registry-exclude.sh IS re-run per change - way-of-working gate registry: the CI-safe list runs on every PR as `smoke` - smoke README: outcomes table + the "which scenarios run in CI" rule - ci.yml: measured cost refined to 1m40s-1m58s over two full local runs - Verification: run-all.sh --ci exits 0, 18/18 PASS (AC5) - Task: T-5 — documentation + decision record Refs: #400
rucka
added
risk:yellow
Collaborator
Author
Verdict
PR: #424 · Author: gianlucacarucci5 · Reviewer: independent review agent · Date: 2026-08-11 · Story: US-400 · Type: bug fix + feature Tier requirements (🟡, quality-model §4): 1 reviewer · SLA 1 working day · standard checklist · reviewer approval; no explicit-approval rule (that is 🔴 only). Classification matrix — per dimension
Tier = max(assessed) = yellow, confirming the story's refinement-time tier (no raise). Review value is a floor (D17): confirmed, not lowered. AssessmentsSecurity — Input validationVerdict: green — the only new externally-influenced input is the git index text parsed by Details
Security — Output handlingVerdict: green — output is console text and a markdown report; no HTML/SQL sink. Details
Security — AuthenticationVerdict: green — not applicable; the job authenticates to nothing and consumes no token. Security — AuthorizationVerdict: yellow — the DetailsFork PRs get a read-only token by GitHub's own rule, so the exposure is limited to same-repo branch PRs. It is nonetheless a new job that runs arbitrary shell from the branch under review; Security — Introduced vulnerabilitiesVerdict: green — 0 introduced, 1 pre-existing (the missing Details
No secret is read, written or logged by the new job; CostVerdict: Details
Architecture (Coupling)Verdict: green — the change reduces coupling: the CI list stops being a second copy inside Details
DetailsFindings by severityCritical (must fix before merge)
Major (should fix before merge)
Minor (consider)
Questions
Positive feedback
Functionality & requirements (AC coverage)
Edge cases: Testing & quality gates
Adoption compliance
Tech debt
Documentation
Performance & deployment
Process notes (independent review): delivered as a PR comment rather than a native review event — a different actor from the author, with self-approve blocked; the artifact is the same code-review template body. |
… .trim() rewrote paths
parseGitIndexEntries took the mode as slice(0, indexOf(' ')): on a line with a
tab but no space that is slice(0, -1) — the line minus its last char — reported
as a non-executable file with a nonsense mode. Now the mode comes from the
pre-tab segment and an unreadable line is dropped, not diagnosed.
The blanket .trim() also rewrote a path with a trailing space, so the chmod
command in the failure named a file that does not exist.
Doc claim corrected too: git ls-files -s reads the INDEX, so the guard asserts
the STAGED mode, not 'the commit' (identical in CI; not identical locally).
…as only grepped The job ran pnpm install then run-all.sh, whose fallback is 'pnpm --filter @pair/pair-cli build' — that bypasses turbo's dependsOn graph, so @pair/content-ops (exports -> dist/) is never built and the CLI compile dies with ~45 TS2307. Nothing packaged, no scenario ran, exit 1. Now the job builds '--filter=@pair/pair-cli...' explicitly, and the runner's own fallback does too. runner-outcomes.sh now EXECUTES run-all.sh against a fixture scenarios/ dir + CI list (pass, fail, 644, listed-absent) and asserts the four report rows, exit 1, that the 644 fixture is untouched, and that both halves of the remedy name ONE path. Previously it grepped the runner's source, so a token in a comment passed. Also: file_mode chain terminated (it could abort the suite under set -e); 00-create-install-package.sh is fatal on a CI dry-run failure instead of a green no-op, and reuses the preflight's artifact instead of packaging twice; the ci.yml chmod guard is scoped to smoke-test paths; smoke job gets contents:read + 15m.
… observed in CI
The stated rule ('CI runs every scenario that declares OFFLINE_SAFE=true') was
stricter than the code: is_offline_safe defaults to true and only
'^OFFLINE_SAFE=false' opts out — while 5 scenarios declared it in a comment and 5
declared nothing, yet all ran. Now stated as enforced (opt-OUT), and the 5
decorative '# OFFLINE_SAFE=true' comments are real declarations.
AC6 figure replaced with the measured CI number: smoke 1m53s vs build 3m59s in
run 31538360697 — threshold arithmetic restated against it (~75% of build = ~3m).
'as committed' -> 'staged/index' wherever git ls-files -s is described, and the
default-resolution exclusion no longer claims release.yml covers the no---source
fallback: smoke-test-npm-artifact.sh Test B WARNS on failure, so it is evidence,
not a gate — the fallback stays a manual guarantee.
…o task `test` is cacheable with package-scoped inputs, so a scenario added mode 644 without touching @pair/dev-tools replays a cached PASS and the guard never runs — the pre-push guarantee the module doc claimed did not hold. Give it the thin CLI the sibling gates already have (`smoke-modes:check`), wire it into the root `quality-gate` chain and into its own CI `build` step; both run unconditionally. Also narrows the predicate to what the runner actually globs (`*.sh` directly in `scenarios/`) so a data fixture next to the scenarios is not told to `chmod +x` a JSON file, and reads the index with `-z` so a non-ASCII path is not C-quoted into a remedy that matches no file. Refs: #400
…e they held
`repo_relative_path`'s `${REPO_ROOT:-}` guard was decorative: with the root
empty the pattern degenerates to `/*`, matches every absolute path and strips
the leading slash, so the NOT EXECUTABLE remedy printed
`chmod +x Users/x/repo/...`. Now the unknown-root case falls through to the
verbatim branch, asserted both ways.
The check-only tripwire matched `chmod` only as a line's FIRST word, so
`... && chmod +x`, `... || chmod +x` and `$(chmod ...)` all passed it; it now
matches any command position while still exempting the runner's PRINTED
remedy. Section 5 greps the workflows DIRECTORY instead of pinning
`ci.yml`, so relocating the smoke job (as #413 does for `format`) is a
refactor rather than a red claiming the suite is not wired into CI. It also
now asserts — and EXECUTES — the guard's uncacheable entrypoints.
Refs: #400
…contain
Answers the review's open question. The artifact's contract is `bin/pair-cli`
— executed, never imported — so bundled declarations are optional; recorded as
ADL 2026-08-12-manual-cli-artifact-types-are-optional.md.
But `package.json` was written with `types: bundle-cli/index.d.ts`
UNCONDITIONALLY, before the generator ran, so every dts-bundle-generator
failure shipped a dangling pointer. Test-first: the packaging scenario now
asserts `types` and the file agree in BOTH directions — red on this tree
("declares types=... but bundle-cli/index.d.ts is not in the artifact"), green
after pruning the field when the file is absent.
The generator stays best-effort, and its message now states the intent instead
of reading like a swallowed failure — since #400 it prints inside a green CI
row on every PR.
Refs: #400
… workflows stop masking their modes Absorbs #431 into this story per ADL `2026-08-12-implementation-never-files-a-card-it-extends-the-story.md`. This story exists to stop committed file modes being masked, and its guard covered `scripts/smoke-tests/` only — while `release.yml` ran `chmod +x scripts/workflows/release/<script>.sh` before executing each of eight scripts, and `website-preview-deploy.yml` a ninth. A release script committed 644 would therefore run green in CI forever and fail for anyone executing it directly: exactly the condition `coverage-gate.sh` sat in for weeks. - `MUST_BE_EXECUTABLE.releaseDir` added; `requiresExecutableBit` applies ONE rule to both folders — a `*.sh` directly inside is executed, anything deeper or of another extension is read - the nine `chmod +x` lines are gone from both workflows - `readSmokeTestsIndex` now reads BOTH trees. Extending the predicate alone was not enough and is worth recording: the repo-level check reads that index, so a release script staged 644 stayed invisible and the guard passed — verified by injection, which was GREEN until this read was widened. A guard that passes by not looking is the failure this module's own empty-index branch exists against. - tests cover the load-bearing negatives too: `README.md` and `fixtures/sample-project/{index.js,package.json}` are committed 644 ON PURPOSE and must not require the bit — demanding it would be a real regression - one test asserts no workflow chmods a committed release script again All nine release `.sh` are already 100755, so the widened guard is green on arrival: this closes the hole before it is used. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The PR body and story #400 justified absorbing #431 by citing `2026-08-12-implementation-never-files-a-card-it-extends-the-story.md`, a file that existed in no commit — untracked in the author's checkout, so the sole authority for the scope extension was unreadable at the merge gate. Records it here (template order + Adoption Impact), plus the way-of-working policy bullet next to "one PR per story". The References section says plainly that the enforcement point — the `implement-batch` reviewer/fixer prompts — lands with the orchestrator's own change, not with this PR. Refs: #400
Since the guard grew to cover `scripts/workflows/release/` (#431), its failure still said "N smoke-test file(s)" and "the runner cannot execute them: the scenario is listed" — a mechanism that does not exist for release scripts, and which hides the real consequence (it runs in CI only because a workflow chmods it, and fails for everyone else). The saw-nothing failure named only the scenarios tree, so a renamed release folder would have disabled half the guard silently. Test-first: two cases red (a release offender must not be called a smoke-test file; the saw-nothing message must name every guarded tree), then the wording. 35/35 green. Refs: #400
… bans The ADL committed in round 2 is Active and bans an implementing/reviewing agent from creating work items; its own Adoption Impact item 3 named /pair-process-review's deferral instruction as the enforcement point and left it unmet. Phase 4.2 Step 5 still said "deliberate promotion after review via /write-issue (with the tech-debt label)" — the exact line that produced #426-#431 — so the policy would have shipped next to the instruction violating it. Test-first: implementation-never-files-a-card.test.ts (cross-cutting, per 2026-07-18-conformance-test-per-file-not-per-story) pins BOTH copies — dataset source + installed mirror — on: no write-issue reference anywhere, the ban stated in the tech-debt step, an already-existing card citable, the merge gate named as the alternative, and the ADL bullet no longer left open. 7/8 red before the edit. Step 5 now: record the debt in the report and stop; cite an existing card by number, otherwise fix it in this PR or leave it as an actionable finding for the maintainer at the merge gate. ADL item 3 records that it landed here.
Collaborator
Author
Remediation — final synthesis (review converged, head
|
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Information
PR Title: [US-400] fix: coverage-gate.sh was committed 644 (unrunnable) — smoke suite now runs pre-merge in CI
Story/Epic: #400 (scope extended 2026-08-12: #431 absorbed and closed)
Type: Bug Fix (defect + the systemic gap that hid it)
Priority: High
Labels: risk:yellow
Head at time of writing:
7d30b9a7· 16 commits · 28 files changed (+1603 / −72)Summary
What Changed
scripts/smoke-tests/run-all.sh --ciwas invoked by no workflow. This PR makes it a real status check, gives the runner a vocabulary that can say "this scenario could not run", and stops the repo masking committed file modes anywhere.smokejob inci.yml, parallel tobuild, runningrun-all.sh --ci --cleanupon every PR — after a dependency-awarepnpm turbo build --filter=@pair/pair-cli....PASS/FAIL/NOT EXECUTABLE/MISSING, decided byscenario_statebefore a scenario is executed. TheNOT EXECUTABLEline names the file, its mode, and one repo-relative command that fixes the tracked mode.packages/dev-tools/src/quality-gates/smoke-scenario-modes.tsreadsgit ls-files -s -z(the git index, never the filesystem). It runs as its own root gate step,pnpm smoke-modes:check, in thequality-gatechain and in the CIbuildjob — not only as a unit test, becauseturbo testis cacheable with package-scoped inputs and would replay a cached PASS for a mode change outside@pair/dev-tools.scripts/workflows/release/too, and the workflows stop chmodding — the absorbed Git-mode guard covers smoke scenarios only — release.yml chmods its scripts before running them, masking their committed modes #431, see the scope-extension section at the end.scripts/smoke-tests/lib/ci-tests.sh, one copy, with a recorded reason for every exclusion;scenarios/runner-outcomes.shaudits the invariant in both directions by executing the runner against fixtures.scenarios/coverage-gate.shwas committed100644in PR [#282] feat: coverage baseline + CI guardrail (tier-aware pipeline gate) #368 and had been unrunnable since. Running it for the first time found two stale assertions inside it, also fixed here./pair-process-review's tech-debt step no longer offers to file the card it defers to (round 3, below).Why This Change
CI_TESTSread like a pipeline consumed it. Nothing did.coverage-gate.shsat dead for weeks because the two failures were indistinguishable: nobody ran the list, and when someone did,Permission deniedrendered as a plainFAIL. Every real-CLI guarantee in the suite — includingassert_pinned_bug, whose whole value is failing loudly when a pinned upstream bug is fixed — depended on a human remembering.Story Context
User Story: #400 — the smoke suite's CI-safe list runs pre-merge, and a scenario that cannot run says so.
Acceptance Criteria coverage (8 original + 3 added with the scope extension):
smokejob runsrun-all.sh --ciin parallel withbuild, own status check.github/workflows/ci.yml(smokejob)buildandsmokeare separate contextsscaffold-kb.sh(pins #395) is inCI_TESTS| scaffold-kb.sh | ✅ PASS |NOT EXECUTABLEdistinct fromFAILandMISSING, naming file + moderun-all.shrun_scenario+scenario_state; asserted byrunner-outcomes.shdriving the runner against fixtures| scaffold-kb.sh | 🚫 NOT EXECUTABLE (mode 644) |test -xsmoke-scenario-modes.ts(+ 35 unit tests) + thesmoke-modes:checkroot steppnpm smoke-modes:check→✓ 31 tracked scripts are staged 100755; red reproduction belowcoverage-gate.shis100755andrun-all.sh --ciexits 0 on a clean checkoutci.ymlMEASURED COST/REVISIT THRESHOLDblock; ADLcf25c981): smoke 1m59s vsbuild3m37s, same runlib/ci-tests.shCI_EXCLUDED; enforced byrunner-outcomes.sh(a scenario in neither array fails the suite)runner-outcomes.shPASS in the green runschmodin runner/workflow;runner-outcomes.shasserts the 644 fixture is untouched after a rungate:compositionalso holds the whole gate check-modescripts/workflows/release/as well asscripts/smoke-tests/MUST_BE_EXECUTABLE.releaseDir, read byrequiresExecutableBitand byreadSmokeTestsIndex(the index read used to name only the smoke tree, so release paths passed by never being looked at)cf25c981; tests "the real index read includes the release scripts", "every tracked release script … is executable by its staged git mode"chmod +x-es a repo-committed script (9 lines removed)release.yml(8),website-preview-deploy.yml(1)cf25c981; test "no workflow chmods a repo-committed release script any more" greps both workflows644fails the guard, named with its mode as read from gitcheckSmokeScenarioModesChanges Made
Implementation Details
smokejob —permissions: {contents: read},timeout-minutes: 15, dependency-aware build, thenrun-all.sh --ci --cleanup.--ciselects the list, setsPAIR_DIAG=1, and appends the report to the job summary.scenario_state/file_modeinlib/utils.sh;run_scenariorenders one row per outcome and never chmods.smoke-scenario-modes.ts+.test.ts, plus the thin CLIpnpm smoke-modes:checkwired into the rootquality-gatechain and its own CIbuildstep (a guard reachable only from a cacheable turbo task is not enforced — now recorded inway-of-working.md§ Quality Gates). Coversscripts/smoke-tests/{run-all.sh,scenarios/*.sh}andscripts/workflows/release/*.sh; fails loudly when the index carries no runnable file at all, naming both trees, so a renamed folder cannot silently disable it.lib/ci-tests.sh(CI_TESTS+CI_EXCLUDEDwith reasons), sourced by the runner, audited byrunner-outcomes.sh.run-all.shacceptsSCENARIOS_DIRandCI_TESTS_FILEoverrides (both absolute by default) so its own report can be tested against fixtures rather than grepped.package-manual.shadvertised atypesentry it did not ship; the packager now emits only what it contains and00-create-install-package.shasserts it (ADL2026-08-12-manual-cli-artifact-types-are-optional.md).coverage-gate.shmode100755; it also demandedgit config --add remote.origin.fetch …that Coverage baseline auto commit-back — CI persists/ratchets the baseline (opt-in, on top of #282) #372/PR [US-372] feat: coverage baseline auto commit-back — monotonic ratchet as a bot PR, opt-in and off by default #405 had already replaced with the transientgit -c …form, and refuted a baregit checkoutsubstring that also matches the restore step.Files Changed
Complete
git diff --name-status origin/main...HEAD(28 paths):packages/dev-tools/src/quality-gates/smoke-scenario-modes.ts,…/smoke-scenario-modes.test.ts,packages/knowledge-hub/src/conformance/implementation-never-files-a-card.test.ts,scripts/smoke-tests/lib/ci-tests.sh,scripts/smoke-tests/scenarios/runner-outcomes.sh,.pair/adoption/decision-log/2026-08-11-smoke-suite-runs-in-ci-pre-merge.md,.pair/adoption/decision-log/2026-08-12-manual-cli-artifact-types-are-optional.md,.pair/adoption/decision-log/2026-08-12-implementation-never-files-a-card-it-extends-the-story.md.github/workflows/ci.yml,.github/workflows/release.yml,.github/workflows/website-preview-deploy.ymlpackage.json(smoke-modes:check+ thequality-gatechain),packages/dev-tools/package.jsonscripts/smoke-tests/run-all.sh,scripts/smoke-tests/lib/utils.sh,scripts/smoke-tests/README.md,scripts/smoke-tests/scenarios/{00-create-install-package,coverage-gate,format-ignore-delegation,pr-state-flow,tier-aware-gate}.shscripts/workflows/release/package-manual.shDEVELOPMENT.md,.pair/adoption/tech/way-of-working.md,.pair/adoption/tech/infrastructure.md,.pair/adoption/decision-log/2026-07-28-marketplace-plugin-packaging.mdpackages/knowledge-hub/dataset/.skills/process/review/SKILL.md(dataset source),.claude/skills/pair-process-review/SKILL.md(installed mirror) — round 3Testing
Test Coverage
smoke-scenario-modes.test.ts— parser edges (malformed line dropped, mode taken from the pre-tab segment, trailing-space path preserved verbatim,-zNUL parsing), the exemption boundary (lib/utils.shsourced ⇒ no bit; nestedscenarios/**and releasefixtures/**⇒ no bit;*.shdirectly in either guarded folder ⇒ bit required), the failure-message contents for both trees, and live assertions against this repo's own index (smoke tree and release tree).runner-outcomes.shexecutesrun-all.shagainst a fixtureSCENARIOS_DIR+ fixture CI list (755 pass, 755 fail, 644, listed-absent) and asserts the four report rows verbatim, exit 1, thatNOT EXECUTABLEnever renders asFAIL, that the 644 fixture is still 644 after the run, and that both halves of the printed remedy name one identical path.packages/knowledge-hub/src/conformance/implementation-never-files-a-card.test.ts(round 3) — asserted on both copies of/pair-process-review's SKILL.md: nowrite-issuereference anywhere in the skill, the tech-debt step states the ban and allows citing an existing card, the merge gate is named as the alternative, the ADL slug is cited, and the ADL's own review-side bullet no longer stands open. 7 of the 8 were red before the fix. The knowledge-hub mirror guard (skill-md-mirror.test.ts, 83 tests) is what proves the two copies did not drift apart.Test Results — CI evidence, re-pointed at the head
These supersede the
d66abf36/0dc43057runs cited in earlier revisions of this description.GREEN on
cf25c981(the parent of the current head; last commit that changed shipped code paths) — run 31616635648:GREEN on the current head
25b1cc21— run 31619416353:build✅,smoke✅,secret-scan✅ — all three green. The two commits on top ofcf25c981are a new ADL + away-of-working.mdbullet (41fdf075) and a guard failure-message rewording with two new unit tests (25b1cc21) — no behavioral path in the runner or the workflows changed.GREEN on the current head
7d30b9a7— run 31621079558:build✅,smoke✅,secret-scan✅. The single commit on top of25b1cc21changes no shipped code path: two markdown skill files, one new conformance test, one ADL bullet. The full local gate was run stage by stage and the pre-push gate re-ran the whole chain — see below.DELIBERATE RED — run 31538595395 on scratch branch
smoke-deliberate-red-us400(=d66abf36+git update-index --chmod=-x scripts/smoke-tests/scenarios/scaffold-kb.sh). Both jobs went red for the two different intended reasons:LOCAL RED on the current head — AC11, the absorbed #431 criterion:
Local gate on the head (tiering is
disabledinway-of-working.md, so the full suite ran): ts:check,turbo test(144 tests), lint, format:check (prettier + mdlint),gate:composition,hygiene:check,smoke-modes:check,docs:staleness,skills:conformance,dup:check(1.07%),turbo build— all PASS.Reviewer Guide
What round 3 changed (1 finding, Major, resolved — none escalated)
The Active ADL this PR commits banned filing cards, while the skill it names still offered to file one. The ADL's § Adoption Impact item 3 said
/pair-process-review's deferral instruction "must stop offering to file the card it defers to", and — unlike item 2, which is explicitly carved out to the orchestrator's own change — carried no deferral qualifier. The skill was untouched: Phase 4.2 Step 5 still read "note it as a recommendation for deliberate promotion after review via/write-issue(with thetech-debtlabel)" — the exact instruction that produced #426–#431. The repo would have shipped an Active policy next to the instruction violating it.Both remedies the finding offered were taken:
7d30b9a7), identically in the dataset source and the installed mirror — the review records the debt and stops; it may cite an already-existing card by number; otherwise the debt is fixed in the PR (extending the story that surfaced it) or left as an actionable finding the maintainer judges at the merge gate. There is now nowrite-issuereference anywhere in the review skill. Test-first: the conformance file above was written and run red (7/8) before the edit.Deliberately not changed, stated rather than deferred to a card:
/pair-capability-analyze-debtstill documents deliberate promotion via/write-issueby the developer — the ADL's own carve-out ("new items enter deliberately, from refinement or from the maintainer"), and the review composition path is now governed by the review skill's own step, which overrides it. Rewriting a human-facing capability's promotion path is a different decision from banning the agent from taking it. ADL item 2 (theimplement-batchprompts) still lands with the orchestrator's change, as it has said since round 2.What round 2 changed (2 findings, both Major, both resolved — none escalated)
Both findings were about the record, not the code, and both are fixed here rather than deferred:
cf25c981had fully implemented it, omitted 7 changed files from Files Changed, and cited CI evidence four commits behind the head. The body is republished from the current head: the scope-extension section below states what shipped, Files Changed is the complete 25-path list, and the CI evidence above supersedes the old SHAs. The four post-round-1 commits (fa3c147e,9d1d2b02,8a7a248d,cf25c981), previously undescribed, now appear in Summary and AC coverage..pair/adoption/decision-log/2026-08-12-implementation-never-files-a-card-it-extends-the-story.md(41fdf075) — with the adoption update its template requires (a policy bullet inway-of-working.mdnext to "one PR per story"). Its References say plainly that the enforcement point (theimplement-batchreviewer/fixer prompts and the tests pinning them) lands with the orchestrator's own change, not with this PR: this commit is the decision, not its mechanism.Found while fixing and fixed here (not a review finding): the guard's failure text still called a
scripts/workflows/release/offender a "smoke-test file" whose "runner cannot execute" it — a mechanism that does not exist for release scripts — and its saw-nothing failure named only the scenarios tree, so a renamed release folder would have disabled half the guard silently. Test-first, 2 cases red → 35/35 green (25b1cc21).What round 1 changed (15 findings, all resolved)
The Critical finding was real: the job failed on its own head because
run-all.sh's build fallback (pnpm --filter @pair/pair-cli build) bypasses turbo'sdependsOn, leaving@pair/content-opsunbuilt (~45TS2307). The job now builds--filter=@pair/pair-cli..., and the runner's own fallback was fixed too. Other notable changes:runner-outcomes.shno longer greps the runner's source (a token in a comment used to pass) — it drives the runner and reads the report;parseGitIndexEntriesno longer invents a mode from a malformed line nor rewrites paths with.trim(); thechmodtripwire onci.ymlis scoped to smoke-test paths;00-create-install-package.shis fatal in CI on a packaging preflight failure instead of a silently-green no-op, and reuses the preflight's artifact instead of packaging twice.Two claims were corrected rather than defended:
git ls-files -sreads the index. Docs say staged/tracked, and say why the index (notls-tree HEAD) is the right target: it catches achmod +xthat was nevergit add-ed, one push earlier.OFFLINE_SAFE=true" — the enforced rule is opt-out (default true). Restated as enforced; the five decorative# OFFLINE_SAFE=truecomments are now real declarations. The five scenarios that declare nothing were left alone deliberately:install-basic.shreads$OFFLINE_SAFEas a control variable.default-resolution.shexclusion reason claimedrelease.ymlcovered the download path. Checked:smoke-test-npm-artifact.sh"Test B" does runpair installwith no--sourceagainst the real artifact, but it warns on failure — evidence, not a gate. Both the ADL andCI_EXCLUDEDnow say the fallback stays a manual guarantee.Review focus areas
ci.ymlsmokejob — the build step's filter syntax and the recorded cost/threshold arithmetic (restated against thecf25c981run: 1m59s vs 3m37s).smoke-scenario-modes.ts— the exemption boundary (requiresExecutableBit, now two trees), the "saw nothing ⇒ fail" guard, and thatreadSmokeTestsIndexreads both trees (the defect AC9 fixes was an index read that never looked at release paths).runner-outcomes.shsection 3 — the fixture-driven run and the seams it needs inrun-all.sh(SCENARIOS_DIR,CI_TESTS_FILE) as production code rather than test-only escape hatches.Testing the changes
Dependencies & Related Work
assert_pinned_bug), Coverage baseline + CI guardrail (tier-aware pipeline gate) #282 / PR [#282] feat: coverage baseline + CI guardrail (tier-aware pipeline gate) #368 (wherecoverage-gate.sharrived unexecutable), Coverage baseline auto commit-back — CI persists/ratchets the baseline (opt-in, on top of #282) #372 / PR [US-372] feat: coverage baseline auto commit-back — monotonic ratchet as a bot PR, opt-in and off by default #405 (the ratchet command plan the stale assertions predated).format-ignore-delegation.sh(a CI-listed scenario) asserts. With this merged first, that breakage shows as a red check on [tech-debt] format:check does not cover files outside workspace packages (.pair/**, scripts/**, root-level JSON) #414's own PR.permissions:/timeout-minutes:for the otherci.ymljobs (build,secret-scan) — a separate hygiene pass.smoke-deliberate-red-us400exists only as the deliberate-red evidence above. It is not for merge and can be deleted once read.Documentation
2026-08-11-smoke-suite-runs-in-ci-pre-merge.md— pre-merge not nightly (on measured data), four outcomes, index-not-filesystem, nothing auto-corrected,CI_TESTSas a rule with recorded exceptions.2026-08-12-manual-cli-artifact-types-are-optional.md— the manual artifact stops advertising atypesfile it does not ship.2026-08-12-implementation-never-files-a-card-it-extends-the-story.md— the decision this PR's scope extension rests on (committed here, round-2 finding 2)./pair-process-reviewSKILL.md (dataset + mirror) — Phase 4.2 Step 5 rewritten so the review never creates a work item (round 3; the ADL above is its decision record).way-of-working.md— the Custom Gate Registry row for smoke tests; the "a guard whose only caller is a turbo task is not enforced" rule; the "implementation and review never file a new card" policy bullet.DEVELOPMENT.md/infrastructure.md— the Turbo-caching gotcha now names this job as its live example; the manual-artifact contract.scripts/smoke-tests/README.md— the outcomes table and the enforced CI-membership rule.Scope extension — absorbed #431 (2026-08-12)
The story's scope was widened after this PR was opened, per ADL
2026-08-12-implementation-never-files-a-card-it-extends-the-story.md— committed in this PR (41fdf075), so the rule that authorizes the extension is readable at the merge gate. #431 is closed as absorbed and its work belongs to this PR.This story exists to stop committed file modes being masked, and the guard it adds covered
scripts/smoke-tests/only — whilerelease.ymlmasked the modes of its own scripts: it ranchmod +x scripts/workflows/release/<script>.shbefore executing each of them (8 occurrences), andwebsite-preview-deploy.ymla 9th. A release script committed644would run green in CI forever and fail for anyone executing it directly — the exact conditioncoverage-gate.shwas in for weeks.All 12 files under
scripts/workflows/release/are committed100755, so widening the guard is green on arrival: this closes the hole before it is used.Status in this PR: implemented in
cf25c981("[US-400] feat: the git-mode guard covers the release scripts, and the workflows stop masking their modes"), refined in25b1cc21:scripts/workflows/release/tooMUST_BE_EXECUTABLE.releaseDiradded;requiresExecutableBitaccepts*.shdirectly in it (notfixtures/**), andreadSmokeTestsIndexnow passes that pathspec togit ls-files— without it the release scripts would have "passed" by never being readchmod +xlines removed:release.yml(8),website-preview-deploy.yml(1). A test greps both workflows so a new one cannot creep back644release script fails the guard, named with its git modecf25c981(+2 in25b1cc21for the message wording); reproduced locally on the head — see LOCAL RED above