[US-372] feat: coverage baseline auto commit-back — monotonic ratchet as a bot PR, opt-in and off by default

[rucka]

PR Information

Story: #372 · Type: Feature · Priority: Medium (P2) · Assignee: @rucka
Classification: risk:yellow · cost:green

Summary

What Changed

CI can now persist a raised coverage baseline back to tech/coverage-baseline.md — opt-in, off by default, ratcheting only upward.

• coverage-baseline-ratchet.ts — all the decisions in one unit-tested module: the skip predicate, the monotonic per-type plan, the in-place value edit, the git/gh command plan and the refusal classification.
• ci.yml — a thin entrypoint (ADL 2026-07-13) that always exits 0. The guardrail step now also hands its measured numbers forward, so extraction is not duplicated.
• Smoke scenario — scripts/smoke-tests/scenarios/coverage-gate.sh drives the CLI end to end, which is how AC4's termination is demonstrated rather than asserted.
• ADL — 2026-07-30-coverage-ratchet-pr-not-push.md.

Why This Change

Raising a baseline relied on someone remembering to bump the file. The default stays human-committed for projects that want the baseline under review; this adds the automation as a nested opt-in.

Story Context

As a maintainer running the coverage guardrail I want CI to persist a raised baseline back, opt-in and ratcheting only upward So that improvements are locked in automatically.

AC1 default off · AC2 raise above baseline · AC3 never lowers · AC4 no CI loop · AC5 PR-vs-push decided explicitly · AC6 refused write degrades to a warning.

Changes Made

Implementation Details

T-1 was the real decision, and it was taken against the post-#234 regime rather than today's main (the story's ⚠️ dependency). PR #390, parked open, makes pair-review and pair-explicit-approval required contexts and sets enforce_admins: true. Under that regime:

• A direct push to main is structurally impossible for CI — required checks are evaluated on pushes to a protected branch, and a workflow commit can never carry pair-explicit-approval, which is produced by a human approving a PR. The "simplest" option in the story body is not merely awkward after [US-234] feat: PR state flow (gate≠review) + pair review as required check #390: it is rejected by the host.
• Writing to the PR branch mutates head.sha — the exact commit both required contexts are pinned to. A raise would invalidate an approval already given, forcing re-approval on every coverage improvement. The automation would consume the very control PR state flow (gate≠review) + pair review as required check #234 introduces.

So: the raise is proposed only by a push to the base branch, and lands as a bot pull request from chore/coverage-baseline-ratchet — never a push to a protected branch, never a mutation of a PR under review. This is why the story lands working and stays working the day #390 merges.

Credential: a dedicated repo-scoped COVERAGE_RATCHET_TOKEN (contents: write + pull requests: write, no protection bypass). The default CI token is unusable on purpose — a PR it opens triggers no workflow run, so that PR could never satisfy required checks.

Two security properties worth a reviewer's eye:

• github.event.head_commit.message is attacker-controlled text. It is read via env and never interpolated into the script body — the loop-guard marker check cannot become shell injection.
• The token is scoped in the event dimension too: bound to push on main, so a pull-request run — where the diff under test can influence what the job executes — never has the credential in its environment. That is belt-and-braces over the module's own not-base-push skip.

AC4 termination is a real predicate, not luck: a run whose head commit carries the [coverage-baseline-ratchet] marker is skipped, and the value written is floor(measured) - 1pp, so an unchanged coverage proposes what is already committed and the plan comes out empty.

Files Changed

• Added: coverage-baseline-ratchet.{ts,test.ts}, the ADL (+79)
• Modified: scripts/smoke-tests/scenarios/coverage-gate.sh (+188/−0) — an existing 195-line scenario from Coverage baseline + CI guardrail (tier-aware pipeline gate) #282, extended here, not a new file (corrected: review Minor)
• Modified: .github/workflows/ci.yml (+34), knowledge-hub/package.json, and surgically: way-of-working.md, coverage-baseline.md, tier-aware-pipeline.md + coverage-config-example.md + coverage-gate.sh (both corpora)

Testing

Test Coverage

60 unit tests, one describe per contract: margin convention · config read (^baseline.<type>=<int> only) · flag read (default disabled) · skip predicate (AC1/AC4/AC5) · monotonic plan · in-place edit with surrounding markdown untouched · refusal classification (AC6) · the exact git/gh command sequence · rendered output · module constants that the CI step and the docs both depend on.

Test Results

pnpm quality-gate green — 22 + 12 turbo tasks successful.

Quality Assurance

Review Areas

