[kyle-sexton]

No related issue: defect found tracing #12647's needs-session arm

Summary

pr-unblock's report-stop-status posted the failing stop status on the head the select job read, before the activity's own push. In run 41575318920 on #12647, apply-feedback-patch pushed efa0184c (parent 55176925), publish-feedback then stopped with feedback-unadjudicated (routed fail), and report-stop-status posted pr-unblock/address-feedback failure on 55176925. The PR's head efa0184c carried no failing status, so ci-status passed and pr-merge's enable-auto-merge armed it with an open finding, while report-escalation (which reads the live head) commented on efa0184c.

Fix

• .github/workflows/pr-unblock.yml, report-stop-status: HEAD_SHA takes each activity's push-job pushed-commit before its own select head: apply-conflict-patch before resolve-conflicts-select, apply-feedback-patch before address-feedback-select, apply-dependency-patch before review-dependency-select. apply-dependency-patch is added to the job's needs. It already ran before this job through apply-dependency-verdict, so the order of jobs does not change. pushed-commit comes only from the base's apply-patch.mjs step (pr-run-activity-write.yml pushed-commit: ${{ steps.act-script.outputs.commit }}). It is not gated on dispatcher-run-id as report-escalation's input is: the push happened either way, so that commit is the PR's head.
• Which activities can stop after their own push, checked in the code:
  • address-feedback: publish-feedback runs after apply-feedback-patch and can stop (feedback-unadjudicated and others). This is the observed defect.
  • review-dependency: apply-dependency-verdict runs after apply-dependency-patch. On a lane push it can stop with dependency-invalid, dependency-config-unreadable or dependency-fix-refused (apply-dependency-verdict.mjs 349/361/367). This is the same defect.
  • resolve-conflicts: no stop can follow apply-conflict-patch's push. conflict-scan-refusal runs only when no patch was written, and apply-patch.mjs pushes or escalates, never both (1395-1400). The change here affects only the clear path, which now targets the pushed commit (a no-op, since that commit has no status yet) instead of a head the PR has left.
  • fix-ci and sync-shared-copies: unchanged. Push and escalation are exclusive within apply-patch.mjs, no reply or verdict job follows, and rung-up=true means apply-patch did not push.
• .github/scripts/pr-merge/enable-auto-merge.mjs: the header now lists the HOLD_LABELS the code reads (do-not-merge, pr-verify-failed, security: outside-actor) plus the human-gated label.
• Per-run API call delta: 0 expected. status and clear make the same calls whatever commit they target, and the added needs entry runs nothing.
• Not fixed (UNVERIFIED): if apply-patch-rung-up pushed and the job then failed, ci-fix-escalated would land on the pre-push head. GitHub does not document whether a failed reusable job's outputs reach its caller.

Verification

• node --test .github/scripts/pr-unblock/workflow-gates.test.mjs: 49 pass, 0 fail. The new test report-stop-status posts on the commit the activity's push job pushed, else its select head fails without the change in two places:
  • With the old needs list it fails with report-stop-status needs apply-dependency-patch.
  • With the new needs list and the old HEAD_SHA expression it fails with apply-conflict-patch over resolve-conflicts-select: 'before' !== 'pushed'.
• node --test over workflow-gates, feedback-dispatch, dependency-dispatch, fix-ci-dispatch, dispatch, refire-refused, maintenance-monitor-signals/collect and pr-merge/enable-auto-merge: 721 pass, 0 fail.
• Shell tests that read pr-unblock.yml, all 0 fail:
  • scripts/pr-unblock-trusted-base.test.sh: 29 pass
  • scripts/read-runner-dispatcher-gate.test.sh: 10 pass
  • scripts/check-read-caller-keys.test.sh: 57 pass
  • scripts/ci-rerun-ci-status.test.sh: 291 pass
  • scripts/plan-test-lanes.test.sh: 58 pass
• actionlint on pr-unblock.yml reports only the existing cache-mode and concurrency.queue keys its version does not know, and nothing on the changed lines.

Related

• fix(discovery): give the research Effort-ceiling sentence headroom in the re-attach slice #12647
• Run 41575318920

🤖 Generated with Claude Code

https://claude.ai/code/session_01VcupomBPg7kuobwLoNpxEW

