fix(agents): preserve valid CLI session bindings (#128732)

* fix(agents): preserve CLI session binding on format-class failover

shouldClearFailedCliSessionBinding cleared the stored CLI session
binding for every FailoverError, including format-class failures
(output limit exceeded, parser error, unsupported image input). A
format-class failover means the stream could not be read, not that
the CLI session on disk is invalid — clearing the binding silently
lost the conversation context on the next turn.

Narrow the condition to only session-invalidating failover reasons
(session_expired, auth, auth_permanent) using a whitelist so new
reasons default to preserving the binding.

Fixes #128698

* fix(agents): preserve valid CLI session bindings

* fix(agents): retain CLI binding across format recovery

* fix(agents): scope fresh CLI recovery by backend

* docs(plugins): document CLI recovery policy

---------

Co-authored-by: Vincent Koc <vincentkoc@users.noreply.github.com>
This commit is contained in:
SunnyShu
2026-08-25 10:42:39 +08:00
committed by GitHub
parent dca05ea39a
commit e2deb87c30
10 changed files with 324 additions and 31 deletions
+15
View File
@@ -215,8 +215,23 @@ model-alias, session, image, and watchdog fields as the bundled
| `imagePathScope` | Where staged image files live before handoff: `temp` or `workspace` |
| `serialize` | Keep same-backend runs ordered |
| `reseedFromRawTranscriptWhenUncompacted` | Opt in to bounded raw-transcript reseed before compaction for safe session resets |
| `freshSessionRecovery` | Fresh recovery policy after a recoverable resumed-session failure |
| `reliability.watchdog` | No-output timeout tuning, separate for fresh vs resumed runs |
`freshSessionRecovery` is a backend-owned compatibility contract:
- Leave it undefined or set it to `"replace-binding"` to preserve the legacy
clear-and-reseed behavior. OpenClaw clears the persisted binding and retries
with a fresh session when the failure is eligible for recovery.
- Set it to `"invalidated-only"` to suppress fresh replacement unless the
canonical invalidation predicate proves the old session is dead. Currently,
only `session_expired` does so.
Choose the value from the CLI or SDK session contract, not from a provider id
or broad error class. The bundled Anthropic backend uses `"invalidated-only"`;
its Agent SDK contract does not treat non-expiration failures as proof that the
conversation can no longer resume.
Prefer the smallest static config that matches the CLI. Add plugin callbacks
only for behavior that really belongs to the backend.
+1
View File
@@ -218,6 +218,7 @@ export function buildAnthropicCliBackend(
sessionArgs: ["--session-id", "{sessionId}"],
sessionMode: "always",
reseedFromRawTranscriptWhenUncompacted: true,
freshSessionRecovery: "invalidated-only",
sessionIdFields: [...CLAUDE_CLI_SESSION_ID_FIELDS],
systemPromptFileArg: "--append-system-prompt-file",
systemPromptMode: "append",
+1
View File
@@ -97,6 +97,7 @@ describe("anthropic provider replay hooks", () => {
expect(backend.bundleMcp).toBe(true);
expectFields(backend.config, {
command: "claude",
freshSessionRecovery: "invalidated-only",
modelArg: "--model",
sessionArgs: ["--session-id", "{sessionId}"],
});
+127
View File
@@ -56,6 +56,7 @@ import {
requestHeartbeatMock,
supervisorSpawnMock,
} from "./cli-runner.test-support.js";
import { runCliRecovery } from "./cli-runner/cli-run-recovery.js";
import { executePreparedCliRun } from "./cli-runner/execute.js";
import {
resolveCliNoOutputTimeoutMs,
@@ -65,6 +66,7 @@ import { prepareCliRunContext } from "./cli-runner/prepare.js";
import { hashCliReseedPrompt } from "./cli-runner/reseed-envelope.js";
import * as sessionHistoryModule from "./cli-runner/session-history.js";
import type { PreparedCliRunContext } from "./cli-runner/types.js";
import { FailoverError } from "./failover-error.js";
import { runAgentHarnessBeforeMessageWriteHook } from "./harness/hook-helpers.js";
import { MAX_AGENT_HOOK_HISTORY_MESSAGES } from "./harness/hook-history.js";
@@ -2092,6 +2094,130 @@ describe("runCliAgent reliability", () => {
expect(clearBeforeRetry).not.toHaveBeenCalled();
});
it("does not start a fresh CLI attempt when format recovery retains the binding", async () => {
supervisorSpawnMock.mockClear();
supervisorSpawnMock.mockResolvedValueOnce(
makeManagedRun({
stdout: [
JSON.stringify({
type: "assistant",
message: {
model: "<synthetic>",
content: [{ type: "text", text: "No response requested." }],
},
}),
JSON.stringify({ type: "result", subtype: "success", result: "" }),
].join("\n"),
}),
);
const clearBeforeRetry = vi.fn(async () => false);
const { dir, sessionFile } = createSessionFile({
history: [{ role: "user", content: "earlier context" }],
});
try {
const context = makeClaudePreparedContext({
sessionKey: "agent:main:subagent:retained-format",
runId: "run-retained-format",
cliSessionId: "retained-cli-session",
openClawHistoryPrompt: CLI_RESEED_PROMPT,
});
context.preparedBackend.backend = {
...context.preparedBackend.backend,
freshSessionRecovery: "invalidated-only",
output: "jsonl",
input: "stdin",
jsonlDialect: "claude-stream-json",
};
context.backendResolved.config = context.preparedBackend.backend;
await expect(
runPreparedCliAgent({
...context,
params: {
...context.params,
agentId: "main",
sessionFile,
workspaceDir: dir,
onBeforeFreshCliSessionRetry: clearBeforeRetry,
},
}),
).rejects.toMatchObject({ reason: "format", code: "cli_synthetic_no_response" });
expect(supervisorSpawnMock).toHaveBeenCalledTimes(1);
expect(clearBeforeRetry).not.toHaveBeenCalled();
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
});
it.each([
["format", "cli_synthetic_no_response"],
["timeout", "cli_no_output_timeout"],
] as const)(
"keeps undefined Gemini recovery policy compatible after %s failover",
async (reason, code) => {
const context = buildPreparedContext({
provider: "google-gemini-cli",
sessionKey: `agent:main:gemini-${reason}`,
cliSessionId: "gemini-resumed-session",
openClawHistoryPrompt: CLI_RESEED_PROMPT,
});
context.preparedBackend.backend = {
command: "gemini",
args: ["--prompt", "{prompt}"],
resumeArgs: ["--resume", "{sessionId}", "--prompt", "{prompt}"],
output: "jsonl",
jsonlDialect: "gemini-stream-json",
input: "arg",
sessionMode: "existing",
};
context.backendResolved.config = context.preparedBackend.backend;
expect(context.preparedBackend.backend.freshSessionRecovery).toBeUndefined();
const executeAttempt = vi
.fn()
.mockRejectedValueOnce(
new FailoverError(`Gemini ${reason} failure`, {
reason,
code,
provider: "google-gemini-cli",
model: "gemini-3.1-pro-preview",
}),
)
.mockResolvedValueOnce({ sessionId: `gemini-fresh-${reason}` });
const clearBeforeRetry = vi.fn(async () => true);
const result = await runCliRecovery({
context: {
...context,
params: {
...context.params,
onBeforeFreshCliSessionRetry: clearBeforeRetry,
},
},
executeAttempt,
finishAttempt: async (attempt: { sessionId: string }) =>
({
payloads: [{ text: "Gemini recovered" }],
meta: { cliSessionId: attempt.sessionId },
}) as never,
finishDeliveredFailure: async () => undefined,
onTerminalFailure: async () => {},
});
expect(executeAttempt).toHaveBeenCalledTimes(2);
expect(executeAttempt.mock.calls[0]?.[0]).toBe("gemini-resumed-session");
expect(executeAttempt.mock.calls[1]?.[0]).toBeUndefined();
expect(clearBeforeRetry).toHaveBeenCalledWith({
provider: "google-gemini-cli",
reason,
sessionId: "gemini-resumed-session",
});
expect(requireRecord(result.meta, "result meta").cliSessionId).toBe(`gemini-fresh-${reason}`);
},
);
it.each(["timeout", "unknown", "context_overflow", "format"] as const)(
"retries a fresh CLI session after recoverable %s failover without a failed agent_end",
async (reason) => {
@@ -4050,6 +4176,7 @@ describe("runCliAgent reliability", () => {
openClawHistoryPrompt:
"Continue this conversation using the OpenClaw transcript below.\n\nUser: recovered history\n\n<next_user_message>\nhi\n</next_user_message>",
});
context.preparedBackend.backend.freshSessionRecovery = "invalidated-only";
const clearBeforeRetry = vi.fn(async () => true);
try {
+11
View File
@@ -1,4 +1,5 @@
import { formatErrorMessage } from "../../infra/errors.js";
import { isCliSessionInvalidatingFailoverReason } from "../cli-session.js";
import type { EmbeddedAgentRunResult } from "../embedded-agent-runner.js";
import { type FailoverError, isFailoverError } from "../failover-error.js";
import { createCliFailoverError } from "./exit-error.js";
@@ -21,10 +22,19 @@ export function resolveCliSessionId(reusableCliSession: CliReusableSession): str
function shouldRetryFreshCliSessionAfterFailover(params: {
error: FailoverError;
hasHistoryPrompt: boolean;
recoveryPolicy?: "replace-binding" | "invalidated-only";
}): boolean {
if (!params.hasHistoryPrompt) {
return false;
}
// Some CLIs can safely replace a resumable conversation after transport or
// format failures. Backends that cannot must positively prove invalidation.
if (
params.recoveryPolicy === "invalidated-only" &&
!isCliSessionInvalidatingFailoverReason(params.error.reason)
) {
return false;
}
switch (params.error.reason) {
case "session_expired":
return true;
@@ -163,6 +173,7 @@ export async function runCliRecovery<TAttempt>(params: {
shouldRetryFreshCliSessionAfterFailover({
error: recoveryError,
hasHistoryPrompt: Boolean(context.openClawHistoryPrompt),
recoveryPolicy: context.preparedBackend.backend.freshSessionRecovery,
}) &&
retryableSessionId &&
runParams.sessionKey
+19 -12
View File
@@ -13,6 +13,7 @@ import {
clearCliSession,
getCliSessionBinding,
hashCliSessionText,
isCliSessionInvalidatingFailoverReason,
resolveCliSessionClearReason,
resolveCliSessionReuse,
setCliSessionBinding,
@@ -20,6 +21,7 @@ import {
shouldClearFailedCliSessionBinding,
} from "./cli-session.js";
import { FailoverError } from "./failover-error.js";
import { FAILOVER_REASONS } from "./failover/signal.js";
describe("cli-session helpers", () => {
it("persists binding metadata alongside legacy session ids", () => {
@@ -628,26 +630,20 @@ describe("cli-session helpers", () => {
});
it("shares failed reused-session cleanup policy across CLI entry points", () => {
const failover = new FailoverError("session expired", {
reason: "session_expired",
provider: "claude-cli",
model: "claude-opus-4-8",
});
const abort = Object.assign(new Error("aborted"), { name: "AbortError" });
const binding = { sessionId: "reused" };
const forkBinding = { sessionId: "fork-source", forkNextResume: true as const };
expect(shouldClearFailedCliSessionBinding({ error: failover, binding })).toBe(true);
expect(shouldClearFailedCliSessionBinding({ error: failover, binding: forkBinding })).toBe(
true,
);
expect(resolveCliSessionClearReason(failover)).toBe("session_expired");
expect(shouldClearFailedCliSessionBinding({ error: abort, binding })).toBe(true);
expect(shouldClearFailedCliSessionBinding({ error: abort, binding: forkBinding })).toBe(false);
expect(
shouldClearFailedCliSessionBinding({
error: failover,
error: new FailoverError("session expired", {
reason: "session_expired",
provider: "claude-cli",
model: "claude-opus-4-8",
}),
binding,
hasNewGeneratedMediaTask: true,
}),
@@ -656,6 +652,17 @@ describe("cli-session helpers", () => {
expect(
shouldClearFailedCliSessionBinding({ error: new Error("provider failed"), binding }),
).toBe(false);
expect(shouldClearFailedCliSessionBinding({ error: failover })).toBe(false);
expect(shouldClearFailedCliSessionBinding({ error: abort })).toBe(false);
});
it.each(FAILOVER_REASONS)("only clears binding for a provider-expired session: %s", (reason) => {
const error = new FailoverError("failover", { reason, provider: "claude-cli" });
const invalidatesSession = reason === "session_expired";
expect(FAILOVER_REASONS).toHaveLength(16);
expect(isCliSessionInvalidatingFailoverReason(reason)).toBe(invalidatesSession);
expect(shouldClearFailedCliSessionBinding({ error, binding: { sessionId: "reused" } })).toBe(
invalidatesSession,
);
});
});
+9 -1
View File
@@ -10,6 +10,7 @@ import type { CliSessionBinding, SessionEntry } from "../config/sessions.js";
import { normalizeCliSessionReseedReceipt } from "../config/sessions/cli-session-binding.js";
import { readErrorName } from "../infra/errors.js";
import { isFailoverError } from "./failover-error.js";
import type { FailoverReason } from "./failover/signal.js";
export {
clearAllCliSessions,
getCliSessionBinding,
@@ -18,6 +19,13 @@ export {
const CLAUDE_CLI_BACKEND_ID = "claude-cli";
/** Whether a failover proves the provider-side conversation can no longer be resumed. */
export function isCliSessionInvalidatingFailoverReason(reason: FailoverReason): boolean {
// Auth identity changes are handled by the reuse fingerprint's auth epoch.
// Other execution failures say nothing about the persisted transcript.
return reason === "session_expired";
}
/** Hash CLI session-sensitive text so reuse checks can compare stable fingerprints. */
export function hashCliSessionText(value: string | undefined): string | undefined {
const trimmed = normalizeOptionalString(value);
@@ -126,7 +134,7 @@ export function shouldClearFailedCliSessionBinding(params: {
return false;
}
if (isFailoverError(params.error)) {
return true;
return isCliSessionInvalidatingFailoverReason(params.error.reason);
}
// A pre-successor fork abort keeps its one-shot marker for the next turn.
return params.binding?.forkNextResume !== true && readErrorName(params.error) === "AbortError";
+121 -17
View File
@@ -973,6 +973,61 @@ describe("CLI attempt execution", () => {
expect(persisted[sessionKey]?.claudeCliSessionId).toBeUndefined();
});
it("preserves and resumes a valid Claude CLI binding after format failover", async () => {
const sessionKey = "agent:main:subagent:cli-format";
const cliSessionId = "format-retry-session";
await writeClaudeCliAssistantTranscript(cliSessionId);
const sessionEntry = makeClaudeCliSessionEntry("session-cli-format", cliSessionId);
const sessionStore: Record<string, SessionEntry> = { [sessionKey]: sessionEntry };
await writeSessionStoreSeed(sessionStore);
runCliAgentMock.mockImplementationOnce(async () => {
expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
expect(readSessionStore()[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
throw new FailoverError("Claude CLI returned an unusable result", {
reason: "format",
code: "cli_synthetic_no_response",
provider: "claude-cli",
model: "opus",
});
});
await expect(
runClaudeCliAttempt({
sessionEntry,
sessionKey,
sessionStore,
body: "retry this malformed turn",
runId: "run-cli-format",
}),
).rejects.toMatchObject({ name: "FailoverError", reason: "format" });
expect(runCliAgentMock).toHaveBeenCalledTimes(1);
expect(firstRunCliAgentArg().cliSessionId).toBe(cliSessionId);
runCliAgentMock.mockResolvedValueOnce(makeCliResult("hello after retained resume"));
await runClaudeCliAttempt({
sessionEntry,
sessionKey,
sessionStore,
body: "continue on the next turn",
runId: "run-cli-format-resume",
});
expect(runCliAgentMock).toHaveBeenCalledTimes(2);
expect(firstRunCliAgentArg(1).cliSessionId).toBe(cliSessionId);
expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
expect(readSessionStore()[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
});
it("clears reused Claude CLI session IDs after AbortError without retrying", async () => {
const sessionKey = "agent:main:direct:cli-abort";
const cliSessionId = "abort-poisoned-session";
@@ -1134,7 +1189,7 @@ describe("CLI attempt execution", () => {
);
});
it("clears a persisted fork successor before transcript fallback", async () => {
it("clears a persisted fork successor when fresh recovery is authorized", async () => {
const sessionKey = "agent:main:direct:cli-fork-timeout";
const cliSessionId = "timeout-parent-session";
const forkedCliSessionId = "timeout-stalled-fork";
@@ -1153,17 +1208,19 @@ describe("CLI attempt execution", () => {
expect(clearFork).toBeTypeOf("function");
await (claimFork as () => Promise<boolean>)();
await (persistFork as (sessionId: string) => Promise<void>)(forkedCliSessionId);
await (
clearFork as (params: {
provider: string;
reason: "timeout";
sessionId: string;
}) => Promise<boolean>
)({
provider: "claude-cli",
reason: "timeout",
sessionId: forkedCliSessionId,
});
await expect(
(
clearFork as (params: {
provider: string;
reason: "timeout";
sessionId: string;
}) => Promise<boolean>
)({
provider: "claude-cli",
reason: "timeout",
sessionId: forkedCliSessionId,
}),
).resolves.toBe(true);
return makeCliResult("hello after fork timeout");
});
@@ -1357,8 +1414,8 @@ describe("CLI attempt execution", () => {
expect(firstRunCliAgentArg().onBeforeFreshCliSessionRetry).toBeUndefined();
});
it.each(["auth", "billing", "rate_limit"] as const)(
"clears reused Claude CLI session IDs after %s failover without retrying",
it.each(["auth", "auth_permanent"] as const)(
"preserves reused Claude CLI session IDs after %s failover without retrying",
async (reason) => {
const sessionKey = `agent:main:direct:cli-${reason}`;
const cliSessionId = `${reason}-poisoned-session`;
@@ -1386,9 +1443,56 @@ describe("CLI attempt execution", () => {
expect(runCliAgentMock).toHaveBeenCalledTimes(1);
expect(firstRunCliAgentArg().cliSessionId).toBe(cliSessionId);
expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]).toBeUndefined();
expect(sessionStore[sessionKey]?.cliSessionIds?.["claude-cli"]).toBeUndefined();
expect(sessionStore[sessionKey]?.claudeCliSessionId).toBeUndefined();
expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
expect(sessionStore[sessionKey]?.cliSessionIds?.["claude-cli"]).toBe(cliSessionId);
expect(sessionStore[sessionKey]?.claudeCliSessionId).toBe(cliSessionId);
const persisted = readSessionStore();
expect(persisted[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
expect(persisted[sessionKey]?.cliSessionIds?.["claude-cli"]).toBe(cliSessionId);
expect(persisted[sessionKey]?.claudeCliSessionId).toBe(cliSessionId);
},
);
it.each(["billing", "rate_limit"] as const)(
"preserves reused Claude CLI session IDs after %s failover without retrying",
async (reason) => {
const sessionKey = `agent:main:direct:cli-${reason}`;
const cliSessionId = `${reason}-poisoned-session`;
await writeClaudeCliAssistantTranscript(cliSessionId);
const sessionEntry = makeClaudeCliSessionEntry(`session-cli-${reason}`, cliSessionId);
const sessionStore: Record<string, SessionEntry> = { [sessionKey]: sessionEntry };
await writeSessionStoreSeed(sessionStore);
runCliAgentMock.mockRejectedValueOnce(
new FailoverError(`${reason} failed`, {
reason,
provider: "claude-cli",
model: "opus",
}),
);
await expect(
runClaudeCliAttempt({
sessionKey,
sessionEntry,
sessionStore,
body: `resume after ${reason}`,
runId: `run-cli-${reason}`,
}),
).rejects.toMatchObject({ name: "FailoverError", reason });
expect(runCliAgentMock).toHaveBeenCalledTimes(1);
expect(firstRunCliAgentArg().cliSessionId).toBe(cliSessionId);
expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]).toBeDefined();
expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]?.sessionId).toBe(
cliSessionId,
);
expect(sessionStore[sessionKey]?.cliSessionIds?.["claude-cli"]).toBe(cliSessionId);
expect(sessionStore[sessionKey]?.claudeCliSessionId).toBe(cliSessionId);
},
);
+13 -1
View File
@@ -22,6 +22,7 @@ import { messageToolOwnsVisibleReply } from "../../auto-reply/source-reply-deliv
import type { ThinkLevel, VerboseLevel } from "../../auto-reply/thinking.js";
import { resolveSessionAuthProfileOverrideSource } from "../../config/sessions/auth-profile-override-provenance.js";
import {
loadSessionEntry,
persistSessionTranscriptTurn,
type SessionTranscriptRuntimeTarget,
} from "../../config/sessions/session-accessor.js";
@@ -1062,7 +1063,18 @@ export function runAgentAttempt(params: {
? {
onBeforeFreshCliSessionRetry: async (retry) => {
if (
hasNewGeneratedMediaTaskForSessionKey(params.sessionKey, mediaTaskIdsBefore)
hasNewGeneratedMediaTaskForSessionKey(
params.sessionKey,
mediaTaskIdsBefore,
) ||
getCliSessionBinding(
loadSessionEntry({
sessionKey: mutableCliSessionStore.sessionKey,
storePath: mutableCliSessionStore.storePath,
readConsistency: "latest",
}),
cliExecutionProvider,
)?.sessionId !== retry.sessionId
) {
return false;
}
+7
View File
@@ -62,6 +62,13 @@ export type CliBackendConfig = {
serialize?: boolean;
/** Opt in to bounded raw transcript reseed before compaction for safe session resets. */
reseedFromRawTranscriptWhenUncompacted?: boolean;
/**
* Controls fresh recovery after a recoverable resumed-session failure.
*
* Undefined and `replace-binding` preserve the legacy clear-and-reseed behavior.
* `invalidated-only` retries fresh only when the failure proves the binding expired.
*/
freshSessionRecovery?: "replace-binding" | "invalidated-only";
/** Runtime reliability tuning for this backend's process lifecycle. */
reliability?: {
/** No-output watchdog tuning (fresh vs resumed runs). */