From ad1a4a653958da4e62be20bb34a3a3aed3089db3 Mon Sep 17 00:00:00 2001 From: Dax Date: Tue, 22 Sep 2026 11:47:07 -0400 Subject: [PATCH] fix(core): simplify shell output notices (#50676) --- packages/core/src/shell.ts | 6 +++-- packages/core/src/shell/result.ts | 12 ++++++--- packages/core/src/tool/plugin/shell.ts | 4 +-- packages/core/test/session-execution.test.ts | 4 +-- packages/core/test/session-shell.test.ts | 7 ++--- packages/core/test/tool-shell.test.ts | 27 +++++++++----------- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/packages/core/src/shell.ts b/packages/core/src/shell.ts index 6089863c7fb..d7a9125cc8c 100644 --- a/packages/core/src/shell.ts +++ b/packages/core/src/shell.ts @@ -239,8 +239,10 @@ const layer = () => if (page.output.endsWith("\n")) lines.pop() const truncated = latest.size > maxBytes || lines.length > maxLines const text = lines.length > maxLines ? lines.slice(-maxLines).join("\n") : page.output - const notice = truncated ? `\n\n[output truncated; full output saved to: ${info.file}]` : "" - return { output: `${text || "(no output)"}${notice}`, truncated } + const notice = truncated + ? `${text ? "\n\n" : ""}[full output saved to ${info.file}]` + : "" + return { output: `${text}${notice}`, truncated } }).pipe(Effect.catchTag("Shell.NotFoundError", () => Effect.succeed(undefined))) return { info, capture } }) diff --git a/packages/core/src/shell/result.ts b/packages/core/src/shell/result.ts index 8860486d9fa..ef7dfa5ed06 100644 --- a/packages/core/src/shell/result.ts +++ b/packages/core/src/shell/result.ts @@ -27,8 +27,8 @@ export function output(result: Result): Output { } export function notice(output: Pick) { - if (output.timeout) return "Command timed out before completion." - if (output.exit !== undefined) return `Command exited with code ${output.exit}.` + if (output.timeout) return "Timed out before completion" + if (output.exit !== undefined && output.exit !== 0) return `Exited with code ${output.exit}` } export function metadata(output: Output) { @@ -62,12 +62,16 @@ export function notification(input: { export function userNotification(result: Result) { const captured = output(result) const status = - result.info.status === "killed" ? "Command cancelled." : (notice(captured) ?? "Command exited with code unknown.") + result.info.status === "killed" + ? "Cancelled" + : captured.exit === 0 + ? undefined + : (notice(captured) ?? "Exited with code unknown") const message = notification({ shellID: result.info.id, command: result.info.command, state: result.info.status === "killed" ? "cancelled" : "completed", - text: `${captured.output}\n\n${status}`, + text: [captured.output, status].filter((item) => item !== undefined && item !== "").join("\n\n"), output: captured, }) return { ...message, text: `The following shell command was executed by the user:\n${message.text}` } diff --git a/packages/core/src/tool/plugin/shell.ts b/packages/core/src/tool/plugin/shell.ts index a5abb1d7cd4..df08402a9ef 100644 --- a/packages/core/src/tool/plugin/shell.ts +++ b/packages/core/src/tool/plugin/shell.ts @@ -76,7 +76,7 @@ type Output = typeof Output.Type const resultMessages = (output: Output) => { const notice = output.status === "running" ? BACKGROUND_INSTRUCTION : ShellResult.notice(output) - return [output.output, ...(notice ? [notice] : [])] + return [...(output.output ? [output.output] : []), ...(notice ? [notice] : [])] } const toolResult = (output: Output) => { @@ -165,7 +165,7 @@ export const Plugin = { ? resultMessages(output).join("\n\n") : info.status === "error" ? (info.error ?? "Command failed") - : "Command cancelled" + : "Cancelled" yield* sessions.synthetic({ ...(info.notificationID ? { id: info.notificationID } : {}), sessionID, diff --git a/packages/core/test/session-execution.test.ts b/packages/core/test/session-execution.test.ts index 9f838e66ba3..3b954dc80d5 100644 --- a/packages/core/test/session-execution.test.ts +++ b/packages/core/test/session-execution.test.ts @@ -507,7 +507,7 @@ describe("SessionRestart background recovery", () => { run: Deferred.await(complete), }) yield* jobs.background("call-completed-shell") - yield* Deferred.succeed(complete, "(no output)\n\nCommand exited with code 7.") + yield* Deferred.succeed(complete, "Exited with code 7") yield* jobs.wait({ id: "call-completed-shell" }) const scope = yield* Scope.make() @@ -529,7 +529,7 @@ describe("SessionRestart background recovery", () => { { type: "synthetic", payload: { - text: '\n(no output)\n\nCommand exited with code 7.\n', + text: '\nExited with code 7\n', }, }, ]) diff --git a/packages/core/test/session-shell.test.ts b/packages/core/test/session-shell.test.ts index f93b6aa21f7..1340c0f67ed 100644 --- a/packages/core/test/session-shell.test.ts +++ b/packages/core/test/session-shell.test.ts @@ -271,7 +271,8 @@ describe("Session.shell", () => { expect(completion.payload.text).toContain(command) expect(completion.payload.text).toContain("user output") expect(completion.payload.text).toContain("user error") - expect(completion.payload.text).toContain(`exited with code ${exit}`) + if (exit === 0) expect(completion.payload.text).not.toContain("Exited with code") + if (exit !== 0) expect(completion.payload.text).toContain(`Exited with code ${exit}`) expect(fixture.control.wakes).toEqual([]) }), ) @@ -281,10 +282,10 @@ describe("Session.shell", () => { { status: "killed", state: "cancelled", - text: "Command cancelled", + text: "Cancelled", output: "Shell command output is no longer available.", }, - { status: "timeout", state: "completed", text: "Command timed out", output: "timeout started" }, + { status: "timeout", state: "completed", text: "Timed out before completion", output: "timeout started" }, ]) { it.live(`records a ${outcome.status} shell and admits its completion without waking the model`, () => Effect.gen(function* () { diff --git a/packages/core/test/tool-shell.test.ts b/packages/core/test/tool-shell.test.ts index 1beaec19c86..a690161c8c8 100644 --- a/packages/core/test/tool-shell.test.ts +++ b/packages/core/test/tool-shell.test.ts @@ -436,7 +436,7 @@ describe("ShellTool scanner permissions", () => { value: { status: "completed", metadata: { exit: 0 }, - content: [{ type: "text", text: `${fixture.outside}\n` }, { type: "text" }], + content: [{ type: "text", text: `${fixture.outside}\n` }], }, }) expect(yield* Effect.promise(() => Bun.file(marker).text())).toBe("reached") @@ -470,7 +470,7 @@ describe("ShellTool scanner permissions", () => { value: { status: "completed", metadata: { exit: 0 }, - content: [{ type: "text", text: `${fixture.outside}\n` }, { type: "text" }], + content: [{ type: "text", text: `${fixture.outside}\n` }], }, }) expect(yield* Effect.promise(() => Bun.file(marker).text())).toBe("reached") @@ -517,7 +517,7 @@ describe("ShellTool scanner permissions", () => { value: { status: "completed", metadata: { exit: 0 }, - content: [{ type: "text", text: `${fixture.outside}\n` }, { type: "text" }], + content: [{ type: "text", text: `${fixture.outside}\n` }], }, }) expect(yield* Effect.promise(() => Bun.file(marker).text())).toBe("reached") @@ -661,7 +661,7 @@ describe("ShellTool compound syntax approval compatibility", () => { value: { status: "completed", metadata: { exit: 0 }, - content: [{ type: "text", text: fixture.output }, { type: "text" }], + content: [{ type: "text", text: fixture.output }], }, }) }), @@ -729,7 +729,7 @@ describe("ShellTool ordinary shell syntax", () => { value: { status: "completed", metadata: { exit: 0 }, - content: [{ type: "text", text: fixture.output }, { type: "text" }], + content: [{ type: "text", text: fixture.output }], }, }) }), @@ -907,10 +907,7 @@ describe("ShellTool", () => { const settled = yield* executeTool(registry, call({ command: helloCommand })) expect(settled.status).toBe("completed") expect(settled.metadata).toMatchObject({ exit: 0, truncated: false }) - expect(settled.content?.[0]).toEqual({ type: "text", text: "hello" }) - expect(settled.content?.[1]).toMatchObject( - Expected.text(expect.stringContaining("Command exited with code 0.")), - ) + expect(settled.content).toEqual([{ type: "text", text: "hello" }]) expect(assertions).toMatchObject([ { sessionID, @@ -1313,7 +1310,7 @@ describe("ShellTool", () => { expect(settled.metadata).toMatchObject({ exit: 7, truncated: false }) expect(settled.content?.[0]).toEqual({ type: "text", text: "body" }) expect(settled.content?.[1]).toMatchObject( - Expected.text(expect.stringContaining("Command exited with code 7")), + Expected.text(expect.stringContaining("Exited with code 7")), ) }), ), @@ -1342,7 +1339,7 @@ describe("ShellTool", () => { expect(content.text.includes("output-start")).toBe(false) expect(content.text.includes("output-end")).toBe(true) expect(content).toMatchObject( - Expected.text(expect.stringContaining("output truncated; full output saved to:")), + Expected.text(expect.stringContaining("full output saved to ")), ) }), ), @@ -1374,7 +1371,7 @@ describe("ShellTool", () => { expect(content.text).not.toContain("one") // Windows shells emit CRLF; the assertion targets line limits, not line endings. expect(content.text.replaceAll("\r\n", "\n")).toStartWith("two\nthree") - expect(content.text).toContain("output truncated; full output saved to:") + expect(content.text).toContain("full output saved to ") }) }, (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]().then(() => undefined)), @@ -1471,7 +1468,7 @@ describe("ShellTool", () => { expect((yield* shell.list()).map((info) => info.id)).toContain(id) expect((yield* shell.wait(id)).status).toBe("timeout") expect((yield* Fiber.join(admitted)).valueOrUndefined?.data.item.payload).toMatchObject({ - text: expect.stringContaining("Command timed out before completion."), + text: expect.stringContaining("Timed out before completion"), description: idleCommand, metadata: { source: "shell", @@ -1508,7 +1505,7 @@ describe("ShellTool", () => { const shellID = settled.metadata?.shellID expect(typeof shellID).toBe("string") expect((yield* Fiber.join(admitted)).valueOrUndefined?.data.item.payload).toMatchObject({ - text: expect.stringContaining("Command exited with code 7."), + text: expect.stringContaining("Exited with code 7"), description: bodyExitCommand, metadata: { source: "shell", @@ -1558,7 +1555,7 @@ describe("ShellTool", () => { { id: settled.metadata?.shellID, status: "completed", - output: "(no output)\n\nCommand exited with code 7.", + output: "Exited with code 7", }, ]) }),