fix(session): route nested blockers to ancestor clients - #30639
fix(session): route nested blockers to ancestor clients#30639literally-dan wants to merge 1 commit into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: I found several related PRs that may be relevant:
These PRs are all tackling related aspects of nested subagent permission handling. You may want to verify:
|
|
For triage, here's how this relates to the other open PRs in this area, so nobody's work gets stepped on:
What this PR additionally covers that I didn't find in the others:
It also covers #7654 (questions from nested subagents), since the questions memo had the same direct-child limitation. If a maintainer would rather land one of the existing TUI PRs, I can narrow this to just the run and ACP parts. |
5817e56 to
dd217d2
Compare
dd217d2 to
080dc2c
Compare
baef53c to
8c821a9
Compare
dcbf719 to
d4243cd
Compare
A permission raised inside a subagent never reached a client. `ACPSession` only holds sessions the client created, so `tryGet(permission.sessionID)` missed every subagent and the handler returned without replying. Since `Permission.ask` waits on a deferred with no timeout, any tool needing permission at depth >= 1 hung the run outright. Run mode had the same gap: it compared the event's session against the root id, so a descendant's prompt was treated as somebody else's. Both now resolve the owning session by walking the parent chain and route the prompt to the nearest ancestor the client actually knows about, at any depth. Run mode additionally seeds the transitive descendant set up front so attach mode surfaces prompts that were already pending, and extends it from `session.created`/`session.updated`. Resolution retries are bounded. The lookup fails transiently for a server restart or a grandchild whose `session.created` has not projected yet, but it also fails permanently for a parent chain cycle or a parentID pointing at a deleted session. Retrying those forever converts the hang this change removes into a polling hang, so `src/session/ancestry.ts` caps the retry at roughly seven seconds and reports exhaustion, and both callers then reject the request rather than leave the nested tool call waiting. That module also gives the two callers one implementation instead of the two near-identical copies they would otherwise keep in step by hand. Replies tolerate a missing request. The server drops pending permissions when a session aborts and rejects the cascade itself, so a reply that lost that race is already answered and must not fail the run. The descendant walk is bounded on every axis: fan-out concurrency, a per-request timeout, a total deadline, a size cap, and cycle detection, so a corrupt parent pointer cannot spin or fan out without limit. The `session.deleted` subscription drops cached parent ids and aborts prompts for the removed session, and stopping the subscription tears down every retained retry. Run mode no longer awaits the descendant seed before subscribing to `/event`. That route has no replay, so the round trip risked losing events published in the gap, including the `session.status: idle` the loop breaks on. The seed resolves in the background and prompt routing awaits it. The two end-to-end tests set `subagent_depth: 2`. Without it the default of 1 makes the second delegation fail, so the prompt comes from the child and the tests pass on depth-1 routing while appearing to cover nesting. Both now assert the grandchild was really prompted.
d4243cd to
1010efc
Compare
Issue for this PR
Closes #13715
Also addresses #7654.
Type of change
What does this PR do?
Nested Task sessions can raise permission and question requests several levels below the root. The TUI,
opencode run, and ACP previously matched only the session they directly tracked, so deeper requests could be dropped.The TUI now reads blockers from the full in-memory session subtree.
opencode rundiscovers existing descendants on attach, follows new descendants while streaming, and resolves ancestry when events arrive out of order. ACP follows parent links to the managed session and serializes its permission prompts.Tree discovery has cancellation, request and total timeouts, bounded concurrency, a session limit, and cycle detection. ACP shutdown cancels retained lookups and queued prompts, and duplicate events share the pending request.
How did you verify your code works?
The focused opencode suite passes 70 tests. The TUI subtree tests pass. Subprocess tests cover a grandchild permission in both headless run and ACP.
bun typecheckpasses inpackages/opencodeandpackages/tui.Screenshots / recordings
Not applicable.
Checklist