From 6a1ea9bf27cf95fa28fe9a72f1be77bd0f8503d1 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 4 Jul 2026 19:40:08 +0000 Subject: [PATCH] fix(tool): preserved bash pipeline output Removed the bash tool execution-path rewrite that stripped trailing head/tail pipeline stages before running commands. Added regression coverage for short-reading final pipeline stages. Fixes #4562 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/config/settings-schema.ts | 12 ----- packages/coding-agent/src/tools/bash.ts | 11 ----- .../test/bash-failure-result.test.ts | 19 +++++++- .../test/tools/bash-interceptor.test.ts | 44 ------------------- 5 files changed, 18 insertions(+), 69 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 9c2395e3d..5a17a50e5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixed +- Fixed bash tool pipeline execution preserving stale upstream output when the final stage was a stripped `head`/`tail` limiter; the tool now runs the command as written so `seq 1 5 | head -n2` returns only `1` and `2`. ([#4562](https://github.com/can1357/oh-my-pi/issues/4562)) - Fixed raw `read` ranges not contributing to edit seen-line provenance, so re-reading an anchor range with `:raw` now unblocks hashline edits without adding non-raw line prefixes. - Fixed replan-driven session title refresh updating the statusline but not the terminal window title: terminal-title updates now fire from the session-name-changed listener, so every `setSessionName` path (first-input titling, `/rename`, plan seeding, replan refresh) sets the OSC title consistently. - Fixed user-interrupt aborts rendering the persisted `Interrupted by user` label in assistant transcripts; replay and live views now suppress that redundant line again while preserving generic/custom abort labels. diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index 2316034da..5fb3e38e7 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -3166,18 +3166,6 @@ export const SETTINGS_SCHEMA = { }, "bashInterceptor.patterns": { type: "array", default: DEFAULT_BASH_INTERCEPTOR_RULES }, - "bash.stripTrailingHeadTail": { - type: "boolean", - default: true, - ui: { - tab: "shell", - group: "Bash", - label: "Strip head/tail Pipes", - description: - "Silently drop trailing `| head`/`| tail` pipes from single-line bash commands. Output is already truncated automatically.", - }, - }, - // Shell output minimizer "shellMinimizer.enabled": { type: "boolean", diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index a349c0a34..a737c6944 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -23,7 +23,6 @@ import { CachedOutputBlock, markFramedBlockComponent, outputBlockContentWidth } import { getSixelLineMask } from "../utils/sixel"; import type { ToolSession } from "."; import { truncateForPrompt } from "./approval"; -import { applyBashFixups } from "./bash-command-fixup"; import { type BashInteractiveResult, runInteractiveBashPty } from "./bash-interactive"; import { checkBashInterception } from "./bash-interceptor"; import { canUseInteractiveBashPty } from "./bash-pty-selection"; @@ -684,16 +683,6 @@ export class BashTool implements AgentTool&1`). The helper is single-line only and refuses anything that could - // change semantics. - if (this.session.settings.get("bash.stripTrailingHeadTail")) { - const fixup = applyBashFixups(command); - if (fixup.stripped.length > 0) { - command = fixup.command; - } - } - // Extract leading `cd && ...` into cwd when the model ignores the cwd parameter. // Constrained to a single line so a `&&` that sits on a later line of a multiline // script can't pull the entire script into the "cwd" capture. diff --git a/packages/coding-agent/test/bash-failure-result.test.ts b/packages/coding-agent/test/bash-failure-result.test.ts index 51bce014e..057ab687d 100644 --- a/packages/coding-agent/test/bash-failure-result.test.ts +++ b/packages/coding-agent/test/bash-failure-result.test.ts @@ -14,7 +14,6 @@ function makeSession(): ToolSession { if (key === "bash.autoBackground.enabled") return false; if (key === "bash.autoBackground.thresholdMs") return 60_000; if (key === "bashInterceptor.enabled") return false; - if (key === "bash.stripTrailingHeadTail") return false; if (key === "astGrep.enabled") return false; if (key === "astEdit.enabled") return false; if (key === "grep.enabled") return false; @@ -29,7 +28,7 @@ function makeSession(): ToolSession { } as unknown as ToolSession; } -describe("BashTool non-zero exit", () => { +describe("BashTool execution results", () => { it("resolves with an error result carrying execution details instead of throwing", async () => { const tool = new BashTool(makeSession()); const result = await tool.execute("call-fail", { command: "exit 3" }); @@ -56,4 +55,20 @@ describe("BashTool non-zero exit", () => { expect(text).toContain("hi"); expect(text).not.toContain("Command exited with code"); }); + + it("preserves final-stage output when a pipeline ends in head or tail", async () => { + const tool = new BashTool(makeSession()); + + for (const scenario of [ + { command: "seq 1 5 | head -n2", expected: "1\n2" }, + { command: "seq 1 5 | tail -n2", expected: "4\n5" }, + ]) { + const result = await tool.execute(`call-pipeline-${scenario.expected[0]}`, { command: scenario.command }); + const text = result.content.find(c => c.type === "text")?.text ?? ""; + const stdout = text.replace(/\n\nWall time: \d+\.\d{2} seconds$/, "").trimEnd(); + + expect(result.isError).toBeUndefined(); + expect(stdout).toBe(scenario.expected); + } + }); }); diff --git a/packages/coding-agent/test/tools/bash-interceptor.test.ts b/packages/coding-agent/test/tools/bash-interceptor.test.ts index ce4eec0f7..5b067aff8 100644 --- a/packages/coding-agent/test/tools/bash-interceptor.test.ts +++ b/packages/coding-agent/test/tools/bash-interceptor.test.ts @@ -126,47 +126,3 @@ describe("BashTool argument validation", () => { ); }); }); - -describe("BashTool head/tail stripping", () => { - function createBashToolWithStrip(stripEnabled: boolean): BashTool { - const session = { - cwd: process.cwd(), - settings: { - get(key: string) { - if (key === "bashInterceptor.enabled") return false; - if (key === "async.enabled") return false; - if (key === "bash.autoBackground.enabled") return false; - if (key === "bash.autoBackground.thresholdMs") return 60_000; - if (key === "bash.stripTrailingHeadTail") return stripEnabled; - return undefined; - }, - getBashInterceptorRules() { - return []; - }, - }, - } as unknown as ToolSession; - return new BashTool(session); - } - - it("executes the stripped command", async () => { - const tool = createBashToolWithStrip(true); - // `seq 1 100 | head -3` would emit "1\n2\n3"; stripped, it emits 1..100. - // We assert on the tail of the output rather than head, so a successful - // strip is observable: line "100" only appears when head is gone. - const result = await tool.execute("tool-call", { command: "seq 1 100 | head -3" }, undefined, undefined, { - toolNames: ["bash"], - } as AgentToolContext); - const text = result.content.find(b => b.type === "text")?.text ?? ""; - expect(text).toContain("100"); - }); - - it("does not strip when the setting is disabled", async () => { - const tool = createBashToolWithStrip(false); - const result = await tool.execute("tool-call", { command: "seq 1 100 | head -3" }, undefined, undefined, { - toolNames: ["bash"], - } as AgentToolContext); - const text = result.content.find(b => b.type === "text")?.text ?? ""; - expect(text).toContain("1\n2\n3"); - expect(text).not.toContain("100"); - }); -});