• Placement in contested files. Three doc files carry live hunks from parked PRs [US-236] feat: code host separate from PM tool (WoW override) — GitHub + Linear reference case #389/[US-234] feat: PR state flow (gate≠review) + pair review as required check #390. Every edit here is a single-line replacement with no reflow or reordering. See the deferral below.
• Is floor(measured) - 1pp the right written value? It doubles as the termination guarantee, so changing the margin changes convergence.

Notes for reviewers

Two dead markdown links were introduced during implementation and removed before this PR. Both pointed at adr-018-pr-state-flow-required-checks.md, which exists only in parked PR #390. Worth flagging as a class of defect rather than an incident: pnpm quality-gate does not catch them — the broken-.md-link check is an e2e test, outside the local gate set. They would have surfaced in CI, or worse, only once #390 merged.

Deliberate deferral — the framework template does not list the new flag. dataset/.pair/adoption/tech/way-of-working.md documents Coverage guardrail: disabled (default) at line 17, and PR #390's hunk on that file is -15,6 — covering exactly that line. Adding the Coverage baseline commit-back line there would be a head-on conflict. The framework default is documented, in tier-aware-pipeline.md (both corpora), and an absent flag reads as disabled. The one-line template addition belongs to the cascade rebase after #390 merges.

Expected rebase conflicts

Declared per the story's constraint, in likelihood order:

File	Owned by	Note
.pair/adoption/tech/way-of-working.md	#389, #390	one-line append to the existing Coverage guardrail bullet
.pair/knowledge/guidelines/infrastructure/cicd-strategy/tier-aware-pipeline.md (+ dataset twin)	#390	one-line replacement inside the existing coverage-guardrail subsection

Open questions answered (review)

AC2 says the raised value is "written … and committed back" — under the bot-PR model, is it? Persistence is proposed, not applied: the ratchet opens a PR and a human merges it. That is a deliberate narrowing forced by the post-#234 regime, not an omission — a CI push to a protected base branch is rejected by the host, and writing to the PR branch would mutate the head.sha both required contexts are pinned to, invalidating an approval already given. The ADL records the reasoning. AC2's wording is stricter than what any design can deliver under branch protection; the behaviour here is the closest honest reading, and the divergence is called out rather than glossed.

Has the enabled path ever executed? No. The smoke scenario runs --dry-run plus a no-token refusal, and pair itself stays disabled, so git push + gh pr create with a real PAT has never run anywhere. Everything about the enabled path is unit-verified at the plan level (the exact argv sequence is asserted) and unexercised end-to-end. First enablement should therefore be treated as a live test, on a throwaway branch, before anyone relies on it — and it cannot happen at all until #234's protection is applied and COVERAGE_RATCHET_TOKEN is provisioned. Stated here so the merge gate does not read "80 tests" as "the feature has run".

Dependencies & Related Work

• Builds on Coverage baseline + CI guardrail (tier-aware pipeline gate) #282 (merged, PR [#282] feat: coverage baseline + CI guardrail (tier-aware pipeline gate) #368) — the guardrail, the config format and coverage-gate.sh.
• ⚠️ Designed against PR state flow (gate≠review) + pair review as required check #234 / PR [US-234] feat: PR state flow (gate≠review) + pair review as required check #390. Merging [US-234] feat: PR state flow (gate≠review) + pair review as required check #390 does not break this; it is what this design assumes. The flag stays disabled here until that protection is applied and the secret is provisioned.

Closes #372

🤖 Generated with Claude Code

[rucka]

Verdict

risk:yellow · cost:green — CHANGES-REQUESTED — excellent design and test discipline, but the enabled path has two latent defects (credential layering, concurrent clobber), an uncaught-exception path that can redden a green gate (AC6 business rule), and shipped KB docs that promise an adopter capability nothing delivers.

PR: #405 · Author: rucka · Reviewer: independent reviewer (agent) · Date: 2026-07-31 · Story: US-372 · Type: feature

Classification matrix — per dimension

Dimension	Tier	Source	Note
Service/domain criticality	yellow	KB default	no Criticality Table declared ⇒ Medium (D21)
Change/diff risk	yellow	diff footprint	CI workflow + new 735-line tool; every decision is unit-tested and the shipped gate is behaviour-frozen
Business impact	green	subdomain class	Integration & Process Standardization (supporting); no product surface
Security relevance	yellow	manual read	introduces a write credential + an automated commit path; no introduced red finding (raise-only, D17 — confirmed, not raised)
Coupling balance	green	manual read	in-repo, same team, low volatility; the workflow↔module contract is via exported constants pinned by tests

