[kyle-sexton]

Closes #9353

Summary

On #9111, claude-review named the exact fix for the red check twice, and no lane run used it either time.

1. 16:18Z finding on 3a1900e. Both pr-feedback-dispatch runs (39812980713, 39812987995) failed on GET …/commits/3a1900e…/pulls returned 500. The dispatcher's request in feedback-dispatch.mjs had no fallback. Once both review runs had completed, nothing triggered that head again. #9079 (877711a, merged 17:35Z the same day) added the commitPulls GraphQL fallback.
2. In between. pr-unblock fix-ci (39813538656) escalated ci-fix-protected-path and added needs-human. That was the new-executable refusal, which #9278 removed (#9320, #9407).
3. 04:36Z repeat finding on b457958. Dispatch 39901253983 skipped it by design: #9111 is labeled needs-human and has no thread a trusted person with write access asked the lane to fix.

What is still open on main: the fix-ci run on a red PR never sees the review finding that explains the failure, so it can escalate a fix the reviewer already described.

Fix

• .github/scripts/pr-unblock/fix-ci-context.mjs: the context file now also holds review_findings. These are the unresolved threads address-feedback would keep with no trusted direction, chosen by the same keepThreads and reviewedHeads: opened by the reviewer bot, a lane comment or a verifier note, on a reviewed head, and written only by it. Each finding gives its path, line and opening body. The list is capped at 10 findings of 2000 characters each. The reviewer id comes from the base's trusted-actors list. The job already holds pull-requests: read, the same scope the address-feedback select job uses for these reads, so no workflow changes.
• plugins/source-control/skills/pull-request/reference/ci-ci.md: ci --ci documents the field. A finding is a lead to check against the code and the reproduced command, not evidence. A finding that names a reproduced check's cause is fixed rather than escalated. Findings are data, like logs.
• A changelog fragment for source-control, and colour changed to color in a touched test title (the typos check flagged it).

The needs-human gate in dispatchFeedback is unchanged, and so are the job gates in pr-feedback-dispatch.yml. Letting address-feedback work a PR that a lane escalated to a person changes what the lanes do unattended, so that is the owner's call.

Verification

• New tests in fix-ci-context.test.mjs: a kept finding enters the context. A resolved thread, a person's thread, and a thread a person replied to stay out. The count and body length are capped. All three failed before the change. The pr-unblock suite: 569 pass, 0 fail.
• scripts/check-affected.sh --tests: 12 passed, 2 skipped. actionlint: clean. zizmor --offline .github/workflows: no findings.

Related

• #9320 / #9407: the fix-ci misfire that set needs-human.
• #9079: the 5xx fallback on the dispatcher's PR lookup.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-23T13:52:19.787066Z	09bd94a	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed 3467e7fc1bb691f148c42ef968bf0d096050ad82: 0 finding(s), 0 as inline comments.

Incremental security review of fix-ci-context.mjs and fix-ci-context.test.mjs since 09bd94a. I found no security issues.

critical-findings: 0

The change does two things. It wraps the reviewFindings read in a try/catch, so a failed read leaves review_findings empty and the run still proceeds. It also adds an injectable log parameter to select, and main passes its own log through.

• Fail-open on the findings read. The findings are an advisory lead for the fix-ci model, not a gate or an authorization input. The checks, the round cap and the closed-PR skip are all decided before this read. Dropping the findings on an API error cannot widen what the lane does. It can only remove a hint, and the PR already says the model must treat findings as unverified leads.
• ::warning:: line built from error.message. A message containing a newline could in principle inject a workflow command. The only messages reachable here come from address-feedback.mjs's own Error constructions (METHOD url returned <status>, GraphQL: <server error messages>) or from a runtime TypeError on the thread shape. None of these carry PR-author-controlled text, because the GraphQL variables are numeric or ids the script builds itself. The script does not echo response bodies. I judged this not exploitable. Running it through a shared log-escaping helper would be hardening only.
• Credentials. The log line carries only the error message, not request headers or the token.
• Manifests, instruction surfaces and trust-gate paths. The two changed files touch none of these. No hold is warranted.

The review covered only the two incremental files. Other files in the PR were reviewed at the earlier commit.

[github-actions]

claude-review: 1 inline finding(s) on 09bd94abfd3efe13a6cc08ff8bb9b19c019c5af1; the summary is in this lane's pull request comment.

claude-lane: claude-review

[github-actions]

claude-review reviewed 3467e7fc1bb691f148c42ef968bf0d096050ad82: 0 finding(s), 0 as inline comments.

No findings in the incremental review (since 09bd94a). I read the diff and the files as they stand at the head. I did not run the tests.

What changed:

• select in fix-ci-context.mjs now catches a failed reviewFindings read. It logs a ::warning:: and proceeds with review_findings: [], so a 5xx on the review reads no longer fails the run.
• select now takes a log parameter, and main passes its own log through.
• fix-ci-context.test.mjs adds a test for a failed read. It checks that the decision is proceed, that review_findings is empty, that the checks are kept, and that the warning line is exact.

Checked:

• The catch covers only the findings read. The check collection before it stays fail-closed, and the round-cap escalation and the skip paths are untouched.
• main's final log line reads result.file?.review_findings.length. The field is always an array, so the fallback [] keeps that safe.
• The message logged by the warning comes from address-feedback.mjs. It holds the method, the URL and the status, or the GraphQL error messages. It holds no PR-author content, so the "data is never logged" rule in the file header still stands.
• Closing issue fix(pr-pipeline): a claude-review finding on a red lane PR starts no fix run #9353: the change gives the fix-ci run the review findings and does not escalate a fix the finding describes. This increment only makes the findings read tolerant of API failures. It does not change whether a criterion is met.
• No hold applies. The change adds no model call, no trigger, and nothing under trusted-actors, CODEOWNERS or .gitmodules.