Skip to content

perf(pr-pipeline): re-fire broker-check breaker denials on the first sweep after the clear - #10383

Merged
kyle-sexton merged 3 commits into
mainfrom
perf/refire-breaker-denials-on-clear
Oct 27, 2026
Merged

kyle-sexton merged 3 commits into
mainfrom
perf/refire-breaker-denials-on-clear

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: efficiency plan item 11 (.work/gh-monitor/efficiency-plan-2026-10-26.md), owner direction 2026-10-26

Summary

When the lanes kill switch stops a work-resolve-issue run at broker-check, no model runs. Today the work-unblock sweep still waits limits.reset-clamp-min (1 hour) after the run ended before it re-fires the run. Two facts make that wait unnecessary for this case:

  • The sweep stops before any read unless the kill switch reads proceed (label-unblocked.mjs:1565). So a sweep that reaches the re-fire code has already seen the switch off.
  • The hub breaker clears only when more than 40% of the window is left and no refusal was seen. That makes an immediate re-trip unlikely. Basis: the hub's local sampler script, which is not in this repository (TRIP_PCT=20 CLEAR_PCT=40).

So a broker-check trip now re-fires on the first sweep after the switch clears.

Expected saving:

Fix

  • didNoWork now returns the trip's stage:
    • check when broker-check refused, or when its kill-switch step skipped the broker ask;
    • token when a Request the lane token step was refused (label-unblocked.mjs:814).
  • A check-stage trip under limits.limit-stops-per-issue is due at once (label-unblocked.mjs:1187).
  • A token-stage trip still waits the clamp. That run already ran the model, and re-firing it runs the model again.
  • Every cap and trust check is unchanged:
    • the trip count, and the stranded backoff past limit-stops-per-issue;
    • the WIP cap pause (wipCapped());
    • limits.restarts-per-run;
    • the untrusted-provenance and blocker refusals;
    • the listed-human resets.
  • No workflow file and no permission changes.
  • Updated the header comment, the docs/conventions/pr-pipeline.yaml label-unblocked description, and a pr-pipeline patch fragment.

The clear event cannot start the sweep. The hub sets CLAUDE_LANES_REPO_DISABLED with gh variable set, and a variable change raises no workflow event. A workflow_dispatch of work-unblock.yml at clear would be refused by the broker: melodic-software/azure-iac src/lanes-token-broker/src/claims.mjs:21 sets UNBLOCK_EVENTS to issues and schedule, and claims.mjs:213-214 denies any other event with event-not-allowed. Allowing it is a trust-path change at the broker, so it is left out. A hub-side relabel of agent-ready was also rejected. It would bypass the re-fire record, the trip count and restarts-per-run.

Post-model token losses are not fixed here. At least 2 runs, 40261927981 and 40261974907, ran the model and then lost the lane token at apply-issue-patch or report-outcome. Moving the token check before the model would not have saved them:

  • A pre-model check already exists: broker-check asks the broker before the run job.
  • Those runs started at 03:00-03:06Z, and the breaker tripped at 03:56Z, during the model run. No earlier check could have seen it.
  • Minting the write token before the model and carrying it to the post-model jobs would put a write token across the read/write job split that ADR 0049 sets. An installation token also expires within the hour, which is shorter than the observed 3,649 s model run plus slot queueing.
  • What would save them is a bounded retry or hold on a kill-switch 403 in request-lane-token's token mode. That calls the broker during a ban-safety trip, so it needs its own decision and is left as a separate follow-up.