Tier = max(assessed) = yellow, unchanged from refinement (no drift note). Cost = green. Review value is a floor (D17): confirmed, never lowered.

Note — /pair-capability-assess-security, /pair-capability-assess-cost and /pair-capability-assess-coupling were not composed as separate skill runs in this independent pass; the three assessments below are a manual read of the diff against the resolved rule set, marked as such rather than reported as capability output.

Assessments

Security — Input validation

Verdict: yellow — attacker-controlled commit message is correctly env-passed, but a PR-influenced coverage number is written unvalidated into $GITHUB_ENV.

Details

• ci.yml:137 — PAIR_RATCHET_HEAD_COMMIT_MESSAGE: ${{ github.event.head_commit.message }} passed via env, never interpolated into the run: body. Correct, and the reason is stated in the YAML. Module side reads it from process.env only.
• coverage-baseline-ratchet.ts:67,88-93,210 — measured values and config values are validated with strict numeric regexes before use; a malformed value degrades to not-measured / no-baseline-configured, never an assumption.
• ci.yml:119-120 — echo "PAIR_COV_SHARED=${BRAND_PCT}" >>"$GITHUB_ENV" writes an unvalidated value into the Actions env-injection sink (finding 5). Low impact in this threat model, real as a new sink.
• CLI arg parsing rejects unknown flags rather than ignoring them (:538).

Security — Output handling

Verdict: yellow — refusal output is echoed verbatim into a ::warning:: annotation.

Details

• :645, :314-337 — classifyWriteRefusal embeds the child process's combined stdout/stderr in the annotation. The token is supplied through http.*.extraheader / GH_TOKEN rather than a URL, so git/gh should not echo it, and Actions masks registered secrets — but the pass-through is unbounded and unredacted by the code itself. Acceptable; noted, not a finding.
• No HTML/template rendering surface in the diff; PR body text is passed as an execFileSync argv element (no shell), so no injection path.

Security — Authentication

Verdict: yellow — dedicated PAT via env, but the credential layering is ambiguous (finding 1).

Details

