Skip to content

Unstick the pr-reviewer Stage 3 agent: lean prompt, real local-daemon timeouts, and TUI recovery for retry ladders and permission dialogs - #6237

Merged
atomantic merged 5 commits into
mainfrom
cos/task-mtn2g3ch/agent-793d0985
Sep 4, 2026
Merged

Unstick the pr-reviewer Stage 3 agent: lean prompt, real local-daemon timeouts, and TUI recovery for retry ladders and permission dialogs#6237
atomantic merged 5 commits into
mainfrom
cos/task-mtn2g3ch/agent-793d0985

Conversation

@atomantic

Copy link
Copy Markdown
Owner

Summary

The pr-reviewer Stage 3 agent (claude-ollama-tui + qwen3-coder:30b) hung twice on 2026-09-04: first at API error · Retrying in 0s · attempt 1/10 for the whole run, then — once the request finally got through — on a tool-permission dialog nobody was present to answer. Both were diagnosed and fixed at the root, and both hang shapes now recover automatically.

Why it hung

  • The stage prompt was ~400KB (~100K tokens): {reviewChecklist} inlined the entire /do:review command with every lib (~260KB the sandboxed stage cannot use), and a string-form String.replace let a $+backtick inside that body splice the stage prompt back into itself seven times.
  • Ollama sends no bytes at all until prefill completes. Claude Code v2.1.260 has four independent ceilings on that silence, and the binding one is the Bun runtime's own fetch timeout (~360s), which only API_FORCE_IDLE_TIMEOUT=0 disables — so PR Public-review Stage 3 dispatches a ~100K-token prompt to a local model without checking it fits the run's time budget #6117's API_TIMEOUT_MS raise changed nothing. Verified with a fake stalled /v1/messages on loopback: with the flag unset or 1 the client re-sends every 361s regardless of every other knob; with 0 it waits out the prefill. (Ollama's context checkpoints are why the run eventually got through after ~5 retries.)
  • The local model then hallucinated an absolute path outside the worktree, and --permission-mode acceptEdits painted Do you want to proceed?.

What changed

  • Stage 3 gets a new {reviewLenses} placeholder (the five review lenses, ~39KB) instead of the whole /do:review; every placeholder uses a function replacer. Resolved prompt drops from ~400KB to ~50KB before the PR envelope.
  • Local-backed Claude harnesses get API_FORCE_IDLE_TIMEOUT=0 plus the two 30-minute stream-idle knobs, allowlisted through both public-review env boundaries.
  • createRetryStallGate: a retry banner that advances twice across ten minutes fails the run over to a fallback provider (decided at a sighting, never by the clock, so a fast cloud ladder that then succeeded is left alone).
  • createToolPermissionGate: any tool-permission dialog is declined (arrow-down to the last "No", whose number varies by tool) and the quiet session is nudged back to work; eight nudges fail the run. The OOM nudge and this one share one createSilenceNudgeGate. The dialog pattern is shared with the post-mortem analyzer.
  • The pipeline hand-off no longer points a stage at data/cos/agents/<prev>/output.txt (outside every worktree) once the output is inlined.

Test plan

  • server: aiToolkit, tuiHandshake, cliChildEnv, agentTuiSpawning, taskPromptService, agentPromptBuilder, taskPromptDefaults, agentErrorAnalysis, agentCompletionCleanup suites — 1432 passing
  • Mutation checks: reverting the function replacer fails the splice test; removing the stall-gate wiring fails the spawner test
  • Offline Claude Code timeout matrix (12 variants × ~7 min against a fake stalled /v1/messages) pins API_FORCE_IDLE_TIMEOUT=0 as the knob that survives a silent prefill
  • After merge: pm2 restart portos-server so the new child env reaches the next Stage 3 spawn, then re-run the pr-reviewer on a contributor PR and confirm the agent dir shows a ~50KB prompt.txt and no Retrying banner

… backreference splice duplicating it

