Reliable OpenCode subagents by default, --resume session continuation, JSONC configs, and session-handoff race fixes - #18
Conversation
The CLI treats --session-id as 'create a NEW session with this UUID' and exits with 'Session ID ... is already in use' whenever a transcript for that ID already exists on disk — so every respawn that tried to continue a session (MCP hot reload, eviction, crash recovery) failed, cleared the session, and fell back to re-injecting history as text. Verified against the real CLI: --session-id reuse fails with no live process holding the ID; --resume continues under the same session ID. Also catch the lowercase 'No conversation found with session ID' error that --resume prints for a purged transcript, so a stale remembered ID still self-heals on the next turn.
The proxy server's close() rejects any in-flight tools/call so the HTTP handler doesn't hang, but that rejection was logged at WARN — surfacing a yellow TUI bubble on every normal teardown (process exit, abort kill, MCP hot-reload respawn, compaction). By the time close() runs, the owning Claude process is gone or being replaced, so nobody can consume the response; the rejection is pure cleanup. Extract the expected-cleanup classification into an exported isExpectedCleanupError(), add the server-closed message to it, and share the message string via SERVER_CLOSED_MESSAGE so the classifier and close() cannot drift apart. Genuine errors still log at WARN.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for proxying the 'Task' tool natively through OpenCode, allowing Claude to orchestrate subagents with proper permission, lifecycle, and background execution support. Key changes include updating the default proxy tools list, replacing the custom JSON comment stripper with jsonc-parser to robustly handle JSONC configurations, switching from --session-id to --resume for continuing sessions, and implementing a deferred result boundary mechanism to wait for parallel or delayed proxy calls at turn-result boundaries. Additionally, process management is improved with graceful shutdown waiting (deleteActiveProcessAndWait) and comprehensive test suites are added in test-proxy-task.ts and test-session-manager.ts. As there are no review comments provided, I have no feedback to provide on the review itself.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Follow-up fix in |
|
Transcript-backed parallel Task fix in |
|
Static model-catalog update in |
khalilgharbaoui
left a comment
khalilgharbaoui
left a comment
There was a problem hiding this comment.
This is an outstanding contribution — live-verified fixes, real root-cause analysis (the --session-id vs --resume finding especially), and thorough tests. Thank you. Direction is accepted on all five changes; here's the plan to get it merged:
-
Models commit cherry-picked ahead. Users were blocked on Opus 5/Sonnet 5 (#19), so I pulled
3e21886onto master (your authorship preserved) and released it as v0.9.3. Please rebase this branch on latest master and drop that commit — the rest of the PR is unaffected. -
Task-proxy-by-default: accepted in principle. This flips a default the roadmap deliberately kept opt-in, and your live verification is exactly the real-world feedback it was waiting for. Before merging I'll run my own smoke test on my setup (foreground + parallel subagent dispatch), since a default change should be verified by more than one environment. Assuming that's clean, this lands as v0.10.0 (minor bump for the behavior change) with a prominent changelog entry covering the
permission.taskrequirement and theproxyToolsescape hatch for anyone who wants the old behavior. -
Small ask: the AGENTS.md gotchas currently document the old
--session-idbehavior and the JSON-with-comment-stripping parser — if you're touching the branch anyway for the rebase, updating those two entries to match the new reality would be great; otherwise I'll do it in a follow-up.
Also flagging: your settlement-window fix for the result-before-broker race looks like the likely root cause behind #15 — I've asked over there for a retest on top of this branch.
|
Everything in this PR is now on master and published as v0.10.0 — landed via cherry-pick with your authorship preserved on all seven remaining commits (the models commit already shipped in v0.9.3). GitHub can't mark a cherry-picked PR as "merged", hence the close, but the git history credits you directly: What shipped on top of your branch during integration:
Verification before release: your 226 tests plus the absorbed suites (256/256 total), typecheck + build clean, your This was a genuinely excellent contribution — the |
2 participants
Summary
This branch makes OpenCode subagent dispatch through the plugin work reliably end-to-end, and fixes four defects found while testing it against a live OpenCode + Claude CLI setup: session continuation used the wrong CLI flag, configs were parsed as strict JSON, a process-handoff race corrupted session ownership, and benign proxy teardown logged as warnings.
All changes were verified live (real Claude CLI, real OpenCode session, foreground + parallel subagent dispatch) in addition to the test suite: 226/226 tests pass, typecheck and build clean.
Changes
1. Reliable OpenCode subagents (
f00972e)taskis now included in the default proxy tool set, so Claude's built-inAgenttool is disabled by default and subagent calls route through OpenCode's native TaskTool — OpenCode owns subagent lifecycle, permissions, and orchestration.task_id(resume a prior subagent session),command, andbackground.resultevent arriving before the broker subscriber attached could drop the Task call entirely (a bounded settlement window preserves thetool-callsfinish boundary). This is likely the same class of failure reported in fix: remove stopReason short-circuit from auto-continue #15.2. Resume sessions with
--resume, not--session-id(a6d0df2)--session-idmeans "create a new session with this UUID" — the CLI rejects it withSession ID … is already in usewhenever a transcript for that id already exists on disk, live process or not. Every respawn that tried to continue a conversation (MCP hot-reload, LRU eviction, crash recovery) failed this way, cleared the session, and silently fell back to a fresh session with history re-injected as text. Verified empirically against the real CLI. Continuation now uses--resume <id>; the stderr recovery matcher also catches the lowercaseNo conversation found with session IDerror--resumeprints for a purged transcript.3. Session handoff race (
3d9a405)MCP hot-reload killed the old Claude process and immediately respawned with the same session id while the old process could still be alive. Replacement now waits for the old owner to exit (SIGKILL escalation after 1.5 s, fresh session as last resort), and a stale process's exit handler can no longer delete its replacement from the session map.
4. Parse configs as JSONC (
0bbf6c0)opencode.json/opencode.jsoncwere comment-stripped and fed toJSON.parse, so trailing commas and other JSONC syntax failed with aWARN: failed to parsebubble. The bridge now usesjsonc-parser@3.3.1withallowTrailingComma: true— the exact parser and options OpenCode itself uses for these files.5. Notice-level logging for proxy teardown (
d9add5e)close()rejecting in-flight proxy calls ("proxy MCP server closed") surfaced as a WARN bubble on every normal teardown (process exit, abort, hot-reload respawn, compaction), even though the owning process is already gone and the rejection is pure cleanup. The expected-cleanup classifier is now an exported, unit-tested function and includes the server-closed message; genuine errors still log at WARN.Usage notes
With Task proxied by default, subagents need Task permission on the agent, e.g.:
{ "agent": { "build": { "permission": { "task": { "*": "allow" } } } } }Background subagents still require
OPENCODE_EXPERIMENTAL_BACKGROUND_SUBAGENTS=true; nested subagents require explicitpermission.taskon the child agent and a largersubagent_depth. Documented in the README updates.