fix(acp): strip command echo when terminal resolves a relative path to absolute (#1975)

apps/backend/cmd/mock-agent/script.go

@@ -222,12 +222,20 @@ func executeSimulatedToolResult(e *emitter, line string) {
 // the actual output - the ACP shell-output normalizer must strip that
 // leading echo so the transcript's command header and Output disclosure
 // don't show the command twice.
-// Format: e2e:shell_result("<command>", "<stdout>")
+// Format: e2e:shell_result("<command>", "<stdout>"[, "<cwd>"])
 //
-// rawInputCommandKey/rawOutputKey mirror the "command"/"output" keys a real
-// shell provider's tool_call/tool_call_update JSON uses.
+// The optional third argument reproduces a provider that resolves a
+// relative file-path argument in the command to an absolute (cwd-joined)
+// one before actually invoking the real terminal, while the reported
+// "command" keeps the original relative form - the normalizer's workDir-
+// aware echo stripping must still recognize that echo.
+//
+// rawInputCommandKey/rawInputCwdKey/rawOutputKey mirror the
+// "command"/"cwd"/"output" keys a real shell provider's
+// tool_call/tool_call_update JSON uses.
 const (
 	rawInputCommandKey = "command"
+	rawInputCwdKey     = "cwd"
 	rawOutputKey       = "output"
 )
 
@@ -235,10 +243,17 @@ func executeSimulatedShellResult(e *emitter, line string) {
 	inner := extractParenContent(line, "e2e:shell_result(")
 	command, rest := extractFirstStringArg(inner)
 	rest = strings.TrimPrefix(strings.TrimSpace(rest), ",")
-	stdout, _ := extractFirstStringArg(strings.TrimSpace(rest))
+	stdout, rest := extractFirstStringArg(strings.TrimSpace(rest))
+	rest = strings.TrimPrefix(strings.TrimSpace(rest), ",")
+	cwd, _ := extractFirstStringArg(strings.TrimSpace(rest))
+
+	rawInput := map[string]any{rawInputCommandKey: command}
+	if cwd != "" {
+		rawInput[rawInputCwdKey] = cwd
+	}
 
 	toolID := nextToolID()
-	e.startTool(toolID, command, acp.ToolKindExecute, map[string]any{rawInputCommandKey: command})
+	e.startTool(toolID, command, acp.ToolKindExecute, rawInput)
 	e.completeTool(toolID, map[string]any{rawOutputKey: stdout})
 }
 

apps/backend/cmd/mock-agent/script_test.go

@@ -504,6 +504,59 @@ func TestExecuteScriptShellResult(t *testing.T) {
 	}
 }
 
+// TestExecuteScriptShellResultWithCwd verifies the optional third
+// e2e:shell_result argument populates rawInput["cwd"], reproducing a
+// provider that resolves a relative file-path argument in the command to
+// an absolute (cwd-joined) one before actually invoking the real terminal
+// (see the ACP shell-output normalizer's workDir-aware echo stripping in
+// shell_output.go).
+func TestExecuteScriptShellResultWithCwd(t *testing.T) {
+	e, mock := newTestEmitter()
+	resetState()
+
+	executeScript(e, "", `e2e:shell_result("cat notes.txt", "$ cat /work/notes.txt\nhello\n", "/work")`)
+
+	updates := mock.getUpdates()
+	if len(updates) != 2 {
+		t.Fatalf("expected 2 updates, got %d", len(updates))
+	}
+
+	tc := updates[0].notification.Update.ToolCall
+	input, ok := tc.RawInput.(map[string]any)
+	if !ok || input["command"] != "cat notes.txt" || input["cwd"] != "/work" {
+		t.Errorf("raw input = %#v, want command %q and cwd %q", tc.RawInput, "cat notes.txt", "/work")
+	}
+
+	tcu := updates[1].notification.Update.ToolCallUpdate
+	output, ok := tcu.RawOutput.(map[string]any)
+	if !ok || output["output"] != "$ cat /work/notes.txt\nhello\n" {
+		t.Errorf("raw output = %#v, want the echoed stdout", tcu.RawOutput)
+	}
+}
+
+// TestExecuteScriptShellResultWithoutCwdOmitsCwdKey confirms the
+// two-argument form (no cwd) keeps working exactly as before: rawInput
+// carries no "cwd" key at all.
+func TestExecuteScriptShellResultWithoutCwdOmitsCwdKey(t *testing.T) {
+	e, mock := newTestEmitter()
+	resetState()
+
+	executeScript(e, "", `e2e:shell_result("cat file.txt", "cat file.txt=== marker ===\n")`)
+
+	updates := mock.getUpdates()
+	if len(updates) != 2 {
+		t.Fatalf("expected 2 updates, got %d", len(updates))
+	}
+	tc := updates[0].notification.Update.ToolCall
+	input, ok := tc.RawInput.(map[string]any)
+	if !ok {
+		t.Fatalf("raw input = %#v, want a map", tc.RawInput)
+	}
+	if _, present := input["cwd"]; present {
+		t.Errorf("raw input = %#v, want no cwd key", tc.RawInput)
+	}
+}
+
 // TestExecuteScriptToolUseNoInput verifies tool_use with no input arg.
 func TestExecuteScriptToolUseNoInput(t *testing.T) {
 	e, mock := newTestEmitter()

apps/backend/internal/agentctl/server/adapter/transport/acp/shell_output.go

@@ -28,7 +28,7 @@ func applyFinalShellResult(payload *streams.ShellExecPayload, result any) {
 	}
 	output := ensureShellOutput(payload)
 	if normalized.hasStdout {
-		output.Stdout = stripLeadingCommandEcho(payload.Command, normalized.stdout)
+		output.Stdout = stripLeadingCommandEcho(payload.Command, payload.WorkDir, normalized.stdout)
 		output.StdoutTruncated = normalized.stdoutTruncated
 	}
 	if normalized.hasStderr {
@@ -56,8 +56,8 @@ func applyFinalShellResult(payload *streams.ShellExecPayload, result any) {
 // Used for final/terminal results, where the buffer is known to be complete:
 // a stdout consisting of nothing but the echoed command commits to an empty
 // result rather than leaving the redundant echo visible.
-func stripLeadingCommandEcho(command, stdout string) string {
-	return stripCommandEcho(command, stdout, true)
+func stripLeadingCommandEcho(command, workDir, stdout string) string {
+	return stripCommandEcho(command, workDir, stdout, true)
 }
 
 // stripPendingCommandEcho is the live-update counterpart of
@@ -67,18 +67,54 @@ func stripLeadingCommandEcho(command, stdout string) string {
 // currently equals nothing but the echo is left untouched - the separator or
 // real output may still be in flight - instead of collapsing to empty and
 // orphaning a later chunk's leading newline.
-func stripPendingCommandEcho(command, stdout string) string {
-	return stripCommandEcho(command, stdout, false)
+func stripPendingCommandEcho(command, workDir, stdout string) string {
+	return stripCommandEcho(command, workDir, stdout, false)
 }
 
-func stripCommandEcho(command, stdout string, commitExactMatch bool) string {
+func stripCommandEcho(command, workDir, stdout string, commitExactMatch bool) string {
 	if command == "" || stdout == "" {
 		return stdout
 	}
-	rest, ok := strings.CutPrefix(stdout, "$ "+command)
-	if !ok {
+	if rest, ok := strings.CutPrefix(stdout, "$ "+command); ok {
+		return finishCommandEchoStrip(rest, stdout, commitExactMatch)
+	}
+	return stripCommandEchoWithWorkDir(command, workDir, stdout, commitExactMatch)
+}
+
+// stripCommandEchoWithWorkDir is stripCommandEcho's fallback for providers
+// that resolve a relative file-path argument in the command to an absolute
+// (workDir-joined) one before actually invoking the real terminal, while the
+// tool call's reported "command" field - the same text the chat header
+// renders - keeps the original relative form. The captured echo line then no
+// longer matches literally. Retry against just the first echoed line with
+// the workDir prefix collapsed back out, so real output later in stdout
+// (which may legitimately contain workDir-prefixed paths of its own) is
+// never touched.
+func stripCommandEchoWithWorkDir(command, workDir, stdout string, commitExactMatch bool) string {
+	if workDir == "" {
 		return stdout
 	}
+	// workDir's trailing slashes are collapsed to exactly one first: a
+	// provider can report cwd already "/"-terminated (e.g. "/repo/"), and
+	// the root directory "/" is inherently "/"-terminated. Appending "/"
+	// unconditionally would search for a doubled separator ("/repo//" or
+	// "//") that the actually-joined path never contains, silently
+	// declining to strip a real echo.
+	firstLine, remainder, hasMore := cutFirstLine(stdout)
+	workDirPrefix := strings.TrimRight(workDir, "/") + "/"
+	if strings.ReplaceAll(firstLine, workDirPrefix, "") != "$ "+command {
+		return stdout
+	}
+	if !hasMore {
+		if !commitExactMatch {
+			return stdout
+		}
+		return ""
+	}
+	return remainder
+}
+
+func finishCommandEchoStrip(rest, stdout string, commitExactMatch bool) string {
 	if rest == "" && !commitExactMatch {
 		return stdout
 	}
@@ -88,6 +124,16 @@ func stripCommandEcho(command, stdout string, commitExactMatch bool) string {
 	return strings.TrimPrefix(rest, "\n")
 }
 
+// cutFirstLine splits s at its first newline, reporting whether one was
+// found. The returned line excludes both the newline and any preceding "\r".
+func cutFirstLine(s string) (line, remainder string, hasMore bool) {
+	idx := strings.IndexByte(s, '\n')
+	if idx < 0 {
+		return s, "", false
+	}
+	return strings.TrimSuffix(s[:idx], "\r"), s[idx+1:], true
+}
+
 // NormalizeShellToolUpdate merges live and final ACP shell result fields into
 // the active normalized payload. It reports whether the update contained a
 // recognized shell output field so statusless updates can be persisted.
@@ -153,9 +199,9 @@ func (n *Normalizer) NormalizeShellToolUpdate(
 			// A definitive completion signal arrived, and Stdout hasn't
 			// already been committed-stripped this call: commit now, even
 			// if the buffer is nothing but the echo.
-			shell.Output.Stdout = stripLeadingCommandEcho(shell.Command, shell.Output.Stdout)
+			shell.Output.Stdout = stripLeadingCommandEcho(shell.Command, shell.WorkDir, shell.Output.Stdout)
 		case !isFinal:
-			shell.Output.Stdout = stripPendingCommandEcho(shell.Command, shell.Output.Stdout)
+			shell.Output.Stdout = stripPendingCommandEcho(shell.Command, shell.WorkDir, shell.Output.Stdout)
 		}
 	}
 

apps/backend/internal/agentctl/server/adapter/transport/acp/shell_output_test.go

@@ -365,6 +365,192 @@ func TestNormalizeShellToolResultStripsLeadingCommandEcho(t *testing.T) {
 	}
 }
 
+// TestNormalizeShellToolResultStripsLeadingCommandEchoWithWorkDirResolvedPath
+// is a regression for a real-world report: PR #1898's fix only strips the
+// echo when the captured terminal line matches the tool call's reported
+// "command" byte-for-byte. Some providers resolve a relative file-path
+// argument to an absolute one (workDir-joined) before actually invoking the
+// real terminal, while the "command" field reported on the tool call - the
+// same text the chat header renders - keeps the original relative form.
+// The literal prefix match then never fires, and the full echoed line
+// (with the resolved absolute path) leaks into the persisted Output.
+func TestNormalizeShellToolResultStripsLeadingCommandEchoWithWorkDirResolvedPath(t *testing.T) {
+	t.Parallel()
+
+	command := `grep -n "^const STRATEGY\|STRATEGY =" apps/web/components/task/chat/message-list.tsx`
+	workDir := "/home/clem/.kandev/tasks/new-message-line_0vc/kdlbs-kandev"
+	absolutePath := workDir + "/apps/web/components/task/chat/message-list.tsx"
+	resolvedCommand := strings.Replace(command, "apps/web/components/task/chat/message-list.tsx", absolutePath, 1)
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": command, "cwd": workDir},
+	})
+
+	normalizer.NormalizeToolResult(payload, "$ "+resolvedCommand+"\n24:const STRATEGY = \"native\";\n")
+
+	require.Equal(t, "24:const STRATEGY = \"native\";\n", payload.ShellExec().Output.Stdout)
+}
+
+// TestNormalizeShellToolUpdateStripsLeadingCommandEchoWithWorkDirResolvedPath
+// covers the same workDir-resolved-path mismatch on the live-update path
+// (terminal_output plus a terminal_exit completion, no rawOutput).
+func TestNormalizeShellToolUpdateStripsLeadingCommandEchoWithWorkDirResolvedPath(t *testing.T) {
+	t.Parallel()
+
+	command := "cat notes.txt"
+	workDir := "/repo"
+	resolvedCommand := "cat /repo/notes.txt"
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": command, "cwd": workDir},
+	})
+
+	normalizer.NormalizeShellToolUpdate(
+		payload,
+		map[string]any{
+			"terminal_output": map[string]any{"data": "$ " + resolvedCommand + "\nhello\n"},
+			"terminal_exit":   map[string]any{"exit_code": float64(0)},
+		},
+		nil,
+		nil,
+	)
+
+	require.Equal(t, "hello\n", payload.ShellExec().Output.Stdout)
+}
+
+// TestNormalizeShellToolUpdateWorkDirFallbackDefersPendingEchoWithoutNewline
+// covers the pending (non-final) counterpart of the workDir fallback: a
+// buffer that is exactly the resolved echo with no trailing newline yet
+// must be left untouched, since the real output (or even just the
+// separator newline) may still be in flight.
+func TestNormalizeShellToolUpdateWorkDirFallbackDefersPendingEchoWithoutNewline(t *testing.T) {
+	t.Parallel()
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": "cat notes.txt", "cwd": "/repo"},
+	})
+
+	normalizer.NormalizeShellToolUpdate(
+		payload,
+		map[string]any{"terminal_output": map[string]any{"data": "$ cat /repo/notes.txt"}},
+		nil,
+		nil,
+	)
+
+	require.Equal(t, "$ cat /repo/notes.txt", payload.ShellExec().Output.Stdout)
+}
+
+// TestNormalizeShellToolUpdateWorkDirFallbackCommitsPendingEchoWithNewline
+// covers the fallback's other pending-path branch: once a trailing newline
+// confirms the echo line is complete (hasMore == true), the strip commits
+// immediately even though commitExactMatch is false - the same behavior
+// finishCommandEchoStrip already applies once its literal-match "rest"
+// carries a newline.
+func TestNormalizeShellToolUpdateWorkDirFallbackCommitsPendingEchoWithNewline(t *testing.T) {
+	t.Parallel()
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": "cat notes.txt", "cwd": "/repo"},
+	})
+
+	normalizer.NormalizeShellToolUpdate(
+		payload,
+		map[string]any{"terminal_output": map[string]any{"data": "$ cat /repo/notes.txt\n"}},
+		nil,
+		nil,
+	)
+
+	require.Equal(t, "", payload.ShellExec().Output.Stdout)
+}
+
+// TestNormalizeShellToolResultWorkDirFallbackNeverCorruptsNearMissOutput is a
+// safety regression for the workDir-collapse fallback added alongside the
+// above test: ReplaceAll(firstLine, workDirPrefix, "") strips every
+// occurrence of that prefix from the echoed line, including one embedded in
+// a quoted argument that isn't a resolved file path (e.g. a grep pattern
+// that happens to contain workDir-looking text). If the collapsed line
+// still doesn't reconstruct "$ "+command exactly - because the real
+// terminal echo genuinely differs (different args, or the workDir text was
+// incidental rather than a resolved path) - the fallback must leave stdout
+// untouched rather than mangling legitimate output.
+func TestNormalizeShellToolResultWorkDirFallbackNeverCorruptsNearMissOutput(t *testing.T) {
+	t.Parallel()
+
+	command := `grep -n "/repo/notes" apps/foo.txt`
+	workDir := "/repo"
+	// The real terminal echo resolves the file argument to an absolute path
+	// (workDir-joined) AND happens to also carry a coincidental "/repo/"
+	// substring inside the quoted pattern - unrelated to path resolution.
+	// Collapsing every "/repo/" occurrence therefore also eats the pattern's
+	// own "/repo/" text, so the result can never exactly reconstruct
+	// "$ "+command: the fallback must decline to strip rather than guess.
+	stdout := `$ grep -n "/repo/notes" /repo/apps/foo.txt` + "\n1:a match\n"
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": command, "cwd": workDir},
+	})
+
+	normalizer.NormalizeToolResult(payload, stdout)
+
+	require.Equal(t, stdout, payload.ShellExec().Output.Stdout)
+}
+
+// TestNormalizeShellToolResultStripsLeadingCommandEchoWithTrailingSlashWorkDir
+// is a regression for a Codex review finding on this PR: a provider can
+// report "cwd" already "/"-terminated (e.g. "/repo/"). Appending "/"
+// unconditionally would then search for "/repo//" - a doubled separator
+// the actually-joined path ("/repo/notes.txt") never contains - silently
+// declining to strip a real echo.
+func TestNormalizeShellToolResultStripsLeadingCommandEchoWithTrailingSlashWorkDir(t *testing.T) {
+	t.Parallel()
+
+	command := "cat notes.txt"
+	workDir := "/repo/"
+	resolvedCommand := "cat /repo/notes.txt"
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": command, "cwd": workDir},
+	})
+
+	normalizer.NormalizeToolResult(payload, "$ "+resolvedCommand+"\nhello\n")
+
+	require.Equal(t, "hello\n", payload.ShellExec().Output.Stdout)
+}
+
+// TestNormalizeShellToolResultStripsLeadingCommandEchoWithRootWorkDir covers
+// the same Codex finding's other reported case: the root directory "/" is
+// itself already "/"-terminated, so the joined path is single- not
+// double-slashed ("/notes.txt", not "//notes.txt").
+func TestNormalizeShellToolResultStripsLeadingCommandEchoWithRootWorkDir(t *testing.T) {
+	t.Parallel()
+
+	command := "cat notes.txt"
+	workDir := "/"
+	resolvedCommand := "cat /notes.txt"
+
+	normalizer := NewNormalizer("")
+	payload := normalizer.NormalizeToolCall("execute", map[string]any{
+		"kind":      "execute",
+		"raw_input": map[string]any{"command": command, "cwd": workDir},
+	})
+
+	normalizer.NormalizeToolResult(payload, "$ "+resolvedCommand+"\nhello\n")
+
+	require.Equal(t, "hello\n", payload.ShellExec().Output.Stdout)
+}
+
 func TestNormalizeShellToolUpdateStripsLeadingCommandEchoFromLiveOutput(t *testing.T) {
 	t.Parallel()
 

apps/web/e2e/tests/chat/shell-output-echo.spec.ts

@@ -58,4 +58,63 @@ test.describe("shell command output echo stripping", () => {
     // disclosure - it's already shown once in the command row above.
     await expect(outputRegion).not.toContainText(command);
   });
+
+  test("does not repeat a workDir-resolved absolute-path echo inside the expanded Output disclosure", async ({
+    testPage,
+    apiClient,
+    seedData,
+  }) => {
+    // Some providers resolve a relative file-path argument in the command
+    // to an absolute (cwd-joined) one before actually invoking the real
+    // terminal, while the reported "command" - the same text the chat
+    // header renders - keeps the original relative form. The captured echo
+    // line then no longer matches the command byte-for-byte.
+    const command = "cat notes.txt";
+    const cwd = "/workspace/example-repo";
+    const resolvedCommand = `cat ${cwd}/notes.txt`;
+    const echoedOutput = `$ ${resolvedCommand}\nhello\n`;
+
+    const script = [
+      `e2e:shell_result("${command}", "${echoedOutput.replace(/\n/g, "\\n")}", "${cwd}")`,
+      'e2e:message("done")',
+    ].join("\n");
+
+    const task = await apiClient.createTaskWithAgent(
+      seedData.workspaceId,
+      "Shell echo workDir regression",
+      seedData.agentProfileId,
+      {
+        description: script,
+        workflow_id: seedData.workflowId,
+        workflow_step_id: seedData.startStepId,
+        repository_ids: [seedData.repositoryId],
+      },
+    );
+
+    await testPage.goto(`/t/${task.id}`);
+    const session = new SessionPage(testPage);
+    await session.waitForLoad();
+    await session.waitForChatIdle({ timeout: 30_000 });
+
+    const chat = session.activeChat();
+    const commandRow = chat.getByTestId("tool-execute-command").filter({ hasText: command });
+    await expect(commandRow).toBeVisible();
+    // The header keeps rendering the original relative command, unaffected
+    // by the provider's internal absolute-path resolution.
+    await expect(commandRow).toHaveText(command);
+
+    const disclosure = chat.getByRole("button", { name: "Show command output" });
+    const responsePromise = testPage.waitForResponse(
+      (response) => response.url().endsWith("/shell-output") && response.status() === 200,
+    );
+    await disclosure.click();
+    await responsePromise;
+
+    const outputRegion = chat.getByTestId("tool-execute-output");
+    await expect(outputRegion).toContainText("hello");
+    // Critical: neither the workDir-prefixed absolute path nor a repeat of
+    // the resolved command may leak into the Output disclosure.
+    await expect(outputRegion).not.toContainText(cwd);
+    await expect(outputRegion).not.toContainText(resolvedCommand);
+  });
 });