`{reviewChecklist}` inlined the entire `/do:review` command with every lib
include (~260KB: argument parsing, five reviewer loops, issue filing, PR
posting) into a sandboxed, network-less stage that can use none of it, and
the placeholder was substituted with a STRING replacement, so a `$`+backtick
inside that body spliced everything before the placeholder back in — seven
copies of the stage prompt in one body. On the local Claude wrapper the
resulting ~100K-token prefill outlived every client timeout
(agent-e057cca7, 2026-09-04).

The public-review actions stage now receives only the five review lenses
(~39KB), and every placeholder uses a function replacer.
…s, the ceiling API_TIMEOUT_MS never reached

A local daemon sends no bytes at all until prefill completes, and against
that silence Claude Code v2.1.260 has four independent ceilings. The one
that actually cancelled every Stage 3 attempt at exactly 6 minutes (the
daemon logged `500 | 6m0s | POST /v1/messages` per retry) is the Bun
runtime's own fetch timeout, which Claude Code only disables when
`API_FORCE_IDLE_TIMEOUT=0` — so PR #6117's `API_TIMEOUT_MS` raise changed
nothing. Measured against a fake stalled /v1/messages on loopback: with
the flag unset or `1` the client re-sends every 361s regardless of the
other knobs; with `0` it waits out the prefill.

Sets that flag plus the two post-first-byte idle watchdogs (30-minute
ceiling) for local-backed Claude harnesses only, and allowlists them
through both public-review env boundaries.
…nd from a permission dialog nobody can approve

Two ways agent-e057cca7 hung for hours while every reaper saw a busy
session:

- The provider kept failing the same request and Claude Code kept
  re-sending it (`API error · Retrying in 0s · attempt N/10`); the repaint
  reset the idle clock each time. `createRetryStallGate` tracks one
  request's retry ladder and fails the run over to a fallback provider once
  the banner has advanced twice across ten minutes — a verdict decided at a
  sighting, never by the clock, so a fast ladder that then succeeded is
  left alone.

- Once prefill completed, the local model asked to Read an absolute path
  outside its worktree and the hardened `acceptEdits` recipe painted a
  permission dialog for a human who was not there. `createToolPermissionGate`
  declines it (arrow-down to the last "No" option, whose number varies by
  tool) and nudges the quiet session back to work; eight declines fail the
  run. The pipeline hand-off also stops pointing a stage at
  data/cos/agents/<prev>/output.txt — outside every worktree — when that
  output is already inlined in full.
- `{reviewLenses}` is its own placeholder on the Stage 3 body instead of a
  promptKey-keyed branch inside `{reviewChecklist}`; the stage body is
  unpersisted, so only the integrity snapshot moves.
- The OOM nudge and the tool-permission nudge share one
  `createSilenceNudgeGate` (cooldown, settle, arm window, budget) instead of
  two copies of the state machine and its constants.
- The tool-permission question pattern is shared with the post-mortem
  analyzer's AWAITING_INPUT_MARKERS so live and after-the-fact detection
  cannot drift.
- The pipeline hand-off never points a stage outside its worktree once the
  producer handed over the output inline; a clipped inline says so.
- Decline keystroke uses SUBMIT_KEY; cooldown is checked before the tail is
  rebuilt; comments trimmed to the load-bearing facts.
… through the cooldown, watch only claude sessions

- The permission gate's exhaustion budget is counted in declines, so a build
  that lets the model continue after a rejection (never quiet, never nudged)
  still caps at eight instead of looping to the max-runtime ceiling.
- The screen tail keeps accumulating through the repaint cooldown, so a
  different dialog painted seconds after a decline is still seen and
  declined once the cooldown lifts.
- createSilenceNudgeGate keeps the OOM gate's original order: a sighting
  while a nudge is pending neither restarts the window nor refreshes the
  cooldown, so a box that outlasts its window can re-arm once it stops.
- Only claude sessions are watched for the dialog (it is Claude Code chrome;
  another vendor's agent would only ever echo it).
- `permission-prompt-loop` is registered in COMPLETION_REASON_ANALYSES so the
  post-mortem does not scrape the dialog chrome from the transcript.
@atomantic
atomantic merged commit 4768b88 into main Sep 4, 2026
7 checks passed
@atomantic
atomantic deleted the cos/task-mtn2g3ch/agent-793d0985 branch September 4, 2026 16:07
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