[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-11-03T07:11:54.878097Z	da343b9	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 6806423e3640c88b41b886189a47e4274743fff0: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12743 (head 6806423)

I found no security issues in this change.

I read every hunk of all 5 changed files in .claude-lane/pr.diff. Two of them are on the trusted-path list and got extra scrutiny: .github/workflows/pr-unblock.yml and .github/scripts/pr-merge/enable-auto-merge.mjs. Since the last review (head da343b9b, 0 findings), two things are new: report-escalation gains a needs entry, and .github/actions/resolve-config/callers.test.mjs is updated.

What was checked

• report-stop-status: the new HEAD_SHA source. HEAD_SHA now prefers each activity's push-job pushed-commit and falls back to the select head (L3487-L3491).
  • The value reaches the shell only through env:, never through direct expression interpolation in run:.
  • It comes from a base-branch script's output, and report-escalation.sh still checks the SHA format before using it.
  • The job's permissions: block is unchanged: contents: read and statuses: write (L3463-L3465).
  • Effect on the merge hold: the failing stop status now lands on the commit the lane actually pushed, which is the pull request's live head. That tightens the ci-status hold pr-merge reads; it does not weaken it.
• The needs list now includes apply-dependency-patch in both jobs. It is added to report-stop-status (L3425) and to report-escalation (L3530).
  • A new dependency could, in principle, skip the escalation job when the dependency is skipped or fails. I checked both if: conditions. report-stop-status starts with !cancelled() (L3433). report-escalation uses !cancelled() and an always() branch (L3551-L3566). Neither gets the implicit success() check, so a skipped or failed apply-dependency-patch does not stop either job.
  • apply-dependency-patch already ran before both jobs through apply-dependency-verdict, so the order of jobs does not change.
  • report-escalation reads no output from the new dependency, and its permissions and inputs are unchanged.
• enable-auto-merge.mjs. Only the header comment changes, to list the HOLD_LABELS the code already reads. The code is unchanged.
• Test files and the .changes/ entry. callers.test.mjs, workflow-gates.test.mjs and the changelog fragment have no runtime surface.
• Trust controls. The trusted-actor list, the token scope, the kill switch, the token broker and the actions under .github/actions/ are all untouched. No control is widened.
• PR context. The description contains no embedded instructions aimed at the review lane.
• Prior findings. The one earlier post is a 0-finding summary, so nothing carries over.
• Spend. None. The added needs entry runs nothing and makes no API calls.

critical-findings: 0

[github-actions]

claude-review reviewed 6806423e3640c88b41b886189a47e4274743fff0: read 5 of 5 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings

I read every hunk of all 5 changed files. Two are on trusted paths, .github/workflows/pr-unblock.yml and .github/scripts/pr-merge/enable-auto-merge.mjs. For those I read the whole change and the jobs it gates.

Since the last review (da343b9b)

• report-escalation now also needs apply-dependency-patch (L3530). This keeps assert.deepEqual(job.needs, escalation.needs) true (callers.test.mjs L684).
  • The job's if: already starts with !cancelled(), so a skipped or failed apply-dependency-patch does not skip report-escalation.
  • apply-dependency-patch already ran before this job through apply-dependency-verdict, so the job order does not change.
  • report-escalation reads no output of apply-dependency-patch. The new NEEDS_ONLY exception in the PATCH_LANES loop still asserts that (reads stays []).
• callers.test.mjs adds a PUSHERS map to the existing report-stop-status test:
  • It checks that the HEAD_SHA expression is exactly each push job's pushed-commit followed by its select job's head-sha.
  • It evaluates the expression with and without a push for every activity. A push job's commit wins over its own select head, and an activity with no push falls back to its select head.

report-stop-status HEAD_SHA (pr-unblock.yml L3487-3491)

• Each pushed-commit comes before its select job's head. This fixes the defect the PR describes: a reply or verdict job that stops after the push now marks the pull request's real head.
• The job's if: still looks only at the select jobs' heads. That is fine: a push job runs only after its select job has set a head.

Other files

• workflow-gates.test.mjs: the new test reads the folded HEAD_SHA scalar and checks every push job is in needs. It evaluates both the push and no-push case for each activity.
• enable-auto-merge.mjs: the header comment now lists the HOLD_LABELS the code reads. It is a comment-only change.
• Changelog fragment: bump: patch fits a CI bug fix.

Notes

• The PR closes no issue, so there are no acceptance criteria to check.
• Prior findings: the only earlier post is the round-0 summary, which had no findings. This is round 1 of 3, so the cap does not apply.
• The test results and the "fails without the change" results in the PR description are the author's claims. I did not run them.
• Spend: the PR expects no change in API calls per run (author's claim, unverified). The added needs entries start no new job.
• The PR leaves one gap open, marked UNVERIFIED: if apply-patch-rung-up pushes and then fails, the stop status may land on the head before the push. That is outside this change.