fix: keep parent chat running in drawer while a subagent works - #233
fix: keep parent chat running in drawer while a subagent works#233yuga-hashimoto wants to merge 2 commits into
Conversation
A session blocked on the task tool emits no events while its subagent runs. If the user navigated away, the chat reported the parent finished and the drawer settled it on the grey idle dot for the subagent's entire run. Learn each subagent's parent from session.created/session.updated events (falling back to the runtime API when the creation was missed) and forward the child's activity up the parent chain, so the drawer keeps showing the spinner. Ancestors the runtime already reported idle are left alone, so an experimental background subagent cannot resurrect a finished turn.
|
🔍 OpenCodeReview found 4 issue(s) in this PR.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "session.created" -> { | ||
| val session = | ||
| json.decodeFromJsonElement( | ||
| OpenCodeSession.serializer(), | ||
| properties["info"]!!.jsonObject, | ||
| ) | ||
| OpenCodeEvent.SessionCreated(session) | ||
| } |
There was a problem hiding this comment.
[maintainability · low]
session.created と session.updated の分岐は properties["info"] を OpenCodeSession にデコードするロジックが完全に重複しています。private fun parseSession(properties: JsonObject) = json.decodeFromJsonElement(OpenCodeSession.serializer(), properties["info"]!!.jsonObject) のようなヘルパーを抽出し、各分岐を OpenCodeEvent.SessionCreated(parseSession(properties)) / OpenCodeEvent.SessionUpdated(parseSession(properties)) とすると冗長さが解消され、今後デコード処理を変更する際の差分も1箇所に集約できます。
| var parentId = parentIdOf(target, sessionId) | ||
| while (parentId != null) { | ||
| if (isRuntimeIdle(parentId)) break | ||
| val ancestorId = parentId |
There was a problem hiding this comment.
[bug · medium]
parentIds は runtime のイベント/API 由来のデータをキャッシュしており、万一親チェーンに循環（A→B→A のような不整合データ）が混入すると、isRuntimeIdle による break 条件が成立しない限りこの while ループが無限に回り続けます。ループ内では parentIdOf がネットワーク呼び出しを伴う可能性があり、さらに mutableState.update が連続実行されてイベント収集コルーチンがハングします。visited 集合で訪問済みセッションを検知する防御を追加してください。
Suggestion:
| var parentId = parentIdOf(target, sessionId) | |
| while (parentId != null) { | |
| if (isRuntimeIdle(parentId)) break | |
| val ancestorId = parentId | |
| var parentId = parentIdOf(target, sessionId) | |
| val visited = mutableSetOf<String>() | |
| while (parentId != null) { | |
| if (!visited.add(parentId)) break | |
| if (isRuntimeIdle(parentId)) break | |
| val ancestorId = parentId |
| parentId: String?, | ||
| ) { | ||
| if (sessionId.isBlank()) return | ||
| synchronized(parentLock) { parentIds[sessionId] = parentId } |
There was a problem hiding this comment.
[bug · medium]
API 解決に失敗した場合も null が parentIds に永続キャッシュされるため、「親なしと確認済み」と「解決失敗」が区別できません。一時的な API エラーで subagent の親解決が失敗すると、以後そのセッションは親なしとして扱われ、SessionIdle 時に onSessionIdle が誤発火したり activateAncestors が祖先を活性化できなくなったりしますが、キャッシュ済みのため再試行もされません。解決に成功した場合のみキャッシュする（失敗時はキャッシュしない）よう修正してください。
Suggestion:
| synchronized(parentLock) { parentIds[sessionId] = parentId } | |
| if (parentId != null || cachedParent(sessionId)) { | |
| synchronized(parentLock) { parentIds[sessionId] = parentId } | |
| } |
| if (sessionId in parentIds) return parentIds[sessionId] | ||
| } | ||
| // Misses the creation event when the stream reconnected mid-run; ask the runtime instead. | ||
| val parentId = runCatching { target.session(sessionId).parentId }.getOrNull() |
There was a problem hiding this comment.
[bug · medium]
target.session(sessionId) は suspend なネットワーク/プロセス呼び出しですが、runCatching は CancellationException も捕捉してしまうため、収集コルーチンがキャンセルされた際にキャンセレーションが握りつぶされ、構造的並行性が壊れます。さらに失敗結果の null が下の行でキャッシュされるため、キャンセル時に誤った親情報が永続化されます。CancellationException は必ず再送出してください。
Suggestion:
| val parentId = runCatching { target.session(sessionId).parentId }.getOrNull() | |
| val parentId = try { | |
| target.session(sessionId).parentId | |
| } catch (e: CancellationException) { | |
| throw e | |
| } catch (e: Exception) { | |
| null | |
| } |
1 participant
Problem
When a session spawns a subagent (the
tasktool), the drawer's session list drops the parent back to the grey idle dot even though the run is plainly still in flight.Reproduced against a live OpenCode 1.18.15 server: while a subagent works, the parent session emits no events at all — its loop is blocked inside the task tool, and every stream event carries the child's session id.
The drawer derives the running state from
RuntimeActivityRepository.activeSessionIds. Two things combine to lose the parent:So the parent sat on the grey dot for the subagent's entire run.
Fix
session.created/session.updatedevents to learn each subagent's parent (falls back to a one-shotsession()lookup when the creation event was missed, e.g. app restart mid-run).No UI changes needed: the drawer already renders
RUNNINGfor anything inactiveSessionIds.Verification
This PRoot/aarch64 environment cannot run the full Android build (the SDK ships x86-64
aapt2), so in addition to careful review I compiled the affected pure-JVM sources standalone and ran their unit tests directly:OpenCodeEventParserTest: 20/20 pass (incl. 2 new tests for the parsed parent link)RuntimeActivityRepositoryTest: 18/18 pass (incl. 4 new tests below)./gradlew spotlessCheck: passNew tests: