Skip to content

Conversation

@Gladdonilli
Copy link
Contributor

@Gladdonilli Gladdonilli commented Jan 10, 2026

Summary

Complete overhaul of the background agent and sisyphus_task systems, building upon PRs #592, #610, #628, #638, #648, #649, #652, and #653.

Key Architectural Change: Push-Based Notification (Not Polling)

Background agents now use a push-based notification system instead of polling:

┌─────────────────────────────────────────────────────────────┐
│                    NOTIFICATION FLOW                        │
├─────────────────────────────────────────────────────────────┤
│  1. DETECTION: session.idle event fires                     │
│  2. GUARD: Wait MIN_IDLE_TIME_MS (5s) elapsed               │
│  3. COMPLETE: task.status = "completed"                     │
│  4. TRACK: pendingByParent.delete(task.id)                  │
│  5. CHECK: allComplete = pendingSet.size === 0              │
│  6. NOTIFY: session.prompt() to parent with noReply flag    │
└─────────────────────────────────────────────────────────────┘

The noReply Batching Pattern

await this.client.session.prompt({
  path: { id: task.parentSessionID },
  body: {
    noReply: !allComplete,  // Silent unless ALL complete
    agent: task.parentAgent,
    parts: [{ type: "text", text: notification }],
  },
})
Scenario noReply Effect
1 of 3 tasks done true Message injected silently, agent continues
2 of 3 tasks done true Message injected silently, agent continues
3 of 3 tasks done false Triggers agent response with summary

This prevents the main agent from being interrupted for each individual completion - it only "wakes up" when ALL background tasks are done.

Changes

Background Agent Completion Detection (PR #638)

  • Simplified completion detection logic that was causing stuck tasks
  • Tasks now complete after MIN_IDLE_TIME_MS (5s) on session.idle event
  • Added 15-minute global timeout (MAX_RUN_TIME_MS) to prevent runaway tasks

Sync Mode Category Model Fix (NEW)

  • Fixed bug where sync mode was NOT passing categoryModel to session.prompt()
  • Category model overrides now work consistently in both sync and async modes

Model Override Architecture

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)

JSONC Config File Support

  • Extended config detection to support both .json and .jsonc extensions
  • Enables comments in config files

Test Improvements

  • Increased sync resume test timeout from 5s to 10s (was flaky)

Verified Test Results (8/8 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 (Not Regressions)

Explore Agent Hangs on Non-Exploration Tasks

Explore is a specialized codebase search agent, not general Q&A. When given non-exploration tasks, it hangs. Recommendation: Add max_steps limit in future PR.

Clickable Child Sessions Not Working

Sessions pass parentID correctly but don't appear clickable in OpenCode sidebar. kdcokenny/opencode-background-agents uses identical approach. Recommendation: May need OpenCode core change.

Files Changed

  • src/features/background-agent/manager.ts
  • src/tools/sisyphus-task/tools.ts
  • src/tools/sisyphus-task/tools.test.ts
  • src/shared/config-path.ts
  • src/plugin-config.ts
  • src/plugin-handlers/config-handler.ts

Gladdonilli and others added 2 commits January 10, 2026 17:57
… tasks

Remove validateSessionHasOutput and checkSessionTodos guards from session.idle handler. These guards were blocking task completion when thinking models created todos or had different content structures. Now marks complete immediately on session.idle after 5s minimum elapsed time, matching kdcokenny/opencode-background-agents pattern. Also adds 15-minute global timeout to prevent runaway tasks.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode)

Co-authored-by: Sisyphus <[email protected]>
…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
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 6 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fa4a87d86a

ℹ️ 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".

Comment on lines +416 to +420
// SIMPLIFIED: Mark complete immediately on session.idle (after min time)
// Reference: kdcokenny/opencode-background-agents uses same pattern
// Previous guards (validateSessionHasOutput, checkSessionTodos) were causing stuck tasks
task.status = "completed"
task.completedAt = new Date()

Choose a reason for hiding this comment

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

P1 Badge Keep output/todo checks before completing on idle

This path now marks a task completed solely on session.idle after 5s, even though the file already notes that idle can be emitted before the agent fully starts. If a model/tool takes longer than 5s to produce its first assistant/tool output or leaves incomplete todos, this will prematurely complete the task and notify the parent with no real result (e.g., background_output returns empty). The polling path still guards with validateSessionHasOutput/checkSessionTodos, so skipping those here creates a regression for slow-start sessions; consider re‑using those guards before completing on idle.

Useful? React with 👍 / 👎.

Choose a reason for hiding this comment

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

claude fix it for me, make no mistake

renekris added a commit to renekris/oh-my-glm that referenced this pull request Jan 10, 2026
Copy link
Contributor

@jkoelker jkoelker left a comment

Choose a reason for hiding this comment

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

LGTM. I have been running with this patch and the series in the description all morning. I have seen no ill effects, and it subjectively feels like background agents complete more often (previous every now and then i'd get one that would "spin" and just chew tokens as the launcher checked on it).

renekris added a commit to renekris/oh-my-glm that referenced this pull request Jan 10, 2026
renekris added a commit to renekris/oh-my-glm that referenced this pull request Jan 10, 2026
@Gladdonilli
Copy link
Contributor Author

more major refactor in new pr, this is now outdated

renekris added a commit to renekris/oh-my-glm that referenced this pull request Jan 11, 2026
renekris added a commit to renekris/oh-my-glm that referenced this pull request Jan 11, 2026
Gladdonilli added a commit to Gladdonilli/oh-my-opencode that referenced this pull request Jan 11, 2026
- 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
Gladdonilli added a commit to Gladdonilli/oh-my-opencode that referenced this pull request Jan 12, 2026
- 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
Gladdonilli added a commit to Gladdonilli/oh-my-opencode that referenced this pull request Jan 12, 2026
- 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
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.

3 participants