fix(acp): child/subagent session permission requests no longer hang forever - #37902
Open
aiken884 wants to merge 1 commit into
Open
fix(acp): child/subagent session permission requests no longer hang forever#37902aiken884 wants to merge 1 commit into
aiken884 wants to merge 1 commit into
Conversation
…rmission gating Child sessions spawned server-side by the `task` tool were never registered in the ACP session store (only session/new|load|resume|fork register one). When such a session raised a permission.asked event, acp/permission.ts's Handler.process() called session.tryGet() on the unregistered child id, got undefined, and silently returned -- leaving the underlying Permission.ask Deferred unresolved forever. Under an "ask" ruleset this hangs the entire session/prompt call indefinitely. This is the same defect described in anomalyco#12133 ("Permission requests from child sessions do not get forwarded via ACP and hangs forever"). That issue was closed by anomalyco#13222, but anomalyco#13222's actual changes (a Windows git-subprocess stdin-inheritance fix in util/git.ts) are unrelated to ACP session registration and never touch acp/permission.ts or acp/session.ts -- the PR's description bundled several superficially similar "ACP hangs" issues together, and anomalyco#12133 appears to have been swept in by that list without the underlying root cause actually being addressed. Reproduced against current dev HEAD with a real subprocess + scripted tool call (hangs at the ask), and separately against a real `opencode acp` + real acpx + real model in production use (model organically delegated to a Task subagent and the whole invocation hung past a 90s timeout) -- this is a live, currently-reproducible defect, not a stale report. Fix: resolveAncestor() in acp/session.ts walks the SDK's `parentID` chain to find the nearest ancestor session that IS registered in the ACP store, so a child session's permission ask still routes through the connection instead of being silently dropped. permission.ts's Handler.process() uses this instead of a direct tryGet(), and replies with an active reject (not a silent return) when no ancestor can be resolved, so the underlying Deferred is never left hanging regardless of outcome. Regression test: test/cli/acp/child-session-permission.test.ts drives a real `opencode acp` subprocess through a task-tool delegation under an "ask" ruleset and asserts the prompt call resolves (not times out) via the root connection's existing fail-closed no-capability path. Fails (20s timeout) without the fix, passes (~2.5s) with it. Full existing acp+permission suite (249 tests) passes unchanged; typecheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Independent confirmation from another ACP client integration. Environment:
I reproduced this deterministically with: OPENCODE_PERMISSION='{"*":"ask","task":"allow"}' \
opencode acp --print-logsThe ACP client:
Observed on both 1.15.10 and 1.17.11:
Result from both runs: {
"childReadAskObserved": true,
"parentPermissionRequestCount": 0,
"childPermissionRequestCount": 0,
"promptSettledBeforeCancel": false,
"routing": "missing-from-acp-wire"
}This independently confirms that #12133 is still reproducible and was not fixed by #13222. The It would also be useful to keep coverage for nested subagents and simultaneous permission requests from multiple children sharing the same ACP parent. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Issue for this PR
Closes #12133
Type of change
What does this PR do?
Child sessions spawned server-side by the
tasktool are never registered in the ACP session store (onlysession/new/load/resume/forkregister one). When such a session raises apermission.askedevent,acp/permission.ts'sHandler.process()callssession.tryGet()on that unregistered id, getsundefined, and just returns — the underlyingPermission.askDeferredis never resolved, so under an "ask" ruleset the wholesession/promptcall hangs forever.This is exactly #12133. That issue shows as closed via #13222, but #13222's actual diff (
util/git.ts,snapshot/index.ts) fixes a Windows-only git-subprocess stdin-inheritance deadlock — it never touchesacp/permission.tsoracp/session.ts. The PR description lists several "ACP hangs forever" issues together and #12133 looks like it got swept into that list without the actual root cause being addressed. I hit the same hang on currentdevwith a real subprocess.Fix:
resolveAncestor()(new, inacp/session.ts) walks the SDK'sparentIDchain for a session id that isn't in the ACP store, until it finds an ancestor that is registered.permission.ts'sHandler.process()uses this instead of a baretryGet(), so a child session's permission ask still routes through the connection. If no ancestor can be resolved at all, it now replies with an active reject instead of silently returning, so theDeferredis never left hanging either way.How did you verify your code works?
Added
test/cli/acp/child-session-permission.test.ts: drives a realopencode acpsubprocess through a task-tool delegation under anedit:"ask"ruleset and asserts thesession/promptcall resolves instead of hanging. Confirmed it fails (20s timeout) ondevwithout this change, passes (~2.5s) with it.Also reproduced this against real
acpx+ a real model outside the test suite: with a strict permission policy, the model chose on its own to delegate a file-write to a Task subagent, and the whole invocation hung past a 90s timeout with the same symptom.Ran the full existing
acp+permissionsuite (249 tests) — all pass, no regressions.tsgo --noEmitclean.Checklist