fix: background task completion detection and silent notifications #628
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
- Fix TS2742 by adding explicit ToolDefinition type annotations - Add stability detection (3 consecutive stable polls after 10s minimum) - Remove early continue when sessionStatus is undefined - Add silent notification system via tool.execute.after hook injection - Change task retention from 200ms to 5 minutes for background_output retrieval - Fix formatTaskResult to sort messages by time descending Fixes hanging background tasks that never complete due to missing sessionStatus.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![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}
ReinaMacCredy
added a commit
to ReinaMacCredy/oh-my-opencode
that referenced
this pull request
Jan 9, 2026
Apply upstream PR code-yeongyu#628 by @Gladdonilli - Fix background tasks that hang indefinitely due to missing sessionStatus - Add silent notification system via tool.execute.after hook injection - Add stability detection (3 consecutive stable polls after 10s minimum) - Change task retention from 200ms to 5 minutes for background_output - Fix formatTaskResult to sort messages by time descending - Fix TS2742 by adding explicit ToolDefinition type annotations Co-authored-by: Gladdonilli <tianyi35@illinois.edu>
Contributor
Author
|
new alternative method found + more issues identified, moving to a new pr |
Gladdonilli
added a commit
to Gladdonilli/oh-my-opencode
that referenced
this pull request
Jan 9, 2026
- Add stability-based completion detection (10s min + 3 stable polls) - Fix message extraction to recognize 'reasoning' parts from thinking models - Switch from promptAsync() to prompt() for proper agent initialization - Remove model parameter from prompt body (use agent's configured model) - Add fire-and-forget prompt pattern for sisyphus_task sync mode - Add silent notification via tool.execute.after hook injection - Fix indentation issues in manager.ts and index.ts Incorporates fixes from: - PR code-yeongyu#592: Stability detection mechanism - PR code-yeongyu#610: Model parameter passing (partially) - PR code-yeongyu#628: Completion detection improvements Known limitation: Thinking models (e.g. claude-*-thinking-*) cause JSON Parse errors in child sessions. Use non-thinking models for background agents until OpenCode core resolves this.
5 tasks
code-yeongyu
pushed a commit
that referenced
this pull request
Jan 10, 2026
…ion (#638) * fix: background task completion detection and silent notifications - Fix TS2742 by adding explicit ToolDefinition type annotations - Add stability detection (3 consecutive stable polls after 10s minimum) - Remove early continue when sessionStatus is undefined - Add silent notification system via tool.execute.after hook injection - Change task retention from 200ms to 5 minutes for background_output retrieval - Fix formatTaskResult to sort messages by time descending Fixes hanging background tasks that never complete due to missing sessionStatus. * fix: improve background task completion detection and message extraction - Add stability-based completion detection (10s min + 3 stable polls) - Fix message extraction to recognize 'reasoning' parts from thinking models - Switch from promptAsync() to prompt() for proper agent initialization - Remove model parameter from prompt body (use agent's configured model) - Add fire-and-forget prompt pattern for sisyphus_task sync mode - Add silent notification via tool.execute.after hook injection - Fix indentation issues in manager.ts and index.ts Incorporates fixes from: - PR #592: Stability detection mechanism - PR #610: Model parameter passing (partially) - PR #628: Completion detection improvements Known limitation: Thinking models (e.g. claude-*-thinking-*) cause JSON Parse errors in child sessions. Use non-thinking models for background agents until OpenCode core resolves this. * fix: add tool_result handling and pendingByParent tracking for resume/external tasks Addresses code review feedback from PR #638: P1: Add tool_result type to validateSessionHasOutput() to prevent false negatives for tool-only background tasks that would otherwise timeout after 30 minutes despite having valid results. P2: Add pendingByParent tracking to resume() and registerExternalTask() to prevent premature 'ALL COMPLETE' notifications when mixing launched and resumed tasks. * fix: address code review feedback - log messages, model passthrough, sorting, race condition - Fix misleading log messages: 'promptAsync' -> 'prompt (fire-and-forget)' - Restore model passthrough in launch() for Sisyphus category configs - Fix call-omo-agent sorting: use time.created number instead of String(time) - Fix race condition: check promptError inside polling loop, not just after 100ms
ReinaMacCredy
added a commit
to ReinaMacCredy/oh-my-opencode
that referenced
this pull request
Jan 10, 2026
Apply upstream PR code-yeongyu#628 by @Gladdonilli - Fix background tasks that hang indefinitely due to missing sessionStatus - Add silent notification system via tool.execute.after hook injection - Add stability detection (3 consecutive stable polls after 10s minimum) - Change task retention from 200ms to 5 minutes for background_output - Fix formatTaskResult to sort messages by time descending - Fix TS2742 by adding explicit ToolDefinition type annotations Co-authored-by: Gladdonilli <tianyi35@illinois.edu>
Gladdonilli
added a commit
to Gladdonilli/oh-my-opencode
that referenced
this pull request
Jan 10, 2026
…ing and sync mode ## Summary This commit represents the culmination of extensive debugging and refinement of the background agent and sisyphus_task systems, building upon changes from PRs code-yeongyu#592, code-yeongyu#610, code-yeongyu#628, code-yeongyu#638, code-yeongyu#648, code-yeongyu#649, code-yeongyu#652, and code-yeongyu#653. ## Investigation Journey ### Initial Problem Background tasks were getting stuck indefinitely. User config model overrides were being ignored for certain agent types. ### Root Cause Analysis 1. Discovered validateSessionHasOutput and checkSessionTodos guards were blocking completion even when tasks had finished 2. Found that sync mode (run_in_background=false) was NOT passing categoryModel to session.prompt(), while async mode was 3. Traced config loading path and found JSONC files weren't being detected 4. Analyzed kdcokenny/opencode-background-agents for reference implementation ### Trial and Error Log **Attempt 1: Add model to resume() in manager.ts** - Hypothesis: Resume needs to pass stored model - Result: REVERTED - PR code-yeongyu#638 intentionally removed this; agent config handles it **Attempt 2: Add userAgents lookup for subagent_type** - Hypothesis: Need to look up agent model from user config - Result: REVERTED - Agent model already applied at creation in config-handler.ts **Attempt 3: Add categoryModel to sync mode prompt** - Hypothesis: Sync mode missing model that async mode passes - Result: SUCCESS - This was the actual bug **Attempt 4: Add debug logging throughout pipeline** - Purpose: Trace model flow through config -> agent -> prompt - Files: 6 files with appendFileSync to omo-debug.log - Result: Confirmed fixes working, then REMOVED all debug logging **Attempt 5: Investigate clickable sessions** - Hypothesis: parentID should make child sessions clickable in UI - Result: parentID IS passed correctly, but sessions don't appear clickable - Analysis: kdcokenny uses same approach; may be OpenCode core limitation - Status: UNRESOLVED - Needs further investigation or OpenCode core change ## Background Agent Completion Detection (PR code-yeongyu#638) Simplified the completion detection logic that was causing tasks to get stuck: - Removed overly complex validateSessionHasOutput and checkSessionTodos guards - Tasks now complete after MIN_IDLE_TIME_MS (5s) elapsed on session.idle event - Added 15-minute global timeout (MAX_RUN_TIME_MS) to prevent runaway tasks - Pattern follows kdcokenny/opencode-background-agents reference implementation ## Model Override Architecture (PRs code-yeongyu#610, code-yeongyu#628, code-yeongyu#638) Established clear separation between category-based and agent-based model handling: | Path | Model Source | |-------------------|-------------------------------------------| | category=X | Explicit from category config (passed) | | subagent_type=X | Agent's configured model (at creation) | | resume | Agent's configured model (not passed) | Key insight from PR code-yeongyu#638: Don't pass model in prompt body for resume/subagent - let OpenCode use the agent's configured model set at creation time in config-handler.ts. ## Sync Mode Category Model Fix (NEW) Fixed critical bug where sync mode (run_in_background=false) with categories was NOT passing the categoryModel to session.prompt(): // BEFORE: Model not passed in sync mode body: { agent: agentToUse, system: systemContent, ... } // AFTER: Model passed when available body: { agent: agentToUse, ...(categoryModel ? { model: categoryModel } : {}), ... } This ensures category model overrides work consistently in both sync and async modes. ## JSONC Config File Support Extended config file detection to support both .json and .jsonc extensions: - getUserConfigDir() now checks for oh-my-opencode.jsonc first - Both cross-platform (~/.config) and Windows (%APPDATA%) paths support JSONC - Enables comments in config files for better documentation ## Test Improvements - Increased sync resume test timeout from 5s to 10s - Test was flaky because timeout = MIN_STABILITY_TIME_MS (race condition) - Added clarifying comments about timing requirements ## Code Cleanup - Removed unused 'os' imports from plugin-config.ts and config-handler.ts - Removed ALL debug logging (hardcoded paths, appendFileSync calls) - Added PR code-yeongyu#638 reference comments for future maintainers ## Verified Test Results (8/8 category + subagent tests pass) | Test | Type | Mode | Result | |-------------------|-------------|-------|--------| | quick | category | async | ✅ | | ultrabrain | category | async | ✅ | | most-capable | category | async | ✅ | | quick | category | sync | ✅ | | librarian | subagent | async | ✅ | | Metis | subagent | async | ✅ | | oracle | subagent | sync | ✅ | | quick + git-master| category | async | ✅ | ## Known Issues & Future Work ### 1. Explore Agent Hangs on Non-Exploration Tasks The explore agent hung when given a simple math query (5+5). This is NOT a regression - explore is a specialized codebase search agent (contextual grep) designed for queries like 'Where is X implemented?' not general Q&A. When given non-exploration tasks, it attempts to search for non-existent patterns and may hang indefinitely due to no max_steps limit. **Recommendation**: Add max_steps: 10 to explore agent config in future PR. ### 2. Clickable Child Sessions Not Working Sessions created via sisyphus_task pass parentID correctly, but don't appear as clickable child sessions in OpenCode's sidebar UI. Investigation showed: - parentID IS being passed to session.create() - kdcokenny/opencode-background-agents uses identical approach - Sessions exist and complete, just not rendered as clickable in UI **Recommendation**: May require OpenCode core change or UI setting discovery. ### 3. Prometheus Agent Correctly Blocked Prometheus (Planner) is a primary agent and correctly rejected when called via sisyphus_task with subagent_type. This is expected behavior - primary agents should only be invoked directly, not via task delegation. ## Files Changed - src/features/background-agent/manager.ts - PR code-yeongyu#638 reference comment - src/tools/sisyphus-task/tools.ts - Sync mode categoryModel fix - src/tools/sisyphus-task/tools.test.ts - Test timeout increase - src/shared/config-path.ts - JSONC extension support - src/plugin-config.ts - Cleanup unused import - src/plugin-handlers/config-handler.ts - Cleanup unused import
renekris
pushed a commit
to renekris/oh-my-glm
that referenced
this pull request
Jan 10, 2026
- Add stability-based completion detection (10s min + 3 stable polls) - Fix message extraction to recognize 'reasoning' parts from thinking models - Switch from promptAsync() to prompt() for proper agent initialization - Remove model parameter from prompt body (use agent's configured model) - Add fire-and-forget prompt pattern for sisyphus_task sync mode - Add silent notification via tool.execute.after hook injection - Fix indentation issues in manager.ts and index.ts Incorporates fixes from: - PR code-yeongyu#592: Stability detection mechanism - PR code-yeongyu#610: Model parameter passing (partially) - PR code-yeongyu#628: Completion detection improvements Known limitation: Thinking models (e.g. claude-*-thinking-*) cause JSON Parse errors in child sessions. Use non-thinking models for background agents until OpenCode core resolves this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
sessionStatusin polling loopToolDefinitiontype annotationsChanges
Completion Detection (PR #592 re-implementation)
continuewhensessionStatusis undefined - allows fall-through to message-based detectionlastMsgCountandstablePollsfields toBackgroundTaskinterfaceMIN_STABILITY_TIME_MSconstant (10 seconds)Silent Notification System (new feature)
PendingNotificationinterface andpendingNotificationsmap toBackgroundManagerhasPendingNotifications()andconsumePendingNotifications()methodssession.prompt()with storing notifications for later injectiontool.execute.afterhook handler that injects pending notifications into tool outputsrc/index.tsOther Fixes
background_outputretrievalformatTaskResultto sort messages by time descending (matches sync pattern)String()wrapper forlocaleCompareto handle non-string time valuesToolDefinitiontype annotations toskillandslashcommandexportsTesting
bun run typecheckpassesbun run buildsucceedsFiles Modified
src/features/background-agent/types.tslastMsgCount,stablePollsfieldssrc/features/background-agent/manager.tssrc/features/background-agent/index.tsPendingNotificationtypesrc/hooks/background-notification/index.tstool.execute.afterhandlersrc/index.tssrc/tools/background-task/tools.tsformatTaskResultsortingsrc/tools/skill/tools.ts: ToolDefinitiontype annotationsrc/tools/slashcommand/tools.ts: ToolDefinitiontype annotationSummary by cubic
Fixes background tasks that could hang and adds silent, inline notifications injected into tool outputs. Improves completion detection, keeps results available longer, and resolves TypeScript typing errors.
Bug Fixes
New Features
Written for commit 9e81260. Summary will update on new commits.