• :551-560 — token supplied to the child process via GIT_CONFIG_COUNT/GIT_CONFIG_KEY_0 (http.https://github.com/.extraheader) + GH_TOKEN, so it never reaches argv or the command line. Good.
• actions/checkout@v3 (ci.yml:51, persist-credentials default true) already persisted a value for that same multi-valued key. See finding 1 — the resulting header ordering is not deterministic from the diff.

Security — Authorization

Verdict: green — least privilege in both the scope and the event dimension.

Details

• ci.yml:145 — COVERAGE_RATCHET_TOKEN is bound to github.event_name == 'push' && github.ref_name == 'main', so a pull_request run (where the diff under test influences what executes) never has the credential in its environment. Belt over the module's own not-base-push skip.
• Permissions requested: contents: write + pull requests: write, no administration, no branch-protection bypass; documented in the ADL, the adoption line and the KB guideline.
• No push to a protected ref by design; the ratchet lands as a reviewable PR, so the human approval gate PR state flow (gate≠review) + pair review as required check #234 introduces stays intact.

Security — Introduced vulnerabilities

Verdict: yellow — 0 introduced red, 1 introduced low/hardening, 0 pre-existing.

Details

Severity	Category	File:location	Introduced / pre-existing	Recommendation
P2	CI env injection (A03)	ci.yml:119-120	introduced	Validate ^[0-9]+(\.[0-9]+)?$ before >>$GITHUB_ENV

No committed secrets (the secret-scan gitleaks job is green on 1ac80e0d); no new dependency; no shell interpolation of untrusted text.

Cost

Verdict: cost:green — one extra ts-node invocation per CI job; no new paid resource.

Details

Signal	Class	Provider	Note
New CI step on every run	green	GitHub Actions	seconds; a no-op skip while the flag is disabled
Extra automated PR + runs	green	GitHub Actions	only when opted in; create-or-update ⇒ at most one open ratchet PR at a time
Storage / egress / LLM	green	—	none

Architecture (Coupling)

Verdict: green — balanced; the workflow↔module contract is explicit and test-pinned.

Details

• Integration strength: moderate (env-var + CLI-flag contract between ci.yml and the module). Distance: same repo, same owner. Volatility: low. Balanced ⇒ no decoupling asked for.
• Positive: the git/gh sequence is returned as data (RatchetGitPlan), so its non-negotiables are asserted by tests instead of eyeballed in YAML — exactly what ADL 2026-07-13 asks for, and the workflow step stays a genuine thin entrypoint.
• Residual duplication: the per-type mapping (shared ← brand, frontend ← website) is expressed twice in ci.yml (the pct extraction and the --measured flags). A new package means two edits. Tolerable at this size.

Details

Findings by severity

Critical (must fix before merge)

• none.

Major (should fix before merge)

• packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts:551-560 — gitAuthEnv adds a second value for http.https://github.com/.extraheader while actions/checkout@v3 (ci.yml:51, persist-credentials defaults to true) has already persisted one in .git/config carrying the read-only GITHUB_TOKEN. extraheader is multi-valued: git emits both as HTTP headers, and which Authorization the server honours is not something the diff (or git's docs) pins down — the plausible outcome is that the persisted GITHUB_TOKEN wins, the push is refused, classifyWriteRefusal reports insufficient-scope, and the ratchet warns on every base-branch push while never working. AC6's degrade-to-warning design makes that failure quiet, and neither the unit tests nor the --dry-run smoke scenario can see it. Fix: make the credential unambiguous — e.g. add git config --local --unset-all http.https://github.com/.extraheader as the first plan command (or git remote set-url / a credential helper) and pin it with a plan-level unit test, so the feature's core credential does not depend on header ordering.
• coverage-baseline-ratchet.ts:704-735 (main), :673-681 (resolveSkip), :714, :724 — every file/exec call on the happy path is unguarded, so an I/O error exits non-zero: the step fails, the build job fails, and a passing coverage gate turns the run red. That contradicts the story's business rule "the gate's verdict and the persistence are independent", AC6, and the module's own docstring ("never a non-zero exit"). Reachable cases: --way-of-working / --config missing or unreadable (adopters relocate the adoption folder — this very workflow parameterises PAIR_ADOPTION_FOLDER while these two paths are hardcoded), git rev-parse HEAD failing, writeFileSync EACCES. Note the shell guardrail it sits next to tolerates a missing config by design. Fix: wrap main() in try/catch → ::warning:: + exit 0, keeping the deliberate ::error:: + exit 1 for argument-parsing (workflow-authoring) errors only; cover it with a smoke assertion (--config /nonexistent ⇒ warning, exit 0).
• coverage-baseline-ratchet.ts:270-291 + :426-461 — the story's exceptional scenario ("two concurrent runs both raising the same baseline — the second must not clobber a higher value written by the first") is only half covered. applyRaises re-reads the config from the base-branch checkout, where a pending raise is not present, and --force-with-lease is taken after fetching the ratchet ref — so the lease is valid and the push succeeds. Concrete: run A pushes baseline.shared=89; run B (measuring 87.x) fetches A's tip, force-pushes 86, and the open ratchet PR now proposes the lower value. Self-healing on a later push, but the module docstring (:24-26, :268) and the ADL claim the guarantee unconditionally. Fix: before writing, also read the baseline from the fetched ratchet ref (git show <remote>/<RATCHET_BRANCH>:<configPath>) and require the proposal to be strictly above max(base, ratchet-branch); otherwise narrow the claim in the docstring and the ADL to "never lowers what is committed on the base branch".
• .pair/knowledge/guidelines/infrastructure/cicd-strategy/tier-aware-pipeline.md:191 + .pair/knowledge/assets/coverage-config-example.md:20 (both corpora) vs .claude/skills/pair-capability-setup-gates/SKILL.md:199 (+ dataset twin) — the shipped, adopter-facing KB now documents the flag, the push-not-PR trigger, the bot-PR landing and the token as if an adopter can enable it, but the implementation lives in packages/knowledge-hub/src/tools/ (pair-internal — the shipped assets are only coverage-gate.sh / tier-resolve.sh), and /pair-capability-setup-gates never asks about the nested flag, never records it and never emits the step. An adopter who sets Coverage baseline commit-back: enabled gets a silent no-op. Compounding it, that skill's Notes still read "automated commit-back is story Coverage baseline auto commit-back — CI persists/ratchets the baseline (opt-in, on top of #282) #372", which is stale the moment this merges. Fix (small, here): state in the guideline + example that the ratchet step is currently pair-internal and not generated by /pair-capability-setup-gates, and update the skill's stale #372 note in both corpora. Shipping the ratchet as a provider-agnostic asset + generating the step is large enough for its own story — file it if the adopter path is wanted.

Minor (consider)

• .github/workflows/ci.yml:119-120 — echo "PAIR_COV_SHARED=${BRAND_PCT}" >>"$GITHUB_ENV" writes an unvalidated value into the classic Actions environment-injection sink. pct() String()s total.lines.pct out of a file produced by the workspace under test, so a crafted multi-line value could set arbitrary env vars for later steps in the job. Impact is low here (a PR run already executes the author's own code with the same permissions, and the token is absent from PR runs), but the sink is new. Fix: [[ "$BRAND_PCT" =~ ^[0-9]+(\.[0-9]+)?$ ]] || BRAND_PCT="" before writing.
• coverage-baseline-ratchet.ts:141 — message.includes(RATCHET_BRANCH) treats any commit message that merely mentions chore/coverage-baseline-ratchet as the ratchet's own. Verified concretely: one commit in this branch contains that literal string, so squash-merging this PR with the default body would make the first post-merge run skip with automated-commit. Fail-safe in direction, but it silently swallows a legitimate raise. Fix: match the generated merge-commit shape on the subject only, e.g. /^Merge pull request #\d+ from \S+\/chore\/coverage-baseline-ratchet$/m.
• coverage-baseline-ratchet.ts:103-108 — the flag parse is an unanchored whole-document regex, first match wins. The KB sentence this PR ships contains the literal Coverage baseline commit-back: enabled; an adopter quoting it in way-of-working.md would turn the ratchet on by accident. Fix: anchor to the bullet (/^[-*]\s*\*{0,2}Coverage baseline commit-back\*{0,2}\s*:/m), and add a test with a document that quotes the phrase in prose.
• coverage-baseline-ratchet.ts:153-176 — the "nested under the guardrail" framing is documentation only: shouldSkipCommitBack never reads the parent Coverage guardrail flag and the CI step is unconditional, so Coverage guardrail: disabled + Coverage baseline commit-back: enabled would ratchet a baseline for a gate that never runs. Fix: read the parent flag too (new skip code guardrail-disabled), or say in the docs that the nesting is by pipeline placement only.
• coverage-baseline-ratchet.ts:519 — --margin is Number(v) unvalidated: --margin abc ⇒ NaN ⇒ proposed > current is always false ⇒ the ratchet silently never raises, instead of failing loudly like every other CLI-authoring error. Fix: validate Number.isFinite(n) && n >= 0 and throw into parseOrExit's ::error:: path.
• coverage-baseline-ratchet.ts:448 + :456 — git config --add remote.<remote>.fetch mutates the checkout's git config and restore (git reset --hard) does not undo it, so the plan's "left exactly as the steps after this one expect" is not literally true and repeat invocations in one checkout accumulate duplicate refspecs. Harmless on an ephemeral runner. Fix: append the unset to restore, or use a transient git -c remote.<remote>.fetch=<refspec> fetch.
• coverage-baseline-ratchet.ts:456 — restore: git reset --hard <sha> reverts all tracked-file modifications, not just the config edit; the step sits before "Run E2E tests", so any tracked file an earlier step legitimately modified would be silently reverted. Nothing does today — latent. Fix: narrow the restore to the config path (git reset --mixed <sha> + git checkout -- <configPath>).
• packages/knowledge-hub/dataset/.pair/adoption/tech/way-of-working.md:16 — the shipped template documents Coverage guardrail: disabled (default) but not the new nested flag, so the framework template no longer lists every quality knob. The PR defers this to "the cascade rebase after [US-234] feat: PR state flow (gate≠review) + pair review as required check #390 merges", which is tracked nowhere but the PR description. The edit is one clause on an existing bullet — the same shape already applied to the adoption twin, and this PR already declares expected conflicts on the sibling file. Fix: add the one-line clause here, or file a tracked follow-up issue instead of leaving it in prose.
• PR description › Files Changed — scripts/smoke-tests/scenarios/coverage-gate.sh is listed under Added; it is a modification of an existing 195-line scenario (+188/−0). The file list is what a reviewer uses to size blast radius. Fix: move it to Modified.
• PR [US-372] feat: coverage baseline auto commit-back — monotonic ratchet as a bot PR, opt-in and off by default #405 labels — no risk:yellow label, although .pair/adoption/tech/risk-matrix.md:7 declares Active: risk and the label exists in the repo; the review-time matrix confirms yellow. Fix: gh pr edit 405 --add-label risk:yellow (story Coverage baseline auto commit-back — CI persists/ratchets the baseline (opt-in, on top of #282) #372 also carries only user story).

Questions

• Story AC2 vs the ADL — AC2 says the raised value is "written … and committed back"; under the bot-PR model it lands on main only when a human merges the ratchet PR, so persistence is proposed automatically and applied manually. The ADL argues this is the only protection-compatible shape and the story's "instead of relying on someone remembering to bump the file by hand" still holds. Confirm the PO accepts that reading of AC2 — no code change implied either way.
• Validating the enabled path — git push + gh pr create with the PAT has never executed anywhere: the smoke scenario is --dry-run plus a no-token refusal, and pair stays disabled. What validates it for real (scratch repo, one-off manual run once COVERAGE_RATCHET_TOKEN exists) before the first adopter turns it on? Findings 1 and 3 are precisely the class a dry-run cannot catch.
• coverage-baseline-ratchet.ts:443-445 — on a push event actions/checkout leaves a local main branch checked out, so git commit advances the local main pointer before restore rewinds it. Harmless, but "no branch is created or switched to" / "never touches the base branch" is true of the remote only. Worth one clarifying word in the docstring/ADL.

Positive feedback

• T-1 is the story's real risk and it was taken seriously. Deciding AC5 against the post-PR state flow (gate≠review) + pair review as required check #234 regime rather than today's unprotected main — and reaching "neither PR nor push: a bot PR" — is the right answer for the right reason, with the rejected options and why each fails recorded. The ADL matches adl-template.md section for section, and every relative link in it resolves.
• The git plan as data (RatchetGitPlan) is the standout design choice: one push, ratchet ref only, no git add -A, no branch switch, absolute-SHA restore — all asserted by tests instead of reviewed by eye in YAML. Exactly ADL 2026-07-13's intent, and it makes the review of a dangerous command sequence tractable.
• The --force-with-lease remote-tracking-ref insight (:392-405) is the kind of empirically-earned detail that would otherwise have shipped as "works once, then warns forever".
• AC4 is genuinely belt and braces: a marker predicate plus a floor(measured) - 1pp fixpoint, so termination survives the marker being lost — and the smoke scenario runs the sequence (marked commit, merge commit, fixpoint) instead of asserting it in prose.
• 60 focused tests, one describe per contract, including CRLF, baseline.sharedish near-miss keys, commented-out lines, and byte-level "surrounding markdown untouched" (line count + exactly one differing line).
• Discipline in contested files: single-line replacements only; the dataset twins of coverage-gate.sh and coverage-config-example.md are byte-identical, and the new tier-aware-pipeline.md sentence is identical in both corpora (only the pre-existing skill-name transform differs).
• Flagging the two dead links removed during implementation as a class of defect (broken-.md-link checking is e2e, outside the local gate) is exactly the kind of note that makes a PR description worth reading.

Functionality & requirements (AC coverage)

AC	Verdict	Evidence
AC1 — default off, unchanged	met	shouldSkipCommitBack short-circuits on absent/disabled (:154); tested + smoke-demonstrated; adoption line and framework default both disabled
AC2 — raise ⇒ commit back	met (as decided)	planRatchet raise + applyRaises + marked commit; lands as a bot PR rather than a base push — see the Questions item
AC3 — never lowers	met	strict proposed > current only (:233); equal/drop/unknown/malformed all hold; tested
AC4 — no CI loop	met	marker predicate and fixpoint, run as a sequence in the smoke scenario (DoD "demonstrated, not asserted" satisfied); see the branch-substring Minor
AC5 — PR vs push decided	met	not-base-push skip + ADL 2026-07-30-coverage-ratchet-pr-not-push.md (DoD "recorded in an ADL" satisfied)
AC6 — refusal degrades	partial	refusals classified + ::warning:: + exit 0 on the write path, but unguarded I/O in main can exit non-zero and redden a green gate (Major 2)
Edge — concurrent raise	partial	on-disk re-check covers a value already on the base branch, not a higher pending proposal on the ratchet branch (Major 3)
Edge — malformed/absent report	met	not-measured / no-baseline-configured; the shell gate's fail-safe is untouched

Testing & quality gates

• Quality gates: PASS — CI run 30609629002 on head 1ac80e0d is green (build 4m19s incl. ts:check, build, lint, hygiene, docs:staleness, skills:conformance, dup:check, tests, coverage, the coverage guardrail, the new ratchet step and E2E; secret-scan green). Meets the 🟡 tier check set (lint + type + build + unit).
• 60 unit tests over the module's decision surface; edge and error scenarios well covered (malformed config, missing measurement, unknown type, CRLF, concurrency-at-the-data-level, every refusal code).
• Gaps: no test for the CLI's failure modes (missing config/way-of-working path — Major 2), and the real write path (git push / gh pr create) is exercised nowhere, by construction (--dry-run + a no-token refusal).
• Note: the smoke suite is not run by ci.yml (release-only / local pnpm smoke-tests), so the AC1/AC3/AC4/AC5/AC6 CLI demonstration is not a merge gate. Pre-existing convention, not this PR's doing — but it does mean the ratchet step in ci.yml is itself the only per-PR check that the pnpm --filter … coverage:ratchet wiring still parses (it did, here).

Adoption compliance

• Degradation level: 2 — /pair-capability-verify-adoption not composed in this independent pass; checks done inline.
• No new dependency (ts-node and vitest already adopted) ⇒ no tech-stack.md change needed; /pair-capability-assess-stack not required.
• Gate/tooling ADL 2026-07-13 respected: logic in a tested module in its owning package, workflow step thin, CLI covered by a smoke scenario rather than a unit test.
• Conformance-test convention (ADL 2026-07-18) — no existing conformance file targets tier-aware-pipeline.md, so nothing to extend; the doc audits live in the smoke scenario, consistent with Coverage baseline + CI guardrail (tier-aware pipeline gate) #282.
• Decisions recorded: the ADL exists, is linked from the adoption line, coverage-baseline.md and the module header. No missing-ADR HALT.
• Non-conformity: the shipped KB documents a capability /pair-capability-setup-gates cannot generate, and that skill's own note is stale (Major 4).

Tech debt

• The adopter delivery path for the ratchet (a provider-agnostic asset + setup-gates generation) does not exist — currently pair-internal. Worth its own story if adopters are meant to opt in (Major 4 asks only for the honest caveat + the stale-note fix here).
• dataset/.pair/adoption/tech/way-of-working.md lacks the new flag line (deferred in prose only — Minor).
• The per-type mapping is duplicated inside ci.yml (pct extraction + --measured).
• restore semantics wider than the change it undoes; remote.*.fetch mutation not restored.
• None of these justify blocking on debt grounds.

Documentation

• Adoption line (way-of-working.md) declares the flag, the default, the credential and the ADL pointer — DoD "documented where a consumer will read it" satisfied on the adoption side.
• tech/coverage-baseline.md explains what writes to the file and when.
• KB guideline + shipped config example updated in both corpora, byte-identical for the new text.
• ADL complete (context, decision, 6 rejected alternatives with reasons, consequences, adoption impact).
• /pair-capability-setup-gates note stale ("automated commit-back is story Coverage baseline auto commit-back — CI persists/ratchets the baseline (opt-in, on top of #282) #372") and silent on the new flag (Major 4).
• Framework template (dataset/.pair/adoption/tech/way-of-working.md) does not list the flag (Minor).

Performance & deployment

• No hot path touched; one extra ts-node start per CI job, a no-op skip while the flag is disabled.
• Rollback is a one-word flag edit; the feature ships off in both the framework default and pair's own adoption, and a refused write degrades to a warning by design (subject to Major 2).

[rucka]

Remediation report — PR #405 · story #372

Closes the loop on the review above. 16 findings (0 Critical · 4 Major · 10 Minor · 2 Questions) → 0 open actionable findings, with two deliberate deferrals named below.

Head at verification: 8fec831c · pnpm quality-gate green · ratchet suite 80 tests (was 70).

Fixes landed in three commits: d6be623d (code), 542ef6ee (adopter-scope docs), 8fec831c (env-injection guard). Part of the code work was recovered from the worktree of an implementer the watchdog killed mid-round — verified green, completed and refactored rather than redone.

Major — all 4 resolved

#	Finding	Outcome
M1	gitAuthEnv added a second value for http.https://github.com/.extraheader, which actions/checkout already populates with the read-only GITHUB_TOKEN. extraHeader is multi-valued, so git sent two Authorization headers and the server chose. The likely loser was ours — a refusal that AC6 turns into a warning on every base-branch push: a feature that never works and never says so.	Fixed. gitAuthConfig + EXTRAHEADER_CONFIG_KEY override the single key instead of layering onto it. The reasoning is in the constant's docblock so the next reader does not re-introduce the layering.
M2	Every file/exec call on the enabled happy path was unguarded, so an I/O error exited the step non-zero — contradicting AC6's own business rule that persistence can never change the gate's verdict.	Fixed. main funnels everything into the warning + exit 0 path, with one deliberate exception: an argument-parsing failure, which is a workflow-authoring error and must be loud rather than silently disabling the feature.
M3	The story's exceptional scenario — two concurrent runs both raising the same baseline, the second must not clobber a higher value — was not implemented.	Fixed. applyRaises re-checks every proposal against both the text on disk (the base-branch checkout) and the config as committed on the open ratchet branch, so a higher value is never clobbered on either ref.
M4	The shipped, adopter-facing KB documented the flag, the trigger, the bot-PR landing and the credential as if an adopter could enable it. They cannot: the logic is pair-internal, and /pair-capability-setup-gates never asks about the flag nor emits the step. An adopter setting enabled got a silent no-op — the worst shape for an opt-in.	Fixed as honesty, tracked as capability. The guideline and the config example now state the pair-internal scope in both corpora. The capability gap is #409, filed and assigned: ship the ratchet as a provider-agnostic asset (or define the extension point — an ADR-worthy call) and teach setup-gates the flag.

Minor — 9 resolved, 1 deferred

#	Finding	Outcome
1	echo "PAIR_COV_SHARED=${BRAND_PCT}" >>"$GITHUB_ENV" wrote an unvalidated value into the classic Actions environment-injection sink	Fixed (8fec831c). Values are validated numeric before the write; a non-numeric one is dropped with a ::warning::, so the ratchet sees no measurement and proposes nothing — the conservative outcome. Verified: 20\nEVIL=1 is rejected while 85.04 passes; sh -n clean.
2	message.includes(RATCHET_BRANCH) treated any commit message that merely mentions the ratchet branch as the ratchet's own — so a human PR whose title quoted it would silently disable the ratchet	Fixed — exact marker plus an anchored merge-subject pattern.
3	The flag parse was an unanchored whole-document regex, first match wins — and the KB sentence this PR ships contains the literal flag string, so an adopter quoting that sentence in their own way-of-working would have turned the flag on	Fixed — anchored to a list bullet. The reasoning is in flagPattern's docblock; this one was a genuine trap.
4	"Nested under the guardrail" was documentation only: shouldSkipCommitBack never read the parent flag	Fixed — readGuardrailFlag + a guardrail-disabled skip code. Ratcheting a floor no gate checks is now impossible, not merely discouraged.
5	--margin was Number(v) unvalidated: --margin abc ⇒ NaN ⇒ every proposed > current false ⇒ the ratchet silently never raises	Fixed — validated; a bad value fails loudly instead of disabling the feature quietly.
6	git config --add remote.<remote>.fetch mutated the checkout's persisted config and restore did not undo it, so the plan's "leaves no trace" was false	Fixed — a transient -c refspec, set not appended, so nothing is persisted to undo.
7	restore: git reset --hard <sha> reverted all tracked modifications, and the step sits before "Run E2E tests"	Fixed — --mixed plus an explicit checkout of the one edited path: the ratchet reverts what the ratchet did, nothing else in the workspace.
8	PR description listed the smoke scenario under Added; it is a modification of an existing 195-line scenario	Fixed in the description.
9	No risk:yellow label on the PR although the adoption declares Active: risk	Fixed — risk:yellow on #405, and on story #372, which declared the tier in its body with no label.
10	The framework template dataset/.pair/adoption/tech/way-of-working.md does not list the new nested flag	Deferred, deliberately. PR #390's hunk on that file is -15,6, covering exactly the line where it would go. The framework default is documented in tier-aware-pipeline.md (both corpora) and an absent flag reads disabled, so nothing is undocumented — only unlisted. The one-line addition belongs to the cascade rebase after #390.

Questions — both answered in the PR body

• AC2 vs the ADL. Persistence is proposed, not applied: a human merges the ratchet PR. AC2's wording is stricter than any design can deliver under branch protection — a CI push to a protected base is rejected, and writing to the PR branch mutates the head.sha both required contexts are pinned to. The divergence is now stated rather than glossed.
• Has the enabled path ever run? No. The smoke scenario is --dry-run plus a no-token refusal, and pair stays disabled, so git push + gh pr create with a real PAT has executed nowhere. The plan-level argv sequence is asserted; the end-to-end path is unexercised. First enablement should be treated as a live test on a throwaway branch. Recorded so nobody reads "80 tests" as "the feature has run".

One thing the tests caught that the review had not

ratchetGitPlan grew to 64 lines against a 50-line ceiling while these fixes went in. vitest was green and lint was red — caught by the pre-push gate, not by the test run. Split via ratchetWriteCommands. Same trap a sibling PR fell into earlier today; worth naming because "tests pass" and "the gate passes" are not the same statement.

Verdict requested: da CHANGES-REQUESTED ad APPROVED, with the two named deferrals (#409 for the adopter path, the template line for the post-#390 rebase) carried to the merge gate as accepted, not hidden.