Skip to content

fix(lanes): re-check an issue's blockers once its run holds a slot - #12933

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/recheck-blockers-in-slot
Nov 3, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
fix/recheck-blockers-in-slot

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: hub drain analysis 11-03 (18 of 33 non-delivered runs lacked a prerequisite on main)

Summary

work-resolve-issue.yml checks an issue's blockers only in admit, before the run waits for its slot (SLOTS: 3, queue: max; median wait 12 min, p90 53 min). A blocked_by edge added during that wait was never seen, so the model ran anyway and ended in retry, wait or stop. Example: #12669 was labeled agent-ready at 04:09Z and got its blocked_by at 06:18Z, after 3 dispatches.

Fix

  • Where the re-check runs: in pr-run-activity-read.yml's broker-check job, step Re-check the issue's blockers in the slot (id: recheck-blockers).
    • Every job of the read runner runs inside the calling job's work-resolve-issue-slot-N group, so this step runs after the slot is acquired and before the run (model) job.
    • A separate caller job would have released the slot when it finished, and resolve-issue would then have queued for it again.
    • It runs after the lease claim, gated on inputs.issue-number != '' && inputs.fallback == '' && steps.lease.outputs.lease == 'held'.
    • Only the first call checks. The subscription and fallback legs pass fallback: handoff / fallback: fallback, so they skip it.
  • The check is admit's own script: check-blockers.mjs, with a new RECHECK=true mode.
    • It reads the blocked_by list once. An empty list lets the run on; an open blocker stops it, with the same notice and outputs admit's check gives.
    • A list of closed blockers only takes the full check, since only that check judges who closed them.
    • A read that fails stops the run, as it does at admit.
  • How a blocked run stops: the step Stop on an open blocker runs and broker-check sets blocked=true. The run job's go step then sets run=false, and issue-prior is skipped, so no reuse branch is named.
    • No model turn runs and no outcome marker is posted. agent-ready stays on.
    • The read runner's report job ends the lease, because issue-run-started is false.
  • The sweep routes it the same way: label-unblocked.mjs didNoWork now counts that stop step in any resolve-issue call as blocked, the same route as admit's Wait for open blockers. The issue waits while a blocker is open, then is re-fired with no needs-human and no per-version bound.
  • Permissions and API budget:
    • No job's permissions changed. broker-check already held issues: write for the lease. The model job still holds only contents: read and pull-requests: read.
    • No new reusable-workflow inputs.
    • The new calls go through broker-check's job-level API_BUDGET_TOKEN and are counted by its if: always() log-api-budget step.

Verification

  • node --test .github/scripts/work-resolve-issue/check-blockers.test.mjs: 27/27 pass. New cases cover RECHECK mode: no blocker (exactly one GET), an open blocker (one GET, same outputs and notice as admit), closed blockers only (full check), and a failed read (stops the run).
  • node --test .github/scripts/work-unblock/label-unblocked.test.mjs: 301/301 pass. New cases:
    • an in-slot stop waits while blocked;
    • once clear it re-fires with the same writes as admit's gate, including past the bound;
    • a re-check that let the run on is no blocker stop;
    • the step-name test pins Stop on an open blocker after the lease step.
  • node --test .github/actions/resolve-config/callers.test.mjs: 237/237 pass (merged with main). The new test pins:
    • the slot group on the three calls;
    • the step's run and env against admit's step;
    • its position after the lease and before run;
    • that the budget step is last and always();
    • unchanged permissions;
    • its if (first call only, issue mode only, lease held);
    • the stop step, the blocked output, go returning run=false, issue-prior skipped, and the report job releasing the lease.
  • scripts/check-affected.sh --base origin/main --no-fetch: 10 passed, 5 skipped.

