mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-04-28 19:52:02 +00:00
feat(cli): cap inline shell output with configurable line limit (#3508)
* feat(cli): cap inline shell output with configurable line limit
Long-running shell commands (npm install, find /, build logs) currently
fill the viewport with the full visible PTY buffer (up to availableHeight,
~24 lines on a typical terminal). The output dominates the screen and
pushes prior context off the top.
This caps inline ANSI shell output to a small window (default 5 lines,
matching Claude Code's ShellProgressMessage). The hidden line count is
already surfaced via the existing `+N lines` indicator in
`ShellStatsBar`, so users still know how much was elided.
The cap applies only when nothing in the existing escape-hatch set is
true:
- `forceShowResult` (errors, !-prefix user-initiated commands,
tools awaiting confirmation, agents pending confirmation)
- `isThisShellFocused` (ctrl+f focus on a running embedded PTY shell)
- `ui.shellOutputMaxLines = 0` (user opt-out)
Also adds a new `ui.shellOutputMaxLines` setting (default 5) so users
can adjust or disable the cap. The SettingsDialog renders it
automatically via the existing `type: 'number'` schema path.
Notes on scope:
- Only the `'ansi'` display branch is capped. `'string'`, `'diff'`,
`'todo'`, `'plan'`, `'task'` renderers are untouched.
- `AnsiOutputDisplay` is only produced by shell tools (`shell.ts`,
`shellCommandProcessor.ts`), so other tool outputs are unaffected.
- The `+N lines` count is bounded by the headless xterm buffer height
(~30 rows) — a pre-existing limitation of the buffer-based stats,
not introduced here.
Tests:
- 4 new ToolMessage tests cover cap default, forceShowResult bypass,
settings disable (cap=0), and custom cap value.
- The existing `MockAnsiOutputText` / `MockShellStatsBar` mocks were
extended to print `availableTerminalHeight` / `displayHeight` so
the cap behavior is asserted at the prop level.
* fix(cli): apply shell output cap to completed string display too
Initial PR caught only the streaming ANSI branch. AI shell tools emit
the final completed result through `shell.ts:returnDisplayMessage =
result.output`, which is a plain string. That string went through
`StringResultRenderer` with the unmodified `availableHeight`, so the
cap was effectively bypassed for the steady-state display the user
actually sees most of the time.
Verified manually in tmux: a `seq 1 30` invocation by the AI now
collapses to "first 26 lines hidden ... 27 28 29 30" instead of
listing all 30 rows. `!`-prefix `seq 1 30` still expands fully via
the existing `isUserInitiated → forceShowResult` bypass.
Changes:
- Detect shell tool by name (matches existing `SHELL_COMMAND_NAME` /
`SHELL_NAME` checks already used in this file)
- Rename `ansiAvailableHeight` → `shellCapHeight` since it now
governs the string branch as well
- Pass `shellCapHeight` to `StringResultRenderer`; the value
falls back to `availableHeight` for non-shell tools so other
tools' string output is unaffected
- Two new tests: shell completed string is capped; non-shell
string is not
- Two existing tests updated to use `name="Shell"` so they actually
exercise the cap path (would previously have passed by accident
since the original code didn't check tool name)
Also picks up the auto-regenerated VSCode IDE companion settings
schema entry for `ui.shellOutputMaxLines`.
* fix(cli): symmetrize ANSI/string row counts and clamp shell cap input
Addresses two non-blocking review observations on #3508.
Off-by-one between paths:
MaxSizedBox reserves one row for its overflow banner when content
exceeds maxHeight (visibleContentHeight = max - 1). The ANSI path
pre-slices to N in AnsiOutputText so MaxSizedBox sees exactly N
rows and renders all N — plus the separate ShellStatsBar line.
The string path passes the raw cap and lets MaxSizedBox handle
overflow, so it shows N-1 content rows + the banner.
Result with cap=5: ANSI showed 5+stats, string showed 4+banner.
Pass shellCapHeight + 1 to StringResultRenderer when capping so
both paths render N visible content rows. Verified in tmux: the
completed Shell tool box now reports `... first 25 lines hidden ...`
followed by lines 26-30 (was 26 + lines 27-30).
Setting validation:
Schema accepts any number; the dialog only rejects NaN. Negatives
silently disabled the cap (only 0 is documented as off) and
fractional values produced fractional slice counts. Added
Math.max(0, Math.floor(value || 0)) at the use site so:
- negatives → 0 → cap disabled (matches the documented opt-out)
- fractions → floor → whole-row cap
- non-numeric (raw settings.json edits) → 0 → cap disabled
Schema-level minimum/integer constraints aren't supported by the
current settings infrastructure (no other number setting uses
them either), so the guard lives at the use site.
Tests:
- Updated string-cap test to assert lines 26-30 visible (catches
the +1 fix; was lines 27-30 before)
- New parameterized test covers -1, 1.5, and a non-numeric value
This commit is contained in:
parent
0c423deedf
commit
d71f2fab70
5 changed files with 341 additions and 6 deletions
|
|
@ -38,9 +38,11 @@ vi.mock('../AnsiOutput.js', () => ({
|
|||
AnsiOutputText: function MockAnsiOutputText({
|
||||
data,
|
||||
maxWidth,
|
||||
availableTerminalHeight,
|
||||
}: {
|
||||
data: AnsiOutput;
|
||||
maxWidth: number;
|
||||
availableTerminalHeight?: number;
|
||||
}) {
|
||||
// Simple serialization for snapshot stability
|
||||
const serialized = data
|
||||
|
|
@ -48,12 +50,19 @@ vi.mock('../AnsiOutput.js', () => ({
|
|||
.join('\n');
|
||||
return (
|
||||
<Text>
|
||||
MockAnsiOutput:{serialized}:width={maxWidth}
|
||||
MockAnsiOutput:{serialized}:width={maxWidth}:height=
|
||||
{availableTerminalHeight ?? 'undef'}
|
||||
</Text>
|
||||
);
|
||||
},
|
||||
ShellStatsBar: function MockShellStatsBar() {
|
||||
return null;
|
||||
ShellStatsBar: function MockShellStatsBar({
|
||||
displayHeight,
|
||||
}: {
|
||||
displayHeight?: number;
|
||||
}) {
|
||||
return (
|
||||
<Text>MockShellStatsBar:displayHeight={displayHeight ?? 'undef'}</Text>
|
||||
);
|
||||
},
|
||||
}));
|
||||
|
||||
|
|
@ -331,6 +340,288 @@ describe('<ToolMessage />', () => {
|
|||
expect(lastFrame()).toContain('width=');
|
||||
});
|
||||
|
||||
it('caps shell ANSI output to default 5 lines when not forced', () => {
|
||||
const ansiOutputDisplay: AnsiOutputDisplay = {
|
||||
ansiOutput: [
|
||||
[
|
||||
{
|
||||
text: 'a',
|
||||
fg: '',
|
||||
bg: '',
|
||||
bold: false,
|
||||
italic: false,
|
||||
underline: false,
|
||||
dim: false,
|
||||
inverse: false,
|
||||
},
|
||||
],
|
||||
],
|
||||
totalLines: 50,
|
||||
};
|
||||
const { lastFrame } = renderWithContext(
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="Shell"
|
||||
resultDisplay={ansiOutputDisplay}
|
||||
availableTerminalHeight={100}
|
||||
/>,
|
||||
StreamingState.Idle,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
expect(output).toContain('height=5');
|
||||
expect(output).toContain('MockShellStatsBar:displayHeight=5');
|
||||
});
|
||||
|
||||
it('does not cap non-shell ANSI output', () => {
|
||||
const ansiOutputDisplay: AnsiOutputDisplay = {
|
||||
ansiOutput: [
|
||||
[
|
||||
{
|
||||
text: 'a',
|
||||
fg: '',
|
||||
bg: '',
|
||||
bold: false,
|
||||
italic: false,
|
||||
underline: false,
|
||||
dim: false,
|
||||
inverse: false,
|
||||
},
|
||||
],
|
||||
],
|
||||
totalLines: 50,
|
||||
};
|
||||
const { lastFrame } = renderWithContext(
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="some-other-tool"
|
||||
resultDisplay={ansiOutputDisplay}
|
||||
availableTerminalHeight={100}
|
||||
/>,
|
||||
StreamingState.Idle,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
// availableHeight = 100 - STATIC_HEIGHT(1) - RESERVED_LINE_COUNT(5) = 94
|
||||
expect(output).toContain('height=94');
|
||||
});
|
||||
|
||||
it('bypasses cap when forceShowResult is true', () => {
|
||||
const ansiOutputDisplay: AnsiOutputDisplay = {
|
||||
ansiOutput: [
|
||||
[
|
||||
{
|
||||
text: 'a',
|
||||
fg: '',
|
||||
bg: '',
|
||||
bold: false,
|
||||
italic: false,
|
||||
underline: false,
|
||||
dim: false,
|
||||
inverse: false,
|
||||
},
|
||||
],
|
||||
],
|
||||
totalLines: 50,
|
||||
};
|
||||
const { lastFrame } = renderWithContext(
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="Shell"
|
||||
resultDisplay={ansiOutputDisplay}
|
||||
availableTerminalHeight={100}
|
||||
forceShowResult={true}
|
||||
/>,
|
||||
StreamingState.Idle,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
// availableHeight = 100 - STATIC_HEIGHT(1) - RESERVED_LINE_COUNT(5) = 94
|
||||
expect(output).toContain('height=94');
|
||||
});
|
||||
|
||||
it('disables cap when ui.shellOutputMaxLines is 0', () => {
|
||||
const ansiOutputDisplay: AnsiOutputDisplay = {
|
||||
ansiOutput: [
|
||||
[
|
||||
{
|
||||
text: 'a',
|
||||
fg: '',
|
||||
bg: '',
|
||||
bold: false,
|
||||
italic: false,
|
||||
underline: false,
|
||||
dim: false,
|
||||
inverse: false,
|
||||
},
|
||||
],
|
||||
],
|
||||
totalLines: 50,
|
||||
};
|
||||
const settingsWithDisabledCap = {
|
||||
merged: { ui: { shellOutputMaxLines: 0 } },
|
||||
} as unknown as LoadedSettings;
|
||||
const { lastFrame } = render(
|
||||
<CompactModeProvider value={{ compactMode: false }}>
|
||||
<SettingsContext.Provider value={settingsWithDisabledCap}>
|
||||
<StreamingContext.Provider value={StreamingState.Idle}>
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="Shell"
|
||||
resultDisplay={ansiOutputDisplay}
|
||||
availableTerminalHeight={100}
|
||||
/>
|
||||
</StreamingContext.Provider>
|
||||
</SettingsContext.Provider>
|
||||
</CompactModeProvider>,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
expect(output).toContain('height=94');
|
||||
});
|
||||
|
||||
it('respects user-configured cap value', () => {
|
||||
const ansiOutputDisplay: AnsiOutputDisplay = {
|
||||
ansiOutput: [
|
||||
[
|
||||
{
|
||||
text: 'a',
|
||||
fg: '',
|
||||
bg: '',
|
||||
bold: false,
|
||||
italic: false,
|
||||
underline: false,
|
||||
dim: false,
|
||||
inverse: false,
|
||||
},
|
||||
],
|
||||
],
|
||||
totalLines: 50,
|
||||
};
|
||||
const settingsWithCustomCap = {
|
||||
merged: { ui: { shellOutputMaxLines: 12 } },
|
||||
} as unknown as LoadedSettings;
|
||||
const { lastFrame } = render(
|
||||
<CompactModeProvider value={{ compactMode: false }}>
|
||||
<SettingsContext.Provider value={settingsWithCustomCap}>
|
||||
<StreamingContext.Provider value={StreamingState.Idle}>
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="Shell"
|
||||
resultDisplay={ansiOutputDisplay}
|
||||
availableTerminalHeight={100}
|
||||
/>
|
||||
</StreamingContext.Provider>
|
||||
</SettingsContext.Provider>
|
||||
</CompactModeProvider>,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
expect(output).toContain('height=12');
|
||||
});
|
||||
|
||||
it('caps shell completed string output (returnDisplayMessage path)', () => {
|
||||
// shell.ts emits the final result as a plain string via
|
||||
// `returnDisplayMessage = result.output`, so the completed shell
|
||||
// tool flows through StringResultRenderer, not the ANSI branch.
|
||||
// The cap must still apply.
|
||||
const longString = Array.from(
|
||||
{ length: 30 },
|
||||
(_, i) => `line ${i + 1}`,
|
||||
).join('\n');
|
||||
const { lastFrame } = renderWithContext(
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="Shell"
|
||||
resultDisplay={longString}
|
||||
status={ToolCallStatus.Success}
|
||||
availableTerminalHeight={100}
|
||||
/>,
|
||||
StreamingState.Idle,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
// With cap=5, the string path should show the last 5 content rows
|
||||
// (the +1 height compensates for MaxSizedBox's overflow banner row,
|
||||
// matching the ANSI path's 5 content rows + stats bar).
|
||||
expect(output).not.toContain('line 1\n');
|
||||
expect(output).not.toContain('line 10');
|
||||
expect(output).toContain('line 26');
|
||||
expect(output).toContain('line 27');
|
||||
expect(output).toContain('line 28');
|
||||
expect(output).toContain('line 29');
|
||||
expect(output).toContain('line 30');
|
||||
});
|
||||
|
||||
it.each([
|
||||
['negative', -1],
|
||||
['fractional', 1.5],
|
||||
['NaN-via-string', 'abc' as unknown as number],
|
||||
])('clamps %s shellOutputMaxLines to a safe value', (_label, badValue) => {
|
||||
const ansiOutputDisplay: AnsiOutputDisplay = {
|
||||
ansiOutput: [
|
||||
[
|
||||
{
|
||||
text: 'a',
|
||||
fg: '',
|
||||
bg: '',
|
||||
bold: false,
|
||||
italic: false,
|
||||
underline: false,
|
||||
dim: false,
|
||||
inverse: false,
|
||||
},
|
||||
],
|
||||
],
|
||||
totalLines: 50,
|
||||
};
|
||||
const settingsWithBadCap = {
|
||||
merged: { ui: { shellOutputMaxLines: badValue } },
|
||||
} as unknown as LoadedSettings;
|
||||
const { lastFrame } = render(
|
||||
<CompactModeProvider value={{ compactMode: false }}>
|
||||
<SettingsContext.Provider value={settingsWithBadCap}>
|
||||
<StreamingContext.Provider value={StreamingState.Idle}>
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="Shell"
|
||||
resultDisplay={ansiOutputDisplay}
|
||||
availableTerminalHeight={100}
|
||||
/>
|
||||
</StreamingContext.Provider>
|
||||
</SettingsContext.Provider>
|
||||
</CompactModeProvider>,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
// -1 → 0 → cap disabled (height=94)
|
||||
// 1.5 → 1 → cap to 1 (height=1)
|
||||
// 'abc' → NaN → 0 → cap disabled (height=94)
|
||||
if (
|
||||
typeof badValue === 'number' &&
|
||||
Number.isFinite(badValue) &&
|
||||
badValue > 0
|
||||
) {
|
||||
expect(output).toContain(`height=${Math.floor(badValue)}`);
|
||||
} else {
|
||||
expect(output).toContain('height=94');
|
||||
}
|
||||
});
|
||||
|
||||
it('does not cap non-shell string output', () => {
|
||||
const longString = Array.from(
|
||||
{ length: 30 },
|
||||
(_, i) => `line ${i + 1}`,
|
||||
).join('\n');
|
||||
const { lastFrame } = renderWithContext(
|
||||
<ToolMessage
|
||||
{...baseProps}
|
||||
name="some-other-tool"
|
||||
resultDisplay={longString}
|
||||
status={ToolCallStatus.Success}
|
||||
availableTerminalHeight={100}
|
||||
/>,
|
||||
StreamingState.Idle,
|
||||
);
|
||||
const output = lastFrame()!;
|
||||
// availableHeight = 94, well above 30 lines → all visible
|
||||
expect(output).toContain('line 1');
|
||||
expect(output).toContain('line 30');
|
||||
});
|
||||
|
||||
it('renders rejected plan content with plan text still visible', () => {
|
||||
const planResultDisplay = {
|
||||
type: 'plan_summary' as const,
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ import { ToolElapsedTime } from '../shared/ToolElapsedTime.js';
|
|||
const STATIC_HEIGHT = 1;
|
||||
const RESERVED_LINE_COUNT = 5; // for tool name, status, padding etc.
|
||||
const MIN_LINES_SHOWN = 2; // show at least this many lines
|
||||
const DEFAULT_SHELL_OUTPUT_MAX_LINES = 5;
|
||||
|
||||
// Large threshold to ensure we don't cause performance issues for very large
|
||||
// outputs that will get truncated further MaxSizedBox anyway.
|
||||
|
|
@ -345,6 +346,33 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
|||
MIN_LINES_SHOWN + 1, // enforce minimum lines shown
|
||||
)
|
||||
: undefined;
|
||||
// Cap inline shell output. Applies to both the streaming ANSI display and
|
||||
// the completed string display (shell.ts emits the final result as a plain
|
||||
// string via `returnDisplayMessage = result.output`). ShellStatsBar surfaces
|
||||
// hidden lines via `+N lines` for ANSI; MaxSizedBox handles overflow for string.
|
||||
const isShellTool = name === SHELL_COMMAND_NAME || name === SHELL_NAME;
|
||||
const rawShellCap =
|
||||
settings.merged.ui?.shellOutputMaxLines ?? DEFAULT_SHELL_OUTPUT_MAX_LINES;
|
||||
// Defensive: clamp non-negative integers; treat negatives / NaN / fractions
|
||||
// as the user's clear intent (0 = disable, otherwise floor to whole rows).
|
||||
const shellOutputMaxLines = Math.max(0, Math.floor(rawShellCap || 0));
|
||||
const isCappingShell =
|
||||
isShellTool &&
|
||||
shellOutputMaxLines > 0 &&
|
||||
!forceShowResult &&
|
||||
!isThisShellFocused;
|
||||
const shellCapHeight = isCappingShell
|
||||
? Math.min(availableHeight ?? shellOutputMaxLines, shellOutputMaxLines)
|
||||
: availableHeight;
|
||||
// String path: MaxSizedBox reserves one row for its overflow banner when
|
||||
// content overflows (see MaxSizedBox.tsx visibleContentHeight = max - 1),
|
||||
// so passing the bare cap shows N-1 content rows. ANSI pre-slices to N
|
||||
// (no MaxSizedBox overflow) and renders N rows + the ShellStatsBar line.
|
||||
// +1 keeps the two paths visually symmetric at N visible content rows.
|
||||
const shellStringCapHeight =
|
||||
isCappingShell && shellCapHeight !== undefined
|
||||
? shellCapHeight + 1
|
||||
: availableHeight;
|
||||
const innerWidth = contentWidth - STATUS_INDICATOR_WIDTH;
|
||||
|
||||
// Long tool call response in MarkdownDisplay doesn't respect availableTerminalHeight properly,
|
||||
|
|
@ -420,13 +448,13 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
|||
<>
|
||||
<AnsiOutputText
|
||||
data={effectiveDisplayRenderer.data}
|
||||
availableTerminalHeight={availableHeight}
|
||||
availableTerminalHeight={shellCapHeight}
|
||||
maxWidth={innerWidth}
|
||||
/>
|
||||
{effectiveDisplayRenderer.stats && (
|
||||
<ShellStatsBar
|
||||
{...effectiveDisplayRenderer.stats}
|
||||
displayHeight={availableHeight}
|
||||
displayHeight={shellCapHeight}
|
||||
/>
|
||||
)}
|
||||
</>
|
||||
|
|
@ -435,7 +463,7 @@ export const ToolMessage: React.FC<ToolMessageProps> = ({
|
|||
<StringResultRenderer
|
||||
data={effectiveDisplayRenderer.data}
|
||||
renderAsMarkdown={renderOutputAsMarkdown}
|
||||
availableHeight={availableHeight}
|
||||
availableHeight={shellStringCapHeight}
|
||||
childWidth={innerWidth}
|
||||
/>
|
||||
)}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue