fix(heartbeat): our own terminal-result cleanup is not the run's failure

`runWithTerminalResultCleanup` SIGTERMs the process group once the CLI prints
its terminal result line, so an unmanaged background task cannot outlive the
run. The process then exits 143 with a good result already parsed.

`execute()` knows this — `parsedSucceeded` is computed and then discarded — and
the server judges the outcome on `exitCode` alone, so a COMPLETED run is
recorded `failed` / `adapter_failed`. Because `adapter_failed` is in
TRANSIENT_INFRA_CONTINUATION_ERROR_CODES, the recovery service then continues
the issue and re-runs work that already finished.

Measured on one board over its last 200 heartbeat runs: 26 completed runs
recorded failed/adapter_failed/exit 143, and every one carried subtype
"success", is_error false and unmanagedBackgroundTask.terminalResultSeen true —
checked individually, not sampled. $28.62 of finished work booked as failed, and
12 retries costing $10.54 redoing it. The other 4 exit-143 runs in the window
were genuine (claude_auth_required, max_turns_exhausted) and stay failures.

`heartbeat-stop-metadata.ts` already has a branch for exactly this case, keyed
on errorCode "unmanaged_background_task_stopped". It is unreachable: no adapter
sets that code.

The fix keeps `exitCode` and `signal` truthful rather than faking a zero. A new
optional `AdapterExecutionResult.stoppedAfterTerminalResult` carries the verdict
the adapter already reached, and the server treats a non-zero exit as succeeded
only when the adapter says it stopped a process whose result it had already
parsed as a success. Additive and optional, so no other adapter changes.

Tests: four cases on the claude_local adapter — the self-stop is flagged with
exitCode 143 preserved; a genuine failure with cleanup also fired is NOT
flagged; a non-zero exit with no cleanup is NOT flagged; a clean exit is
untouched. Red before green.
This commit is contained in:
Roy Bales 2026-09-10 09:03:14 -04:00
parent 2a05b5ed34
commit 2075859f53
4 changed files with 131 additions and 0 deletions

View File

