diff --git a/CHANGELOG.md b/CHANGELOG.md index 98efdbf4f94..c1332a5b3f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ Docs: https://docs.openclaw.ai - Telegram/polling: rebuild the polling HTTP transport after `getUpdates` 409 conflicts, so retries use a fresh TCP connection instead of looping on a Telegram-terminated keep-alive socket. (#69873) Thanks @hclsys. - Slack/files: resolve `downloadFile` bot tokens from the runtime config when callers provide `cfg` without an explicit token or prebuilt client, preserving cfg-only file downloads outside the action runtime path. (#70160) Thanks @martingarramon. - Slack/HTTP: dispatch registered Request URL webhooks through the same handler registry used by Slack monitor setup, so HTTP-mode Slack events no longer 404 after successful route registration. (#70275) Thanks @FroeMic. +- CLI/Claude: verify stored Claude CLI session ids have a readable project transcript before resuming, clearing phantom bindings with `reason=transcript-missing` instead of silently starting fresh under `--resume`. Fixes #70177. - 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. - Agents/subagents: drop bare `NO_REPLY` from the parent turn when the session still has pending spawned children, so direct-conversation surfaces such as Telegram DMs no longer rewrite the sentinel into visible fallback chatter while waiting for the child completion event. (#69942) Thanks @neeravmakwana. diff --git a/src/agents/command/attempt-execution.cli.test.ts b/src/agents/command/attempt-execution.cli.test.ts index 495a55e0e01..01765c3a27a 100644 --- a/src/agents/command/attempt-execution.cli.test.ts +++ b/src/agents/command/attempt-execution.cli.test.ts @@ -9,6 +9,7 @@ import type { EmbeddedPiRunResult } from "../pi-embedded.js"; import { persistCliTurnTranscript, runAgentAttempt } from "./attempt-execution.js"; const runCliAgentMock = vi.hoisted(() => vi.fn()); +const ORIGINAL_HOME = process.env.HOME; vi.mock("../cli-runner.js", () => ({ runCliAgent: runCliAgentMock, @@ -75,11 +76,28 @@ describe("CLI attempt execution", () => { }); afterEach(async () => { + if (ORIGINAL_HOME === undefined) { + delete process.env.HOME; + } else { + process.env.HOME = ORIGINAL_HOME; + } await fs.rm(tmpDir, { recursive: true, force: true }); }); it("clears stale Claude CLI session IDs before retrying after session expiration", async () => { const sessionKey = "agent:main:subagent:cli-expired"; + const homeDir = path.join(tmpDir, "home"); + const projectsDir = path.join(homeDir, ".claude", "projects", "demo-workspace"); + process.env.HOME = homeDir; + await fs.mkdir(projectsDir, { recursive: true }); + await fs.writeFile( + path.join(projectsDir, "stale-cli-session.jsonl"), + `${JSON.stringify({ + type: "assistant", + message: { role: "assistant", content: [{ type: "text", text: "old reply" }] }, + })}\n`, + "utf-8", + ); const sessionEntry: SessionEntry = { sessionId: "session-cli-123", updatedAt: Date.now(), @@ -143,6 +161,144 @@ describe("CLI attempt execution", () => { expect(persisted[sessionKey]?.claudeCliSessionId).toBeUndefined(); }); + it("does not pass --resume when the stored Claude CLI transcript is missing", async () => { + const sessionKey = "agent:main:direct:claude-missing-transcript"; + const homeDir = path.join(tmpDir, "home"); + process.env.HOME = homeDir; + const sessionEntry: SessionEntry = { + sessionId: "openclaw-session-123", + updatedAt: Date.now(), + cliSessionBindings: { + "claude-cli": { + sessionId: "phantom-claude-session", + authProfileId: "anthropic:claude-cli", + }, + }, + cliSessionIds: { "claude-cli": "phantom-claude-session" }, + claudeCliSessionId: "phantom-claude-session", + }; + const sessionStore: Record = { [sessionKey]: sessionEntry }; + await fs.writeFile(storePath, JSON.stringify(sessionStore, null, 2), "utf-8"); + runCliAgentMock.mockResolvedValueOnce(makeCliResult("fresh cli response")); + + await runAgentAttempt({ + providerOverride: "claude-cli", + modelOverride: "opus", + cfg: {} as OpenClawConfig, + sessionEntry, + sessionId: sessionEntry.sessionId, + sessionKey, + sessionAgentId: "main", + sessionFile: path.join(tmpDir, "session.jsonl"), + workspaceDir: tmpDir, + body: "remember me", + isFallbackRetry: false, + resolvedThinkLevel: "medium", + timeoutMs: 1_000, + runId: "run-cli-missing-transcript", + opts: { senderIsOwner: false } as Parameters[0]["opts"], + runContext: {} as Parameters[0]["runContext"], + spawnedBy: undefined, + messageChannel: undefined, + skillsSnapshot: undefined, + resolvedVerboseLevel: undefined, + agentDir: tmpDir, + onAgentEvent: vi.fn(), + authProfileProvider: "claude-cli", + sessionStore, + storePath, + sessionHasHistory: false, + }); + + expect(runCliAgentMock).toHaveBeenCalledTimes(1); + expect(runCliAgentMock.mock.calls[0]?.[0]?.cliSessionId).toBeUndefined(); + expect(runCliAgentMock.mock.calls[0]?.[0]?.cliSessionBinding).toBeUndefined(); + expect(sessionStore[sessionKey]?.cliSessionBindings?.["claude-cli"]).toBeUndefined(); + expect(sessionStore[sessionKey]?.cliSessionIds?.["claude-cli"]).toBeUndefined(); + expect(sessionStore[sessionKey]?.claudeCliSessionId).toBeUndefined(); + + const persisted = JSON.parse(await fs.readFile(storePath, "utf-8")) as Record< + string, + SessionEntry + >; + expect(persisted[sessionKey]?.cliSessionBindings?.["claude-cli"]).toBeUndefined(); + expect(persisted[sessionKey]?.cliSessionIds?.["claude-cli"]).toBeUndefined(); + expect(persisted[sessionKey]?.claudeCliSessionId).toBeUndefined(); + }); + + it("keeps Claude CLI resume when the stored transcript has assistant content", async () => { + const sessionKey = "agent:main:direct:claude-transcript-present"; + const cliSessionId = "existing-claude-session"; + const homeDir = path.join(tmpDir, "home"); + const projectsDir = path.join(homeDir, ".claude", "projects", "demo-workspace"); + process.env.HOME = homeDir; + await fs.mkdir(projectsDir, { recursive: true }); + await fs.writeFile( + path.join(projectsDir, `${cliSessionId}.jsonl`), + `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [{ type: "text", text: "previous reply" }], + }, + })}\n`, + "utf-8", + ); + const sessionEntry: SessionEntry = { + sessionId: "openclaw-session-456", + updatedAt: Date.now(), + cliSessionBindings: { + "claude-cli": { + sessionId: cliSessionId, + authProfileId: "anthropic:claude-cli", + }, + }, + cliSessionIds: { "claude-cli": cliSessionId }, + claudeCliSessionId: cliSessionId, + }; + const sessionStore: Record = { [sessionKey]: sessionEntry }; + await fs.writeFile(storePath, JSON.stringify(sessionStore, null, 2), "utf-8"); + runCliAgentMock.mockResolvedValueOnce(makeCliResult("resumed cli response")); + + await runAgentAttempt({ + providerOverride: "claude-cli", + modelOverride: "opus", + cfg: {} as OpenClawConfig, + sessionEntry, + sessionId: sessionEntry.sessionId, + sessionKey, + sessionAgentId: "main", + sessionFile: path.join(tmpDir, "session.jsonl"), + workspaceDir: tmpDir, + body: "continue", + isFallbackRetry: false, + resolvedThinkLevel: "medium", + timeoutMs: 1_000, + runId: "run-cli-transcript-present", + opts: { senderIsOwner: false } as Parameters[0]["opts"], + runContext: {} as Parameters[0]["runContext"], + spawnedBy: undefined, + messageChannel: undefined, + skillsSnapshot: undefined, + resolvedVerboseLevel: undefined, + agentDir: tmpDir, + onAgentEvent: vi.fn(), + authProfileProvider: "claude-cli", + sessionStore, + storePath, + sessionHasHistory: false, + }); + + expect(runCliAgentMock).toHaveBeenCalledTimes(1); + expect(runCliAgentMock.mock.calls[0]?.[0]?.cliSessionId).toBe(cliSessionId); + expect(runCliAgentMock.mock.calls[0]?.[0]?.cliSessionBinding).toEqual({ + sessionId: cliSessionId, + authProfileId: "anthropic:claude-cli", + }); + expect(sessionStore[sessionKey]?.cliSessionIds?.["claude-cli"]).toBe(cliSessionId); + expect(sessionStore[sessionKey]?.claudeCliSessionId).toBe(cliSessionId); + }); + it("persists CLI replies into the session transcript", async () => { const sessionKey = "agent:main:subagent:cli-transcript"; const sessionEntry: SessionEntry = { diff --git a/src/agents/command/attempt-execution.helpers.ts b/src/agents/command/attempt-execution.helpers.ts index 2315346ecf5..ff9ecbbe0ea 100644 --- a/src/agents/command/attempt-execution.helpers.ts +++ b/src/agents/command/attempt-execution.helpers.ts @@ -1,4 +1,6 @@ import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import readline from "node:readline"; import { isSilentReplyPrefixText, @@ -10,24 +12,27 @@ import { /** Maximum number of JSONL records to inspect before giving up. */ const SESSION_FILE_MAX_RECORDS = 500; +const CLAUDE_PROJECTS_RELATIVE_DIR = path.join(".claude", "projects"); -/** - * Check whether a session transcript file exists and contains at least one - * assistant message, indicating that the SessionManager has flushed the - * initial user+assistant exchange to disk. - */ -export async function sessionFileHasContent(sessionFile: string | undefined): Promise { - if (!sessionFile) { +function normalizeClaudeCliSessionId(sessionId: string | undefined): string | undefined { + const trimmed = sessionId?.trim(); + if (!trimmed || trimmed.includes("\0") || trimmed.includes("/") || trimmed.includes("\\")) { + return undefined; + } + return trimmed; +} + +async function jsonlFileHasAssistantMessage(filePath: string | undefined): Promise { + if (!filePath) { return false; } try { - // Guard against symlink-following (CWE-400 / arbitrary-file-read vector). - const stat = await fs.lstat(sessionFile); - if (stat.isSymbolicLink()) { + const stat = await fs.lstat(filePath); + if (stat.isSymbolicLink() || !stat.isFile()) { return false; } - const fh = await fs.open(sessionFile, "r"); + const fh = await fs.open(filePath, "r"); try { const rl = readline.createInterface({ input: fh.createReadStream({ encoding: "utf-8" }) }); let recordCount = 0; @@ -46,10 +51,7 @@ export async function sessionFileHasContent(sessionFile: string | undefined): Pr continue; } const rec = obj as Record | null; - if ( - rec?.type === "message" && - (rec.message as Record | undefined)?.role === "assistant" - ) { + if ((rec?.message as Record | undefined)?.role === "assistant") { return true; } } @@ -62,6 +64,43 @@ export async function sessionFileHasContent(sessionFile: string | undefined): Pr } } +/** + * Check whether a session transcript file exists and contains at least one + * assistant message, indicating that the SessionManager has flushed the + * initial user+assistant exchange to disk. + */ +export async function sessionFileHasContent(sessionFile: string | undefined): Promise { + return await jsonlFileHasAssistantMessage(sessionFile); +} + +export async function claudeCliSessionTranscriptHasContent(params: { + sessionId: string | undefined; + homeDir?: string; +}): Promise { + const sessionId = normalizeClaudeCliSessionId(params.sessionId); + if (!sessionId) { + return false; + } + const homeDir = params.homeDir?.trim() || process.env.HOME || os.homedir(); + const projectsDir = path.join(homeDir, CLAUDE_PROJECTS_RELATIVE_DIR); + let projectEntries: import("node:fs").Dirent[]; + try { + projectEntries = await fs.readdir(projectsDir, { withFileTypes: true }); + } catch { + return false; + } + for (const entry of projectEntries) { + if (!entry.isDirectory()) { + continue; + } + const candidate = path.join(projectsDir, entry.name, `${sessionId}.jsonl`); + if (await jsonlFileHasAssistantMessage(candidate)) { + return true; + } + } + return false; +} + export function resolveFallbackRetryPrompt(params: { body: string; isFallbackRetry: boolean; diff --git a/src/agents/command/attempt-execution.test.ts b/src/agents/command/attempt-execution.test.ts index f869cf1c7d4..97f21822d4a 100644 --- a/src/agents/command/attempt-execution.test.ts +++ b/src/agents/command/attempt-execution.test.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { + claudeCliSessionTranscriptHasContent, createAcpVisibleTextAccumulator, resolveFallbackRetryPrompt, sessionFileHasContent, @@ -162,6 +163,73 @@ describe("sessionFileHasContent", () => { }); }); +describe("claudeCliSessionTranscriptHasContent", () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "oc-claude-session-test-")); + }); + + afterEach(async () => { + await fs.rm(tmpDir, { recursive: true, force: true }); + }); + + async function writeClaudeProjectFile(sessionId: string, content: string) { + const projectDir = path.join(tmpDir, ".claude", "projects", "demo-workspace"); + await fs.mkdir(projectDir, { recursive: true }); + const file = path.join(projectDir, `${sessionId}.jsonl`); + await fs.writeFile(file, content, "utf-8"); + return file; + } + + it("returns false when the Claude project transcript is missing or empty", async () => { + expect( + await claudeCliSessionTranscriptHasContent({ + sessionId: "missing-session", + homeDir: tmpDir, + }), + ).toBe(false); + + await writeClaudeProjectFile("empty-session", ""); + expect( + await claudeCliSessionTranscriptHasContent({ + sessionId: "empty-session", + homeDir: tmpDir, + }), + ).toBe(false); + }); + + it("returns true when the Claude project transcript has an assistant message", async () => { + await writeClaudeProjectFile( + "session-with-assistant", + `${JSON.stringify({ + type: "assistant", + message: { + role: "assistant", + content: [{ type: "text", text: "hello" }], + }, + })}\n`, + ); + + expect( + await claudeCliSessionTranscriptHasContent({ + sessionId: "session-with-assistant", + homeDir: tmpDir, + }), + ).toBe(true); + }); + + it("rejects path-like session ids instead of escaping the Claude projects tree", async () => { + await writeClaudeProjectFile("safe-session", ""); + expect( + await claudeCliSessionTranscriptHasContent({ + sessionId: "../safe-session", + homeDir: tmpDir, + }), + ).toBe(false); + }); +}); + describe("createAcpVisibleTextAccumulator", () => { it("preserves cumulative raw snapshots after stripping a glued NO_REPLY prefix", () => { const acc = createAcpVisibleTextAccumulator(); diff --git a/src/agents/command/attempt-execution.ts b/src/agents/command/attempt-execution.ts index c3871298014..59779b7cbd1 100644 --- a/src/agents/command/attempt-execution.ts +++ b/src/agents/command/attempt-execution.ts @@ -19,13 +19,17 @@ import { prepareSessionManagerForRun } from "../pi-embedded-runner/session-manag import { runEmbeddedPiAgent, type EmbeddedPiRunResult } from "../pi-embedded.js"; import { buildWorkspaceSkillSnapshot } from "../skills.js"; import { buildUsageWithNoCost } from "../stream-message-shared.js"; -import { resolveFallbackRetryPrompt } from "./attempt-execution.helpers.js"; +import { + claudeCliSessionTranscriptHasContent, + 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 { + claudeCliSessionTranscriptHasContent, createAcpVisibleTextAccumulator, resolveFallbackRetryPrompt, sessionFileHasContent, @@ -157,6 +161,10 @@ function resolveCliTranscriptReplyText(result: EmbeddedPiRunResult): string { .join("\n\n"); } +function isClaudeCliProvider(provider: string): boolean { + return provider.trim().toLowerCase() === "claude-cli"; +} + export async function persistAcpTurnTranscript(params: { body: string; finalText: string; @@ -260,7 +268,35 @@ export function runAgentAttempt(params: { : undefined; if (isCliProvider(params.providerOverride, params.cfg)) { const cliSessionBinding = getCliSessionBinding(params.sessionEntry, params.providerOverride); - const runCliWithSession = (nextCliSessionId: string | undefined) => + const resolveReusableCliSessionBinding = async () => { + if ( + !isClaudeCliProvider(params.providerOverride) || + !cliSessionBinding?.sessionId || + (await claudeCliSessionTranscriptHasContent({ sessionId: cliSessionBinding.sessionId })) + ) { + return cliSessionBinding; + } + + log.warn( + `cli session reset: provider=${sanitizeForLog(params.providerOverride)} reason=transcript-missing sessionKey=${params.sessionKey ?? params.sessionId}`, + ); + + if (params.sessionKey && params.sessionStore && params.storePath) { + params.sessionEntry = + (await clearCliSessionInStore({ + provider: params.providerOverride, + sessionKey: params.sessionKey, + sessionStore: params.sessionStore, + storePath: params.storePath, + })) ?? params.sessionEntry; + } + + return undefined; + }; + const runCliWithSession = ( + nextCliSessionId: string | undefined, + activeCliSessionBinding = cliSessionBinding, + ) => runCliAgent({ sessionId: params.sessionId, sessionKey: params.sessionKey, @@ -277,7 +313,9 @@ export function runAgentAttempt(params: { extraSystemPrompt: params.opts.extraSystemPrompt, cliSessionId: nextCliSessionId, cliSessionBinding: - nextCliSessionId === cliSessionBinding?.sessionId ? cliSessionBinding : undefined, + nextCliSessionId === activeCliSessionBinding?.sessionId + ? activeCliSessionBinding + : undefined, authProfileId, bootstrapPromptWarningSignaturesSeen, bootstrapPromptWarningSignature, @@ -289,56 +327,60 @@ export function runAgentAttempt(params: { agentAccountId: params.runContext.accountId, senderIsOwner: params.opts.senderIsOwner, }); - return runCliWithSession(cliSessionBinding?.sessionId).catch(async (err) => { - if ( - err instanceof FailoverError && - err.reason === "session_expired" && - cliSessionBinding?.sessionId && - params.sessionKey && - params.sessionStore && - params.storePath - ) { - log.warn( - `CLI session expired, clearing from session store: provider=${sanitizeForLog(params.providerOverride)} sessionKey=${params.sessionKey}`, - ); + return resolveReusableCliSessionBinding().then(async (activeCliSessionBinding) => { + try { + return await runCliWithSession(activeCliSessionBinding?.sessionId, activeCliSessionBinding); + } catch (err) { + if ( + err instanceof FailoverError && + err.reason === "session_expired" && + activeCliSessionBinding?.sessionId && + params.sessionKey && + params.sessionStore && + params.storePath + ) { + log.warn( + `CLI session expired, clearing from session store: provider=${sanitizeForLog(params.providerOverride)} sessionKey=${params.sessionKey}`, + ); - params.sessionEntry = - (await clearCliSessionInStore({ - provider: params.providerOverride, - sessionKey: params.sessionKey, - sessionStore: params.sessionStore, - storePath: params.storePath, - })) ?? params.sessionEntry; + params.sessionEntry = + (await clearCliSessionInStore({ + provider: params.providerOverride, + sessionKey: params.sessionKey, + sessionStore: params.sessionStore, + storePath: params.storePath, + })) ?? params.sessionEntry; - return runCliWithSession(undefined).then(async (result) => { - if ( - result.meta.agentMeta?.cliSessionBinding?.sessionId && - params.sessionKey && - params.sessionStore && - params.storePath - ) { - const entry = params.sessionStore[params.sessionKey]; - if (entry) { - const updatedEntry = { ...entry }; - setCliSessionBinding( - updatedEntry, - params.providerOverride, - result.meta.agentMeta.cliSessionBinding, - ); - updatedEntry.updatedAt = Date.now(); + return await runCliWithSession(undefined).then(async (result) => { + if ( + result.meta.agentMeta?.cliSessionBinding?.sessionId && + params.sessionKey && + params.sessionStore && + params.storePath + ) { + const entry = params.sessionStore[params.sessionKey]; + if (entry) { + const updatedEntry = { ...entry }; + setCliSessionBinding( + updatedEntry, + params.providerOverride, + result.meta.agentMeta.cliSessionBinding, + ); + updatedEntry.updatedAt = Date.now(); - await persistSessionEntry({ - sessionStore: params.sessionStore, - sessionKey: params.sessionKey, - storePath: params.storePath, - entry: updatedEntry, - }); + await persistSessionEntry({ + sessionStore: params.sessionStore, + sessionKey: params.sessionKey, + storePath: params.storePath, + entry: updatedEntry, + }); + } } - } - return result; - }); + return result; + }); + } + throw err; } - throw err; }); }