Related

  • Cost per dispatch on the first call: 1 extra GITHUB_TOKEN read (GET .../dependencies/blocked_by, one page) when the issue has no blockers or an open one. When only closed blockers are listed, the full admission check runs instead.
  • Not changed here: the shared rule (admit.mjs blockerDone) counts a blocker closed for any reason by a listed human as done (owner decision Q95). The tracker contract's blocked_by_count instead keeps a duplicate or not-planned blocker blocking. That conflict is left for an owner decision; the re-check reuses whichever rule is in force.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MWmASNtP63MZxdDeoxYztD

kyle-sexton and others added 2 commits November 3, 2026 15:09
admit checks blockers before the slot wait (median 12 min, p90 53), so
a blocked_by edge added meanwhile was never seen and the model ran.
The read runner's broker-check now re-runs check-blockers.mjs in a
RECHECK mode after the lease, on the first call only, and an open or
unreadable blocker stops the run at "Stop on an open blocker": no
model turn, no outcome marker, the lease released by the report job.
work-unblock's sweep routes that stop as admit's blocker gate.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MWmASNtP63MZxdDeoxYztD
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

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-03T20:14:55.917452Z 4eab9b4 PR opened
ℹ️ 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

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Code review: no findings

I read every hunk of all 8 changed files in .claude-lane/pr.diff. I also read at head the code the new step relies on: main in check-blockers.mjs (L72-L149) and the broker-check job in pr-run-activity-read.yml (L424-L573). The new head commit merges origin/main. Its diff matches what the earlier round reviewed with no findings, and the merge changed nothing these hunks rely on.

What I checked

  • RECHECK mode (check-blockers.mjs L90-L95, L143-L146). There is one blockersOf read, and stillBlocking treats every closed blocker as done.
    • An empty list writes proceed=true after that single read.
    • An open blocker reaches the existing pending notice and writes the same outputs as admit.
    • A list of closed blockers only falls through to the full check.
    • A thrown read reaches the existing catch, which writes proceed=false. That is a soft stop the sweep re-fires.
  • Workflow wiring.
    • The step runs after lease and before the always-run budget step.
    • .base and Node are set up earlier in the same job, so the script and its admit.mjs imports resolve.
    • The blocked job output is false when the step is skipped.
    • The go step adds [ "$BLOCKED" = true ] and issue-prior adds blocked != 'true', so no model turn runs and no reuse branch is named. The report job then ends the held lease, because issue-run-started is false.
    • The if limits the check to the first call (inputs.fallback == ''). The subscription and fallback legs pass handoff and fallback.
  • Sweep routing (label-unblocked.mjs didNoWork). The new calls.some(endedAt(job, RECHECK_STOP_STEP, "success")) arm sends the in-slot stop to the blocked route. The step-name pin test checks that the stop step comes after the lease step, so checkStopped cannot catch it.
  • Tests. The new cases cover the step's if truth table, the blocked and stop expressions, go, issue-prior, the lease release by report, the sweep's wait and re-fire, and the shell harness row. I couldn't run tests in this lane, so the PR description's pass counts are the author's claim and unverified.

Note, not a finding (unchanged from the last round). Sometimes the in-slot check falls through to the full check and stops for a reason other than an open blocker: an outsider author, untrusted provenance, or an open sub-issue. The sweep still routes that stop as blocked. Once no blocker is open it re-fires the issue, and admit then stops it for good. The cost is one extra admit run, and the end result is still correct.

Spend. Each first call of an issue run makes one extra GITHUB_TOKEN read (GET .../dependencies/blocked_by), or the full admission read set when only closed blockers are listed. None of this is billed.

Other checks

  • The PR names no closing issue, so there were no acceptance criteria to check.
  • The PR context has no embedded directives.
  • There are no prior inline findings. The only prior post is an earlier no-findings summary. The round cap is not reached.
  • No plugin manifest changed.
  • Two changed files match the trusted paths: .github/workflows/pr-run-activity-read.yml and .github/workflows/work-resolve-issue.yml. I read their whole change. No job's permissions changed, and the new step uses the job's existing github.token.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed f53ae54adcca11201938ba17ce32d79a3b5429bb: read 8 of 8 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12933, re-checking an issue's blockers in the slot

