feat(desktop): add 3-color workspace status indicators#588
Conversation
📝 WalkthroughWalkthroughReplaces per-pane boolean attention with a typed PaneStatus system, renames AGENT_COMPLETE → AGENT_LIFECYCLE across notification plumbing, adds shared notification types, templates for notify/opencode scripts, introduces status aggregation/utilities, and propagates status-driven UI, store, terminal env, and hook-server changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Hook as Hook Script (terminal)
participant Server as Hook Server (/hook/complete)
participant Emitter as notificationsEmitter
participant Renderer as Renderer (useAgentHookListener / store)
participant UI as Workspace UI (StatusIndicator)
Note over Hook,Server: Local hook posts normalized lifecycle payload
Hook->>Server: POST /hook/complete { eventType, paneId?, tabId?, workspaceId?, env, version }
Server->>Emitter: emit next { type: "agent-lifecycle", data: AgentLifecycleEvent }
Emitter->>Renderer: deliver AgentLifecycleEvent
Renderer->>Renderer: mapEventType() -> ("Start"|"Stop"|"PermissionRequest") or null
Renderer->>Renderer: resolvePaneId(...) -> paneId?
Renderer->>Renderer: determine status (working/permission/review/idle)
Renderer->>Renderer: setPaneStatus(paneId, status)
Renderer->>UI: UI re-renders affected panes, aggregate via getHighestPriorityStatus/pickHigherStatus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
andreasasprou
force-pushed
the
working-indicator
branch
from
391f555 to
cf56018
Compare
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
andreasasprou
force-pushed
the
working-indicator
branch
from
cf56018 to
2b7b66d
Compare
andreasasprou
changed the base branch from
workspace-sidebar
to
persistent-terminals
andreasasprou
force-pushed
the
persistent-terminals
branch
3 times, most recently
from
9788ed3 to
bcc8317
Compare
andreasasprou
force-pushed
the
working-indicator
branch
from
f2b3230 to
4290fee
Compare
When user presses ESC (Claude Code "stop generating") or Ctrl+C while an agent is working, clear the status immediately. This handles the case where the agent is interrupted but the terminal is still running, so the terminal exit handler doesn't trigger. The oracle recommended this approach as the most robust solution since ESC/Ctrl+C is an explicit user intent to stop the agent.
OpenCode doesn't emit a separate 'session.busy' event - the busy state is communicated via 'session.status' with status.type === "busy". The previous fix incorrectly assumed separate events existed. Now we: - Listen to session.status for both busy and idle transitions - Keep session.idle as a deprecated backup for idle detection - Bump plugin version to v6
Bumped to v7. Added console.log statements to understand what events OpenCode emits when transitioning between busy and idle states. Will be removed once the issue is diagnosed.
… flow Adding temporary logs to trace: 1. Server: notification received 2. Renderer: event received by listener 3. Stop handler: isAlreadyActive check result Will help diagnose why green indicator doesn't show after OpenCode completion.
Added shared/debug.ts with debugLog() function that only logs when SUPERSET_DEBUG=1 is set. Replaced hardcoded console.log statements in notification server and agent hook listener with debugLog calls. Enable debug mode: SUPERSET_DEBUG=1 bun run desktop Or add to .env: SUPERSET_DEBUG=1
Implements Oracle's recommendations for robustness: 1. **Safe defaults on error**: isChildSession now returns true (skip) on error instead of false. This prevents race conditions where failed lookups cause child session events to be treated as root events. 2. **Session-scoping**: Tracks the root sessionID when first busy event arrives. Subsequent events from other sessions are ignored, preventing child session events from interfering with root session state. 3. **State deduplication**: Tracks currentState (idle/busy) and stopSent flag. Only sends Start on idle→busy transition, only sends Stop once per busy period. Prevents duplicate/out-of-order notifications. 4. **Caching**: Caches isChildSession lookups to avoid repeated async calls for the same session. 5. **Debug logging**: When SUPERSET_DEBUG=1 is set, logs all events, state transitions, and notification attempts for troubleshooting. These changes address intermittent failures where the green "finished" indicator wouldn't appear after OpenCode completion.
P0: Remove thoughts/ directory and add to .gitignore P1: Memoize workspacePaneIds in WorkspaceListItem - workspacePaneIds now properly memoized with useMemo - Uses direct pane lookup by ID for O(workspacePanes) instead of O(totalPanes) - Prevents sidebar jank with many workspaces/panes P1: Add session.busy fallback for OpenCode version compatibility - Now handles both session.status with nested type AND separate session.busy/session.idle events for different OpenCode versions P2: Move NotificationIds type to shared/ - Prevents renderer importing from main/ (boundary violation) - Re-exports from server.ts for backwards compatibility
When OpenCode starts a new conversation/session within the same process, we need to be able to track it. Previously we stayed locked to the first session's ID forever. Now we reset rootSessionID after sending Stop so the next busy event can establish a new root session.
Extract duplicated priority order (permission > working > review) into shared utilities: - Add STATUS_PRIORITY constant as single source of truth - Add pickHigherStatus() for map aggregation patterns - Add getHighestPriorityStatus() for iterable scanning - Use generators to avoid array allocations Simplifies WorkspaceListItem (22 → 9 lines) and GroupStrip (18 → 9 lines) while making priority order easily testable and future-proof.
andreasasprou
force-pushed
the
working-indicator
branch
from
399a777 to
7c7a7b7
Compare
| * and replacing the notify path placeholder. | ||
| */ | ||
| export function getOpenCodePluginContent(notifyPath: string): string { | ||
| // Build "${" via char codes to avoid JS template literal interpolation in generated code |
There was a problem hiding this comment.
@andreasasprou fyi i moved this into a js file and uses template replace instead. we get the benefit of linting and making the script more maintainable
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In
@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx:
- Around line 228-233: The handler currently reads pane?.status from the render
closure (stale) when deciding to call setPaneStatus(paneId, "idle"); instead,
read the current pane status at event time (e.g., call the state
getter/selectors or store getter for the pane ID such as getPane(paneId) or
usePaneStore.getState().panes[paneId]) and base the check on that fresh value,
or use a functional/state-getter API to update only if the current status is
"working" or "permission" (for example, call a get-current-pane-status(paneId)
and then setPaneStatus(paneId, "idle") only if that returns "working" or
"permission"). Ensure you reference paneId when fetching the live pane and
remove reliance on the closed-over pane variable.
🧹 Nitpick comments (4)
apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh (1)
37-46: Consider adding&for background execution to fully decouple.While the timeouts prevent long blocking, the curl call still waits up to 2 seconds. For truly non-blocking behavior, consider backgrounding the curl:
🔎 Optional: Background the curl call
# Timeouts prevent blocking agent completion if notification server is unresponsive curl -sG "http://127.0.0.1:${SUPERSET_PORT:-{{DEFAULT_PORT}}}/hook/complete" \ --connect-timeout 1 --max-time 2 \ --data-urlencode "paneId=$SUPERSET_PANE_ID" \ --data-urlencode "tabId=$SUPERSET_TAB_ID" \ --data-urlencode "workspaceId=$SUPERSET_WORKSPACE_ID" \ --data-urlencode "eventType=$EVENT_TYPE" \ --data-urlencode "env=$SUPERSET_ENV" \ --data-urlencode "version=$SUPERSET_HOOK_VERSION" \ - > /dev/null 2>&1 + > /dev/null 2>&1 &apps/desktop/src/main/lib/agent-setup/templates/opencode-plugin.template.js (1)
67-90: Consider bounding the childSessionCache size.The
childSessionCacheMap grows indefinitely during long-running sessions. While unlikely to be problematic in practice, consider adding a size limit or LRU eviction for very long sessions with many subagents.🔎 Optional: Add cache size limit
const childSessionCache = new Map(); + const MAX_CACHE_SIZE = 100; const isChildSession = async (sessionID) => { if (!sessionID) return true; // No sessionID = can't verify, skip if (!client?.session?.list) return true; // Can't check, skip // Check cache first if (childSessionCache.has(sessionID)) { return childSessionCache.get(sessionID); } try { const sessions = await client.session.list(); const session = sessions.data?.find((s) => s.id === sessionID); const isChild = !!session?.parentID; + // Prevent unbounded cache growth + if (childSessionCache.size >= MAX_CACHE_SIZE) { + const firstKey = childSessionCache.keys().next().value; + childSessionCache.delete(firstKey); + } childSessionCache.set(sessionID, isChild);apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx (1)
12-12: Consider usingActivePaneStatus | nullfor stricter typing.The prop type is
PaneStatus | null, butGroupStrippassesActivePaneStatus | null(which excludes"idle"). UsingActivePaneStatus | nullhere would:
- More accurately reflect the actual data being passed
- Make the
status !== "idle"check on line 43 unnecessaryThis is a minor type refinement and the current implementation is functionally correct.
🔎 Suggested type refinement
+import type { ActivePaneStatus } from "shared/tabs-types"; -import type { PaneStatus, Tab } from "renderer/stores/tabs/types"; +import type { Tab } from "renderer/stores/tabs/types"; interface GroupItemProps { tab: Tab; isActive: boolean; - status: PaneStatus | null; + status: ActivePaneStatus | null; onSelect: () => void; onClose: () => void; }And simplify line 43:
-{status && status !== "idle" && <StatusIndicator status={status} />} +{status && <StatusIndicator status={status} />}apps/desktop/src/renderer/stores/tabs/store.ts (1)
551-583: Consider extracting duplicate status transition logic.The status transition logic (lines 567-578) is identical to lines 206-217 in
setActiveTab. Consider extracting this to a helper function to reduce duplication and ensure consistent behavior.🔎 Suggested refactor
// Helper at module level or within the store function transitionStatusOnActivation( panes: Record<string, Pane>, paneIds: string[], ): { newPanes: Record<string, Pane>; hasChanges: boolean } { const newPanes = { ...panes }; let hasChanges = false; for (const paneId of paneIds) { const currentStatus = newPanes[paneId]?.status; if (currentStatus === "review") { newPanes[paneId] = { ...newPanes[paneId], status: "idle" }; hasChanges = true; } else if (currentStatus === "permission") { newPanes[paneId] = { ...newPanes[paneId], status: "working" }; hasChanges = true; } } return { newPanes, hasChanges }; }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.gitignoreapps/desktop/docs/EXTERNAL_FILES.mdapps/desktop/src/lib/trpc/routers/notifications.tsapps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/main/lib/agent-setup/notify-hook.tsapps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.shapps/desktop/src/main/lib/agent-setup/templates/opencode-plugin.template.jsapps/desktop/src/main/lib/notifications/server.test.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/main/lib/terminal/env.test.tsapps/desktop/src/main/lib/terminal/env.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/renderer/screens/main/components/StatusIndicator/index.tsapps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceSidebarControl.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.tsapps/desktop/src/shared/constants.tsapps/desktop/src/shared/debug.tsapps/desktop/src/shared/notification-types.tsapps/desktop/src/shared/tabs-types.tsbiome.jsonc
🚧 Files skipped from review as they are similar to previous changes (12)
- apps/desktop/src/main/windows/main.ts
- apps/desktop/src/main/lib/notifications/server.test.ts
- apps/desktop/src/shared/notification-types.ts
- apps/desktop/src/renderer/screens/main/components/StatusIndicator/index.ts
- apps/desktop/src/main/lib/terminal/env.ts
- apps/desktop/src/main/lib/terminal/env.test.ts
- apps/desktop/src/shared/debug.ts
- .gitignore
- apps/desktop/src/lib/trpc/routers/notifications.ts
- apps/desktop/src/lib/trpc/routers/ui-state/index.ts
- apps/desktop/src/renderer/stores/tabs/utils/resolve-notification-target.ts
- apps/desktop/src/renderer/screens/main/components/TopBar/WorkspaceSidebarControl.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/main/lib/agent-setup/notify-hook.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/shared/tabs-types.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/shared/constants.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/main/lib/agent-setup/notify-hook.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/shared/tabs-types.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/shared/constants.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/main/lib/agent-setup/notify-hook.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/shared/tabs-types.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/shared/constants.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/main/lib/agent-setup/notify-hook.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/main/lib/agent-setup/templates/opencode-plugin.template.jsapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/shared/tabs-types.tsapps/desktop/src/main/lib/notifications/server.tsapps/desktop/src/shared/constants.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/main/lib/agent-setup/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
🧠 Learnings (3)
📚 Learning: 2025-11-24T21:32:21.725Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/CLAUDE.md:0-0
Timestamp: 2025-11-24T21:32:21.725Z
Learning: Applies to apps/desktop/**/AGENTS.md : Document agent responsibilities, capabilities, and interaction patterns in AGENTS.md
Applied to files:
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.tsapps/desktop/docs/EXTERNAL_FILES.mdapps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Use Biome for formatting and linting - run at root level with `bun run lint:fix` or `biome check --write`
Applied to files:
biome.jsonc
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/shared/ipc-channels.ts : Define IPC channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Applied to files:
apps/desktop/src/shared/tabs-types.tsapps/desktop/src/main/lib/notifications/server.ts
🧬 Code graph analysis (8)
apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts (2)
apps/desktop/src/shared/constants.ts (1)
NOTIFICATION_EVENTS(42-45)apps/desktop/src/shared/debug.ts (1)
debugLog(26-30)
apps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsx (2)
apps/desktop/src/shared/tabs-types.ts (1)
ActivePaneStatus(23-23)apps/desktop/src/renderer/screens/main/components/StatusIndicator/index.ts (3)
ActivePaneStatus(2-2)StatusIndicator(4-4)getStatusTooltip(3-3)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
apps/desktop/src/shared/tabs-types.ts (1)
PaneStatus(20-20)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (3)
apps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsx (1)
ActivePaneStatus(5-5)apps/desktop/src/shared/tabs-types.ts (2)
ActivePaneStatus(23-23)pickHigherStatus(40-48)apps/desktop/src/renderer/screens/main/components/StatusIndicator/index.ts (1)
ActivePaneStatus(2-2)
apps/desktop/src/shared/tabs-types.ts (3)
apps/desktop/src/renderer/stores/tabs/types.ts (1)
PaneStatus(12-12)apps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsx (1)
ActivePaneStatus(5-5)apps/desktop/src/renderer/screens/main/components/StatusIndicator/index.ts (1)
ActivePaneStatus(2-2)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (2)
apps/desktop/src/main/lib/agent-setup/templates/opencode-plugin.template.js (1)
notifyPath(32-32)apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
getNotifyScriptPath(15-17)
apps/desktop/src/renderer/stores/tabs/store.ts (2)
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
extractPaneIdsFromLayout(54-65)apps/desktop/src/renderer/stores/tabs/types.ts (1)
TabsState(26-28)
apps/desktop/src/main/lib/agent-setup/index.ts (1)
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (1)
cleanupGlobalOpenCodePlugin(191-209)
🪛 LanguageTool
apps/desktop/docs/EXTERNAL_FILES.md
[style] ~85-~85: This phrase is redundant. Consider using “outside”.
Context: ...External Files Before adding new files outside of ~/.superset[-dev]/: 1. **Consider if...
(OUTSIDE_OF)
🪛 Shellcheck (0.11.0)
apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh
[error] 2-2: You need a space after the '{'.
(SC1054)
[error] 2-2: Couldn't parse this brace group. Fix to allow more checks.
(SC1073)
[error] 2-2: You need a space after the '{'.
(SC1054)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Web
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy API
- GitHub Check: Deploy Admin
- GitHub Check: Deploy Docs
- GitHub Check: Build
🔇 Additional comments (42)
biome.jsonc (1)
9-9: LGTM!The template file exclusions are appropriate since
{{MARKER}}and similar placeholders in the template files would cause Biome parsing errors. This aligns with the template-driven approach introduced in this PR.apps/desktop/docs/EXTERNAL_FILES.md (1)
1-99: Well-documented external file management.This documentation clearly explains the environment-specific directories, shell integration, and debugging steps. It will be valuable for preventing dev/prod conflicts and onboarding new contributors.
apps/desktop/src/main/lib/agent-setup/agent-wrappers.ts (4)
15-24: LGTM! Good versioning and template approach.The versioned marker allows detection of stale plugins during cleanup, and the template path approach improves maintainability as noted in prior review feedback.
80-80: LGTM! UserPromptSubmit hook enables "Start" event detection.Adding this hook allows the shell script to detect when a user submits a prompt, enabling the working indicator to activate immediately rather than waiting for agent activity.
191-209: LGTM! Safe cleanup with signature verification.The cleanup function correctly:
- Checks for file existence before reading
- Verifies the signature to avoid deleting user plugins
- Catches and logs errors without crashing
- Uses informative logging
143-148: Verify that thetemplates/directory exists and is properly bundled with the application.The
OPENCODE_PLUGIN_TEMPLATE_PATHresolves topath.join(__dirname, "../../templates/opencode-plugin-template.txt"). In production builds,__dirnamepoints to the directory of the bundled main process. Ensure the template file is either:
- Present in the expected location relative to the bundled output, or
- Configured to be copied as an asset during the build process
apps/desktop/src/main/lib/agent-setup/index.ts (1)
38-39: LGTM! Correct cleanup ordering.Placing the global plugin cleanup after directory creation but before creating new plugins ensures the migration from global to environment-specific paths is clean.
apps/desktop/src/main/lib/agent-setup/notify-hook.ts (1)
9-24: LGTM! Consistent template-based generation.The template approach matches the pattern used in
agent-wrappers.tsfor the OpenCode plugin. The placeholder replacement for{{MARKER}}and{{DEFAULT_PORT}}is straightforward and correct.apps/desktop/src/main/lib/agent-setup/templates/notify-hook.template.sh (2)
1-3: Shellcheck errors are false positives for template placeholders.The Shellcheck errors on line 2 (
SC1054,SC1073,SC1083) are expected because{{MARKER}}is a template placeholder that will be replaced with# Superset agent notification hookat generation time. The resulting script will be valid.
15-24: Robust event type extraction with good fallback handling.The regex patterns handle optional whitespace around JSON colons, and the Codex-specific mapping (
agent-turn-complete→Stop) is correct. The decision to not default to "Stop" on parse failure (lines 26-28) prevents false completion notifications.apps/desktop/src/main/lib/agent-setup/templates/opencode-plugin.template.js (3)
25-28: LGTM! Singleton pattern prevents duplicate registration.Using a global flag (
__supersetOpencodeNotifyPluginV8) ensures the plugin only registers once even if the module is loaded multiple times. The version suffix allows future versions to re-register if needed.
96-143: Well-designed state machine for session lifecycle.The
handleBusyandhandleStopfunctions correctly implement:
- Root session tracking (first busy session becomes root)
- State transitions with deduplication (
idle→busyfor Start,busy→idlefor Stop)- Reset of
rootSessionIDafter Stop to allow tracking new conversationsThe
stopSentflag prevents duplicate Stop notifications within a busy period.
145-186: LGTM! Comprehensive event handling with backward compatibility.The event handler covers:
- Modern
session.statuswithbusy/idlesubtypes- Legacy
session.busy/session.idleevents- Error handling via
session.error- Permission requests via
permission.askChild session filtering at line 151 ensures subagent events don't trigger notifications.
apps/desktop/src/shared/constants.ts (1)
43-43: LGTM!The rename from
AGENT_COMPLETEtoAGENT_LIFECYCLEaccurately reflects the broader lifecycle event model (idle → working → permission → review) introduced in this PR.apps/desktop/src/shared/tabs-types.ts (4)
13-34: Well-designed type system with proper documentation.The
PaneStatushierarchy andSTATUS_PRIORITYconstant provide a clear, type-safe foundation for status aggregation. Good use ofsatisfiesto enforce theRecord<PaneStatus, number>constraint while preserving literal types.
40-48: LGTM!The
pickHigherStatusfunction correctly implements priority comparison with proper handling of undefined values.
54-69: Efficient implementation with early exit optimization.The
getHighestPriorityStatusfunction correctly aggregates statuses with an early exit for maximum priority ("permission"). Returningnullfor all-idle scenarios is semantically appropriate for UI components.
114-114: LGTM!The migration from
needsAttention?: booleantostatus?: PaneStatusprovides richer state modeling. The migration path (version 2) in the store handles the breaking change appropriately.apps/desktop/src/renderer/stores/tabs/types.ts (1)
3-12: LGTM!The type imports and API method updates (
setPaneStatus,clearWorkspaceAttentionStatus) are consistent with the new status-based model and maintain clear, descriptive naming.Also applies to: 82-83
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (3)
318-325: LGTM!Correctly uses
getState()to access current pane status within the effect, avoiding stale closure issues.
387-394: LGTM!The Ctrl+C handler correctly uses
getState()to read and update pane status, clearing transient indicators when the user interrupts agent execution.
395-403: LGTM!The Escape handler correctly uses
getState()to read and update pane status, supporting agent stop operations (e.g., Claude Code "stop generating").apps/desktop/src/renderer/screens/main/components/StatusIndicator/StatusIndicator.tsx (3)
7-30: Excellent use of lookup object pattern.The
STATUS_CONFIGlookup follows the coding guideline to use lookup objects instead of repeated conditionals, and thesatisfiesconstraint ensures type safety while preserving literal types.
43-64: LGTM!The
StatusIndicatorcomponent cleanly implements the three-state visual system (red pulsing, amber pulsing, green static) with proper conditional rendering and accessibility structure.
67-69: LGTM!The
getStatusTooltiphelper provides a clean public API for tooltip integration while maintaining the single source of truth inSTATUS_CONFIG.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx (2)
78-89: LGTM! Clean status aggregation logic.The
tabStatusMapcorrectly aggregates pane statuses per tab using the sharedpickHigherStatusutility. The logic properly filters out idle statuses and only stores active statuses in the map.
140-146: LGTM!The status prop correctly falls back to
nullwhen no aggregated status exists for the tab, matching the expectedPaneStatus | nulltype inGroupItem.apps/desktop/src/renderer/stores/tabs/useAgentHookListener.ts (3)
10-34: Excellent documentation of status mapping and known limitations.The comprehensive JSDoc clearly documents the status transitions and external limitations (Ctrl+C, permission denied, tool failures). This will help maintainers understand the expected behavior and edge cases.
68-73: LGTM!The
StartandPermissionRequestevent handling correctly sets the appropriate statuses, withpermissionproperly overridingworkingas per the priority system.
74-98: LGTM! Well-reasoned Stop event handling.The logic correctly distinguishes between active and inactive panes—setting
idlewhen the user is already watching (avoiding unnecessary indicators) andreviewwhen they need to be notified. The debug logging will be valuable for troubleshooting.apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceListItem/WorkspaceListItem.tsx (3)
115-132: Nice optimization using a generator to avoid array allocation.The generator pattern in
paneStatuses()is a good performance optimization, avoiding intermediate array creation when computing the aggregate status. The memoization dependencies are correct.
134-141: LGTM!The click handler correctly clears workspace attention status when the user selects a workspace, ensuring indicators are acknowledged.
228-239: LGTM! Correct priority ordering for indicators.Status indicators correctly take precedence over unread dots, ensuring users see the most important information first.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupItem.tsx (1)
40-44: LGTM!The StatusIndicator is correctly positioned within the tab button and conditionally rendered based on status.
apps/desktop/src/renderer/stores/tabs/store.ts (4)
202-230: LGTM! Well-designed status clearing on tab activation.The status transition logic is correct:
review→idle: User acknowledged completionpermission→working: Assume permission grantedworkingpersists: Only cleared by Stop eventThe
hasChangesgate prevents unnecessary state updates.
538-549: LGTM!The guard correctly prevents corrupting the panes map when called with an unknown
paneId. This defensive approach protects against race conditions where a pane might be removed between the lifecycle event and status update.
799-828: LGTM! Robust migration and merge logic.The migration correctly converts legacy
needsAttentionto the newstatusfield. The merge logic appropriately resets transient states (working,permission) on startup while preservingreviewso users don't miss completion notifications.
525-536: LGTM! Consistent guard pattern across pane mutations.The guards in
markPaneAsUsed,updatePaneCwd, andclearPaneInitialDataconsistently protect against corrupting the panes map when called with unknown pane IDs.Also applies to: 585-617
apps/desktop/src/main/lib/notifications/server.ts (4)
40-68: LGTM! Well-documented event normalization.The
mapEventTypefunction correctly normalizes various event type strings to canonical lifecycle events. The explicitnullreturn for unknown events ensures forward compatibility, and the documentation clearly explains the design decision to not default missingeventTypeto "Stop".
78-116: LGTM! Robust pane resolution with fallbacks.The resolution logic correctly validates that
paneIdexists in state before returning it, preventing stale references from corrupting the store. The fallback chain (paneId → tabId → workspaceId) provides resilience when direct pane IDs aren't available.
119-181: LGTM! Well-designed non-blocking endpoint.The endpoint handler correctly:
- Logs environment mismatches without blocking the agent
- Handles unknown event types gracefully for forward compatibility
- Includes resolved
paneIdin the response for debugging- Emits to
AGENT_LIFECYCLEwith the normalized event type
10-14: LGTM!Re-exporting types from the shared module maintains backward compatibility for existing consumers while centralizing type definitions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
228-238: Consider extracting the transient status clearing pattern.The pattern of checking and clearing transient statuses (
working|permission→idle) is repeated four times throughout the file. Extracting this into a helper would reduce duplication and make the intent clearer.🔎 Proposed helper function
Add at module level:
/** * Clear transient pane status (working/permission) if currently set. * Review status persists (user needs to acknowledge completed work). */ function clearTransientPaneStatus(paneId: string) { const currentPane = useTabsStore.getState().panes[paneId]; if ( currentPane?.status === "working" || currentPane?.status === "permission" ) { useTabsStore.getState().setPaneStatus(paneId, "idle"); } }Then replace the four duplicated blocks with:
-const currentPane = useTabsStore.getState().panes[paneId]; -if ( - currentPane?.status === "working" || - currentPane?.status === "permission" -) { - setPaneStatus(paneId, "idle"); -} +clearTransientPaneStatus(paneId);Also applies to: 323-330, 392-408
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/tsconfig.json
🧰 Additional context used
📓 Path-based instructions (6)
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use object parameters for functions with 2+ parameters instead of positional arguments
Functions with 2+ parameters should accept a single params object with named properties for self-documentation and extensibility
Use prefixed console logging with context pattern: [domain/operation] message
Extract magic numbers and hardcoded values to named constants at module top
Use lookup objects/maps instead of repeated if (type === ...) conditionals
Avoid usinganytype - maintain type safety in TypeScript code
Never swallow errors silently - at minimum log them with context
Import from concrete files directly when possible - avoid barrel file abuse that creates circular dependencies
Avoid deep nesting (4+ levels) - use early returns, extract functions, and invert conditions
Use named properties in options objects instead of boolean parameters to avoid boolean blindness
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
One component per file - do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Drizzle ORM for all database operations - never use raw SQL
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome for formatting and linting - run at root level with
bun run lint:fixorbiome check --write
Files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
🧠 Learnings (7)
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/tsconfig.json
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/renderer/**/*.{ts,tsx} : Never import Node.js modules (fs, path, os, net) in renderer process or shared code - they are externalized for browser compatibility
Applied to files:
apps/desktop/tsconfig.json
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/*/src/components/{ui,ai-elements,react-flow}/*.tsx : Use kebab-case single files for shadcn/ui components (e.g., button.tsx, base-node.tsx) in src/components/ui/, src/components/ai-elements, and src/components/react-flow/
Applied to files:
apps/desktop/tsconfig.json
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to **/*.{ts,tsx} : Avoid using `any` type - maintain type safety in TypeScript code
Applied to files:
apps/desktop/tsconfig.json
📚 Learning: 2025-12-21T04:39:28.543Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-12-21T04:39:28.543Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/tsconfig.json
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/*.ts : Never import Node.js modules in shared code like electron-router-dom.ts - it runs in both main and renderer processes
Applied to files:
apps/desktop/tsconfig.json
📚 Learning: 2026-01-02T06:50:28.671Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-02T06:50:28.671Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules like node:path or dotenv in electron-router-dom.ts and similar shared files - they run in both main and renderer processes
Applied to files:
apps/desktop/tsconfig.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Deploy Docs
- GitHub Check: Deploy Admin
- GitHub Check: Deploy API
- GitHub Check: Deploy Marketing
- GitHub Check: Deploy Web
- GitHub Check: Build
🔇 Additional comments (2)
apps/desktop/tsconfig.json (1)
17-23: LGTM! Appropriate exclusion of template files.The addition of
"src/**/templates/**"correctly excludes the runtime template assets (.template.js,.template.sh) introduced in this PR from TypeScript compilation. These files are copied/used as-is at runtime and should not be type-checked or compiled.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (1)
228-238: Race condition fix confirmed—correctly uses fresh state.The previous race condition has been properly addressed. Line 232 now reads
useTabsStore.getState().panes[paneId]to get fresh pane status at event time instead of relying on the stale closure. This ensures the status check reflects the current state when the terminal exit event arrives.
2 participants
Summary
UserPromptSubmit,Stop,PermissionRequest) and OpenCode plugin (session.status,permission.ask)Demo Loom
P.s. the bug in the loom is fixed now.
Why / Context
Users managing multiple agent workspaces need visual feedback on agent state without constantly switching tabs. Status indicators surface:
How It Works
Status Flow:
notify.shscript sends HTTP POST to local notification server (port 31416)useAgentHookListenerupdates pane status in Zustand storeStatus Priority: permission (red) > working (amber) > review (green)
UI Locations:
GroupItem+StatusIndicatorWorkspaceListItem+StatusIndicatorClick Behavior:
review→idle(acknowledged)permission→working(assumes permission granted, agent continues)working→ unchanged (persists until Stop event)Manual QA Checklist
Claude Code Integration
OpenCode Integration
session.statuswithstatus.type === "busy"session.statuswithstatus.type === "idle"permission.askparentID)Indicator Colors
App Lifecycle
Dev/Prod Separation
Testing
bun run typecheck✓bun run lint✓bun test- 450/451 pass (1 unrelated pre-existing failure)Debug Mode
Enable debug logging for future QA with:
Logs:
[debug:notifications](server),[debug:agent-hooks](renderer)Design Decisions
UserPromptSubmitfor Start instead ofPreToolUse: Simpler mental model; user submits prompt = agent starts working. Tool-level granularity adds complexity without clear UX benefit.session.idleon completion. Without filtering, users would get notification spam.isUnreadflag is a manual marker (Mark as Unread context menu), independent of agent lifecycle. Blue dot only shows when no agent status is active.Known Limitations
These are external limitations of Claude Code / OpenCode hook systems:
Solved limitations:
User interrupt (Ctrl+C)→ Now detected in terminal, clears status immediatelyESC to stop generation→ Now detected in terminal, clears status immediatelyTerminal exit→ Now clears "working" and "permission" status automaticallyDocumented inline in
useAgentHookListener.ts.Commits
a9cd65d4- Base feature: 3-color workspace status indicators6e4a8d0e- Fix: Use StatusIndicator in workspace sidebar649258f7- Fix: Correct OpenCode event types for status indicators60c36400- Fix: Clear pane status on terminal exit56548784- Fix: Clear status on ESC/Ctrl+C to handle agent interrupts252f7f3d- Fix: Use session.status event for OpenCode busy detection1889ad4f- Feat: Add debug logging utility with SUPERSET_DEBUG flagFollow-ups
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.