Verification

  • Tests came first. Two changed expectations failed on the old code and pass now:

    • a broker-check trip that ended 30 min before the sweep re-fires;
    • a green kill-switch-skipped run that ended 2 min before re-fires.

    The old test asserted the 1-hour wait for the broker-check case. Changing that expectation is the intended behavior change, not a weakened test.

  • New tests pin what must not move:

    • a post-model mint trip that ended 30 min ago still waits;
    • neither broker-check trip shape re-fires while the issue lane is paused over its cap.

    The existing past-limit-stops-per-issue backoff test still passes unchanged.

  • node --test .github/scripts/work-unblock/*.test.mjs: 188 pass, 0 fail.

  • node --test .github/actions/resolve-config/*.test.mjs .github/actions/check-wip-cap/*.test.mjs: 748 pass, 0 fail.

  • scripts/workflow-self-paths.test.sh: PASS=40 FAIL=0.

  • scripts/check-changelog-fragments.sh --check and --check-required origin/main: pass.

  • actionlint on work-unblock.yml and work-resolve-issue.yml: clean. Neither file changed.

  • Re-measure after the next trip: the hours from a broker-check kill-switch refusal to its re-fire record. Source: events.log LANE-OUTCOME and the issue's breaker record, which names the refused run and its end time since fix(work-unblock): name the refused run and its end time in a breaker re-fire record #10337.

Related

🤖 Generated with Claude Code

kyle-sexton and others added 3 commits October 26, 2026 19:09
…sweep after the switch clears

A work-resolve-issue run the lanes kill switch stopped at broker-check ran
no model, and the work-unblock sweep proceeds only with the kill switch
off. The sweep now re-fires it at once instead of waiting
limits.reset-clamp-min hours after the run ended. didNoWork returns the
trip's stage; a trip at a post-model `Request the lane token` step still
waits the clamp, since its re-fire runs the model again.

The trip count, the stranded backoff past limits.limit-stops-per-issue,
the WIP cap pause and limits.restarts-per-run are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

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 🔄 Running since 2026-10-26T23:21:16.951616Z 103e19c 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

Copy link
Copy Markdown
Contributor

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

No security issues found in this pull request.

Scope read: all 4 changed files in .claude-lane/pr.diff: label-unblocked.mjs, its test file, a changelog fragment, and docs/conventions/pr-pipeline.yaml. I also read the killed, checked and minting definitions around label-unblocked.mjs:799-803 to check the new stage classification.

Trusted path: docs/conventions/pr-pipeline.yaml matches a trusted path. Its change is a YAML comment only, inside the label-unblocked activity description. No lane slot, activity effect, merge setting or grant changed, so what the broker mints is the same. This is not a trust-control change.

What the change does: a work-resolve-issue run that the lanes kill switch stopped at broker-check is now re-fired on the first work-unblock sweep after the switch clears. Before, it waited limits.reset-clamp-min after the run ended. A trip at a mint step still waits the clamp.

Why no finding:

  • The kill switch is not weakened. The PR says the sweep stops before any read unless the kill switch reads proceed. I did not re-read that code (label-unblocked.mjs:1565), because it is outside the diff. The change only removes a wait that sits after that check.
  • The stage tag is derived from job and step conclusions (killed || checked ? "check" : "token"). It does not come from user text. Both checked and killed mean broker-check stopped the run before the run job, so no model ran.
  • The caps and trust checks run before the changed line (stoppedUntrusted, WIP cap) or after it (limits.restarts-per-run). The trip count, the limit-stops-per-issue stranded backoff (over >= 0 still uses backoffDue), the untrusted-provenance refusal and the WIP cap pause are not touched. The new test pins the WIP cap pause for both broker-check trip shapes.
  • A repeated trip after the switch clears still adds a breaker record, so the trip count and the backoff bound it. This is a repeat-run bound, not a new bypass.
  • Equal-rank handling is unchanged: atLeast keeps the first trip when ranks are equal, so an earlier token trip is not turned into a check trip.
  • No workflow, permission, token-scope or broker-side change.

Cost: this adds at most one extra lane run per tripped issue. That spends prepaid API credits or subscription usage only, so it is not a hold.

manifest: no plugin manifest changed.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed 103e19c5d2171e71ad782b0375c1652a45ad3a65: 0 finding(s), 0 as inline comments.

No findings. I read .claude-lane/pr.diff and .claude-lane/pr-context.md, and read didNoWork and refireRefusal in label-unblocked.mjs directly. I did not run the tests. The 188 and 748 passing counts in the PR description are author-claimed and unverified by me.

What the change does. didNoWork now returns a stage. It is check when broker-check refused or its kill-switch step skipped the broker ask. It is token when a mint was refused. refireRefusal sets the clamp to 0 only for stalled === "breaker" && stage === "check". Because due = ended + 0 there, a broker-check trip re-fires on the first sweep after the switch clears.

What I checked.

  • Precedence. The ternary killed || checked ? "check" : "token" parses as (killed || checked) ? ..., which is the intended reading.
  • Mint-failed runs. The mint-failed and capped runs call atLeast with no stage. That is harmless, because the clamp only reads stage when stalled === "breaker".
  • Cross-run ranking. A run with a mint-stage breaker can outrank a check-stage one only through the existing equal-rank "first stays" rule. A later mint-stage trip never downgrades an earlier check-stage one, and the reverse also holds. So stage always matches the run recorded in trip.
  • Backoff past limit-stops-per-issue. That path is unchanged: over >= 0 still uses backoffDue, and the WIP cap check and restarts-per-run still follow the due check.
  • resets. It now equals ended for check-stage trips. It is only used to order candidates (the ready.sort and clampReset paths), so the earlier value cannot unlock anything the due check did not.
  • Tests. They cover the new immediate re-fire for both trip shapes and the unchanged one-hour wait for a mint trip. They also cover the WIP-cap pause for both check-stage shapes, and the over-limit backoff test is unchanged.
  • Docs and fragment. The pr-pipeline.yaml comment, the header comment and the .changes fragment (bump: patch) agree with the code. A patch bump fits a timing change in a sweep.

Closing issue. The PR states "No related issue", so there are no acceptance criteria to check. The PR body contains no embedded instructions.

Manifests. The diff touches no plugin manifest.

Cost. The only effect is that re-fired lane runs start about an hour earlier. That spends prepaid or subscription usage and is not a hold.

Security. The security surface did not change. The sweep still runs only with the kill switch off, no workflow or permission files changed, and the broker-side trust path is left alone.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 27, 2026
Merged via the queue into main with commit 80e8440 Oct 27, 2026
40 checks passed
@kyle-sexton
kyle-sexton deleted the perf/refire-breaker-denials-on-clear branch October 27, 2026 02:04
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