Skip to content

fix(lanes): send an oversize-patch scan refusal to intake for a split - #12931

Merged
kyle-sexton merged 4 commits into
mainfrom
fix/too-large-split-route
Nov 3, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
fix/too-large-split-route

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: hub drain analysis 11-03 (#12700 retried twice on an oversize patch)

Summary

A run whose patch the scan job refuses for its size (scan-refused-patch) was retried, and a fresh run on work too large for one patch fails the same way: #12700 hit the 262144-byte cap on both retries (11:52Z and 19:17Z, 2026-11-03), each one a model run spent. This sends that refusal to intake for a split instead, as split-required already does.

Fix

  • .github/scripts/work-resolve-issue/stop-routes.json: scan-refused-patch takes split-required's route, "route": "intake", "retries": 1, so intake's split (owner Q93) files its sub-issues. scan-refused-fault keeps its retry: a job fault says nothing about the work's size.
  • docs/conventions/pr-pipeline/pr-run-activity.md: the convention line states the new route.
  • report-outcome.test.mjs: the oversize-patch test now expects outcome intake.

The route came in with #10533 as a retry ("a fresh run may make a smaller one"); no owner decision set it. The other half of the hub's report, #12772's two already-done retries, needs no change: the close route closes only when the cited PR says Closes #<issue> (.github/scripts/lib/done-evidence.mjs:57), and the cited PR #12769 says only Refs: #12365.

Verification

  • node --test .github/scripts/work-resolve-issue/report-outcome.test.mjs: 393/393 pass.
  • scripts/check-affected.sh --base origin/main --no-fetch: 11 passed, 4 skipped.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01MWmASNtP63MZxdDeoxYztD

kyle-sexton and others added 2 commits November 3, 2026 14:58
A scan refusal for patch size (scan-refused-patch) routed to retry, so a
fresh run re-spent a model run on work too large for one patch: #12700
failed the same way on both retries. It now takes split-required's
intake route, one re-entry, so intake's split files sub-issues.

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:10:03.078017Z 09ab823 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.

@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Security review: no security issues found.

I read every hunk of all 9 changed files. Three of them are on trusted paths, so I read their whole change and the code it gates: .github/scripts/intake-triage/admit.mjs, .github/scripts/work-resolve-issue/stop-routes.json and docs/conventions/pr-pipeline.yaml. Since the last review (at 09ab8237), the PR has added admission logic: patchCapStopped and PATCH_SPLIT_PENDING. I judged that logic again here.

  • stop-routes.json: scan-refused-patch moves from retry to "route": "intake", "retries": 1, the same route split-required uses (L4). The security holds are unchanged: scan-refused-output and collect-refused still have "route": "hold" with class: security-risk.

  • admit.mjs patchCapStopped: this cannot be forged by anyone else.

    • It counts only a comment whose author passes isAppId against the lanes App id (L1696). The comment must also start with an anchored outcome=intake marker and the fixed stopLine sentence.
    • The flag is cleared only when a listed human (humans.has(event.actor?.id)) adds agent-ready again.
    • A comment from an outsider can only produce a wait, which is the more restrictive result.
  • decide: the new overCap branch can only push onto parked, and a parked issue gets a wait.

    • refuse is still checked before parked (L1574-L1590), so no refusal or hold is skipped. No issue is admitted that the base code would not have admitted.
  • refine.mjs / label-unblocked.mjs: these now accept either reason in SPLIT_PENDING. The split's own checks are unchanged:

    • only an issue a listed human filed is split (refine.mjs L326);
    • the run needs exactly one intake lane comment of its own;
    • the model sees only trusted task-list lines (splitTaskLines).

    So no new untrusted text reaches a lane that holds credentials. refreshSplitWait only checks the reasons in the App's own wait comment.

  • pr-pipeline.yaml: only a comment line changed. The lane slots and activity effects that decide what the token broker mints are the same.

  • No change to the trusted-actor check, token scope, the kill switch or any merge-hold file.

  • Other files: the tests, the changelog fragment and the convention doc have no security surface.

Spend (not a finding): a patch the scan job refuses for its size now goes to intake for a split, at most once, instead of a fresh retry. That spends an intake model run, plus the split's run, in place of a resolve-issue retry.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 554f3eae0c12392b8fea1e5ff1342732ff55c087: read 9 of 9 changed file(s); 1 finding(s), 0 as inline comments.

Code review: PR #12931, send an oversize-patch scan refusal to intake for a split (round 2)

I read every hunk of all 9 changed files. To trace the new mechanism end to end, I also read admissionFacts and the admit() caller in admit.mjs, admissionDecision in label-unblocked.mjs, stopLine in done-evidence.mjs, and selectSizeSplit/renderSplit in refine.mjs.

Prior finding (round 1, stop-routes.json:4): addressed. Before this round, a scan-refused-patch issue would have been re-triaged at Small or Medium and admitted again. The new patchCapStopped check fixes that: the lanes App's own outcome=intake comment with the scan-refused-patch stop line now makes decide park the issue as PATCH_SPLIT_PENDING whatever its Size. A listed human adding agent-ready again clears it.

  • Both decide callers get comments and events from admissionFacts: admit() (admit.mjs:2555) and admissionDecision (label-unblocked.mjs:951).
  • refine.mjs sizeSplitPending and label-unblocked.mjs refreshSplitWait now accept both wait reasons, so the parked issue reaches the split.
  • The new test in report-outcome.test.mjs feeds the real stop comment to patchCapStopped. That links the marker format to the reader.
  • A split child's rules take Size Large, so it never re-splits, and the tests cover that case.

Finding (1, advisory, not on a changed line): the split comment still tells a Small or Medium issue that it is Size Large. This PR made the postDecline text Size-neutral (refine.mjs:197). It missed the two other texts a patch-cap split now reaches:

  • refine.mjs#L852: renderSplit posts "Split at intake (owner Q93). This issue is Size Large, so its done-when is split into N sub-issues…" on the issue itself.
  • refine.mjs#L346: the refinement reason reads "Size large, waiting on its split into sub-issues".

How it fails: a Medium issue that hits scan-refused-patch is parked as PATCH_SPLIT_PENDING and then split. The lanes App then posts a public comment saying it is Size Large, which contradicts the Size field shown on the issue. Fix: make both strings Size-neutral, as postDecline now is, or pick the wording from the wait reason.

Other checks

  • Closing issue: none. The PR body cites the 11-03 hub drain analysis and docs(plugin-options): remove restated Claude Code behavior from the generated README block #12700 and links no closing issue, so there were no acceptance criteria to check.
  • Path-scoped rules: no criteria in them are broken by the changed files.
  • Plugin manifests: the diff touches none.
  • Prior findings: the one inline entry is marked lines changed since posted: yes, and this round's code answers it, so it is not re-posted. Round 1 of 3; the cap does not apply.
  • Settled threads: the wrapper named no settled-threads file.
  • Embedded instructions in the PR context: none found.
  • Verification: the PR body's test results (393/393 pass; check-affected 11 passed, 4 skipped) are claimed by the author and come from before this round's admit.mjs change. I ran nothing, because this lane has no shell, so I have not checked whether the new admit.test.mjs cases pass.

Spend: this lowers model-run spend. An oversize refusal now costs one intake re-triage and a split instead of up to limits.stop-retries lane runs. That is prepaid or subscription usage only, so it is not a hold.

@kyle-sexton kyle-sexton removed the do-not-merge Hard merge gate: do not merge while applied. label Nov 3, 2026
kyle-sexton and others added 2 commits November 3, 2026 15:50
intake splits only an issue parked at Size Large, and re-triage rates
Size from the issue text alone (a listed human's Size is kept), so a
scan-refused-patch issue came back at Small or Medium, was admitted,
refused again and ended at retry-spent with no sub-issue.

Admission now parks an issue at any admitted Size for intake's split
when the lanes App's report-outcome comment routed a
scan-refused-patch stop to intake after a listed human last added
agent-ready (patchCapStopped). It reads the comments admissionFacts
already holds, so no new API call. refine.mjs and label-unblocked
accept the new pending reason beside Size Large's.

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 24139bf Nov 3, 2026
51 checks passed
@kyle-sexton
kyle-sexton deleted the fix/too-large-split-route branch November 3, 2026 21:38
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