Skip to content

Commit 653612e

Browse files
committed
fix(vscode): address CodeRabbit findings on workflow lanes
Pluralize the lane step count, add aria-expanded to the lane toggle, guard parseArgs against non-object JSON.parse results, merge duplicate tool-label cases, and gate the lane-failure sweep on the parent ToolResult's is_error flag so a successful batch result no longer marks still-running lanes as failed.
1 parent 11aaa83 commit 653612e

4 files changed

Lines changed: 33 additions & 10 deletions

File tree

apps/vscode/test/event-handlers.test.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
* Run: pnpm exec vitest run --config apps/vscode/vitest.config.ts test/event-handlers.test.ts
77
*/
88
import { beforeEach, describe, expect, it, vi } from "vitest";
9+
import { useChatStore } from "../webview-ui/src/stores/chat.store";
10+
import { deriveWorkflowLanes, maxLaneStepCount } from "../webview-ui/src/lib/workflow-lanes";
11+
import type { UIStepItem } from "../webview-ui/src/stores/chat.store";
912

1013
const boundary = vi.hoisted(() => ({
1114
saveConfig: vi.fn(),
@@ -28,10 +31,6 @@ vi.mock("@/components/ui/sonner", () => ({
2831
toast: { error: boundary.toastError, warning: boundary.toastWarning },
2932
}));
3033

31-
import { useChatStore } from "../webview-ui/src/stores/chat.store";
32-
import { deriveWorkflowLanes, maxLaneStepCount } from "../webview-ui/src/lib/workflow-lanes";
33-
import type { UIStepItem } from "../webview-ui/src/stores/chat.store";
34-
3534
beforeEach(() => {
3635
boundary.streamChat.mockReset();
3736
boundary.abortChat.mockReset();
@@ -220,6 +219,28 @@ describe("Webview DynamicWorkflow per-agent lanes", () => {
220219
// Already-terminal lanes are left alone.
221220
expect(status["agentC"]!.status).toBe("done");
222221
});
222+
223+
it("leaves spawned/running lanes alone when the parent ToolResult succeeds", () => {
224+
startWorkflowTurn();
225+
226+
useChatStore.getState().processEvent({
227+
type: "SubagentStatus",
228+
payload: { parent_tool_call_id: "wf-1", agent_id: "agentA", agent_label: "explore", agent_index: 1, status: "spawned" },
229+
});
230+
useChatStore.getState().processEvent({
231+
type: "SubagentStatus",
232+
payload: { parent_tool_call_id: "wf-1", agent_id: "agentA", status: "running" },
233+
});
234+
235+
useChatStore.getState().processEvent({
236+
type: "ToolResult",
237+
payload: { tool_call_id: "wf-1", return_value: { is_error: false, output: "ok", message: "", display: [] } },
238+
});
239+
240+
const status = workflowToolItem().subagent_status!;
241+
expect(status["agentA"]!.status).toBe("running");
242+
expect(status["agentA"]!.endedAt).toBeUndefined();
243+
});
223244
});
224245

225246
describe("workflow lane derivation", () => {

apps/vscode/webview-ui/src/components/WorkflowCard.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,12 @@ function LaneRow({ lane, maxSteps, renderStepItem }: { lane: WorkflowLane; maxSt
6363

6464
return (
6565
<div className="text-xs">
66-
<button onClick={() => setExpanded(!expanded)} className="w-full flex items-center gap-2 py-1 hover:bg-muted/50 transition-colors text-left" disabled={lane.stepCount === 0}>
66+
<button onClick={() => setExpanded(!expanded)} aria-expanded={expanded} className="w-full flex items-center gap-2 py-1 hover:bg-muted/50 transition-colors text-left" disabled={lane.stepCount === 0}>
6767
<StatusDot status={lane.status} />
6868
<span className="font-mono text-[11px] shrink-0">{laneLabel(lane)}</span>
6969
<LaneBar fraction={fraction} done={done} />
7070
<span className="text-muted-foreground tabular-nums shrink-0">
71-
{queued ? "queued" : `${lane.status === "done" ? "done · " : ""}${lane.stepCount} steps`}
71+
{queued ? "queued" : `${lane.status === "done" ? "done · " : ""}${lane.stepCount} step${lane.stepCount === 1 ? "" : "s"}`}
7272
{duration && ` · ${duration}`}
7373
</span>
7474
{lane.stepCount > 0 && (expanded ? <IconChevronDown className="size-3 text-muted-foreground" /> : <IconChevronRight className="size-3 text-muted-foreground" />)}

apps/vscode/webview-ui/src/lib/tool-args.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ export function parseArgs(args: string | null): Record<string, unknown> {
55
return {};
66
}
77
try {
8-
return JSON.parse(args);
8+
const parsed: unknown = JSON.parse(args);
9+
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
10+
return { raw: args };
11+
}
12+
return parsed as Record<string, unknown>;
913
} catch {
1014
return { raw: args };
1115
}
@@ -17,9 +21,7 @@ export function getToolLabel(call: UIToolCall): string {
1721
case "Shell":
1822
return (args.command as string) || "command";
1923
case "ReadFile":
20-
return (args.path as string)?.split("/").pop() || "file";
2124
case "WriteFile":
22-
return (args.path as string)?.split("/").pop() || "file";
2325
case "StrReplaceFile":
2426
return (args.path as string)?.split("/").pop() || "file";
2527
case "Glob":

apps/vscode/webview-ui/src/stores/event-handlers.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ function applyEventToSteps(
285285

286286
// Batch aborted: lanes still spawned/running when the parent result lands
287287
// have no further lifecycle events coming, so freeze them as failed.
288-
if (toolItem?.subagent_status) {
288+
if (result.return_value.is_error && toolItem?.subagent_status) {
289289
for (const status of Object.values(toolItem.subagent_status)) {
290290
if (status.status === "spawned" || status.status === "running") {
291291
status.status = "failed";

0 commit comments

Comments
 (0)