Two blockers from the maintainer's Oracle review on PR #4121:
Blocker 1 — load_skills=null should still throw, omitted should default
The previous PR collapsed both `loadSkills === undefined` and
`loadSkills === null` into a silent default of `[]`. The closing
rationale of PR #1663 (which reverted PR #1493) and the maintainer's
review both call out the importance of preserving the distinct
"omitted -> default, explicit invalid -> throw" contract. `null`
strongly signals "I tried to pass something and it was wrong";
silently coercing it hides bugs upstream.
Restored the split: `undefined` -> default `[]` + log,
`null` -> throw with the historical error string.
Blocker 2 — task_id continuation test rewritten, not deleted
The original PR removed the `task_id without run_in_background ->
throws` test entirely. The behavior IS preserved (default false ->
`isExplicitSyncRun` true -> `executeSyncContinuation`), but with the
test gone the new contract was unprotected.
Added a regression test that asserts the new contract: when
`task_id` is present and `run_in_background` is omitted,
`tool.execute` must route through sync continuation without throwing
the legacy required-parameter error. Mocks include `session.abort`
because the sync poller calls it during shutdown.
Also flipped the existing `load_skills=null` regression test from
"normalizes to []" back to "throws with the legacy error string" to
match the restored contract.
Tests:
- bun test src/tools/delegate-task/tools.test.ts -> 132/132 pass
- bun test src/tools/delegate-task/ -> 406/406 pass
- bun run typecheck -> clean
Sisyphus and other delegators occasionally invoke the task() tool without
an explicit run_in_background or load_skills argument. The runtime
validators in tool-argument-preparation.ts threw a hard Error in that
case, which short-circuited tool.execute() entirely. Because OpenCode's
tool.execute.after hook only runs on returned results, the
delegate-task-retry hook never had a chance to attach corrective
guidance — so the model saw a raw failure and either burned several
retries or fell back to a synchronous Explore call, silently losing
parallel execution.
Behavior change:
- run_in_background omitted -> defaults to false (sync delegation), with
a log entry for observability.
- load_skills omitted or null -> normalized to [] with a log entry on
the explicit-null path.
- The Zod schema entries are now .optional() and their .describe()
strings declare the defaults honestly; the markdown tool description
was updated to match (no more 'REQUIRED' lie).
The orthogonal validation 'Must provide either category or
subagent_type.' is unchanged and still surfaces as a returned error.
Tests:
- The five throw-on-missing tests in tools.test.ts are rewritten to
assert the new default-and-proceed contract.
- The 'no category, no subagent_type' test now asserts the
missing-target error remains intact.
Refs the workaround the reporter validated in the original issue body;
matches the design from PR #2375 which was previously reverted by
566031f4.
After the 4.2.0 unified-dispatch refactor (a42f894f / df198d8b / fee515c5 / 989ab717 / dd3fecaf / 1bbe065c / 12bd6580), at least one caller in the new prompt-async-gate path forwards a FallbackModelObject (or some other non-string shape) into parsers that statically claim 'model: string'. The downstream .trim() call then throws 'model.trim is not a function', which rejects the session.processor promise and surfaces as 'Aborted process' + UI 'interrupted'. The issue (#4145) reports this aborts 90% of subagent dispatches across every provider on 4.2.0 + opencode 1.15.4.
This patch adds a 'typeof x !== "string"' runtime guard at the four parser entrypoints called from the dispatch path:
- src/shared/fallback-chain-from-models.ts :: parseVariantFromModel, parseFallbackModelEntry
- src/tools/delegate-task/model-string-parser.ts :: parseVariantFromModelID, parseModelString
- src/shared/model-string-parser.ts (duplicate file with same API) :: parseVariantFromModelID, parseModelString
- src/features/claude-code-agent-loader/claude-model-mapper.ts :: mapClaudeModelString
Each parser now returns undefined / { modelID: "" } for non-string input instead of throwing. This unblocks subagent dispatch and leaves the underlying caller bug for a follow-up.
Regression coverage: three new tests in src/shared/fallback-chain-from-models.test.ts pin the non-string behavior (object, null/undefined, number). Existing 38 tests still pass. Total: 41/41 green, typecheck clean.
Addresses cubic-dev-ai P1 + P2 findings on #4115.
The original PR relaxed `exitCode > 1` to `> 2` based on the (wrong)
claim that ripgrep exits 2 only on non-fatal I/O issues. ripgrep
actually uses exit code 2 for BOTH fatal errors (pattern syntax,
invalid args) AND non-fatal I/O issues; GNU grep (the fallback backend
in grep/cli.ts) likewise uses 2 for fatal errors. So `> 2` would
silently suppress fatal errors.
The correct fix is just `--no-messages`, which suppresses ripgrep's
stderr only for soft I/O issues (broken symlinks, permission denied)
while leaving fatal-error messages intact. With the gate kept at
`exitCode > 1 && stderr.trim()`:
- Broken symlink: ripgrep exits 2, stderr is empty (suppressed) →
`stderr.trim()` is falsy → gate fails → partial results survive.
- Fatal error: ripgrep exits 2, stderr has the real error message
(not suppressed by --no-messages) → gate triggers → error returned.
Reverting both `exitCode > 1` → `> 2` changes; keeping the
`--no-messages` flag additions and the regression test (test comment
updated to describe the cleaner architecture).
Verification: bun test src/tools/glob/ src/tools/grep/ → 30 pass / 0 fail.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes#3726.
## Root cause
Two coupled bugs in the ripgrep integration caused the glob and grep
tools to fail outright when the search path contained a broken
(dangling) symlink:
1. **Missing `--no-messages`** in `RG_FILES_FLAGS` (glob) and
`RG_SAFETY_FLAGS` (grep). ripgrep prints a stderr warning for every
broken symlink:
`rg: /path/to/broken-link: No such file or directory (os error 2)`
2. **Overly strict exit-code gate** in both `glob/cli.ts` and
`grep/cli.ts`: `if (exitCode > 1 && stderr.trim())` treated exit
code 2 as fatal and discarded the stdout, even though ripgrep exits
2 on *non-fatal* I/O issues (broken symlinks, permission denied)
while still printing valid results to stdout.
Combined effect: a single broken symlink anywhere in the search path
turned all subsequent file matches into an empty result with an error.
## Fix
- Append `--no-messages` to `RG_FILES_FLAGS` (`src/tools/glob/constants.ts`)
and to `RG_SAFETY_FLAGS` (`src/tools/grep/constants.ts`). This silences
ripgrep's non-fatal stderr warnings without changing match behavior.
- Relax the exit-code gate in both `glob/cli.ts` and `grep/cli.ts` from
`> 1` to `> 2`, matching ripgrep's documented contract:
- 0 = matches found
- 1 = no matches (success, just nothing matched)
- 2 = non-fatal I/O issues (partial success — stdout is still valid)
- >2 = fatal error
## Test changes
- New regression assertion in `src/tools/glob/cli.test.ts` for
`buildRgArgs` confirms `--no-messages` is included in the args.
- The grep change is symmetric (same flag, same rationale, same
exit-code constant) and rides on the parallel structure.
## Verification
- `bun test src/tools/glob/` → all pass (19 tests in cli.test.ts)
- `bun test src/tools/grep/` → all pre-existing tests still pass
- `bunx tsc --noEmit` → clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root cause: `formatTaskResult` and `formatFullSession` both call
`client.session.messages({ path: { id: task.sessionId } })` with no
timeout. The OpenCode SDK explicitly disables fetch timeout
(`req.timeout = false` in `packages/sdk/js/src/client.ts:37`), so when
`session.processor` enters an "Aborted process" loop (same condition
patched in #4086 for the Read hot path) every subsequent
`session.messages` call hangs forever.
User-visible impact: the parent agent running a heavy slash command
such as `/init-deep` fires many background `task` agents in parallel
and then calls `background_output(task_id, block=true)` for each. The
existing `timeoutMs` (max 10min) caps only the polling loop that waits
for the child task to transition out of `running`. Once the child is
`completed`, the loop exits and the code falls through to
`formatTaskResult` / `formatFullSession`. The 10min cap does not apply
to that post-completion fetch, so a wedged `session.messages` leaves
the tool call hanging indefinitely. The parent session appears stuck
to the user (the symptom they report on `/init-deep ultrafucking
deep`).
Fix: race the underlying `session.messages` call against a 5s timeout
through a new `withSdkCallTimeout` helper local to
`src/tools/background-task/` (mirrors `withFetchTimeout` in
`src/shared/dynamic-truncator.ts` and `withDispatchTimeout` in
`src/shared/prompt-async-gate.ts`). On timeout the caller returns a
clearly-marked `"Error fetching messages: ... timed out after 5000ms"`
fallback so the agent can recognise the failure and continue instead
of waiting forever.
Tests:
- New `sdk-call-timeout.test.ts` pins the fix with three BDD cases
using a never-settling mock and the new
`_setBackgroundOutputFetchTimeoutMsForTesting(50)` override:
(1) `formatTaskResult` resolves to the timeout fallback under the
fetch budget, (2) same for `formatFullSession`, (3) two parallel
callers against the wedged client both resolve cleanly.
- All three failed (timed out) before the fix and pass in 51ms each
after.
Verification:
- `bun test src/tools/background-task/` -- 33 pass.
- `bun test` (full suite) -- 7014 pass, 1 skip, 0 fail across 723
files.
- `bun run typecheck` -- clean (tsgo --noEmit).
- LSP diagnostics on the four changed files -- 0 errors.
- Manual QA harness `.local-ignore/init-deep-hang-qa.ts` (gitignored)
drove production default 5s timeout against a wedged client:
serial `formatTaskResult` resolved in 5001ms, serial
`formatFullSession` in 5002ms, 10 parallel `formatTaskResult` calls
all resolved within 5002ms with the timeout fallback. Without the
fix every call hangs indefinitely.
Refs: builds on #4086 (dynamic-truncator) which patched the Read hot
path of the same SDK hang.
Detect OpenCode promptAsync calls that return before a child session has any durable message, and surface a prompt acceptance error before the generic five-minute sync poll timeout.
Add a failing-first regression for the idle zero-message case and keep the existing durable-message completion path covered.
Debugging-Journal: .debugging
External review on PR #4074 noted that adding setSessionAgent for child
sessions left sessionAgentMap holding entries after the session was
deleted or after the sync call_omo_agent executor cleaned up other
owned state. The map only grows; entries never get reused but they do
accumulate across long-running plugin instances.
Close both gaps:
- BackgroundManager.handleEvent for session.deleted now calls
clearSessionAgent for the deleted session id on both the early-return
no-task branch and the cascade tail. This pairs with the existing
clearDelegatedChildSessionBootstrap and SessionCategoryRegistry.remove
so all owned session state is dropped together.
- sync-executor finally for createdSessionForExecution now calls
clearSessionAgent alongside the existing subagentSessions,
syncSubagentSessions, and deleteSessionTools cleanup so sessions this
executor created cannot leak their agent mapping.
Adds focused tests:
- BackgroundManager.handleEvent - session.deleted cascade > should
clear session agent state for deleted sessions to prevent map leak
- executeSync > registers child-session bootstrap and tracked prompt
state before sync prompt dispatch (extended assertion for cleanup)
Two call sites built the sync delegate tool gate independently:
sync-prompt-sender's prompt body construction and sync-task's bootstrap
registration. Drift between them would let bootstrap claim one tool set
while the actual prompt sent a different one. Extract buildSyncPromptTools
and route both call sites through it so the registered bootstrap and the
dispatched prompt always agree.
call_omo_agent sync path created the child OpenCode session and went
straight into promptAsync without registering child session agent,
session tools, or bootstrap state. If first dispatch failed before any
durable user message persisted, runtime fallback could not reconstruct
the original prompt or the agent identity for that child session.
Bind setSessionAgent and setSessionTools to the child session id with
the same tool restrictions that the prompt body sends, register a
delegated child session bootstrap with the prompt text, fallback chain,
and prompt tools, then clean bootstrap + session tools in finally for
sessions this call created.
Preserve delegated child prompt/bootstrap metadata for early runtime fallback before OpenCode has persisted the first user turn. Bind prompt gate calls to the SDK session receiver and keep completed background task lookup visible across plugin manager instances.
YeonGyu-Kim