No security issues found. I read every hunk of all 8 changed files. Since the last review at 4eab9b42, two things changed: main was merged in, and scripts/read-lane-broker-check.test.sh is now in the diff.

What I checked

.github/workflows/pr-run-activity-read.yml (a trusted path; I read its whole change and what it gates)

  • Where the new step's code comes from. The recheck-blockers step (L550-L559) runs .base/.github/scripts/work-resolve-issue/check-blockers.mjs.
    • .base is a checkout of the default branch with persist-credentials: false (L455-L460).
    • So no code from a pull request runs with GH_TOKEN.
    • Its env values match admit's step: TRUSTED_ACTORS_PATH points into .base, and LANES_APP_ID comes from a repository variable.
  • Permissions. The broker-check permissions: block (contents: read, id-token: write, issues: write, L430-L433) is unchanged. The new step only makes GET requests.
  • When it runs. The step's if: needs steps.lease.outputs.lease == 'held'. The lease step itself already needs the broker's allow (L500).
  • It can only stop a run.
    • The new blocked output, the BLOCKED test in the go step and the extra condition on issue-prior can only set run=false or skip a step.
    • None of them lets through anything the base stopped.
  • It fails closed.
    • When the read fails, the script exits 0 with proceed=false, which counts as blocked.
    • When the script crashes, broker-check fails, and the run job needs broker-check, so it does not start.

check-blockers.mjs RECHECK mode

  • Every blocker is treated as trusted. stillBlocking(blockers, repo, async () => true, async () => true) returns only the open blockers (admit.mjs L276-L280).
  • When the recheck decides alone. rechecked is true only when the list is empty or names an open blocker.
    • With an empty list, the recheck returns proceed=true and skips the author and provenance checks.
    • That output only feeds blocked, so it cannot admit a run.
    • Admission still comes from the caller's admit job, which runs before the slot wait.
  • Closed blockers only. When every listed blocker is closed, the full check runs and decides who closed them. RECHECK mode can only add stops.

label-unblocked.mjs didNoWork

  • A successful Stop on an open blocker step is now routed as blocked, the same route as admit's Wait for open blockers gate.
  • Only the default-branch workflow names that step, and the sweep reads it from the run's job listing.
  • It is not a merge hold: no merge-hold script imports this file.

Tests (callers.test.mjs, check-blockers.test.mjs, label-unblocked.test.mjs, read-lane-broker-check.test.sh): only tests changed here, so there is nothing to attack. The shell test takes the new blocked argument with a default of "".

work-resolve-issue.yml: only a comment changed.

Trust controls

None of them is widened:

  • the trusted-trigger check and the trusted-actor list;
  • the lanes token request and its scope;
  • the kill switch;
  • the broker's grants;
  • the merge holds.

Embedded text

None found in the PR context.

Spend (not a finding)

  • The first call of an issue run makes one extra read with the workflow's own GITHUB_TOKEN. When only closed blockers are listed, it runs the full admission check instead.
  • The sweep may re-fire runs the recheck stopped.
  • Both draw only on existing API and subscription usage.

critical-findings: 0

kyle-sexton and others added 2 commits November 3, 2026 15:23
read-lane-broker-check.test.sh evaluates the run job's issue-prior if
with a fixed context; the new needs.broker-check.outputs.blocked term
was unresolved, so every lease-ownership case printed nothing. The
context now carries it (and the go step's BLOCKED), and a new case pins
that a run the re-check stopped releases its own lease and reports
nothing, even with a lane branch an earlier run left.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01MWmASNtP63MZxdDeoxYztD
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 376168b Nov 3, 2026
64 of 65 checks passed
@kyle-sexton
kyle-sexton deleted the fix/recheck-blockers-in-slot branch November 3, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant