diff --git a/.github/workflows/codex-autofix-review-comments.yml b/.github/workflows/codex-autofix-review-comments.yml index fb11624867..a2fd9f1339 100644 --- a/.github/workflows/codex-autofix-review-comments.yml +++ b/.github/workflows/codex-autofix-review-comments.yml @@ -81,6 +81,7 @@ jobs: ]; const scopedResolveCommand = "@codex resolve actionable Codex review findings for this pull request and current head"; const resolvedDispositionMarker = ""; + const noChangeMarker = ""; if ( review.user?.type !== "Bot" || @@ -100,6 +101,11 @@ jobs: const owner = context.repo.owner; const repo = context.repo.repo; const issue_number = pr.number; + const headRepository = pr.head.repo?.full_name; + if (!headRepository) { + core.setFailed("Codex auto-resolve cannot identify the pull request head repository; refusing to route a repair request."); + return; + } const labels = new Set( (pr.labels || []) .map((label) => (typeof label === "string" ? label : label?.name)) @@ -273,7 +279,7 @@ jobs: ``, ``, ``, - `${scopedResolveCommand} using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with ${resolvedDispositionMarker} as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.`, + `${scopedResolveCommand} using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is ${headRepository}, and the only branch destination is the pull request head branch ${pr.head.ref} at starting commit ${pr.head.sha}; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to ${headRepository}:${pr.head.ref}, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with ${resolvedDispositionMarker} as the first line and as the second line. For a no-code disposition, use ${resolvedDispositionMarker} followed by ${noChangeMarker}. These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.`, ].join("\n\n"); try { @@ -328,6 +334,8 @@ jobs: "chatgpt-codex-connector[bot]", ]); const resolvedDispositionMarker = ""; + const fixedHeadPattern = /^$/m; + const noChangeMarker = ""; if ( reviewComment.user?.type !== "Bot" || @@ -351,6 +359,20 @@ jobs: return; } + const fixedHeadMatch = replyBody.match(fixedHeadPattern); + const isNoChangeDisposition = replyBody.includes(noChangeMarker); + if (Boolean(fixedHeadMatch) === isNoChangeDisposition) { + core.setFailed("Codex disposition must declare exactly one result: a verified fixed head or no code change."); + return; + } + + if (fixedHeadMatch && pr.head.sha !== fixedHeadMatch[1]) { + core.setFailed( + `Codex reported fixed commit ${fixedHeadMatch[1]}, but the pull request head is ${pr.head.sha}; leaving the thread open.`, + ); + return; + } + const owner = context.repo.owner; const repo = context.repo.repo; const issue_number = pr.number; diff --git a/AGENTS.md b/AGENTS.md index 5e06689b93..bb41a0d8a8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -791,7 +791,7 @@ When explicitly asked to fix or resolve review findings: - After fixing a P0 or P1 finding, reply with the fix summary and resolve the review conversation when supported by GitHub permissions/tooling. - After fixing an approved P2 or lower finding, reply with the fix summary and resolve the review conversation when supported. - After deciding not to fix a P2 or lower finding, reply with the reason, note whether it is deferred or not actionable, and resolve the review conversation when supported. -- For every fixed or fully dispositioned thread, start the thread reply with ``. The workflow uses this trusted marker to close that exact thread. +- For every fixed or fully dispositioned thread, start the thread reply with ``. On the next line, use `` for a code fix or `` for a no-code disposition. The workflow closes the thread only when exactly one result is declared and a reported fixed commit is the pull request head. - Do not use the marker when human input or new authorization is required; explain the blocker and leave that thread open. - Do not leave a review conversation open after it has been fixed or fully dispositioned. If direct resolution is unavailable, the marker reply is the required fallback and the workflow performs the closure. @@ -809,7 +809,7 @@ Automatic Codex review is review-only by default. This repository includes `.git - Pin the supported Node 24-based `actions/github-script` release to its reviewed immutable commit SHA. - Post the `@codex` resolve request with a real (non-bot) user identity — a fine-grained PAT held in the `CODEX_TRIGGER_TOKEN` secret. The Codex connector ignores commands authored by `github-actions[bot]`, so a bot-authored request is silently dropped. The token needs `pull-requests: write` (issue-comment) access and no more. - The workflow must treat unmarked review-thread replies as inert. A trusted Codex reply beginning with `` may only resolve the exact containing thread, and a non-reply Codex review comment must never be turned into a new repair request. -- The workflow must ask Codex to resolve only existing actionable Codex review findings for the triggering pull request and current head using these repository instructions; the resolve task must not perform a new review or create new findings. +- The workflow must ask Codex to resolve only existing actionable Codex review findings for the triggering pull request and current head using these repository instructions; the resolve task must not perform a new review or create new findings. It must name the exact repository and PR head branch, require fixes to be published there through the authenticated GitHub connector, forbid detached `work` branches and stacked pull requests, and treat a local-only commit as a visible failure. - The workflow may request one automatic repair pass per pull request lifetime. Later heads require an explicit human request. - Only trust a pull-request deduplication marker when it was posted by the trigger-token account (the same identity that posts the request), resolved at runtime rather than hard-coded. - Permission failures while reading or creating pull-request comments must fail the workflow visibly, not return a successful soft-skip. @@ -820,7 +820,7 @@ Automatic Codex review is review-only by default. This repository includes `.git ### Primary PR command -`@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.` +`@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The workflow will provide the only allowed repository, pull-request head branch, and starting commit. Publish every approved fix to that exact head branch through the authenticated GitHub connector; never use a detached or synthetic work branch and never create a stacked pull request. Verify the pull-request head contains the pushed commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with followed by . For a no-code disposition, use followed by . A local-only commit is not a fix. If publication or verification fails, use neither result marker, do not claim success, and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.` ## Codex Cloud environment diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 160c8bb428..dc5aab036d 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -243,4 +243,6 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-30 | PR-1473 | a2b2820c13a47425cfc0ea751e57ee35e9bd1105 | PR #1473 full diff vs origin/main | PASS after review repair: governance refusal and error-state contracts are consistent | outstanding-issues guard passed; docs links 1412 passed; docs index passed; Prettier passed; git diff --check | | 2026-07-30 | pr/1483 | a84fa60eebdbe7a00193c268b401f7abd3cc554e | docs: reopen issue 105 after withdrawn verification | approved after PR 1473 sync; issue 105 remains correctly open | issue/ledger; docs inventory/links/scripts; Prettier; diff-check | | 2026-07-30 | pr/1476 | 0fd5a3cdba612d30cfd75ea997177f6e29c34bd3 | docs: record ESLint 10 ecosystem blocker | approved after PR 1483 sync; ESLint blocker and issue 105 correction preserved | issue/ledger; docs inventory/links/scripts; Prettier; diff-check | +| 2026-07-30 | PR #1477 | 26d713922006c1af8187994edfa76669dc14cd46 | PR #1477 fork-safe Codex autofix routing | Fixed fork routing to the PR head repository, added fail-closed metadata handling, reconciled current main, and found no remaining actionable defects. | check:codex-autofix-workflow; check:github-actions; check:pr-policy; check:outstanding-issues; check:branch-review-ledger; docs:check-inventory; docs:check-links; docs:check-scripts; typecheck; focused Vitest 53 passed; Prettier | | 2026-07-30 | pr/1465 | 4e34d97bb9eb5122b9d8f8e54c42793c727f5085 | issues: record fresh #133 evidence | approved; duplicate-ID race and Prettier prerequisite accurately recorded | issue/ledger; docs inventory/links; Prettier; diff-check | +| 2026-07-30 | PR #1477 | 20f795da2d9d0adafa6cb3117429ab3665129c0d | PR #1477 fork-safe Codex autofix routing | Refreshed onto current main after #1465; issue and ledger reconciliation remained clean and no new actionable defects were introduced. | check:outstanding-issues; check:branch-review-ledger; check:codex-autofix-workflow; focused Vitest 53 passed | diff --git a/docs/codex-review-protocol.md b/docs/codex-review-protocol.md index 1a15b24b8e..c77259029b 100644 --- a/docs/codex-review-protocol.md +++ b/docs/codex-review-protocol.md @@ -28,7 +28,7 @@ Use this protocol for every Codex review, audit, bug hunt, PR review, release-re - Exception: append the completed review record to `docs/branch-review-ledger.md` so throttling state persists. - If the user clearly asks to fix confirmed findings, make the smallest safe change and verify with local, static, or mocked checks first. - During an automatic resolve task, work only existing unresolved Codex threads. Do not start a new review, add standalone findings, or request another review. -- After fixing or fully dispositioning a thread, start the reply with ``; the workflow will close that exact thread. Do not use the marker when human input or new authorization is required, and leave that blocked thread open with a concise reason. +- After fixing or fully dispositioning a thread, start the reply with ``, then declare exactly one result: `` for a published fix or `` for a no-code disposition. The workflow closes a fixed thread only when the reported commit is the pull-request head. A local-only commit is not a fix; when publication, verification, human input, or new authorization blocks completion, use no result marker and leave the thread open with a concise reason. - Ask before any OpenAI, Supabase, GitHub/GitLab, hosted CI, or provider-backed workflow. - After any completed branch/PR review, append to `docs/branch-review-ledger.md` with `npm run ledger:append -- --ref --head --scope --outcome --checks `. Record the full 40-character SHA; `see PR head` and abbreviations make the record unmatchable and cause the review to run again. The ledger is append-only: never edit or delete an existing record; append a correction or superseding record (`--supersede`) instead. This ledger append is allowed even during a pure review. Do not hand-write the markdown row — hand-written rows are what produced the mojibake, wrong-width, and duplicate records the 2026-07-28 hygiene pass had to repair. Do not push a tip whose sole delta is a babysit ledger append; after merging `origin/main` into a branch that touched the ledger, run `npm run ledger:dedupe` when exact twins appear. diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index a6407fc9b9..ec74bf1c5d 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -151,7 +151,6 @@ removed after current-main verification; it is not missing recommended work. | #128 | P2 | issue | Draft-to-ready alone does not retrigger required CI | **Outcome:** un-drafting a PR is not by itself enough to get its full required-check suite running. **Detail:** on 2026-07-30 PR #1406 sat with only 3-4 minimal checks (`PR policy`, `GitGuardian`, `Supabase Preview`) for 30+ minutes after being marked ready for review, with `mergeable_state` clean (not conflicted — distinct from #116). `.github/workflows/ci.yml`'s `on: pull_request` block has no explicit `types:`, which defaults to GitHub's `[opened, synchronize, reopened]`; `ready_for_review` is not in that list, so converting a draft to ready fires no workflow run on its own. The full suite only started once an actual new commit landed (a `synchronize` event), e.g. syncing the branch with `main`. **Next:** if drafts routinely go ready without an accompanying commit, add `ready_for_review` to `ci.yml`'s `pull_request.types` list ALONGSIDE the current implicit defaults — `types: [opened, synchronize, reopened, ready_for_review]`, not a bare `[ready_for_review]`, which would replace the defaults and stop CI firing on ordinary `opened`/`synchronize`/`reopened` events. Otherwise document that un-drafting alone is insufficient and a session should push a no-op/sync commit to actually kick off CI. **Stop:** do not conclude a draft's CI is "just slow" from elapsed time alone — check whether it actually has the full check set (16-19 checks, not 3-4) before waiting further. | PR #1406; session 2026-07-30 PR babysit | 2026-07-30 | | #129 | P2 | issue | GitHub's `update-branch` API doesn't honor this repo's `merge=ledger` driver | **Outcome:** `update-branch` can report a 422 "merge conflict between base and head" on a branch that a local `git merge origin/main` resolves cleanly. **Detail:** on 2026-07-30 PR #1406's branch was several commits behind `main` and touched `docs/branch-review-ledger.md`, which carries `merge=ledger` in `.gitattributes` specifically so parallel ledger appends resolve without conflict (see #088/#112). GitHub's own server-side merge/update-branch implementation does not read `.gitattributes` custom merge drivers, so it computed a real textual conflict at the same hunk the local `ledger` driver resolves. `git merge-tree --write-tree origin/main ` confirmed clean; the API call still 422'd. **Next:** when `update-branch` fails on a branch touching `docs/branch-review-ledger.md` (or any other `merge=ledger` path) and `git merge-tree` shows no real conflict, treat it as staleness rather than a genuine conflict needing manual resolution and fall back to a local `git merge origin/main` + push (per the existing "Open PR branch sync" guidance) — same as any other push, this still needs the explicit user confirmation AGENTS.md's "API and provider confirmation boundary" requires outside an authorized sweep (`Run PR`/`upload`), not a standing exemption for `merge=ledger` paths. **Stop:** do not conclude a real content conflict from `update-branch`'s response alone on a custom-merge-driven file; verify with `git merge-tree` first — same discipline as the existing GitHub `dirty`/`CONFLICTING` staleness guidance. | PR #1406; session 2026-07-30 PR babysit | 2026-07-30 | | #130 | P2 | issue | PR #1396 merged shared phone-chrome behaviour without its own declared physical-device gate | **Outcome:** a shared-chrome PR does not merge with a self-declared merge prerequisite left undone, or the ledger records that it did. **Detail:** PR #1396 ("overlay the phone header so hiding it never moves content") repeatedly stated in its own body and PR comments that `docs/phone-chrome-physical-acceptance.md` "genuinely applies before merge" because local Chromium cannot certify Safari chrome-minimisation or cold-launch PWA paint (invariant 23) — restated at least three times across the review thread, including after the final `a638b66e`/`f7347144` fix. It merged at 06:49:55 anyway. Checked 2026-07-30: `docs/phone-chrome-physical-acceptance.md` on `main` is still the blank checklist template — every "Result / evidence" cell is empty, no PR comment attaches a filled-in copy or device evidence, and no existing ledger row (`#120`, `#122`) covers this gap. Related but distinct: one Codex thread on this PR also names a still-missing guard — a pre-paint/cold-load hydration test comparing content position before and after hydration, which the author explicitly said they would "rather file it than ship a test that looks like it covers the window and does not" — and that filing never happened either. **Next:** run the physical-device matrix in `docs/phone-chrome-physical-acceptance.md` against `main`'s current tip on a real iPhone (Safari tab + cold-launch PWA, light/dark, portrait/landscape) and commit the filled-in evidence; separately, add the pre-paint/cold-load Playwright pattern this PR's own review identified as missing. **Stop:** do not treat this PR's extensive Codex/CI remediation (13 findings fixed, 9 threads resolved) as a substitute for the physical-device proof — headless Chromium was explicitly stated as unable to certify the two things this checklist exists for. **Design constraints recorded 2026-07-30, so the guard is not re-derived from scratch:** the value under test is the pre-paint reserve seed `calc(max(0.5rem, var(--safe-area-top)) + var(--shell-header-h))` in `globals.css`, refined by `useLayoutEffect` in `use-phone-overlay-chrome-reserve.ts`. The window that needs covering is _before_ hydration, so the test must sample content top on the cold load and again after hydration and compare them; a single post-hydration read passes on the broken shape and is the "looks like coverage" outcome this item exists to avoid. The `max()` is the part that actually breaks: seeding the bare inset under-reserves by `max(0, 0.5rem − inset)`, which is **zero on a notched iPhone and 8px on any phone reporting no top inset** — Android, and Playwright's default emulation — so the assertion must run on a zero-inset profile or it cannot fail. Prove it against the broken shape before trusting it (re-seed with the bare inset and confirm the test goes red), per the lesson recorded on `#120`. **Environment blocker:** this cannot be verified in a remote container. The Chromium build mismatch in `#121` means browser tests never launch, and the documented symlink bridge writes under `/opt/pw-browsers`, which the session sandbox refuses — so this needs a local session, or an operator-granted exception, before any claim that the guard works. | PR #1396 (merged 2026-07-30); session 2026-07-30 PR babysit | 2026-07-30 | -| #131 | P2 | issue | `@codex fix` produces commits that never reach the repository | **Outcome:** a finding routed to Codex is either fixed on the branch or visibly not fixed. **Detail:** on 2026-07-30 PR #1400, eleven `@codex fix` dispatches produced at least eleven commits — `f632ba2`, `bb27822`, `1ab2c40`, `749a183`, `08a6b7a`, `ed6be22`, `6e07327`, `9353757`, `c2e56d0` and others — every one reported as committed on a branch named `work` with "stacked pull request metadata" created. **None is fetchable:** `git cat-file -e` fails locally and `git fetch origin ` fails for each, and no PR carries them. The same single finding was rewritten four separate times under different SHAs, none landing. Its reports also show it could not execute tests (`node_modules` absent, Node 20 vs the required 24), so the claims were unverified as well as unlanded. The danger is that it reads as success: threads get authoritative-looking "Summary / Testing" replies while the branch is unchanged. **Next:** decide whether the Codex connector is expected to push to the PR branch and, if so, why it is writing to a detached `work` branch instead; until then treat `@codex fix` as advisory only and land fixes another way. **Stop:** never resolve a review thread on the strength of a Codex report — verify against the actual ref content first, per AGENTS.md. | PR #1400; session 2026-07-30 | 2026-07-30 | | #132 | P3 | issue | Both client-side push guards are inert for agent pushes | **Outcome:** the format and auto-merge guards protect every push, or their blind spot is explicit. **Detail:** `scripts/guard-push.mjs` printed `auto-merge: gh not available — auto-merge check skipped (fail-open)` for pushes from a remote agent environment, so the auto-merge race sentinel never evaluated; and `core.hooksPath` is set only by a local `npm install`, so an agent pushing from its own checkout bypasses `.githooks/pre-push` entirely. Both guards therefore protect exactly the environment least likely to break the rule, which is why the AGENTS.md format-before-push instruction is still load-bearing even though the tooling now exists. Observed directly on PR #1400: a push landed while auto-merge was armed with nothing to stop it. **Next:** provide `gh` (or a token-based equivalent) in agent environments so the sentinel can evaluate. **Do not move the format check into `pull_request_target`** — that context carries secrets and a write token, and a format check must execute PR-head code including this repo's now-loadable dynamic `prettier.config.*`, which is the classic privileged-context vector; `.github/workflows/pr-policy.yml` deliberately checks out only `github.workflow_sha` for exactly this reason. Formatting is already enforced server-side by `Static PR checks` running `format:check` on ordinary `pull_request` CI, so the guard's only unique value is failing fast before the push — nothing to duplicate. The auto-merge sentinel reads PR metadata only and could safely live in `pull_request_target` if it is ever worth moving. | PR #1400; session 2026-07-30 | 2026-07-30 | | #133 | P3 | rec | Ledger conflicts on nearly every `main` advance (union driver removed) | **Outcome:** two agents editing different rows of this ledger do not silently corrupt it. **Driver half RESOLVED 2026-07-30:** `merge=union` is removed from `.gitattributes` and `check:outstanding-issues` now requires that **no** driver is set, so a reappearance is a red gate (proved by reintroducing it: `must have NO merge driver (found merge=union)`). This row already recorded union as worse — "two sides each bumping the marker produce two `next-id` lines, corrupting the file silently where a conflict would fail loudly" — but the attribute stayed in place and the gate _mandated_ it, so the repo's own tested conclusion was contradicted by its own config. PR #1430 confirmed the cost at scale: four merges in one session, each reporting success while duplicating the **entire** open-items table (`#059 appears 2 times (lines 101, 166)` and so on), every one needing a manual rebuild from `origin/main`. Union also makes `git merge-tree` report a clean tree, so the pre-merge conflict check cannot warn. Unlike `docs/branch-review-ledger.md`, this file allocates IDs by read-modify-write, so concurrent appends need manual renumbering whatever the driver does — union bought nothing. `AGENTS.md`, `docs/process-hardening.md`, `.claude/skills/issues/SKILL.md` and `docs/scripts-index.md` are updated to match. **Still open — the conflict frequency itself:** the table is padded to fixed column widths, so one row's edit re-pads every open row and git sees the whole table as one changed hunk; on 2026-07-30 it conflicted twice within an hour on PR #1400, and each conflict silently stopped **all** CI on that PR (`#116`). **Next:** stop padding this table (Prettier still renders it readably, and one-row edits become one-line diffs), or split the open items into per-row files. **Re-confirmed 2026-07-30 (PR #1451):** removal did not reduce the pain — that PR conflicted on this file, its session resolved it (renumbering a colliding row), and `git merge-tree` showed it conflicting again within minutes, because four further `main` commits (#1455, #1446, #1445, and the X3 coverage record) each touched the table. Both sides had also allocated `#141` concurrently, so a duplicate id reached a pushed tip and failed `check:outstanding-issues` there (`#141 appears 2 times`) independently of the conflict — the read-modify-write allocation race this row already predicts. **Prerequisite for the un-pad fix:** Prettier enforces padded markdown tables under `docs/`, verified by checking identical ragged content in an ignored path (passes) and under `docs/` (fails) — so un-padding also needs `docs/outstanding-issues.md` added to `.prettierignore`, precisely as its sibling `docs/branch-review-ledger.md` already is at `.prettierignore:23`. Do that in one commit while the ledger queue is quiet: the un-pad rewrites every open row and will conflict with any in-flight ledger edit. **Stop:** do not reintroduce a merge driver here; if concurrent-append pain returns, write a dedupe driver like `merge=ledger`'s rather than stock union. | `.gitattributes`; `scripts/check-outstanding-issues.mjs`; PR #1400; PR #1430 | 2026-07-30 | | #134 | P2 | issue | Ledger union-merge driver is absent wherever `npm install` was skipped | **Outcome:** the ledger's union-merge protection is present wherever a merge happens, or its absence is loud. **Detail:** `.gitattributes` declares `docs/branch-review-ledger.md merge=ledger`, but the driver itself lives in git _config_, installed by `postinstall` -> `scripts/install-git-hooks.mjs`. A container that skips `npm install` (this repo's remote agent sessions do — the session hook reports "node_modules matches the lockfile, skipping install") therefore has the attribute without the driver, and git silently falls back to an ordinary merge. On 2026-07-30 a `git merge origin/main` on PR #1424 produced **conflict markers inside the append-only ledger** at three lines; `git merge` itself did not name the file, so only `npm run check:branch-review-ledger` caught it. Committing that would have corrupted the file the guard exists to protect. **Next:** make the absence loud — have `check:branch-review-ledger` (already in `verify:cheap` and `static-pr`) fail when `.gitattributes` declares `merge=ledger` but `git config merge.ledger.driver` is unset, so the environment is caught before a merge rather than after. `npm run hooks:install` is the one-line fix once detected. **Stop:** never trust a `merge=union`-style attribute to be active just because `.gitattributes` declares it; the driver is per-checkout config. | PR #1424; session 2026-07-30 | 2026-07-30 | @@ -174,6 +173,7 @@ Move resolved rows here with the resolution date and a one-line outcome. Keep th | ID | Type | Summary | Outcome | Resolved | | ---- | ----- | ----------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| #131 | issue | `@codex fix` produced commits that never reached the repository | RESOLVED 2026-07-30. Automatic repair requests now bind Codex to the exact repository, pull-request head branch, and starting SHA; explicitly forbid detached/synthetic `work` branches and stacked pull requests; and require the authenticated GitHub connector to publish and verify the fix before success is reported. Thread closure is fail-closed: a fixed reply must name its 40-character pushed commit SHA, and the workflow resolves the thread only when that SHA is the pull-request head. No-code dispositions carry a distinct marker, while missing, conflicting, or unlanded result claims leave the thread open with a visible workflow failure. The guard and executable workflow tests pin the destination prompt, both valid outcomes, malformed results, and mismatched-head refusal. Source: issue #131; session 2026-07-30 | 2026-07-30 | | #136 | rec | Reuse Next build cache across isolated Playwright production builds | CLOSED 2026-07-30 after an end-to-end implementation benchmark rejected the proposed cache as a net CI loss. Keeping the reusable cache outside each disposable run root preserved the runner cleanup contract, but a warm build saved only 34 seconds (97s cold to 63s warm) while producing an 804 MB cache. Persisting that entry per commit would consume the repository cache budget and evict the substantially more valuable Playwright browser cache. No cache wiring ships; reconsider only if Next materially reduces the cache size or a later measurement changes the storage/time trade-off. Source: `scripts/run-playwright.mjs`; `docs/testing.md`; session 2026-07-30 | 2026-07-30 | | #085 | rec | Upload-limit client/server sync is unguarded | Resolved 2026-07-30. A provider-free parity checker now validates both configured limits, their shared 150 MB default, invalid and over-ceiling values, and mismatches. It runs in `verify:cheap`, the static PR job, and before every production build so a client-side value cannot silently diverge from the server runtime limit. | 2026-07-30 | | #119 | issue | `ci/circleci: verify` is failing repo-wide | Retired 2026-07-30. CircleCI was removed from the repository in commit `977982857`, and the user confirmed the integration is deleted. The prior request to inspect build 672 is obsolete; GitHub Actions remains the repository CI path. | 2026-07-30 | diff --git a/scripts/check-codex-autofix-workflow.mjs b/scripts/check-codex-autofix-workflow.mjs index 677e0ed86e..cb08361caf 100644 --- a/scripts/check-codex-autofix-workflow.mjs +++ b/scripts/check-codex-autofix-workflow.mjs @@ -11,7 +11,7 @@ const failures = []; const githubScriptPin = "3a2844b7e9c422d3c10d287c895573f7108da1b3"; const scopedResolveCommand = "@codex resolve actionable Codex review findings for this pull request and current head"; const resolvedDispositionMarker = ""; -const scopedResolvePrompt = `\${scopedResolveCommand} using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with \${resolvedDispositionMarker} as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.`; +const scopedResolvePrompt = `\${scopedResolveCommand} using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. The only repository destination is \${headRepository}, and the only branch destination is the pull request head branch \${pr.head.ref} at starting commit \${pr.head.sha}; never publish fixes to a detached or synthetic work branch and never create a stacked pull request. Use the authenticated GitHub connector to commit each approved fix to \${headRepository}:\${pr.head.ref}, then verify that the pull request head contains the published commit before reporting success. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. For a fixed thread, reply with \${resolvedDispositionMarker} as the first line and as the second line. For a no-code disposition, use \${resolvedDispositionMarker} followed by \${noChangeMarker}. These result markers authorize the workflow to close that exact thread only after it verifies a fixed commit is the pull request head; a local-only commit is not a fix. If publication or verification fails, do not use either result marker, do not claim success, and leave the thread open with the blocker. If human input or new authorization is required, do the same. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.`; const forbiddenPatterns = [ { @@ -232,9 +232,25 @@ for (const requiredCheck of requiredRiskRoutingChecks) { } } +const requiredHeadRoutingChecks = [ + "const headRepository = pr.head.repo?.full_name", + "The only repository destination is ${headRepository}", + "${headRepository}:${pr.head.ref}", +]; + +for (const requiredCheck of requiredHeadRoutingChecks) { + if (!workflow.includes(requiredCheck)) { + failures.push(`Codex auto-resolve workflow is missing PR-head repository routing: ${requiredCheck}`); + } +} + const requiredThreadResolutionChecks = [ `const resolvedDispositionMarker = "${resolvedDispositionMarker}"`, "replyBody.startsWith(resolvedDispositionMarker)", + "codex-thread-result:fixed-head:([0-9a-f]{40})", + "codex-thread-result:no-change", + "pr.head.sha !== fixedHeadMatch[1]", + "leaving the thread open", "reviewThreads(first: 100, after: $cursor)", "resolveReviewThread(input: { threadId: $threadId })", "pull-requests: write", diff --git a/tests/codex-autofix-workflow.test.ts b/tests/codex-autofix-workflow.test.ts index e5187b313c..70d04bd7d8 100644 --- a/tests/codex-autofix-workflow.test.ts +++ b/tests/codex-autofix-workflow.test.ts @@ -120,6 +120,7 @@ async function runRequestScript(options?: { files?: PullRequestFile[]; filesError?: unknown; getAuthenticatedError?: unknown; + pullRequestHeadRepository?: string | null; pullRequestHeadSha?: string; pullRequestLabels?: Array; review?: Partial; @@ -187,7 +188,14 @@ async function runRequestScript(options?: { payload: { review, pull_request: { - head: { sha: options?.pullRequestHeadSha ?? "head-sha-4" }, + head: { + ref: "feature/codex-fix", + repo: + options?.pullRequestHeadRepository === null + ? null + : { full_name: options?.pullRequestHeadRepository ?? "clinical-kb/database" }, + sha: options?.pullRequestHeadSha ?? "head-sha-4", + }, labels: options?.pullRequestLabels ?? [], number: 42, state: "open", @@ -209,6 +217,7 @@ async function runThreadScript(options?: { comment?: Partial; graphqlError?: unknown; graphqlResults?: unknown[]; + pullRequestHeadSha?: string; }) { const failures: string[] = []; const graphqlCalls: GraphqlCall[] = []; @@ -216,7 +225,7 @@ async function runThreadScript(options?: { const warnings: string[] = []; const comment: Comment = { - body: "\n\nFixed.", + body: "\n\n\nDispositioned.", id: 99, in_reply_to_id: 41, user: { login: "chatgpt-codex-connector[bot]", type: "Bot" }, @@ -236,7 +245,7 @@ async function runThreadScript(options?: { { payload: { comment, - pull_request: { head: { sha: "head-sha-4" }, number: 42, state: "open" }, + pull_request: { head: { sha: options?.pullRequestHeadSha ?? "head-sha-4" }, number: 42, state: "open" }, }, repo: { owner: "clinical-kb", repo: "database" }, }, @@ -650,9 +659,32 @@ describe("Codex auto-resolve request script", () => { expect(result.createdComments).toHaveLength(1); expect(result.createdComments[0]?.body).toContain("single automatic repair pass"); expect(result.createdComments[0]?.body).toContain(""); + expect(result.createdComments[0]?.body).toContain("clinical-kb/database:feature/codex-fix"); + expect(result.createdComments[0]?.body).toContain("never publish fixes to a detached or synthetic work branch"); + expect(result.createdComments[0]?.body).toContain( + "", + ); expect(result.createdComments[0]?.body).toContain("do not perform a fresh review"); }); + it("routes a fork repair to the pull request head repository", async () => { + const result = await runRequestScript({ + pullRequestHeadRepository: "external/contributor-repo", + }); + + expect(result.failures).toHaveLength(0); + expect(result.createdComments).toHaveLength(1); + expect(result.createdComments[0]?.body).toContain("external/contributor-repo:feature/codex-fix"); + expect(result.createdComments[0]?.body).not.toContain("clinical-kb/database:feature/codex-fix"); + }); + + it("fails closed when the pull request head repository is unavailable", async () => { + const result = await runRequestScript({ pullRequestHeadRepository: null }); + + expect(result.createdComments).toHaveLength(0); + expect(result.failures).toContainEqual(expect.stringContaining("cannot identify the pull request head repository")); + }); + it("fails visibly when it cannot identify the trigger-token account", async () => { const result = await runRequestScript({ getAuthenticatedError: new Error("no user") }); @@ -724,6 +756,60 @@ describe("Codex auto-resolve thread-resolution script", () => { expect(result.notices).toContainEqual(expect.stringContaining("without the trusted resolved disposition marker")); }); + it("leaves a fixed thread open when the reported commit is not the pull request head", async () => { + const reportedHead = "a".repeat(40); + const result = await runThreadScript({ + comment: { + body: `\n`, + }, + pullRequestHeadSha: "b".repeat(40), + }); + + expect(result.graphqlCalls).toHaveLength(0); + expect(result.failures).toContainEqual(expect.stringContaining("leaving the thread open")); + }); + + it("accepts a fixed thread only when the reported commit is the pull request head", async () => { + const reportedHead = "a".repeat(40); + const result = await runThreadScript({ + comment: { + body: `\n`, + }, + pullRequestHeadSha: reportedHead, + graphqlResults: [ + { + repository: { + pullRequest: { + reviewThreads: { + nodes: [ + { + comments: { nodes: [{ databaseId: 41 }, { databaseId: 99 }] }, + id: "thread-1", + isResolved: false, + }, + ], + pageInfo: { endCursor: null, hasNextPage: false }, + }, + }, + }, + }, + { resolveReviewThread: { thread: { id: "thread-1", isResolved: true } } }, + ], + }); + + expect(result.failures).toHaveLength(0); + expect(result.graphqlCalls).toHaveLength(2); + }); + + it("rejects a disposition without exactly one machine-readable result", async () => { + const result = await runThreadScript({ + comment: { body: "\nFixed locally." }, + }); + + expect(result.graphqlCalls).toHaveLength(0); + expect(result.failures).toContainEqual(expect.stringContaining("exactly one result")); + }); + it("resolves the exact review thread after a trusted disposition reply", async () => { const result = await runThreadScript({ graphqlResults: [