feat(docs): Add MCP improvements / improve MCP local command flakiness#1608
Conversation
…parallel agents pages Inspired by Conductor's documentation architecture: - Add workflow.mdx, task-management.mdx, parallel-agents.mdx - Rewrite overview as card-grid welcome/landing page - Reorganize navigation into 5 sections (Get Started, Core Features, Guides, Tips, Help) - Fix ResourceCard to support internal links - Update MCP docs with get_workspace_details and unified start_claude_session
📝 WalkthroughWalkthroughAdds server-persisted command status with retrying TRPC calls, consolidates Claude subagent into a pane-aware start session tool, introduces workspace-introspection tools for desktop and MCP, removes collection onUpdate syncing, and updates documentation, navigation, and a ResourceCard link behavior. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
saddlepaddle
changed the title
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
saddlepaddle
had a problem deploying
to
preview
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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)
apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts (1)
209-222:⚠️ Potential issue | 🟡 MinorFix the unawaited
persistCommandStatuscall in the timeout case.The
agentCommandscollection updates are intentionally optimistic-only for intermediate statuses (claimed, executing), with server persistence happening only through explicitpersistCommandStatus()calls for final states. This design is sound and documented in the code comment.However, there is an inconsistency: the
processCommandhandler awaitspersistCommandStatus()for both "completed" and "failed" states, but the timeout handling at line 190 does not await the call. This creates a reliability gap — if the device crashes or unmounts before the retry logic completes, the timeout persistence may be lost.Timeout case (not awaited)
persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired before execution", });Add
awaitto match the pattern in the handler:await persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired before execution", });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts` around lines 209 - 222, The timeout branch currently calls persistCommandStatus(...) without awaiting it, causing potential loss of the final "timeout" state; update the timeout handling inside the processCommand flow to await persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired before execution" }) so it matches the awaited calls for "completed" and "failed" states and ensures the server persistence completes before proceeding; locate the timeout logic near the processCommand handler that updates the agentCommands collection and change the call to use await persistCommandStatus(...) so persistence is reliable.
🧹 Nitpick comments (7)
packages/mcp/src/tools/devices/switch-workspace/switch-workspace.ts (1)
26-28: Redundantascasts — consider removing them.Zod already infers the correct types from the schema (
string,string | undefined), so the explicitascasts add noise without providing any type safety benefit.♻️ Proposed cleanup
- const deviceId = args.deviceId as string; - const workspaceId = args.workspaceId as string | undefined; - const workspaceName = args.workspaceName as string | undefined; + const { deviceId, workspaceId, workspaceName } = args;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp/src/tools/devices/switch-workspace/switch-workspace.ts` around lines 26 - 28, Redundant explicit type assertions are used when extracting args (deviceId, workspaceId, workspaceName); remove the unnecessary "as string" and "as string | undefined" casts in the switchWorkspace handler so the variables rely on Zod-inferred types, i.e., change assignments that reference args.deviceId, args.workspaceId, and args.workspaceName to plain variable bindings without "as" casts and ensure any downstream usages rely on their inferred types.apps/docs/src/components/ResourceCard/ResourceCard.tsx (1)
30-35: Usenext/link<Link>for internal routes to enable client-side navigation and prefetching.ResourceCard is a Next.js MDX component that currently renders both internal and external links as plain
<a>tags. Internal routes should use Next.js<Link>for client-side navigation, prefetching, and scroll restoration. This pattern is already established elsewhere in the docs app—NavigationBar'sNavLinkhelper (lines 103–125) demonstrates the exact approach: conditionally renderingLinkfor internal hrefs and<a target="_blank">for external links.♻️ Suggested refactor
Add
"use client"directive and import at the top:+"use client"; + import { ArrowUpRight } from "lucide-react"; import { cn } from "@/lib/cn"; +import Link from "next/link";Then swap the link element:
- <a - href={href} - {...(isExternal - ? { target: "_blank", rel: "noopener noreferrer" } - : {})} - > - <h3 className="font-semibold text-md tracking-tight no-underline"> - {title} - </h3> - </a> + {isExternal ? ( + <a href={href} target="_blank" rel="noopener noreferrer"> + <h3 className="font-semibold text-md tracking-tight no-underline"> + {title} + </h3> + </a> + ) : ( + <Link href={href}> + <h3 className="font-semibold text-md tracking-tight no-underline"> + {title} + </h3> + </Link> + )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/src/components/ResourceCard/ResourceCard.tsx` around lines 30 - 35, ResourceCard currently renders all links as raw <a> tags; change it to use Next.js client-side Link for internal routes: add the "use client" directive at the top of ResourceCard and import Link from "next/link", then in the JSX replace the unconditional <a href={href} ...> with a conditional render that uses <Link href={href}>...</Link> when isExternal is false and the existing <a target="_blank" rel="noopener noreferrer"> when isExternal is true (follow the NavLink pattern used elsewhere for reference). Ensure href and isExternal props used by ResourceCard are passed into the Link correctly and that external links keep the security attributes.packages/mcp/src/tools/devices/get-workspace-details/get-workspace-details.ts (1)
11-13: Consider adding.uuid()validation toworkspaceId.
workspaceIdshould be validated as a UUID for consistency with other workspace-modifying tools.update-workspace.tsusesz.string().uuid()anddelete-workspace.tsvalidates workspaceIds as UUIDs. If workspace IDs are always UUIDs in the system, adding this validation would catch malformed IDs earlier.Proposed change
inputSchema: { deviceId: z.string().describe("Target device ID"), - workspaceId: z.string().describe("Workspace ID to get details for"), + workspaceId: z.string().uuid().describe("Workspace ID to get details for"), },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp/src/tools/devices/get-workspace-details/get-workspace-details.ts` around lines 11 - 13, The input schema in get-workspace-details.ts currently defines workspaceId as z.string(); update the inputSchema entry for workspaceId to use z.string().uuid() (matching other handlers like update-workspace.ts and delete-workspace.ts) so workspace IDs are validated as UUIDs; locate the inputSchema object in the getWorkspaceDetails handler (the inputSchema with deviceId and workspaceId) and change the workspaceId validator to .uuid() to enforce consistent ID format.apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts (1)
194-198: Floating promise frompersistCommandStatusinside.filter()callback.
persistCommandStatusreturns aPromise<void>but is called withoutawaitinside the synchronous.filter(). While the function handles its own errors internally, this is still a floating promise. Consider moving timeout handling into its own loop before the filter, or usingvoid persistCommandStatus(...)to signal intentional fire-and-forget.Minimal signal of intent
- persistCommandStatus({ + void persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired before execution", });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts` around lines 194 - 198, In useCommandWatcher, the call to persistCommandStatus inside the .filter() callback creates a floating Promise; either handle timeouts in a separate loop before calling .filter() or explicitly mark the fire-and-forget intent (e.g., use void persistCommandStatus(...)) so the Promise is not silently leaked; update the .filter() callback around persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired before execution" }) to either await the call in an async timeout-handling loop or prefix it with void to signal intentional non-awaiting.apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/persistCommandStatus.ts (1)
21-52: Silent swallowing of persistence failures after retries.After all 4 attempts exhaust, the function logs the error and returns
void— callers have no way to detect that persistence failed. Since local state was already optimistically updated, the server and client will silently diverge. This is acceptable if eventual consistency is guaranteed elsewhere (e.g., a background sync), but worth documenting or surfacing as a metric/alert.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/persistCommandStatus.ts` around lines 21 - 52, The function persistCommandStatus currently swallows failures after retrying; change it so callers can detect failure by re-throwing the final error (lastError) or returning a failure indicator instead of silently returning void — update persistCommandStatus to either throw lastError after the retries or change its return type to a Result/boolean and return false on exhaustion; ensure the thrown/returned value includes the original error from apiClient.agent.updateCommand.mutate and keep the existing console.error/console.warn logging (and optionally emit a metric or call a telemetry helper) so callers like any code invoking persistCommandStatus can react to persistence failure.packages/mcp/src/tools/devices/start-claude-session/start-claude-session.ts (1)
42-54: Type assertions rely on upstream Zod validation.
validateArgsusesas stringcasts without its own runtime type checks. This is safe because the MCP framework validatesinputSchema(Zod) before the handler runs, but it makesvalidateArgsfragile if called from a different context.Alternatively, reuse the Zod schema for validation
-function validateArgs(args: Record<string, unknown>): { - deviceId: string; - taskId: string; - workspaceId: string; - paneId?: string; -} | null { - const deviceId = args.deviceId as string; - const taskId = args.taskId as string; - const workspaceId = args.workspaceId as string; - const paneId = args.paneId as string | undefined; - if (!deviceId || !taskId || !workspaceId) return null; - return { deviceId, taskId, workspaceId, ...(paneId ? { paneId } : {}) }; -} +const argsSchema = z.object({ + deviceId: z.string().min(1), + taskId: z.string().min(1), + workspaceId: z.string().min(1), + paneId: z.string().optional(), +}); + +function validateArgs(args: Record<string, unknown>) { + const result = argsSchema.safeParse(args); + return result.success ? result.data : null; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp/src/tools/devices/start-claude-session/start-claude-session.ts` around lines 42 - 54, validateArgs currently uses "as string" casts without runtime checks, making it fragile if called outside the MCP-validated path; fix it by either (A) performing runtime type checks inside validateArgs (use typeof on args.deviceId, args.taskId, args.workspaceId and allow paneId only if typeof === 'string', return null on any mismatch) or (B) reuse the existing Zod input schema (e.g., inputSchema.safeParse(args) or inputSchema.parse within a try/catch) to validate and extract deviceId, taskId, workspaceId, and optional paneId, then return the typed object or null on failure. Ensure you reference the validateArgs function and the inputSchema symbol so the implementation replaces the unsafe "as string" casts with real validation.apps/docs/content/docs/task-management.mdx (1)
39-45: Clarify that the fenced block contains example agent prompts, not executable code.The unlabeled code fence may read as a generic snippet. A short lead-in sentence makes intent explicit.
✏️ Suggested copy tweak
AI agents can manage tasks programmatically via the [MCP server](/mcp): +For example, you can tell an agent: +"Create a task for fixing the login validation bug"
"List all my high-priority tasks"
"Update the auth task status to done"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/content/docs/task-management.mdx` around lines 39 - 45, Add a one-line lead-in before the fenced block clarifying these are example AI agent prompts (not executable code) so readers understand intent; update the copy immediately above the fence that contains the three sample prompts ("Create a task for fixing the login validation bug", "List all my high-priority tasks", "Update the auth task status to done") to a sentence like “Example agent prompts:” (or similar) to make the snippet explicit.
🤖 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/persistCommandStatus.ts`:
- Around line 8-19: Replace the duplicated tRPC client in
persistCommandStatus.ts by importing and using the existing apiTrpcClient
(instead of creating a new apiClient via createTRPCProxyClient) so all calls
reuse the centralized configuration from renderer/lib/api-trpc-client.ts; also
update the persistence calls made by the functions in persistCommandStatus (the
methods that call the backend to persist command state) to include the local
claimedAt value that useCommandWatcher.ts sets on the command, ensuring
claimedAt is sent with the payload so server state matches the local command
state.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts`:
- Around line 110-116: The local optimistic update sets draft.claimedAt = new
Date() but that timestamp is never sent to the backend; capture that timestamp
into a claimedAt variable when you set draft.claimedAt and include claimedAt in
every persistCommandStatus call (the calls inside useCommandWatcher that pass
id/status/claimedBy/result/executedAt) so the server receives the claim time for
completed, failed and cancelled branches; update the persistCommandStatus
payloads (the calls that reference commandId, deviceInfo?.deviceId,
result/executedAt/etc.) to include this claimedAt value.
In `@apps/docs/content/docs/parallel-agents.mdx`:
- Around line 20-28: Supported Agents table in parallel-agents.mdx lists Gemini
CLI and "Any CLI agent" but agent-integration.mdx only lists Claude Code, Codex,
and OpenCode; update the agent lists to be consistent by either adding Gemini
and a note about generic CLI agents to agent-integration.mdx or removing those
entries from the Supported Agents table in parallel-agents.mdx so both pages
mention the same agents (reference the "Supported Agents" table and the
agent-integration.mdx agent list entries like Claude Code, Codex, OpenCode,
Gemini CLI, and Any CLI agent when making the change).
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts`:
- Around line 209-222: The timeout branch currently calls
persistCommandStatus(...) without awaiting it, causing potential loss of the
final "timeout" state; update the timeout handling inside the processCommand
flow to await persistCommandStatus({ id: cmd.id, status: "timeout", error:
"Command expired before execution" }) so it matches the awaited calls for
"completed" and "failed" states and ensures the server persistence completes
before proceeding; locate the timeout logic near the processCommand handler that
updates the agentCommands collection and change the call to use await
persistCommandStatus(...) so persistence is reliable.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/persistCommandStatus.ts`:
- Around line 21-52: The function persistCommandStatus currently swallows
failures after retrying; change it so callers can detect failure by re-throwing
the final error (lastError) or returning a failure indicator instead of silently
returning void — update persistCommandStatus to either throw lastError after the
retries or change its return type to a Result/boolean and return false on
exhaustion; ensure the thrown/returned value includes the original error from
apiClient.agent.updateCommand.mutate and keep the existing
console.error/console.warn logging (and optionally emit a metric or call a
telemetry helper) so callers like any code invoking persistCommandStatus can
react to persistence failure.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts`:
- Around line 194-198: In useCommandWatcher, the call to persistCommandStatus
inside the .filter() callback creates a floating Promise; either handle timeouts
in a separate loop before calling .filter() or explicitly mark the
fire-and-forget intent (e.g., use void persistCommandStatus(...)) so the Promise
is not silently leaked; update the .filter() callback around
persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired
before execution" }) to either await the call in an async timeout-handling loop
or prefix it with void to signal intentional non-awaiting.
In `@apps/docs/content/docs/task-management.mdx`:
- Around line 39-45: Add a one-line lead-in before the fenced block clarifying
these are example AI agent prompts (not executable code) so readers understand
intent; update the copy immediately above the fence that contains the three
sample prompts ("Create a task for fixing the login validation bug", "List all
my high-priority tasks", "Update the auth task status to done") to a sentence
like “Example agent prompts:” (or similar) to make the snippet explicit.
In `@apps/docs/src/components/ResourceCard/ResourceCard.tsx`:
- Around line 30-35: ResourceCard currently renders all links as raw <a> tags;
change it to use Next.js client-side Link for internal routes: add the "use
client" directive at the top of ResourceCard and import Link from "next/link",
then in the JSX replace the unconditional <a href={href} ...> with a conditional
render that uses <Link href={href}>...</Link> when isExternal is false and the
existing <a target="_blank" rel="noopener noreferrer"> when isExternal is true
(follow the NavLink pattern used elsewhere for reference). Ensure href and
isExternal props used by ResourceCard are passed into the Link correctly and
that external links keep the security attributes.
In
`@packages/mcp/src/tools/devices/get-workspace-details/get-workspace-details.ts`:
- Around line 11-13: The input schema in get-workspace-details.ts currently
defines workspaceId as z.string(); update the inputSchema entry for workspaceId
to use z.string().uuid() (matching other handlers like update-workspace.ts and
delete-workspace.ts) so workspace IDs are validated as UUIDs; locate the
inputSchema object in the getWorkspaceDetails handler (the inputSchema with
deviceId and workspaceId) and change the workspaceId validator to .uuid() to
enforce consistent ID format.
In `@packages/mcp/src/tools/devices/start-claude-session/start-claude-session.ts`:
- Around line 42-54: validateArgs currently uses "as string" casts without
runtime checks, making it fragile if called outside the MCP-validated path; fix
it by either (A) performing runtime type checks inside validateArgs (use typeof
on args.deviceId, args.taskId, args.workspaceId and allow paneId only if typeof
=== 'string', return null on any mismatch) or (B) reuse the existing Zod input
schema (e.g., inputSchema.safeParse(args) or inputSchema.parse within a
try/catch) to validate and extract deviceId, taskId, workspaceId, and optional
paneId, then return the typed object or null on failure. Ensure you reference
the validateArgs function and the inputSchema symbol so the implementation
replaces the unsafe "as string" casts with real validation.
In `@packages/mcp/src/tools/devices/switch-workspace/switch-workspace.ts`:
- Around line 26-28: Redundant explicit type assertions are used when extracting
args (deviceId, workspaceId, workspaceName); remove the unnecessary "as string"
and "as string | undefined" casts in the switchWorkspace handler so the
variables rely on Zod-inferred types, i.e., change assignments that reference
args.deviceId, args.workspaceId, and args.workspaceName to plain variable
bindings without "as" casts and ensure any downstream usages rely on their
inferred types.
| const apiClient = createTRPCProxyClient<AppRouter>({ | ||
| links: [ | ||
| httpBatchLink({ | ||
| url: `${env.NEXT_PUBLIC_API_URL}/api/trpc`, | ||
| headers: () => { | ||
| const token = getAuthToken(); | ||
| return token ? { Authorization: `Bearer ${token}` } : {}; | ||
| }, | ||
| transformer: superjson, | ||
| }), | ||
| ], | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for existing tRPC clients in the desktop app
rg -n "createTRPCProxyClient|httpBatchLink" --type=ts --type=tsx -C3 apps/desktop/src/Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check the structure of src/lib/trpc and what it exports
find apps/desktop/src -path "*lib/trpc*" -type f | head -20Repository: superset-sh/superset
Length of output: 1203
🏁 Script executed:
# Check if there's a shared API client pattern
rg -n "tRPC.*client|API.*client" --type=ts --type=tsx apps/desktop/src/ -C2 | head -50Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Look at the file structure to understand the app organization
tree -L 3 apps/desktop/src/ -d 2>/dev/null || find apps/desktop/src -type d | head -20Repository: superset-sh/superset
Length of output: 1897
🏁 Script executed:
# Check what src/lib/trpc exports
cat -n apps/desktop/src/lib/trpc/index.tsRepository: superset-sh/superset
Length of output: 1642
🏁 Script executed:
# Check the ElectronTRPCProvider to understand how tRPC is used
find apps/desktop/src/renderer -name "*ElectronTRPC*" -o -name "*electron-trpc*" | head -10Repository: superset-sh/superset
Length of output: 249
🏁 Script executed:
# Look for API client patterns in the renderer
rg -l "apiClient|api\..*\(" apps/desktop/src/renderer --max-count=20Repository: superset-sh/superset
Length of output: 317
🏁 Script executed:
# Check the persistCommandStatus file itself
cat -n apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/persistCommandStatus.tsRepository: superset-sh/superset
Length of output: 1831
🏁 Script executed:
# Check ElectronTRPCProvider to see how it's structured
cat -n apps/desktop/src/renderer/providers/ElectronTRPCProvider/ElectronTRPCProvider.tsxRepository: superset-sh/superset
Length of output: 1272
🏁 Script executed:
# Check the electron-trpc.ts file
cat -n apps/desktop/src/renderer/lib/electron-trpc.tsRepository: superset-sh/superset
Length of output: 615
🏁 Script executed:
# Look for other HTTP API client creations in the renderer
rg "httpBatchLink|createTRPCProxyClient|NEXT_PUBLIC_API_URL" apps/desktop/src/renderer --max-count=30Repository: superset-sh/superset
Length of output: 3845
🏁 Script executed:
# Check useCommandWatcher to understand the context and claimedAt usage
find apps/desktop/src/renderer -name "*useCommandWatcher*" -type fRepository: superset-sh/superset
Length of output: 178
🏁 Script executed:
# Check the existing api-trpc-client.ts
cat -n apps/desktop/src/renderer/lib/api-trpc-client.tsRepository: superset-sh/superset
Length of output: 1025
🏁 Script executed:
# Check useCommandWatcher.ts to see how persistCommandStatus is used
cat -n apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.tsRepository: superset-sh/superset
Length of output: 8548
🏁 Script executed:
# Check collections.ts to see if it also has the same duplicate pattern
cat -n apps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/collections.ts | head -50Repository: superset-sh/superset
Length of output: 2058
Use the existing apiTrpcClient instead of duplicating the tRPC client configuration.
An identical HTTP tRPC client already exists in renderer/lib/api-trpc-client.ts with the same URL, authentication headers, and transformer. Creating a new instance here duplicates configuration and risks divergence if the API client setup changes. Import and use apiTrpcClient from that module instead.
Additionally, the local command state sets claimedAt (line 94 in useCommandWatcher.ts), but it's not included in the persistence calls—this creates a data inconsistency between local and server state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/persistCommandStatus.ts`
around lines 8 - 19, Replace the duplicated tRPC client in
persistCommandStatus.ts by importing and using the existing apiTrpcClient
(instead of creating a new apiClient via createTRPCProxyClient) so all calls
reuse the centralized configuration from renderer/lib/api-trpc-client.ts; also
update the persistence calls made by the functions in persistCommandStatus (the
methods that call the backend to persist command state) to include the local
claimedAt value that useCommandWatcher.ts sets on the command, ensuring
claimedAt is sent with the payload so server state matches the local command
state.
| ## Supported Agents | ||
|
|
||
| | Agent | Command | | ||
| |-------|---------| | ||
| | **Claude Code** | `claude` | | ||
| | **Codex** | `codex` | | ||
| | **Gemini CLI** | `gemini` | | ||
| | **OpenCode** | `opencode` | | ||
| | **Any CLI agent** | Works in the terminal | |
There was a problem hiding this comment.
Minor inconsistency: agent-integration.mdx lists only three agents.
The Supported Agents table here includes Gemini CLI (line 26) and "Any CLI agent" (line 28), but agent-integration.mdx (lines 14–16) only lists Claude Code, Codex, and OpenCode. Consider keeping both pages in sync to avoid confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docs/content/docs/parallel-agents.mdx` around lines 20 - 28, Supported
Agents table in parallel-agents.mdx lists Gemini CLI and "Any CLI agent" but
agent-integration.mdx only lists Claude Code, Codex, and OpenCode; update the
agent lists to be consistent by either adding Gemini and a note about generic
CLI agents to agent-integration.mdx or removing those entries from the Supported
Agents table in parallel-agents.mdx so both pages mention the same agents
(reference the "Supported Agents" table and the agent-integration.mdx agent list
entries like Claude Code, Codex, OpenCode, Gemini CLI, and Any CLI agent when
making the change).
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts (1)
192-203:⚠️ Potential issue | 🟡 MinorTimed-out commands should be added to
handledCommandsbefore returning.When a command times out,
handledCommands.add(cmd.id)is never called. Because theuseEffectdependency array includespendingCommands, if the effect re-fires before the optimistic"timeout"status update propagates out of the live-query (a plausible timing edge given React batching anduseLiveQueryinternals), the same command will pass the filter again — triggering a secondcollections.agentCommands.updateand a second fire-and-forgetpersistCommandStatuscall.
persistCommandStatusis idempotent and the local update is harmless, so there's no correctness breakage, but the extra retry loop (up to ~3.5 s) could fire unnecessarily. Adding the guard eliminates the edge case entirely and keeps the pattern consistent with the rest of the watcher:🛡️ Proposed fix
if (cmd.timeoutAt && new Date(cmd.timeoutAt) < now) { + handledCommands.add(cmd.id); collections.agentCommands.update(cmd.id, (draft) => { draft.status = "timeout"; draft.error = "Command expired before execution"; }); persistCommandStatus({ id: cmd.id, status: "timeout", error: "Command expired before execution", }); return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts` around lines 192 - 203, When a command times out in the timeout handling block inside useCommandWatcher, add the command id to handledCommands before returning to prevent the effect from re-processing it; specifically, in the branch that calls collections.agentCommands.update(...) and persistCommandStatus({ id: cmd.id, status: "timeout", ... }), also call handledCommands.add(cmd.id) prior to the return false so the command is marked handled (consistent with how other branches use handledCommands and avoids duplicate retries from useEffect with pendingCommands).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/components/AgentHooks/hooks/useCommandWatcher/useCommandWatcher.ts`:
- Around line 192-203: When a command times out in the timeout handling block
inside useCommandWatcher, add the command id to handledCommands before returning
to prevent the effect from re-processing it; specifically, in the branch that
calls collections.agentCommands.update(...) and persistCommandStatus({ id:
cmd.id, status: "timeout", ... }), also call handledCommands.add(cmd.id) prior
to the return false so the command is marked handled (consistent with how other
branches use handledCommands and avoids duplicate retries from useEffect with
pendingCommands).
The docs restructure (new MDX pages, overview rewrite, meta.json reorganization, redirect changes) was part of a separate docs task that was accidentally included in the MCP auth flakiness PR (#1608). This reverts only the docs-task-specific changes: - Remove workflow.mdx, task-management.mdx, parallel-agents.mdx - Restore overview.mdx, meta.json, agent-integration.mdx to original - Restore next.config.mjs redirect to /installation - Restore ResourceCard to use target="_blank" for all links MCP-related changes (mcp.mdx, get_workspace_details, etc.) are kept.
The docs restructure (new MDX pages, overview rewrite, meta.json reorganization, redirect changes) was part of a separate docs task that was accidentally included in the MCP auth flakiness PR (#1608). This reverts only the docs-task-specific changes: - Remove workflow.mdx, task-management.mdx, parallel-agents.mdx - Restore overview.mdx, meta.json, agent-integration.mdx to original - Restore next.config.mjs redirect to /installation - Restore ResourceCard to use target="_blank" for all links MCP-related changes (mcp.mdx, get_workspace_details, etc.) are kept.
1 participant
Summary
get_workspace_detailstool, unifiedstart_claude_session/start_claude_subagentinto a single tool withpaneIdparameterTest plan
bun run lint:fixpassesbun run typecheckpasses (17/17 packages)bun testpasses (1230 pass, 0 fail)bunx sherif --fixpassesbun run --filter @superset/docs buildproduces 18 static pageslist_devices,list_members,list_tasks,list_projects,list_workspaces,get_app_context,get_workspace_details,get_task,update_taskall respond correctlySummary by CodeRabbit
New Features
Documentation