Skip to content

Commit 54d48ad

Browse files
authored
🤖 fix: stabilize bash story expansion and strict bash args (#1130)
Fix Storybook flakiness in App/Bash Foreground by scoping expand clicks to the message window and removing the timing-based delay. Also ensure Storybook bash tool mocks always include required fields so they render with BashToolCall instead of GenericToolCall. _Generated with `mux`_
1 parent 3955a3d commit 54d48ad

File tree

9 files changed

+60
-26
lines changed

9 files changed

+60
-26
lines changed

src/browser/stories/App.bash.stories.tsx

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,27 +35,36 @@ async function expandAllBashTools(canvasElement: HTMLElement) {
3535
{ timeout: 5000 }
3636
);
3737

38-
// Now find and expand all tool icons
38+
// Now find and expand all tool icons (scoped to the message window so we don't click unrelated ▶)
39+
const messageWindow = canvasElement.querySelector('[data-testid="message-window"]');
40+
if (!(messageWindow instanceof HTMLElement)) {
41+
throw new Error("Message window not found");
42+
}
43+
44+
// Wait for at least one expand icon to appear in the message window
3945
await waitFor(
40-
async () => {
41-
const allSpans = canvasElement.querySelectorAll("span");
46+
() => {
47+
const allSpans = messageWindow.querySelectorAll("span");
4248
const expandIcons = Array.from(allSpans).filter((span) => span.textContent?.trim() === "▶");
4349
if (expandIcons.length === 0) {
4450
throw new Error("No expand icons found");
4551
}
46-
for (const icon of expandIcons) {
47-
const header = icon.closest("[class*='cursor-pointer']");
48-
if (header) {
49-
await userEvent.click(header as HTMLElement);
50-
}
51-
}
5252
},
5353
{ timeout: 5000 }
5454
);
5555

56-
// Wait for focus auto-select timer, then blur
57-
await new Promise((resolve) => setTimeout(resolve, 150));
58-
(document.activeElement as HTMLElement)?.blur();
56+
const allSpans = messageWindow.querySelectorAll("span");
57+
const expandIcons = Array.from(allSpans).filter((span) => span.textContent?.trim() === "▶");
58+
59+
for (const icon of expandIcons) {
60+
const header = icon.closest("div.cursor-pointer");
61+
if (header) {
62+
await userEvent.click(header as HTMLElement);
63+
}
64+
}
65+
66+
// Avoid leaving focus on a tool header (some components auto-focus inputs on timers)
67+
(document.activeElement as HTMLElement | null)?.blur?.();
5968
}
6069

6170
export default {

src/browser/stories/mockFactory.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -261,14 +261,20 @@ export function createBashTool(
261261
output: string,
262262
exitCode = 0,
263263
timeoutSecs = 3,
264-
durationMs = 50
264+
durationMs = 50,
265+
displayName = "Bash"
265266
): MuxPart {
266267
return {
267268
type: "dynamic-tool",
268269
toolCallId,
269270
toolName: "bash",
270271
state: "output-available",
271-
input: { script, run_in_background: false, timeout_secs: timeoutSecs },
272+
input: {
273+
script,
274+
run_in_background: false,
275+
timeout_secs: timeoutSecs,
276+
display_name: displayName,
277+
},
272278
output: { success: exitCode === 0, output, exitCode, wall_duration_ms: durationMs },
273279
};
274280
}
@@ -342,14 +348,20 @@ export function createBackgroundBashTool(
342348
toolCallId: string,
343349
script: string,
344350
processId: string,
345-
displayName?: string
351+
displayName = "Background",
352+
timeoutSecs = 60
346353
): MuxPart {
347354
return {
348355
type: "dynamic-tool",
349356
toolCallId,
350357
toolName: "bash",
351358
state: "output-available",
352-
input: { script, run_in_background: true, display_name: displayName },
359+
input: {
360+
script,
361+
run_in_background: true,
362+
display_name: displayName,
363+
timeout_secs: timeoutSecs,
364+
},
353365
output: {
354366
success: true,
355367
output: `Background process started with ID: ${processId}`,
@@ -365,8 +377,9 @@ export function createMigratedBashTool(
365377
toolCallId: string,
366378
script: string,
367379
processId: string,
368-
displayName?: string,
369-
capturedOutput?: string
380+
displayName = "Bash",
381+
capturedOutput?: string,
382+
timeoutSecs = 30
370383
): MuxPart {
371384
const outputLines = capturedOutput?.split("\n") ?? [];
372385
const outputSummary =
@@ -379,7 +392,12 @@ export function createMigratedBashTool(
379392
toolName: "bash",
380393
state: "output-available",
381394
// No run_in_background flag - this started as foreground
382-
input: { script, run_in_background: false, display_name: displayName, timeout_secs: 30 },
395+
input: {
396+
script,
397+
run_in_background: false,
398+
display_name: displayName,
399+
timeout_secs: timeoutSecs,
400+
},
383401
output: {
384402
success: true,
385403
output: `Process sent to background with ID: ${processId}\n\nOutput so far (${outputLines.length} lines):\n${outputSummary}`,

src/browser/utils/messages/retryEligibility.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe("hasInterruptedStream", () => {
7474
historyId: "assistant-1",
7575
toolName: "bash",
7676
toolCallId: "call-1",
77-
args: { script: "echo test" },
77+
args: { script: "echo test", timeout_secs: 10, display_name: "Test" },
7878
status: "interrupted",
7979
isPartial: true,
8080
historySequence: 2,

src/browser/utils/messages/sanitizeToolInput.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe("sanitizeToolInputs", () => {
7878
toolCallId: "toolu_02test",
7979
toolName: "bash",
8080
state: "output-available",
81-
input: { script: "ls", timeout_secs: 10 },
81+
input: { script: "ls", timeout_secs: 10, display_name: "Test" },
8282
output: { success: true },
8383
},
8484
],
@@ -89,7 +89,7 @@ describe("sanitizeToolInputs", () => {
8989
const sanitized = sanitizeToolInputs(messages);
9090
expect(sanitized[0].parts[0]).toMatchObject({
9191
type: "dynamic-tool",
92-
input: { script: "ls", timeout_secs: 10 },
92+
input: { script: "ls", timeout_secs: 10, display_name: "Test" },
9393
});
9494
});
9595

src/common/types/tools.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import type { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions";
99
// Bash Tool Types
1010
export interface BashToolArgs {
1111
script: string;
12-
timeout_secs?: number; // Optional: defaults to 3 seconds for interactivity
12+
timeout_secs: number; // Required - defaults should be applied by producers
1313
run_in_background?: boolean; // Run without blocking (for long-running processes)
1414
display_name: string; // Required - used as process identifier if sent to background
1515
}

src/node/services/mock/scenarios/permissionModes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ const executePlanTurn: ScenarioTurn = {
8686
script:
8787
'apply_patch <<\'PATCH\'\n*** Begin Patch\n*** Update File: src/utils/legacyFunction.ts\n@@\n-export function handleRequest(input: Request) {\n- if (!input.userId || !input.payload) {\n- throw new Error("Missing fields");\n- }\n-\n- const result = heavyFormatter(input.payload);\n- return {\n- id: input.userId,\n- details: result,\n- };\n-}\n+function verifyInputs(input: Request) {\n+ if (!input.userId || !input.payload) {\n+ throw new Error("Missing fields");\n+ }\n+}\n+\n+function buildResponse(input: Request) {\n+ const result = heavyFormatter(input.payload);\n+ return { id: input.userId, details: result };\n+}\n+\n+export function handleRequest(input: Request) {\n+ verifyInputs(input);\n+ return buildResponse(input);\n+}\n*** End Patch\nPATCH',
8888
timeout_secs: 10,
89+
display_name: "Apply refactor",
8990
},
9091
},
9192
{

src/node/services/mock/scenarios/toolFlows.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ const listDirectoryTurn: ScenarioTurn = {
9090
delay: STREAM_BASE_DELAY,
9191
toolCallId: "tool-bash-ls",
9292
toolName: "bash",
93-
args: { script: "ls -1", timeout_secs: 10 },
93+
args: { script: "ls -1", timeout_secs: 10, display_name: "List directory" },
9494
},
9595
{
9696
kind: "tool-end",
@@ -164,7 +164,11 @@ const createTestFileTurn: ScenarioTurn = {
164164
delay: STREAM_BASE_DELAY,
165165
toolCallId: "tool-bash-create-test-file",
166166
toolName: "bash",
167-
args: { script: "printf 'hello' > test.txt", timeout_secs: 10 },
167+
args: {
168+
script: "printf 'hello' > test.txt",
169+
timeout_secs: 10,
170+
display_name: "Create test file",
171+
},
168172
},
169173
{
170174
kind: "tool-end",

src/node/services/partialService.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ describe("PartialService - Error Recovery", () => {
108108
toolCallId: "call-1",
109109
toolName: "bash",
110110
state: "input-available",
111-
input: { script: "echo test" },
111+
input: { script: "echo test", timeout_secs: 10, display_name: "Test" },
112112
},
113113
],
114114
};

src/node/services/tools/bash.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,6 +1459,7 @@ describe("bash tool - background execution", () => {
14591459
const tool = testEnv.tool;
14601460
const args: BashToolArgs = {
14611461
script: "echo test",
1462+
timeout_secs: 5,
14621463
run_in_background: true,
14631464
display_name: "test",
14641465
};
@@ -1508,6 +1509,7 @@ describe("bash tool - background execution", () => {
15081509
const tool = createBashTool(config);
15091510
const args: BashToolArgs = {
15101511
script: "echo hello",
1512+
timeout_secs: 5,
15111513
run_in_background: true,
15121514
display_name: "test",
15131515
};

0 commit comments

Comments
 (0)