Skip to content

feat(agents): launch Superset Chat sessions from desktop / CLI / SDK / MCP#4116

Merged
saddlepaddle merged 5 commits intomainfrom
superset-chat-session-fix
May 6, 2026
Merged

feat(agents): launch Superset Chat sessions from desktop / CLI / SDK / MCP#4116
saddlepaddle merged 5 commits intomainfrom
superset-chat-session-fix

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented May 6, 2026

Summary

Folds the built-in Superset Chat agent into the existing agents sugar so the same one-call surface spawns terminal and chat sessions across desktop, CLI, SDK, and MCP-v2. Before this PR, the v2 agent picker only showed the host's terminal-config rows, and even if you forced superset-chat through, the host's runAgentInWorkspace only knew how to spawn PTYs.

Plumbing

  • runAgentInWorkspace branches on chat-builtin ids: mints a sessionId, registers the cloud chat_sessions row, and fires the first turn through ChatRuntimeManager.startTurn. Returns a kind-tagged result so callers materialize the right pane.
  • ChatRuntimeManager.startTurn boots the runtime and queues the message, but returns as soon as streaming begins (agents.run from CLI/SDK/MCP no longer waits ~30s for the assistant's full reply).
  • getSnapshot cold-start fast path returns an empty skeleton when no runtime is in memory while creation runs in the background — renderer's "Loading conversation..." clears on the first poll instead of perma-spinning on the harness boot.
  • Host's API client now carries the bound organizationId via x-superset-organization-id. The session-exchanged JWT only ships organizationIds[]; without the header protectedProcedure middleware can't resolve activeOrganizationId and any host→cloud call hits "No active organization selected".

Surface updates

  • Desktop v2 picker: new useV2AgentChoices hook overlays builtin chat agents on top of the host's terminal rows. Wired into the new workspace modal, OpenInWorkspaceV2, and RunInWorkspacePopoverV2.
  • appendLaunchesToPaneLayout seeds a kind: "chat" pane for chat results.
  • CLI workspaces create --agent --prompt --attachment and agents run --attachment accept local file paths and upload them to the host's attachment store before launch.
  • agents list includes Superset Chat.
  • SDK + MCP-v2 result types gain the kind discriminator. JSDoc and tool descriptions document superset-chat as a valid agent value.

Test plan

  • bun run lint clean
  • bun run typecheck clean
  • CLI: bun cli agents run --workspace <id> --agent superset-chat --prompt "..." returns { kind: "chat", sessionId, label } end-to-end
  • Cloud chat_sessions row inserted with correct v2WorkspaceId and organizationId
  • Mastra thread + user/assistant messages persisted on disk
  • Desktop v2 modal: pick Superset Chat, submit, workspace opens with a chat pane attached to the running session
  • Renderer click-into-session for an evicted runtime: spinner clears immediately, history loads on the next poll (5-10s instead of hanging)
  • Regression: pick Claude (terminal) — terminal pane spawns as before, no chat pane, kind: "terminal" on the result
  • CLI agents list --local shows Superset Chat alongside terminal rows
  • MCP-v2: mcp__superset-v2__workspaces_create with agents: [{ agent: "superset-chat", prompt: ... }] (not exercised in this session)

Summary by cubic

Launch Superset sessions via the agents surface across desktop, CLI, SDK, and MCP, returning a kind so UIs open the right pane. Headless launches return faster, and host→cloud requests include the org header.

  • New Features

    • Launch superset from desktop v2 picker (useV2AgentChoices), CLI (agents run, workspaces create --agent --prompt --attachment), SDK, and MCP; agents list now includes Superset.
    • CLI can upload local files via --attachment; outputs include kind: "chat" | "terminal".
    • appendLaunchesToPaneLayout seeds a kind: "chat" pane; chat tab title is “Superset”.
  • Refactors

    • Standardize on superset id and “Superset” label (drop superset-chat).
    • Host runAgentInWorkspace returns { kind, sessionId, label } and void‑fires chat.sendMessage for Superset; terminal path unchanged.
    • API client sets x-superset-organization-id; SDK/MCP result types include kind, and docs now list superset in examples.

Written for commit b909aa3. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Added built-in "Superset" chat agent alongside terminal agents and unified agent listing.
    • CLI: upload local attachments and new options to attach agents, prompts, and files when creating workspaces or running agents.
  • Improvements

    • Agent selection and initialization simplified with a consolidated agent hook; UI preview texts updated to "Superset".
    • API/SDK now return distinct terminal vs. chat results to better represent session types.

…/ MCP

Folds the built-in Superset Chat agent into the existing `agents` sugar
so the same one-call surface spawns terminal *and* chat sessions across
every entry point. Picker today shows only the host's terminal-config
rows; submitting `superset-chat` 404s on the host because
`runAgentInWorkspace` only knows how to spawn PTYs.

Plumbing changes:

- `agents.ts runAgentInWorkspace` branches on chat-builtin ids: mints a
  sessionId, registers the cloud `chat_sessions` row, and fires the
  first turn through `ChatRuntimeManager.startTurn`. Returns a
  `kind`-tagged result so callers materialize the right pane.
- `ChatRuntimeManager.startTurn` boots the runtime and queues the
  message, but returns as soon as streaming begins instead of awaiting
  the assistant's full reply (~30s -> ~5-8s for headless callers).
- `getSnapshot` cold-start fast path: returns an empty skeleton
  immediately when no runtime is in memory while creation runs in the
  background. Renderer's "Loading conversation..." clears on the first
  poll instead of hanging on the harness boot.
- Host's API client carries the bound organizationId via
  x-superset-organization-id. The session-exchanged JWT only ships
  organizationIds[]; without the header protectedProcedure middleware
  can't resolve activeOrganizationId and any host->cloud call hits
  "No active organization selected".

Surface updates:

- Desktop v2 picker: new useV2AgentChoices hook overlays builtin chat
  agents on top of the host's terminal rows. Wired into the new
  workspace modal and the two task launch pickers.
- appendLaunchesToPaneLayout seeds a kind: "chat" pane for chat
  results.
- CLI workspaces create --agent --prompt --attachment and agents run
  --attachment accept local file paths and upload them to the host's
  attachment store before launch.
- agents list includes Superset Chat.
- SDK + MCP-v2 result types gain the kind discriminator. JSDoc and
  tool descriptions document superset-chat as a valid agent value.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 57e5e4d4-21f3-4769-b517-011d8e3b1b51

📥 Commits

Reviewing files that changed from the base of the PR and between f632c25 and b909aa3.

📒 Files selected for processing (23)
  • apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts
  • apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts
  • apps/desktop/src/renderer/routes/_authenticated/settings/agents/components/AgentsSettings/components/AgentCard/agent-card.utils.ts
  • packages/cli/src/commands/agents/list/command.ts
  • packages/cli/src/commands/agents/run/command.ts
  • packages/cli/src/commands/workspaces/create/command.ts
  • packages/cli/src/lib/upload-attachments.ts
  • packages/host-service/src/trpc/router/agents/agents.ts
  • packages/host-service/src/trpc/router/settings/agent-configs.test.ts
  • packages/mcp-v2/src/tools/agents/run.ts
  • packages/mcp-v2/src/tools/workspaces/create.ts
  • packages/mcp/src/tools/devices/start-agent-session/shared.ts
  • packages/mcp/src/tools/devices/start-agent-session/start-agent-session-with-prompt.ts
  • packages/mcp/src/tools/devices/start-agent-session/start-agent-session.ts
  • packages/sdk/src/resources/agents.ts
  • packages/sdk/src/resources/workspaces.ts
  • packages/shared/src/agent-catalog.ts
  • packages/shared/src/agent-launch-request.test.ts
  • packages/shared/src/agent-launch.test.ts
  • packages/shared/src/agent-launch.ts
  • packages/shared/src/agent-settings.test.ts
  • packages/shared/src/host-agent-presets.ts
  • packages/ui/src/assets/icons/preset-icons/index.ts

📝 Walkthrough

Walkthrough

This PR standardizes the built-in Superset chat preset id/label to superset, adds a new useV2AgentChoices hook that appends a built-in Superset agent to host agent lists, introduces discriminated union agent run results (terminal vs chat), adds CLI attachment upload support, and wires these changes across desktop, host-service, SDK, MCP, and workspace pane orchestration.

Changes

Superset Agent + Attachment & Result Shape Integration

Layer / File(s) Summary
Data Shape & Types
packages/sdk/src/resources/agents.ts, packages/sdk/src/resources/workspaces.ts, packages/shared/src/agent-catalog.ts
AgentRunResult becomes a discriminated union with `kind: "terminal"
Core Host-Service Execution
packages/host-service/src/trpc/router/agents/agents.ts, packages/shared/src/agent-launch.ts
Added runChatAgent, resolveAttachmentsAsFiles; runAgentInWorkspace now accepts HostServiceContext and returns discriminated AgentRunResult. Legacy mapping normalized to superset. New constants SUPERSET_AGENT_ID/SUPERSET_AGENT_LABEL.
CLI: commands & upload utility
packages/cli/src/commands/agents/list/command.ts, packages/cli/src/commands/agents/run/command.ts, packages/cli/src/commands/workspaces/create/command.ts, packages/cli/src/lib/upload-attachments.ts, packages/cli/package.json
Agent list merges host terminal configs with built-in Superset entry. agents run and workspaces create add/validate agent, prompt, and variadic attachment options; local files are uploaded via uploadAttachments. Added mime-types dependency and types.
Desktop: agent selection & UI wiring
apps/desktop/src/renderer/hooks/useV2AgentChoices/*, apps/desktop/src/renderer/components/AgentSelect/index.ts, apps/desktop/src/renderer/routes/.../OpenInWorkspaceV2.tsx, .../RunInWorkspacePopoverV2.tsx, .../PromptGroup.tsx
New useV2AgentChoices(hostUrl) returns { agents: AgentSelectAgent[]; isFetched }, merging host configs and built-in Superset. AgentSelect index export now exposes type AgentSelectAgent. Desktop components replaced useV2AgentConfigs with the new hook and use v2Agents/v2AgentsFetched.
Workspace Pane Orchestration
apps/desktop/src/renderer/stores/workspace-creates/appendLaunchesToPaneLayout.ts
Refactored to accept typed AgentLaunchResult[], build PaneLaunch[], use createWorkspaceStore, and create either chat or terminal panes using typed ChatPaneData / TerminalPaneData.
MCP / Tooling
packages/mcp-v2/src/tools/agents/run.ts, packages/mcp-v2/src/tools/workspaces/create.ts, packages/mcp/src/tools/devices/start-agent-session/*
Tool schemas and handlers updated to include superset preset and to consume discriminated agent run results (`kind: "terminal"
Renames, strings, icons, and tests
packages/shared/src/*, packages/ui/src/assets/icons/preset-icons/index.ts, apps/desktop/src/renderer/lib/agent-session-orchestrator/adapters/chat-adapter.ts, test files under packages/shared and packages/host-service
Replaced occurrences of superset-chatsuperset (IDs, labels, docs, tests). Updated tab title and preview strings from "Superset Chat" → "Superset". Removed obsolete preset icon entry. Updated test expectations and seeds to match the new id.

Sequence Diagram

sequenceDiagram
    participant User
    participant Desktop as Desktop App
    participant Hook as useV2AgentChoices
    participant HostAPI as Host API
    participant HostService as Host Service

    rect rgba(200, 150, 255, 0.5)
    Note over User,HostService: Agent Selection Flow
    User->>Desktop: Open agent selector
    Desktop->>Hook: useV2AgentChoices(hostUrl)
    Hook->>HostAPI: Fetch host agent configs
    HostAPI-->>Hook: Terminal agent configs
    Hook->>Hook: Map configs → AgentSelectAgent[]
    Hook->>Hook: Append built-in Superset agent
    Hook-->>Desktop: { agents, isFetched }
    Desktop->>Desktop: Render agent options
    User->>Desktop: Select agent + prompt
    end

    rect rgba(100, 200, 150, 0.5)
    Note over User,HostService: Agent Execution Flow
    User->>Desktop: Submit agent + prompt + attachments
    Desktop->>HostService: runAgentInWorkspace(ctx, { agent, prompt, attachmentIds })
    alt Agent is "superset"
        HostService->>HostService: runChatAgent()
        HostService->>HostService: Create chat session, attach files, send prompt
        HostService-->>Desktop: { kind: "chat", sessionId, label }
    else Terminal agent
        HostService->>HostService: runTerminalAgent()
        HostService-->>Desktop: { kind: "terminal", sessionId, label }
    end
    Desktop->>Desktop: Open appropriate pane (chat or terminal)
    Desktop-->>User: Session launched
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 I hopped through code to make a shift,

Superset named clear—a tidy gift.
Hooks fetch agents, files take flight,
Chats or shells launch into the night.
A rabbit cheers for every merge — delight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(agents): launch Superset Chat sessions from desktop / CLI / SDK / MCP' clearly and specifically describes the main feature being added—enabling Superset Chat session launches across multiple platforms.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering the summary, plumbing details, surface updates, and test plan with clear explanations of changes and their impacts.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch superset-chat-session-fix

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 6, 2026

Greptile Summary

This PR wires Superset Chat sessions into the existing agents launch surface so that desktop, CLI, SDK, and MCP-v2 callers can spawn chat sessions using the same one-call API as terminal agents. It adds a kind discriminator to AgentRunResult, routes superset-chat through ChatRuntimeManager.startTurn instead of a PTY, and fixes a long-standing activeOrganizationId gap by pinning x-superset-organization-id on every host→cloud request.

  • runAgentInWorkspace now branches on chat-builtin ids, registers a cloud chat_sessions row, and fires the first turn via startTurn — a new fire-and-forget path that returns after runtime boot rather than after the assistant finishes streaming.
  • getSnapshot gains a cold-start fast path via peekRuntime that returns an empty skeleton immediately and kicks off background runtime creation, so the renderer clears the spinner after one poll instead of blocking on the 3–8 s harness boot.
  • CLI workspaces create and agents run accept --attachment <path> to upload local files before launch; agents list now includes chat builtins; desktop agent pickers are unified under a new useV2AgentChoices hook.

Confidence Score: 3/5

The new chat launch path works end-to-end under the happy path, but two code paths can leave users stuck without actionable feedback.

The getSnapshot cold-start change drops the previous error-propagation path and replaces it with fire-and-forget that silences creation failures indefinitely — the renderer cannot distinguish a loading runtime from a permanently broken one. Separately, useV2AgentChoices forwards the terminal-config query's isFetched flag unchanged, so selecting superset-chat when hostUrl is null results in a submit button permanently disabled with 'Checking agents…'.

packages/host-service/src/runtime/chat/chat.ts (cold-start error handling) and apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts (isFetched gate logic)

Important Files Changed

Filename Overview
packages/host-service/src/runtime/chat/chat.ts Adds peekRuntime (non-blocking cold-start) and startTurn (fire-and-forget headless launch). The peekRuntime path silently swallows creation errors, leaving the renderer stuck on 'Loading conversation...' with no error surfaced.
apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts New hook merges terminal configs with builtin chat agents. Returns isFetched: query.isFetched which is permanently false when hostUrl is null, incorrectly gating chat builtins on the terminal query.
packages/host-service/src/trpc/router/agents/agents.ts Splits runAgentInWorkspace into chat and terminal paths, adds resolveAttachmentsAsFiles for base64-inlining files into chat messages. Logic is sound; attachment resolution could block on large files.
packages/host-service/src/api/createApiClient/createApiClient.ts Adds organizationId parameter and pins it as x-superset-organization-id header on every host→cloud request, fixing the missing activeOrganizationId for session-JWT callers.
packages/cli/src/lib/upload-attachments.ts New helper uploads local files to the host attachment store. Uses readFileSync which blocks the event loop for large files; otherwise logic is correct.
apps/desktop/src/renderer/stores/workspace-creates/appendLaunchesToPaneLayout.ts Extends pane layout builder to handle kind: 'chat' agent results, creating a ChatPaneData pane instead of a terminal pane. Change is clean and additive.
packages/cli/src/commands/agents/run/command.ts Adds --attachment (local file path) option and updates output message to discriminate between terminal and chat results. Clean changes.
packages/cli/src/commands/workspaces/create/command.ts Adds --agent, --prompt, --attachment options with mutual-dependency validation guards. Logic is thorough and error messages are helpful.
packages/mcp-v2/src/tools/agents/run.ts Updates tool description and result type to include kind discriminator. Documentation changes are accurate and the type update is correct.
packages/sdk/src/resources/agents.ts Converts AgentRunResult from an interface to a discriminated union with kind field. JSDoc is accurate and complete.

Sequence Diagram

sequenceDiagram
    participant C as CLI / SDK / MCP
    participant A as agentsRouter (host)
    participant Chat as ChatRuntimeManager
    participant Cloud as Cloud API

    C->>A: agents.run { agent: superset-chat, prompt }
    A->>Cloud: chat.createSession { sessionId, v2WorkspaceId }
    Cloud-->>A: ok
    A->>Chat: startTurn { sessionId, workspaceId, payload }
    Chat->>Chat: getOrCreateRuntime (3-8s boot)
    Chat-->>Chat: fire sendMessage (fire-and-forget)
    Chat-->>A: returns
    A-->>C: { kind: chat, sessionId, label }
    Note over C: CLI exits / Desktop opens chat pane
    loop Renderer polling
        C->>Chat: getSnapshot { sessionId, workspaceId }
        alt runtime in memory
            Chat-->>C: { displayState, messages }
        else runtime evicted or not yet booted
            Chat->>Chat: peekRuntime kicks off background creation
            Chat-->>C: empty skeleton { isRunning:false, messages:[] }
        end
    end
Loading

Comments Outside Diff (1)

  1. apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts, line 64 (link)

    P1 isFetched gates builtin chat agents on the host query unnecessarily. isFetched reflects query.isFetched — the terminal-config fetch — not whether the chat builtins are ready (they're synchronously available). When hostUrl is null (query disabled), React Query keeps isFetched: false permanently, so callers like OpenInWorkspaceV2 show "Checking agents…" forever even if superset-chat is already in validAgentIds. The returned value should be true as soon as the builtins are known, regardless of the host query state.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts
    Line: 64
    
    Comment:
    `isFetched` gates builtin chat agents on the host query unnecessarily. `isFetched` reflects `query.isFetched` — the terminal-config fetch — not whether the chat builtins are ready (they're synchronously available). When `hostUrl` is `null` (query disabled), React Query keeps `isFetched: false` permanently, so callers like `OpenInWorkspaceV2` show "Checking agents…" forever even if `superset-chat` is already in `validAgentIds`. The returned value should be `true` as soon as the builtins are known, regardless of the host query state.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
packages/host-service/src/runtime/chat/chat.ts:735-744
**Cold-start skeleton silently hides runtime creation errors**

`peekRuntime` kicks off `getOrCreateRuntime` as a fire-and-forget and swallows the rejection entirely. If creation fails (workspace directory deleted, harness binary missing, permissions error), the inflight entry is removed from `runtimeCreations` via `.finally()`, the next `getSnapshot` call starts a fresh background attempt, and the cycle repeats — all while the skeleton returned here permanently sets `errorMessage: null`. The renderer's "Loading conversation…" spinner never clears and no error is surfaced to the user.

Before this PR, `getSnapshot` awaited `getOrCreateRuntime` directly, so creation failures propagated as tRPC errors that the client could catch and display. The new fast path loses that signal. At minimum, the empty skeleton should carry a sentinel flag (e.g., `isLoading: true`) so the renderer can distinguish "loading" from "error", or the retry loop needs a bounded attempt count before surfacing a synthetic error message.

### Issue 2 of 3
apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts:64
`isFetched` gates builtin chat agents on the host query unnecessarily. `isFetched` reflects `query.isFetched` — the terminal-config fetch — not whether the chat builtins are ready (they're synchronously available). When `hostUrl` is `null` (query disabled), React Query keeps `isFetched: false` permanently, so callers like `OpenInWorkspaceV2` show "Checking agents…" forever even if `superset-chat` is already in `validAgentIds`. The returned value should be `true` as soon as the builtins are known, regardless of the host query state.

```suggestion
	// Chat builtins are synchronously available; only block on the host query
	// if terminal agents were actually fetched. This avoids a permanent
	// "Checking agents…" gate when hostUrl is null or the query is disabled.
	const isFetched = !hostUrl || query.isFetched;
	return { agents, isFetched };
```

### Issue 3 of 3
packages/cli/src/lib/upload-attachments.ts:28-30
`readFileSync` loads each attachment entirely into memory before the next upload starts. On large files this blocks the Node/Bun event loop for the duration of the read. Switching to `fs.promises.readFile` avoids the block without changing upload ordering.

```suggestion
		const bytes = await import("node:fs/promises").then((fs) =>
			fs.readFile(path),
		);
		const result = await client.attachments.upload.mutate({
			data: { kind: "base64", data: bytes.toString("base64") },
```

Reviews (1): Last reviewed commit: "feat(agents): launch Superset Chat sessi..." | Re-trigger Greptile

Comment on lines +735 to +744
if (!runtime) {
return {
displayState: {
isRunning: false,
currentMessage: null,
pendingQuestion: null,
errorMessage: null,
} as unknown as ChatDisplayState,
messages: [] as unknown as RuntimeMessages,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cold-start skeleton silently hides runtime creation errors

peekRuntime kicks off getOrCreateRuntime as a fire-and-forget and swallows the rejection entirely. If creation fails (workspace directory deleted, harness binary missing, permissions error), the inflight entry is removed from runtimeCreations via .finally(), the next getSnapshot call starts a fresh background attempt, and the cycle repeats — all while the skeleton returned here permanently sets errorMessage: null. The renderer's "Loading conversation…" spinner never clears and no error is surfaced to the user.

Before this PR, getSnapshot awaited getOrCreateRuntime directly, so creation failures propagated as tRPC errors that the client could catch and display. The new fast path loses that signal. At minimum, the empty skeleton should carry a sentinel flag (e.g., isLoading: true) so the renderer can distinguish "loading" from "error", or the retry loop needs a bounded attempt count before surfacing a synthetic error message.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/runtime/chat/chat.ts
Line: 735-744

Comment:
**Cold-start skeleton silently hides runtime creation errors**

`peekRuntime` kicks off `getOrCreateRuntime` as a fire-and-forget and swallows the rejection entirely. If creation fails (workspace directory deleted, harness binary missing, permissions error), the inflight entry is removed from `runtimeCreations` via `.finally()`, the next `getSnapshot` call starts a fresh background attempt, and the cycle repeats — all while the skeleton returned here permanently sets `errorMessage: null`. The renderer's "Loading conversation…" spinner never clears and no error is surfaced to the user.

Before this PR, `getSnapshot` awaited `getOrCreateRuntime` directly, so creation failures propagated as tRPC errors that the client could catch and display. The new fast path loses that signal. At minimum, the empty skeleton should carry a sentinel flag (e.g., `isLoading: true`) so the renderer can distinguish "loading" from "error", or the retry loop needs a bounded attempt count before surfacing a synthetic error message.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +28 to +30
const bytes = readFileSync(path);
const result = await client.attachments.upload.mutate({
data: { kind: "base64", data: bytes.toString("base64") },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 readFileSync loads each attachment entirely into memory before the next upload starts. On large files this blocks the Node/Bun event loop for the duration of the read. Switching to fs.promises.readFile avoids the block without changing upload ordering.

Suggested change
const bytes = readFileSync(path);
const result = await client.attachments.upload.mutate({
data: { kind: "base64", data: bytes.toString("base64") },
const bytes = await import("node:fs/promises").then((fs) =>
fs.readFile(path),
);
const result = await client.attachments.upload.mutate({
data: { kind: "base64", data: bytes.toString("base64") },
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/lib/upload-attachments.ts
Line: 28-30

Comment:
`readFileSync` loads each attachment entirely into memory before the next upload starts. On large files this blocks the Node/Bun event loop for the duration of the read. Switching to `fs.promises.readFile` avoids the block without changing upload ordering.

```suggestion
		const bytes = await import("node:fs/promises").then((fs) =>
			fs.readFile(path),
		);
		const result = await client.attachments.upload.mutate({
			data: { kind: "base64", data: bytes.toString("base64") },
```

How can I resolve this? If you propose a fix, please make it concise.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/sdk/src/resources/workspaces.ts (1)

149-158: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update attachmentIds docs to cover chat launches too.

This type now advertises "superset-chat", but attachmentIds still says the host always resolves attachments to filesystem paths in the prompt. That is no longer true for chat sessions, so the SDK docs are contradictory.

Suggested doc update
-	/** Host-scoped attachment ids; host resolves to absolute paths in the prompt. */
+	/**
+	 * Host-scoped attachment ids. For terminal agents the host appends
+	 * absolute paths to the prompt; for `"superset-chat"` it inlines the
+	 * file bytes as base64 data URLs on the chat message.
+	 */
 	attachmentIds?: string[];
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/sdk/src/resources/workspaces.ts` around lines 149 - 158, The JSDoc
for WorkspaceAgentLaunch.attachmentIds is incorrect for chat launches; update
the comment on the attachmentIds field in the WorkspaceAgentLaunch interface so
it explains that for host-scoped agents the host resolves IDs to filesystem
paths in the prompt, but for the "superset-chat" agent IDs are treated as chat
attachments (not necessarily filesystem paths) and are resolved differently;
reference the agent field and the special "superset-chat" value in the comment
so consumers understand the distinction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/cli/src/lib/upload-attachments.ts`:
- Around line 28-33: Wrap the file read and upload calls in a try/catch and
rethrow failures as CLIError annotated with the filename/path to provide
actionable CLI output: around the readFileSync(...) and
client.attachments.upload.mutate(...) calls in upload-attachments.ts, catch any
error and throw new CLIError(`Failed to read/upload attachment "${filename}"
(path: ${path}): ${err.message}`, { cause: err }) or similar so the original
error is preserved; ensure you import/use CLIError and reference the
functions/variables bytes, readFileSync, client.attachments.upload.mutate,
filename and path in the error message.

In `@packages/host-service/src/runtime/chat/chat.ts`:
- Around line 582-584: peekRuntime() currently swallows getOrCreateRuntime()
failures (called via void this.getOrCreateRuntime(...).catch(() => {})), causing
getSnapshot() to return a silent empty running state; instead propagate and
record the boot error so callers see an error snapshot. Change the void-catch
pattern in the getOrCreateRuntime invocation(s) (the call sites that use
this.getOrCreateRuntime(sessionId, workspaceId) around the peekRuntime flow and
the similar block at lines ~734-744) to capture the thrown error and set the
runtime entry/state used by peekRuntime/getSnapshot with an errorMessage and
isRunning=false (or otherwise mark it failed) so getSnapshot() returns an
error-containing snapshot rather than an empty success; ensure the same behavior
for both call sites and reference peekRuntime, getOrCreateRuntime, and
getSnapshot when updating the runtime state/metadata.

In `@packages/host-service/src/trpc/router/agents/agents.ts`:
- Around line 208-224: The createSession call
(ctx.api.chat.createSession.mutate) is committed before starting the runtime
turn (ctx.runtime.chat.startTurn), so if startTurn throws you must roll back the
persisted session to avoid orphaned/duplicate chat_sessions rows; wrap the
startTurn invocation in a try/catch and on any error call the session delete
mutation (e.g., ctx.api.chat.deleteSession.mutate or the existing delete API)
with the same sessionId (and workspaceId) before rethrowing the error, ensuring
delete is awaited so the DB row is removed when startup fails.

---

Outside diff comments:
In `@packages/sdk/src/resources/workspaces.ts`:
- Around line 149-158: The JSDoc for WorkspaceAgentLaunch.attachmentIds is
incorrect for chat launches; update the comment on the attachmentIds field in
the WorkspaceAgentLaunch interface so it explains that for host-scoped agents
the host resolves IDs to filesystem paths in the prompt, but for the
"superset-chat" agent IDs are treated as chat attachments (not necessarily
filesystem paths) and are resolved differently; reference the agent field and
the special "superset-chat" value in the comment so consumers understand the
distinction.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5c56d5dd-96bd-4778-9e0f-e638639d1636

📥 Commits

Reviewing files that changed from the base of the PR and between af7a1a2 and f632c25.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (20)
  • apps/desktop/src/renderer/components/AgentSelect/index.ts
  • apps/desktop/src/renderer/hooks/useV2AgentChoices/index.ts
  • apps/desktop/src/renderer/hooks/useV2AgentChoices/useV2AgentChoices.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/$taskId/components/PropertiesSidebar/components/OpenInWorkspaceV2/OpenInWorkspaceV2.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/tasks/components/TasksView/components/TasksTopBar/components/RunInWorkspacePopoverV2/RunInWorkspacePopoverV2.tsx
  • apps/desktop/src/renderer/routes/_authenticated/components/DashboardNewWorkspaceModal/components/DashboardNewWorkspaceForm/PromptGroup/PromptGroup.tsx
  • apps/desktop/src/renderer/stores/workspace-creates/appendLaunchesToPaneLayout.ts
  • packages/cli/package.json
  • packages/cli/src/commands/agents/list/command.ts
  • packages/cli/src/commands/agents/run/command.ts
  • packages/cli/src/commands/workspaces/create/command.ts
  • packages/cli/src/lib/upload-attachments.ts
  • packages/host-service/src/api/createApiClient/createApiClient.ts
  • packages/host-service/src/app.ts
  • packages/host-service/src/runtime/chat/chat.ts
  • packages/host-service/src/trpc/router/agents/agents.ts
  • packages/mcp-v2/src/tools/agents/run.ts
  • packages/mcp-v2/src/tools/workspaces/create.ts
  • packages/sdk/src/resources/agents.ts
  • packages/sdk/src/resources/workspaces.ts

Comment on lines +28 to +33
const bytes = readFileSync(path);
const result = await client.attachments.upload.mutate({
data: { kind: "base64", data: bytes.toString("base64") },
mediaType,
originalFilename: filename,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Wrap read/upload failures in CLIError with file context.

Line 28 and Line 29 can throw raw errors; surfacing them as CLIError keeps CLI output actionable and consistent.

Suggested patch
 		const filename = basename(path);
 		const mediaType = mimeTypes.lookup(filename);
 		if (!mediaType) {
 			throw new CLIError(
 				`Could not determine media type for attachment: ${path}`,
 				"Use a recognizable file extension (e.g. .png, .pdf, .md)",
 			);
 		}
-		const bytes = readFileSync(path);
-		const result = await client.attachments.upload.mutate({
-			data: { kind: "base64", data: bytes.toString("base64") },
-			mediaType,
-			originalFilename: filename,
-		});
-		ids.push(result.attachmentId);
+		try {
+			const bytes = readFileSync(path);
+			const result = await client.attachments.upload.mutate({
+				data: { kind: "base64", data: bytes.toString("base64") },
+				mediaType,
+				originalFilename: filename,
+			});
+			ids.push(result.attachmentId);
+		} catch (error) {
+			throw new CLIError(
+				`Failed to upload attachment: ${path}`,
+				error instanceof Error ? error.message : "Unknown attachment upload error",
+			);
+		}
 	}
📝 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
const bytes = readFileSync(path);
const result = await client.attachments.upload.mutate({
data: { kind: "base64", data: bytes.toString("base64") },
mediaType,
originalFilename: filename,
});
try {
const bytes = readFileSync(path);
const result = await client.attachments.upload.mutate({
data: { kind: "base64", data: bytes.toString("base64") },
mediaType,
originalFilename: filename,
});
ids.push(result.attachmentId);
} catch (error) {
throw new CLIError(
`Failed to upload attachment: ${path}`,
error instanceof Error ? error.message : "Unknown attachment upload error",
);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/src/lib/upload-attachments.ts` around lines 28 - 33, Wrap the
file read and upload calls in a try/catch and rethrow failures as CLIError
annotated with the filename/path to provide actionable CLI output: around the
readFileSync(...) and client.attachments.upload.mutate(...) calls in
upload-attachments.ts, catch any error and throw new CLIError(`Failed to
read/upload attachment "${filename}" (path: ${path}): ${err.message}`, { cause:
err }) or similar so the original error is preserved; ensure you import/use
CLIError and reference the functions/variables bytes, readFileSync,
client.attachments.upload.mutate, filename and path in the error message.

Comment thread packages/host-service/src/runtime/chat/chat.ts Outdated
Comment on lines +208 to +224
await ctx.api.chat.createSession.mutate({
sessionId,
v2WorkspaceId: input.workspaceId,
});

// Fire-and-forget the turn: the caller (CLI/SDK/MCP) gets the sessionId
// back as soon as the runtime is ready, instead of waiting ~30s for the
// assistant stream to finish. The renderer materializes the streaming
// state via `chat.getSnapshot` when a chat pane opens.
await ctx.runtime.chat.startTurn({
sessionId,
workspaceId: input.workspaceId,
payload: {
content: input.prompt,
...(files.length > 0 ? { files } : {}),
},
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Rollback the persisted chat session when startup fails.

chat.createSession is committed before startTurn(). If runtime boot, model selection, or initial-turn setup throws here, the mutation returns an error but leaves a chat_sessions row behind, so retries create duplicate empty chats.

Suggested direction
 	await ctx.api.chat.createSession.mutate({
 		sessionId,
 		v2WorkspaceId: input.workspaceId,
 	});

-	await ctx.runtime.chat.startTurn({
-		sessionId,
-		workspaceId: input.workspaceId,
-		payload: {
-			content: input.prompt,
-			...(files.length > 0 ? { files } : {}),
-		},
-	});
+	try {
+		await ctx.runtime.chat.startTurn({
+			sessionId,
+			workspaceId: input.workspaceId,
+			payload: {
+				content: input.prompt,
+				...(files.length > 0 ? { files } : {}),
+			},
+		});
+	} catch (error) {
+		await ctx.api.chat.deleteSession.mutate({ sessionId });
+		throw error;
+	}

If there is no delete mutation yet, this path should still roll back the row before returning the launch error.

📝 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
await ctx.api.chat.createSession.mutate({
sessionId,
v2WorkspaceId: input.workspaceId,
});
// Fire-and-forget the turn: the caller (CLI/SDK/MCP) gets the sessionId
// back as soon as the runtime is ready, instead of waiting ~30s for the
// assistant stream to finish. The renderer materializes the streaming
// state via `chat.getSnapshot` when a chat pane opens.
await ctx.runtime.chat.startTurn({
sessionId,
workspaceId: input.workspaceId,
payload: {
content: input.prompt,
...(files.length > 0 ? { files } : {}),
},
});
await ctx.api.chat.createSession.mutate({
sessionId,
v2WorkspaceId: input.workspaceId,
});
// Fire-and-forget the turn: the caller (CLI/SDK/MCP) gets the sessionId
// back as soon as the runtime is ready, instead of waiting ~30s for the
// assistant stream to finish. The renderer materializes the streaming
// state via `chat.getSnapshot` when a chat pane opens.
try {
await ctx.runtime.chat.startTurn({
sessionId,
workspaceId: input.workspaceId,
payload: {
content: input.prompt,
...(files.length > 0 ? { files } : {}),
},
});
} catch (error) {
await ctx.api.chat.deleteSession.mutate({ sessionId });
throw error;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/host-service/src/trpc/router/agents/agents.ts` around lines 208 -
224, The createSession call (ctx.api.chat.createSession.mutate) is committed
before starting the runtime turn (ctx.runtime.chat.startTurn), so if startTurn
throws you must roll back the persisted session to avoid orphaned/duplicate
chat_sessions rows; wrap the startTurn invocation in a try/catch and on any
error call the session delete mutation (e.g., ctx.api.chat.deleteSession.mutate
or the existing delete API) with the same sessionId (and workspaceId) before
rethrowing the error, ensuring delete is awaited so the DB row is removed when
startup fails.

…rset

- Drop ChatRuntimeManager.startTurn and the cold-start workarounds
  (peekRuntime, skeleton snapshot). runChatAgent now just void-fires
  chat.sendMessage and returns the sessionId. CLI agents run goes from
  ~18s to ~2s; runtime boot finishes async on the host.
- Rename superset-chat -> superset (id, type literals, tests, mcp v1,
  icon map). Label "Superset Chat" -> "Superset" everywhere it
  appeared (mcp / cli / sdk / agent-card / orchestrator title).
- Trim function-level JSDocs that restated the code.
There's exactly one chat builtin, so iterating
BUILTIN_AGENT_DEFINITIONS to filter for chat-kinds is busywork —
the id and label are known at the call site. Drops the catalog
import + type predicate in CLI agents list, the v2 picker hook,
and runAgentInWorkspace.
Restore the original MCP and SDK descriptions and just add `superset`
to the e.g. lists rather than spelling out the chat-vs-terminal split
in every docstring.
@saddlepaddle saddlepaddle merged commit 7712238 into main May 6, 2026
15 checks passed
saddlepaddle added a commit that referenced this pull request May 6, 2026
…/ MCP (#4116)

* feat(agents): launch Superset Chat sessions from desktop / CLI / SDK / MCP

Folds the built-in Superset Chat agent into the existing `agents` sugar
so the same one-call surface spawns terminal *and* chat sessions across
every entry point. Picker today shows only the host's terminal-config
rows; submitting `superset-chat` 404s on the host because
`runAgentInWorkspace` only knows how to spawn PTYs.

Plumbing changes:

- `agents.ts runAgentInWorkspace` branches on chat-builtin ids: mints a
  sessionId, registers the cloud `chat_sessions` row, and fires the
  first turn through `ChatRuntimeManager.startTurn`. Returns a
  `kind`-tagged result so callers materialize the right pane.
- `ChatRuntimeManager.startTurn` boots the runtime and queues the
  message, but returns as soon as streaming begins instead of awaiting
  the assistant's full reply (~30s -> ~5-8s for headless callers).
- `getSnapshot` cold-start fast path: returns an empty skeleton
  immediately when no runtime is in memory while creation runs in the
  background. Renderer's "Loading conversation..." clears on the first
  poll instead of hanging on the harness boot.
- Host's API client carries the bound organizationId via
  x-superset-organization-id. The session-exchanged JWT only ships
  organizationIds[]; without the header protectedProcedure middleware
  can't resolve activeOrganizationId and any host->cloud call hits
  "No active organization selected".

Surface updates:

- Desktop v2 picker: new useV2AgentChoices hook overlays builtin chat
  agents on top of the host's terminal rows. Wired into the new
  workspace modal and the two task launch pickers.
- appendLaunchesToPaneLayout seeds a kind: "chat" pane for chat
  results.
- CLI workspaces create --agent --prompt --attachment and agents run
  --attachment accept local file paths and upload them to the host's
  attachment store before launch.
- agents list includes Superset Chat.
- SDK + MCP-v2 result types gain the kind discriminator. JSDoc and
  tool descriptions document superset-chat as a valid agent value.

* refactor(agents): trim chat launch path, rename superset-chat -> superset

- Drop ChatRuntimeManager.startTurn and the cold-start workarounds
  (peekRuntime, skeleton snapshot). runChatAgent now just void-fires
  chat.sendMessage and returns the sessionId. CLI agents run goes from
  ~18s to ~2s; runtime boot finishes async on the host.
- Rename superset-chat -> superset (id, type literals, tests, mcp v1,
  icon map). Label "Superset Chat" -> "Superset" everywhere it
  appeared (mcp / cli / sdk / agent-card / orchestrator title).
- Trim function-level JSDocs that restated the code.

* refactor(agents): hardcode 'superset' instead of filtering catalog

There's exactly one chat builtin, so iterating
BUILTIN_AGENT_DEFINITIONS to filter for chat-kinds is busywork —
the id and label are known at the call site. Drops the catalog
import + type predicate in CLI agents list, the v2 picker hook,
and runAgentInWorkspace.

* docs(agents): trim verbose mcp / sdk descriptions

Restore the original MCP and SDK descriptions and just add `superset`
to the e.g. lists rather than spelling out the chat-vs-terminal split
in every docstring.

* docs(agents): trim fire-and-forget comment to just the WHY
@Kitenite Kitenite deleted the superset-chat-session-fix branch May 6, 2026 04:50
@saddlepaddle saddlepaddle mentioned this pull request May 6, 2026
3 tasks
saddlepaddle added a commit that referenced this pull request May 6, 2026
Changes since v0.2.9:

- New `superset` chat agent surfaced in `agents list`. (#4116)
- `workspaces create` accepts `--agent`, `--prompt`, and `--attachment`
  to spawn a session at create-time. (#4116)
- `agents run` accepts `--attachment` to upload local files alongside
  a prompt. (#4116)
- `agents run --agent superset` returns immediately once streaming
  begins instead of waiting on the full assistant reply (~18s -> ~2s).
  (#4116)

Push cli-v0.2.10 after this lands to fire the release pipeline.
saddlepaddle added a commit that referenced this pull request May 6, 2026
Changes since v0.2.9:

- New `superset` chat agent surfaced in `agents list`. (#4116)
- `workspaces create` accepts `--agent`, `--prompt`, and `--attachment`
  to spawn a session at create-time. (#4116)
- `agents run` accepts `--attachment` to upload local files alongside
  a prompt. (#4116)
- `agents run --agent superset` returns immediately once streaming
  begins instead of waiting on the full assistant reply (~18s -> ~2s).
  (#4116)

Push cli-v0.2.10 after this lands to fire the release pipeline.
@saddlepaddle saddlepaddle mentioned this pull request May 7, 2026
3 tasks
saddlepaddle added a commit that referenced this pull request May 7, 2026
npm has alpha.6 as the most recent published; alpha.7 was bumped in
71bf008 but never `npm publish`-ed. Skip alpha.7 on the registry
and ship the current repo state as alpha.8.

Changes since alpha.6:

- workspaces.create adopts the canonical host-service shape (#3893)
- automations.list accepts --name filter (#3952)
- automations.prompt split into automations.prompt.get / .set (#3959)
- agents.list (presets demoted to UI-only configuration) (#4097)
- agents.run / workspaces.create gain `superset-chat` agent + `kind`
  discriminator on launch results (terminal vs chat) (#4116)
- type adjustments for v2 workspace render path (#4141)
- redact x-api-key in debug-log header dumps (#3956, was alpha.7)

After merge: `cd packages/sdk && bun run build && cd dist && npm publish --access public`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant