diff --git a/.claude/skills/issues/SKILL.md b/.claude/skills/issues/SKILL.md index 5d6145be90..c04936235a 100644 --- a/.claude/skills/issues/SKILL.md +++ b/.claude/skills/issues/SKILL.md @@ -64,6 +64,9 @@ paragraph; put the smallest next action in **Detail / next action**. open or resolved table as appropriate. - IDs are monotonic and never reused — always allocate from the `issues:next-id` marker and bump it. - Escape `|` inside cell text (write `\|`) so the markdown table stays intact. +- This file uses `merge=union` in `.gitattributes`. Never resolve a conflict by taking one side + wholesale — that drops the other agent's rows. `npm run check:outstanding-issues` fails on + duplicate IDs or a stale next-id marker. - Respect the repo's RAG/clinical/privacy flagging rules if an item _itself_ touches a protected surface — recording it here is fine, but acting on it later still needs the usual gate. diff --git a/.gitattributes b/.gitattributes index 1d799a170f..94ba096249 100644 --- a/.gitattributes +++ b/.gitattributes @@ -12,3 +12,8 @@ # and drops exact duplicate dated rows (stock `union` kept concurrent appends # but also reintroduced babysit twins that failed the ledger guard). docs/branch-review-ledger.md merge=ledger + +# Outstanding-issue rows are also append-mostly across concurrent agent sessions. +# Union merge preserves both sides' rows; scripts/check-outstanding-issues.mjs +# still fails on duplicate IDs because row-level union alone cannot allocate IDs. +docs/outstanding-issues.md merge=union diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc38734af4..4c5ccc3895 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -214,6 +214,9 @@ jobs: - name: Outstanding-issues ledger integrity run: npm run check:outstanding-issues + - name: PR mergeability workflow contract + run: npm run check:pr-mergeability + - name: Codebase index coverage run: npm run docs:check-index diff --git a/.github/workflows/pr-mergeability.yml b/.github/workflows/pr-mergeability.yml new file mode 100644 index 0000000000..bfc1002e4e --- /dev/null +++ b/.github/workflows/pr-mergeability.yml @@ -0,0 +1,202 @@ +name: PR mergeability + +on: + pull_request_target: + branches: [main, "release/**"] + types: [opened, synchronize, reopened, ready_for_review, edited] + push: + branches: [main, "release/**"] + +concurrency: + group: pr-mergeability-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + pull-requests: read + +jobs: + mergeability: + name: PR mergeability + if: github.event_name == 'pull_request_target' + runs-on: ubuntu-24.04 + timeout-minutes: 5 + steps: + # pull_request_target runs trusted workflow-revision code. Checkout + # github.workflow_sha so the classifier is the exact revision that + # triggered this run. Never execute the PR head or persist credentials. + - name: Checkout trusted classifier + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.workflow_sha }} + persist-credentials: false + + - name: Signal real merge conflicts + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const { pathToFileURL } = require("node:url"); + const moduleUrl = pathToFileURL(`${process.env.GITHUB_WORKSPACE}/scripts/pr-mergeability.mjs`).href; + const { classifyMergeability } = await import(moduleUrl); + const prNumber = context.payload.pull_request?.number; + if (!prNumber) { + core.setFailed("Missing pull_request.number in pull_request_target payload."); + return; + } + + const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + const maxAttempts = 5; + let verdict; + + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + let latestPr; + try { + latestPr = ( + await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: prNumber, + }) + ).data; + } catch (error) { + core.setFailed( + `Unable to fetch latest PR metadata for PR #${prNumber}: ${error instanceof Error ? error.message : String(error)}. If this was transient, rerun the job.`, + ); + return; + } + + verdict = classifyMergeability({ + mergeable: latestPr.mergeable, + mergeableState: latestPr.mergeable_state, + draft: latestPr.draft, + }); + + await core.summary + .addHeading("PR mergeability") + .addRaw(`Attempt: ${attempt}/${maxAttempts}
`) + .addRaw(`Draft: ${latestPr.draft ? "yes" : "no"}
`) + .addRaw(`mergeable: ${String(latestPr.mergeable)}
`) + .addRaw(`mergeable_state: ${String(latestPr.mergeable_state)}
`) + .addRaw(`Verdict: ${verdict.action} (${verdict.reason})
`) + .write(); + + if (verdict.action !== "retry") break; + if (attempt < maxAttempts) await sleep(3000); + } + + if (verdict.action === "retry") { + core.setFailed( + `GitHub did not finish computing mergeability for PR #${prNumber} after ${maxAttempts} attempts. Rerun this check; do not treat a missing CI check list as a pass.`, + ); + return; + } + + if (verdict.action === "skip") { + core.notice(verdict.message); + return; + } + + if (!verdict.ok) { + core.setFailed(verdict.message); + return; + } + + core.notice(verdict.message); + + refresh-after-base-push: + name: Refresh PR mergeability after base push + if: github.event_name == 'push' + runs-on: ubuntu-24.04 + timeout-minutes: 10 + permissions: + contents: read + pull-requests: read + checks: write + steps: + # A base push is trusted repository code. Pin the checkout to that exact + # base commit and never fetch or execute an open PR's head. + - name: Checkout trusted base classifier + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.sha }} + persist-credentials: false + + - name: Refresh unchanged PR heads + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const { pathToFileURL } = require("node:url"); + const moduleUrl = pathToFileURL(`${process.env.GITHUB_WORKSPACE}/scripts/pr-mergeability.mjs`).href; + const { classifyMergeability } = await import(moduleUrl); + const base = context.ref.replace(/^refs\/heads\//, ""); + const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms)); + const maxAttempts = 5; + let refreshErrors = 0; + + const pulls = await github.paginate(github.rest.pulls.list, { + owner: context.repo.owner, + repo: context.repo.repo, + state: "open", + base, + per_page: 100, + }); + + for (const listedPr of pulls) { + let latestPr = listedPr; + let verdict; + + try { + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + latestPr = ( + await github.rest.pulls.get({ + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: listedPr.number, + }) + ).data; + verdict = classifyMergeability({ + mergeable: latestPr.mergeable, + mergeableState: latestPr.mergeable_state, + draft: latestPr.draft, + }); + if (verdict.action !== "retry") break; + if (attempt < maxAttempts) await sleep(3000); + } + + const unresolved = verdict?.action === "retry"; + const skipped = verdict?.action === "skip"; + const conclusion = unresolved ? "failure" : skipped || verdict?.ok ? "success" : "failure"; + + const reason = unresolved + ? `GitHub did not finish computing mergeability after ${maxAttempts} attempts.` + : verdict.message; + await github.rest.checks.create({ + owner: context.repo.owner, + repo: context.repo.repo, + name: "PR mergeability", + head_sha: latestPr.head.sha, + status: "completed", + conclusion, + output: { + title: `PR #${latestPr.number}: ${conclusion}`, + summary: [ + `Base ${base} advanced to ${context.sha}.`, + `Head remained ${latestPr.head.sha}.`, + `mergeable: ${String(latestPr.mergeable)}`, + `mergeable_state: ${String(latestPr.mergeable_state)}`, + reason, + ].join("\n\n"), + }, + }); + } catch (error) { + refreshErrors += 1; + core.error( + `Unable to refresh PR #${listedPr.number}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + + core.info(`Refreshed PR mergeability for ${pulls.length} open PR(s) targeting ${base}.`); + if (refreshErrors > 0) { + core.setFailed(`${refreshErrors} PR mergeability refresh(es) could not be published.`); + } diff --git a/AGENTS.md b/AGENTS.md index 7da7596f8e..9b4ee92ab3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -602,6 +602,61 @@ several into one PR/session rather than a dedicated branch each. Bundling saves PR/CI-invocation count, not verification rigor — every bundled item still gets the smallest correct gate run against it before it joins the PR. + + +## Anti-conflict and CI-speed operating procedure + +Goal: fewer false merge conflicts, less cancelled CI, and faster feedback — without +weakening required gates, flake policy, provider boundaries, or clinical/RAG safeguards. +Do not touch unrelated active PRs unless the user explicitly asks (`Run PR`, sync, or a +named PR). Future process only. + +### Prevent conflicts before they start + +- Prefer fewer, shorter-lived PRs. Bundle independently low-risk append-only docs/ledger + chores (see "## PR bundling") instead of one PR per line. +- Start from a fresh `origin/main` worktree/branch (`newtask`); do not pile new work onto a + stale head that already shares hot files with the open queue. +- Treat `docs/branch-review-ledger.md` and `docs/outstanding-issues.md` as hot shared files. + Both use `merge=union`. Append with `npm run ledger:append` / the `/issues` skill — never + hand-write ledger rows, and never resolve an outstanding-issues conflict by taking one side + wholesale. `npm run check:outstanding-issues` fails on duplicate IDs or a stale + `issues:next-id` marker. +- Before calling GitHub `DIRTY`/`CONFLICTING` a real conflict, run + `git merge-tree --write-tree origin/main `. Clean tree + behind = sync; dirty tree = + real conflict. + +### Speed CI without skipping quality + +- Assemble every commit for a head before the first push, or wait for the current PR CI run + to settle before pushing again. `cancel-in-progress: true` cancels Production UI mid-flight + on every superseding push (~40% of recent PR CI runs were cancellations). +- Before push: `npm run format` **and commit the result**, then + `npm run verify:pr-local` (or the smallest gate that covers the change). Format is in + `static-pr` but not in `verify:cheap`; an uncommitted format leaves CI red on the pushed + blob. Whole-tree Prettier, not a single edited file. +- If a `claude/*` PR has auto-merge armed, disable it before a settle-then-push bundle, push, + then re-enable — otherwise the first green head can squash-merge before the bundled commit + lands. +- Missing CI checks are not a green pass. `pull_request` workflows do not run when GitHub + cannot build `refs/pull//merge`. The `PR mergeability` check uses trusted + `pull_request_target` events and refreshes unchanged PR heads after protected-base + pushes; it fails explicitly on `mergeable_state: dirty`. Behind-but-clean heads still use + `npm run sync:pr-branches` / `:apply` with a human `gh` identity — never bot + `update-branch`. +- Keep Playwright blocking tests at zero retries. Quarantine only after three reproductions + on the same SHA via `tests/flake-ledger.json` (`@quarantine`, not `@critical`, ≤30-day + expiry). Do not weaken tap targets to `min-h-11` to chase generic a11y guidance — that + reintroduces a known `ui-smoke` flake. + +### Operator sync (explicit only) + +- Leave active PRs alone unless the user asks. Report-only inventory: + `npm run sync:pr-branches`. Apply only with confirmation and human/operator `gh` auth: + `npm run sync:pr-branches:apply`. + + + ## Codex productivity defaults diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 036c7dc9b0..d68aac08f7 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -144,6 +144,9 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | PR #1396 / claude/latency-findings-impl-s8g01v | 70e810b66881e17aa9f58126fdad970986bda911 | User ask: resolve comments + Production UI phone-scroll + main sync | FIXED: synced main (DIRTY was staleness); removed union ledger dup; adapted phone-scroll asserts for Answer strategy-overlay + overlay/reserve-only calculator budget + focus pre-scroll inside 8px reveal band. Codex P1s already on tip; 0 unresolved threads. Focused Chromium phone-scroll 9/9 green (system Chrome). | phone-scroll focused 9/9; check:branch-review-ledger PASS; merge-tree clean; prior Codex P1s retained | | 2026-07-30 | HEAD | 13c16cf07c854b50daa35a2ef2a2ea76d5e059e1 | ci-testing-approach | findings: UI-load flake #093 dominates PR reds; schedule full-sentinel blocks release-browser via audit; UI scope overfires on src/app/api; ~40% PR runs cancelled wasting ~12 UI-hrs; CI_TRIAGE inert; eval:rag:offline claimed-in-CI but only fixtures run | gh-ci-500-runs,ci.yml,ci-change-scope,testing.md,process-hardening,outstanding-issues-093-095-097-023,flake-ledger-empty | | 2026-07-30 | cursor/ci-testing-review-1bf5 | 13c16cf07c854b50daa35a2ef2a2ea76d5e059e1 | ci-testing-approach | Corrects the ref cell from the unresolved placeholder "HEAD" to the actual branch name, so ledger:lookup can match this review by branch (Codex P2 finding on PR #1406). | node scripts/branch-review-ledger.mjs lookup cursor/ci-testing-review-1bf5 --scope ci-testing-approach | +| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 2a31fcee0ef2e330a4901481c0810103d89c96cf | process anti-conflict merge readiness | NOT READY. CI green on stale head, but merge-tree vs current main is CONFLICTING in ci.yml, package.json, and add/add on check-outstanding-issues.mjs after #1410 landed a stronger #112 gate. Keep unique value: AGENTS anti-conflict procedure, #116 PR mergeability workflow, merge=union on outstanding-issues (explicitly still open after #1410). Drop duplicate weaker outstanding-issues checker; re-verify after sync. | ledger:lookup NOT REVIEWED; merge-tree dirty vs origin/main; ManagePullRequest CI SUCCESS (15 ok / 0 fail, Production UI skipped as non-UI); local verify:pr-local earlier on pre-conflict head 4467 passed | +| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 387ffd07887f1160fca8fe98c1c4809e852531ae | process anti-conflict merge readiness | READY after sync with main. Kept #1410 structural outstanding-issues gate; added merge=union + runtime attr check; retained #116 PR mergeability workflow and AGENTS anti-conflict playbook; dropped duplicate weaker checker/test. merge-tree clean vs origin/main. | merge-tree clean; check:outstanding-issues pass; check:pr-mergeability pass; check:gate-manifest pass; verify:pr-local pass | +| 2026-07-30 | cursor/process-anti-conflict-speed-1edf / PR #1416 | 38ae07b989e6414026235debad0e429ba64cf462 | process anti-conflict merge readiness | READY at final tip. Same as prior READY plus this ledger append only; merge-tree still clean vs origin/main. | merge-tree clean; verify:pr-local on 387ffd07 parent (4480 passed); tip is ledger-only follow-up | | 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | ad9da6a6f8ba3884b389fa78e678bb88ee72d9d1 | ci-hygiene-gates | implemented matrix unblock, scope narrow, cancelled≠failure, pinned gitleaks, critical-first UI, eval:rag:offline; skipped #093; verify:cheap 4471 pass | verify:cheap,check:ci-scope,check:gitleaks-pinned,check:gate-manifest,eval:rag:offline | | 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | b660dbc5a10d7ca3da03541028017f0abc6b5bd3 | ci-hygiene-gates merge-readiness | findings | check:ci-scope;check:gitleaks-pinned;scope-classify PR files ui_changed=false;sim cancelled-as-neutral | | 2026-07-30 | cursor/ci-hygiene-gates-1bf5 | 8f3283d00da274dee507a1b8e9b611321d1f35be | pr-1413-merge-readiness | READY after main sync + cancel-to-green fix; draft until tip CI green; deferred #093 + CI_TRIAGE_ENABLED confirm | verify:cheap:4481-pass;format:outstanding-issues;merge-tree:clean;cancelled:!cancelled();hosted:awaiting-tip | diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index e54e4b9ae9..ebdc55c8db 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -149,7 +149,6 @@ removed after current-main verification; it is not missing recommended work. | #109 | P2 | issue | Remote sessions clone shallow, silently invalidating all branch/merge analysis | **Outcome:** no session draws branch conclusions from a truncated history. **Detail:** on 2026-07-29 this repo's remote session had `git rev-parse --is-shallow-repository` = **true** with only **74** commits of `origin/main` (full history is 2829). Every merge-base, `--cherry-pick`, and ahead/behind number computed in that state was wrong: local `main` reported `ahead 52` and `refusing to merge unrelated histories` (it is actually 0 ahead with a shared base), and an all-branch sweep wrongly showed **90 of 91** branches as carrying unmerged work. Acting on that would have meant either deleting live branches or abandoning cleanup entirely. `git fetch --unshallow` corrected both. **FIXED 2026-07-29:** `scripts/sweep-branch-ledger.mjs` now refuses outright on a shallow clone via the exported `shallowCloneRefusal`, printing no inventory and exiting 1 in both text and `--json` mode, before the fetch and before any branch maths. `docs/branch-cleanup-guide.md` §Safety Rules gains the `is-shallow-repository` precondition ahead of its numbered steps, because the raw `git` commands it documents have no such guard. Proven in a real `--depth 1` clone: unguarded the sweep exited **0** and named the live checked-out branch a deletion candidate with "no unique patch content"; guarded it exits 1 with the `--unshallow` remedy. Five cases in `tests/repo-hygiene.test.ts` cover both directions, including that the string `"false"` (truthy) must NOT be read as shallow — the way this guard could fail dangerously in reverse. **Hardened in review:** an indeterminate `is-shallow-repository` result (empty output from a swallowed `git` failure) is now refused as its own failure rather than treated as complete, and the same refusal was extended to `scripts/reconciliation-preflight.mjs`, which reports its own merge-base-derived ahead/behind. That guard then had to move OUT of the preflight CLI and INTO the exported `collectReconciliationState`, because `buildReconciliationEvidencePack` calls the collector directly and stamps `status: "complete"`: in a `--depth 1` clone the guarded CLI exited 1 while the evidence-pack command exited 0 and persisted shallow ahead/behind as completed evidence. The collector now throws `UnverifiedHistoryError` (`code: "history-not-verified"`), so every current and future caller fails closed by default instead of by remembering to ask; the CLI catches it only to keep the `--json` envelope. Regression cases live with each entry point (`tests/reconciliation-preflight.test.ts`, `tests/reconciliation-evidence-pack.test.ts`) and build a real `--depth 1` clone, asserting `is-shallow-repository` is `true` first so a git behaviour change cannot make them pass vacuously. **Second failure mode, found in review after the first fix landed: complete history is not complete branch coverage.** `git clone --depth 1` implies `--single-branch`, pinning `remote.origin.fetch` to the one cloned branch; `git fetch --unshallow` converts the history so `--is-shallow-repository` reads `false` and the shallow guard passes, but it does not widen the refspec, and an ordinary `git fetch origin` respects the narrow one. Measured in a `main`+`feature` fixture: after unshallowing, `git ls-remote --heads origin` listed both while `refs/remotes/origin` held only `origin/main`, and the sweep exited **0** reporting `"branches": []` — and an empty inventory is not a safe failure, since it reads as "nothing to clean up" and a missing `origin/main` makes every `rev-list` fail into `0/0`, i.e. every branch a deletion candidate. Fixed both ways: the sweep's fetch now passes an explicit `+refs/heads/*:refs/remotes/origin/*` (repairing coverage without rewriting the operator's config), and `branchCoverageRefusal` refuses when neither the configured refspec nor a completed wildcard fetch establishes coverage — `--no-fetch`, offline, or a failed fetch. Its remedy is deliberately `git remote set-branches origin '*'`, not `--unshallow`, which fixes history and does nothing here. **Two further routes to the same empty-inventory answer, both found in review, both from checking only half of the refspec.** (1) The DESTINATION matters as much as the source, because the sweep enumerates `refs/remotes/origin` and nothing else: with `+refs/heads/*:refs/remotes/upstream/*`, `refs/remotes/upstream` held `upstream/main` and `upstream/feature` while `refs/remotes/origin` stayed empty and the sweep exited **0** with `"branches": []`. (2) Git substitutes the matched suffix into ``, so a `refs/*` source nests one level deeper: `+refs/*:refs/remotes/origin/*` writes `refs/remotes/origin/heads/main`, `origin/main` then does not resolve at all, every comparison fails into `0/0`, and the sweep exited **0** naming both `heads/feature` and **`heads/main`** as deletion candidates — a green run recommending the deletion of `main`. Coverage from config therefore requires exactly `refs/heads/*` to `refs/remotes/origin/*`; a completed wildcard fetch still establishes coverage by itself, since the sweep passes that destination explicitly. **Stop:** never delete a branch, or report a branch as unmerged, from a shallow clone, a single-branch refspec, or a refspec whose destination is not `refs/remotes/origin/*`. | session 2026-07-29; `docs/branch-cleanup-guide.md`; `scripts/sweep-branch-ledger.mjs` | 2026-07-29 | | #110 | P3 | task | Design-system project token manifest lags its stylesheet | **Outcome:** the claude.ai/design token panel matches the shipped stylesheet. **Detail:** PR #1375 pushed a recompiled `_ds_bundle.css` (Clinical Sky, `--e0`–`--e4`, 4px radius grid, `--tracking-eyebrow`/`--leading-display`/`--leading-prose`) plus the four changed guideline docs to project `08d6f126`, but `_ds_manifest.json` is converter-generated and still advertises `--text-4xs: 0.5rem`, the old `--radius-lg/xl/2xl` values, and `--tw-leading`/`--tw-tracking` entries scoped to the retired `.leading-[…]` / `.tracking-[0.08em]` utilities. Rendering is correct; only the token inventory lags. Hand-editing was rejected — `kind`/`scope`/`annotation` are converter heuristics and a wrong panel is worse than a stale one. **Next:** in a session with the `/design-sync` skill, `npm ci`, then `npm install --prefix .ds-sync --no-save --package-lock=false esbuild ts-morph @types/react @tailwindcss/cli geist`, read `.design-sync/NOTES.md`, and run `resync.mjs --remote` so bundle and manifest regenerate together. **Stop:** do not hand-author `_ds_manifest.json`; the converter is not a published npm package and ships with the skill. | PR #1375; `.design-sync/NOTES.md`; project `08d6f126` (`_ds_needs_recompile` marker present) | 2026-07-29 | | #115 | P3 | rec | Band adoption gate treats a discovered import as rendered | **Closed 2026-07-30.** `tests/search-results-band-adoption.test.ts` no longer asks "does this file mention the band?" but "does anything the route actually mounts reach it?". It parses each module with `@babel/parser` into a small graph — exported name to local declaration, local to the identifiers its body references, and which locals render the band — then walks from the route's default export, carrying at each hop the set of exports the importer mounts. So a static `import { X }` is followed only when `X` is reachable from a mounted declaration; `dynamic(() => import("…").then((m) => m.Named))` follows only that binding, which is how the code-split dashboard workspaces are written; a bare `import "…"` is not followed at all; `export { X } from "…"` is followed only when the importer wants `X`; and `export * from "…"` never supplies a default, so a page whose importer wants only the default gets no hop from it. **Why the redesign rather than more patches:** six false greens were reported in one day (unrendered import, `export { X }`, `export { X } from`, `export *`, bare side-effect import, JSX in an unmounted helper, and a lazy import reaching every sibling export), all one defect — presence is not reach. Two of the six were introduced by an earlier patch to the same walker. **Verified:** all five production search routes still reach the band; gutting `(search-app)/services/page.tsx` and `tools/page.tsx` to `
` each reports an orphan; fourteen temp-dir fixtures cover both directions, and the two guarding the new mechanisms were confirmed to fail against the prior behaviour by targeted mutation (presence-based band check, and following bare imports). Residual: reachability is per module, so a mounted declaration referencing an identifier anywhere in its body counts, and control flow inside it is not modelled. | PR #1400; session 2026-07-30 | 2026-07-30 | -| #116 | P2 | issue | An unmergeable PR runs no CI at all, with no signal that it stopped | **Outcome:** a PR that has silently stopped being tested says so. **Detail:** on 2026-07-30 PR #1400 ran **no** `CI`, `Gitleaks` or `Semgrep` workflow across three consecutive pushes, and nothing anywhere said why. Cause: the PR had a real content conflict with `main`, so GitHub could not build `refs/pull/1400/merge`, and every `pull_request`-triggered workflow is skipped in that state. `pull_request_target` ones (`PR Policy`) still ran, and CircleCI posted failure ~3s after each push - faster than a checkout, so no step executed. The symptom reads as "CI is broken" or "my tests fail", and about 40 minutes went into replicating CI steps locally (all green) before the cause was found. **Diagnostic:** compare check-run counts with another open PR (3 vs 16-19), list workflow runs and note only `pull_request_target` fired, then confirm with `git merge-tree --write-tree origin/main HEAD`. **Next:** make staleness visible instead of silent - e.g. a `pull_request_target` job that fails when `mergeable_state` is `dirty`, so a conflicted PR shows one red check naming the conflict rather than an empty check list. `npm run sync:pr-branches` covers the behind-but-clean case only. **Stop:** never conclude "CI is failing" from a missing check - the absence of a check is not a failing check. | PR #1400; session 2026-07-30 | 2026-07-30 | | #117 | P2 | rec | Therapy Compass catalogue payload is the mobile LCP outlier | **Outcome:** `/therapy-compass` mobile LCP lands near the other mobile routes instead of double them. **Measured 2026-07-30** by the new pre-merge Lighthouse budget: mobile LCP 5229 ms, TBT 612 ms, CLS 0.142, against 2123-2460 ms on every other mobile route and 826 ms on desktop — so it is client-side work under mobile CPU/network throttling, not server latency. **Cause:** `useTherapyData` fetches `public/therapy-compass-data/therapies-index.json` (690 KB raw, 139 KB gzipped, 205 records x 16 fields) for the home/search/pathways screens, so the download plus JSON parse sits on the critical path before content paints. 90% of that weight is long-form clinical prose — indications 159 KB (26%), contraindicationsOrCautions 139 KB (23%), bestUsedFor 73 KB (12%), clinicalSummary 67 KB (11%), patientPopulation 59 KB (10%), targetSymptoms 48 KB (8%) — while name, slug, category, tags and setting together are 54 KB (7%). **Blocked on one decision per field group: rendered on the card, matched by search, or neither.** `therapy-card.tsx` references five of those prose fields and the same index feeds the search screen, so stripping fields could silently change clinical display or search recall. **Next:** settle that per-field question, then either pre-truncate prose that only feeds card display, or move search matching server-side / load prose on first keystroke. **Gate:** `check:therapy-data-index` plus the therapy Playwright journeys; re-measure with `npm run verify:lighthouse`. **Stop:** do not drop a field from the catalogue payload without confirming no card renders it and no search path matches on it. Same class as #013 (route-chunk / catalogue JSON weight), different route and now measured. | session 2026-07-30 Lighthouse budget first run; PR #1404 | 2026-07-30 | | #118 | P2 | task | Adopt the visual and Lighthouse baselines so the two new gates actually gate | **Outcome:** `visual-baseline` and `lighthouse-budget` stop reporting and start blocking. **Detail:** PR #1404 added both as `continue-on-error` jobs outside `pr-required`, deliberately. `tests/ui-visual-baseline.spec.ts` has no committed baselines, so all six targets fail with a missing-snapshot error by design; the job uploads them on every run (run 30513537912, artifact 8748062487, 31 files). `lighthouse-budget.json` ships `enforce: false` with `baseline: null`, so the grader warns rather than grades. **Next:** (1) download that artifact, review the six PNGs and commit them under the platform-scoped screenshots directory that `playwright.visual.config.ts` names in its `snapshotPathTemplate` — from CI, never a developer machine, because font hinting differs between them; (2) run `npm run check:lighthouse-budget -- --update` against a known-good CI build and flip `enforce`, but not before #117 or the baseline pins a known-slow route; (3) then add each job to `pr-required` and drop `continue-on-error` in the same edit. **Also:** PR #1404 added the first rendered-effect contract for #094, but 37 of the 38 unlayered visual classes still carry exemptions in `tests/helpers/style-contracts.ts` rather than contracts; and `scripts/run-lighthouse-budget.mjs` duplicates about 50 lines of the isolated-server boot in `scripts/run-playwright.mjs`, deferred to avoid destabilising the required UI gate in the same change. **Stop:** do not make a missing baseline skip instead of fail — that is the soft-skip-green pattern `AGENTS.md` forbids. | session 2026-07-30; PR #1404 | 2026-07-30 | | #119 | P2 | issue | `ci/circleci: verify` is failing repo-wide | **Outcome:** the CircleCI status is trustworthy again. **Detail:** it failed on every head of PR #1404 (builds 646, 654, 658, 668) and also on PR #1400, which changes two lines of markdown — so it is not any one diff. Every command `.circleci/config.yml` runs is green under GitHub Actions on the same commits: `static-pr` runs the same `format:check`, `lint` and `typecheck`, the `coverage` job runs the same unit suite, and CircleCI's own pre-step `node scripts/ci-change-scope.mjs --base origin/main --head HEAD` exits 0 locally. That leaves the steps unique to CircleCI — the `cimg/node:24.18` executor and its engine asserts, `npm ci` inside that image, and the `apt-get` plus `python3 -m venv` plus pinned `PyMuPDF==1.28.0` bootstrap that exports `PYTHON_BIN` for the test run. Checked 2026-07-30: the `PyMuPDF==1.28.0` pin is valid on PyPI — latest, not yanked, wheels plus an sdist — so a bad pin is NOT the cause; if that step is still implicated it would be a build-from-sdist failure under the image's Python, not a resolution failure. A docs-only commit (`a6f5fa6`, one markdown file) also failed, so the result is invariant to what is pushed. **Next:** someone with CircleCI log access must read build 672; no session here holds those credentials. Overlaps the CI-health review in PR #1406. **Stop:** do not treat a red CircleCI status as evidence about a branch's own diff until this is resolved. | session 2026-07-30 PR #1404 CI triage; PR #1400; PR #1406 | 2026-07-30 | @@ -166,6 +165,7 @@ Move resolved rows here with the resolution date and a one-line outcome. Keep th | ID | Type | Summary | Outcome | Resolved | | ---- | ----- | ---------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| #116 | issue | Missing CI signal on conflicted PRs | RESOLVED 2026-07-30: `.github/workflows/pr-mergeability.yml` checks trusted `pull_request_target` events and refreshes unchanged heads after protected-base pushes. A conflicted PR gets a red `PR mergeability` check naming the conflict instead of an empty CI list. The base-push job alone has scoped `checks: write`; neither path checks out PR code or updates branches. Classifier: `scripts/pr-mergeability.mjs`; contract: `npm run check:pr-mergeability`. Behind-but-clean remains `sync:pr-branches`. | 2026-07-30 | | #088 | task | Union-driver ledger duplication watch after repair | CLOSED 2026-07-30. Post-repair merges take main's repaired lines; residual exact-dupe babysit twins from stock `merge=union` are addressed by the custom `merge=ledger` driver (union + exact-row dedupe), `npm run ledger:dedupe`, and the Run PR anti-churn ledger policy. Success criterion (three consecutive post-repair guard passes) met on ordinary main syncs; ongoing exact-dupe class is now gated rather than watched. | 2026-07-30 | | #087 | issue | `npm run check:knip` reported pre-existing dependency findings | NOT A DEFECT — false positive. The findings (unused `rimraf`/`tsx`/`@testing-library/dom`, unlisted `playwright-core`) only appeared because the worktree had no `node_modules` of its own and tooling resolved from the parent checkout. After `npm ci` in the same worktree, `npm run check:knip` exits 0 with no ignore-list change. Durable lesson: never act on a knip finding from a worktree that has not been installed. | 2026-07-28 | | #089 | issue | Branch-review-ledger hygiene deferred from PR #1275 | Closed by the 2026-07-28 hygiene pass. The MD056 cell-count and duplicate-row findings dispositioned as "belongs in a dedicated main ledger hygiene pass" on PR #1275 are fixed: 140 mojibake lines restored byte-exact from git history, 6 residual separators repaired, 46 exact duplicates removed, 21 wrong-width rows normalised, and 4 heading+bullet records converted to table rows — 1067 records, all six cells. Root cause closed too: `npm run ledger:lookup` / `ledger:append` (`scripts/branch-review-ledger.mjs`) replace hand-written rows, and `check:branch-review-ledger` now fails on mojibake, cell width, heading records, impossible dates, table gaps, and (from 2026-07-29) unresolvable HEADs and near-duplicates. | 2026-07-28 | diff --git a/docs/process-hardening.md b/docs/process-hardening.md index bfdc0b45f8..527b8f5180 100644 --- a/docs/process-hardening.md +++ b/docs/process-hardening.md @@ -80,6 +80,24 @@ artifact before release; see deliberate "1 PR per work order" convention for tracked staged rollouts (maturity backlog, `#086`) or anything crossing a clinical-risk/RAG-ranking-surface path. +## Anti-conflict and silent-CI signal (2026-07-30) + +- **Operating procedure:** AGENTS.md "Anti-conflict and CI-speed operating procedure". + Future-process only — do not mutate unrelated active PRs unless explicitly asked. +- **Outstanding-issues concurrency (`#112`):** the structural gate landed in PR #1410 + (`npm run check:outstanding-issues` in `verify:cheap` / CI `static-pr` — duplicate IDs, + both-tables, stale `issues:next-id`, malformed rows). PR #1416 adds `merge=union` in + `.gitattributes` and a runtime attribute check so concurrent appends keep both sides' + rows; union merge still cannot allocate unique IDs, so the structural gate remains + required. +- **Silent CI on conflicted PRs (`#116`):** when GitHub cannot build + `refs/pull//merge`, every `pull_request` workflow is skipped with no failing check. + `.github/workflows/pr-mergeability.yml` checks trusted `pull_request_target` events and + uses a protected-base `push` sweep to publish a fresh `PR mergeability` check on each + unchanged open-PR head. Only that sweep receives job-scoped `checks: write`; neither path + checks out PR code or updates branches. Behind-but-clean heads remain an operator + `sync:pr-branches` concern. Contract: `npm run check:pr-mergeability`. + ## Phase 1 - Active now - `npm run verify:cheap` is the default broad local gate for source/config/test changes: `check:runtime`, `sitemap:check`, lint, typecheck, and unit tests. diff --git a/docs/scripts-index.md b/docs/scripts-index.md index 34a8cef2d4..fcd0b25993 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -11,21 +11,23 @@ migration has shipped (see `docs/maturity-backlog-workorders.md` L1). ## Runner & guard infrastructure [infra] -| Script | Role | -| ------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------- | -| `run-heavy.mjs` | Acquires shared/exclusive cross-worktree leases (`test-run-lock.mjs`) so focused checks can overlap safely | -| `run-tsx.mjs`, `run-vitest.mjs`, `run-playwright.mjs`, `run-eval-safe.mjs` | Typed/test/e2e/eval entrypoint wrappers | -| `dev-free-port.mjs`, `ensure-local-server.mjs` | Project-stable localhost port selection + background server ensure | -| `check-node-engine.cjs`, `install-git-hooks.mjs`, `guard-push.mjs`, `guard-next-build.mjs` | Install/preflight guards | -| `ci-change-scope.mjs`, `ci-triage.mjs`, `pr-policy.mjs` | CI change classification + PR policy (self-tested via `check:ci-scope`/`check:ci-triage`/`check:pr-policy`) | -| `check-installed-lock-parity.mjs`, `phone-chrome-plan.mjs`, `verify-phone-chrome.mjs` | Lock-trust preflight plus change-scoped phone contracts, ownership journeys, and smart full-UI escalation | -| `final-merge-audit.mjs` | Fail-closed local merge-tree audit; explicit provider mode adds PR/check/thread/tree/deployment proof | -| `child-process-result.mjs`, `cli-utils.ts`, `productivity-core.mjs` | Shared helpers | +| Script | Role | +| ------------------------------------------------------------------------------------------ | ----------------------------------------------------------------------------------------------------------------------------------------------------- | +| `run-heavy.mjs` | Acquires shared/exclusive cross-worktree leases (`test-run-lock.mjs`) so focused checks can overlap safely | +| `run-tsx.mjs`, `run-vitest.mjs`, `run-playwright.mjs`, `run-eval-safe.mjs` | Typed/test/e2e/eval entrypoint wrappers | +| `dev-free-port.mjs`, `ensure-local-server.mjs` | Project-stable localhost port selection + background server ensure | +| `check-node-engine.cjs`, `install-git-hooks.mjs`, `guard-push.mjs`, `guard-next-build.mjs` | Install/preflight guards | +| `ci-change-scope.mjs`, `ci-triage.mjs`, `pr-policy.mjs`, `pr-mergeability.mjs` | CI change classification + PR policy + conflict signal (self-tested via `check:ci-scope`/`check:ci-triage`/`check:pr-policy`/`check:pr-mergeability`) | +| `check-outstanding-issues.mjs`, `check-pr-mergeability-workflow.mjs` | Outstanding-issues ID/marker/union guard + PR mergeability workflow contract | +| `check-installed-lock-parity.mjs`, `phone-chrome-plan.mjs`, `verify-phone-chrome.mjs` | Lock-trust preflight plus change-scoped phone contracts, ownership journeys, and smart full-UI escalation | +| `final-merge-audit.mjs` | Fail-closed local merge-tree audit; explicit provider mode adds PR/check/thread/tree/deployment proof | +| `child-process-result.mjs`, `cli-utils.ts`, `productivity-core.mjs` | Shared helpers | ## Verification gates [live] `verify:cheap` → `verify:pr-local` → `verify:ui` → `verify:release`. Building blocks: `check-runtime.ts`, `check-github-action-pins.mjs`, `check-gate-manifest.mjs`, +`check-outstanding-issues.mjs`, `check-pr-mergeability-workflow.mjs`, `check-maintainability-budgets.mjs`, `check-codebase-index-coverage.mjs`, `check-docs-links.mjs`, `check-docs-script-refs.mjs`, `check-bundle-budget.mjs`, `check-type-scale.mjs`, `check-icon-scale.mjs`, `check-design-system-contract.mjs`, `check-function-grants.mjs`, diff --git a/package.json b/package.json index 05cd0988c4..de5da2ef2c 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "clean:worktree": "node scripts/clean-worktree.mjs", "verify:preflight": "npm run check:installed-lock-parity && npm run typecheck && npm run verify:cheap && npm run clean:worktree", "verify:cheap": "npm run verify:cheap:internal", - "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:gitleaks-pinned && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run sitemap:check && npm run docs:check-index && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", + "verify:cheap:internal": "npm run check:runtime && npm run check:installed-lock-parity && npm run check:github-actions && npm run check:ci-scope && npm run check:gitleaks-pinned && npm run check:ci-triage && npm run check:pr-policy && npm run check:gate-manifest && npm run check:branch-review-ledger && npm run check:outstanding-issues && npm run check:pr-mergeability && npm run sitemap:check && npm run docs:check-index && npm run docs:check-scripts && npm run docs:check-links && npm run check:knip && npm run check:maintainability-budgets && npm run brand:check && npm run check:assets && npm run check:therapy-data-index && npm run check:cross-mode-index && npm run check:type-scale && npm run check:icon-scale && npm run check:design-system-contract && npm run check:migration-role && npm run check:function-grants && npm run check:owner-scope && npm run lint && npm run typecheck && npm run test", "verify:pr-local": "node scripts/verify-pr-local.mjs", "verify:phone-chrome": "node scripts/verify-phone-chrome.mjs", "audit:final-merge": "node scripts/final-merge-audit.mjs", @@ -210,7 +210,8 @@ "drift:manifest": "node scripts/run-tsx.mjs scripts/generate-drift-manifest.ts", "sync:pr-branches": "node scripts/sync-open-pr-branches.mjs", "sync:pr-branches:apply": "node scripts/sync-open-pr-branches.mjs --apply", - "check:assets": "node scripts/check-assets.mjs" + "check:assets": "node scripts/check-assets.mjs", + "check:pr-mergeability": "node scripts/pr-mergeability.mjs --self-test && node scripts/check-pr-mergeability-workflow.mjs" }, "dependencies": { "@next/env": "16.2.12", diff --git a/scripts/branch-review-ledger.mjs b/scripts/branch-review-ledger.mjs index 1e2a4590ec..bd3bf8a963 100644 --- a/scripts/branch-review-ledger.mjs +++ b/scripts/branch-review-ledger.mjs @@ -66,10 +66,15 @@ export function dedupeLedgerMarkdown(markdown) { } /** - * Union ours + theirs dated records (ours order first), drop exact duplicate rows, - * and keep ours preamble (header/contract prose). Used by the `merge.ledger` driver. + * Three-way merge dated records while preserving intentional deletions. + * + * Rows added independently on either side are unioned. A row that existed in the + * merge base is retained only while both tips still contain it, so rotating rows + * out of the live ledger on one side cannot be undone by a stale branch. Preamble + * and trailing prose use ordinary three-way semantics and fail on divergent edits. + * Used by the `merge.ledger` driver. */ -export function mergeLedgerMarkdown(ours, theirs) { +export function mergeLedgerMarkdown(base, ours, theirs) { const split = (markdown) => { const lines = markdown.split(/\r?\n/); const preamble = []; @@ -88,17 +93,34 @@ export function mergeLedgerMarkdown(ours, theirs) { return { preamble, records, trailing }; }; + const ancestor = split(base); const a = split(ours); const b = split(theirs); + const baseRecords = new Set(ancestor.records); + const oursRecords = new Set(a.records); + const theirsRecords = new Set(b.records); const seen = new Set(); const records = []; + const keep = (line) => !baseRecords.has(line) || (oursRecords.has(line) && theirsRecords.has(line)); for (const line of [...a.records, ...b.records]) { + if (!keep(line)) continue; if (seen.has(line)) continue; seen.add(line); records.push(line); } - const preamble = a.preamble.length > 0 ? a.preamble : b.preamble; - const trailing = a.trailing.length > 0 ? a.trailing : b.trailing; + + const chooseSection = (label, baseSection, oursSection, theirsSection) => { + const ancestorText = baseSection.join("\n"); + const oursText = oursSection.join("\n"); + const theirsText = theirsSection.join("\n"); + if (oursText === theirsText) return oursSection; + if (oursText === ancestorText) return theirsSection; + if (theirsText === ancestorText) return oursSection; + throw new Error(`ledger ${label} changed differently on both sides`); + }; + + const preamble = chooseSection("preamble", ancestor.preamble, a.preamble, b.preamble); + const trailing = chooseSection("trailing content", ancestor.trailing, a.trailing, b.trailing); let markdown = [...preamble, ...records, ...trailing].join("\n"); if (!markdown.endsWith("\n")) markdown += "\n"; return { markdown, recordCount: records.length }; @@ -206,7 +228,9 @@ export function rotateLedgerMarkdown(liveMarkdown, { before, existingArchives = const archives = []; for (const [label, moved] of [...byLabel.entries()].sort(([a], [b]) => a.localeCompare(b))) { - const relative = path.join(ARCHIVE_DIR, `${ARCHIVE_PREFIX}${label}.md`); + // Repository-relative paths are serialized into docs and command output, so + // keep them stable across Windows and POSIX hosts. + const relative = path.posix.join(ARCHIVE_DIR, `${ARCHIVE_PREFIX}${label}.md`); const prior = existingArchives.get(relative) ?? ""; const priorRecords = prior ? splitLedgerMarkdown(prior).records : []; const seen = new Set(priorRecords); @@ -673,6 +697,7 @@ function selfTest() { const dedupedRows = dedupeLedgerMarkdown(`${preamble}\n${twinRow}\n${twinRow}\n`); assert(dedupedRows.removed === 1 && dedupedRows.kept === 1, "dedupe drops exact twin"); const mergedTips = mergeLedgerMarkdown( + `${preamble}\n${twinRow}\n`, `${preamble}\n${twinRow}\n`, `${preamble}\n${twinRow}\n| 2026-07-28 | y | ${"f".repeat(40)} | s | o | c |\n`, ); diff --git a/scripts/check-outstanding-issues.mjs b/scripts/check-outstanding-issues.mjs index 9d2ad9ef88..3d681fee66 100644 --- a/scripts/check-outstanding-issues.mjs +++ b/scripts/check-outstanding-issues.mjs @@ -2,13 +2,13 @@ // Structural gate for docs/outstanding-issues.md. // // Ledger #112. The `issues:next-id` marker is a plain HTML comment that every -// editor read-modify-writes with no lock, and this file — unlike -// docs/branch-review-ledger.md — has NO `merge=union` driver. So two agents -// allocating in the same hour collide, and the collision surfaces as an -// ordinary content conflict that a hurried resolution can settle by taking one -// side wholesale and dropping the other's rows. On 2026-07-29 that happened -// three times in one hour on a single PR, and nothing noticed: no gate read -// this file's structure at all. +// editor read-modify-writes with no lock. The file now has `merge=union` (PR +// #1416), which preserves concurrent row appends the same way as +// docs/branch-review-ledger.md, but union merge cannot allocate unique IDs — +// two agents can still collide, and a hurried conflict resolution can still +// take one side wholesale. On 2026-07-29 that happened three times in one hour +// on a single PR, and nothing noticed: no gate read this file's structure at +// all. This structural gate is what makes those failures loud. // // This makes each of those failures loud: // - an id used twice is a merge that kept both sides' rows under one number @@ -21,6 +21,7 @@ // is right, because that is a judgement a gate cannot make and pretending // otherwise would make the gate noisy enough to be ignored. +import { execFileSync } from "node:child_process"; import { readFileSync } from "node:fs"; export const ISSUES_PATH = "docs/outstanding-issues.md"; @@ -403,6 +404,14 @@ function selfTest() { console.log("outstanding-issues self-test passed."); } +function effectiveMergeAttribute() { + const output = execFileSync("git", ["check-attr", "merge", "--", ISSUES_PATH], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }).trim(); + return output.match(/:\s*merge:\s*(\S+)$/)?.[1] ?? ""; +} + function main() { if (process.argv.includes("--self-test")) { selfTest(); @@ -410,6 +419,13 @@ function main() { } const markdown = readFileSync(ISSUES_PATH, "utf8"); const problems = checkIssues(markdown); + const mergeAttribute = effectiveMergeAttribute(); + if (mergeAttribute !== "union") { + problems.push( + `${ISSUES_PATH} must resolve to merge=union (found ${JSON.stringify(mergeAttribute || "unset")}) — ` + + "set it in .gitattributes so concurrent appends keep both sides' rows", + ); + } if (problems.length > 0) { console.error(`${ISSUES_PATH} check FAILED:`); for (const problem of problems) console.error(` - ${problem}`); @@ -423,7 +439,7 @@ function main() { const open = rows.filter((row) => row.table === "open").length; console.log( `Outstanding-issues guard passed: ${rows.length} rows (${open} open, ${rows.length - open} archived), ` + - `unique ids, next-id=${nextId} above the highest.`, + `unique ids, next-id=${nextId} above the highest, union merge active.`, ); } diff --git a/scripts/check-pr-mergeability-workflow.mjs b/scripts/check-pr-mergeability-workflow.mjs new file mode 100644 index 0000000000..1c77b7d0f8 --- /dev/null +++ b/scripts/check-pr-mergeability-workflow.mjs @@ -0,0 +1,159 @@ +#!/usr/bin/env node +/** + * Contract guard for .github/workflows/pr-mergeability.yml. + * Mirrors the trusted pull_request_target shape used by pr-policy.yml so the + * conflict signal never executes PR-head code or mutates branches. + */ +import fs from "node:fs"; +import { yamlBlock } from "./yaml-contract.mjs"; + +const workflowPath = ".github/workflows/pr-mergeability.yml"; +const workflow = fs.readFileSync(workflowPath, "utf8"); +const githubScriptPin = "3a2844b7e9c422d3c10d287c895573f7108da1b3"; +const checkoutPin = "3d3c42e5aac5ba805825da76410c181273ba90b1"; +const failures = []; + +function collectCheckoutRefs(block) { + return block + .split(/\r?\n/) + .map((line) => line.match(/^\s+ref:\s*(.+?)\s*(?:#.*)?$/)?.[1]?.trim()) + .filter(Boolean); +} + +if (!/^on:\s*$/m.test(workflow) || !workflow.includes("pull_request_target:")) { + failures.push("pr-mergeability.yml must trigger on pull_request_target."); +} +if (!/^\s{2}push:\s*$/m.test(workflow) || !workflow.includes('branches: [main, "release/**"]')) { + failures.push("pr-mergeability.yml must retrigger on protected-base pushes."); +} +if (/^\s*pull_request:\s*$/m.test(workflow)) { + failures.push("pr-mergeability.yml must not use pull_request (that event is skipped when the merge ref is missing)."); +} +if ( + !/^permissions:\s*$/m.test(workflow) || + !workflow.includes("contents: read") || + !workflow.includes("pull-requests: read") +) { + failures.push("pr-mergeability.yml must declare read-only workflow permissions."); +} +if (/pull-requests:\s*write/.test(workflow) || /contents:\s*write/.test(workflow)) { + failures.push("pr-mergeability.yml must not request write permissions."); +} +if ((workflow.match(/checks:\s*write/g) ?? []).length !== 1) { + failures.push("pr-mergeability.yml must grant checks:write exactly once, on the base-push job."); +} +if (/updateBranch|update-branch|sync:pr-branches:apply|sync-open-pr-branches\.mjs\s+--apply/.test(workflow)) { + failures.push("pr-mergeability.yml must not mutate PR branches."); +} + +const job = yamlBlock(workflow, "mergeability:", 2); +if (!job) { + failures.push("pr-mergeability.yml is missing the mergeability job."); +} else { + if (!/if:\s*github\.event_name == 'pull_request_target'/.test(job)) { + failures.push("pr-mergeability job must run only for pull_request_target events."); + } + if (!/runs-on:\s*ubuntu-24\.04/.test(job)) { + failures.push("pr-mergeability job must pin runs-on to ubuntu-24.04."); + } + const checkoutStep = yamlBlock(job, "- name: Checkout trusted classifier", 6); + if (!checkoutStep) { + failures.push("pr-mergeability job is missing the trusted checkout step."); + } else { + if (!checkoutStep.includes(`uses: actions/checkout@${checkoutPin}`)) { + failures.push("pr-mergeability checkout must use the pinned actions/checkout SHA."); + } + const refs = collectCheckoutRefs(checkoutStep); + if (!refs.includes("${{ github.workflow_sha }}")) { + failures.push("pr-mergeability checkout must use github.workflow_sha."); + } + if (refs.some((ref) => /pull_request\.head|base_ref|pull_request\.base\.sha/.test(ref))) { + failures.push("pr-mergeability checkout must not use untrusted PR refs."); + } + if (!/persist-credentials:\s*false/.test(checkoutStep) || /persist-credentials:\s*true/.test(checkoutStep)) { + failures.push("pr-mergeability checkout must set persist-credentials: false."); + } + } + + const signalStep = yamlBlock(job, "- name: Signal real merge conflicts", 6); + if (!signalStep) { + failures.push("pr-mergeability job is missing the conflict signal step."); + } else { + if (!signalStep.includes(`uses: actions/github-script@${githubScriptPin} # v9.0.0`)) { + failures.push("pr-mergeability signal step must use the pinned github-script action."); + } + if (!signalStep.includes("GITHUB_WORKSPACE}/scripts/pr-mergeability.mjs")) { + failures.push("pr-mergeability signal step must import scripts/pr-mergeability.mjs from the trusted checkout."); + } + if (!signalStep.includes("github.rest.pulls.get")) { + failures.push("pr-mergeability signal step must refresh PR metadata before classifying."); + } + if (!signalStep.includes("classifyMergeability")) { + failures.push("pr-mergeability signal step must call classifyMergeability."); + } + } +} + +const basePushJob = yamlBlock(workflow, "refresh-after-base-push:", 2); +if (!basePushJob) { + failures.push("pr-mergeability.yml is missing the protected-base refresh job."); +} else { + if (!/if:\s*github\.event_name == 'push'/.test(basePushJob)) { + failures.push("protected-base refresh job must run only for push events."); + } + if (!/runs-on:\s*ubuntu-24\.04/.test(basePushJob)) { + failures.push("protected-base refresh job must pin runs-on to ubuntu-24.04."); + } + if (!/permissions:[\s\S]*?contents:\s*read[\s\S]*?pull-requests:\s*read[\s\S]*?checks:\s*write/.test(basePushJob)) { + failures.push("protected-base refresh job must scope checks:write to that job only."); + } + if (/contents:\s*write|pull-requests:\s*write/.test(basePushJob)) { + failures.push("protected-base refresh job must not write contents or pull requests."); + } + + const checkoutStep = yamlBlock(basePushJob, "- name: Checkout trusted base classifier", 6); + if (!checkoutStep) { + failures.push("protected-base refresh job is missing the trusted checkout step."); + } else { + if (!checkoutStep.includes(`uses: actions/checkout@${checkoutPin}`)) { + failures.push("protected-base refresh checkout must use the pinned actions/checkout SHA."); + } + const refs = collectCheckoutRefs(checkoutStep); + if (!refs.includes("${{ github.sha }}")) { + failures.push("protected-base refresh checkout must use the pushed base SHA."); + } + if (refs.some((ref) => /pull_request\.head|head\.sha/.test(ref))) { + failures.push("protected-base refresh checkout must not use a PR head ref."); + } + if (!/persist-credentials:\s*false/.test(checkoutStep)) { + failures.push("protected-base refresh checkout must set persist-credentials: false."); + } + } + + const refreshStep = yamlBlock(basePushJob, "- name: Refresh unchanged PR heads", 6); + if (!refreshStep) { + failures.push("protected-base refresh job is missing the PR refresh step."); + } else { + for (const required of [ + `uses: actions/github-script@${githubScriptPin} # v9.0.0`, + "github.rest.pulls.list", + "github.rest.pulls.get", + "classifyMergeability", + "github.rest.checks.create", + 'name: "PR mergeability"', + "head_sha: latestPr.head.sha", + ]) { + if (!refreshStep.includes(required)) { + failures.push(`protected-base refresh step is missing ${JSON.stringify(required)}.`); + } + } + } +} + +if (failures.length > 0) { + console.error("PR mergeability workflow guard failed:"); + for (const failure of failures) console.error(`- ${failure}`); + process.exit(1); +} + +console.log("PR mergeability workflow guard passed."); diff --git a/scripts/merge-branch-review-ledger.mjs b/scripts/merge-branch-review-ledger.mjs index 9ea150516b..9b37c7b711 100644 --- a/scripts/merge-branch-review-ledger.mjs +++ b/scripts/merge-branch-review-ledger.mjs @@ -3,10 +3,10 @@ * Git merge driver for docs/branch-review-ledger.md. * * Stock `merge=union` keeps concurrent appends, but when the same babysit row already - * exists on both tips it also keeps a byte-identical twin — which then fails - * `check:branch-review-ledger` and forces a ledger-hygiene follow-up commit on every - * open PR that syncs main. This driver unions both sides and drops exact duplicate - * dated records (the ledger contract has always allowed that). + * exists on both tips it also keeps a byte-identical twin. A two-tip replacement is + * unsafe too: it reintroduces rows intentionally rotated out of the live ledger by + * the other side. This driver uses the merge base to preserve independent appends, + * exact-row deletions, and unambiguous prose changes. * * Installed by `scripts/install-git-hooks.mjs` as `merge.ledger.driver`. * Git invokes: node scripts/merge-branch-review-ledger.mjs %O %A %B @@ -33,9 +33,10 @@ function selfTest() { const b = "b".repeat(40); const c = "c".repeat(40); + const base = `${preamble}\n${row(a)}\n`; const ours = `${preamble}\n${row(a)}\n${row(b)}\n`; const theirs = `${preamble}\n${row(a)}\n${row(c)}\n`; - const merged = mergeLedgerMarkdown(ours, theirs); + const merged = mergeLedgerMarkdown(base, ours, theirs); assert(merged.recordCount === 3, "keeps three distinct rows"); assert(merged.markdown.includes(row(a)), "keeps shared row once"); assert(merged.markdown.includes(row(b)), "keeps ours-only row"); @@ -63,11 +64,14 @@ function main() { process.exit(2); } - // Ancestor is unused for content: append-only union+dedupe only needs both tips. - void basePath; + if (!basePath) { + console.error("merge base path is required"); + process.exit(2); + } + const base = readFileSync(basePath, "utf8"); const ours = readFileSync(oursPath, "utf8"); const theirs = readFileSync(theirsPath, "utf8"); - const { markdown } = mergeLedgerMarkdown(ours, theirs); + const { markdown } = mergeLedgerMarkdown(base, ours, theirs); writeFileSync(oursPath, markdown, "utf8"); process.exit(0); } diff --git a/scripts/pr-mergeability.mjs b/scripts/pr-mergeability.mjs new file mode 100644 index 0000000000..00f512a34c --- /dev/null +++ b/scripts/pr-mergeability.mjs @@ -0,0 +1,85 @@ +#!/usr/bin/env node +/** + * Classify GitHub PR mergeability for the pull_request_target signal job. + * + * When a PR has a real content conflict, GitHub cannot build refs/pull/N/merge, + * so every pull_request-triggered workflow is skipped with no failing check + * (#116). This helper turns that silence into an explicit fail/pass/retry + * verdict from mergeable / mergeable_state. It never updates branches. + */ +export function classifyMergeability({ mergeable, mergeableState, draft } = {}) { + if (draft) { + return { + ok: true, + action: "skip", + reason: "draft", + message: "Draft PR: mergeability will be enforced when the PR is marked ready for review.", + }; + } + + const state = String(mergeableState ?? "") + .trim() + .toLowerCase(); + + if (mergeable === null || mergeable === undefined || state === "" || state === "unknown") { + return { + ok: true, + action: "retry", + reason: "computing", + message: "GitHub has not finished computing mergeability yet; retry shortly.", + }; + } + + // Fail only on real content conflicts. `behind` / `blocked` / `unstable` still + // produce a merge ref, so pull_request CI can run; those states must not page + // operators as "CI disappeared". GitHub's boolean `mergeable` alone is not + // used as a fail signal because it can lag or disagree with mergeable_state. + if (state === "dirty") { + return { + ok: false, + action: "fail", + reason: "conflict", + message: + "This PR has a real merge conflict with its base branch, so GitHub cannot build refs/pull//merge and pull_request CI (CI, Gitleaks, Semgrep, …) will not run. Resolve the conflict locally (git merge origin/main or rebase), push, and treat a missing check list as a conflict signal — not a green pass. Behind-but-clean staleness is a different case; use npm run sync:pr-branches (report) / sync:pr-branches:apply (human gh identity).", + }; + } + + return { + ok: true, + action: "pass", + reason: state || "clean", + message: `Mergeability is ${state || "clean"}; pull_request CI can run.`, + }; +} + +function assert(condition, label) { + if (!condition) throw new Error(`self-test failed: ${label}`); +} + +function selfTest() { + assert(classifyMergeability({ draft: true }).action === "skip", "drafts skip"); + assert(classifyMergeability({ mergeable: null, mergeableState: "unknown" }).action === "retry", "unknown retries"); + assert(classifyMergeability({ mergeable: undefined, mergeableState: "" }).action === "retry", "empty retries"); + + const conflict = classifyMergeability({ mergeable: false, mergeableState: "dirty" }); + assert(conflict.ok === false && conflict.action === "fail", "dirty fails"); + assert(conflict.message.includes("merge conflict"), "conflict message names the cause"); + + assert(classifyMergeability({ mergeable: true, mergeableState: "behind" }).ok === true, "behind passes"); + assert(classifyMergeability({ mergeable: true, mergeableState: "blocked" }).ok === true, "blocked passes"); + assert( + classifyMergeability({ mergeable: false, mergeableState: "blocked" }).ok === true, + "blocked ignores boolean lag", + ); + assert(classifyMergeability({ mergeable: true, mergeableState: "clean" }).action === "pass", "clean passes"); + assert( + classifyMergeability({ mergeable: false, mergeableState: "unstable" }).ok === true, + "unstable still has a merge ref", + ); + + console.log("pr-mergeability self-test passed."); +} + +if (process.argv.includes("--self-test")) { + selfTest(); +} diff --git a/tests/repo-hygiene.test.ts b/tests/repo-hygiene.test.ts index 83d5890ec2..7b3abbcd9b 100644 --- a/tests/repo-hygiene.test.ts +++ b/tests/repo-hygiene.test.ts @@ -457,6 +457,7 @@ describe("branch-review-ledger row parsing", () => { expect(deduped.removed).toBe(1); expect(deduped.kept).toBe(1); const merged = mergeLedgerMarkdown( + `${preamble}\n${shared}\n`, `${preamble}\n${shared}\n${oursOnly}\n`, `${preamble}\n${shared}\n${theirsOnly}\n`, ); @@ -464,6 +465,33 @@ describe("branch-review-ledger row parsing", () => { expect(merged.markdown.match(/Run PR sweep/g)).toHaveLength(1); }); + it("does not restore rotated base rows or discard the updated preamble", () => { + const oldPreamble = [ + "# Branch Review Ledger", + "", + "Old live-ledger instructions.", + "", + "| Date | Branch or ref | Reviewed HEAD | Scope | Outcome | Checks |", + "| --- | --- | --- | --- | --- | --- |", + ].join("\n"); + const newPreamble = oldPreamble.replace("Old live-ledger instructions.", "New archive-aware instructions."); + const archived = `| 2026-07-01 | old/x | ${"a".repeat(40)} | s | o | c |`; + const oursOnly = `| 2026-07-30 | ours/x | ${"b".repeat(40)} | s | o | c |`; + const theirsOnly = `| 2026-07-30 | theirs/x | ${"c".repeat(40)} | s | o | c |`; + + const merged = mergeLedgerMarkdown( + `${oldPreamble}\n${archived}\n`, + `${oldPreamble}\n${archived}\n${oursOnly}\n`, + `${newPreamble}\n${theirsOnly}\n`, + ); + + expect(merged.markdown).not.toContain(archived); + expect(merged.markdown).toContain(oursOnly); + expect(merged.markdown).toContain(theirsOnly); + expect(merged.markdown).toContain("New archive-aware instructions."); + expect(merged.markdown).not.toContain("Old live-ledger instructions."); + }); + it("rotates older rows into a quarterly archive while keeping newer live rows", () => { expect(calendarQuarterStart("2026-07-30")).toBe("2026-07-01"); expect(archiveQuarterLabel("2026-07-15")).toBe("2026-q3"); diff --git a/tests/upload-structure.test.ts b/tests/upload-structure.test.ts index ff0d78c3d0..5231a71f15 100644 --- a/tests/upload-structure.test.ts +++ b/tests/upload-structure.test.ts @@ -178,8 +178,9 @@ describe("assertUploadStructure — OOXML", () => { it("rejects high-compression-ratio archives (zip bomb shape)", async () => { const zip = buildDocxZip((bomb) => { - // 24MB of zeros deflates to a few KB — far beyond the allowed ratio. - bomb.file("word/media/zeros.bin", Buffer.alloc(24 * 1024 * 1024)); + // Highly compressible zeros still clear the 150:1 ratio (~480:1 at 1MB) + // without allocating a 24MB buffer that has timed out under CI coverage load. + bomb.file("word/media/zeros.bin", Buffer.alloc(1024 * 1024)); }); await expectRejection(assertUploadStructure(docxMime, await toBuffer(zip)), "compression ratio"); });