Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/cli/src/ui/interactive-mode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ import {
parseSkillBlock,
type ReadonlyFooterDataProvider,
rawKeyHint,
readCommandExecutionOutcome,
readStepLoginCredential,
readStepLoginProfile,
resolveModelScopeFromModels,
Expand Down Expand Up @@ -3556,6 +3557,7 @@ export class InteractiveMode {
message.cancelled,
message.truncated ? ({ truncated: true } as TruncationResult) : undefined,
message.fullOutputPath,
readCommandExecutionOutcome(message.execution),
);
this.chatContainer.addChild(component);
break;
Expand Down Expand Up @@ -6757,6 +6759,7 @@ ${followUpRow}| \`${dequeue}\` | Restore queued messages |
result.cancelled,
result.truncated ? ({ truncated: true, content: result.output } as TruncationResult) : undefined,
result.fullOutputPath,
result.execution,
);

// Record the result in session
Expand Down Expand Up @@ -6798,6 +6801,7 @@ ${followUpRow}| \`${dequeue}\` | Restore queued messages |
result.cancelled,
result.truncated ? ({ truncated: true, content: result.output } as TruncationResult) : undefined,
result.fullOutputPath,
result.execution,
);
}
} catch (error) {
Expand Down
34 changes: 26 additions & 8 deletions apps/cli/src/ui/view/transcript/bash-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@
*/

import {
type CommandExecutionOutcome,
commandOutcomeNotice,
DEFAULT_MAX_BYTES,
DEFAULT_MAX_LINES,
DynamicBorder,
keyHint,
keyText,
readCommandExecutionOutcome,
stripAnsi,
type TruncationResult,
theme,
Expand Down Expand Up @@ -49,6 +52,7 @@ export class BashExecutionComponent extends Container {
private outputLines: string[] = [];
private status: "running" | "complete" | "cancelled" | "error" = "running";
private exitCode: number | undefined = undefined;
private commandStatus: string | undefined;
private loader: Loader;
private truncationResult?: TruncationResult;
private fullOutputPath?: string;
Expand Down Expand Up @@ -165,9 +169,14 @@ export class BashExecutionComponent extends Container {
),
);
}
if (this.status === "cancelled") lines.push(`${muted(STEP_BODY_PREFIX)}${theme.fg("warning", "cancelled")}`);
if (this.status === "error")
lines.push(`${muted(STEP_BODY_PREFIX)}${theme.fg("error", `exit ${this.exitCode ?? "?"}`)}`);
if (this.status === "cancelled") {
lines.push(`${muted(STEP_BODY_PREFIX)}${theme.fg("warning", this.commandStatus ?? "cancelled")}`);
}
if (this.status === "error") {
lines.push(
`${muted(STEP_BODY_PREFIX)}${theme.fg("error", this.commandStatus ?? `exit ${this.exitCode ?? "?"}`)}`,
);
}
const wasTruncated = this.truncationResult?.truncated || contextTruncation.truncated;
if (wasTruncated && this.fullOutputPath) {
lines.push(`${muted(STEP_BODY_PREFIX)}${theme.fg("warning", `Full output: ${this.fullOutputPath}`)}`);
Expand Down Expand Up @@ -222,13 +231,22 @@ export class BashExecutionComponent extends Container {
cancelled: boolean,
truncationResult?: TruncationResult,
fullOutputPath?: string,
/** Structured facts when the caller has them; older callers omit this. */
execution?: CommandExecutionOutcome,
): void {
const observed = readCommandExecutionOutcome(execution);
this.exitCode = exitCode;
this.status = cancelled
? "cancelled"
: exitCode !== 0 && exitCode !== undefined && exitCode !== null
? "error"
: "complete";
this.commandStatus = observed ? commandOutcomeNotice(observed) : undefined;
this.status =
cancelled || observed?.status === "canceled"
? "cancelled"
: observed
? observed.status === "exited" && observed.exitCode === 0
? "complete"
: "error"
: exitCode !== 0 && exitCode !== undefined && exitCode !== null
? "error"
: "complete";
this.truncationResult = truncationResult;
this.fullOutputPath = fullOutputPath;

Expand Down
19 changes: 16 additions & 3 deletions apps/cli/src/ui/view/transcript/tool-execution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
createAllToolDefinitions,
getTextOutput as getRenderedTextOutput,
keyHint,
readToolCommandNotice,
renderDiff,
renderToolPath,
replaceTabs,
Expand Down Expand Up @@ -880,13 +881,25 @@ export class ToolExecutionComponent extends Container {
return this.result?.content.some((content) => content.type === "text" && Boolean(content.text?.trim())) ?? false;
}

/**
* A command's exit status is a result fact, not an invocation failure, so the
* card's success/failure presentation must read `details.execution` too.
*/
private commandNotice(): string | undefined {
return readToolCommandNotice(this.result?.details);
}

private showsFailure(): boolean {
return this.result?.isError === true || this.commandNotice() !== undefined;
}

private renderStepFallbackOutput(existing?: string[]): string[] {
if (!this.result) return existing ?? [];
const output = this.getTextOutput();
if (!output) return existing ?? [];
// Failed runs paint their output with the error color so the why is
// scannable at a glance, mirroring the ✗ glyph in the header row.
const paint = this.result.isError
const paint = this.showsFailure()
? (line: string) => theme.fg("error", line)
: (line: string) => theme.fg("toolOutput", line);
return output.split(/\r?\n/u).map(paint);
Expand Down Expand Up @@ -935,7 +948,7 @@ export class ToolExecutionComponent extends Container {

private presentationGlyph(): string {
if (this.isPartial || !this.result) return theme.fg("accent", this.spinner?.frame ?? "⠋");
if (this.result.isError) return theme.fg("error", "✗");
if (this.showsFailure()) return theme.fg("error", "✗");
return theme.fg("success", "●");
}

Expand All @@ -953,7 +966,7 @@ export class ToolExecutionComponent extends Container {
? undefined
: this.isPartial
? (text: string) => theme.bg("toolPendingBg", text)
: this.result?.isError
: this.showsFailure()
? (text: string) => theme.bg("toolErrorBg", text)
: (text: string) => theme.bg("toolSuccessBg", text);

Expand Down
42 changes: 42 additions & 0 deletions apps/cli/test/bash-execution-width.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,48 @@ describe("BashExecutionComponent width handling (#2569)", () => {
expect(failedLines.join("\n")).toContain("└ exit 2");
});

it("reports an interrupted or unknown command through its execution facts", () => {
const { stub } = createTuiStub(100);

const signaled = new BashExecutionComponent("kill -9 $$", stub, false, "step");
signaled.setComplete(undefined, false, undefined, undefined, {
status: "signaled",
exitCode: null,
signal: "SIGKILL",
timedOut: false,
aborted: false,
});
const signaledLines = signaled.render(80).map(stripTerminalSequences);
expect(signaledLines[0]).toContain("✗ $ kill -9 $$");
expect(signaledLines.join("\n")).toContain("Command terminated by signal SIGKILL");

// A deadline that coexists with exit code zero is still an interruption.
const deadline = new BashExecutionComponent("sleep 5", stub, false, "step");
deadline.setComplete(undefined, false, undefined, undefined, {
status: "timed_out",
exitCode: 0,
signal: null,
timedOut: true,
aborted: false,
timeoutSeconds: 1,
});
const deadlineLines = deadline.render(80).map(stripTerminalSequences);
expect(deadlineLines[0]).toContain("✗ $ sleep 5");
expect(deadlineLines.join("\n")).toContain("Command timed out after 1 seconds");

const canceled = new BashExecutionComponent("fixture", stub, false, "step");
canceled.setComplete(undefined, true, undefined, undefined, {
status: "canceled",
exitCode: null,
signal: null,
timedOut: false,
aborted: true,
});
const canceledLines = canceled.render(80).map(stripTerminalSequences);
expect(canceledLines[0]).toContain("■ $ fixture");
expect(canceledLines.join("\n")).toContain("Command aborted");
});

it("wraps CJK output within the Step rail at narrow widths", () => {
const { stub } = createTuiStub(100);
const component = new BashExecutionComponent("printf cjk", stub, false, "step");
Expand Down
67 changes: 67 additions & 0 deletions apps/cli/test/tool-execution-component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,73 @@ describe("ToolExecutionComponent parity", () => {
expect(rendered).not.toContain("[Showing lines 2001-4000 of 4000. Full output:");
});

test("shows a failed command as a failure even though the invocation succeeded", async () => {
const operations: BashOperations = {
exec: async () => ({ exitCode: 1 }),
};
const tool = createBashToolDefinition(process.cwd(), { operations });
const result = await tool.execute("tool-bash-failed", { command: "false" }, undefined, undefined, {} as never);
const component = new ToolExecutionComponent(
"bash",
"tool-bash-failed",
{ command: "false" },
{ presentation: "step" },
tool,
createFakeTui(),
process.cwd(),
);
component.updateResult({ ...result, isError: false }, false);

const rendered = stripAnsi(component.render(120).join("\n"));
expect(result.isError).toBe(false);
expect(rendered).toContain("✗");
expect(rendered).toContain("Command exited with code 1");
});

test("styles an applied mutation whose embedded check failed as a failed check", () => {
const args = { path: "sample.txt", search: "a", replace: "b", then_run: "check" };
const result = {
content: [{ type: "text", text: "Successfully replaced 1 occurrence(s) in sample.txt.\n\nthen_run: $ check\nboom" }],
details: {
changed: true,
thenRun: {
command: "check",
exitCode: 3,
timedOut: false,
stepTruncated: false,
execution: {
status: "exited",
exitCode: 3,
signal: null,
timedOut: false,
aborted: false,
},
},
},
};
const component = new ToolExecutionComponent(
"edit_file",
"then-run-failed",
args,
{ presentation: "step" },
createBaseToolDefinition("edit_file"),
createFakeTui(),
process.cwd(),
);
component.updateResult(result as never, false);

const rendered = component.render(120).join("\n");
const plain = stripAnsi(rendered);
// The call itself succeeded (the file was written); the card still shows
// that its embedded check failed rather than a green success dot.
expect((result as { isError?: boolean }).isError).toBeFalsy();
expect(plain).toContain("✗ edit_file(sample.txt)");
expect(plain).not.toContain("● edit_file(sample.txt)");
expect(plain).toContain("then_run: $ check");
// Styling is applied (the plain text differs from the styled render).
expect(rendered).not.toBe(plain);
});

test("does not duplicate built-in headers when passed the active built-in definition", () => {
const component = new ToolExecutionComponent(
"read",
Expand Down
1 change: 1 addition & 0 deletions packages/agent-core/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Inherits the [root rules](../../AGENTS.md). This package is `@step-harness/agent
- Preserve tool-call/result pairing, event ordering, cancellation propagation, steer / follow-up timing, and the distinction between errors and normal completion.
- Tool updates are replacement snapshots and may coalesce. Observe background delivery failures and report them through the awaited tool-result path, without leaving a run unsettled.
- Generic session context and persistence contracts belong in the harness. Product session adapters may compose them; client caches must not become authoritative state.
- `isError` describes an invocation failure, not a tool-specific business outcome. Preserve observed content, details, usage, and termination hints carried by `ToolExecutionError` or an already-completed result when result delivery or hooks fail.
- Pass telemetry contexts explicitly. Disabling telemetry must not change business return values, errors, or execution order.

## Validation and documentation
Expand Down
26 changes: 26 additions & 0 deletions packages/agent-core/docs/harness.md
Original file line number Diff line number Diff line change
Expand Up @@ -2615,6 +2615,32 @@ Replay across retry and resume:

## 5.7 Agent-loop building blocks

The current low-level loop uses `AgentToolResult.isError` for invocation failure,
not for a product tool's business result. A tool can throw `ToolExecutionError`
with observed content and details; these survive into its error event and
transcript message. If result delivery or a return-stage hook fails after a
result exists, the loop retains its observations, usage, and termination hint.
Output-preparation failures may replace text with recovery diagnostics while
retaining metadata and termination policy. The loop does not automatically
execute the tool again.

`CommandExecutionResult` describes adapter observations (`exitCode` and optional
`signal`, `timedOut`, `aborted`, and `timeoutSeconds`). `CommandExecutionOutcome`
normalizes those facts with an explicit `status`: `exited`, `timed_out`,
`signaled`, `canceled`, or `unknown`. A deadline can coexist with exit code zero;
an unknown exit is not success. These DTOs, the normalizer, and their formatters
live in [`src/harness/command-outcome.ts`](../src/harness/command-outcome.ts).
The generic `bash` tool in [`src/harness/tools/bash.ts`](../src/harness/tools/bash.ts)
returns a nonzero exit or deadline as an ordinary result with `details.execution`;
only a cancellation or executor fault is an invocation failure, and it retains
the same observations.

The canonical [`BashExecutionMessage`](../src/harness/messages.ts) stores the
optional `execution` outcome and projects it through
`commandExecutionNotice`, which stays silent for a clean exit and states every
other observation. Older messages without the field retain their existing
projection.

The existing `agent-loop.ts` remains behavior-compatible and is refactored into these exported phases. Existing fields on `AgentTool`, `AgentToolResult`, and provider messages are retained. Add recovery declaration `replay?: "never" | "safe"` to `AgentTool`; omission means `"never"`. `AgentHarnessTool` inherits it. The `AgentEventSink` below is the existing agent-loop sink, not the harness event listener; the harness adapts agent events into §5.5 events.

```ts
Expand Down
Loading
Loading