fix: hide empty shell output disclosure (#2378)

This commit is contained in:
Carlos Florêncio
2026-08-07 18:14:32 +01:00
committed by GitHub
parent 4f7298a5d1
commit 31d4f1e65f
9 changed files with 335 additions and 33 deletions
@@ -54,6 +54,10 @@ function executeMessage(
};
}
function retainedOutputSummary(overrides: ShellOutputSummary = {}): ShellOutputSummary {
return { has_output: true, stdout_bytes: 1, ...overrides };
}
function hookResult(
overrides: Partial<UseShellCommandOutputResult> = {},
): UseShellCommandOutputResult {
@@ -71,14 +75,30 @@ function openOutput() {
}
describe("ToolExecuteMessage command row", () => {
it("hides the output disclosure when the projected summary is empty", () => {
render(
<ToolExecuteMessage
comment={executeMessage("complete", {
exit_code: 0,
has_output: false,
stdout_bytes: 0,
stderr_bytes: 0,
})}
/>,
);
expect(screen.getByTestId("tool-execute-command")).toBeTruthy();
expect(screen.queryByRole("button", { name: /show command output/i })).toBeNull();
expect(useShellOutputMock).not.toHaveBeenCalled();
});
it("keeps the normalized command and working directory visible while output is collapsed", () => {
render(
<ToolExecuteMessage
comment={executeMessage("running", {
has_output: true,
stdout_bytes: 2048,
stderr_bytes: 0,
})}
comment={executeMessage(
"running",
retainedOutputSummary({ stdout_bytes: 2048, stderr_bytes: 0 }),
)}
/>,
);
@@ -103,7 +123,8 @@ describe("ToolExecuteMessage command row", () => {
it("renders loading and empty snapshot states only after opening", () => {
useShellOutputMock.mockReturnValue(hookResult({ isLoading: true }));
const view = render(<ToolExecuteMessage comment={executeMessage("running")} />);
const outputSummary = retainedOutputSummary();
const view = render(<ToolExecuteMessage comment={executeMessage("running", outputSummary)} />);
expect(screen.queryByText("Loading command output...")).toBeNull();
openOutput();
@@ -119,7 +140,7 @@ describe("ToolExecuteMessage command row", () => {
},
}),
);
view.rerender(<ToolExecuteMessage comment={executeMessage("running")} />);
view.rerender(<ToolExecuteMessage comment={executeMessage("running", outputSummary)} />);
expect(screen.getByText("No command output yet.")).toBeTruthy();
expect(screen.queryByText(/Exit code/)).toBeNull();
});
@@ -142,7 +163,11 @@ describe("ToolExecuteMessage output states", () => {
},
}),
);
render(<ToolExecuteMessage comment={executeMessage("complete", { exit_code: 7 })} />);
render(
<ToolExecuteMessage
comment={executeMessage("complete", retainedOutputSummary({ exit_code: 7 }))}
/>,
);
openOutput();
expect(screen.getByText("standard output")).toBeTruthy();
@@ -164,11 +189,10 @@ describe("ToolExecuteMessage output states", () => {
);
render(
<ToolExecuteMessage
comment={executeMessage("cancelled", {
has_output: true,
stdout_bytes: 999,
stdout: "forbidden summary transcript",
})}
comment={executeMessage(
"cancelled",
retainedOutputSummary({ stdout_bytes: 999, stdout: "forbidden summary transcript" }),
)}
/>,
);
@@ -192,7 +216,7 @@ describe("ToolExecuteMessage output states", () => {
retry,
}),
);
render(<ToolExecuteMessage comment={executeMessage("running")} />);
render(<ToolExecuteMessage comment={executeMessage("running", retainedOutputSummary())} />);
openOutput();
expect(screen.getByText("retained transcript")).toBeTruthy();
@@ -213,7 +237,7 @@ describe("ToolExecuteMessage output states", () => {
error: new Error("network unavailable"),
}),
);
render(<ToolExecuteMessage comment={executeMessage("running")} />);
render(<ToolExecuteMessage comment={executeMessage("running", retainedOutputSummary())} />);
openOutput();
expect(screen.getByText("Command output unavailable.")).toBeTruthy();
@@ -233,7 +257,11 @@ describe("ToolExecuteMessage cancelled result", () => {
},
}),
);
render(<ToolExecuteMessage comment={executeMessage("cancelled", { exit_code: 0 })} />);
render(
<ToolExecuteMessage
comment={executeMessage("cancelled", retainedOutputSummary({ exit_code: 0 }))}
/>,
);
openOutput();
expect(screen.getByText("Exit code 0").className).toContain("text-muted-foreground");
@@ -7,7 +7,7 @@ import { transformPathsInText } from "@/lib/utils";
import type { Message } from "@/lib/types/http";
import { ShellOutputDisclosure } from "./shell-output-disclosure";
import { normalizeToolCallStatus } from "./tool-status";
import type { ToolCallMetadata } from "../types";
import { hasProjectedShellOutput, type ToolCallMetadata } from "../types";
import { useTranslation } from "react-i18next";
type ToolExecuteMessageProps = {
@@ -89,12 +89,14 @@ export const ToolExecuteMessage = memo(function ToolExecuteMessage({
</span>
</div>
)}
<ShellOutputDisclosure
sessionId={comment.session_id}
messageId={comment.id}
messageStatus={status}
summary={shellExec?.output}
/>
{hasProjectedShellOutput(shellExec?.output) && (
<ShellOutputDisclosure
sessionId={comment.session_id}
messageId={comment.id}
messageStatus={status}
summary={shellExec?.output}
/>
)}
</div>
</div>
);
@@ -7,7 +7,7 @@ import { GridSpinner } from "@/components/grid-spinner";
import { cn, transformPathsInText } from "@/lib/utils";
import type { Message } from "@/lib/types/http";
import type { TurnGroup } from "@/hooks/use-processed-messages";
import type { ToolCallMetadata } from "@/components/task/chat/types";
import { hasProjectedShellOutput, type ToolCallMetadata } from "@/components/task/chat/types";
import { MessageRenderer } from "@/components/task/chat/message-renderer";
import { isSubagentEffectivelyActive } from "@/components/task/chat/messages/tool-subagent-message";
@@ -167,14 +167,6 @@ function isZeroExitCode(shellExec: ShellExecPayload): boolean {
return exitCode === 0;
}
function hasProjectedShellOutput(output: ShellExecPayload["output"]): boolean {
return (
Boolean(output?.has_output) ||
(output?.stdout_bytes ?? 0) > 0 ||
(output?.stderr_bytes ?? 0) > 0
);
}
function createShellExecSummary(message: Message, shellExec: ShellExecPayload): ShellExecSummary {
const output = shellExec.output;
return {
+8
View File
@@ -103,6 +103,14 @@ export type ShellExecOutputSummary = {
truncated?: boolean;
};
export function hasProjectedShellOutput(output: ShellExecOutputSummary | undefined): boolean {
return (
Boolean(output?.has_output) ||
(output?.stdout_bytes ?? 0) > 0 ||
(output?.stderr_bytes ?? 0) > 0
);
}
export type ShellExecPayload = {
command?: string;
work_dir?: string;
@@ -13,6 +13,57 @@ async function expectInsideViewport(
}
test.describe("mobile: shell command output", () => {
test("does not show an output disclosure for a command with no output", async ({
testPage,
apiClient,
seedData,
}) => {
const task = await apiClient.createTask(seedData.workspaceId, "Mobile Empty Command Output", {
description: "seeded mobile shell command without output",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
agent_profile_id: seedData.agentProfileId,
repository_ids: [seedData.repositoryId],
});
const { session_id: sessionId } = await apiClient.seedTaskSession(task.id, {
state: "IDLE",
agentProfileId: seedData.agentProfileId,
});
const command = "true";
await apiClient.seedSessionMessage(sessionId, {
type: "tool_execute",
content: command,
metadata: {
status: "complete",
tool_call_id: "tool-mobile-empty-output",
normalized: {
shell_exec: {
command,
work_dir: "/workspace",
output: { exit_code: 0 },
},
},
},
});
const shellOutputRequests: string[] = [];
testPage.on("request", (request) => {
if (request.url().endsWith("/shell-output")) shellOutputRequests.push(request.url());
});
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
const chat = session.activeChat();
await expect(chat.getByTestId("tool-execute-command").filter({ hasText: command })).toBeVisible(
{ timeout: 15_000 },
);
await expect(chat.getByLabel("Command succeeded")).toBeVisible();
await expect(chat.getByRole("button", { name: "Show command output" })).toHaveCount(0);
await expect(chat.getByTestId("tool-execute-output")).toHaveCount(0);
expect(shellOutputRequests).toHaveLength(0);
});
test("keeps a long command and lazy output inside the viewport", async ({
testPage,
apiClient,
@@ -46,6 +46,37 @@ async function seedShellMessage(
}
test.describe("shell command output", () => {
test("does not show an output disclosure for a command with no output", async ({
testPage,
apiClient,
seedData,
}) => {
const command = "true";
const taskId = await seedShellMessage(apiClient, seedData, {
title: "Empty Command Output",
command,
status: "complete",
output: { exit_code: 0 },
});
const shellOutputRequests: string[] = [];
testPage.on("request", (request) => {
if (request.url().endsWith("/shell-output")) shellOutputRequests.push(request.url());
});
await testPage.goto(`/t/${taskId}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
const chat = session.activeChat();
await expect(chat.getByTestId("tool-execute-command").filter({ hasText: command })).toBeVisible(
{ timeout: 15_000 },
);
await expect(chat.getByLabel("Command succeeded")).toBeVisible();
await expect(chat.getByRole("button", { name: "Show command output" })).toHaveCount(0);
await expect(chat.getByTestId("tool-execute-output")).toHaveCount(0);
expect(shellOutputRequests).toHaveLength(0);
});
test("keeps full commands visible and fetches completed output only after expansion", async ({
testPage,
apiClient,
@@ -0,0 +1,110 @@
---
spec: docs/specs/ui/acp-shell-command-output.md
created: 2026-08-07
status: done
---
# Implementation Plan: Hide empty shell output disclosures
## Root cause
`ToolExecuteMessage` always mounts `ShellOutputDisclosure`, including when the
projected shell summary has no stdout or stderr. The disclosure formats a
zero-byte summary as `Output`, so users can expand a control that only reveals
the empty-output or exit-status state. The summary already carries the data
needed to decide whether an output body exists.
## Overview
Guard the shared shell output disclosure at the command-row boundary using the
browser-facing summary. Preserve the existing collapsed, on-demand behavior for
commands with output, then prove both paths in the existing component, desktop,
and mobile chat tests. No backend, API, persistence, or mobile composition
change is required.
## Frontend
### Command row disclosure guard
Files:
- `apps/web/components/task/chat/messages/tool-execute-message.tsx`
- `apps/web/components/task/chat/messages/tool-execute-message.test.tsx`
Render `ShellOutputDisclosure` only when the normalized summary represents
retained stdout/stderr: `has_output` is true or either projected byte count is
positive. Keep the disclosure unmounted for zero-output commands so the
on-demand hook cannot fetch or poll accidentally. Commands with output keep
their existing collapsed default, endpoint fetch, polling, truncation, and exit
status behavior.
### Mobile parity contract
Desktop and mobile use the same `ToolExecuteMessage` data and disclosure guard;
mobile renders it inside the existing `TaskChatPanel` composition. The nearest
shipped mobile exemplar is
`apps/web/e2e/tests/chat/mobile-tool-execute-output.spec.ts`, which already
proves the long-command/output path. The mobile hierarchy remains the command
row first and the optional output disclosure second; a zero-output command has
no secondary action because there is no content to inspect. The chat transcript
continues to own vertical scrolling, with no new drawer, overlay, viewport, or
safe-area behavior.
## Tests
- **What:** A zero-output summary renders the command/status context without a
`Show command output` control, while a summary with `has_output` or positive
byte counts retains the collapsed disclosure and lazy-fetch behavior.
**File:** `apps/web/components/task/chat/messages/tool-execute-message.test.tsx`.
**How:** Extend the existing React Testing Library fixtures with an explicit
zero-output case and keep the current output-state assertions.
- **What:** Desktop chat does not expose an empty output affordance, while a
real-output command still fetches only after expansion.
**File:** `apps/web/e2e/tests/chat/tool-execute-output.spec.ts`.
**How:** Seed a shell message with an empty projected output summary, assert
the command/status remain visible and the disclosure/request are absent, then
retain the existing lazy completed-output scenario.
- **What:** Mobile chat has the same no-affordance behavior and preserves the
existing long-command/output path inside the viewport.
**File:** `apps/web/e2e/tests/chat/mobile-tool-execute-output.spec.ts`.
**How:** Add a no-output seeded command assertion alongside the existing
mobile output scenario.
## E2E Tests
- **Scenario:** GIVEN a projected shell command with no retained output, WHEN
desktop chat renders it, THEN the command/status are visible and no output
disclosure or shell-output request exists.
**File:** `apps/web/e2e/tests/chat/tool-execute-output.spec.ts`.
- **Scenario:** GIVEN the same no-output command on the mobile chat surface,
WHEN the transcript renders, THEN the empty disclosure is absent without
changing the existing mobile command wrapping or output behavior.
**File:** `apps/web/e2e/tests/chat/mobile-tool-execute-output.spec.ts`.
## Verification Results
- `cd apps && pnpm install --frozen-lockfile` — passed.
- `cd apps && pnpm --filter @kandev/web test -- --run components/task/chat/messages/tool-execute-message.test.tsx` — passed, 9 tests.
- `cd apps/web && pnpm run typecheck` — passed.
- `cd apps/web && pnpm e2e:run --project chromium tests/chat/tool-execute-output.spec.ts` — passed, 4 tests.
- `cd apps/web && pnpm e2e:run --project mobile-chrome tests/chat/mobile-tool-execute-output.spec.ts` — passed, 2 tests.
- `pnpm exec prettier --check` on the changed TypeScript files — passed.
- `git diff --check` — passed.
- Managed desktop/mobile screenshot captures with synthetic data — passed; compressed assets are listed in `.pr-assets/manifest.json` and are not committed to the PR branch.
## Implementation Waves
Wave 1 (sequential):
- [x] [task-01-gate-empty-output-disclosure](task-01-gate-empty-output-disclosure.md) (done)
## Risks
- Older or partially populated summaries may omit `has_output`; positive byte
counts must still keep the disclosure visible so valid output is not hidden.
- No new mobile interaction pattern is introduced; the only mobile change is
removing an unusable control when there is no output body.
## Open Questions
None.
@@ -0,0 +1,78 @@
---
id: "01-gate-empty-output-disclosure"
title: "Gate empty output disclosure"
status: done
wave: 1
depends_on: []
plan: "plan.md"
spec: "../../specs/ui/acp-shell-command-output.md"
---
# Task 01: Gate empty output disclosure
## Acceptance
- Shell command rows with `has_output: false` and zero stdout/stderr byte
counts show the command, working directory, and status without a `Show
command output` control or an on-demand output request.
- Shell command rows with `has_output: true` or positive byte counts preserve
the existing collapsed disclosure and fetch output only after expansion.
- Desktop and mobile E2E coverage proves the no-output path while retaining
the existing real-output and viewport behavior.
## Verification
```bash
cd apps && pnpm install --frozen-lockfile
cd apps && pnpm --filter @kandev/web test -- --run components/task/chat/messages/tool-execute-message.test.tsx
cd apps/web && pnpm run typecheck
cd apps/web && pnpm e2e:run --project chromium tests/chat/tool-execute-output.spec.ts
cd apps/web && pnpm e2e:run --project mobile-chrome tests/chat/mobile-tool-execute-output.spec.ts
```
## Files likely touched
- `apps/web/components/task/chat/messages/tool-execute-message.tsx`
- `apps/web/components/task/chat/messages/tool-execute-message.test.tsx`
- `apps/web/e2e/tests/chat/tool-execute-output.spec.ts`
- `apps/web/e2e/tests/chat/mobile-tool-execute-output.spec.ts`
- `docs/plans/shell-output-disclosure-visibility/plan.md`
## Dependencies
None.
## Parallelism
Sequential. Production code, component coverage, and desktop/mobile coverage
share the same summary predicate and must be verified together.
## Inputs
- `docs/specs/ui/acp-shell-command-output.md`, especially the disclosure and
no-output scenarios.
- `docs/plans/shell-output-disclosure-visibility/plan.md`.
- Existing `ShellOutputDisclosure` lazy-fetch behavior and the current desktop
and mobile shell-output fixtures.
- Mobile parity contract in the plan: reuse the shared command row and make no
new drawer or responsive surface.
## Output contract
Report the root cause addressed, files changed, the failing-then-passing
component regression, exact desktop/mobile E2E and typecheck results, any
artifacts or blockers, and synchronized task/plan status.
## Results
Implemented the shared command-row guard in `ToolExecuteMessage`. The output
disclosure now mounts only when `has_output` is true or a projected stdout or
stderr byte count is positive. Zero-output commands retain their command,
working directory, and status while never mounting the lazy output hook.
The component regression was first run red against the unguarded row, then
passed after the guard and fixture updates (9 tests). The desktop E2E suite
passed all 4 tests, and the mobile E2E suite passed both tests. Web typecheck,
Prettier, and `git diff --check` also passed. Fresh synthetic desktop and
mobile screenshots were captured, inspected, compressed, and recorded in the
ignored PR asset manifest; no production artifacts or blockers remain.
+3 -1
View File
@@ -1,7 +1,7 @@
---
status: shipped
created: 2026-07-14
updated: 2026-07-16
updated: 2026-08-07
owner: cfl
---
@@ -27,6 +27,7 @@ Agent chat shows that a shell command ran, but ACP agents encode terminal output
- Each normalized output text field is bounded to 256 KiB. When a field exceeds the bound, Kandev retains its most recent valid UTF-8 content and sets `truncated: true`.
- The full normalized command is always visible and wraps instead of truncating. The message content remains its fallback when a normalized command is absent. The working directory, when present, remains visible with the command.
- Stdout and stderr render in a separate disclosure that is collapsed by default for both running and completed commands. Running commands never auto-expand their output.
- The output disclosure is offered only when the browser-facing summary indicates retained stdout or stderr (`has_output: true` or a positive stdout/stderr byte count). A command with no retained output shows its command, working directory, and status without an expandable output control.
- Expanding the disclosure fetches the latest output snapshot on demand. A collapsed disclosure does not fetch or mount the output body.
- While an expanded command is running, Kandev refreshes its output with non-overlapping polling until the command becomes complete, failed, or cancelled. Polling stops immediately when the disclosure collapses or unmounts.
- The expanded disclosure shows combined output in a scrollable monospace region. On terminal completion it visibly shows `Exit code N` when known, or `Exit code unavailable` when unknown. Unknown is neutral, not success or failure.
@@ -138,6 +139,7 @@ Live output updates use the existing tool-message update path and are persisted
- **GIVEN** terminal output exceeds 256 KiB, **WHEN** live and final updates are normalized, **THEN** stored output remains within the bound, keeps the newest valid UTF-8 text, and the expanded row indicates truncation.
- **GIVEN** a persisted shell message with a long command, **WHEN** chat opens on desktop or mobile, **THEN** the complete command wraps visibly while its output disclosure remains collapsed.
- **GIVEN** a collapsed shell command with persisted output, **WHEN** task boot state, message REST/WS lists, and live message notifications reach the browser, **THEN** they contain byte counts and result metadata but no stdout or stderr body, and no output endpoint request occurs.
- **GIVEN** a shell command whose browser-facing summary has `has_output: false`, `stdout_bytes: 0`, and `stderr_bytes: 0`, **WHEN** its command row renders on desktop or mobile, **THEN** the command, working directory, and status remain visible but no output disclosure is offered and no shell-output request can be initiated.
- **GIVEN** a persisted completed shell message, **WHEN** the user expands its output disclosure, **THEN** exactly one on-demand snapshot is fetched and its output, truncation, and exit status are readable without overlapping adjacent chat content.
- **GIVEN** a running shell message with its disclosure expanded, **WHEN** output changes and the command later completes, **THEN** bounded non-overlapping polling refreshes the transcript and stops after the terminal snapshot.
- **GIVEN** an expanded running shell message, **WHEN** the user collapses it or navigates away, **THEN** scheduled polling stops and an in-flight request cannot update the unmounted disclosure.