feat(host-service): terminalAgents tracker + pane wiring#4901
Merged
Conversation
In-process per-terminal agent binding tracker on host-service, plus tRPC surface (listByWorkspace, findActive, getOrCreate, onWorkspaceChange). No consumers wired yet — module + API only. Decisions logged: in-mem Map, per-terminal granularity, delete on exit, latest-event tie-break, primitives over bundled send.
In-process per-terminal agent binding store on host-service, populated by the existing notifications.hook event receiver and drained on terminal exit. Exposes a tRPC surface (listByWorkspace, findActive, getOrCreate, onWorkspaceChange observable) so non-renderer consumers (automations) can reuse live agent sessions instead of always spawning new ones. No persistence: Map clears on host-service restart and rehydrates from the next hook event. Per-terminal granularity; agent swap inside the same pty overwrites in place.
Replace the renderer-side useV2AgentBindingStore (Zustand) with a React Query hook against host-service terminalAgents.listByWorkspace, invalidated by agent:lifecycle and terminal:lifecycle workspace events. Host store is now the single source of truth; the renderer just mirrors it. TerminalPaneIcon takes a new workspaceId prop; HostNotificationSubscriber no longer mutates the (now-deleted) Zustand store.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (9)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR implements host-service TerminalAgentStore, records lifecycle events, exposes terminalAgents tRPC procedures (list/find/getOrCreate/subscribe), integrates the store into host-service context and notifications, adds renderer React Query hooks, updates UI components to use them, and removes the old local v2 agent binding store usage. ChangesTerminal Agent Tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
|
Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
Ready to review this PR? Stage has broken it down into 9 individual chapters for you: 
Chapters generated by Stage for commit 5f6ecc8 on May 28, 2026 12:13am UTC. |
Contributor
Greptile SummaryThis PR introduces a host-service
Confidence Score: 3/5The store and renderer wiring are solid, but the The core store, event wiring, and renderer migration are clean and well-tested. The risk concentrates in packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts — the
|
| Filename | Overview |
|---|---|
| packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts | New tRPC router for terminal agent tracking; getOrCreate has a TOCTOU race that can spawn duplicate terminals, and waitForBinding timeouts leave orphaned pty processes. |
| packages/host-service/src/terminal-agents/store.ts | New in-process TerminalAgentBinding store backed by a Map; well-structured with correct exit semantics and agent-swap handling, but missing setMaxListeners to avoid Node.js warnings at scale. |
| packages/host-service/src/trpc/router/notifications/notifications.ts | Correctly wires terminalAgentStore.recordEvent alongside the existing broadcastAgentLifecycle call, sharing the same occurredAt timestamp; clean change. |
| packages/host-service/src/trpc/router/terminal/terminal.ts | Adds a single markTerminalExited call after disposeSessionAndWait; correct placement and no regressions to the existing dispose path. |
| apps/desktop/src/renderer/hooks/host-service/useTerminalAgentBindings/useTerminalAgentBindings.ts | New React Query hook replacing the deleted Zustand store; invalidates on agent:lifecycle and terminal:lifecycle events, staleTime: Infinity is appropriate given event-driven invalidation. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalPaneIcon/TerminalPaneIcon.tsx | Clean migration from Zustand selectV2AgentBinding to useTerminalAgentBinding; props interface now correctly carries workspaceId. |
| packages/host-service/src/terminal-agents/store.test.ts | Thorough 9-case unit test suite covering start/intermediate/exit, agent swap, findActive tie-break, change emission, and filters; no gaps identified. |
| apps/desktop/src/renderer/routes/_authenticated/components/V2NotificationController/components/HostNotificationSubscriber/HostNotificationSubscriber.tsx | Renderer-side Zustand store mutation calls removed cleanly; host service is now the authoritative mutation point. |
Sequence Diagram
sequenceDiagram
participant Agent as Agent Process (hook)
participant Notif as notifications.hook
participant Store as TerminalAgentStore
participant TRPCRouter as terminalAgents tRPC
participant RQ as React Query (renderer)
participant Icon as TerminalPaneIcon
Agent->>Notif: POST hook (Attached / Start / Detached)
Notif->>Store: recordEvent(terminalId, agentId, eventType)
Store-->>Store: upsert or delete byTerminal[terminalId]
Store->>Store: emit("change", workspaceId)
Note over Notif: eventBus.broadcastAgentLifecycle also fires
Notif-->>RQ: agent:lifecycle workspace event
RQ->>TRPCRouter: invalidateQueries → listByWorkspace
TRPCRouter->>Store: listByWorkspace(workspaceId)
Store-->>TRPCRouter: TerminalAgentBinding[]
TRPCRouter-->>RQ: updated bindings
RQ-->>Icon: "Map<terminalId, binding>"
Icon-->>Icon: swap to agent logo (or terminal glyph)
Note over TRPCRouter: getOrCreate path
TRPCRouter->>Store: findActive(workspaceId, agentId)
alt binding exists
Store-->>TRPCRouter: existing binding (created: false)
else no binding
TRPCRouter->>TRPCRouter: createTerminalSessionInternal
TRPCRouter->>Store: on("change") → waitForBinding (10s)
Agent->>Notif: POST hook (Attached)
Notif->>Store: recordEvent
Store->>TRPCRouter: emit("change") resolves waitForBinding
TRPCRouter-->>TRPCRouter: "return { binding, created: true }"
end
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts:63-113
**TOCTOU race in `getOrCreate` spawns duplicate terminals**
Two concurrent `getOrCreate` calls for the same `(workspaceId, agentId, definitionId)` triple can both reach `findActive` before either call advances past the first `await` inside `createTerminalSessionInternal`. Both see `undefined`, both fall through, and both spawn a new terminal — producing two live agent processes where one was requested. In an automation or rapid UI-retry scenario this is a real path: the caller gets `{ created: true }` twice with two different `terminalId`s, each holding a running agent session that the other caller has no reference to.
### Issue 2 of 3
packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts:97-110
**Orphaned terminal when `waitForBinding` times out**
If the agent's hook never fires within 10 seconds, `waitForBinding` rejects with `TIMEOUT` and the caller receives an error. The terminal created by `createTerminalSessionInternal` at line 85 is left running indefinitely — it is not disposed and has no owner tracking it after the `getOrCreate` call returns. Over time (or after multiple failed launches) this leaks pty/process resources. A `try/finally` that calls `disposeSessionAndWait(created.terminalId, ...)` on rejection would close the gap.
### Issue 3 of 3
packages/host-service/src/terminal-agents/store.ts:31-32
**No `setMaxListeners` — default limit of 10 may be exceeded**
Every active `onWorkspaceChange` subscription and every concurrent in-flight `waitForBinding` call (inside `getOrCreate`) attaches one listener to the store's `"change"` event. Node.js emits a `MaxListenersExceededWarning` and prints a stack-trace-like warning when more than 10 listeners exist, which would surface in production logs. Calling `this.setMaxListeners(0)` (or a generous ceiling like `100`) in the constructor removes the noise without masking real leaks — listener teardown is already correct in both code paths.
Reviews (1): Last reviewed commit: "refactor(desktop): wire TerminalPaneIcon..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+63
to
+113
| getOrCreate: protectedProcedure | ||
| .input( | ||
| z.object({ | ||
| workspaceId: z.string(), | ||
| agentId: terminalAgentIdSchema, | ||
| definitionId: agentDefinitionIdSchema.optional(), | ||
| initialCommand: z.string().trim().min(1).optional(), | ||
| cwd: z.string().optional(), | ||
| }), | ||
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const { workspaceId, agentId, definitionId } = input; | ||
| const existing = ctx.terminalAgentStore.findActive( | ||
| workspaceId, | ||
| agentId, | ||
| definitionId, | ||
| ); | ||
| if (existing) { | ||
| return { binding: existing, created: false as const }; | ||
| } | ||
|
|
||
| const terminalId = crypto.randomUUID(); | ||
| const created = await createTerminalSessionInternal({ | ||
| terminalId, | ||
| workspaceId, | ||
| db: ctx.db, | ||
| eventBus: ctx.eventBus, | ||
| ...(input.initialCommand | ||
| ? { initialCommand: input.initialCommand } | ||
| : {}), | ||
| ...(input.cwd ? { cwd: input.cwd } : {}), | ||
| }); | ||
|
|
||
| if ("error" in created) { | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: created.error, | ||
| }); | ||
| } | ||
|
|
||
| const binding = await waitForBinding({ | ||
| store: ctx.terminalAgentStore, | ||
| workspaceId, | ||
| agentId, | ||
| definitionId, | ||
| terminalId: created.terminalId, | ||
| timeoutMs: GET_OR_CREATE_TIMEOUT_MS, | ||
| }); | ||
|
|
||
| return { binding, created: true as const }; | ||
| }), |
Contributor
There was a problem hiding this comment.
getOrCreate spawns duplicate terminals
Two concurrent getOrCreate calls for the same (workspaceId, agentId, definitionId) triple can both reach findActive before either call advances past the first await inside createTerminalSessionInternal. Both see undefined, both fall through, and both spawn a new terminal — producing two live agent processes where one was requested. In an automation or rapid UI-retry scenario this is a real path: the caller gets { created: true } twice with two different terminalIds, each holding a running agent session that the other caller has no reference to.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts
Line: 63-113
Comment:
**TOCTOU race in `getOrCreate` spawns duplicate terminals**
Two concurrent `getOrCreate` calls for the same `(workspaceId, agentId, definitionId)` triple can both reach `findActive` before either call advances past the first `await` inside `createTerminalSessionInternal`. Both see `undefined`, both fall through, and both spawn a new terminal — producing two live agent processes where one was requested. In an automation or rapid UI-retry scenario this is a real path: the caller gets `{ created: true }` twice with two different `terminalId`s, each holding a running agent session that the other caller has no reference to.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+97
to
+110
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: created.error, | ||
| }); | ||
| } | ||
|
|
||
| const binding = await waitForBinding({ | ||
| store: ctx.terminalAgentStore, | ||
| workspaceId, | ||
| agentId, | ||
| definitionId, | ||
| terminalId: created.terminalId, | ||
| timeoutMs: GET_OR_CREATE_TIMEOUT_MS, | ||
| }); |
Contributor
There was a problem hiding this comment.
waitForBinding times out
If the agent's hook never fires within 10 seconds, waitForBinding rejects with TIMEOUT and the caller receives an error. The terminal created by createTerminalSessionInternal at line 85 is left running indefinitely — it is not disposed and has no owner tracking it after the getOrCreate call returns. Over time (or after multiple failed launches) this leaks pty/process resources. A try/finally that calls disposeSessionAndWait(created.terminalId, ...) on rejection would close the gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts
Line: 97-110
Comment:
**Orphaned terminal when `waitForBinding` times out**
If the agent's hook never fires within 10 seconds, `waitForBinding` rejects with `TIMEOUT` and the caller receives an error. The terminal created by `createTerminalSessionInternal` at line 85 is left running indefinitely — it is not disposed and has no owner tracking it after the `getOrCreate` call returns. Over time (or after multiple failed launches) this leaks pty/process resources. A `try/finally` that calls `disposeSessionAndWait(created.terminalId, ...)` on rejection would close the gap.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+31
to
+32
| export class TerminalAgentStore extends EventEmitter { | ||
| private readonly byTerminal = new Map<string, TerminalAgentBinding>(); |
Contributor
There was a problem hiding this comment.
setMaxListeners — default limit of 10 may be exceeded
Every active onWorkspaceChange subscription and every concurrent in-flight waitForBinding call (inside getOrCreate) attaches one listener to the store's "change" event. Node.js emits a MaxListenersExceededWarning and prints a stack-trace-like warning when more than 10 listeners exist, which would surface in production logs. Calling this.setMaxListeners(0) (or a generous ceiling like 100) in the constructor removes the noise without masking real leaks — listener teardown is already correct in both code paths.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal-agents/store.ts
Line: 31-32
Comment:
**No `setMaxListeners` — default limit of 10 may be exceeded**
Every active `onWorkspaceChange` subscription and every concurrent in-flight `waitForBinding` call (inside `getOrCreate`) attaches one listener to the store's `"change"` event. Node.js emits a `MaxListenersExceededWarning` and prints a stack-trace-like warning when more than 10 listeners exist, which would surface in production logs. Calling `this.setMaxListeners(0)` (or a generous ceiling like `100`) in the constructor removes the noise without masking real leaks — listener teardown is already correct in both code paths.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/host-service/src/terminal-agents/store.ts`:
- Around line 64-79: The code currently carries over existing.agentSessionId and
existing.definitionId into next even when a swap changes the agent identity,
causing stale metadata to surface; update the logic in store.ts (around
variables existing, next, agentSessionId, definitionId, agentId, startedAt) so
that when existing exists and the agentId changes (or agentSessionId is provided
and differs) you not only reset next.startedAt = occurredAt but also clear any
carried-over identity fields—set next.agentSessionId = undefined and
next.definitionId = undefined unless an explicit agentSessionId/definitionId was
passed in; ensure explicit values still override.
In `@packages/host-service/src/trpc/router/terminal-agents/terminal-agents.ts`:
- Around line 12-15: Replace the unsafe type assertions on terminalAgentIdSchema
and agentDefinitionIdSchema with real runtime validation against the allowed ID
set: remove the "as z.ZodType<...>" casts on terminalAgentIdSchema and
agentDefinitionIdSchema and instead construct Zod validators using z.enum(...)
or a z.union of z.literal(...) values built from the shared agent catalog (the
same source used by getOrCreate/findActive/waitForBinding), so incoming
agentId/definitionId are validated at runtime before being passed into
getOrCreate, findActive, or waitForBinding; ensure the new schemas export the
correct TypeScript types (TerminalAgentId/AgentDefinitionId) without casting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e1867be-acd0-4319-9b73-15dcbc5b5dc3
📒 Files selected for processing (22)
apps/desktop/src/renderer/hooks/host-service/useTerminalAgentBindings/index.tsapps/desktop/src/renderer/hooks/host-service/useTerminalAgentBindings/useTerminalAgentBindings.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalPaneIcon/TerminalPaneIcon.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/TerminalSessionDropdown/TerminalSessionDropdown.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/components/V2NotificationController/components/HostNotificationSubscriber/HostNotificationSubscriber.tsxapps/desktop/src/renderer/stores/v2-agent-bindings/index.tsapps/desktop/src/renderer/stores/v2-agent-bindings/store.test.tsapps/desktop/src/renderer/stores/v2-agent-bindings/store.tspackages/host-service/src/app.tspackages/host-service/src/terminal-agents/index.tspackages/host-service/src/terminal-agents/store.test.tspackages/host-service/src/terminal-agents/store.tspackages/host-service/src/terminal-agents/types.tspackages/host-service/src/trpc/router/notifications/notifications.test.tspackages/host-service/src/trpc/router/notifications/notifications.tspackages/host-service/src/trpc/router/router.tspackages/host-service/src/trpc/router/terminal-agents/index.tspackages/host-service/src/trpc/router/terminal-agents/terminal-agents.tspackages/host-service/src/trpc/router/terminal/terminal.tspackages/host-service/src/types.tsplans/20260523-agent-session-tracking.md
💤 Files with no reviewable changes (4)
- apps/desktop/src/renderer/stores/v2-agent-bindings/store.test.ts
- apps/desktop/src/renderer/stores/v2-agent-bindings/index.ts
- apps/desktop/src/renderer/stores/v2-agent-bindings/store.ts
- apps/desktop/src/renderer/routes/_authenticated/components/V2NotificationController/components/HostNotificationSubscriber/HostNotificationSubscriber.tsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
3 issues found across 22 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Plan shipped in this PR — module + tRPC surface + renderer pane wiring.
Favicon source had ~85px of empty padding (16%) on all sides of the 508×508 canvas. Crop the viewBox to the path bbox so droid renders at the same visual weight as the other agent icons.
- terminal-agents.ts: log disposeSessionAndWait failures from the orphaned-pty cleanup path so observability isn't a black hole. - desktop-agent-setup.ts: run bootstrap setup actions (notify-script, etc.) in setupSingleAgent too. Per-agent hooks reference the shared notify script; if boot setup didn't run, per-agent setup needs to be self-sufficient.
Pass focused on removing restatement and stale references while keeping intent comments. Net -29 lines across 6 files. - store.ts: drop stale "(plan decision #3)" reference; collapse class docstring. - terminal-agents.ts: tighten getOrCreate docstring; trim coalesce comment. - host-agent-presets.ts: move HOST_AGENT_PRESETS docstring from above tokenize() to above the export it actually documents. - desktop-agent-setup.ts, settings/index.ts, V2AgentsSettings.tsx: compress 4-line "why" comments to 1-2 lines without losing intent.
This was referenced May 28, 2026
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
1 participant
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.
Summary
terminalAgents— in-process per-terminal agent binding store, populated fromnotifications.hookevents and drained on terminal exit. Exposes a tRPC surface (listByWorkspace,findActive,getOrCreate,onWorkspaceChangeobservable) so non-renderer consumers can reuse live agent sessions instead of always spawning new ones.TerminalPaneIconnow consumes the host-service tracker via a newuseTerminalAgentBindingsReact Query hook (invalidated onagent:lifecycle/terminal:lifecycle). The renderer-sideuseV2AgentBindingStore(Zustand) is deleted — host store is the single source of truth.Design plan:
plans/20260523-agent-session-tracking.md.Test plan
notifications.hookregression — now also records onto the storebun run typecheck(28/28)bun run lintclean/exitand confirm it reverts to the generic terminal glyphSummary by cubic
Adds a
host-serviceterminalAgentstracker with tRPC APIs, rewires the terminal pane icon to read from it, and promotesdroidto a first‑class builtin with icons and derived presets; also reruns per‑agent setup on Add. Crops thedroidicon and tightens setup/cleanup reliability.New Features
terminalAgentsstore tracksterminalId → agentper workspace; filled vianotifications.hook, cleared on terminal exit; emits workspacechange.listByWorkspace,findActive,getOrCreate(coalesces concurrent calls; 10s timeout disposes orphaned terminal),onWorkspaceChangeobservable.useTerminalAgentBindings(@tanstack/react-query) mirrors the host;TerminalPaneIconnow takesworkspaceIdand shows the agent logo from the host tracker.droidadded as a builtin with icons;HOST_AGENT_PRESETSnow derived fromBUILTIN_TERMINAL_AGENTS; all presets seed by default.settings.setupAgent(electron) andsetupSingleAgent; Add flow reruns per‑agent setup.Refactors
useV2AgentBindingStore(Zustand) andHostNotificationSubscribermutations; host store is the single source of truth.agentSessionId/definitionIdacross agent swaps; Zod validates agent ids viaBUILTIN_AGENT_IDS.terminal.disposeSessionalso callsterminalAgentStore.markTerminalExited.@superset/ui. Croppeddroidicon viewBox to match visual weight.getOrCreatenow logs orphaned‑terminal dispose failures;setupSingleAgentruns bootstrap actions before per‑agent hooks.Written for commit 5f6ecc8. Summary will update on new commits.
Review in cubic
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests