feat: superset chat launch config + icon#1983
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an agent-session orchestration layer (launchAgentSession, queueAgentSessionLaunch, adapter selection) with typed Zod agent-launch schemas, chat/terminal adapters, idempotent launch handling, telemetry, and UI/store wiring to route agent launches through the orchestrator. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI / Tool
participant Orch as Agent Session Orchestrator
participant Adapter as Adapter (Chat/Terminal)
participant Tabs as Tabs Store
participant Device as Terminal / Host
UI->>Orch: launchAgentSession(request, context)
Orch->>Orch: select adapter & check idempotency
Orch->>Adapter: invoke adapter(request, context)
alt Terminal
Adapter->>Tabs: create terminal tab/pane
Adapter->>Device: launch command in pane
Device-->>Adapter: command result / error
else Chat
Adapter->>Tabs: create or reuse chat pane/tab
Adapter->>Tabs: setChatMastraLaunchConfig(paneId, launchConfig)
Tabs-->>Adapter: ack
end
Adapter-->>Orch: LaunchResultPayload
Orch->>Orch: capture telemetry & update caches
Orch-->>UI: AgentLaunchResult (status, tabId, paneId, sessionId)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
![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: 6
🧹 Nitpick comments (1)
packages/shared/src/agent-launch.ts (1)
92-92:chatLaunchConfigSchema.partial()is redundant here.All fields in
chatLaunchConfigSchemaare already optional, so.partial()adds noise without changing behavior.♻️ Suggested cleanup
- chatLaunchConfig: chatLaunchConfigSchema.partial().optional(), + chatLaunchConfig: chatLaunchConfigSchema.optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/shared/src/agent-launch.ts` at line 92, The chatLaunchConfig field is using chatLaunchConfigSchema.partial().optional(), but chatLaunchConfigSchema already has all optional fields so .partial() is redundant; remove the .partial() call and use chatLaunchConfigSchema.optional() for the chatLaunchConfig property (referencing the chatLaunchConfig field and chatLaunchConfigSchema symbol).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts`:
- Around line 77-79: The code currently only calls
tabs.setChatLaunchConfig(paneId, launchConfig) when launchConfig is truthy,
leaving previous config intact when launchConfig is null/undefined; change the
logic to always call tabs.setChatLaunchConfig(paneId, launchConfig) (even when
launchConfig is null) so the pane’s stored launch config is explicitly cleared
when no new config is provided, ensuring no stale prompts are reused; update the
block around paneId/launchConfig in chat-adapter.ts to remove the conditional
and always persist the passed launchConfig.
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/agent-session-orchestrator.ts`:
- Around line 17-18: The settledByIdempotency Map is unbounded and will grow
indefinitely; change it to a bounded/evicting cache (or use an LRU cache
library) so entries are removed after a limit or TTL: introduce a
MAX_SETTLED_CACHE_SIZE and/or SETTLED_ENTRY_TTL and on insert into
settledByIdempotency (which stores AgentLaunchResult) evict the oldest entry
when size > MAX or schedule removal after TTL; apply the same eviction/TTL
strategy to the other similar maps (including the one referenced around lines
186-189) and keep inFlightByIdempotency behavior unchanged except to ensure
completed keys move into the bounded settled cache rather than an unbounded Map.
- Around line 43-48: The idempotency key built by buildIdempotencyKey is too
broad and must include the launch kind (and target shape) to avoid collisions
between different launch types; update buildIdempotencyKey to incorporate
request.kind and the relevant target identifier (e.g., request.target or
request.launchTarget/shape field) into the returned string along with
workspaceId and idempotencyKey so keys look like
workspaceId:kind:target:idempotencyKey (use the actual property names present on
AgentLaunchRequest such as kind and launchTarget/target/shape) and guard
null/undefined target values consistently.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`:
- Around line 379-385: The guard allows consumedLaunchConfigRef to permanently
block identical future auto-launches because consumedLaunchConfigRef is only set
after a send; modify the effect and related logic (useEffect that defines
attemptAutoLaunch, and the post-send path where consumedLaunchConfigRef.current
is set) to clear consumedLaunchConfigRef.current (and optionally
autoLaunchInFlightRef.current) when initialLaunchConfig becomes null or is
cleared; specifically ensure the useEffect that reads initialLaunchConfig resets
consumedLaunchConfigRef.current when initialLaunchConfig is falsy and also clear
autoLaunchInFlightRef.current after a completed or cancelled launch so identical
payloads can auto-send again.
- Around line 405-410: When the retry limit check (using
autoLaunchAttemptsRef.current, launchConfigKey and
initialLaunchConfig.retryCount) prevents another attempt, ensure you consume and
clear per-key launch state before returning: remove/delete the entry for
launchConfigKey from autoLaunchAttemptsRef.current and invoke the existing
per-launch cleanup (or delete the launch-config entry from whatever
launch-config store you maintain) so the pane doesn't remain stuck; apply the
same cleanup logic to the analogous early-return at the other occurrence (around
the 468-475 block).
In `@packages/mcp/src/tools/devices/start-agent-session/start-agent-session.ts`:
- Around line 174-178: The code sets the chat model at the top-level key when
building params.chatLaunchConfig (see params.chatLaunchConfig assignment and
request.chat usage in start-agent-session.ts), but the new launch-config
contract expects the model under metadata.model; update the construction so that
instead of adding a root-level model property you add metadata: { ...(existing
metadata?), model: request.chat.model } (only include metadata when
request.chat.model exists) so the selected model survives Superset Chat
launches.
---
Nitpick comments:
In `@packages/shared/src/agent-launch.ts`:
- Line 92: The chatLaunchConfig field is using
chatLaunchConfigSchema.partial().optional(), but chatLaunchConfigSchema already
has all optional fields so .partial() is redundant; remove the .partial() call
and use chatLaunchConfigSchema.optional() for the chatLaunchConfig property
(referencing the chatLaunchConfig field and chatLaunchConfigSchema symbol).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/ui/src/assets/icons/preset-icons/superset.svgis excluded by!**/*.svg
📒 Files selected for processing (30)
apps/desktop/src/lib/trpc/routers/ui-state/index.tsapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/NewWorkspaceCreateFlow/NewWorkspaceCreateFlow.tsxapps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/terminal-adapter.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/agent-session-orchestrator.test.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/agent-session-orchestrator.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/index.tsapps/desktop/src/renderer/lib/agent-session-orchestrator/types.tsapps/desktop/src/renderer/react-query/workspaces/useCreateWorkspace.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsxapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.tsapps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/types.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/hooks/useChatMastraPaneController/useChatMastraPaneController.tsapps/desktop/src/renderer/stores/tabs/store.tsapps/desktop/src/renderer/stores/tabs/types.tsapps/desktop/src/renderer/stores/tabs/utils.test.tsapps/desktop/src/renderer/stores/tabs/utils.tsapps/desktop/src/renderer/stores/workspace-init.tsapps/desktop/src/shared/tabs-types.tspackages/mcp/src/tools/devices/start-agent-session/start-agent-session.tspackages/shared/package.jsonpackages/shared/src/agent-command.tspackages/shared/src/agent-launch.test.tspackages/shared/src/agent-launch.tspackages/shared/src/constants.tspackages/ui/src/assets/icons/preset-icons/index.ts
Comment on lines
+77
to
+79
| if (launchConfig) { | ||
| tabs.setChatLaunchConfig(paneId, launchConfig); | ||
| } |
Contributor
There was a problem hiding this comment.
Always persist launch config (including null) to avoid stale prompts.
When a pane is reused and the new request has no launch config, the current conditional leaves prior launchConfig in place. That can replay stale chat initialization behavior.
💡 Suggested fix
- if (launchConfig) {
- tabs.setChatLaunchConfig(paneId, launchConfig);
- }
+ tabs.setChatLaunchConfig(paneId, launchConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts`
around lines 77 - 79, The code currently only calls
tabs.setChatLaunchConfig(paneId, launchConfig) when launchConfig is truthy,
leaving previous config intact when launchConfig is null/undefined; change the
logic to always call tabs.setChatLaunchConfig(paneId, launchConfig) (even when
launchConfig is null) so the pane’s stored launch config is explicitly cleared
when no new config is provided, ensuring no stale prompts are reused; update the
block around paneId/launchConfig in chat-adapter.ts to remove the conditional
and always persist the passed launchConfig.
Comment on lines
+17
to
+18
| const inFlightByIdempotency = new Map<string, Promise<AgentLaunchResult>>(); | ||
| const settledByIdempotency = new Map<string, AgentLaunchResult>(); |
Contributor
There was a problem hiding this comment.
Settled idempotency cache is unbounded in a long-lived process.
settledByIdempotency is append-only and never evicted, so unique keys accumulate for the app lifetime.
Suggested fix
const inFlightByIdempotency = new Map<string, Promise<AgentLaunchResult>>();
const settledByIdempotency = new Map<string, AgentLaunchResult>();
+const MAX_SETTLED_IDEMPOTENCY_ENTRIES = 500;
@@
if (idempotencyKey) {
inFlightByIdempotency.delete(idempotencyKey);
settledByIdempotency.set(idempotencyKey, result);
+ if (settledByIdempotency.size > MAX_SETTLED_IDEMPOTENCY_ENTRIES) {
+ const oldestKey = settledByIdempotency.keys().next().value;
+ if (oldestKey) settledByIdempotency.delete(oldestKey);
+ }
}Also applies to: 186-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/agent-session-orchestrator.ts`
around lines 17 - 18, The settledByIdempotency Map is unbounded and will grow
indefinitely; change it to a bounded/evicting cache (or use an LRU cache
library) so entries are removed after a limit or TTL: introduce a
MAX_SETTLED_CACHE_SIZE and/or SETTLED_ENTRY_TTL and on insert into
settledByIdempotency (which stores AgentLaunchResult) evict the oldest entry
when size > MAX or schedule removal after TTL; apply the same eviction/TTL
strategy to the other similar maps (including the one referenced around lines
186-189) and keep inFlightByIdempotency behavior unchanged except to ensure
completed keys move into the bounded settled cache rather than an unbounded Map.
Comment on lines
+43
to
+48
| function buildIdempotencyKey(request: AgentLaunchRequest): string | null { | ||
| if (!request.idempotencyKey) { | ||
| return null; | ||
| } | ||
| return `${request.workspaceId}:${request.idempotencyKey}`; | ||
| } |
Contributor
There was a problem hiding this comment.
Idempotency key scope is too broad and can collide across launch kinds.
The key currently ignores request.kind (and target shape), so the same idempotencyKey reused in one workspace can incorrectly dedupe chat vs terminal launches.
Suggested fix
function buildIdempotencyKey(request: AgentLaunchRequest): string | null {
if (!request.idempotencyKey) {
return null;
}
- return `${request.workspaceId}:${request.idempotencyKey}`;
+ return `${request.workspaceId}:${request.kind}:${request.idempotencyKey}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function buildIdempotencyKey(request: AgentLaunchRequest): string | null { | |
| if (!request.idempotencyKey) { | |
| return null; | |
| } | |
| return `${request.workspaceId}:${request.idempotencyKey}`; | |
| } | |
| function buildIdempotencyKey(request: AgentLaunchRequest): string | null { | |
| if (!request.idempotencyKey) { | |
| return null; | |
| } | |
| return `${request.workspaceId}:${request.kind}:${request.idempotencyKey}`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/agent-session-orchestrator.ts`
around lines 43 - 48, The idempotency key built by buildIdempotencyKey is too
broad and must include the launch kind (and target shape) to avoid collisions
between different launch types; update buildIdempotencyKey to incorporate
request.kind and the relevant target identifier (e.g., request.target or
request.launchTarget/shape field) into the returned string along with
workspaceId and idempotencyKey so keys look like
workspaceId:kind:target:idempotencyKey (use the actual property names present on
AgentLaunchRequest such as kind and launchTarget/target/shape) and guard
null/undefined target values consistently.
Comment on lines
+379
to
+385
| useEffect(() => { | ||
| if (!initialLaunchConfig) return; | ||
|
|
||
| const launchConfigKey = getLaunchConfigKey(initialLaunchConfig); | ||
| const attemptAutoLaunch = async (): Promise<void> => { | ||
| if (consumedLaunchConfigRef.current === launchConfigKey) return; | ||
| if (autoLaunchInFlightRef.current === launchConfigKey) return; |
Contributor
There was a problem hiding this comment.
Reset consumed launch key when config is cleared, or identical future launches are dropped.
After successful send, consumedLaunchConfigRef is set (Line 443), but it is never reset when initialLaunchConfig becomes null. A later launch with the same payload hits the guard at Line 384 and never auto-sends.
Suggested fix
useEffect(() => {
- if (!initialLaunchConfig) return;
+ if (!initialLaunchConfig) {
+ consumedLaunchConfigRef.current = null;
+ return;
+ }
const launchConfigKey = getLaunchConfigKey(initialLaunchConfig);Also applies to: 443-447
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`
around lines 379 - 385, The guard allows consumedLaunchConfigRef to permanently
block identical future auto-launches because consumedLaunchConfigRef is only set
after a send; modify the effect and related logic (useEffect that defines
attemptAutoLaunch, and the post-send path where consumedLaunchConfigRef.current
is set) to clear consumedLaunchConfigRef.current (and optionally
autoLaunchInFlightRef.current) when initialLaunchConfig becomes null or is
cleared; specifically ensure the useEffect that reads initialLaunchConfig resets
consumedLaunchConfigRef.current when initialLaunchConfig is falsy and also clear
autoLaunchInFlightRef.current after a completed or cancelled launch so identical
payloads can auto-send again.
Comment on lines
+405
to
+410
| const previousAttempts = | ||
| autoLaunchAttemptsRef.current[launchConfigKey] ?? 0; | ||
| const retryLimit = | ||
| initialLaunchConfig.retryCount ?? AUTO_LAUNCH_MAX_RETRIES; | ||
| if (previousAttempts >= retryLimit) return; | ||
|
|
Contributor
There was a problem hiding this comment.
Exhausted retry path should consume and clean launch-config state.
At Line 409, hitting retry limit returns immediately without consuming launch config or clearing per-key state. This can leave the pane in a stuck state for that key.
Suggested fix
const previousAttempts =
autoLaunchAttemptsRef.current[launchConfigKey] ?? 0;
const retryLimit =
initialLaunchConfig.retryCount ?? AUTO_LAUNCH_MAX_RETRIES;
- if (previousAttempts >= retryLimit) return;
+ if (previousAttempts >= retryLimit) {
+ consumedLaunchConfigRef.current = launchConfigKey;
+ delete autoLaunchAttemptsRef.current[launchConfigKey];
+ delete autoLaunchSessionLockRef.current[launchConfigKey];
+ onConsumeLaunchConfig();
+ return;
+ }Also applies to: 468-475
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraInterface/ChatMastraInterface.tsx`
around lines 405 - 410, When the retry limit check (using
autoLaunchAttemptsRef.current, launchConfigKey and
initialLaunchConfig.retryCount) prevents another attempt, ensure you consume and
clear per-key launch state before returning: remove/delete the entry for
launchConfigKey from autoLaunchAttemptsRef.current and invoke the existing
per-launch cleanup (or delete the launch-config entry from whatever
launch-config store you maintain) so the pane doesn't remain stuck; apply the
same cleanup logic to the analogous early-return at the other occurrence (around
the 468-475 block).
Comment on lines
+174
to
+178
| params.chatLaunchConfig = { | ||
| initialPrompt: request.chat.initialPrompt, | ||
| retryCount: request.chat.retryCount, | ||
| ...(request.chat.model ? { model: request.chat.model } : {}), | ||
| }; |
Contributor
There was a problem hiding this comment.
Map chat model into metadata.model instead of top-level model.
This currently builds chatLaunchConfig with model at the root, but the new launch-config contract in this PR uses metadata.model. As-is, model selection can be silently lost for Superset Chat launches.
💡 Suggested fix
} else {
params.openChatPane = true;
params.chatLaunchConfig = {
initialPrompt: request.chat.initialPrompt,
retryCount: request.chat.retryCount,
- ...(request.chat.model ? { model: request.chat.model } : {}),
+ ...(request.chat.model
+ ? { metadata: { model: request.chat.model } }
+ : {}),
};
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| params.chatLaunchConfig = { | |
| initialPrompt: request.chat.initialPrompt, | |
| retryCount: request.chat.retryCount, | |
| ...(request.chat.model ? { model: request.chat.model } : {}), | |
| }; | |
| params.chatLaunchConfig = { | |
| initialPrompt: request.chat.initialPrompt, | |
| retryCount: request.chat.retryCount, | |
| ...(request.chat.model | |
| ? { metadata: { model: request.chat.model } } | |
| : {}), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mcp/src/tools/devices/start-agent-session/start-agent-session.ts`
around lines 174 - 178, The code sets the chat model at the top-level key when
building params.chatLaunchConfig (see params.chatLaunchConfig assignment and
request.chat usage in start-agent-session.ts), but the new launch-config
contract expects the model under metadata.model; update the construction so that
instead of adding a root-level model property you add metadata: { ...(existing
metadata?), model: request.chat.model } (only include metadata when
request.chat.model exists) so the selected model survives Superset Chat
launches.
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx (2)
73-77: Remove unnecessaryselectableAgentsfrom dependency array.
selectableAgentsis derived from the module-level constantSTARTABLE_AGENT_TYPESon line 49 and will never change between renders. Including it in the dependency array is unnecessary.♻️ Proposed fix
useEffect(() => { - if (selectableAgents.includes(selectedAgent)) return; + if ((STARTABLE_AGENT_TYPES as readonly string[]).includes(selectedAgent)) return; setSelectedAgent("claude"); localStorage.setItem("lastSelectedAgent", "claude"); - }, [selectedAgent, selectableAgents]); + }, [selectedAgent]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx around lines 73 - 77, The useEffect watching selectedAgent currently lists selectableAgents in its dependency array even though selectableAgents is derived from the module-level constant STARTABLE_AGENT_TYPES and will never change; update the effect in OpenInWorkspace by removing selectableAgents from the dependency array so it only depends on selectedAgent, keeping the logic that checks selectableAgents.includes(selectedAgent) and the fallback that calls setSelectedAgent("claude") and localStorage.setItem("lastSelectedAgent", "claude").
135-158: Simplify dead code:buildLaunchRequestalways returns a valid object.
buildLaunchRequestreturns anAgentLaunchRequestin all code paths (either chat or terminal kind), solaunchRequestTemplateis always truthy. The ternary conditionals and theif (!launchRequest)guard are dead code that can be simplified.♻️ Proposed simplification
- const launchRequestTemplate = buildLaunchRequest("pending-workspace"); + const launchRequestTemplate = buildLaunchRequest("pending-workspace"); try { const result = await createWorkspace.mutateAsyncWithPendingSetup( { projectId, name: task.slug, branchName, }, - launchRequestTemplate - ? { agentLaunchRequest: launchRequestTemplate } - : undefined, + { agentLaunchRequest: launchRequestTemplate }, ); - const launchRequest = launchRequestTemplate - ? { - ...launchRequestTemplate, - workspaceId: result.workspace.id, - } - : null; - - if (!launchRequest) { - return; - } + const launchRequest: AgentLaunchRequest = { + ...launchRequestTemplate, + workspaceId: result.workspace.id, + }; + if (result.wasExisting) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx around lines 135 - 158, buildLaunchRequest always returns a valid AgentLaunchRequest, so remove the redundant null/ternary handling: stop treating launchRequestTemplate as possibly falsy and always pass { agentLaunchRequest: launchRequestTemplate } to createWorkspace.mutateAsyncWithPendingSetup, construct launchRequest by spreading launchRequestTemplate and adding workspaceId from result (e.g., const launchRequest = { ...launchRequestTemplate, workspaceId: result.workspace.id }), and eliminate the final if (!launchRequest) early return; update any related types or callers of launchRequest to reflect it is non-null.apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (2)
107-110: Consider simplifying the memoization.
STARTABLE_AGENT_TYPESis a module-level constant that already has a stable reference. TheuseMemowrapper provides no runtime benefit here.♻️ Suggested simplification
- const selectableAgents = useMemo( - () => STARTABLE_AGENT_TYPES as readonly StartableAgentType[], - [], - ); + const selectableAgents = STARTABLE_AGENT_TYPES;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx` around lines 107 - 110, The useMemo around STARTABLE_AGENT_TYPES is unnecessary because STARTABLE_AGENT_TYPES is a stable module-level constant; remove the useMemo and assign selectableAgents directly from STARTABLE_AGENT_TYPES (keeping the same readonly StartableAgentType[] cast) instead of using useMemo—update the declaration of selectableAgents in NewWorkspaceModal.tsx (replace the useMemo expression that references STARTABLE_AGENT_TYPES with a direct assignment).
343-356: Show workspace success confirmation even when agent launch fails.When
launchAgentSessionfails for an existing workspace (wasExisting: true), the early return prevents the success toast from showing. Users won't see confirmation that the workspace was successfully opened, only the agent error.Consider restructuring to show workspace success separately from agent status, or use a combined message like "Opened existing workspace, but agent failed to start."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx` around lines 343 - 356, The current logic aborts showing the workspace success toast when launchAgentSession fails (inside the launchRequest && result.wasExisting block), because you return early on launchResult.status === "failed"; update the flow so the confirmation that the workspace was opened is shown regardless of agent start success: either move the workspace success toast to execute before calling launchAgentSession (or after result.wasExisting handling but before checking launchResult), or remove the early return and only show an error toast for the agent failure (e.g., toast.error with launchResult.error) while still allowing the function to continue; modify the code around launchAgentSession, launchRequest, result.wasExisting and launchResult accordingly so workspace success and agent error are reported separately.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.ts`:
- Around line 47-59: The code currently passes either params.request or a
fallback object into normalizeAgentLaunchRequest, causing partial params.request
objects to skip legacy fields like workspaceId; update the start flow to
shallow-merge the legacy fields into params.request before calling
normalizeAgentLaunchRequest (e.g., build a mergedRequest from params.request
with fallback properties: workspaceId, command, name, paneId, openChatPane,
chatLaunchConfig, idempotencyKey, agentType, source:"command-watcher"), then
call normalizeAgentLaunchRequest(mergedRequest) so required fields are preserved
and normalized; reference normalizeAgentLaunchRequest and the
start-agent-session request construction to locate where to apply the merge.
In `@apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx`:
- Around line 158-160: The code currently forces agent launches into the setup
pane by calling launchAgentViaOrchestrator(setup, setupPaneId) when
agentLaunchRequest or agentCommand are present; change the call so agent
launches triggered by PendingTerminalSetup (i.e., when agentCommand is present
or agentLaunchRequest belongs to setup) do NOT pass setupPaneId as targetPaneId
— call launchAgentViaOrchestrator with no targetPaneId (or explicitly
undefined/null) so the orchestrator creates/uses a separate pane for agent
commands, and apply the same fix where a similar call appears later (the other
occurrence that mirrors this behavior).
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 107-110: The useMemo around STARTABLE_AGENT_TYPES is unnecessary
because STARTABLE_AGENT_TYPES is a stable module-level constant; remove the
useMemo and assign selectableAgents directly from STARTABLE_AGENT_TYPES (keeping
the same readonly StartableAgentType[] cast) instead of using useMemo—update the
declaration of selectableAgents in NewWorkspaceModal.tsx (replace the useMemo
expression that references STARTABLE_AGENT_TYPES with a direct assignment).
- Around line 343-356: The current logic aborts showing the workspace success
toast when launchAgentSession fails (inside the launchRequest &&
result.wasExisting block), because you return early on launchResult.status ===
"failed"; update the flow so the confirmation that the workspace was opened is
shown regardless of agent start success: either move the workspace success toast
to execute before calling launchAgentSession (or after result.wasExisting
handling but before checking launchResult), or remove the early return and only
show an error toast for the agent failure (e.g., toast.error with
launchResult.error) while still allowing the function to continue; modify the
code around launchAgentSession, launchRequest, result.wasExisting and
launchResult accordingly so workspace success and agent error are reported
separately.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx:
- Around line 73-77: The useEffect watching selectedAgent currently lists
selectableAgents in its dependency array even though selectableAgents is derived
from the module-level constant STARTABLE_AGENT_TYPES and will never change;
update the effect in OpenInWorkspace by removing selectableAgents from the
dependency array so it only depends on selectedAgent, keeping the logic that
checks selectableAgents.includes(selectedAgent) and the fallback that calls
setSelectedAgent("claude") and localStorage.setItem("lastSelectedAgent",
"claude").
- Around line 135-158: buildLaunchRequest always returns a valid
AgentLaunchRequest, so remove the redundant null/ternary handling: stop treating
launchRequestTemplate as possibly falsy and always pass { agentLaunchRequest:
launchRequestTemplate } to createWorkspace.mutateAsyncWithPendingSetup,
construct launchRequest by spreading launchRequestTemplate and adding
workspaceId from result (e.g., const launchRequest = { ...launchRequestTemplate,
workspaceId: result.workspace.id }), and eliminate the final if (!launchRequest)
early return; update any related types or callers of launchRequest to reflect it
is non-null.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsxapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.tsapps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx
Comment on lines
+158
to
160
| if (agentLaunchRequest || agentCommand) { | ||
| launchAgentViaOrchestrator(setup, setupPaneId); | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t force agent launch into the setup pane when running setup scripts.
These calls pass the setup pane as targetPaneId, which can make legacy agentCommand run in the same pane as setup commands. That conflicts with the PendingTerminalSetup.agentCommand contract (“run in a separate pane”) and can interleave commands.
💡 Proposed fix
- if (agentLaunchRequest || agentCommand) {
- launchAgentViaOrchestrator(setup, setupPaneId);
- }
+ if (agentLaunchRequest || agentCommand) {
+ launchAgentViaOrchestrator(setup);
+ }
...
- if (agentLaunchRequest || agentCommand) {
- launchAgentViaOrchestrator(setup, paneId);
- }
+ if (agentLaunchRequest || agentCommand) {
+ launchAgentViaOrchestrator(setup);
+ }Also applies to: 208-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx`
around lines 158 - 160, The code currently forces agent launches into the setup
pane by calling launchAgentViaOrchestrator(setup, setupPaneId) when
agentLaunchRequest or agentCommand are present; change the call so agent
launches triggered by PendingTerminalSetup (i.e., when agentCommand is present
or agentLaunchRequest belongs to setup) do NOT pass setupPaneId as targetPaneId
— call launchAgentViaOrchestrator with no targetPaneId (or explicitly
undefined/null) so the orchestrator creates/uses a separate pane for agent
commands, and apply the same fix where a similar call appears later (the other
occurrence that mirrors this behavior).
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 31 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.ts:133">
P1: Validate that normalized request.workspaceId matches params.workspaceId before launching to prevent cross-workspace/project mismatches.</violation>
</file>
<file name="packages/mcp/src/tools/devices/start-agent-session/start-agent-session.ts">
<violation number="1" location="packages/mcp/src/tools/devices/start-agent-session/start-agent-session.ts:51">
P2: Required IDs are no longer enforced as non-empty strings. Add non-empty constraints to preserve prior validation behavior and reject invalid empty identifiers early.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx:156">
P3: Unreachable check for `!launchRequest` since it is always truthy.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx (1)
94-123: Extract duplicated task payload mapping to one local constant.The same task-shape object is built twice. Consolidating it will reduce drift risk and simplify edits.
♻️ Suggested refactor
const buildLaunchRequest = (workspaceId: string): AgentLaunchRequest => { + const taskInput = { + id: task.id, + slug: task.slug, + title: task.title, + description: task.description, + priority: task.priority, + statusName: task.status.name, + labels: task.labels, + }; + if (selectedAgent === "superset-chat") { return { kind: "chat", workspaceId, agentType: "superset-chat", source: "open-in-workspace", chat: { - initialPrompt: buildAgentTaskPrompt({ - id: task.id, - slug: task.slug, - title: task.title, - description: task.description, - priority: task.priority, - statusName: task.status.name, - labels: task.labels, - }), + initialPrompt: buildAgentTaskPrompt(taskInput), retryCount: 1, }, }; } return { kind: "terminal", workspaceId, agentType: selectedAgent, source: "open-in-workspace", terminal: { command: buildAgentCommand({ - task: { - id: task.id, - slug: task.slug, - title: task.title, - description: task.description, - priority: task.priority, - statusName: task.status.name, - labels: task.labels, - }, + task: taskInput, randomId: window.crypto.randomUUID(), agent: selectedAgent, }), name: task.slug, }, }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx around lines 94 - 123, The task payload object used for buildAgentTaskPrompt and buildAgentCommand is duplicated; extract a single local constant (e.g., const taskPayload = { id: task.id, slug: task.slug, title: task.title, description: task.description, priority: task.priority, statusName: task.status.name, labels: task.labels }) and use that constant when calling buildAgentTaskPrompt and when passing task to buildAgentCommand in OpenInWorkspace.tsx so both usages reference the same object shape and avoid drift between buildAgentTaskPrompt and buildAgentCommand inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 350-359: The success toast for existing workspaces is shown even
when the agent launch failed; update the control flow in NewWorkspaceModal.tsx
so that after detecting launchResult.status === "failed" (where you call
toast.error) you abort further processing for that branch—either return early
from the surrounding async handler or guard the later result.wasExisting toast
with a check that launchResult.status !== "failed". Ensure you reference the
existing launchResult.status check and the result.wasExisting toast.success call
so the success toast is only reachable when the launch succeeded.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/`$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsx:
- Around line 94-123: The task payload object used for buildAgentTaskPrompt and
buildAgentCommand is duplicated; extract a single local constant (e.g., const
taskPayload = { id: task.id, slug: task.slug, title: task.title, description:
task.description, priority: task.priority, statusName: task.status.name, labels:
task.labels }) and use that constant when calling buildAgentTaskPrompt and when
passing task to buildAgentCommand in OpenInWorkspace.tsx so both usages
reference the same object shape and avoid drift between buildAgentTaskPrompt and
buildAgentCommand inputs.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsxapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.tspackages/mcp/src/tools/devices/start-agent-session/start-agent-session.ts
Comment on lines
+350
to
+359
| if (launchResult.status === "failed") { | ||
| toast.error("Failed to start agent", { | ||
| description: launchResult.error ?? "Failed to start agent session.", | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (result.isInitializing) { | ||
| if (result.wasExisting) { | ||
| toast.success("Opened existing workspace"); | ||
| } else if (result.isInitializing) { |
Contributor
There was a problem hiding this comment.
Prevent success toast after failed existing-workspace agent launch.
On Line 350, failure only shows an error toast, but execution continues and still emits the success toast on Line 357 for existing workspaces.
🛠️ Suggested fix
if (launchRequest && result.wasExisting) {
const launchResult = await launchAgentSession(launchRequest, {
source: "new-workspace",
createOrAttach: (input) =>
terminalCreateOrAttach.mutateAsync(input),
write: (input) => terminalWrite.mutateAsync(input),
});
if (launchResult.status === "failed") {
toast.error("Failed to start agent", {
description: launchResult.error ?? "Failed to start agent session.",
});
+ return;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (launchResult.status === "failed") { | |
| toast.error("Failed to start agent", { | |
| description: launchResult.error ?? "Failed to start agent session.", | |
| }); | |
| } | |
| } | |
| if (result.isInitializing) { | |
| if (result.wasExisting) { | |
| toast.success("Opened existing workspace"); | |
| } else if (result.isInitializing) { | |
| if (launchResult.status === "failed") { | |
| toast.error("Failed to start agent", { | |
| description: launchResult.error ?? "Failed to start agent session.", | |
| }); | |
| return; | |
| } | |
| } | |
| if (result.wasExisting) { | |
| toast.success("Opened existing workspace"); | |
| } else if (result.isInitializing) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`
around lines 350 - 359, The success toast for existing workspaces is shown even
when the agent launch failed; update the control flow in NewWorkspaceModal.tsx
so that after detecting launchResult.status === "failed" (where you call
toast.error) you abort further processing for that branch—either return early
from the surrounding async handler or guard the later result.wasExisting toast
with a check that launchResult.status !== "failed". Ensure you reference the
existing launchResult.status check and the result.wasExisting toast.success call
so the success toast is only reachable when the launch succeeded.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts (1)
79-81:⚠️ Potential issue | 🟠 MajorAlways persist launch config (including
null) to avoid stale prompts.When a pane is reused and the new request has no launch config, the current conditional leaves prior
launchConfigin place. This can replay stale chat initialization behavior.Proposed fix
- if (launchConfig) { - tabs.setChatLaunchConfig(paneId, launchConfig); - } + tabs.setChatLaunchConfig(paneId, launchConfig);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts` around lines 79 - 81, The conditional around setting the launch config causes stale configs to persist when a new request has no launchConfig; instead always persist the value (including null) for the pane. Remove the if-check and unconditionally call tabs.setChatLaunchConfig(paneId, launchConfig) (or add an explicit else that calls it with null) so that the stored launch config for the pane is always updated when handling new requests.apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
349-354:⚠️ Potential issue | 🟡 MinorPrevent success toast after failed agent launch.
On Line 349, failure only shows an error toast, but execution continues and still emits the success toast on Line 356-357 for existing workspaces.
Proposed fix
if (launchResult.status === "failed") { toast.error("Failed to start agent", { description: launchResult.error ?? "Failed to start agent session.", }); + return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx` around lines 349 - 354, The code shows an error toast when launchResult.status === "failed" but execution continues and later still calls toast.success for the workspace; modify the control flow in NewWorkspaceModal (the handler that creates/launches the workspace where launchResult is used) to stop further execution when launchResult.status === "failed" — e.g., return early or wrap the success-toast/continuation logic in an if-check for launchResult.status !== "failed" — so that toast.success (and any subsequent success-only actions) are only run when the launch succeeded.apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx (1)
154-156:⚠️ Potential issue | 🟠 MajorDon't force agent launch into the setup pane when running setup scripts.
These calls pass the setup pane as
targetPaneId, which makes the agent command run in the same pane as setup commands. This can interleave commands and conflicts with the intended behavior of running agents in a separate pane.Proposed fix - omit targetPaneId when setup scripts are present
if (hasSetupScript && hasPresets) { // ... tab creation ... if (agentLaunchRequest || agentCommand) { - launchAgentViaOrchestrator(setup, setupPaneId); + launchAgentViaOrchestrator(setup); } // ... } if (hasSetupScript) { // ... tab creation ... if (agentLaunchRequest || agentCommand) { - launchAgentViaOrchestrator(setup, paneId); + launchAgentViaOrchestrator(setup); } // ... }Also applies to: 204-206
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx` around lines 154 - 156, The agent launch calls currently force the agent into the setup pane by passing setupPaneId to launchAgentViaOrchestrator; change the two call sites (the call guarded by agentLaunchRequest || agentCommand and the similar call around lines 204-206) to omit the targetPaneId when setup scripts are present—i.e., if setup is truthy call launchAgentViaOrchestrator(setup) (no setupPaneId) so the agent opens in a separate pane, otherwise keep passing setupPaneId as before; update the conditional logic around agentLaunchRequest/agentCommand to choose the correct argument form.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts (1)
49-58: Note:launchConfigis set twice for newly created panes in existing tabs.When
targetPane.type !== "chat-mastra", the config is passed toaddChatPane(line 53-55) and then potentially set again at line 79-81. This is harmless but redundant.Consider skipping the second set for newly created panes
+ let needsConfigSet = false; + const targetPaneId = request.chat.paneId; if (targetPaneId) { // ... existing validation ... if (targetPane.type === "chat-mastra") { tabId = tab.id; paneId = targetPane.id; + needsConfigSet = true; } else { const nextPaneId = tabs.addChatPane(tab.id, { launchConfig, }); tabId = tab.id; paneId = nextPaneId; } } else { const created = tabs.addChatTab(request.workspaceId, { launchConfig, }); tabId = created.tabId; paneId = created.paneId; } // ... session handling ... - if (launchConfig) { - tabs.setChatLaunchConfig(paneId, launchConfig); - } + if (needsConfigSet) { + tabs.setChatLaunchConfig(paneId, launchConfig); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts` around lines 49 - 58, The code is passing launchConfig into tabs.addChatPane and then redundantly setting it again for the same pane; change the flow so that when you create a new pane via tabs.addChatPane (the path that sets nextPaneId), you do not call the subsequent set operation again. Concretely, track whether the pane was newly created (e.g., isNewPane when you call tabs.addChatPane and assign paneId) and only call tabs.setPaneLaunchConfig (or whichever method at the later block sets launchConfig) when isNewPane is false (i.e., for existing panes like targetPane.type === "chat-mastra"), thus avoiding the duplicate set for newly created panes while keeping existing-pane behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 349-354: The code shows an error toast when launchResult.status
=== "failed" but execution continues and later still calls toast.success for the
workspace; modify the control flow in NewWorkspaceModal (the handler that
creates/launches the workspace where launchResult is used) to stop further
execution when launchResult.status === "failed" — e.g., return early or wrap the
success-toast/continuation logic in an if-check for launchResult.status !==
"failed" — so that toast.success (and any subsequent success-only actions) are
only run when the launch succeeded.
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts`:
- Around line 79-81: The conditional around setting the launch config causes
stale configs to persist when a new request has no launchConfig; instead always
persist the value (including null) for the pane. Remove the if-check and
unconditionally call tabs.setChatLaunchConfig(paneId, launchConfig) (or add an
explicit else that calls it with null) so that the stored launch config for the
pane is always updated when handling new requests.
In `@apps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsx`:
- Around line 154-156: The agent launch calls currently force the agent into the
setup pane by passing setupPaneId to launchAgentViaOrchestrator; change the two
call sites (the call guarded by agentLaunchRequest || agentCommand and the
similar call around lines 204-206) to omit the targetPaneId when setup scripts
are present—i.e., if setup is truthy call launchAgentViaOrchestrator(setup) (no
setupPaneId) so the agent opens in a separate pane, otherwise keep passing
setupPaneId as before; update the conditional logic around
agentLaunchRequest/agentCommand to choose the correct argument form.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts`:
- Around line 49-58: The code is passing launchConfig into tabs.addChatPane and
then redundantly setting it again for the same pane; change the flow so that
when you create a new pane via tabs.addChatPane (the path that sets nextPaneId),
you do not call the subsequent set operation again. Concretely, track whether
the pane was newly created (e.g., isNewPane when you call tabs.addChatPane and
assign paneId) and only call tabs.setPaneLaunchConfig (or whichever method at
the later block sets launchConfig) when isNewPane is false (i.e., for existing
panes like targetPane.type === "chat-mastra"), thus avoiding the duplicate set
for newly created panes while keeping existing-pane behavior unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspace/OpenInWorkspace.tsxapps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/tools/start-agent-session.tsapps/desktop/src/renderer/screens/main/components/WorkspaceInitEffects.tsxapps/desktop/src/renderer/stores/tabs/utils.ts
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\n- persist Superset Chat launch config on chat panes and auto-send from the UI\n- add chat-pane launch support when targeting an existing pane and propagate launch config\n- add Superset preset icon mapping for superset chat\n\n## Testing\n- bun test apps/desktop/src/renderer/stores/tabs/utils.test.ts apps/desktop/src/renderer/lib/agent-session-orchestrator/agent-session-orchestrator.test.ts
Summary by cubic
Adds a unified agent-launch orchestrator and Superset Chat with per-pane chat launchConfig that auto-sends the first message with safe retries. All entry points now use the canonical @superset/shared/agent-launch; Superset icons were added.
New Features
Migration
Written for commit 4107992. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests