-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: new subagents system + memo anchor + ULW toggle (hide legacy delegate_task; lower token usage) #918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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: a25d004ae0
ℹ️ 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".
| async acquire(model: string): Promise<void> { | ||
| const limit = this.getConcurrencyLimit(model) | ||
| if (limit === Infinity) { | ||
| return | ||
| if (limit <= 0) { | ||
| throw new Error("Background tasks are disabled (concurrency=0)") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surface disabled background tasks instead of silent queue
When background_task.*Concurrency is set to 0 (now allowed), ConcurrencyManager.acquire() throws immediately, but BackgroundManager.launch() fires processKey() without awaiting/catching, so the rejection goes unhandled and the queued task stays in pending while the calling tool reports success. This means users who explicitly disable background tasks still get “launched successfully” responses and stuck tasks instead of a clear error. Consider handling the limit<=0 case at launch time (or catching the error in processKey) and marking the task as failed/cancelled with an explicit message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 54 files
Confidence score: 3/5
- Some risk due to a likely runtime failure:
src/agents/document-writer.tsstill instructs Task tool usage while permissions deny those tools, which can break the workflow at runtime. - Tests may become flaky because
src/features/ulw/state.test.tsandsrc/features/omo-onboarding/persistence.test.tsrestore XDG_CONFIG_HOME to the string "undefined" instead of clearing it when originally unset. - These are concrete, user-impacting or workflow-impacting issues (medium severity), so this isn’t a low‑risk merge.
- Pay close attention to
src/agents/document-writer.ts,src/features/ulw/state.test.ts,src/features/omo-onboarding/persistence.test.ts- tool permission mismatch and env var restoration logic.
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/features/ulw/state.test.ts">
<violation number="1" location="src/features/ulw/state.test.ts:17">
P2: Restoring XDG_CONFIG_HOME assigns `undefined`, leaving the env var set to the string "undefined" and polluting later tests instead of clearing it when originally unset.</violation>
</file>
<file name="src/agents/document-writer.ts">
<violation number="1" location="src/agents/document-writer.ts:20">
P2: Document Writer prompt still requires Task tool usage but the new permissions deny `task`/`delegate_task`/`call_omo_agent`, causing a tool-permission mismatch and likely runtime failures when following the workflow.</violation>
</file>
<file name="src/features/omo-onboarding/persistence.test.ts">
<violation number="1" location="src/features/omo-onboarding/persistence.test.ts:16">
P2: afterEach restores XDG_CONFIG_HOME as the string "undefined" when originally unset, polluting the global test env</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@codex review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 55 files
Confidence score: 3/5
src/agents/document-writer.tsnow denies thetasktool despite the prompt requiring it, which could block mandated exploration flows and cause user-visible failures- Two test files (
src/features/omo-onboarding/persistence.test.ts,src/features/ulw/state.test.ts) leave XDG_CONFIG_HOME as the string "undefined" when unset, risking cross-test pollution but likely limited to test stability - Given the concrete behavioral risk in the document writer and medium-severity test cleanup issues, this carries some merge risk
- Pay close attention to
src/agents/document-writer.ts,src/features/omo-onboarding/persistence.test.ts,src/features/ulw/state.test.ts- task tool denial and environment variable cleanup handling.
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/features/omo-onboarding/persistence.test.ts">
<violation number="1" location="src/features/omo-onboarding/persistence.test.ts:16">
P2: Environment variable restoration sets XDG_CONFIG_HOME to string "undefined" when originally unset, polluting later tests</violation>
</file>
<file name="src/agents/document-writer.ts">
<violation number="1" location="src/agents/document-writer.ts:20">
P1: Document Writer prompt requires the Task tool, but the agent now explicitly denies `task`, so mandated exploration calls will be rejected.</violation>
</file>
<file name="src/features/ulw/state.test.ts">
<violation number="1" location="src/features/ulw/state.test.ts:17">
P2: afterEach leaves XDG_CONFIG_HOME set to string "undefined" when originally unset instead of deleting it</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this 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: 4a3c1c8493
ℹ️ 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".
| If you want true concurrency, include bash/LSP/read calls inside the same \`batch\` call. | ||
| Otherwise, run them before/after the subagent batch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove non-task batch concurrency guidance
This template now tells the model to “include bash/LSP/read calls inside the same batch call” for true concurrency, but tool.execute.before explicitly throws when any batch entry isn’t tool: "task" (see src/index.ts around the new nonTaskTools check). If the user runs init-deep and follows this instruction, the batch call will error and abort the flow. Update the template to keep non-task tools outside batch, or relax the batch guard if mixed tools are actually supported.
Useful? React with 👍 / 👎.
65b0385 to
c9885f6
Compare
Title
feat(omo): task-first subagents (batch barrier) + memo anchor + ULW toggle (hide legacy delegate_task)
Summary (what this PR does)
This PR reworks OmO’s sub-agent behavior to align with OpenCode’s native Task semantics, while making the experience more stable, more inspectable in the TUI, and cheaper in tokens:
taskfor sub-agents so the TUI can show real, openable child sessions (the “Momus/Metis style” UX).batch(task...)as a barrier for parallel research: spawn N tasks in parallel, then wait for all results before the parent continues.call_omo_agent=false, and spawning tools are denied.delegate_taskimplementation for compatibility, but removes it from prompts and denies it by default, so the model naturally stops using it..sisyphus/memo.md, built to survive repeated compactions.ulwin chat” with a global ULW toggle (/omo ulw on|off|toggle) + cleaner injection.User-facing highlights (why you’ll feel the difference)
These are the same changes as above, phrased for day-to-day use:
.sisyphus/memo.mdto re-anchor the mission).task(session_id=...)), and that sub-agent retains its own context window.ulw: ULW becomes a proper toggle and default, not a keyword ritual.Motivation / problem statement
OmO previously relied heavily on a custom background/delegate pattern to run sub-agents “in the background”.
In complex tasks, this led to:
OpenCode already has a strong native pattern (Momus/Metis) based on
task:blocking semantics + visible sub-sessions. This PR generalizes that pattern for OmO.
In other words: this PR is not just “a different orchestration API”—it’s a stability + UX + cost fix. The parent agent now waits for complete sub-results (especially in parallel research), so it doesn’t thrash, and you can open each child session to see what it did.
Key design changes
1) Task-first orchestration (native UI + blocking semantics)
Main orchestrators (see “Agent responsibilities”) are instructed to use:
taskfor a single sub-agentbatch(tool_calls=[{tool:"task",...}, ...])for parallel researchBecause the tool is OpenCode-native, users get:
This directly addresses the “big tasks get messy” failure mode: the parent no longer advances on incomplete evidence, which also reduces rework and token waste.
2) Batch as a barrier (parallel, deterministic aggregation)
We treat
batchas a concurrency primitive for Task calls:batchis allowed only fortasktool calls (max 10 entries).Practically: you can kick off 3–10 research sub-agents in parallel, and the parent receives one aggregated payload, once, in-order.
3) Flat sub-agents (no nesting)
Child sessions are created with:
call_omo_agent=falsetask/delegate_task), to prevent sub-sub-agent spawningThis is enforced for both:
This is the core “no nuclear fission” guarantee: sub-agents can research and answer, but cannot spawn more agents.
4) Multi-turn sub-agent follow-ups
Native Task sessions are real sessions: a parent can ask follow-ups by re-invoking
task(session_id=...).This turns sub-agents from “one-shot tools” into “inspectable, iteratable mini-sessions,” which is both more powerful and more transparent for users.
Prompt rewrites (3 main agents)
This PR rewrites:
orchestrator-sisyphusPrometheus (Planner)SisyphusGoals:
Measured prompt size reductions vs upstream
origin/dev(beta.11 era), raw bytes:src/agents/orchestrator-sisyphus.ts: 57,732 → 21,683 (~62% smaller)src/agents/prometheus-prompt.ts: 40,932 → 14,145 (~65% smaller)src/agents/sisyphus.ts: 21,866 → 10,196 (~53% smaller)Expected impact:
Memo anchor (durable external memory)
When enabled, OmO uses a single durable anchor file:
.sisyphus/memo.mdDesign intent:
This is explicitly meant to handle the real-world scenario where users do long, multi-phase work: even if the chat compacts repeatedly, the memo persists the core requirements and “why we’re doing this.”
Compaction handling:
During compaction, the continuation prompt is instructed to avoid duplicating memo content.
After compaction:
.sisyphus/memo.mdULW changes (no more keyword trigger)
Previous behavior: users had to type
ulw/ultraworkin chat to trigger injection (and it was noisy).New behavior:
ULW is controlled via a global toggle (
/omo ulw on|off|toggle).Keyword trigger is disabled.
ULW injection is cleaner and aligned to the new sub-agent system.
When ULW is enabled, the assistant’s first line must be:
ULW MODE ENABLEDso users can confirm the mode is active.
This keeps ULW as an intentional “mode,” not something that accidentally triggers or requires repeated manual incantations.
Agent responsibilities (clear separation of concerns)
The three “main” agents have explicit roles:
task/batch/delegate_task)..sisyphus/*.md).task/batchto consult Metis/Momus and gather evidence before planning.task/batch) and implement code changes.This clearer separation is a major contributor to stability: it prevents “everyone can do everything,” which is where tool misuse and uncontrolled spawning tend to originate.
Background tasks / legacy path changes
This PR keeps the background-task system for compatibility, but tightens it:
Concurrency keys now prefer
input.model ?? input.parentModel(providerID/modelID) so:background_task.providerConcurrencyandbackground_task.modelConcurrencyactually apply.Limits are clamped at runtime (max 10).
0disables legacy background tasks for the matching scope.Cancellation keys match acquisition keys (avoid “can’t cancel queued task” edge cases).
The intent is: legacy remains available for advanced setups, but the default experience is firmly “task-first, visible, and controlled.”
How to use (for users)
/omotogglesType these into chat:
/omo status/omo memo on|off|toggle/omo ulw on|off|toggleHelp:
/omo-help(builtin command template)Config defaults (oh-my-opencode.json)
You can set default states globally and restrict injection to specific agents:
{ "memo": { "enabled": true, "agents": ["Sisyphus", "Prometheus (Planner)", "orchestrator-sisyphus"] }, "ulw": { "enabled": true, "agents": ["Prometheus (Planner)", "orchestrator-sisyphus"] }, "background_task": { "defaultConcurrency": 5, "providerConcurrency": { "anthropic": 3, "openai": 5, "google": 10 }, "modelConcurrency": { "anthropic/claude-opus-4-5": 2 } } }Notes:
memo.enabled=trueand.sisyphus/memo.mddoes not exist, OmO auto-creates it.background_task.*Concurrencyvalues are clamped (max 10).0disables the legacy path.Reviewer guide (where to look)
Agent prompts:
src/agents/orchestrator-sisyphus.tssrc/agents/prometheus-prompt.tssrc/agents/sisyphus.tssrc/agents/ulw-contract.tssrc/agents/memo-contract.ts/omotoggle + injection + compaction recovery:src/index.tssrc/features/omo-command/*src/features/memo-anchor/*src/features/ulw/*src/features/omo-onboarding/*Legacy background tasks tightening:
src/features/background-agent/*Tests / verification
bun run typecheckbun testbun run buildCompatibility notes
delegate_taskis preserved but denied by default and removed from main agent prompts.Potential follow-ups (optional)
batchvisibility for subagents (depending on OpenCode tool permission semantics).Summary by cubic
Shifted OmO to native Task-based sub-agents with a batch barrier, added a durable memo anchor and a global ULW toggle, and rewrote core prompts to improve stability, inspectability, and token usage.
New Features
Migration
Written for commit c9885f6. Summary will update on new commits.