From 3bafc56fdc4d90a1497f1fbc2c49ee24a11bfd7c Mon Sep 17 00:00:00 2001 From: Gianluca Carucci Date: Thu, 30 Jul 2026 23:27:23 +0200 Subject: [PATCH 1/6] =?UTF-8?q?[US-372]=20docs:=20ADL=20=E2=80=94=20ratche?= =?UTF-8?q?t=20lands=20as=20a=20bot=20PR,=20never=20a=20base=20push?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decide AC5 (PR vs push) and the credential model against the POST-#234 regime, not today's unprotected main: - a pull_request run NEVER writes (uniform for forks; no head-SHA mutation, so a human approval is never invalidated) - the post-merge push proposes the raise as a bot PR from chore/coverage-baseline-ratchet — no branch-protection bypass is requested, which is the point of #234 - credential: repo-scoped COVERAGE_RATCHET_TOKEN (contents:write + pull-requests:write, no admin, no bypass list); GITHUB_TOKEN rejected because a PR it opens triggers no run and can never satisfy the required contexts - loop termination: marker predicate PLUS a floor(measured)-1pp fixpoint; [skip ci] rejected (it would skip the checks the ratchet PR must pass) - rejected options recorded with the reason each fails - Task: T-1 — Decide and record PR-vs-push behaviour + credential model Refs: #372 --- ...2026-07-30-coverage-ratchet-pr-not-push.md | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 .pair/adoption/decision-log/2026-07-30-coverage-ratchet-pr-not-push.md diff --git a/.pair/adoption/decision-log/2026-07-30-coverage-ratchet-pr-not-push.md b/.pair/adoption/decision-log/2026-07-30-coverage-ratchet-pr-not-push.md new file mode 100644 index 00000000..20249e46 --- /dev/null +++ b/.pair/adoption/decision-log/2026-07-30-coverage-ratchet-pr-not-push.md @@ -0,0 +1,79 @@ +# Decision: Coverage baseline commit-back writes a bot PR from the post-merge run — never a push to the protected base, never the PR branch + +## Date + +2026-07-30 + +## Status + +Active + +## Category + +Process Decision + +## Context + +Story #282 shipped the coverage guardrail with an explicitly **advisory** persistence model: `coverage-gate.sh` prints a suggested `baseline.=NN` to stderr and a human commits it. Story #372 adds the opt-in automation, and its AC5/AC6 force two decisions that cannot be left implicit: + +1. **Does a pull-request run write back, or only the post-merge run on the base branch?** +2. **With what credential?** + +Both must be decided against the **post-#234 regime**, not against today's unprotected `main`. Story #234 (PR #390, parked open) makes `pair-review` and `pair-explicit-approval` **required status checks** on `main` and sets **`enforce_admins: true`** ([adr-018](../tech/adr/adr-018-pr-state-flow-required-checks.md)). Under that regime: + +- **A direct push to `main` is structurally impossible for CI.** Required status checks are evaluated on pushes to a protected branch, not only on merges; `enforce_admins: true` removes the admin escape hatch. A commit pushed by a workflow will never carry a `pair-explicit-approval` context — that context is produced by a *human* approving a PR (adr-018 pins it to `pull_request.head.sha`). So the "simplest" option in the story body — defer to the post-merge run and push — is not merely awkward after #390, it is **rejected by the host**. +- **Writing to the PR branch mutates `head.sha`**, which is exactly the commit both required contexts are pinned to. A raise would therefore invalidate a human approval that had already been given, forcing a re-approval on every coverage improvement — the automation would consume the very control #234 introduces. +- **A fork PR has no write access at all** (`GITHUB_TOKEN` is read-only on `pull_request` from a fork), so a PR-branch write is not uniformly available anyway. +- **`GITHUB_TOKEN` cannot open a PR that CI will run.** Events triggered by `GITHUB_TOKEN` do not start new workflow runs, so a PR opened with it would sit forever with `CI` and `pair-review` **pending** — un-mergeable under the same required-check regime. + +## Decision + +**AC5 — PR vs. push: neither. The write-back happens only on the post-merge `push` run on the base branch, and it lands as a bot-authored pull request, not as a push to the base branch.** + +- A `pull_request` run **never** writes. The skip predicate refuses any event that is not a `push` whose ref is the configured base branch (skip code `not-base-push`). This is uniform for fork and same-repo PRs — no fork special case, no head-SHA mutation, no invalidated approval. +- The post-merge run computes the raise, commits it **on the checked-out base commit without creating or switching to any local branch**, pushes that commit by explicit refspec to a dedicated remote branch `chore/coverage-baseline-ratchet` (`HEAD:refs/heads/chore/coverage-baseline-ratchet`, `--force-with-lease`), creates-or-updates **one** PR from it, and then resets the checkout back to the SHA it was given — the later steps of the same job must find the workspace they were handed, not a bot branch. That PR goes through the *same* protection the regime requires: CI runs, `pair-review` reports, a human approves. Nothing bypasses branch protection and no exemption/bypass allowance is requested — which is the point: #234 exists to make `main` unwritable except through review, and an automated coverage bump is not a reason to punch a hole in it. +- **The lease is made to work, not assumed.** `--force-with-lease` needs a remote-tracking ref for the destination, and a CI checkout has none for the ratchet branch (it fetches the base ref only, and the clone's fetch refspec does not map anything else). Verified empirically: without one, git rejects every non-fast-forward lease push as `stale info` — the ratchet would land its first raise and then warn forever. So the plan adds a fetch refspec for the ratchet branch and fetches it (tolerating the failure on the first run, when the branch does not exist yet) *before* pushing. A bare `--force` was rejected: the branch is bot-owned, but a force push should still be the kind git can refuse. Note what does **not** rest on the lease — never clobbering a higher baseline is a data-level guarantee (`applyRaises` re-reads the config and re-checks each proposal), not a push-level one. +- **Automated, not manual, in the sense the story asks for**: nobody has to *remember* to bump the file or *measure* anything — the raise arrives as a reviewable PR. What stays human is the one irreducible act the protection regime mandates: approving a change to `main`. + +**Credential model — a dedicated, repo-scoped, non-bypassing token, and its absence is a warning.** + +- The step reads `COVERAGE_RATCHET_TOKEN`: a **fine-grained PAT (or GitHub App installation token) scoped to this repository only**, with exactly two permissions — `contents: write` (to push the `chore/coverage-baseline-ratchet` branch) and `pull requests: write` (to open/update the ratchet PR). **No** `administration` scope, and the token holder is **not** added to any branch-protection bypass list. +- **Least privilege in the event dimension as well**: the workflow binds the secret to `push` on the base branch, so a `pull_request` run — the one whose diff can influence what the job executes — never has the token in its environment at all. That is belt; the module's `not-base-push` skip is braces. +- `GITHUB_TOKEN` is deliberately *not* used, for a functional reason rather than a scope one: a PR it opens gets no workflow run, so it can never satisfy the required contexts. +- The workflow's default token permissions are unchanged (the repo does not grant job-wide write); the ratchet's credential enters through a single `env:` on a single step. +- **Missing/insufficient credential is a refusal, not a failure** (AC6): the step emits `::warning::` naming the reason and exits 0. It runs *after* the guardrail step and never touches the gate's exit code, so a persistence failure can neither redden a green gate nor green a red one. + +**Loop termination (AC4) is a predicate plus a fixpoint, not a `[skip ci]`.** + +- The skip predicate refuses a run whose head commit message carries the marker `[coverage-baseline-ratchet]` (skip code `automated-commit`). The marker is placed in the **PR title** as well as the commit subject, so it survives a squash merge (whose subject defaults to the PR title); the ratchet **branch name** is accepted as a second marker so a plain merge commit (`Merge pull request #N from chore/coverage-baseline-ratchet`) is also caught. +- Independently, the ratchet is a **fixpoint**: the value it writes is `floor(measured) - 1pp`, so re-running on the same coverage proposes the value already committed and produces no raise. Termination therefore does not depend on the marker surviving — that is belt, this is braces. `[skip ci]` was rejected as the mechanism: it suppresses the whole pipeline, including the very gates the ratchet PR must satisfy under #234. + +**Ratchet semantics.** Monotonic — the writer only ever raises a `baseline.` value in place, never lowers or reorders, and never rewrites the surrounding markdown. The margin (`floor(measured) - 1`) is not a new convention: it is the one `tech/coverage-baseline.md` already documents and reproduces exactly (85.04 → 84, 20.16 → 19). Concurrency: the config is re-read from disk immediately before the write and each proposal re-checked for a strict raise, so a higher value written by another run is never clobbered. + +**Framework default stays `disabled`**, and **pair itself stays `disabled`** for now: enabling it before #390 lands and before `COVERAGE_RATCHET_TOKEN` is provisioned would only produce a warning on every push. The flag is a one-word edit in `way-of-working.md` when both are true. + +## Alternatives Considered + +- **Post-merge direct push to the base branch (the story body's "simplest, protection-compatible" option)**: Rejected. It is *not* protection-compatible after #234 — required status checks apply to direct pushes and `enforce_admins: true` leaves no bypass; the pushed commit can never carry `pair-explicit-approval`. Making it work requires a bypass allowance for the ratchet identity, i.e. a permanent write path into `main` that skips review — precisely the hole #234 closes. +- **Write to the PR branch during the PR run**: Rejected on three counts: no write access on fork PRs; mutating `head.sha` invalidates the `pair-review`/`pair-explicit-approval` contexts pinned to it, so every coverage improvement would silently revoke a human approval; and two concurrent PRs both raising the same key conflict in the config file. +- **Ratchet PR opened with the default `GITHUB_TOKEN`**: Rejected. Events from `GITHUB_TOKEN` do not trigger workflows, so the PR would never obtain the required `CI`/`pair-review` contexts and would be un-mergeable — an automation that reliably produces a stuck PR. +- **`[skip ci]` in the automated commit as the loop guard**: Rejected. It skips the whole pipeline, so the ratchet PR would never get the checks #234 requires; and it guards the wrong thing (the coverage job is what must be skipped, not the build). +- **Keep #282's advisory model and do nothing** (`disabled` forever): Rejected as the *shipped capability* — it is, however, exactly what the default and pair's own current flag state give, so nothing regresses for adopters who never opt in. +- **Persist the baseline from inside `coverage-gate.sh`**: Rejected, unchanged from #282. The gate decides pass/fail and must stay a pure reader; persistence is a separable side effect that is allowed to fail alone (and per the gate-tooling ADL its logic belongs in a tested module, not in the shell asset). + +## Consequences + +- `coverage-gate.sh` is untouched in behaviour: it still never persists. Only its PERSISTENCE comment is updated to point at the new, decided model instead of "tracked separately". +- The ratchet logic lives in `packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts` (unit-tested, white-box) per [2026-07-13-gate-tooling-code-in-tested-modules.md](./2026-07-13-gate-tooling-code-in-tested-modules.md); the CI step and the package's `coverage:ratchet` script are thin entrypoints, and the CLI is verified end-to-end by the `coverage-gate.sh` smoke scenario (dry-run), never by a unit test. The git/gh command sequence is data (a plan the module returns), so its non-negotiable properties — one push, to the ratchet ref only, leaseable, no `git add -A`, no branch switch, always restored — are asserted by tests rather than reviewed by eye in YAML. +- Adopters who enable the flag must provision `COVERAGE_RATCHET_TOKEN`; without it the step warns on every base-branch push. Documented on the adoption line and in the KB guideline, not only in the workflow YAML. +- A merged ratchet PR produces a base-branch push whose subject carries the marker, so the next run skips — and even without the marker the fixpoint yields no raise. Both are demonstrated in the smoke scenario. +- One extra open PR may exist at a time (`chore/coverage-baseline-ratchet` is create-or-update, never a second PR per raise). + +## Adoption Impact + +- [way-of-working.md](../tech/way-of-working.md): the existing **Coverage guardrail** bullet under Quality Gates gains the `Coverage baseline commit-back: disabled` sub-flag, its default, the token requirement and a pointer here. Edited surgically (that bullet only) because the file is contended by parked PRs #389/#390. +- [tech/coverage-baseline.md](../tech/coverage-baseline.md): the "Human-committed" note is amended to describe the opt-in ratchet and the margin rule the writer reproduces. +- [tier-aware-pipeline.md](../../knowledge/guidelines/infrastructure/cicd-strategy/tier-aware-pipeline.md) (+ dataset twin): the coverage-guardrail subsection's commit-back sentence replaces "tracked separately — see story #372" with the decided model, the flag, the default and the credential requirement. Surgical for the same reason (contended by #390). +- [coverage-config-example.md](../../knowledge/assets/coverage-config-example.md) (+ dataset twin): the "tracked separately — see story #372" sentence is replaced by the decided model, so the shipped config documentation no longer points at an open question. +- **Deliberately deferred**: the sub-flag is NOT added to the shipped `dataset/.pair/adoption/tech/way-of-working.md` template, because that file is contended by two parked PRs (#389, #390) and a template line is not worth a merge conflict. Nothing depends on it: an **absent** flag reads as off (`readCommitBackFlag` → `absent` → `flag-disabled`, unit-tested), which is exactly the framework default, and the KB guideline already documents the flag, its default and its credential where an adopter reads them. Adding the template line is a one-line follow-up once those PRs land. +- No dataset mirror for this ADL: `adoption/decision-log/` is an adoption-only record (same rationale as the sibling ADLs). From 00c12b54f6ffbb5fd74ffe723bdd3ef39334d987 Mon Sep 17 00:00:00 2001 From: Gianluca Carucci Date: Thu, 30 Jul 2026 23:27:42 +0200 Subject: [PATCH 2/6] [US-372] feat: monotonic coverage-baseline ratchet + opt-in CI wiring Gate logic in a tested module (ADL 2026-07-13); ci.yml stays a thin entrypoint that only forwards the measured numbers and exits 0. - ratchet writer: raises `^baseline.=` values IN PLACE, never rewrites the surrounding markdown; only a value strictly above the committed one is a raise (equal/drop/unknown-type/malformed all hold) - concurrency: the config is re-read immediately before writing and every proposal re-checked, so a higher value from a concurrent run is dropped, never clobbered - skip predicate: flag-disabled (default) / not-base-push (every PR run) / automated-commit (marker or ratchet branch in the head commit) - git plan is data, so its non-negotiables are asserted by tests: one push, to the ratchet ref only, no `git add -A`, no branch switch, always restored to the checked-out SHA - push uses --force-with-lease AND first maps+fetches a remote-tracking ref for the ratchet branch: without it git rejects every later non-fast-forward push as `stale info` (verified), so the ratchet would have worked exactly once and then warned forever - refused write => named ::warning:: and exit 0; the guardrail's verdict is untouched (step runs after it) - token bound to the base-branch push event in ci.yml, so a PR run never has the credential in its environment - smoke scenario DEMONSTRATES loop termination as a real CLI sequence (marked commit, merge commit, and the fixpoint that needs no marker) - Tasks: T-2 ratchet writer, T-3 flag + CI wiring, T-4 loop termination, T-5 degradation on refused write Refs: #372 --- .github/workflows/ci.yml | 34 + packages/knowledge-hub/package.json | 3 +- .../tools/coverage-baseline-ratchet.test.ts | 504 ++++++++++++ .../src/tools/coverage-baseline-ratchet.ts | 735 ++++++++++++++++++ .../smoke-tests/scenarios/coverage-gate.sh | 188 +++++ 5 files changed, 1463 insertions(+), 1 deletion(-) create mode 100644 packages/knowledge-hub/src/tools/coverage-baseline-ratchet.test.ts create mode 100644 packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7ea4334b..58c49931 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -114,7 +114,41 @@ jobs: BRAND_PCT="$(pct packages/brand/coverage/coverage-summary.json || true)" WEBSITE_PCT="$(pct apps/website/coverage/coverage-summary.json || true)" echo "coverage: brand(shared)=${BRAND_PCT}% website(frontend)=${WEBSITE_PCT}%" + # Hand the measured numbers to the commit-back step (#372) so the + # extraction is not duplicated. Values only — no decision is made here. + echo "PAIR_COV_SHARED=${BRAND_PCT}" >>"$GITHUB_ENV" + echo "PAIR_COV_FRONTEND=${WEBSITE_PCT}" >>"$GITHUB_ENV" coverage_gate "$TIER" shared "$BRAND_PCT" "$CFG" coverage_gate "$TIER" frontend "$WEBSITE_PCT" "$CFG" + # Coverage baseline commit-back (story #372) — OPT-IN and nested under the + # guardrail above: `Coverage baseline commit-back` in way-of-working.md is + # `disabled` here (and is the framework default), so today this step is a + # no-op that prints WHY it skipped and writes nothing (AC1). + # Thin entrypoint (ADL 2026-07-13): every decision — the skip predicate + # (flag / base-branch push only / ratchet-marker loop guard), the monotonic + # ratchet, the git+gh command plan and the refusal classification — lives in + # the unit-tested module packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts; + # the CLI wiring is demonstrated by the coverage-gate smoke scenario. + # Placed AFTER the guardrail and always exiting 0: a refused write degrades + # to a ::warning:: and can never change the gate's verdict (AC6). + - name: Coverage baseline commit-back (ratchet — opt-in #372) + env: + # Attacker-controlled text: read via env, never interpolated into the script. + PAIR_RATCHET_HEAD_COMMIT_MESSAGE: ${{ github.event.head_commit.message }} + PAIR_RATCHET_BASE_BRANCH: main + # Dedicated, repo-scoped credential (contents:write + pull-requests:write, + # no protection bypass). Absent => the step warns, the gate is unaffected. + # Least privilege in the EVENT dimension too: bound to the only event + # allowed to write, so a pull-request run — where the diff under test can + # influence what this job executes — never has the token in its + # environment. Belt to the module's own `not-base-push` skip (braces). + COVERAGE_RATCHET_TOKEN: ${{ (github.event_name == 'push' && github.ref_name == 'main') && secrets.COVERAGE_RATCHET_TOKEN || '' }} + run: | + set -euo pipefail + pnpm --filter @pair/knowledge-hub coverage:ratchet \ + --config .pair/adoption/tech/coverage-baseline.md \ + --way-of-working .pair/adoption/tech/way-of-working.md \ + --measured "shared=${PAIR_COV_SHARED:-}" \ + --measured "frontend=${PAIR_COV_FRONTEND:-}" - name: Run E2E tests run: pnpm --filter @pair/website e2e diff --git a/packages/knowledge-hub/package.json b/packages/knowledge-hub/package.json index 11747682..ec9cd4dd 100644 --- a/packages/knowledge-hub/package.json +++ b/packages/knowledge-hub/package.json @@ -19,7 +19,8 @@ "mdlint:fix": "markdownlint-fix", "check:links": "ts-node src/tools/check-broken-links.ts", "transfer:dataset": "ts-node src/tools/transfer-dataset.ts", - "skills:conformance": "ts-node src/tools/skills-conformance-check.ts" + "skills:conformance": "ts-node src/tools/skills-conformance-check.ts", + "coverage:ratchet": "ts-node src/tools/coverage-baseline-ratchet.ts" }, "devDependencies": { "@pair/eslint-config": "workspace:*", diff --git a/packages/knowledge-hub/src/tools/coverage-baseline-ratchet.test.ts b/packages/knowledge-hub/src/tools/coverage-baseline-ratchet.test.ts new file mode 100644 index 00000000..df3767dc --- /dev/null +++ b/packages/knowledge-hub/src/tools/coverage-baseline-ratchet.test.ts @@ -0,0 +1,504 @@ +import { describe, it, expect } from 'vitest' +import { + RATCHET_MARKER, + RATCHET_BRANCH, + DEFAULT_MARGIN_PP, + TOKEN_ENV, + proposeBaseline, + readBaselineValue, + readCommitBackFlag, + shouldSkipCommitBack, + planRatchet, + applyRaises, + classifyWriteRefusal, + ratchetGitPlan, + renderRatchetPlan, +} from './coverage-baseline-ratchet' + +// A config fixture shaped like the real adoption file: the parseable `key=value` +// block lives inside a fenced code block, wrapped in markdown that MUST survive +// a write untouched (business rule: "edits values in place, never rewrites the +// surrounding markdown"). +const CONFIG = `# \`tech/coverage-baseline.md\` — config + +Some prose that must not move. + +## Config + +\`\`\`ini +# comment line +target.default=70 +target.shared=90 +target.frontend=25 + +baseline.shared=84 +baseline.frontend=19 +\`\`\` + +## Notes + +- trailing prose +` + +const measuredPush = { + eventName: 'push', + refName: 'main', + baseBranch: 'main', + headCommitMessage: 'feat: something unrelated', +} + +describe('proposeBaseline — the margin convention the adoption file already documents', () => { + it('reproduces the committed baselines from the measured values (85.04 -> 84, 20.16 -> 19)', () => { + expect(proposeBaseline(85.04)).toBe(84) + expect(proposeBaseline(20.16)).toBe(19) + }) + + it('is floor(measured) - margin', () => { + expect(proposeBaseline(90)).toBe(89) + expect(proposeBaseline(90.99)).toBe(89) + expect(proposeBaseline(100)).toBe(99) + expect(proposeBaseline(90, 0)).toBe(90) + expect(proposeBaseline(90, 5)).toBe(85) + }) + + it('never goes negative', () => { + expect(proposeBaseline(0)).toBe(0) + expect(proposeBaseline(0.5)).toBe(0) + }) + + it('is a fixpoint — proposing from a value already at the proposal yields no increase', () => { + // measured 85.04 => 84; a later run measuring the same never proposes 85. + const first = proposeBaseline(85.04) + expect(proposeBaseline(85.04)).toBe(first) + }) +}) + +describe('readBaselineValue — `^baseline.=` only, format unchanged', () => { + it('reads a committed integer baseline', () => { + expect(readBaselineValue(CONFIG, 'shared')).toBe(84) + expect(readBaselineValue(CONFIG, 'frontend')).toBe(19) + }) + + it('returns null for a type absent from the config', () => { + expect(readBaselineValue(CONFIG, 'backend')).toBeNull() + }) + + it('returns null for a malformed value', () => { + expect(readBaselineValue('baseline.shared=not-a-number\n', 'shared')).toBeNull() + }) + + it('tolerates a CRLF-authored config', () => { + expect(readBaselineValue('baseline.shared=84\r\n', 'shared')).toBe(84) + }) + + it('does not match a key that merely contains the type name', () => { + expect(readBaselineValue('baseline.sharedish=99\nbaseline.shared=84\n', 'shared')).toBe(84) + }) + + it('ignores a commented-out baseline line (only `^baseline.` matches)', () => { + expect(readBaselineValue('# baseline.shared=99\nbaseline.shared=84\n', 'shared')).toBe(84) + }) +}) + +describe('readCommitBackFlag — the opt-in, default disabled', () => { + it('reads `enabled` from the way-of-working bullet', () => { + expect(readCommitBackFlag('- **Coverage baseline commit-back**: `enabled` — foo')).toBe( + 'enabled', + ) + }) + + it('reads `disabled`', () => { + expect(readCommitBackFlag('- **Coverage baseline commit-back**: `disabled` — foo')).toBe( + 'disabled', + ) + }) + + it('reports `absent` when the flag is not declared at all (framework default = off)', () => { + expect(readCommitBackFlag('- **Coverage guardrail**: `enabled`')).toBe('absent') + expect(readCommitBackFlag('')).toBe('absent') + }) + + it('is case-insensitive and tolerates missing backticks/bold', () => { + expect(readCommitBackFlag('Coverage Baseline Commit-Back: Enabled')).toBe('enabled') + }) +}) + +describe('shouldSkipCommitBack — AC1 (default off), AC4 (loop termination), AC5 (PR vs push)', () => { + it('AC1: skips when the flag is absent — the framework default writes nothing', () => { + const d = shouldSkipCommitBack({ ...measuredPush, commitBackFlag: 'absent' }) + expect(d.skip).toBe(true) + expect(d.code).toBe('flag-disabled') + }) + + it('AC1: skips when the flag is explicitly disabled', () => { + expect(shouldSkipCommitBack({ ...measuredPush, commitBackFlag: 'disabled' }).code).toBe( + 'flag-disabled', + ) + }) + + it('AC5: skips a pull_request run — a PR never writes back (fork or not)', () => { + const d = shouldSkipCommitBack({ + ...measuredPush, + commitBackFlag: 'enabled', + eventName: 'pull_request', + refName: 'feature/US-1-foo', + }) + expect(d.skip).toBe(true) + expect(d.code).toBe('not-base-push') + }) + + it('AC5: skips a push to a non-base branch', () => { + const d = shouldSkipCommitBack({ + ...measuredPush, + commitBackFlag: 'enabled', + refName: 'feature/US-1-foo', + }) + expect(d.code).toBe('not-base-push') + }) + + it('AC4: skips a run whose head commit carries the ratchet marker', () => { + const d = shouldSkipCommitBack({ + ...measuredPush, + commitBackFlag: 'enabled', + headCommitMessage: `chore: ratchet coverage baseline (shared 84->89) ${RATCHET_MARKER}`, + }) + expect(d.skip).toBe(true) + expect(d.code).toBe('automated-commit') + }) + + it('AC4: skips a squash-merge subject taken from the ratchet PR title', () => { + const d = shouldSkipCommitBack({ + ...measuredPush, + commitBackFlag: 'enabled', + headCommitMessage: `chore: ratchet coverage baseline ${RATCHET_MARKER} (#123)`, + }) + expect(d.code).toBe('automated-commit') + }) + + it('AC4: skips a plain merge commit naming the ratchet branch (marker not in the subject)', () => { + const d = shouldSkipCommitBack({ + ...measuredPush, + commitBackFlag: 'enabled', + headCommitMessage: `Merge pull request #123 from foomakers/${RATCHET_BRANCH}`, + }) + expect(d.code).toBe('automated-commit') + }) + + it('proceeds on a base-branch push from a human commit with the flag enabled', () => { + expect(shouldSkipCommitBack({ ...measuredPush, commitBackFlag: 'enabled' })).toEqual({ + skip: false, + }) + }) + + it('evaluates the flag before anything else (an enabled-only concern never leaks)', () => { + const d = shouldSkipCommitBack({ + ...measuredPush, + commitBackFlag: 'disabled', + eventName: 'pull_request', + headCommitMessage: RATCHET_MARKER, + }) + expect(d.code).toBe('flag-disabled') + }) +}) + +describe('planRatchet — monotonic, per-type', () => { + it('AC2: raises when the proposal is strictly above the committed baseline', () => { + const plan = planRatchet(CONFIG, { shared: 90.5 }) + expect(plan).toEqual([ + { + type: 'shared', + measured: 90.5, + current: 84, + proposed: 89, + action: 'raise', + reason: 'measured 90.5% => baseline 89 (floor - 1pp margin), above committed 84', + }, + ]) + }) + + it('AC3: holds when coverage is exactly at the baseline', () => { + const [p] = planRatchet(CONFIG, { shared: 84 }) + expect(p.action).toBe('hold') + expect(p.reason).toContain('not above') + }) + + it('AC3: holds when the proposal equals the committed baseline (fixpoint, no churn)', () => { + const [p] = planRatchet(CONFIG, { shared: 85.04 }) + expect(p.proposed).toBe(84) + expect(p.action).toBe('hold') + }) + + it('AC3 + business rule: NEVER lowers — a drop is a hold, not a write', () => { + const [p] = planRatchet(CONFIG, { shared: 40 }) + expect(p.action).toBe('hold') + expect(p.proposed).toBeLessThan(p.current as number) + }) + + it('edge case: a type measured but absent from the config is reported, not written', () => { + const [p] = planRatchet(CONFIG, { backend: 99 }) + expect(p.action).toBe('no-baseline-configured') + expect(p.current).toBeNull() + }) + + it('edge case: a malformed committed baseline is reported, not overwritten', () => { + const [p] = planRatchet('baseline.shared=oops\n', { shared: 99 }) + expect(p.action).toBe('no-baseline-configured') + }) + + it('edge case: a missing/unparseable measurement is reported, never assumed', () => { + expect(planRatchet(CONFIG, { shared: '' }).map(p => p.action)).toEqual(['not-measured']) + expect(planRatchet(CONFIG, { shared: 'null' }).map(p => p.action)).toEqual(['not-measured']) + expect(planRatchet(CONFIG, { shared: undefined }).map(p => p.action)).toEqual(['not-measured']) + }) + + it('plans every type independently and preserves input order', () => { + const plan = planRatchet(CONFIG, { shared: 90.5, frontend: 19.2, backend: 50 }) + expect(plan.map(p => [p.type, p.action])).toEqual([ + ['shared', 'raise'], + ['frontend', 'hold'], + ['backend', 'no-baseline-configured'], + ]) + }) + + it('accepts a custom margin', () => { + const [p] = planRatchet(CONFIG, { shared: 90.5 }, 0) + expect(p.proposed).toBe(90) + }) +}) + +describe('applyRaises — in-place value edit, surrounding markdown untouched', () => { + it('changes only the matched baseline line', () => { + const raises = planRatchet(CONFIG, { shared: 90.5 }).filter(p => p.action === 'raise') + const { text, changedLines } = applyRaises(CONFIG, raises) + expect(changedLines).toBe(1) + + const before = CONFIG.split('\n') + const after = text.split('\n') + expect(after.length).toBe(before.length) + const differing = before + .map((line, i) => (line === after[i] ? null : i)) + .filter((i): i is number => i !== null) + expect(differing.length).toBe(1) + expect(before[differing[0]]).toBe('baseline.shared=84') + expect(after[differing[0]]).toBe('baseline.shared=89') + }) + + it('leaves the file byte-identical when there is nothing to raise', () => { + expect(applyRaises(CONFIG, []).text).toBe(CONFIG) + expect(applyRaises(CONFIG, []).changedLines).toBe(0) + }) + + it('raises several types in one pass', () => { + const raises = planRatchet(CONFIG, { shared: 95, frontend: 40 }).filter( + p => p.action === 'raise', + ) + const { text, changedLines } = applyRaises(CONFIG, raises) + expect(changedLines).toBe(2) + expect(text).toContain('baseline.shared=94') + expect(text).toContain('baseline.frontend=39') + }) + + it('is idempotent — re-planning against the written text yields no further raise', () => { + const raises = planRatchet(CONFIG, { shared: 90.5 }).filter(p => p.action === 'raise') + const { text } = applyRaises(CONFIG, raises) + expect(planRatchet(text, { shared: 90.5 }).map(p => p.action)).toEqual(['hold']) + }) + + it('refuses to write a value that is not strictly above what is on disk now (concurrency)', () => { + // Simulates: this run planned 89 from a stale read; a concurrent run already + // committed 92. Re-checking against the CURRENT text must drop the proposal + // so the higher value is never clobbered. + const concurrent = CONFIG.replace('baseline.shared=84', 'baseline.shared=92') + const stale = planRatchet(CONFIG, { shared: 90.5 }).filter(p => p.action === 'raise') + const { text, changedLines, dropped } = applyRaises(concurrent, stale) + expect(changedLines).toBe(0) + expect(text).toBe(concurrent) + expect(dropped.map(d => d.type)).toEqual(['shared']) + }) + + it('preserves CRLF line endings on the edited line', () => { + const crlf = 'x\r\nbaseline.shared=84\r\ny\r\n' + const raises = planRatchet(crlf, { shared: 90.5 }).filter(p => p.action === 'raise') + expect(applyRaises(crlf, raises).text).toBe('x\r\nbaseline.shared=89\r\ny\r\n') + }) +}) + +describe('classifyWriteRefusal — AC6 names the reason', () => { + it('names a missing credential', () => { + expect(classifyWriteRefusal('', { hasToken: false }).code).toBe('missing-credential') + expect(classifyWriteRefusal('', { hasToken: false }).message).toContain(TOKEN_ENV) + }) + + it('names branch protection', () => { + const r = classifyWriteRefusal( + 'remote: error: GH006: Protected branch update failed for refs/heads/main.', + { hasToken: true }, + ) + expect(r.code).toBe('protected-branch') + }) + + it('names an insufficient scope / permission', () => { + expect( + classifyWriteRefusal('remote: Permission to foomakers/pair.git denied to github-actions.', { + hasToken: true, + }).code, + ).toBe('insufficient-scope') + expect( + classifyWriteRefusal('gh: Resource not accessible by integration (HTTP 403)', { + hasToken: true, + }).code, + ).toBe('insufficient-scope') + }) + + it('names a stale-lease race', () => { + expect( + classifyWriteRefusal('! [rejected] chore/x -> chore/x (stale info)', { hasToken: true }).code, + ).toBe('stale-lease') + }) + + it('falls back to an explicit unknown rather than swallowing the output', () => { + const r = classifyWriteRefusal('something nobody predicted', { hasToken: true }) + expect(r.code).toBe('unknown') + expect(r.message).toContain('something nobody predicted') + }) +}) + +describe('ratchetGitPlan — the exact command sequence (a bot PR, never a push to base)', () => { + const raises = planRatchet(CONFIG, { shared: 90.5 }).filter(p => p.action === 'raise') + const HEAD_SHA = '0123456789abcdef0123456789abcdef01234567' + const plan = ratchetGitPlan({ + raises, + configPath: '.pair/adoption/tech/coverage-baseline.md', + baseBranch: 'main', + remote: 'origin', + headCommit: HEAD_SHA, + }) + const argvs = plan.commands.map(c => c.argv) + const flat = argvs.flat() + + it('never pushes to the base branch — only to the dedicated ratchet branch', () => { + const pushes = argvs.filter(c => c[0] === 'git' && c[1] === 'push') + expect(pushes.length).toBe(1) + expect(pushes[0]).toContain(`HEAD:refs/heads/${RATCHET_BRANCH}`) + expect(pushes.flat().join(' ')).not.toContain('refs/heads/main') + }) + + it('pushes with --force-with-lease (never a bare force)', () => { + const push = argvs.find(c => c[1] === 'push') as string[] + expect(push).toContain('--force-with-lease') + expect(push).not.toContain('--force') + }) + + it('makes the lease possible: maps + fetches a remote-tracking ref BEFORE the push', () => { + // Without this a CI checkout (which fetches only the base ref) has no + // remote-tracking ref for the destination, and git rejects every + // non-fast-forward lease push as `stale info` — the ratchet would work once + // and then warn forever. + const refspec = `+refs/heads/${RATCHET_BRANCH}:refs/remotes/origin/${RATCHET_BRANCH}` + const configIdx = argvs.findIndex(c => c[1] === 'config' && c[2] === '--add') + const fetchIdx = argvs.findIndex(c => c[1] === 'fetch') + const pushIdx = argvs.findIndex(c => c[1] === 'push') + expect(argvs[configIdx]).toEqual(['git', 'config', '--add', 'remote.origin.fetch', refspec]) + expect(argvs[fetchIdx]).toEqual(['git', 'fetch', '--no-tags', 'origin', refspec]) + expect(configIdx).toBeLessThan(fetchIdx) + expect(fetchIdx).toBeLessThan(pushIdx) + }) + + it('tolerates ONLY the fetch failing — the branch does not exist on the first run', () => { + const optional = plan.commands.filter(c => c.optional === true).map(c => c.argv[1]) + expect(optional).toEqual(['fetch']) + }) + + it('creates no local branch and never switches branches', () => { + expect(argvs.some(c => c[1] === 'checkout' || c[1] === 'switch' || c[1] === 'branch')).toBe( + false, + ) + }) + + it('restores the checkout to the SHA the run was given, by SHA and not HEAD~1', () => { + // A relative reset would destroy the base branch's own tip if the sequence + // failed before the commit existed. + expect(plan.restore).toEqual(['git', 'reset', '--hard', HEAD_SHA]) + expect(plan.restore).not.toContain('HEAD~1') + }) + + it('stages ONLY the config file — never `git add -A`', () => { + const add = argvs.find(c => c[1] === 'add') as string[] + expect(add).toEqual(['git', 'add', '--', '.pair/adoption/tech/coverage-baseline.md']) + expect(flat).not.toContain('-A') + }) + + it('marks the commit subject as automated (loop marker)', () => { + expect(plan.commitMessage).toContain(RATCHET_MARKER) + expect(plan.commitMessage).toContain('shared 84->89') + }) + + it('carries the marker in the PR title too, so a squash subject keeps it', () => { + expect(plan.prTitle).toContain(RATCHET_MARKER) + }) + + it('creates-or-updates ONE PR against the base branch from the ratchet branch', () => { + const gh = argvs.filter(c => c[0] === 'gh') + expect(gh.length).toBe(1) + const ghFlat = gh.map(c => c.join(' ')).join('\n') + expect(ghFlat).toContain(`--base main`) + expect(ghFlat).toContain(`--head ${RATCHET_BRANCH}`) + }) + + it('explains itself in the PR body: why a PR, the flag, and the ADL', () => { + expect(plan.prBody).toContain('#372') + expect(plan.prBody).toContain('2026-07-30-coverage-ratchet-pr-not-push') + expect(plan.prBody).toContain('shared') + }) + + it('refreshes an already-open ratchet PR in place instead of opening a second one', () => { + expect(plan.prUpdate.slice(0, 4)).toEqual(['gh', 'pr', 'edit', RATCHET_BRANCH]) + expect(plan.prUpdate).toContain(plan.prTitle) + expect(plan.prUpdate).toContain(plan.prBody) + }) + + it('has no commands at all when there is nothing to raise', () => { + const empty = ratchetGitPlan({ + raises: [], + configPath: 'x.md', + baseBranch: 'main', + remote: 'origin', + headCommit: HEAD_SHA, + }) + expect(empty.commands).toEqual([]) + expect(empty.prUpdate).toEqual([]) + expect(empty.restore).toEqual([]) + }) +}) + +describe('renderRatchetPlan — the human-readable step output', () => { + it('reports a skip with its code and reason', () => { + const out = renderRatchetPlan({ + skip: { skip: true, code: 'flag-disabled', reason: 'commit-back is disabled' }, + plan: [], + }) + expect(out).toContain('SKIPPED') + expect(out).toContain('flag-disabled') + }) + + it('reports each per-type action', () => { + const out = renderRatchetPlan({ plan: planRatchet(CONFIG, { shared: 90.5, frontend: 19.2 }) }) + expect(out).toContain('shared') + expect(out).toContain('raise') + expect(out).toContain('frontend') + expect(out).toContain('hold') + }) + + it('says so explicitly when nothing is to be raised', () => { + expect(renderRatchetPlan({ plan: planRatchet(CONFIG, { shared: 84 }) })).toContain('no raise') + }) +}) + +describe('module constants pin the contract the CI step and the docs rely on', () => { + it('exposes the marker, branch, margin and token env name', () => { + expect(RATCHET_MARKER).toBe('[coverage-baseline-ratchet]') + expect(RATCHET_BRANCH).toBe('chore/coverage-baseline-ratchet') + expect(DEFAULT_MARGIN_PP).toBe(1) + expect(TOKEN_ENV).toBe('COVERAGE_RATCHET_TOKEN') + }) +}) diff --git a/packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts b/packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts new file mode 100644 index 00000000..fec56323 --- /dev/null +++ b/packages/knowledge-hub/src/tools/coverage-baseline-ratchet.ts @@ -0,0 +1,735 @@ +/** + * coverage-baseline-ratchet — the OPT-IN commit-back half of the coverage + * guardrail (story #372, on top of #282). + * + * The guardrail itself (`.pair/knowledge/assets/coverage-gate.sh`) decides + * pass/fail and NEVER persists anything. This module is the separable side + * effect: given the coverage a run measured and the committed config, it decides + * whether a `baseline.` may be RAISED, produces the edited config text, + * and produces the git/gh command plan that lands the raise as a bot pull + * request. The two are independent by construction — a refused write can never + * change the gate's verdict (AC6). + * + * The decisions this module encodes are recorded in + * `.pair/adoption/decision-log/2026-07-30-coverage-ratchet-pr-not-push.md`: + * + * - AC5 — a `pull_request` run NEVER writes back. Only the post-merge `push` + * on the base branch does, and it writes a bot PR from a dedicated branch, + * never a push to the (post-#234 protected) base branch. + * - AC4 — loop termination is a skip predicate (`shouldSkipCommitBack`) plus a + * fixpoint (`proposeBaseline` is idempotent on unchanged coverage), not + * `[skip ci]`. + * - AC6 — a refused write is classified and named (`classifyWriteRefusal`) and + * reported as a warning; the CLI exits 0. + * - Monotonic ratchet — `applyRaises` re-checks every proposal against the text + * currently on disk, so a higher value written by a concurrent run is never + * clobbered, and a value is never lowered. + * + * Per the gate-tooling ADL (2026-07-13) the logic lives HERE, white-box unit + * tested, and the CI step + `coverage:ratchet` scripts are thin entrypoints; the + * CLI wiring is verified end-to-end by the `coverage-gate.sh` smoke scenario + * (`--dry-run`), never by a unit test. + */ +import { execFileSync } from 'node:child_process' +import { readFileSync, writeFileSync } from 'node:fs' + +/** Marker that makes an automated commit (and the ratchet PR title) self-identifying. */ +export const RATCHET_MARKER = '[coverage-baseline-ratchet]' + +/** The dedicated branch the ratchet PR is opened from. Never the base branch. */ +export const RATCHET_BRANCH = 'chore/coverage-baseline-ratchet' + +/** + * Margin, in percentage points, between the measured coverage and the baseline + * written. NOT a new convention: `tech/coverage-baseline.md` already documents + * "~1pp below the measured value, to absorb per-run float jitter and legitimate + * line churn", and `floor(measured) - 1` reproduces both committed values + * exactly (85.04 -> 84, 20.16 -> 19). It is also what makes the ratchet a + * fixpoint, which is half of the loop-termination guarantee. + */ +export const DEFAULT_MARGIN_PP = 1 + +/** Env var carrying the dedicated, repo-scoped write credential (see the ADL). */ +export const TOKEN_ENV = 'COVERAGE_RATCHET_TOKEN' + +/** Slug of the ADL that decided the PR-vs-push behaviour and the credential model. */ +export const ADL_SLUG = '2026-07-30-coverage-ratchet-pr-not-push' + +const DEFAULT_CONFIG_PATH = '.pair/adoption/tech/coverage-baseline.md' +const DEFAULT_WOW_PATH = '.pair/adoption/tech/way-of-working.md' +const BOT_NAME = 'pair-coverage-ratchet[bot]' +const BOT_EMAIL = 'pair-coverage-ratchet[bot]@users.noreply.github.com' + +/** Escape regex metacharacters so a dotted config key matches literally. */ +const esc = (s: string): string => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + +/** True iff the value is a plain non-negative number (same shape the shell gate accepts). */ +const isNumeric = (v: unknown): boolean => typeof v === 'string' && /^\d+(\.\d+)?$/.test(v.trim()) + +// --------------------------------------------------------------------------- +// Ratchet arithmetic + config reading +// --------------------------------------------------------------------------- + +/** + * The baseline value a measured coverage justifies: `floor(measured) - margin`, + * clamped at 0. Idempotent by construction — re-running on the same coverage + * proposes the value already committed. + */ +export function proposeBaseline(measuredPct: number, marginPp: number = DEFAULT_MARGIN_PP): number { + return Math.max(0, Math.floor(measuredPct) - marginPp) +} + +/** + * The committed `baseline.` value, or null when absent or malformed. + * Config format is UNCHANGED from #282: a `^baseline.=` line, matched + * literally, ignoring the surrounding markdown. A trailing CR (CRLF-authored + * config) is tolerated. + */ +export function readBaselineValue(configText: string, type: string): number | null { + const m = new RegExp(`^baseline\\.${esc(type)}=([^\\r\\n]*)`, 'm').exec(configText) + if (!m) return null + const raw = (m[1] ?? '').trim() + return /^\d+$/.test(raw) ? Number(raw) : null +} + +/** Declared state of the `Coverage baseline commit-back` opt-in. */ +export type CommitBackFlag = 'enabled' | 'disabled' | 'absent' + +/** + * Read the opt-in from a way-of-working document. Absent => off: the framework + * default ships `disabled`, so a project that never declares the flag behaves + * exactly as before this story (AC1). + */ +export function readCommitBackFlag(wayOfWorkingText: string): CommitBackFlag { + const m = /coverage\s+baseline\s+commit-back\**\s*:\s*`?\s*(enabled|disabled)/i.exec( + wayOfWorkingText, + ) + return m ? ((m[1] ?? '').toLowerCase() as CommitBackFlag) : 'absent' +} + +// --------------------------------------------------------------------------- +// Skip predicate (AC1 / AC4 / AC5) +// --------------------------------------------------------------------------- + +export type SkipCode = 'flag-disabled' | 'not-base-push' | 'automated-commit' + +export interface SkipDecision { + skip: boolean + /** Set only when `skip` is true. */ + code?: SkipCode + /** Set only when `skip` is true. */ + reason?: string +} + +export interface RunContext { + commitBackFlag: CommitBackFlag + /** `GITHUB_EVENT_NAME` — only `push` may write. */ + eventName: string + /** `GITHUB_REF_NAME` — must equal `baseBranch`. */ + refName: string + baseBranch: string + /** Subject+body of the commit the run is for. */ + headCommitMessage: string +} + +/** True iff a commit message identifies itself as produced by the ratchet. */ +function isRatchetCommitMessage(message: string): boolean { + // The marker covers a direct commit and a squash merge (whose subject defaults + // to the ratchet PR title, which carries the marker). The branch name covers a + // plain merge commit ("Merge pull request #N from owner/"), + // whose subject GitHub generates without the PR title. + return message.includes(RATCHET_MARKER) || message.includes(RATCHET_BRANCH) +} + +/** + * Whether this run must NOT attempt a commit-back. Evaluated in order so the + * opt-in short-circuits everything else: + * + * 1. `flag-disabled` — the flag is not `enabled` (default; AC1). + * 2. `not-base-push` — not a `push` to the base branch, i.e. every + * `pull_request` run, fork or not (AC5). + * 3. `automated-commit`— the head commit is the ratchet's own (AC4). + */ +export function shouldSkipCommitBack(ctx: RunContext): SkipDecision { + if (ctx.commitBackFlag !== 'enabled') { + return { + skip: true, + code: 'flag-disabled', + reason: `Coverage baseline commit-back is '${ctx.commitBackFlag}' (default: off) — nothing is written`, + } + } + if (ctx.eventName !== 'push' || ctx.refName !== ctx.baseBranch) { + return { + skip: true, + code: 'not-base-push', + reason: `only a push to '${ctx.baseBranch}' writes back (got event '${ctx.eventName}' on '${ctx.refName}') — a pull request never does, see ADL ${ADL_SLUG}`, + } + } + if (isRatchetCommitMessage(ctx.headCommitMessage)) { + return { + skip: true, + code: 'automated-commit', + reason: `head commit is the ratchet's own ${RATCHET_MARKER} — stopping the loop`, + } + } + return { skip: false } +} + +// --------------------------------------------------------------------------- +// Planning (AC2 / AC3 + edge cases) +// --------------------------------------------------------------------------- + +export type RatchetAction = 'raise' | 'hold' | 'no-baseline-configured' | 'not-measured' + +export interface RatchetProposal { + type: string + measured: number | null + /** The committed baseline, or null when absent/malformed. */ + current: number | null + /** The value the measurement justifies, or null when nothing was measured. */ + proposed: number | null + action: RatchetAction + reason: string +} + +/** Per-type measured coverage, as extracted from the coverage report. */ +export type MeasuredCoverage = Record + +/** + * Decide, per type, whether the committed baseline may be raised. Nothing is + * written here. The ratchet is monotonic: only a proposal STRICTLY above the + * committed value is a `raise`; equal, lower and unmeasured are all holds. + */ +export function planRatchet( + configText: string, + measured: MeasuredCoverage, + marginPp: number = DEFAULT_MARGIN_PP, +): RatchetProposal[] { + return Object.keys(measured).map(type => { + const raw = measured[type] + if (!isNumeric(raw === undefined ? undefined : String(raw))) { + return { + type, + measured: null, + current: readBaselineValue(configText, type), + proposed: null, + action: 'not-measured', + reason: `no usable coverage measured for '${type}' — nothing written (the gate's own fail-safe still applies)`, + } + } + const measuredPct = Number(raw) + const current = readBaselineValue(configText, type) + const proposed = proposeBaseline(measuredPct, marginPp) + if (current === null) { + return { + type, + measured: measuredPct, + current: null, + proposed, + action: 'no-baseline-configured', + reason: `no valid committed baseline.${type} in the config — reporting the suggestion ${proposed} only, writing nothing (a first baseline stays a human commit)`, + } + } + if (proposed > current) { + return { + type, + measured: measuredPct, + current, + proposed, + action: 'raise', + reason: `measured ${measuredPct}% => baseline ${proposed} (floor - ${marginPp}pp margin), above committed ${current}`, + } + } + return { + type, + measured: measuredPct, + current, + proposed, + action: 'hold', + reason: `measured ${measuredPct}% => baseline ${proposed}, not above committed ${current} — the ratchet only ever moves up`, + } + }) +} + +export interface ApplyResult { + text: string + changedLines: number + /** Proposals refused at write time because the value on disk is already >=. */ + dropped: RatchetProposal[] +} + +/** + * Apply raises to the config text by editing the `baseline.=` VALUE in + * place — the surrounding markdown, line count, ordering and line endings are + * untouched. + * + * Every proposal is re-checked against the text passed in (which the caller + * re-reads from disk immediately before writing), so a higher value committed by + * a concurrent run is never clobbered: such a proposal is `dropped`, not written. + */ +export function applyRaises(configText: string, raises: RatchetProposal[]): ApplyResult { + let text = configText + let changedLines = 0 + const dropped: RatchetProposal[] = [] + + for (const raise of raises) { + if (raise.proposed === null) { + dropped.push(raise) + continue + } + const onDisk = readBaselineValue(text, raise.type) + if (onDisk === null || raise.proposed <= onDisk) { + dropped.push(raise) + continue + } + const re = new RegExp(`^(baseline\\.${esc(raise.type)}=)([^\\r\\n]*)`, 'm') + text = text.replace(re, `$1${raise.proposed}`) + changedLines += 1 + } + + return { text, changedLines, dropped } +} + +// --------------------------------------------------------------------------- +// Refusal classification (AC6) +// --------------------------------------------------------------------------- + +export type RefusalCode = + | 'missing-credential' + | 'protected-branch' + | 'insufficient-scope' + | 'stale-lease' + | 'unknown' + +export interface Refusal { + code: RefusalCode + message: string +} + +/** + * Name the reason a write was refused. AC6: the run degrades to a WARNING that + * names the reason and the gate's verdict is unchanged — so the reason must be + * identified, never swallowed (`unknown` still carries the raw output). + */ +export function classifyWriteRefusal(output: string, opts: { hasToken: boolean }): Refusal { + if (!opts.hasToken) { + return { + code: 'missing-credential', + message: `no write credential: ${TOKEN_ENV} is not set (a repo-scoped token with contents:write + pull-requests:write — see ADL ${ADL_SLUG})`, + } + } + if (/GH006|protected branch/i.test(output)) { + return { code: 'protected-branch', message: `refused by branch protection: ${output.trim()}` } + } + if (/permission to .* denied|not accessible by integration|403|insufficient/i.test(output)) { + return { + code: 'insufficient-scope', + message: `credential lacks the required permission: ${output.trim()}`, + } + } + if (/stale info|force-with-lease|non-fast-forward|\[rejected\]/i.test(output)) { + return { + code: 'stale-lease', + message: `the ratchet branch moved under us (concurrent run): ${output.trim()}`, + } + } + return { code: 'unknown', message: `write refused, reason not recognized: ${output.trim()}` } +} + +// --------------------------------------------------------------------------- +// Command plan — a bot PR, never a push to the base branch +// --------------------------------------------------------------------------- + +/** + * One step of the plan. `optional` marks a command whose failure is EXPECTED in + * a legitimate state and must not abort the sequence — currently only the + * ratchet-branch fetch, which fails on the first ever run because the branch + * does not exist remotely yet. + */ +export interface RatchetCommand { + argv: string[] + optional?: boolean +} + +export interface RatchetGitPlan { + /** Ordered command sequence. Empty when there is nothing to raise. */ + commands: RatchetCommand[] + /** Run only when `gh pr create` reports the PR already exists. */ + prUpdate: string[] + /** + * Run afterwards whatever happened — success, refusal or crash — so the + * checkout is left exactly as the steps after this one expect to find it + * (the local ratchet commit and the edited config are dropped; the pushed + * ratchet branch is unaffected). + */ + restore: string[] + commitMessage: string + prTitle: string + prBody: string +} + +export interface RatchetGitPlanInput { + raises: RatchetProposal[] + configPath: string + baseBranch: string + remote: string + /** SHA the run checked out — what `restore` returns the workspace to. */ + headCommit: string +} + +/** The ratchet PR's body: what changed, why it is a PR, and what merging it means. */ +function ratchetPrBody(raises: RatchetProposal[], configPath: string, baseBranch: string): string { + return [ + `Automated coverage-baseline ratchet — opt-in commit-back from story #372.`, + ``, + `| Type | Committed | Measured | New baseline |`, + `| --- | --- | --- | --- |`, + ...raises.map(r => `| \`${r.type}\` | ${r.current} | ${r.measured}% | **${r.proposed}** |`), + ``, + `Only \`baseline.\` values in \`${configPath}\` are edited, in place; the ratchet never lowers a baseline.`, + `New values are \`floor(measured) - ${DEFAULT_MARGIN_PP}pp\`, the margin that file already documents.`, + ``, + `Why a pull request and not a push to \`${baseBranch}\`: \`.pair/adoption/decision-log/${ADL_SLUG}.md\`.`, + `Merging this raises the guardrail's floor. Closing it declines the raise; the next base-branch push will propose it again.`, + ].join('\n') +} + +/** + * The exact command sequence that lands the raise. Deliberate properties, each + * asserted by a unit test: + * + * - the ONLY push targets `RATCHET_BRANCH`, never the base branch, and no + * local branch is created or switched to: the commit is made on the checked- + * out HEAD and pushed by explicit refspec, then `restore` undoes it. Leaving + * the workspace on a bot branch would silently change what every later step + * in the job runs against. + * - the push uses `--force-with-lease`, never a bare `--force`. The lease needs + * a remote-tracking ref for the destination, and a CI checkout has none for + * this branch (it fetches only the base ref) — without one git rejects EVERY + * non-fast-forward push as `stale info`, which would make the ratchet work + * exactly once and then warn forever. So the plan first teaches the remote a + * fetch refspec for the ratchet branch and fetches it (optional: absent on + * the first run). The lease then means what it says: if the ratchet branch + * moved between that fetch and the push, this run loses. + * - staging is an explicit path — never `git add -A`; + * - the commit subject AND the PR title carry `RATCHET_MARKER`, so the loop + * guard survives both a squash and a merge commit. + * + * The force-push to the ratchet branch updates an already-open ratchet PR in + * place, which is why create-or-update needs no branching beyond `prUpdate` + * (title/body refresh). + * + * Note what does NOT depend on the lease: never clobbering a higher baseline is + * guaranteed at the data level by `applyRaises` re-reading the config and + * re-checking every proposal, not by the push. + */ +export function ratchetGitPlan(input: RatchetGitPlanInput): RatchetGitPlan { + const { raises, configPath, baseBranch, remote, headCommit } = input + if (raises.length === 0) { + return { commands: [], prUpdate: [], restore: [], commitMessage: '', prTitle: '', prBody: '' } + } + + const summary = raises.map(r => `${r.type} ${r.current}->${r.proposed}`).join(', ') + const commitMessage = `chore: ratchet coverage baseline (${summary}) ${RATCHET_MARKER}` + const prTitle = `chore: ratchet coverage baseline ${RATCHET_MARKER}` + const prBody = ratchetPrBody(raises, configPath, baseBranch) + const trackingRefspec = `+refs/heads/${RATCHET_BRANCH}:refs/remotes/${remote}/${RATCHET_BRANCH}` + const ghCreate = ['gh', 'pr', 'create', '--base', baseBranch, '--head', RATCHET_BRANCH] + + return { + commands: [ + { argv: ['git', 'config', 'user.name', BOT_NAME] }, + { argv: ['git', 'config', 'user.email', BOT_EMAIL] }, + // Commit on the checked-out HEAD — no branch is created or switched to. + { argv: ['git', 'add', '--', configPath] }, + { argv: ['git', 'commit', '-m', commitMessage] }, + // Make the destination leaseable: map it to a remote-tracking ref, then + // fetch it. Optional — on the first run the branch does not exist yet. + { argv: ['git', 'config', '--add', `remote.${remote}.fetch`, trackingRefspec] }, + { argv: ['git', 'fetch', '--no-tags', remote, trackingRefspec], optional: true }, + { argv: ['git', 'push', '--force-with-lease', remote, `HEAD:refs/heads/${RATCHET_BRANCH}`] }, + { argv: [...ghCreate, '--title', prTitle, '--body', prBody] }, + ], + prUpdate: ['gh', 'pr', 'edit', RATCHET_BRANCH, '--title', prTitle, '--body', prBody], + // An explicit SHA, never `HEAD~1`: a relative reset would destroy the base + // branch's own tip if the sequence failed before the commit was created. + restore: ['git', 'reset', '--hard', headCommit], + commitMessage, + prTitle, + prBody, + } +} + +// --------------------------------------------------------------------------- +// Reporting +// --------------------------------------------------------------------------- + +export interface RenderInput { + skip?: SkipDecision + plan: RatchetProposal[] +} + +/** The step's human-readable output: one line per type, or the skip and its code. */ +export function renderRatchetPlan({ skip, plan }: RenderInput): string { + if (skip?.skip) { + return `coverage-ratchet: SKIPPED (${skip.code}) — ${skip.reason}` + } + const lines = plan.map( + p => + `coverage-ratchet: ${p.type} — ${p.action}: ${p.reason} [measured=${p.measured ?? 'n/a'} committed=${p.current ?? 'n/a'} proposed=${p.proposed ?? 'n/a'}]`, + ) + if (!plan.some(p => p.action === 'raise')) { + lines.push('coverage-ratchet: no raise — nothing to commit back') + } + return lines.join('\n') +} + +// --------------------------------------------------------------------------- +// Thin CLI entrypoint (verified by the coverage-gate smoke scenario, not by a +// unit test — gate-tooling ADL 2026-07-13) +// --------------------------------------------------------------------------- + +interface CliOptions { + configPath: string + wowPath: string + measured: MeasuredCoverage + baseBranch: string + remote: string + marginPp: number + dryRun: boolean +} + +/** One flag's effect. `value` is undefined for the flags that take no argument. */ +type FlagHandler = (opts: CliOptions, value: string | undefined) => void + +/** `takesValue` says whether the next argv entry belongs to this flag. */ +const CLI_FLAGS: Record = { + '--config': { takesValue: true, apply: (o, v) => void (o.configPath = v as string) }, + '--way-of-working': { takesValue: true, apply: (o, v) => void (o.wowPath = v as string) }, + '--measured': { + takesValue: true, + apply: (o, v) => { + const [type, pct] = (v as string).split('=') + if (!type) throw new Error('--measured expects =') + o.measured[type] = pct + }, + }, + '--base-branch': { takesValue: true, apply: (o, v) => void (o.baseBranch = v as string) }, + '--remote': { takesValue: true, apply: (o, v) => void (o.remote = v as string) }, + '--margin': { takesValue: true, apply: (o, v) => void (o.marginPp = Number(v)) }, + '--dry-run': { takesValue: false, apply: o => void (o.dryRun = true) }, + // POSIX separator — `pnpm run