diff --git a/packages/agent-core/src/harness/session/memory-storage.test.ts b/packages/agent-core/src/harness/session/memory-storage.test.ts index fe479446fc86..6d4c8d62e246 100644 --- a/packages/agent-core/src/harness/session/memory-storage.test.ts +++ b/packages/agent-core/src/harness/session/memory-storage.test.ts @@ -62,6 +62,44 @@ describe("InMemorySessionStorage", () => { }); }); + it("returns a selected branch in root-to-leaf order", async () => { + const activeLeaf: SessionTreeEntry = { + type: "custom", + id: "active-leaf", + parentId: "child", + timestamp: "2026-01-01T00:00:02.000Z", + customType: "active", + }; + const sideLeaf: SessionTreeEntry = { + type: "custom", + id: "side-leaf", + parentId: "root", + timestamp: "2026-01-01T00:00:03.000Z", + customType: "side", + }; + const storage = new InMemorySessionStorage({ + entries: [rootEntry, childEntry, activeLeaf, sideLeaf], + }); + + expect((await storage.getPathToRoot(activeLeaf.id)).map((entry) => entry.id)).toEqual([ + "root", + "child", + "active-leaf", + ]); + expect((await storage.getPathToRoot(sideLeaf.id)).map((entry) => entry.id)).toEqual([ + "root", + "side-leaf", + ]); + }); + + it("normalizes session names to one line", async () => { + const session = new Session(new InMemorySessionStorage()); + + await session.appendSessionName(" first\nsecond\r\nthird "); + + expect(await session.getSessionName()).toBe("first second third"); + }); + it("traverses descendants of leaf markers through the selected target", async () => { const leafEntry: SessionTreeEntry = { type: "leaf", diff --git a/packages/agent-core/src/harness/session/session.ts b/packages/agent-core/src/harness/session/session.ts index bbab9c85c8da..a155d459784b 100644 --- a/packages/agent-core/src/harness/session/session.ts +++ b/packages/agent-core/src/harness/session/session.ts @@ -259,7 +259,7 @@ export class Session { id: await this.storage.createEntryId(), parentId: await this.getAppendParentId(), timestamp: new Date().toISOString(), - name: name.trim(), + name: name.replace(/[\r\n]+/g, " ").trim(), } satisfies SessionInfoEntry); } diff --git a/packages/agent-core/src/harness/session/storage-base.ts b/packages/agent-core/src/harness/session/storage-base.ts index 5cf654e2235e..e3da31b10ffa 100644 --- a/packages/agent-core/src/harness/session/storage-base.ts +++ b/packages/agent-core/src/harness/session/storage-base.ts @@ -252,7 +252,7 @@ export abstract class BaseSessionStorage< } seen.add(current.id); if (current.type !== "leaf") { - path.unshift(current); + path.push(current); } // Leaf rows are control records. Descendants written by older appenders // may point at the marker, but their visible ancestry starts at its target. @@ -271,6 +271,7 @@ export abstract class BaseSessionStorage< } current = parent; } + path.reverse(); return path; } diff --git a/src/agents/embedded-agent-runner/transcript-file-state.test.ts b/src/agents/embedded-agent-runner/transcript-file-state.test.ts index 9e5592d0f066..a1b04035ab9f 100644 --- a/src/agents/embedded-agent-runner/transcript-file-state.test.ts +++ b/src/agents/embedded-agent-runner/transcript-file-state.test.ts @@ -25,6 +25,28 @@ afterEach(async () => { }); describe("readTranscriptFileState", () => { + it("normalizes appended session names to one line", async () => { + const root = await makeRoot("openclaw-transcript-state-name-"); + const sessionFile = path.join(root, "session.jsonl"); + await fs.writeFile( + sessionFile, + `${JSON.stringify({ + type: "session", + version: 3, + id: "session-1", + timestamp: "2026-07-16T00:00:00.000Z", + cwd: root, + })}\n`, + "utf-8", + ); + const state = await readTranscriptFileState(sessionFile); + + const entry = state.appendSessionInfo(" first\nsecond\r\nthird "); + + expect(entry.name).toBe("first second third"); + expect(state.getEntries().at(-1)).toMatchObject({ name: "first second third" }); + }); + it("skips malformed session entries without moving the active leaf", async () => { // Bad rows are ignored for branch construction, but valid legacy orphan // roots remain reachable so partial imports can still be replayed. diff --git a/src/agents/embedded-agent-runner/transcript-file-state.ts b/src/agents/embedded-agent-runner/transcript-file-state.ts index e067fe556afe..737ffe7254cb 100644 --- a/src/agents/embedded-agent-runner/transcript-file-state.ts +++ b/src/agents/embedded-agent-runner/transcript-file-state.ts @@ -366,9 +366,10 @@ function readableSessionState(fileEntries: FileEntry[]): ReadableSessionState { if (!entry) { break; } - pathLocal.unshift(entry); + pathLocal.push(entry); id = entry.parentId; } + pathLocal.reverse(); return pathLocal; }; const firstReadableDescendantOnBranch = ( @@ -836,7 +837,7 @@ export class TranscriptFileState { id: generateEntryId(this.byId), parentId: this.appendParentId, timestamp: new Date().toISOString(), - name: name.trim(), + name: name.replace(/[\r\n]+/g, " ").trim(), }); } diff --git a/src/agents/sessions/auth-storage.test.ts b/src/agents/sessions/auth-storage.test.ts index 38ebed7dc49b..9054fbc28e9d 100644 --- a/src/agents/sessions/auth-storage.test.ts +++ b/src/agents/sessions/auth-storage.test.ts @@ -4,6 +4,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import lockfile from "proper-lockfile"; import { afterEach, describe, expect, it, vi } from "vitest"; +import type { AuthStorageBackend } from "./auth-storage.js"; // auth-storage.ts persists via the named import `writeFileSync` from node:fs, // and replaceFileAtomicSync (in @openclaw/fs-safe) writes its temp file via the @@ -127,4 +128,44 @@ describe("file auth storage", () => { await expect(storage.withLockAsync(async () => ({ result: undefined }))).rejects.toBe(error); }); + + it("throws without changing memory when malformed storage cannot be reloaded", () => { + tmpDir = fs.mkdtempSync(join(tmpdir(), "auth-malformed-")); + const authPath = join(tmpDir, "auth.json"); + fs.writeFileSync(authPath, "{invalid-json", "utf-8"); + const storage = AuthStorage.create(authPath); + + expect(() => storage.set("openai", { type: "api_key", key: "test-token-placeholder" })).toThrow( + "Cannot update auth storage because it could not be loaded", + ); + expect(storage.has("openai")).toBe(false); + expect(fs.readFileSync(authPath, "utf-8")).toBe("{invalid-json"); + }); + + it("throws without changing memory when the durable write fails", () => { + const writeError = new Error("simulated durable write failure"); + let persisted = "{}"; + const backend: AuthStorageBackend = { + withLock: (fn) => { + const update = fn(persisted); + if (update.next !== undefined) { + throw writeError; + } + return update.result; + }, + withLockAsync: async (fn) => { + const update = await fn(persisted); + if (update.next !== undefined) { + persisted = update.next; + } + return update.result; + }, + }; + const storage = AuthStorage.fromStorage(backend); + + expect(() => storage.set("openai", { type: "api_key", key: "test-token-placeholder" })).toThrow( + /simulated durable write failure/, + ); + expect(storage.has("openai")).toBe(false); + }); }); diff --git a/src/agents/sessions/auth-storage.ts b/src/agents/sessions/auth-storage.ts index 4a50887318a3..74fee78c6684 100644 --- a/src/agents/sessions/auth-storage.ts +++ b/src/agents/sessions/auth-storage.ts @@ -34,6 +34,13 @@ export type AuthCredential = ApiKeyCredential | OAuthCredential; export type AuthStorageData = Record; +class AuthStoragePersistenceError extends Error { + constructor(message: string, cause: unknown) { + super(message, { cause }); + this.name = "AuthStoragePersistenceError"; + } +} + export type AuthStatus = { configured: boolean; source?: @@ -248,11 +255,20 @@ export class AuthStorage { private persistProviderChange(provider: string, credential: AuthCredential | undefined): void { if (this.loadError) { - return; + this.reload(); + } + + if (this.loadError) { + const error = new AuthStoragePersistenceError( + `Cannot update auth storage because it could not be loaded: ${this.loadError.message}`, + this.loadError, + ); + this.recordError(error); + throw error; } try { - this.storage.withLock((current) => { + const persistedData = this.storage.withLock((current) => { const currentData = this.parseStorageData(current); const merged: AuthStorageData = { ...currentData }; if (credential) { @@ -260,10 +276,20 @@ export class AuthStorage { } else { delete merged[provider]; } - return { result: undefined, next: JSON.stringify(merged, null, 2) }; + return { result: merged, next: JSON.stringify(merged, null, 2) }; }); + this.loadError = null; + this.data = persistedData; } catch (error) { - this.recordError(error); + const persistenceError = + error instanceof AuthStoragePersistenceError + ? error + : new AuthStoragePersistenceError( + `Failed to persist auth storage update for provider "${provider}": ${error instanceof Error ? error.message : String(error)}`, + error, + ); + this.recordError(persistenceError); + throw persistenceError; } } @@ -278,7 +304,6 @@ export class AuthStorage { * Set credential for a provider. */ set(provider: string, credential: AuthCredential): void { - this.data[provider] = credential; this.persistProviderChange(provider, credential); } @@ -286,7 +311,6 @@ export class AuthStorage { * Remove credential for a provider. */ remove(provider: string): void { - delete this.data[provider]; this.persistProviderChange(provider, undefined); } diff --git a/src/agents/sessions/model-registry.test.ts b/src/agents/sessions/model-registry.test.ts index 386615a40dd3..df3dd1bb870b 100644 --- a/src/agents/sessions/model-registry.test.ts +++ b/src/agents/sessions/model-registry.test.ts @@ -150,7 +150,7 @@ describe("ModelRegistry models.json auth", () => { }); }); - it("still rejects api-key custom models without apiKey", () => { + it("uses stored auth for custom models without an inline apiKey", async () => { const modelsPath = writeModelsJson({ providers: { custom: { @@ -161,10 +161,42 @@ describe("ModelRegistry models.json auth", () => { }, }); - const registry = ModelRegistry.create(AuthStorage.inMemory(), modelsPath); + const registry = ModelRegistry.create( + AuthStorage.inMemory({ + custom: { type: "api_key", key: "test-token-placeholder" }, + }), + modelsPath, + ); + const model = registry.find("custom", "example-model"); - expect(registry.getError()).toContain('Provider custom: "apiKey" is required'); - expect(registry.find("custom", "example-model")).toBeUndefined(); + expect(registry.getError()).toBeUndefined(); + expect(registry.getAvailable()).toEqual([model]); + await expect(registry.getApiKeyForProvider("custom")).resolves.toBe("test-token-placeholder"); + }); + + it("uses stored auth for dynamically registered provider models", () => { + const authStorage = AuthStorage.inMemory({ + custom: { type: "api_key", key: "test-token-placeholder" }, + }); + const registry = ModelRegistry.inMemory(authStorage); + + registry.registerProvider("custom", { + baseUrl: "https://models.example/v1", + api: "openai-responses", + models: [ + { + id: "example-model", + name: "Example Model", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 128_000, + maxTokens: 16_384, + }, + ], + }); + + expect(registry.getAvailable().map((model) => model.id)).toEqual(["example-model"]); }); it("loads provider models from generated plugin catalog shards", () => { diff --git a/src/agents/sessions/model-registry.ts b/src/agents/sessions/model-registry.ts index 0730802f880b..3a00df7cc9a8 100644 --- a/src/agents/sessions/model-registry.ts +++ b/src/agents/sessions/model-registry.ts @@ -223,10 +223,6 @@ function formatValidationPath(error: TLocalizedValidationError): string { return path || "root"; } -function allowsMissingProviderApiKey(auth: ProviderAuthMode | undefined): boolean { - return auth === "aws-sdk" || auth === "oauth"; -} - /** Strip `//` line comments and trailing commas from JSON, leaving string literals untouched. */ function stripJsonComments(input: string): string { return input @@ -503,12 +499,6 @@ export class ModelRegistry { `Provider ${providerName}: "baseUrl" is required when defining custom models.`, ); } - if (!providerConfig.apiKey && !allowsMissingProviderApiKey(providerConfig.auth)) { - throw new Error( - `Provider ${providerName}: "apiKey" is required when defining custom models.`, - ); - } - for (const modelDef of models) { const hasModelApi = Boolean(modelDef.api); @@ -841,12 +831,6 @@ export class ModelRegistry { if (!config.baseUrl) { throw new Error(`Provider ${providerName}: "baseUrl" is required when defining models.`); } - if (!config.apiKey && !config.oauth && !allowsMissingProviderApiKey(config.auth)) { - throw new Error( - `Provider ${providerName}: "apiKey" or "oauth" is required when defining models.`, - ); - } - for (const modelDef of config.models) { const api = modelDef.api || config.api; if (!api) { diff --git a/src/agents/sessions/model-resolver.test.ts b/src/agents/sessions/model-resolver.test.ts index d94680b409b3..ed084f39e0cf 100644 --- a/src/agents/sessions/model-resolver.test.ts +++ b/src/agents/sessions/model-resolver.test.ts @@ -4,7 +4,12 @@ import { describe, expect, it } from "vitest"; import type { Model } from "../../llm/types.js"; import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../defaults.js"; import type { ModelRegistry } from "./model-registry.js"; -import { findInitialModel, parseModelPattern, restoreModelFromSession } from "./model-resolver.js"; +import { + findInitialModel, + parseModelPattern, + resolveCliModel, + restoreModelFromSession, +} from "./model-resolver.js"; function model(provider: string, id: string): Model { return { @@ -21,12 +26,13 @@ function model(provider: string, id: string): Model { }; } -function registry(models: Model[]): ModelRegistry { +function registry(models: Model[], authenticatedModels: Model[] = models): ModelRegistry { return { find: (provider: string, modelId: string) => models.find((entry) => entry.provider === provider && entry.id === modelId), - getAvailable: () => models, - hasConfiguredAuth: (entry: Model) => models.includes(entry), + getAll: () => models, + getAvailable: () => authenticatedModels, + hasConfiguredAuth: (entry: Model) => authenticatedModels.includes(entry), } as ModelRegistry; } @@ -56,6 +62,78 @@ describe("model resolver fallback selection", () => { expect(result.model).toBe(firstAvailable); }); + + it("ignores an unauthenticated saved default", async () => { + const savedDefault = model("saved-provider", "saved-model"); + const available = model("available-provider", "available-model"); + + const result = await findInitialModel({ + scopedModels: [], + isContinuing: false, + defaultProvider: savedDefault.provider, + defaultModelId: savedDefault.id, + modelRegistry: registry([savedDefault, available], [available]), + }); + + expect(result.model).toBe(available); + }); +}); + +describe("custom model fallback", () => { + it.each([ + { suffix: "high", reasoning: true }, + { suffix: "off", reasoning: false }, + ] as const)("parses :$suffix and configures reasoning", ({ suffix, reasoning }) => { + const providerModel = model("custom-provider", "known-model"); + const result = resolveCliModel({ + cliModel: `custom-provider/new-model:${suffix}`, + modelRegistry: registry([providerModel]), + }); + + expect(result.error).toBeUndefined(); + expect(result.model).toMatchObject({ + provider: "custom-provider", + id: "new-model", + reasoning, + }); + expect(result.thinkingLevel).toBe(suffix); + }); + + it("keeps an invalid suffix as part of the custom model id", () => { + const result = resolveCliModel({ + cliProvider: "custom-provider", + cliModel: "new-model:specialized", + modelRegistry: registry([model("custom-provider", "known-model")]), + }); + + expect(result.model?.id).toBe("new-model:specialized"); + expect(result.thinkingLevel).toBeUndefined(); + }); + + it("preserves an explicit thinking level for a custom model", () => { + const result = resolveCliModel({ + cliProvider: "custom-provider", + cliModel: "new-model", + cliThinking: "low", + modelRegistry: registry([model("custom-provider", "known-model")]), + }); + + expect(result.model).toMatchObject({ id: "new-model", reasoning: true }); + expect(result.thinkingLevel).toBe("low"); + }); + + it("uses the parsed thinking level during initial model selection", async () => { + const result = await findInitialModel({ + cliProvider: "custom-provider", + cliModel: "new-model:high", + scopedModels: [], + isContinuing: false, + modelRegistry: registry([model("custom-provider", "known-model")]), + }); + + expect(result.model).toMatchObject({ id: "new-model", reasoning: true }); + expect(result.thinkingLevel).toBe("high"); + }); }); describe("parseModelPattern version sorting", () => { diff --git a/src/agents/sessions/model-resolver.ts b/src/agents/sessions/model-resolver.ts index c6829bf1822d..4237d950697e 100644 --- a/src/agents/sessions/model-resolver.ts +++ b/src/agents/sessions/model-resolver.ts @@ -330,9 +330,10 @@ export interface ResolveCliModelResult { export function resolveCliModel(options: { cliProvider?: string; cliModel?: string; + cliThinking?: ThinkingLevel; modelRegistry: ModelRegistry; }): ResolveCliModelResult { - const { cliProvider, cliModel, modelRegistry } = options; + const { cliProvider, cliModel, cliThinking, modelRegistry } = options; if (!cliModel) { return { model: undefined, warning: undefined, error: undefined }; @@ -443,14 +444,32 @@ export function resolveCliModel(options: { } if (provider) { - const fallbackModel = buildFallbackModel(provider, pattern, availableModels); + let fallbackPattern = pattern; + let fallbackThinking: ThinkingLevel | undefined; + if (!cliThinking) { + const lastColon = pattern.lastIndexOf(":"); + if (lastColon !== -1) { + const suffix = pattern.slice(lastColon + 1); + if (isValidThinkingLevel(suffix)) { + fallbackPattern = pattern.slice(0, lastColon); + fallbackThinking = suffix; + } + } + } + + const fallbackModel = buildFallbackModel(provider, fallbackPattern, availableModels); if (fallbackModel) { + const requestedThinking = cliThinking ?? fallbackThinking; + const resolvedModel = + requestedThinking && requestedThinking !== "off" + ? { ...fallbackModel, reasoning: true } + : fallbackModel; const fallbackWarning = warning - ? `${warning} Model "${pattern}" not found for provider "${provider}". Using custom model id.` - : `Model "${pattern}" not found for provider "${provider}". Using custom model id.`; + ? `${warning} Model "${fallbackPattern}" not found for provider "${provider}". Using custom model id.` + : `Model "${fallbackPattern}" not found for provider "${provider}". Using custom model id.`; return { - model: fallbackModel, - thinkingLevel: undefined, + model: resolvedModel, + thinkingLevel: requestedThinking, warning: fallbackWarning, error: undefined, }; @@ -518,7 +537,7 @@ export async function findInitialModel(options: { if (resolved.model) { return { model: resolved.model, - thinkingLevel: DEFAULT_THINKING_LEVEL, + thinkingLevel: resolved.thinkingLevel ?? DEFAULT_THINKING_LEVEL, fallbackMessage: undefined, }; } @@ -537,10 +556,10 @@ export async function findInitialModel(options: { }; } - // 3. Try saved default from settings + // 3. Try saved default from settings when its auth is configured if (defaultProvider && defaultModelId) { const found = modelRegistry.find(defaultProvider, defaultModelId); - if (found) { + if (found && modelRegistry.hasConfiguredAuth(found)) { model = found; if (defaultThinkingLevel) { thinkingLevel = defaultThinkingLevel; diff --git a/src/agents/sessions/package-manager.test.ts b/src/agents/sessions/package-manager.test.ts index 54c03a12c507..b2774093e416 100644 --- a/src/agents/sessions/package-manager.test.ts +++ b/src/agents/sessions/package-manager.test.ts @@ -1,14 +1,31 @@ // Package manager tests cover resource discovery boundaries for package, // project, and npm-declared agent resources. -import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import { mkdtemp, mkdir, realpath, rm, stat, symlink, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { join, relative } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { DefaultPackageManager } from "./package-manager.js"; import { SettingsManager } from "./settings-manager.js"; const tempDirs: string[] = []; +type PackageManagerInternals = { + parseSource( + source: string, + ): + | { type: "npm"; spec: string; name: string; pinned: boolean } + | { type: "git"; host: string; path: string } + | { type: "local"; path: string }; + getNpmInstallPath( + source: { type: "npm"; spec: string; name: string; pinned: boolean }, + scope: "user" | "project" | "temporary", + ): string; + getGitInstallPath( + source: { type: "git"; host: string; path: string }, + scope: "user" | "project" | "temporary", + ): string; +}; + async function makeTempDir(prefix: string): Promise { const dir = await mkdtemp(join(tmpdir(), prefix)); tempDirs.push(dir); @@ -249,4 +266,30 @@ describe("DefaultPackageManager", () => { expect(resolved.prompts).toEqual([]); expect(resolved.themes).toEqual([]); }); + + it("keeps temporary package paths in a private per-agent directory", async () => { + const createdRoot = await makeTempDir("openclaw-package-manager-temp-"); + const root = await realpath(createdRoot); + const agentDir = join(root, "agent"); + const manager = new DefaultPackageManager({ + cwd: root, + agentDir, + settingsManager: SettingsManager.inMemory({}), + }) as unknown as PackageManagerInternals; + const npmSource = manager.parseSource("npm:@openclaw/example"); + const gitSource = manager.parseSource("https://github.com/openclaw/example.git"); + if (npmSource.type !== "npm" || gitSource.type !== "git") { + throw new Error("Expected package sources"); + } + + const npmPath = manager.getNpmInstallPath(npmSource, "temporary"); + const gitPath = manager.getGitInstallPath(gitSource, "temporary"); + const tempRoot = join(agentDir, "tmp", "resources"); + + expect(relative(tempRoot, npmPath).startsWith("..")).toBe(false); + expect(relative(tempRoot, gitPath).startsWith("..")).toBe(false); + if (process.platform !== "win32") { + expect((await stat(tempRoot)).mode & 0o777).toBe(0o700); + } + }); }); diff --git a/src/agents/sessions/package-manager.ts b/src/agents/sessions/package-manager.ts index 9abf03cc798c..4a6825932025 100644 --- a/src/agents/sessions/package-manager.ts +++ b/src/agents/sessions/package-manager.ts @@ -4,8 +4,17 @@ * Resolves extension, skill, prompt, and theme sources from npm, git, local paths, and project manifests. */ import { createHash } from "node:crypto"; -import { existsSync, globSync, readdirSync, readFileSync, realpathSync, statSync } from "node:fs"; -import { homedir, tmpdir } from "node:os"; +import { + chmodSync, + existsSync, + globSync, + mkdirSync, + readdirSync, + readFileSync, + realpathSync, + statSync, +} from "node:fs"; +import { homedir } from "node:os"; import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; import { minimatch } from "minimatch"; import { addIgnoreRules, toPosixPath, type IgnoreMatcher } from "../../shared/ignore-rules.js"; @@ -123,6 +132,14 @@ function getHomeDir(): string { return process.env.HOME || homedir(); } +function getAgentResourceTempDir(agentDir: string): string { + const tempDir = join(agentDir, "tmp", "resources"); + // Temporary packages can contain executable code, so other local users must not modify them. + mkdirSync(tempDir, { recursive: true, mode: 0o700 }); + chmodSync(tempDir, 0o700); + return tempDir; +} + function isPattern(s: string): boolean { return ( s.startsWith("!") || @@ -935,7 +952,7 @@ export class DefaultPackageManager implements PackageManager { .update(`${prefix}-${suffix ?? ""}`) .digest("hex") .slice(0, 8); - return join(tmpdir(), "openclaw-resources", prefix, hash, suffix ?? ""); + return join(getAgentResourceTempDir(this.agentDir), prefix, hash, suffix ?? ""); } private getBaseDirForScope(scope: SourceScope): string { diff --git a/src/agents/sessions/resolve-config-value.ts b/src/agents/sessions/resolve-config-value.ts index bac58dbdb1dd..04ba345a5f7c 100644 --- a/src/agents/sessions/resolve-config-value.ts +++ b/src/agents/sessions/resolve-config-value.ts @@ -4,7 +4,7 @@ */ import { execSync, spawnSync } from "node:child_process"; -import { getBashShellConfig } from "../shell-utils.js"; +import { buildShellCommandInvocation, getBashShellConfig } from "../shell-utils.js"; // Cache for shell command results (persists for process lifetime) const commandResultCache = new Map(); @@ -27,11 +27,13 @@ function executeWithConfiguredShell(command: string): { value: string | undefined; } { try { - const { shell, args } = getBashShellConfig(); - const result = spawnSync(shell, [...args, command], { + const invocation = buildShellCommandInvocation(command, getBashShellConfig()); + const [shell, ...args] = invocation.argv; + const result = spawnSync(shell, args, { encoding: "utf-8", + ...(invocation.input === undefined ? {} : { input: invocation.input }), timeout: 10000, - stdio: ["ignore", "pipe", "ignore"], + stdio: [invocation.stdin, "pipe", "ignore"], shell: false, windowsHide: true, }); diff --git a/src/agents/sessions/session-manager-codec.ts b/src/agents/sessions/session-manager-codec.ts index 0e640d88a5a1..b228de55f85d 100644 --- a/src/agents/sessions/session-manager-codec.ts +++ b/src/agents/sessions/session-manager-codec.ts @@ -132,9 +132,10 @@ export function buildSessionContext( const path: SessionEntry[] = []; let current: SessionEntry | undefined = leaf; while (current) { - path.unshift(current); + path.push(current); current = current.parentId ? byId.get(current.parentId) : undefined; } + path.reverse(); return buildCoreSessionContext(path as CoreSessionTreeEntry[]) as SessionContext; } diff --git a/src/agents/sessions/session-manager-entries.ts b/src/agents/sessions/session-manager-entries.ts index 0e2dcd9e7883..dea62ba297c9 100644 --- a/src/agents/sessions/session-manager-entries.ts +++ b/src/agents/sessions/session-manager-entries.ts @@ -127,7 +127,7 @@ export class SessionManagerEntries extends SessionManagerPersistence { id: generateSessionEntryId(this.byId), parentId: this.appendParentId, timestamp: new Date().toISOString(), - name: name.trim(), + name: name.replace(/[\r\n]+/g, " ").trim(), }; this.appendEntry(entry); return entry.id; @@ -222,12 +222,13 @@ export class SessionManagerEntries extends SessionManagerPersistence { const current = this.byId.get(currentId); if (current) { const normalizedCurrent = this.normalizeEntryParent(current); - path.unshift(normalizedCurrent); + path.push(normalizedCurrent); currentId = normalizedCurrent.parentId; } else { currentId = this.opaqueParentsById.get(currentId) ?? null; } } + path.reverse(); return path; } diff --git a/src/agents/sessions/session-manager.test.ts b/src/agents/sessions/session-manager.test.ts index c53a81bf6d1a..63f5eb014af8 100644 --- a/src/agents/sessions/session-manager.test.ts +++ b/src/agents/sessions/session-manager.test.ts @@ -19,6 +19,7 @@ import { prepareSessionManagerForRun } from "../embedded-agent-runner/session-ma import { repairSessionFileIfNeeded } from "../session-file-repair.js"; import { loadSqliteMarkedSessionFile } from "./session-manager-file.js"; import { + buildSessionContext, CURRENT_SESSION_VERSION, findMostRecentSession, loadEntriesFromFile, @@ -26,6 +27,7 @@ import { SessionManager, type FileEntry, type SessionEntry, + type SessionMessageEntry, } from "./session-manager.js"; const tempPaths: string[] = []; @@ -149,6 +151,66 @@ describe("SessionManager.open", () => { ); }); + it("preserves root-to-leaf ordering across session branches", () => { + const entries = [ + { + type: "message", + id: "root", + parentId: null, + timestamp: "2026-07-16T00:00:00.000Z", + message: { role: "user", content: "root", timestamp: 1 }, + }, + { + type: "message", + id: "main-leaf", + parentId: "root", + timestamp: "2026-07-16T00:00:01.000Z", + message: { role: "user", content: "main", timestamp: 2 }, + }, + { + type: "message", + id: "side-middle", + parentId: "root", + timestamp: "2026-07-16T00:00:02.000Z", + message: { role: "user", content: "side middle", timestamp: 3 }, + }, + { + type: "message", + id: "side-leaf", + parentId: "side-middle", + timestamp: "2026-07-16T00:00:03.000Z", + message: { role: "user", content: "side leaf", timestamp: 4 }, + }, + ] satisfies SessionMessageEntry[]; + const manager = SessionManager.inMemory(); + for (const entry of entries) { + manager.appendMessage(entry.message); + if (entry.id === "main-leaf") { + manager.branch(manager.getBranch().at(0)!.id); + } + } + + expect(buildSessionContext(entries, "side-leaf").messages).toMatchObject([ + { content: "root" }, + { content: "side middle" }, + { content: "side leaf" }, + ]); + expect( + manager + .getBranch() + .filter((entry) => entry.type === "message") + .map((entry) => entry.message), + ).toMatchObject([{ content: "root" }, { content: "side middle" }, { content: "side leaf" }]); + }); + + it("normalizes session names to one line", () => { + const manager = SessionManager.inMemory(); + + manager.appendSessionInfo(" first\nsecond\r\nthird "); + + expect(manager.getSessionName()).toBe("first second third"); + }); + it("ignores opaque SQLite rows while resolving the session cwd", async () => { const dir = await makeTempDir(); const storePath = path.join(dir, "sessions.json"); diff --git a/src/agents/sessions/tools/bash.test.ts b/src/agents/sessions/tools/bash.test.ts index 78ac2520f85f..e4162c3cb8c5 100644 --- a/src/agents/sessions/tools/bash.test.ts +++ b/src/agents/sessions/tools/bash.test.ts @@ -2,7 +2,8 @@ import path from "node:path"; // Bash tool helper tests cover conversion from model-facing timeout seconds to // timer-safe millisecond values. import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import { buildShellCommandInvocation } from "../../shell-utils.js"; import type { BashOperations } from "./bash-operations.js"; import { createBashTool, createLocalBashOperations } from "./bash.js"; import { resolveBashTimeoutMs } from "./bash.test-support.js"; @@ -19,11 +20,38 @@ describe("bash tool timeout helpers", () => { expect(resolveBashTimeoutMs(Number.MAX_SAFE_INTEGER)).toBe(MAX_TIMER_TIMEOUT_MS); }); - it("ignores absent, invalid, and non-positive timeout seconds", () => { + it("allows an absent timeout", () => { expect(resolveBashTimeoutMs(undefined)).toBeUndefined(); - expect(resolveBashTimeoutMs(Number.NaN)).toBeUndefined(); - expect(resolveBashTimeoutMs(0)).toBeUndefined(); - expect(resolveBashTimeoutMs(-1)).toBeUndefined(); + }); + + it.each([Number.NaN, 0, -1])("rejects invalid timeout %s", (timeout) => { + expect(() => resolveBashTimeoutMs(timeout)).toThrow( + "Invalid timeout: must be a positive finite number of seconds", + ); + }); + + it.each([Number.NaN, 0, -1])("rejects invalid timeout %s before execution", async (timeout) => { + const exec = vi.fn(); + const tool = createBashTool(process.cwd(), { operations: { exec } }); + + await expect( + tool.execute("call-invalid-timeout", { command: "echo ok", timeout }), + ).rejects.toThrow("Invalid timeout: must be a positive finite number of seconds"); + expect(exec).not.toHaveBeenCalled(); + }); + + it("omits the command argv and supplies stdin for legacy WSL bash", () => { + expect( + buildShellCommandInvocation("printf ready", { + shell: "C:\\Windows\\System32\\bash.exe", + args: ["-s"], + commandTransport: "stdin", + }), + ).toEqual({ + argv: ["C:\\Windows\\System32\\bash.exe", "-s"], + input: "printf ready", + stdin: "pipe", + }); }); }); diff --git a/src/agents/sessions/tools/bash.ts b/src/agents/sessions/tools/bash.ts index 9091f9b40d2e..2768dbe8632e 100644 --- a/src/agents/sessions/tools/bash.ts +++ b/src/agents/sessions/tools/bash.ts @@ -15,7 +15,12 @@ import { keyHint } from "../../modes/interactive/components/keybinding-hints.js" import { truncateToVisualLines } from "../../modes/interactive/components/visual-truncate.js"; import { theme } from "../../modes/interactive/theme/theme.js"; import type { AgentTool } from "../../runtime/index.js"; -import { getBashShellConfig, getShellEnv, killProcessTree } from "../../shell-utils.js"; +import { + buildShellCommandInvocation, + getBashShellConfig, + getShellEnv, + killProcessTree, +} from "../../shell-utils.js"; import type { ToolDefinition, ToolRenderResultOptions } from "../extensions/types.js"; import type { BashOperations } from "./bash-operations.js"; import { OutputAccumulator } from "./output-accumulator.js"; @@ -29,12 +34,15 @@ const bashSchema = Type.Object({ timeout: Type.Optional(Type.Number({ description: "Optional timeout seconds; default none." })), }); function resolveBashTimeoutMs(timeoutSeconds: unknown): number | undefined { + if (timeoutSeconds === undefined) { + return undefined; + } if ( typeof timeoutSeconds !== "number" || !Number.isFinite(timeoutSeconds) || timeoutSeconds <= 0 ) { - return undefined; + throw new Error("Invalid timeout: must be a positive finite number of seconds"); } return resolveTimerTimeoutMs(timeoutSeconds * 1000, 1); } @@ -55,21 +63,23 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas return { exec: (command, cwd, { onData, signal, timeout, env }) => { return new Promise((resolve, reject) => { - const { shell, args } = getBashShellConfig(options?.shellPath); + const shellConfig = getBashShellConfig(options?.shellPath); + const invocation = buildShellCommandInvocation(command, shellConfig); if (!existsSync(cwd)) { reject( new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`), ); return; } - const child = spawnCommand([shell, ...args, command], { + const child = spawnCommand(invocation.argv, { baseEnv: {}, buffer: false, cwd, detached: process.platform !== "win32", env: env ?? getShellEnv(), + ...(invocation.input === undefined ? {} : { input: invocation.input }), reject: false, - stdio: ["ignore", "pipe", "pipe"], + stdio: [invocation.stdin, "pipe", "pipe"], }); const releaseOutput = releaseChildProcessOutputAfterExit(child); let timedOut = false; @@ -105,7 +115,7 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas if (result instanceof Error) { throw result; } - throw new Error(`Failed to launch shell: ${shell}`, { cause: result }); + throw new Error(`Failed to launch shell: ${shellConfig.shell}`, { cause: result }); } if (timeoutHandle) { clearTimeout(timeoutHandle); @@ -310,6 +320,7 @@ export function createBashToolDefinition( ) { void toolCallId; void ctx; + resolveBashTimeoutMs(timeout); const resolvedCommand = commandPrefix ? `${commandPrefix}\n${command}` : command; const spawnContext = resolveSpawnContext(resolvedCommand, cwd, spawnHook); const output = new OutputAccumulator({ tempFilePrefix: "openclaw-bash" }); diff --git a/src/agents/sessions/tools/edit.test.ts b/src/agents/sessions/tools/edit.test.ts index 2a8207ad0674..6ead3df2ed0e 100644 --- a/src/agents/sessions/tools/edit.test.ts +++ b/src/agents/sessions/tools/edit.test.ts @@ -222,15 +222,21 @@ describe("edit tool", () => { expect(applyPatch(original, details.patch)).toBe(expected); }); - it("strips model-added metadata while retaining the strict edit schema", async () => { + it("accepts and strips model-added metadata while keeping required fields strict", async () => { const filePath = await createTempFile("before\n"); const tool = createEditTool(tmpDir); - const prepared = tool.prepareArguments?.({ + const raw = { path: filePath, reason: "model explanation", edits: [{ oldText: "before", newText: "after", reason: "why" }], - }); + }; + const prepared = tool.prepareArguments?.(raw); + expect(Value.Check(tool.parameters, raw)).toBe(true); + expect(Value.Check(tool.parameters, { edits: raw.edits })).toBe(false); + expect(Value.Check(tool.parameters, { path: filePath, edits: [{ oldText: "before" }] })).toBe( + false, + ); expect(prepared).toEqual({ path: filePath, edits: [{ oldText: "before", newText: "after" }], diff --git a/src/agents/sessions/tools/edit.ts b/src/agents/sessions/tools/edit.ts index be7e869534dd..2f04600ed63f 100644 --- a/src/agents/sessions/tools/edit.ts +++ b/src/agents/sessions/tools/edit.ts @@ -53,7 +53,7 @@ const replaceEditSchema = Type.Object( description: "Replacement text.", }), }, - { additionalProperties: false }, + {}, ); const editSchema = Type.Object( @@ -66,7 +66,7 @@ const editSchema = Type.Object( "Targeted replacements against original file; no overlap/nesting. Merge nearby changes.", }), }, - { additionalProperties: false }, + {}, ); type LegacyEditToolInput = Record & { edits?: unknown; diff --git a/src/agents/shell-utils.test.ts b/src/agents/shell-utils.test.ts index 29dc71d5f9de..695e11df4c14 100644 --- a/src/agents/shell-utils.test.ts +++ b/src/agents/shell-utils.test.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { captureEnv } from "../test-utils/env.js"; import { + buildShellCommandInvocation, detectRuntimeShell, getBashShellConfig, getShellEnv, @@ -55,6 +56,8 @@ function createTempCommandDir( return dir; } +type ShellConfig = ReturnType; + describe("getShellConfig", () => { let envSnapshot: ReturnType; const tempDirs: string[] = []; @@ -138,7 +141,11 @@ describe("getShellConfig", () => { const binDir = createTempCommandDir(tempDirs, [{ name: "zsh" }]); const shellPath = path.join(binDir, "zsh"); - expect(getShellConfig(shellPath)).toEqual({ shell: shellPath, args: ["-f", "-c"] }); + expect(getShellConfig(shellPath)).toEqual({ + shell: shellPath, + args: ["-f", "-c"], + commandTransport: "argv", + }); }); it("rejects a missing explicit custom shell path", () => { @@ -211,7 +218,52 @@ describe("getBashShellConfig", () => { process.env.ProgramFiles = programFiles; process.env.PATH = ""; - expect(getBashShellConfig()).toEqual({ shell: bashPath, args: ["-c"] }); + expect(getBashShellConfig()).toEqual({ + shell: bashPath, + args: ["-c"], + commandTransport: "argv", + }); + }); + + it.each(["System32", "Sysnative"])( + "uses stdin transport for the legacy %s WSL launcher", + (systemDirectory) => { + const shellPath = `C:\\Windows\\${systemDirectory}\\bash.exe`; + vi.spyOn(fs, "existsSync").mockImplementation((candidate) => String(candidate) === shellPath); + + expect(getBashShellConfig(shellPath)).toEqual({ + shell: shellPath, + args: ["-s"], + commandTransport: "stdin", + }); + }, + ); + + it("builds a stdin invocation for the legacy WSL launcher", () => { + const config: ShellConfig = { + shell: "C:\\Windows\\System32\\bash.exe", + args: ["-s"], + commandTransport: "stdin", + }; + + expect(buildShellCommandInvocation("printf ready", config)).toEqual({ + argv: [config.shell, "-s"], + input: "printf ready", + stdin: "pipe", + }); + }); + + it("builds an argv invocation for regular shells", () => { + const config: ShellConfig = { + shell: "/bin/bash", + args: ["-c"], + commandTransport: "argv", + }; + + expect(buildShellCommandInvocation("printf ready", config)).toEqual({ + argv: [config.shell, "-c", "printf ready"], + stdin: "ignore", + }); }); }); diff --git a/src/agents/shell-utils.ts b/src/agents/shell-utils.ts index 884079a642b9..3d5154d6f0bd 100644 --- a/src/agents/shell-utils.ts +++ b/src/agents/shell-utils.ts @@ -13,9 +13,17 @@ import { } from "../process/kill-tree.js"; import { getBinDir } from "./config.js"; -interface ShellConfig { +type ShellConfig = { shell: string; args: string[]; +} & ({ commandTransport: "argv" } | { commandTransport: "stdin" }); + +type ShellCommandInvocation = + | { argv: [string, ...string[]]; input?: undefined; stdin: "ignore" } + | { argv: [string, ...string[]]; input: string; stdin: "pipe" }; + +function createArgvShellConfig(shell: string, args: string[]): ShellConfig { + return { shell, args, commandTransport: "argv" }; } function resolvePowerShellPath(): string { @@ -93,12 +101,38 @@ function resolveWindowsBashPath(env: NodeJS.ProcessEnv = process.env): string | return resolveShellFromPath("bash.exe", env) ?? resolveShellFromPath("bash", env); } +function isLegacyWslBashPath(shellPath: string): boolean { + const normalized = shellPath.replace(/\//g, "\\").toLowerCase(); + return /(?:^|\\)windows\\(?:system32|sysnative)\\bash\.exe$/.test(normalized); +} + +function resolveBashCommandConfig(shell: string): ShellConfig { + if (isLegacyWslBashPath(shell)) { + return { shell, args: ["-s"], commandTransport: "stdin" }; + } + return createArgvShellConfig( + shell, + process.platform === "win32" ? ["-c"] : getPosixShellArgs(shell), + ); +} + +export function buildShellCommandInvocation( + command: string, + config: ShellConfig, +): ShellCommandInvocation { + if (config.commandTransport === "stdin") { + // The legacy WSL launcher mangles command argv, so its -s mode must read the command from stdin. + return { argv: [config.shell, ...config.args], input: command, stdin: "pipe" }; + } + return { argv: [config.shell, ...config.args, command], stdin: "ignore" }; +} + export function getShellConfig(customShellPath?: string): ShellConfig { if (customShellPath) { if (!fs.existsSync(customShellPath)) { throw new Error(`Custom shell path not found: ${customShellPath}`); } - return { shell: customShellPath, args: getPosixShellArgs(customShellPath) }; + return createArgvShellConfig(customShellPath, getPosixShellArgs(customShellPath)); } if (process.platform === "win32") { @@ -107,10 +141,11 @@ export function getShellConfig(customShellPath?: string): ShellConfig { // directly to the console via WriteConsole API, bypassing stdout pipes. // When Node.js spawns cmd.exe with piped stdio, these utilities produce no output. // PowerShell properly captures and redirects their output to stdout. - return { - shell: resolvePowerShellPath(), - args: ["-NoProfile", "-NonInteractive", "-Command"], - }; + return createArgvShellConfig(resolvePowerShellPath(), [ + "-NoProfile", + "-NonInteractive", + "-Command", + ]); } const rawEnvShell = process.env.SHELL?.trim(); @@ -120,20 +155,20 @@ export function getShellConfig(customShellPath?: string): ShellConfig { if (shellName === "fish") { const bash = resolveShellFromPath("bash"); if (bash) { - return { shell: bash, args: getPosixShellArgs(bash) }; + return createArgvShellConfig(bash, getPosixShellArgs(bash)); } const sh = resolveShellFromPath("sh"); if (sh) { - return { shell: sh, args: getPosixShellArgs(sh) }; + return createArgvShellConfig(sh, getPosixShellArgs(sh)); } } if (envShell) { - return { shell: envShell, args: getPosixShellArgs(envShell) }; + return createArgvShellConfig(envShell, getPosixShellArgs(envShell)); } // Placeholder SHELL (or unset): prefer a resolved sh/bash on PATH so we do not // re-invoke the placeholder and get a spurious exitCode=1. const shell = resolveShellFromPath("sh") ?? resolveShellFromPath("bash") ?? "sh"; - return { shell, args: getPosixShellArgs(shell) }; + return createArgvShellConfig(shell, getPosixShellArgs(shell)); } export function getBashShellConfig(customShellPath?: string): ShellConfig { @@ -141,19 +176,19 @@ export function getBashShellConfig(customShellPath?: string): ShellConfig { if (!fs.existsSync(customShellPath)) { throw new Error(`Custom shell path not found: ${customShellPath}`); } - return { shell: customShellPath, args: getPosixShellArgs(customShellPath) }; + return resolveBashCommandConfig(customShellPath); } if (process.platform === "win32") { const bash = resolveWindowsBashPath(); if (bash) { - return { shell: bash, args: ["-c"] }; + return resolveBashCommandConfig(bash); } throw new Error("No bash shell found. Install Git for Windows or add bash.exe to PATH."); } if (fs.existsSync("/bin/bash")) { - return { shell: "/bin/bash", args: getPosixShellArgs("/bin/bash") }; + return resolveBashCommandConfig("/bin/bash"); } const shell = @@ -161,7 +196,7 @@ export function getBashShellConfig(customShellPath?: string): ShellConfig { resolveShellFromWhich("bash") ?? resolveShellFromPath("sh") ?? "sh"; - return { shell, args: getPosixShellArgs(shell) }; + return resolveBashCommandConfig(shell); } function resolveShellFromPath(