fix(opencode): surface child-session activity and approvals in parent thread - #7840
fix(opencode): surface child-session activity and approvals in parent thread#7840sabraman wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
One finding: the new child-session suppression in ProviderRuntimeIngestion changes backend behavior without a focused test. Everything else in the changed Effect code (namespace imports from effect/* subpaths, runOpenCodeSdk/toRequestError error mapping, Effect.catch on fully handled channels, no new runtimes or service-instance injection) matches the conventions.
Posted via Macroscope — Effect Service Conventions
| : activeTurnId === null || eventTurnId === undefined || sameId(activeTurnId, eventTurnId); | ||
|
|
||
| if (shouldApplyRuntimeError) { | ||
| if (shouldApplyRuntimeError && event.providerRefs?.providerParentSessionId === undefined) { |
There was a problem hiding this comment.
This guard silently changes ingestion behavior: a runtime.error originating from a child session no longer moves the thread session into error with lastError, while the activity is still recorded. Since backend behavior changed, consider adding a focused ingestion test — the suite currently covers only the parent-session case ("maps runtime.error into errored session state") and has no event carrying providerRefs.providerParentSessionId, so this branch is untested and a regression would go unnoticed.
Posted via Macroscope — Effect Service Conventions
There was a problem hiding this comment.
Added in 72d7d4d — focused ingestion test: a runtime.error carrying providerParentSessionId records the activity but leaves parent session state untouched (keeps parent session state when a child-session runtime.error arrives).
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces substantial new capability for child/nested session management in the OpenCode adapter (~1500 lines). Multiple unresolved high-severity findings identify potential bugs in permission synchronization logic that could overwrite inherited permissions or permanently drop nested child events. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
d528006 to
350c5ad
Compare
| const probe = yield* loadOpenCodeSession(context, child.sessionId, child.directory); | ||
| const existing = | ||
| probe.kind === "found" | ||
| ? ((probe.info as { readonly permission?: PermissionRuleset }).permission ?? []) | ||
| : []; |
There was a problem hiding this comment.
🟠 High Layers/OpenCodeAdapter.ts:879
When session.get returns unavailable (or missing), synchronizeChildPermissions updates the child with only the runtime baseline, erasing inherited or agent-specific restrictions; in full-access mode this can turn a restricted child into */* allow. Return without updating unless the probe is found, so existing permissions are never overwritten without first loading them.
const probe = yield* loadOpenCodeSession(context, child.sessionId, child.directory);
- const existing =
- probe.kind === "found"
- ? ((probe.info as { readonly permission?: PermissionRuleset }).permission ?? [])
- : [];
+ if (probe.kind !== "found") {
+ return;
+ }
+ const existing = (probe.info as { readonly permission?: PermissionRuleset }).permission ?? [];🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/Layers/OpenCodeAdapter.ts around lines 879-883:
When `session.get` returns `unavailable` (or `missing`), `synchronizeChildPermissions` updates the child with only the runtime baseline, erasing inherited or agent-specific restrictions; in `full-access` mode this can turn a restricted child into `*`/`*` allow. Return without updating unless the probe is `found`, so existing permissions are never overwritten without first loading them.
| if (parentProbe.kind === "missing") { | ||
| context.ignoredSessionIds.add(sessionId); | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Parent probe blacklists on directory miss
High Severity
The leaf-session probe retries session.get without a directory after a scoped miss, but the parentID walk only tries a scoped lookup and treats a directory-scoped 404 as a confirmed miss. That permanently adds the leaf id to ignoredSessionIds, so nested child events (including approvals) stay dropped for the rest of the session.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 72d7d4d. Configure here.
| context.client.session.update({ | ||
| sessionID: child.sessionId, | ||
| directory: child.directory, | ||
| permission: merged, |
There was a problem hiding this comment.
Failed permission probe overwrites rules
High Severity
When loading a child's existing ruleset fails (unavailable or non-found), synchronizeChildPermissions still writes the runtime-mode baseline alone and sets permissionsSynchronized. That permanently drops OpenCode-derived inherited allows and denies instead of retrying the merge on a later event.
Reviewed by Cursor Bugbot for commit 72d7d4d. Configure here.
… thread A Task-tool child session's permission.asked events were filtered out because the adapter dropped every event whose session id differed from the parent's, so external_directory prompts hung invisibly and the thread looked idle forever. Track child sessions via parentID chain resolution, route their events to the parent thread with real child session refs, map unmapped permission kinds to a visible generic approval carrying raw permission detail, reply against the child session directory, keep the turn open while children run, settle pending permissions/questions on abort, error, exit, and teardown, and re-assert the T3 runtime-mode ruleset on each child at registration layered under OpenCode's derived rules so inherited allows and plan/subagent denies stay authoritative while full-access vs supervised behavior no longer depends on upstream inheritance (#30527/#30529). Subagent narration stays out of the parent transcript, mirroring the Claude adapter.
72d7d4d to
cbd40e7
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cbd40e7. Configure here.
| raw, | ||
| child.failure ? "failed" : "completed", | ||
| child.failure ?? "OpenCode child session completed.", | ||
| ); |
There was a problem hiding this comment.
Child idle cancels open approvals
High Severity
handleOpenCodeSessionIdle always runs settlePendingOpenCodeRequests for child sessions before completing the child task. Root idle deliberately leaves pending approvals open so the turn can finish after the user replies, and this PR’s own root regression relies on idle-while-pending. The same OpenCode interleaving on a child auto-rejects the approval and marks the task done, so child permission prompts can disappear before anyone can answer.
Reviewed by Cursor Bugbot for commit cbd40e7. Configure here.
1 participant
Problem
When an OpenCode Task-tool child session requested a permission — for example
external_directoryto read a sibling project — the adapter dropped every event whose session id differed from the parent's. The child sat waiting on an approval nobody could see: OpenCode's REST API reported the child busy with a pending permission,projection_pending_approvalshad no row for the thread, and the T3 thread showed as running forever with no way forward. Reproduced on T3 Code nightly 0.0.34 with OpenCode 1.18.19.What this does
parentIDchain (with per-hop directories and an ignore-list for unrelated sessions), so child Task activity flows into the parent thread carrying real provider session refs.permission.asked/question.askedrender as regular approvals. Permission kinds the canonical map doesn't know (external_directory,task,webfetch, …) get a generic approval card that preserves the raw permission detail and args instead of disappearing. Replies go back to the exact child request using its real request ID and directory.external_directory) and agent-level denies authoritative while the baseline covers everything else. Full-access stays full-access, supervised stays supervised, nothing is auto-allowed. This also makes behavior independent of upstream permission-inheritance bugs (Subagent loses inherited external_directory allow when spawned via task tool anomalyco/opencode#30527 / #30529).Evidence
Before/after screenshots of the invisible-vs-visible child approval will be attached shortly.
Tests
88 adapter + ingestion tests pass, including new coverage for: child approval → visible parent approval + reply routed to the child directory; inherited-rule layering; root turn completing once its last pending approval resolves after idle; child narration suppression; teardown settling pending approvals; full-access inheritance; supervised prompts staying enforced; and child runtime errors leaving parent session state alone.
Note
High Risk
Touches OpenCode session lifecycle, permission replies, and parent-thread error/turn completion. Bugs here can strand approvals, mis-route replies, or leave threads running or in error.
Overview
OpenCode Task-tool child sessions are no longer dropped when their session id differs from the parent. The adapter walks the
parentIDchain, tracks children, and emitstask.*rows plusproviderRefsso activity and approvals land on the parent thread.Child permission prompts (including unmapped kinds like
external_directory) show as generic Permission approval cards with raw detail. Replies go to the child’s directory. Pending requests settle on abort, error, idle, or teardown. The parent turn stays open while children run, and only completes when the root is idle with no active children or pending prompts. Childruntime.erroris recorded as activity and does not flip the parent session to error.Child permission rules layer the thread’s runtime-mode baseline under OpenCode’s inherited rules (last-match-wins). Subagent narration stays out of the parent transcript. Web and mobile treat
requestKind: "unknown"as a first-class pending approval.Reviewed by Cursor Bugbot for commit cbd40e7. Bugbot is set up for automated code reviews on this repo. Configure here.