fix: prevent infinite background agent resubscribe loop and add output guard#130
fix: prevent infinite background agent resubscribe loop and add output guard#130harrryyd wants to merge 9 commits into
Conversation
…umber, render PR diff with @pierre/diffs - Replace thread runtimeMode-based review mode with a local 'Review' toggle button in the diff panel header - Auto-detect PR number from VCS status (same source as sidebar badge), fallback to overrideable input - Render PR diff using Virtualizer+FileDiff from @pierre/diffs instead of raw <pre> text block - Auto-switch diffSourceMode between 'checkpoint' and 'pr' when toggling review mode
…fix header click behavior
…anel-local toggle (#116)
…, and removed Review button
…handle legacy 'review' rows (#116)
…owSchema in snapshot query (#116)
…lose stdin - Fix 'Agent process failed or timed out: undefined' by checking cause.message is truthy before use - Add -m opencode/deepseek-v4-flash-free to opencode CLI args for fast background agent responses - Close stdin with stdio: ['ignore', 'pipe', 'pipe'] to prevent agent process blocking - Reduce AGENT_TIMEOUT_MS from 5min to 2min for faster failure feedback - Add isDiffOpenValue handling for TanStack Router JSON-encoded search params - Add E2E test (pr-review-agent-e2e.ts) exercising full background agent flow via WS RPC
…t guard Root cause: wsTransport.subscribe() has an infinite for(;;) loop that auto-resubscribes after stream completion. runBackgroundAgent was wired through subscribe() (meant for persistent streams), causing the agent to be re-invoked infinitely — each completion spawned a new opencode process that made new changes, pushed, triggered another run. Fix: - Client (useReviewComments): call unsub() in done/error listeners to break the subscribe() loop after one completion - Server (BackgroundAgentService): add MAX_AGENT_OUTPUT_BYTES (500KB) output guard that kills runaway agent processes - Server (BackgroundAgentService): improve prompt to instruct agent to STOP on vague/non-actionable comments rather than forcing 'fix' mode The subscribe() re-trigger was verified via wsTransport.test.ts at line 479 which explicitly tests that a stream re-subscribes after normal exit.
github-actions
Bot
added
vouch:trusted
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7deddaea61
ℹ️ 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".
| const apiPath = `repos/${nameWithOwner}/pulls/${input.reference}/reviews`; | ||
| yield* execute({ | ||
| cwd: input.cwd, | ||
| args: ["api", apiPath, "--input", input.commentsJsonFile], |
There was a problem hiding this comment.
When a draft contains any line comment, this runs gh api repos/.../reviews --input ... without --method POST. The GitHub CLI manual says gh api defaults to GET unless field parameters are added, while --input only supplies the request body (https://cli.github.com/manual/gh_api), so these inline-review submissions hit the create-review endpoint with GET and fail instead of posting the review. Add --method POST (or -X POST) here.
Useful? React with 👍 / 👎.
| : cli.createPullRequestReview({ | ||
| cwd: input.cwd, | ||
| reference: String(input.prNumber), | ||
| bodyFile: commentsJsonFile, |
There was a problem hiding this comment.
When the draft has only file-level comments (apiComments.length === 0), this passes the JSON payload file to gh pr review --body-file; the CLI manual says --body-file reads body text from the file (https://cli.github.com/manual/gh_pr_review), so GitHub will receive the raw {"event":"COMMENT",...} JSON as the review body instead of the formatted markdown in body. Use a markdown body file on this fallback path, or submit the JSON payload via gh api as well.
Useful? React with 👍 / 👎.
| Effect.runFork(Ref.update(combinedOutputRef, (prev) => prev + text)); | ||
| safeOffer({ | ||
| type: "detail", | ||
| commentId: input.commentId, | ||
| title: "Agent stderr", | ||
| content: text, | ||
| }); | ||
| if (!killedByOutputGuard) { |
There was a problem hiding this comment.
The new 500 KB guard only runs in the stdout handler; if an agent gets into a loop that logs to stderr (common for CLI warnings/errors), this handler keeps appending and streaming those chunks without ever calling killForOutputExceeded(). That bypasses the intended runaway-process protection and can still grow memory/WS output until the timeout, so stderr should share the same guarded append path as stdout.
Useful? React with 👍 / 👎.
1 participant
Root cause
wsTransport.subscribe()has an infinitefor (;;)loop that auto-resubscribes after stream completion.runBackgroundAgentwas wired throughsubscribe()(meant for persistent streams), causing the agent to be re-invoked infinitely — each completion spawned a new opencode process that made new changes, pushed, triggering another run.The
subscribe()re-trigger was verified viawsTransport.test.ts:479which explicitly tests that a stream re-subscribes after normal exit.Fixes
Client (
useReviewComments.ts)unsub()indone/errorlisteners to break thesubscribe()loop after one completionSet, callunsub()when all doneServer (
BackgroundAgentService.ts)MAX_AGENT_OUTPUT_BYTES(500KB): output guard that kills runaway agent processes with SIGTERMTests (
BackgroundAgentService.test.ts)kills agent process when output exceeds MAX_AGENT_OUTPUT_BYTES— simulates flooding stdout, verifies child killedprompt instructs agent to check if comment is actionable before making changes— verifies guard clauses in promptTest results
DiffCommentPanel.test.tsxunrelated to this change)