fix: Improve background agent stability and completion detection #715
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add max_steps limit to explore (25) and librarian (30) agents - Block sisyphus_task/call_omo_agent tools in librarian to prevent spawning - Add global 15-minute timeout for background tasks (MAX_RUN_TIME_MS) - Simplify session.idle handler - remove validateSessionHasOutput/checkSessionTodos guards - Add JSONC config file support (.jsonc checked before .json) - Fix categoryModel passing in sisyphus_task sync mode Reference: PR code-yeongyu#655, kdcokenny/opencode-background-agents
- Reset startedAt when resuming tasks to prevent immediate completion (MIN_IDLE_TIME_MS check was passing immediately for resumed tasks) - Previous commit already fixed timeout.unref() and double-release prevention
- Set concurrencyKey = undefined after every release() call to prevent double-release when multiple code paths try to release the same key - Add 15-minute timeout timer for resumed tasks (was missing) - Fixes: promptAsync error, session.deleted, pruneStaleTasksAndNotifications
Address P2 feedback: resume() error handler now properly: - Clears the timeout timer created for the resumed task - Releases concurrency key to unblock queued tasks
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 6 files
Confidence score: 3/5
- Late prompt rejections in
src/tools/sisyphus-task/tools.tsare dropped after the 100 ms fire-and-forget check, so users can see a misleading “No assistant response found” after 10 minutes of polling when the prompt never succeeded. - Early returns on prompt errors leave
subagentSessionsentries uncleared insrc/tools/sisyphus-task/tools.ts, risking stale session tracking and incorrect future runs. - Pay close attention to
src/tools/sisyphus-task/tools.ts– prompt error handling needs to avoid losing failures and leaking session entries.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/tools/sisyphus-task/tools.ts">
<violation number="1" location="src/tools/sisyphus-task/tools.ts:426">
P2: Late prompt rejections are dropped: fire-and-forget prompt is only checked for 100 ms, so failures after that window lead to 10‑minute polling and a misleading "No assistant response found" when the prompt was never sent.</violation>
<violation number="2" location="src/tools/sisyphus-task/tools.ts:445">
P2: Prompt error early return leaves subagentSessions entry uncleared, causing stale session tracking</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // For category-based tasks, pass the model from category config | ||
| // For agent-based tasks, use agent's configured model (don't pass model in body) | ||
| let promptError: Error | undefined | ||
| client.session.prompt({ |
There was a problem hiding this comment.
P2: Late prompt rejections are dropped: fire-and-forget prompt is only checked for 100 ms, so failures after that window lead to 10‑minute polling and a misleading "No assistant response found" when the prompt was never sent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-task/tools.ts, line 426:
<comment>Late prompt rejections are dropped: fire-and-forget prompt is only checked for 100 ms, so failures after that window lead to 10‑minute polling and a misleading "No assistant response found" when the prompt was never sent.</comment>
<file context>
@@ -419,21 +419,30 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id
+ // For category-based tasks, pass the model from category config
+ // For agent-based tasks, use agent's configured model (don't pass model in body)
+ let promptError: Error | undefined
+ client.session.prompt({
+ path: { id: sessionID },
+ body: {
</file context>
Comment on lines
+445
to
448
| if (promptError) { | ||
| if (toastManager && taskId !== undefined) { | ||
| toastManager.removeTask(taskId) | ||
| } |
There was a problem hiding this comment.
P2: Prompt error early return leaves subagentSessions entry uncleared, causing stale session tracking
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/sisyphus-task/tools.ts, line 445:
<comment>Prompt error early return leaves subagentSessions entry uncleared, causing stale session tracking</comment>
<file context>
@@ -419,21 +419,30 @@ System notifies on completion. Use \`background_output\` with task_id="${task.id
+ // Small delay to let the prompt start
+ await new Promise(resolve => setTimeout(resolve, 100))
+
+ if (promptError) {
if (toastManager && taskId !== undefined) {
toastManager.removeTask(taskId)
</file context>
Contributor
Author
|
adding dynamic approach for "noreply", updating tormorrow in new pr |
Collaborator
if you're making a new pr please close this one |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Complete overhaul of background agent stability, combining improvements from closed PRs #697, #655, and #714.
Changes
Agent Safety Guards (from #697)
max_steps: 25to prevent infinite loopsmax_steps: 30+ blockstask,sisyphus_task,call_omo_agenttoolsBackground Completion Detection (from #655, #697)
prompt()instead of awaiting (fixes JSON parse errors with thinking models)MIN_IDLE_TIME_MS(5s): Minimum time before considering session idleMAX_RUN_TIME_MS(15min): Global timeout to prevent stuck tasksvalidateSessionHasOutputandcheckSessionTodosguards that were causing stuck taskstimeoutTimeron task completionMemory Leak Prevention (from #714)
session.idlecompletion (not just onsession.deleted)concurrencyKey = undefinedafter releaseOther Fixes
categoryModelfor category-based tasks in sync mode.jsoncbefore.jsonstartedAton resume: Proper timing for resumed tasksTesting
Related PRs
References
Summary by cubic
Improves background agent stability and reliable completion by adding safety limits, simplifying idle handling, and fixing cleanup. Tasks now complete predictably, avoid leaks, and subagents can’t spiral.
Written for commit d59f890. Summary will update on new commits.