Repository navigation
fix(pr-pipeline): route the issue lane's non-owner stops away from a person - #10978
Conversation
…person stop-routes.mjs says only an intent, design, security or cost reason holds for a person, yet the table held an uncoded stop, a split and a wait on an unmerged pull request. - Every hold row names its class; a test fails any hold row whose class is not intent-change, design-change, security-risk or cost-risk. - An uncoded stop retries once (the row's own retries), then fails. - split-required takes a new intake route: agent-ready, status: ready and admission-deferred come off, and the triage sweep, which now counts outcome=intake as a hand-back, sends the issue to intake, whose split files its sub-issues (owner Q93). - blocked-by-pr takes a new wait route: agent-ready stays with an outcome=wait pr=<n> marker, and work-unblock restarts the issue once that pull request merges or closes, its waits counted apart from retries under limits.stop-retries. GitHub's blockedBy is an IssueConnection, so a pull request cannot be a blocked_by edge. - already-done and report-delivered take a new close route: the issue closes as completed, already-done only once the cited merged pull request or commit is on the default branch, report-delivered only on a work-class: read-only issue with no commit. Either retries otherwise. - /work-items:work teaches the three new codes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- A shared module, .github/scripts/lib/stop-reason.mjs, writes and validates the stop-reason marker `<!-- stop-reason lane=<lane> reason=<code> -->` and the visible line "Reason: `<code>`", the code one of intent, design, security, cost or none; it refuses none on a comment that reaches a person. - The issue lane's hand-back and dimension-skip comments carry both, the code taken from the hold row's class. A test checks every hold row's comment. A reason the route table does not list now fails instead of holding. - collect-base-activity copies the base .github/scripts/lib/ beside a script's base copy for its ../lib/ imports; the hold floor lists the new module as enforcement, and sync-hold-floor regenerated its copies. - The intake route is capped by its row's retries (1): a split child that keeps Size Large past admission fails on its second intake instead of looping through triage. - A wait on a pull request that is not open, and an already-done stop with no evidence on the default branch, say why they retry; already-done tries up to three SHA-looking words. - /work-items:work names the four codes that reach a person. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe3f660ac1
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
claude-security-review reviewed critical-findings: 0 Security review of #10978 at feb8a1c. I read the whole diff (32 files) and the PR context, and found no security issues. I could not run tests. I read the code only. Trust-control widening: the lanes App's
Other trust-adjacent changes I read and found tighter or neutral
Owner threat-model line (throughput phase): an uncoded manifest: this diff has no Spend: no change to turn limits, schedules or paid resources, and no charge-bearing change. There is no |
|
claude-review reviewed Code review: no findingsI read all 32 files of the diff and the PR context. I did not run any tests, because this session has no shell. The test results in the PR description are the author's claims, and I did not check them. Closing issue: none. The PR context says "No related issue", so there are no acceptance criteria to check. Manifests: the diff changes no What I checked, by reading the code:
Minor observations, not counted as findings:
Spend: the new |
…the issue doneEvidence took any merged pull request or any commit on the default branch as proof, so run 40844966342 closed issue #10858 on release commit abb359b, which never touched it, and the close admitted its dependents. A cited merged pull request or default-branch commit now counts only when its title, body or message cites #<issue> as a whole token, or it changes a path the issue body names. Any failed read still proves nothing, and the stop retries instead of closing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Security review on #10978: - runCode reads a stop code in any case, past emphasis, backticks and a Reason: label, with a space or hyphen between its words; a reason with no code that names security, intent, design or cost as a word takes that hold row, so a misformatted owner code reaches a person instead of retrying and failing. - doneEvidence counts a citation only as a closing keyword before #<issue>, takes a pull request only when merged into the default branch, takes evidence only from after the issue opened, and matches only a named path that holds a directory. - A blocked-by-pr wait counts only a pull request the lanes App or a listed human opened, by the same author check closed-by-open-pr uses. - stop-routes.json does not load when a hold row's class is not an owner reason. - The triage sweep's hand-off search counts a hand-back only from the lanes App's account, as readState does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
runCode's backstop held an uncoded stop that named intent, design or cost as a word, so plain reasons such as "the test design needs X" or "the cost of retrying" reached a person, against the owner's rule that only real owner reasons do. Only a whole-word `security` or `vulnerab*` now routes an uncoded stop to the security-risk hold; the tolerant code parsing stays. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Re-review of 603166e on #10978: - runCode reads the reason NFKC-normalized, takes the security code with a hyphen, underscore, space or nothing between its words, and holds an uncoded reason that contains `security` or `vulnerab` anywhere, so SECURITY_RISK, SecurityRisk and full-width forms reach a person. - Every other code needs its hyphenated form followed by a colon, a backtick, closing emphasis, a space or the end, so "Design change in X" no longer holds for the owner. - doneEvidence takes only a closing keyword, from the merge commit of a cited pull request or from a cited commit; the issue-body path fallback and bodyPaths are gone, and an edited pull request body proves nothing. - stop-routes.json does not load without a security-risk hold row of class security-risk, or with a class on a row that does not hold. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hidden security words A hyphenated code now ends at any non-word character, so design-change. and design-change, hold again; format and combining characters are stripped and vuln counts, so secu<ZWSP>rity and vuln: hold as security; a closing keyword needs #<issue> followed by no word character. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ate the shared retry cap Review threads on #10978: - doneEvidence tries the first three cited #<n> other than the issue, so "duplicate of #50, fixed by #60" reads #60's merge commit instead of retrying on #50's 404. - A stop row's `retries` caps the issue's shared count of retry markers since a listed human last added agent-ready, whatever stop wrote them; the stopped and retry-spent whys, the route header, the convention, the work skill and the fragments now say so. - pr-run-activity says a reason the table does not list fails, as stopRoute does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A report-delivered stop on an issue that is not work-class: read-only, or whose run left a branch, retried with only the close wording. Pass a why line, as the already-done retry does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The pr-verify verifier failed head What it wrote (35 lines)This pull request leaves the merge queue, and ci-status holds it out while it carries |
|
The pr-unblock lane stopped
When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean. To run it again on this head now, dispatch the gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10978 -f activity=resolve-conflicts |
…d more security words doneEvidence now takes only work merged after the issue was last reopened, reading the issue's events and failing closed when they do not read whole, so the lanes App's already-done close no longer counts as trusted past a person's reopen. SECURITY_WORD adds password, API key, private key, backdoor, exploit, whole-word XSS/RCE/CVE and "ignore ... instructions". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…everted closes An uncoded stop naming branch protection now holds as security-risk, and the docs and changelog list every security word the code matches. A "does not fix #N" line or a revert commit no longer counts as already-done evidence. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-rule # Conflicts: # .github/scripts/intake-triage/admit.test.mjs
No related issue: verified audit findings, .work/gh-monitor/audit-2026-10-29/route-automation-lanes.md
Summary
The issue lane's route table (
.github/scripts/work-resolve-issue/stop-routes.json) sent stops to a person that name none of the owner's reasons for one, against its own header (stop-routes.mjs: "Only an intent, design, security or cost reason routes here"). An uncodedstopped,split-required, a wait on an unmerged pull request, an issue whose work was already on main and a read-only report all ended atneeds-human. A wait on a pull request also stayed parked after that pull request merged, because nothing cleared it.Fix
holdrow names itsclass:intent-change,design-change,security-riskorcost-risk. A test fails any hold row with another class.pushbackisintent-change. The four trust gates (scan-refused-output,collect-refused,issue-edited,patch-invalid) aresecurity-risk.research-firstandprobe-firstaredesign-change.stopped: it retries once (the row'sretries: 1) and then fails, never reaching a person. A reason the table does not list now fails instead of holding.blocked-by-pr→ newwaitroute. GitHub'sIssue.blockedByis anIssueConnection(GraphQL introspection), so a pull request cannot be ablocked_byedge. Instead, the issue keepsagent-readywith anoutcome=wait pr=<n>marker. work-unblock's sweep restarts it once that pull request merges or closes, and counts its waits againstlimits.stop-retriesseparately from retries. If the pull request named is not open, or none is named, the stop retries instead.already-done/report-delivered→ newcloseroute. The issue is closed as completed:already-donecloses only after the lane confirms the cited merged pull request, or the cited commit, is on the default branch.report-deliveredcloses only awork-class: read-onlyissue the run committed nothing for, and posts the run'ssummaryas the report.split-required→ newintakeroute. It removesagent-ready,status: readyandadmission-deferred. The triage sweep now countsoutcome=intakeas a hand-back, so it sends the issue back through intake, whose split files the sub-issues (owner Q93). The route allows one pass (retries: 1): a second one since a listed human addedagent-readyfails, so a split child that keeps Size Large cannot loop.already-doneclose counts as a trusted close (owner Q9, 2026-10-29).isTrustedClosein.github/actions/check-trusted-trigger/check-trusted-trigger.mjstakes anisApppredicate. A close by the lanes App counts only when the App's last close comment namesalready-doneand the new.github/scripts/lib/done-evidence.mjsre-reads that the cited pull request or commit is on the default branch with a message that closes the issue. Areport-deliveredclose, or any failed read, counts for nothing. Its callers pass the predicate: intake-triageadmit.mjs(a blocker the App closed as done admits its dependents) and work-unblocklabel-unblocked.mjsclosedByHumanOrMerge(it unblocks dependents and can close split parents).done-evidence.mjsis on the hold floor astrust, andsync-hold-floor.pyregenerated the copies.SECURITY_WORDinstop-routes.mjsis wider, and a security word after aclose,host-runorattendedcode now holds assecurity-risk. This holds more stops for a person, never fewer..github/scripts/lib/stop-reason.mjs, writes and validates<!-- stop-reason lane=<lane> reason=<code> -->and a visible line,Reason: `<code>`. The code is one ofintent,design,security,costornone, and the module refusesnoneon a comment that reaches a person.collect-base-activitycopies.github/scripts/lib/next to a script's base copy, so its../lib/import resolves.enforcement, andsync-hold-floor.pyregenerated the copies./work-items:workdocuments the three new codes and names the four that reach a person.Verification
node --testpasses in.github/scripts/lib(4),work-resolve-issue(388),intake-triage(633),work-unblock(224),maintenance-audit-lane-runs(81) andpr-merge(645).outcome=intake;sync-hold-floor.py --checkis clean.scripts/check-affected.shpasses on the branch.Related
.work/gh-monitor/audit-2026-10-29/route-automation-lanes.md(local).split-required). It does not settle that issue.refine.mjs).admit.mjs,apply-verdict.mjsandrefine.mjs, intake-reenterreenter.mjs, work-unblocklabel-unblocked.mjsandtriage-sweep.mjsontostop-reason.mjsafter fix(pr-pipeline): keep the issue body untouched when intake splits off person-only done-when items #10951 merges. This PR changeslabel-unblocked.mjsandtriage-sweep.mjsonly for the wait restart, the intake hand-back and the App done-close predicate.🤖 Generated with Claude Code