Repository navigation
feat(app): add subagents side-panel tab - #55496
sdpfigueiredo wants to merge 2 commits into
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
5337c0f to
fa7612d
Compare
There was a problem hiding this comment.
I couldn't check this in the app: the panel is desktop/web-only and this environment only drives the TUI. The 33 new tests pass and bun run typecheck in packages/gui-extensions is clean. A short recording against a real session, with running and finished subagents, would help.
Public API: unchanged. This is a new built-in extension only.
Main concerns (details inline):
- Copied status logic:
status.tscopies howpackages/app/src/session/requests/background.ts(#53247) works out background subagents (synthetic completion notices, the latest unfinished message, backgrounded completions) instead of sharing it. That logic changes often, so the panel and the header's running menu will start to disagree. Please move the shared reader to one place both can import, e.g.session-uior the client. - Translations: every non-English locale file has only 12 keys. They include
showAll, which nothing uses, and are missing about 20 keys fromen.ts(search, sort, tabs, empty states, elapsed, footer…). Most of the panel would show in English everywhere. Please either translate the full key set or ship onlyen.ts. - Repeated fetches: while subagents are running, every change to the running set re-fetches the messages of all running children.
- English-only times: the relative times in the activity feed are hardcoded English.
Smaller points:
- The first-open fetch swallows every error.
renderer.tsxhasn't been through the formatter (), draggable: false).- This adds a second header button next to the existing running menu. Worth checking with maintainers that both are wanted.
| * the latest uncompleted assistant message (older ones are stale), and a | ||
| * backgrounded completion only counts until its synthetic notice lands. | ||
| */ | ||
| export function subagentTasks(messages: SessionMessageInfo[]): SubagentTasks { |
There was a problem hiding this comment.
This copies how packages/app/src/session/requests/background.ts reads background tasks: the synthetic source: "subagent" notices, "latest uncompleted assistant message" and metadata.status === "running" backgrounded completions. The two copies will drift, and the panel and the header's running menu will then show different states for the same subagent. Could this reader move to a shared package (e.g. session-ui or the client data layer) so both use it?
There was a problem hiding this comment.
Fixed in 0dc2b8b: new shared reader at packages/session-ui/src/subagent-transcript.ts (scanSubagentTranscript plus taskStatus, partBackgrounded, toolSessionID, toolDescription, toolAgent), exported via ./subagent-transcript. Both packages/app/src/session/requests/background.ts and packages/gui-extensions/src/agents/status.ts now consume it, so header menu and panel cannot drift on notices, liveness, or backgrounded completions. The scan returns raw calls with live/backgrounded flags; shell handling stays in background.ts and row derivation stays in the panel. Parity is locked by new background.test.ts (4 tests) and the moved subagent-transcript.test.ts (14 tests). session-ui typecheck clean, import-boundaries green.
| return ids.length > 0 ? ids.join("\0") : undefined | ||
| }) | ||
|
|
||
| createKeyed(runningKey, (key) => { |
There was a problem hiding this comment.
runningKey changes whenever any subagent starts or finishes. Each change then re-fetches messages for every running child, not only the new ones. With N parallel subagents finishing one by one, that is roughly N² requests, which contradicts the "no per-child requests" claim in the description. Syncing only the IDs that weren't synced before (or relying on the live stream after the first sync) would avoid this.
There was a problem hiding this comment.
Fixed in 718354e: the panel keeps a synced set and each runningKey change syncs only IDs not synced before (fresh), so membership churn no longer refetches survivors. One-shot per child; the live event stream owns freshness after that, and history rows sync on expand.
| "total": "Gesamtkosten", | ||
| "subagents": "Sub-Agents", | ||
| "tokens": "Token", | ||
| "showAll": "Alle anzeigen ({{count}})", |
There was a problem hiding this comment.
All 62 locale files have the same 12 keys, apparently carried over from the v1 attempt. They include showAll, which en.ts doesn't have and nothing uses, and they are missing about 20 keys en.ts has (view, count, search.placeholder, sort.*, tab.all/running/history, empty*, elapsed, latestActivity, lastResponse, footerNote, cost, session). Non-English users would mostly see English. Please translate the full key set, or drop these files and ship only en.ts.
There was a problem hiding this comment.
Fixed in 0dc2b8b per the suggested option: dropped the 61 carried-over locale files (stale 12-key set with the unused showAll) and ship i18n: { en } only, with runtime fallback to English. Full translation of the ~32-key set is left for a follow-up by translators.
| } | ||
|
|
||
| /** Short relative stamp: 2s ago, 18m ago, 3h ago, older falls back to the clock. */ | ||
| export function formatRelative(when: number, now: number, locale: string): string { |
There was a problem hiding this comment.
"s ago", "m ago" and "h ago" are hardcoded English, so the activity feed won't be translated. Intl.RelativeTimeFormat(locale) or i18n keys would fix that.
There was a problem hiding this comment.
Fixed in 718354e: formatRelative now uses Intl.RelativeTimeFormat(locale, { numeric: "auto", style: "narrow" }) for the seconds/minutes/hours stamps; 24h+ still falls back to the localized clock.
| export type DerivedStatus = "idle" | "busy" | "cancelled" | "completed" | "error" | ||
|
|
||
| /** Tool names that spawn a child session, current first for the common case. */ | ||
| const subagentNames = ["subagent", "task"] |
There was a problem hiding this comment.
task is the v1 tool name. In v2 subagents are spawned only by subagent (task appears only in the v1 database migration). Unless there is a v2 path that still produces task parts, this can be dropped.
There was a problem hiding this comment.
Kept intentionally: the V1 migration (packages/core/src/database/v1-migration.bun.ts, migrateTool) preserves the original tool name (name: part.tool), so migrated transcripts still contain task parts. The shared matcher defaults to ["subagent", "task"] with subagent first, and accepts an override. Covered by the legacy-parts test in subagent-transcript.test.ts. Happy to drop task if maintainers prefer subagent-only going forward.
|
All review points addressed in
Verification: agents 20/20, subagent-transcript 14/14, background 4/4; gui-extensions + session-ui typecheck clean. ( |
There was a problem hiding this comment.
Re-reviewed at 0dc2b8b. I still couldn't run the panel itself: it is desktop/web-only and this environment only drives the TUI. New tests pass in session-ui (14), app (4, background.test.ts) and gui-extensions (20), and typecheck is clean in all three packages.
Public API: unchanged. The new @opencode/session-ui/subagent-transcript export is internal to the workspace.
Earlier points:
- Copied status logic: mostly resolved. Both readers now use
scanSubagentTranscript. One remaining piece is noted inline. - Locale files: resolved, only
en.tsships. - Refetching every running child: resolved, only new IDs are fetched now.
- English-only relative times: resolved,
Intl.RelativeTimeFormatis used. tasktool name: keeping it for transcripts migrated from v1 makes sense.- Formatting: fixed.
| ] | ||
| }) | ||
|
|
||
| const assistant = messages.reduce<SessionMessageAssistant | undefined>((latest, message) => { |
There was a problem hiding this comment.
This reduce still finds the latest uncompleted assistant message itself, and blocking re-checks the subagent parts in it. scanSubagentTranscript already returns live and per-call live flags. Building the subagent half of blocking from scan.calls.filter((c) => c.live && c.status === "running") (and using scan.live for the shell half) would finish the de-duplication. As it is, the header's "what counts as live" rule can still drift from the panel's.
There was a problem hiding this comment.
Done in 968ef8b: the reduce is gone. Subagent blocking now builds from scan.calls (live && status === "running", legacy task parts surfacing as subagent, matching the panel) and the shell half reads scan.live content. Shell background-task collection stays a separate pass over all messages, untouched. The type assertion and its suppression are gone with it (plain as const literals). Guarded by the existing running/stale cases plus one new shell-blocking case (live vs stale shell parts); 5/5 pass, oxlint 0, no background.ts typecheck errors. Focused-suite timing unchanged vs baseline (background ~300ms, transcript ~130ms).
d71fb63 to
1ebcd4c
Compare
|
Thanks. I checked 1ebcd4c: |
|
Good catch, but it turns out to be unobservable: the only renderer of |
|
Agreed, I checked it: |
f726ae4 to
67a15f5
Compare
There was a problem hiding this comment.
The latest push only rebases the same four commits onto newer dev; their content is unchanged since 1ebcd4c. Re-ran the tests on the rebased head: session-ui 14, app 5, gui-extensions 20, all passing, and typecheck is clean in all three packages. I couldn't run the panel itself, since it is desktop/web-only.
Public API: unchanged.
Resolved: the shared subagent transcript reader (now also used for blocking), syncing only new running children, shipping only en.ts, and localized relative times. No new concerns.
67a15f5 to
67860ab
Compare
There was a problem hiding this comment.
The latest push only rebases the same four commits onto newer dev; the only difference is rebase context in main.ts and the README. Re-ran the tests on the new head: session-ui 14, app 5, gui-extensions 20, all passing, and tsgo -b is clean in all three packages. I couldn't run the panel itself, since it is desktop/web-only.
Public API: unchanged. No new concerns.
67860ab to
ba9583c
Compare
There was a problem hiding this comment.
The latest push only rebases the same four commits onto newer dev, with no content changes. Re-ran the tests on the new head: session-ui 14, app 5, gui-extensions 20, all passing, and tsgo -b is clean in all three packages. I couldn't run the panel itself, since it is desktop/web-only.
Public API: unchanged. No new concerns.
ba9583c to
29313d4
Compare
There was a problem hiding this comment.
The four panel commits are unchanged, only rebased again. gui-extensions tests pass (20). I couldn't run the panel itself, since it is desktop/web-only.
The new commit 29313d4 (.opencode/ tool import, README and test) is unrelated to this feature and broken:
- It switches
github-pr-search.tsto@opencode-ai/plugin/tool, the v1 package, which isn't installed here. Latestdevstill imports@opencode/plugin. - The new test imports
../tool/github-triage.ts, which #58888 removed. bun test ./test/tool-sdk.test.tsin.opencodefails with "Cannot find module '@opencode-ai/plugin/tool'".
Please drop this commit from the PR. If the local tool import really is broken, a separate PR against current dev is the place for it.
Public API: unchanged.
29313d4 to
5a5d06d
Compare
There was a problem hiding this comment.
The unrelated .opencode commit is gone. The PR is back to the four panel commits, unchanged in content. Tests pass in app (5), gui-extensions (20) and session-ui (14). I couldn't run the panel itself, since it is desktop/web-only.
Public API: unchanged. No new concerns.
5a5d06d to
4c9f13f
Compare
e5fc2ab to
dd95aa1
Compare
Issue for this PR
Closes #37267
This ports #39382 (closed as V1-only) to the current architecture.
Type of change
What does this PR do?
Adds a "Subagents" side-panel tab so subagent activity can be followed without being buried by the main agent's log stream, as described in #37267.
The tab lists each subagent spawned by the current session with its task name, live status, cost, token usage and model. Clicking a row opens that subagent's session, which is the drill-down to its full output. New subagents appear at the top. A header button with a busy dot and a
+menu entry open the panel.How it works, briefly: the panel is a
gui-extensionsside-region contribution (packages/gui-extensions/src/agents). Children come from the live sync store filtered byparentIDplus a one-shot family backfill, so there is no polling and no per-child requests. Running/retrying state comes from the live status map; terminal states (completed / cancelled / failed) come from the parent session'ssubagenttool parts, mirroring how the header's background-task reader derives them (includingabortedstructured errors and synthetic completion notices). Foreground subagent calls that have no child session yet render as static Running rows.Status icons match the timeline's task cards (spinner while running, subagent glyph otherwise). Copy lives in the extension's own
i18n/en.ts(62 locales ship translated).How did you verify your code works?
bun test src/agents/inpackages/gui-extensions(33 tests): status derivation precedence (running part wins, backgrounded completions, synthetic-notice expiry, stale-part rejection, aborted/interrupted cancellation), children sorting/filtering, cost/token formatting, link shape.bun run typecheckinpackages/gui-extensions: clean.oxlinton touched files: 0 warnings.extension-graphcomponent spec: all built-ins includingagentsboot with every optional provider disabled.Screenshots / recordings
Checklist