From 647f4ee8ce4fc29870fd8be325a45aa8b11796be Mon Sep 17 00:00:00 2001 From: HFConsultant <62076556+HFConsultant@users.noreply.github.com> Date: Wed, 22 Apr 2026 12:01:35 -0700 Subject: [PATCH] fix: persist CLI session clearing atomically (#70298) Persist stale CLI session clearing through the session-store merge path and add regression coverage for Claude binding removal.\n\nThanks @HFConsultant. --- CHANGELOG.md | 1 + src/agents/cli-session.ts | 8 +-- src/agents/command/attempt-execution.ts | 22 ++----- src/agents/command/session-store.test.ts | 82 +++++++++++++++++++++++- src/agents/command/session-store.ts | 27 +++++++- 5 files changed, 119 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 029b12d29b2..34b8c8fc749 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- CLI sessions: persist CLI session clearing through the atomic session-store merge path, so expired Claude/Codex CLI bindings are actually removed before retrying without the stale session id. (#70298) Thanks @HFConsultant. - ACP/sessions_spawn: honor explicit `model` overrides for ACP child sessions instead of silently falling back to the target agent default model. (#70210) Thanks @felix-miao. - CLI/Claude: hash only static extra system prompt parts when deciding whether to reuse a CLI session, so per-message inbound metadata no longer resets Claude CLI conversations on every turn. (#70122) Thanks @zijunl. - Hooks/Slack: standardize shared message hook routing fields (`threadId` / `replyToId`) and stop Slack outbound delivery from re-running `message_sending` inside the channel adapter, so plugins like thread-ownership make one outbound routing decision per reply. Thanks @vincentkoc. diff --git a/src/agents/cli-session.ts b/src/agents/cli-session.ts index 0504a1c5571..8f7214c5f8f 100644 --- a/src/agents/cli-session.ts +++ b/src/agents/cli-session.ts @@ -112,14 +112,14 @@ export function clearCliSession(entry: SessionEntry, provider: string): void { entry.cliSessionIds = Object.keys(next).length > 0 ? next : undefined; } if (normalized === CLAUDE_CLI_BACKEND_ID) { - delete entry.claudeCliSessionId; + entry.claudeCliSessionId = undefined; } } export function clearAllCliSessions(entry: SessionEntry): void { - delete entry.cliSessionBindings; - delete entry.cliSessionIds; - delete entry.claudeCliSessionId; + entry.cliSessionBindings = undefined; + entry.cliSessionIds = undefined; + entry.claudeCliSessionId = undefined; } export function resolveCliSessionReuse(params: { diff --git a/src/agents/command/attempt-execution.ts b/src/agents/command/attempt-execution.ts index 6c2f3910bfa..c3871298014 100644 --- a/src/agents/command/attempt-execution.ts +++ b/src/agents/command/attempt-execution.ts @@ -12,7 +12,7 @@ import { sanitizeForLog } from "../../terminal/ansi.js"; import { resolveMessageChannel } from "../../utils/message-channel.js"; import { resolveBootstrapWarningSignaturesSeen } from "../bootstrap-budget.js"; import { runCliAgent } from "../cli-runner.js"; -import { clearCliSession, getCliSessionBinding, setCliSessionBinding } from "../cli-session.js"; +import { getCliSessionBinding, setCliSessionBinding } from "../cli-session.js"; import { FailoverError } from "../failover-error.js"; import { isCliProvider } from "../model-selection.js"; import { prepareSessionManagerForRun } from "../pi-embedded-runner/session-manager-init.js"; @@ -22,6 +22,7 @@ import { buildUsageWithNoCost } from "../stream-message-shared.js"; import { resolveFallbackRetryPrompt } from "./attempt-execution.helpers.js"; import { persistSessionEntry } from "./attempt-execution.shared.js"; import { resolveAgentRunContext } from "./run-context.js"; +import { clearCliSessionInStore } from "./session-store.js"; import type { AgentCommandOpts } from "./types.js"; export { @@ -301,22 +302,13 @@ export function runAgentAttempt(params: { `CLI session expired, clearing from session store: provider=${sanitizeForLog(params.providerOverride)} sessionKey=${params.sessionKey}`, ); - const entry = params.sessionStore[params.sessionKey]; - if (entry) { - const updatedEntry = { ...entry }; - clearCliSession(updatedEntry, params.providerOverride); - updatedEntry.updatedAt = Date.now(); - - await persistSessionEntry({ - sessionStore: params.sessionStore, + params.sessionEntry = + (await clearCliSessionInStore({ + provider: params.providerOverride, sessionKey: params.sessionKey, + sessionStore: params.sessionStore, storePath: params.storePath, - entry: updatedEntry, - clearedFields: ["cliSessionBindings", "cliSessionIds", "claudeCliSessionId"], - }); - - params.sessionEntry = updatedEntry; - } + })) ?? params.sessionEntry; return runCliWithSession(undefined).then(async (result) => { if ( diff --git a/src/agents/command/session-store.test.ts b/src/agents/command/session-store.test.ts index 61b8b41916f..0c26b4acb03 100644 --- a/src/agents/command/session-store.test.ts +++ b/src/agents/command/session-store.test.ts @@ -6,7 +6,7 @@ import type { OpenClawConfig } from "../../config/config.js"; import type { SessionEntry } from "../../config/sessions.js"; import { loadSessionStore } from "../../config/sessions.js"; import type { EmbeddedPiRunResult } from "../pi-embedded.js"; -import { updateSessionStoreAfterAgentRun } from "./session-store.js"; +import { clearCliSessionInStore, updateSessionStoreAfterAgentRun } from "./session-store.js"; import { resolveSession } from "./session.js"; vi.mock("../model-selection.js", () => ({ @@ -515,3 +515,83 @@ describe("updateSessionStoreAfterAgentRun", () => { }); }); }); + +describe("clearCliSessionInStore", () => { + it("persists cleared Claude CLI bindings through session-store merge", async () => { + await withTempSessionStore(async ({ storePath }) => { + const sessionKey = "agent:main:explicit:test-clear-claude-cli"; + const entry: SessionEntry = { + sessionId: "openclaw-session-1", + updatedAt: 1, + cliSessionBindings: { + "claude-cli": { + sessionId: "claude-session-1", + authEpoch: "epoch-1", + }, + "codex-cli": { + sessionId: "codex-session-1", + }, + }, + cliSessionIds: { + "claude-cli": "claude-session-1", + "codex-cli": "codex-session-1", + }, + claudeCliSessionId: "claude-session-1", + }; + const sessionStore: Record = { [sessionKey]: entry }; + await fs.writeFile(storePath, JSON.stringify(sessionStore, null, 2), "utf8"); + + const cleared = await clearCliSessionInStore({ + provider: "claude-cli", + sessionKey, + sessionStore, + storePath, + }); + + expect(cleared?.cliSessionBindings?.["claude-cli"]).toBeUndefined(); + expect(cleared?.cliSessionBindings?.["codex-cli"]).toEqual({ + sessionId: "codex-session-1", + }); + expect(cleared?.cliSessionIds?.["claude-cli"]).toBeUndefined(); + expect(cleared?.cliSessionIds?.["codex-cli"]).toBe("codex-session-1"); + expect(cleared?.claudeCliSessionId).toBeUndefined(); + expect(sessionStore[sessionKey]).toEqual(cleared); + + const persisted = loadSessionStore(storePath, { skipCache: true })[sessionKey]; + expect(persisted?.cliSessionBindings?.["claude-cli"]).toBeUndefined(); + expect(persisted?.cliSessionBindings?.["codex-cli"]).toEqual({ + sessionId: "codex-session-1", + }); + expect(persisted?.cliSessionIds?.["claude-cli"]).toBeUndefined(); + expect(persisted?.cliSessionIds?.["codex-cli"]).toBe("codex-session-1"); + expect(persisted?.claudeCliSessionId).toBeUndefined(); + }); + }); + + it("leaves the caller snapshot intact when the session entry is missing", async () => { + await withTempSessionStore(async ({ storePath }) => { + const existingKey = "agent:main:explicit:existing"; + const sessionStore: Record = { + [existingKey]: { + sessionId: "openclaw-session-1", + updatedAt: 1, + claudeCliSessionId: "claude-session-1", + }, + }; + await fs.writeFile(storePath, JSON.stringify(sessionStore, null, 2), "utf8"); + + const cleared = await clearCliSessionInStore({ + provider: "claude-cli", + sessionKey: "agent:main:explicit:missing", + sessionStore, + storePath, + }); + + expect(cleared).toBeUndefined(); + expect(sessionStore[existingKey]?.claudeCliSessionId).toBe("claude-session-1"); + expect( + loadSessionStore(storePath, { skipCache: true })[existingKey]?.claudeCliSessionId, + ).toBe("claude-session-1"); + }); + }); +}); diff --git a/src/agents/command/session-store.ts b/src/agents/command/session-store.ts index 8391bb89abf..1ae8ed29e94 100644 --- a/src/agents/command/session-store.ts +++ b/src/agents/command/session-store.ts @@ -5,7 +5,7 @@ import { updateSessionStore, } from "../../config/sessions.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { setCliSessionBinding, setCliSessionId } from "../cli-session.js"; +import { clearCliSession, setCliSessionBinding, setCliSessionId } from "../cli-session.js"; import { DEFAULT_CONTEXT_TOKENS } from "../defaults.js"; import { isCliProvider } from "../model-selection.js"; import { deriveSessionTotalTokens, hasNonzeroUsage } from "../usage.js"; @@ -154,3 +154,28 @@ export async function updateSessionStoreAfterAgentRun(params: { }); sessionStore[sessionKey] = persisted; } + +export async function clearCliSessionInStore(params: { + provider: string; + sessionKey: string; + sessionStore: Record; + storePath: string; +}): Promise { + const { provider, sessionKey, sessionStore, storePath } = params; + const entry = sessionStore[sessionKey]; + if (!entry) { + return undefined; + } + + const next = { ...entry }; + clearCliSession(next, provider); + next.updatedAt = Date.now(); + + const persisted = await updateSessionStore(storePath, (store) => { + const merged = mergeSessionEntry(store[sessionKey], next); + store[sessionKey] = merged; + return merged; + }); + sessionStore[sessionKey] = persisted; + return persisted; +}