@ -90,6 +90,14 @@ export interface AdapterExecutionResult {
errorFamily?: AdapterExecutionErrorFamily | null;
retryNotBefore?: string | null;
errorMeta?: Record<string, unknown>;
/**
* True when the adapter itself stopped a process it had already seen a SUCCESSFUL terminal result from
* i.e. `terminalResultCleanup` fired (see `runWithTerminalResultCleanup` in server-utils) and the parsed
* result was a success. The process then exits non-zero because Paperclip signalled it, not because the
* work failed, so `exitCode` alone cannot decide the outcome. `exitCode`/`signal` stay truthful; the
* server reads this to avoid recording its own cleanup as an adapter failure. Absent means "not applicable".
*/
stoppedAfterTerminalResult?: boolean;
usage?: UsageSummary;
/**
* How `usage` totals are scoped. "per_run" means the tokens cover only this

View File

@ -0,0 +1,109 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
// Paperclip stops a process itself once it has seen the CLI's terminal result line
// (`runWithTerminalResultCleanup`), so an unmanaged background task cannot outlive the run. The process then
// exits 143 with a good result already parsed. The adapter knew that and discarded it, so the server judged the
// run on `exitCode` alone and recorded our own cleanup as an adapter failure.
//
// Measured on one board over 200 runs: 26 completed runs recorded `failed`/`adapter_failed`/exit 143, every one
// carrying subtype "success", is_error false and unmanagedBackgroundTask.terminalResultSeen true. Because
// `adapter_failed` is in TRANSIENT_INFRA_CONTINUATION_ERROR_CODES, recovery then re-ran work that had finished.
const { runAdapterExecutionTargetProcess } = vi.hoisted(() => ({
runAdapterExecutionTargetProcess: vi.fn(),
}));
vi.mock("./acp.js", () => ({
createClaudeAcpExecutor: () => vi.fn(),
formatClaudeAcpFallbackMessage: () => "",
resolveClaudeExecutionEngineForRun: async () => ({ engine: "cli", explicit: true }),
}));
vi.mock("@paperclipai/adapter-utils/execution-target", async () => {
const actual = await vi.importActual<typeof import("@paperclipai/adapter-utils/execution-target")>(
"@paperclipai/adapter-utils/execution-target",
);
return {
...actual,
ensureAdapterExecutionTargetCommandResolvable: vi.fn(async () => undefined),
ensureAdapterExecutionTargetRuntimeCommandInstalled: vi.fn(async () => undefined),
resolveAdapterExecutionTargetCommandForLogs: vi.fn(async () => "claude"),
runAdapterExecutionTargetProcess,
};
});
import { execute } from "./execute.js";
const resultLine = (over: Record<string, unknown> = {}) =>
JSON.stringify({
type: "result",
subtype: "success",
is_error: false,
session_id: "s-1",
result: "the work is done",
total_cost_usd: 1.4,
usage: { input_tokens: 10, cache_read_input_tokens: 0, output_tokens: 5 },
...over,
});
const proc = (over: Record<string, unknown> = {}) => ({
exitCode: 143,
signal: null,
timedOut: false,
stdout: resultLine(),
stderr: "",
pid: 1,
startedAt: new Date().toISOString(),
terminalResultCleanup: {
kind: "terminal_result_cleanup",
stopped: true,
stopReason: "unmanaged_background_task_stopped",
reason: "unmanaged background task stopped; no durable live path",
terminalResultSeen: true,
signal: "SIGTERM",
forceKilled: false,
},
...over,
});
const ctx = () => ({
runId: "run-1",
agent: { id: "a-1", companyId: "c-1", name: "Claude", adapterType: "claude_local", adapterConfig: {} },
runtime: { sessionId: null, sessionParams: null, sessionDisplayId: null, taskKey: null },
config: { engine: "cli" },
context: {},
onLog: vi.fn(async () => {}),
});
describe("claude_local: our own terminal-result cleanup is not the run's failure", () => {
beforeEach(() => vi.clearAllMocks());
it("flags a success we stopped ourselves, and keeps exitCode/signal truthful", async () => {
runAdapterExecutionTargetProcess.mockResolvedValue(proc());
const r = await execute(ctx() as never);
expect(r.stoppedAfterTerminalResult).toBe(true);
expect(r.exitCode).toBe(143); // forensics survive: the fix is not to fake a zero
expect(r.errorCode ?? null).toBeNull();
expect(r.resultJson?.unmanagedBackgroundTask).toMatchObject({ terminalResultSeen: true });
});
it("does NOT flag a run that genuinely failed, even when cleanup also fired", async () => {
runAdapterExecutionTargetProcess.mockResolvedValue(
proc({ stdout: resultLine({ subtype: "error_during_execution", is_error: true }) }),
);
const r = await execute(ctx() as never);
expect(r.stoppedAfterTerminalResult).toBeUndefined();
});
it("does NOT flag a non-zero exit with no cleanup — an ordinary failure stays a failure", async () => {
runAdapterExecutionTargetProcess.mockResolvedValue(proc({ exitCode: 1, terminalResultCleanup: null }));
const r = await execute(ctx() as never);
expect(r.stoppedAfterTerminalResult).toBeUndefined();
});
it("leaves a clean exit alone — the flag is only for a non-zero exit we caused", async () => {
runAdapterExecutionTargetProcess.mockResolvedValue(proc({ exitCode: 0, terminalResultCleanup: null }));
const r = await execute(ctx() as never);
expect(r.exitCode).toBe(0);
expect(r.stoppedAfterTerminalResult).toBeUndefined();
});
});

View File

@ -1227,10 +1227,16 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
...(proc.terminalResultCleanup ? { unmanagedBackgroundTask: proc.terminalResultCleanup } : {}),
};
// The terminal-result cleanup is OUR signal, not the model's failure: once the CLI prints its result line,
// `runWithTerminalResultCleanup` SIGTERMs the process group so an unmanaged background task cannot outlive
// the run. The process then exits 143 with a perfectly good result already parsed. `parsedSucceeded` knows
// that here and was previously discarded, leaving the server to judge on `exitCode` alone.
const stoppedAfterTerminalResult = parsedSucceeded && proc.terminalResultCleanup?.terminalResultSeen === true;
return {
exitCode: proc.exitCode,
signal: proc.signal,
timedOut: false,
...(stoppedAfterTerminalResult ? { stoppedAfterTerminalResult: true } : {}),
errorMessage,
errorCode: resolvedErrorCode,
errorFamily,

View File

@ -21545,6 +21545,14 @@ export function heartbeatService(
!adapterResult.errorMessage
) {
outcome = "succeeded";
} else if (
adapterResult.stoppedAfterTerminalResult &&
!adapterResult.errorMessage
) {
// The non-zero exit is Paperclip's own terminal-result cleanup signalling a process whose result it
// had already parsed as a success. Recording that as a failure blames the run for our cleanup, and
// (because `adapter_failed` is in TRANSIENT_INFRA_CONTINUATION_ERROR_CODES) re-runs completed work.
outcome = "succeeded";
} else {
outcome = "failed";
}