From ad1072842e778ade84d95107381b49f403fc8863 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 17:11:17 +0000 Subject: [PATCH] test: dedupe agent tests and session helpers --- src/agents/auth-health.test.ts | 11 +- ...tize-lastgood-round-robin-ordering.test.ts | 109 +++----- ...s-stored-profiles-no-config-exists.test.ts | 87 +++--- .../oauth.fallback-to-main-agent.test.ts | 261 ++++++++---------- src/agents/auth-profiles/oauth.test.ts | 139 +++++----- src/agents/bash-tools.test.ts | 77 +++--- src/agents/compaction.retry.test.ts | 122 +++----- src/agents/memory-search.test.ts | 68 ++--- src/agents/model-fallback.probe.test.ts | 23 +- ...fault-baseurl-token-exchange-fails.test.ts | 22 +- ...subagents.sessions-spawn.lifecycle.test.ts | 94 +++---- ...helpers.buildbootstrapcontextfiles.test.ts | 18 +- .../pi-embedded-runner-extraparams.test.ts | 98 +++---- ...ed-runner.google-sanitize-thinking.test.ts | 198 ++++--------- .../pi-embedded-subscribe.e2e-harness.ts | 37 +++ ...-subscribe.lifecycle-billing-error.test.ts | 41 +-- ...session.subscribeembeddedpisession.test.ts | 21 +- ...tools.before-tool-call.integration.test.ts | 62 +++-- src/agents/pi-tools.before-tool-call.test.ts | 55 ++-- src/agents/session-write-lock.test.ts | 32 ++- src/agents/skills-install-fallback.test.ts | 77 ++---- src/agents/skills-install.download.test.ts | 2 +- src/agents/skills-install.test-mocks.ts | 32 +++ src/agents/skills-install.test.ts | 7 +- src/agents/tool-call-id.test.ts | 50 ++-- src/agents/tool-loop-detection.test.ts | 106 ++++--- src/agents/tools/cron-tool.test.ts | 176 +++++------- src/agents/tools/message-tool.test.ts | 18 +- src/agents/tools/slack-actions.test.ts | 60 ++-- src/agents/tools/web-search.ts | 14 +- src/test-utils/auth-token-assertions.ts | 13 + 31 files changed, 1021 insertions(+), 1109 deletions(-) create mode 100644 src/agents/skills-install.test-mocks.ts create mode 100644 src/test-utils/auth-token-assertions.ts diff --git a/src/agents/auth-health.test.ts b/src/agents/auth-health.test.ts index 97da6f280f8..a6d5b80b8f8 100644 --- a/src/agents/auth-health.test.ts +++ b/src/agents/auth-health.test.ts @@ -7,6 +7,9 @@ import { describe("buildAuthHealthSummary", () => { const now = 1_700_000_000_000; + const profileStatuses = (summary: ReturnType) => + Object.fromEntries(summary.profiles.map((profile) => [profile.profileId, profile.status])); + afterEach(() => { vi.restoreAllMocks(); }); @@ -50,9 +53,7 @@ describe("buildAuthHealthSummary", () => { warnAfterMs: DEFAULT_OAUTH_WARN_MS, }); - const statuses = Object.fromEntries( - summary.profiles.map((profile) => [profile.profileId, profile.status]), - ); + const statuses = profileStatuses(summary); expect(statuses["anthropic:ok"]).toBe("ok"); // OAuth credentials with refresh tokens are auto-renewable, so they report "ok" @@ -84,9 +85,7 @@ describe("buildAuthHealthSummary", () => { warnAfterMs: DEFAULT_OAUTH_WARN_MS, }); - const statuses = Object.fromEntries( - summary.profiles.map((profile) => [profile.profileId, profile.status]), - ); + const statuses = profileStatuses(summary); expect(statuses["google:no-refresh"]).toBe("expired"); }); diff --git a/src/agents/auth-profiles.resolve-auth-profile-order.does-not-prioritize-lastgood-round-robin-ordering.test.ts b/src/agents/auth-profiles.resolve-auth-profile-order.does-not-prioritize-lastgood-round-robin-ordering.test.ts index ae2b636f8c3..a13ce8fd06d 100644 --- a/src/agents/auth-profiles.resolve-auth-profile-order.does-not-prioritize-lastgood-round-robin-ordering.test.ts +++ b/src/agents/auth-profiles.resolve-auth-profile-order.does-not-prioritize-lastgood-round-robin-ordering.test.ts @@ -10,6 +10,29 @@ describe("resolveAuthProfileOrder", () => { const store = ANTHROPIC_STORE; const cfg = ANTHROPIC_CFG; + function resolveWithAnthropicOrderAndUsage(params: { + orderSource: "store" | "config"; + usageStats: NonNullable; + }) { + const configuredOrder = { anthropic: ["anthropic:default", "anthropic:work"] }; + return resolveAuthProfileOrder({ + cfg: + params.orderSource === "config" + ? { + auth: { + order: configuredOrder, + profiles: cfg.auth?.profiles, + }, + } + : undefined, + store: + params.orderSource === "store" + ? { ...store, order: configuredOrder, usageStats: params.usageStats } + : { ...store, usageStats: params.usageStats }, + provider: "anthropic", + }); + } + it("does not prioritize lastGood over round-robin ordering", () => { const order = resolveAuthProfileOrder({ cfg, @@ -62,47 +85,27 @@ describe("resolveAuthProfileOrder", () => { }); expect(order).toEqual(["anthropic:work", "anthropic:default"]); }); - it("pushes cooldown profiles to the end even with store order", () => { - const now = Date.now(); - const order = resolveAuthProfileOrder({ - store: { - ...store, - order: { anthropic: ["anthropic:default", "anthropic:work"] }, + it.each(["store", "config"] as const)( + "pushes cooldown profiles to the end even with %s order", + (orderSource) => { + const now = Date.now(); + const order = resolveWithAnthropicOrderAndUsage({ + orderSource, usageStats: { "anthropic:default": { cooldownUntil: now + 60_000 }, "anthropic:work": { lastUsed: 1 }, }, - }, - provider: "anthropic", - }); - expect(order).toEqual(["anthropic:work", "anthropic:default"]); - }); - it("pushes cooldown profiles to the end even with configured order", () => { - const now = Date.now(); - const order = resolveAuthProfileOrder({ - cfg: { - auth: { - order: { anthropic: ["anthropic:default", "anthropic:work"] }, - profiles: cfg.auth?.profiles, - }, - }, - store: { - ...store, - usageStats: { - "anthropic:default": { cooldownUntil: now + 60_000 }, - "anthropic:work": { lastUsed: 1 }, - }, - }, - provider: "anthropic", - }); - expect(order).toEqual(["anthropic:work", "anthropic:default"]); - }); - it("pushes disabled profiles to the end even with store order", () => { - const now = Date.now(); - const order = resolveAuthProfileOrder({ - store: { - ...store, - order: { anthropic: ["anthropic:default", "anthropic:work"] }, + }); + expect(order).toEqual(["anthropic:work", "anthropic:default"]); + }, + ); + + it.each(["store", "config"] as const)( + "pushes disabled profiles to the end even with %s order", + (orderSource) => { + const now = Date.now(); + const order = resolveWithAnthropicOrderAndUsage({ + orderSource, usageStats: { "anthropic:default": { disabledUntil: now + 60_000, @@ -110,34 +113,10 @@ describe("resolveAuthProfileOrder", () => { }, "anthropic:work": { lastUsed: 1 }, }, - }, - provider: "anthropic", - }); - expect(order).toEqual(["anthropic:work", "anthropic:default"]); - }); - it("pushes disabled profiles to the end even with configured order", () => { - const now = Date.now(); - const order = resolveAuthProfileOrder({ - cfg: { - auth: { - order: { anthropic: ["anthropic:default", "anthropic:work"] }, - profiles: cfg.auth?.profiles, - }, - }, - store: { - ...store, - usageStats: { - "anthropic:default": { - disabledUntil: now + 60_000, - disabledReason: "billing", - }, - "anthropic:work": { lastUsed: 1 }, - }, - }, - provider: "anthropic", - }); - expect(order).toEqual(["anthropic:work", "anthropic:default"]); - }); + }); + expect(order).toEqual(["anthropic:work", "anthropic:default"]); + }, + ); it("mode: oauth config accepts both oauth and token credentials (issue #559)", () => { const now = Date.now(); diff --git a/src/agents/auth-profiles.resolve-auth-profile-order.uses-stored-profiles-no-config-exists.test.ts b/src/agents/auth-profiles.resolve-auth-profile-order.uses-stored-profiles-no-config-exists.test.ts index eebbc030c9d..c4e49dbe400 100644 --- a/src/agents/auth-profiles.resolve-auth-profile-order.uses-stored-profiles-no-config-exists.test.ts +++ b/src/agents/auth-profiles.resolve-auth-profile-order.uses-stored-profiles-no-config-exists.test.ts @@ -9,6 +9,32 @@ describe("resolveAuthProfileOrder", () => { const store = ANTHROPIC_STORE; const cfg = ANTHROPIC_CFG; + function resolveMinimaxOrderWithProfile(profile: { + type: "token"; + provider: "minimax"; + token: string; + expires?: number; + }) { + return resolveAuthProfileOrder({ + cfg: { + auth: { + order: { + minimax: ["minimax:default"], + }, + }, + }, + store: { + version: 1, + profiles: { + "minimax:default": { + ...profile, + }, + }, + }, + provider: "minimax", + }); + } + it("uses stored profiles when no config exists", () => { const order = resolveAuthProfileOrder({ store, @@ -145,51 +171,26 @@ describe("resolveAuthProfileOrder", () => { }); expect(order).toEqual(["minimax:prod"]); }); - it("drops token profiles with empty credentials", () => { - const order = resolveAuthProfileOrder({ - cfg: { - auth: { - order: { - minimax: ["minimax:default"], - }, - }, + it.each([ + { + caseName: "drops token profiles with empty credentials", + profile: { + type: "token" as const, + provider: "minimax" as const, + token: " ", }, - store: { - version: 1, - profiles: { - "minimax:default": { - type: "token", - provider: "minimax", - token: " ", - }, - }, + }, + { + caseName: "drops token profiles that are already expired", + profile: { + type: "token" as const, + provider: "minimax" as const, + token: "sk-minimax", + expires: Date.now() - 1000, }, - provider: "minimax", - }); - expect(order).toEqual([]); - }); - it("drops token profiles that are already expired", () => { - const order = resolveAuthProfileOrder({ - cfg: { - auth: { - order: { - minimax: ["minimax:default"], - }, - }, - }, - store: { - version: 1, - profiles: { - "minimax:default": { - type: "token", - provider: "minimax", - token: "sk-minimax", - expires: Date.now() - 1000, - }, - }, - }, - provider: "minimax", - }); + }, + ])("$caseName", ({ profile }) => { + const order = resolveMinimaxOrderWithProfile(profile); expect(order).toEqual([]); }); it("keeps oauth profiles that can refresh", () => { diff --git a/src/agents/auth-profiles/oauth.fallback-to-main-agent.test.ts b/src/agents/auth-profiles/oauth.fallback-to-main-agent.test.ts index ce745cdb051..62a38347bcd 100644 --- a/src/agents/auth-profiles/oauth.fallback-to-main-agent.test.ts +++ b/src/agents/auth-profiles/oauth.fallback-to-main-agent.test.ts @@ -30,6 +30,50 @@ describe("resolveApiKeyForProfile fallback to main agent", () => { process.env.PI_CODING_AGENT_DIR = mainAgentDir; }); + function createOauthStore(params: { + profileId: string; + access: string; + refresh: string; + expires: number; + provider?: string; + }): AuthProfileStore { + return { + version: 1, + profiles: { + [params.profileId]: { + type: "oauth", + provider: params.provider ?? "anthropic", + access: params.access, + refresh: params.refresh, + expires: params.expires, + }, + }, + }; + } + + async function writeAuthProfilesStore(agentDir: string, store: AuthProfileStore) { + await fs.writeFile(path.join(agentDir, "auth-profiles.json"), JSON.stringify(store)); + } + + function stubOAuthRefreshFailure() { + const fetchSpy = vi.fn(async () => { + return new Response(JSON.stringify({ error: "invalid_grant" }), { + status: 400, + headers: { "Content-Type": "application/json" }, + }); + }); + vi.stubGlobal("fetch", fetchSpy); + } + + async function resolveFromSecondaryAgent(profileId: string) { + const loadedSecondaryStore = ensureAuthProfileStore(secondaryAgentDir); + return resolveApiKeyForProfile({ + store: loadedSecondaryStore, + profileId, + agentDir: secondaryAgentDir, + }); + } + afterEach(async () => { vi.unstubAllGlobals(); @@ -78,60 +122,34 @@ describe("resolveApiKeyForProfile fallback to main agent", () => { const freshTime = now + 60 * 60 * 1000; // 1 hour from now // Write expired credentials for secondary agent - const secondaryStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "expired-access-token", - refresh: "expired-refresh-token", - expires: expiredTime, - }, - }, - }; - await fs.writeFile( - path.join(secondaryAgentDir, "auth-profiles.json"), - JSON.stringify(secondaryStore), + await writeAuthProfilesStore( + secondaryAgentDir, + createOauthStore({ + profileId, + access: "expired-access-token", + refresh: "expired-refresh-token", + expires: expiredTime, + }), ); // Write fresh credentials for main agent - const mainStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "fresh-access-token", - refresh: "fresh-refresh-token", - expires: freshTime, - }, - }, - }; - await fs.writeFile(path.join(mainAgentDir, "auth-profiles.json"), JSON.stringify(mainStore)); + await writeAuthProfilesStore( + mainAgentDir, + createOauthStore({ + profileId, + access: "fresh-access-token", + refresh: "fresh-refresh-token", + expires: freshTime, + }), + ); // Mock fetch to simulate OAuth refresh failure - const fetchSpy = vi.fn(async () => { - return new Response(JSON.stringify({ error: "invalid_grant" }), { - status: 400, - headers: { "Content-Type": "application/json" }, - }); - }); - vi.stubGlobal("fetch", fetchSpy); + stubOAuthRefreshFailure(); // Load the secondary agent's store (will merge with main agent's store) - const loadedSecondaryStore = ensureAuthProfileStore(secondaryAgentDir); - - // Call resolveApiKeyForProfile with the secondary agent's expired credentials - // This should: - // 1. Try to refresh the expired token (fails due to mocked fetch) - // 2. Fall back to main agent's fresh credentials - // 3. Copy those credentials to the secondary agent - const result = await resolveApiKeyForProfile({ - store: loadedSecondaryStore, - profileId, - agentDir: secondaryAgentDir, - }); + // Call resolveApiKeyForProfile with the secondary agent's expired credentials: + // refresh fails, then fallback copies main credentials to secondary. + const result = await resolveFromSecondaryAgent(profileId); expect(result).not.toBeNull(); expect(result?.apiKey).toBe("fresh-access-token"); @@ -153,43 +171,27 @@ describe("resolveApiKeyForProfile fallback to main agent", () => { const secondaryExpiry = now + 30 * 60 * 1000; const mainExpiry = now + 2 * 60 * 60 * 1000; - const secondaryStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "secondary-access-token", - refresh: "secondary-refresh-token", - expires: secondaryExpiry, - }, - }, - }; - await fs.writeFile( - path.join(secondaryAgentDir, "auth-profiles.json"), - JSON.stringify(secondaryStore), + await writeAuthProfilesStore( + secondaryAgentDir, + createOauthStore({ + profileId, + access: "secondary-access-token", + refresh: "secondary-refresh-token", + expires: secondaryExpiry, + }), ); - const mainStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "main-newer-access-token", - refresh: "main-newer-refresh-token", - expires: mainExpiry, - }, - }, - }; - await fs.writeFile(path.join(mainAgentDir, "auth-profiles.json"), JSON.stringify(mainStore)); + await writeAuthProfilesStore( + mainAgentDir, + createOauthStore({ + profileId, + access: "main-newer-access-token", + refresh: "main-newer-refresh-token", + expires: mainExpiry, + }), + ); - const loadedSecondaryStore = ensureAuthProfileStore(secondaryAgentDir); - const result = await resolveApiKeyForProfile({ - store: loadedSecondaryStore, - profileId, - agentDir: secondaryAgentDir, - }); + const result = await resolveFromSecondaryAgent(profileId); expect(result?.apiKey).toBe("main-newer-access-token"); @@ -207,43 +209,27 @@ describe("resolveApiKeyForProfile fallback to main agent", () => { const now = Date.now(); const mainExpiry = now + 2 * 60 * 60 * 1000; - const secondaryStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "secondary-stale", - refresh: "secondary-refresh", - expires: NaN, - }, - }, - }; - await fs.writeFile( - path.join(secondaryAgentDir, "auth-profiles.json"), - JSON.stringify(secondaryStore), + await writeAuthProfilesStore( + secondaryAgentDir, + createOauthStore({ + profileId, + access: "secondary-stale", + refresh: "secondary-refresh", + expires: NaN, + }), ); - const mainStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "main-fresh-token", - refresh: "main-refresh", - expires: mainExpiry, - }, - }, - }; - await fs.writeFile(path.join(mainAgentDir, "auth-profiles.json"), JSON.stringify(mainStore)); + await writeAuthProfilesStore( + mainAgentDir, + createOauthStore({ + profileId, + access: "main-fresh-token", + refresh: "main-refresh", + expires: mainExpiry, + }), + ); - const loadedSecondaryStore = ensureAuthProfileStore(secondaryAgentDir); - const result = await resolveApiKeyForProfile({ - store: loadedSecondaryStore, - profileId, - agentDir: secondaryAgentDir, - }); + const result = await resolveFromSecondaryAgent(profileId); expect(result?.apiKey).toBe("main-fresh-token"); }); @@ -298,42 +284,21 @@ describe("resolveApiKeyForProfile fallback to main agent", () => { const expiredTime = now - 60 * 60 * 1000; // 1 hour ago // Write expired credentials for both agents - const expiredStore: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "oauth", - provider: "anthropic", - access: "expired-access-token", - refresh: "expired-refresh-token", - expires: expiredTime, - }, - }, - }; - await fs.writeFile( - path.join(secondaryAgentDir, "auth-profiles.json"), - JSON.stringify(expiredStore), - ); - await fs.writeFile(path.join(mainAgentDir, "auth-profiles.json"), JSON.stringify(expiredStore)); + const expiredStore = createOauthStore({ + profileId, + access: "expired-access-token", + refresh: "expired-refresh-token", + expires: expiredTime, + }); + await writeAuthProfilesStore(secondaryAgentDir, expiredStore); + await writeAuthProfilesStore(mainAgentDir, expiredStore); // Mock fetch to simulate OAuth refresh failure - const fetchSpy = vi.fn(async () => { - return new Response(JSON.stringify({ error: "invalid_grant" }), { - status: 400, - headers: { "Content-Type": "application/json" }, - }); - }); - vi.stubGlobal("fetch", fetchSpy); - - const loadedSecondaryStore = ensureAuthProfileStore(secondaryAgentDir); + stubOAuthRefreshFailure(); // Should throw because both agents have expired credentials - await expect( - resolveApiKeyForProfile({ - store: loadedSecondaryStore, - profileId, - agentDir: secondaryAgentDir, - }), - ).rejects.toThrow(/OAuth token refresh failed/); + await expect(resolveFromSecondaryAgent(profileId)).rejects.toThrow( + /OAuth token refresh failed/, + ); }); }); diff --git a/src/agents/auth-profiles/oauth.test.ts b/src/agents/auth-profiles/oauth.test.ts index 60c112aef68..a91d3e4a5b7 100644 --- a/src/agents/auth-profiles/oauth.test.ts +++ b/src/agents/auth-profiles/oauth.test.ts @@ -13,6 +13,38 @@ function cfgFor(profileId: string, provider: string, mode: "api_key" | "token" | } satisfies OpenClawConfig; } +function tokenStore(params: { + profileId: string; + provider: string; + token: string; + expires?: number; +}): AuthProfileStore { + return { + version: 1, + profiles: { + [params.profileId]: { + type: "token", + provider: params.provider, + token: params.token, + ...(params.expires !== undefined ? { expires: params.expires } : {}), + }, + }, + }; +} + +async function resolveWithConfig(params: { + profileId: string; + provider: string; + mode: "api_key" | "token" | "oauth"; + store: AuthProfileStore; +}) { + return resolveApiKeyForProfile({ + cfg: cfgFor(params.profileId, params.provider, params.mode), + store: params.store, + profileId: params.profileId, + }); +} + describe("resolveApiKeyForProfile config compatibility", () => { it("accepts token credentials when config mode is oauth", async () => { const profileId = "anthropic:token"; @@ -41,21 +73,31 @@ describe("resolveApiKeyForProfile config compatibility", () => { it("rejects token credentials when config mode is api_key", async () => { const profileId = "anthropic:token"; - const store: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "token", - provider: "anthropic", - token: "tok-123", - }, - }, - }; - - const result = await resolveApiKeyForProfile({ - cfg: cfgFor(profileId, "anthropic", "api_key"), - store, + const result = await resolveWithConfig({ profileId, + provider: "anthropic", + mode: "api_key", + store: tokenStore({ + profileId, + provider: "anthropic", + token: "tok-123", + }), + }); + + expect(result).toBeNull(); + }); + + it("rejects credentials when provider does not match config", async () => { + const profileId = "anthropic:token"; + const result = await resolveWithConfig({ + profileId, + provider: "openai", + mode: "token", + store: tokenStore({ + profileId, + provider: "anthropic", + token: "tok-123", + }), }); expect(result).toBeNull(); }); @@ -87,70 +129,37 @@ describe("resolveApiKeyForProfile config compatibility", () => { email: undefined, }); }); - - it("rejects credentials when provider does not match config", async () => { - const profileId = "anthropic:token"; - const store: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "token", - provider: "anthropic", - token: "tok-123", - }, - }, - }; - - const result = await resolveApiKeyForProfile({ - cfg: cfgFor(profileId, "openai", "token"), - store, - profileId, - }); - expect(result).toBeNull(); - }); }); describe("resolveApiKeyForProfile token expiry handling", () => { it("returns null for expired token credentials", async () => { const profileId = "anthropic:token-expired"; - const store: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "token", - provider: "anthropic", - token: "tok-expired", - expires: Date.now() - 1_000, - }, - }, - }; - - const result = await resolveApiKeyForProfile({ - cfg: cfgFor(profileId, "anthropic", "token"), - store, + const result = await resolveWithConfig({ profileId, + provider: "anthropic", + mode: "token", + store: tokenStore({ + profileId, + provider: "anthropic", + token: "tok-expired", + expires: Date.now() - 1_000, + }), }); expect(result).toBeNull(); }); it("accepts token credentials when expires is 0", async () => { const profileId = "anthropic:token-no-expiry"; - const store: AuthProfileStore = { - version: 1, - profiles: { - [profileId]: { - type: "token", - provider: "anthropic", - token: "tok-123", - expires: 0, - }, - }, - }; - - const result = await resolveApiKeyForProfile({ - cfg: cfgFor(profileId, "anthropic", "token"), - store, + const result = await resolveWithConfig({ profileId, + provider: "anthropic", + mode: "token", + store: tokenStore({ + profileId, + provider: "anthropic", + token: "tok-123", + expires: 0, + }), }); expect(result).toEqual({ apiKey: "tok-123", diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index a22ba9be0f8..435e344a68d 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -34,6 +34,14 @@ const normalizeText = (value?: string) => .join("\n") .trim(); +function captureShellEnv() { + const envSnapshot = captureEnv(["SHELL"]); + if (!isWin && defaultShell) { + process.env.SHELL = defaultShell; + } + return envSnapshot; +} + async function waitForCompletion(sessionId: string) { let status = "running"; await expect @@ -62,6 +70,34 @@ async function runBackgroundEchoLines(lines: string[]) { return sessionId; } +async function readProcessLog( + sessionId: string, + options: { offset?: number; limit?: number } = {}, +) { + return processTool.execute("call-log", { + action: "log", + sessionId, + ...options, + }); +} + +async function runBackgroundAndWaitForCompletion(params: { + tool: ReturnType; + callId: string; + command: string; +}) { + const result = await params.tool.execute(params.callId, { + command: params.command, + background: true, + }); + + expect(result.details.status).toBe("running"); + const sessionId = (result.details as { sessionId: string }).sessionId; + const status = await waitForCompletion(sessionId); + expect(status).toBe("completed"); + return { sessionId }; +} + beforeEach(() => { resetProcessRegistryForTests(); resetSystemEventsForTest(); @@ -71,10 +107,7 @@ describe("exec tool backgrounding", () => { let envSnapshot: ReturnType; beforeEach(() => { - envSnapshot = captureEnv(["SHELL"]); - if (!isWin && defaultShell) { - process.env.SHELL = defaultShell; - } + envSnapshot = captureShellEnv(); }); afterEach(() => { @@ -225,10 +258,7 @@ describe("exec tool backgrounding", () => { const lines = Array.from({ length: 201 }, (_value, index) => `line-${index + 1}`); const sessionId = await runBackgroundEchoLines(lines); - const log = await processTool.execute("call2", { - action: "log", - sessionId, - }); + const log = await readProcessLog(sessionId); const textBlock = log.content.find((c) => c.type === "text")?.text ?? ""; const firstLine = textBlock.split("\n")[0]?.trim(); expect(textBlock).toContain("showing last 200 of 201 lines"); @@ -260,11 +290,7 @@ describe("exec tool backgrounding", () => { const lines = Array.from({ length: 201 }, (_value, index) => `line-${index + 1}`); const sessionId = await runBackgroundEchoLines(lines); - const log = await processTool.execute("call2", { - action: "log", - sessionId, - offset: 30, - }); + const log = await readProcessLog(sessionId, { offset: 30 }); const textBlock = log.content.find((c) => c.type === "text")?.text ?? ""; const renderedLines = textBlock.split("\n"); @@ -310,10 +336,7 @@ describe("exec exit codes", () => { let envSnapshot: ReturnType; beforeEach(() => { - envSnapshot = captureEnv(["SHELL"]); - if (!isWin && defaultShell) { - process.env.SHELL = defaultShell; - } + envSnapshot = captureShellEnv(); }); afterEach(() => { @@ -384,15 +407,11 @@ describe("exec notifyOnExit", () => { sessionKey: "agent:main:main", }); - const result = await tool.execute("call2", { + await runBackgroundAndWaitForCompletion({ + tool, + callId: "call2", command: shortDelayCmd, - background: true, }); - - expect(result.details.status).toBe("running"); - const sessionId = (result.details as { sessionId: string }).sessionId; - const status = await waitForCompletion(sessionId); - expect(status).toBe("completed"); expect(peekSystemEvents("agent:main:main")).toEqual([]); }); @@ -405,15 +424,11 @@ describe("exec notifyOnExit", () => { sessionKey: "agent:main:main", }); - const result = await tool.execute("call3", { + await runBackgroundAndWaitForCompletion({ + tool, + callId: "call3", command: shortDelayCmd, - background: true, }); - - expect(result.details.status).toBe("running"); - const sessionId = (result.details as { sessionId: string }).sessionId; - const status = await waitForCompletion(sessionId); - expect(status).toBe("completed"); const events = peekSystemEvents("agent:main:main"); expect(events.length).toBeGreaterThan(0); expect(events.some((event) => event.includes("Exec completed"))).toBe(true); diff --git a/src/agents/compaction.retry.test.ts b/src/agents/compaction.retry.test.ts index 50fe043cb91..078ceffed85 100644 --- a/src/agents/compaction.retry.test.ts +++ b/src/agents/compaction.retry.test.ts @@ -34,26 +34,22 @@ describe("compaction retry integration", () => { model: "claude-3-opus", } as unknown as NonNullable; + const invokeGenerateSummary = (signal = new AbortController().signal) => + mockGenerateSummary(testMessages, testModel, 1000, "test-api-key", signal); + + const runSummaryRetry = (options: Parameters[1]) => + retryAsync(() => invokeGenerateSummary(), options); + it("should successfully call generateSummary with retry wrapper", async () => { mockGenerateSummary.mockResolvedValueOnce("Test summary"); - const result = await retryAsync( - () => - mockGenerateSummary( - testMessages, - testModel, - 1000, - "test-api-key", - new AbortController().signal, - ), - { - attempts: 3, - minDelayMs: 500, - maxDelayMs: 5000, - jitter: 0.2, - label: "compaction/generateSummary", - }, - ); + const result = await runSummaryRetry({ + attempts: 3, + minDelayMs: 500, + maxDelayMs: 5000, + jitter: 0.2, + label: "compaction/generateSummary", + }); expect(result).toBe("Test summary"); expect(mockGenerateSummary).toHaveBeenCalledTimes(1); @@ -64,22 +60,12 @@ describe("compaction retry integration", () => { .mockRejectedValueOnce(new Error("Network timeout")) .mockResolvedValueOnce("Success after retry"); - const result = await retryAsync( - () => - mockGenerateSummary( - testMessages, - testModel, - 1000, - "test-api-key", - new AbortController().signal, - ), - { - attempts: 3, - minDelayMs: 0, - maxDelayMs: 0, - label: "compaction/generateSummary", - }, - ); + const result = await runSummaryRetry({ + attempts: 3, + minDelayMs: 0, + maxDelayMs: 0, + label: "compaction/generateSummary", + }); expect(result).toBe("Success after retry"); expect(mockGenerateSummary).toHaveBeenCalledTimes(2); @@ -93,22 +79,12 @@ describe("compaction retry integration", () => { mockGenerateSummary.mockRejectedValueOnce(abortErr); await expect( - retryAsync( - () => - mockGenerateSummary( - testMessages, - testModel, - 1000, - "test-api-key", - new AbortController().signal, - ), - { - attempts: 3, - minDelayMs: 0, - label: "compaction/generateSummary", - shouldRetry: (err: unknown) => !(err instanceof Error && err.name === "AbortError"), - }, - ), + retryAsync(() => invokeGenerateSummary(), { + attempts: 3, + minDelayMs: 0, + label: "compaction/generateSummary", + shouldRetry: (err: unknown) => !(err instanceof Error && err.name === "AbortError"), + }), ).rejects.toThrow("aborted"); // Should NOT retry on user cancellation (AbortError filtered by shouldRetry) @@ -119,22 +95,12 @@ describe("compaction retry integration", () => { mockGenerateSummary.mockRejectedValue(new Error("Persistent API error")); await expect( - retryAsync( - () => - mockGenerateSummary( - testMessages, - testModel, - 1000, - "test-api-key", - new AbortController().signal, - ), - { - attempts: 3, - minDelayMs: 0, - maxDelayMs: 0, - label: "compaction/generateSummary", - }, - ), + runSummaryRetry({ + attempts: 3, + minDelayMs: 0, + maxDelayMs: 0, + label: "compaction/generateSummary", + }), ).rejects.toThrow("Persistent API error"); expect(mockGenerateSummary).toHaveBeenCalledTimes(3); @@ -149,24 +115,14 @@ describe("compaction retry integration", () => { .mockResolvedValueOnce("Success on 3rd attempt"); const delays: number[] = []; - const promise = retryAsync( - () => - mockGenerateSummary( - testMessages, - testModel, - 1000, - "test-api-key", - new AbortController().signal, - ), - { - attempts: 3, - minDelayMs: 500, - maxDelayMs: 5000, - jitter: 0, - label: "compaction/generateSummary", - onRetry: (info) => delays.push(info.delayMs), - }, - ); + const promise = runSummaryRetry({ + attempts: 3, + minDelayMs: 500, + maxDelayMs: 5000, + jitter: 0, + label: "compaction/generateSummary", + onRetry: (info) => delays.push(info.delayMs), + }); await vi.runAllTimersAsync(); const result = await promise; diff --git a/src/agents/memory-search.test.ts b/src/agents/memory-search.test.ts index 0e0d8f83f53..8316346a828 100644 --- a/src/agents/memory-search.test.ts +++ b/src/agents/memory-search.test.ts @@ -5,6 +5,28 @@ import { resolveMemorySearchConfig } from "./memory-search.js"; const asConfig = (cfg: OpenClawConfig): OpenClawConfig => cfg; describe("memory search config", () => { + function configWithDefaultProvider(provider: "openai" | "local" | "gemini"): OpenClawConfig { + return asConfig({ + agents: { + defaults: { + memorySearch: { + provider, + }, + }, + }, + }); + } + + function expectDefaultRemoteBatch(resolved: ReturnType): void { + expect(resolved?.remote?.batch).toEqual({ + enabled: false, + wait: true, + concurrency: 2, + pollIntervalMs: 2000, + timeoutMinutes: 60, + }); + } + it("returns null when disabled", () => { const cfg = asConfig({ agents: { @@ -108,57 +130,21 @@ describe("memory search config", () => { }); it("includes batch defaults for openai without remote overrides", () => { - const cfg = asConfig({ - agents: { - defaults: { - memorySearch: { - provider: "openai", - }, - }, - }, - }); + const cfg = configWithDefaultProvider("openai"); const resolved = resolveMemorySearchConfig(cfg, "main"); - expect(resolved?.remote?.batch).toEqual({ - enabled: false, - wait: true, - concurrency: 2, - pollIntervalMs: 2000, - timeoutMinutes: 60, - }); + expectDefaultRemoteBatch(resolved); }); it("keeps remote unset for local provider without overrides", () => { - const cfg = asConfig({ - agents: { - defaults: { - memorySearch: { - provider: "local", - }, - }, - }, - }); + const cfg = configWithDefaultProvider("local"); const resolved = resolveMemorySearchConfig(cfg, "main"); expect(resolved?.remote).toBeUndefined(); }); it("includes remote defaults for gemini without overrides", () => { - const cfg = asConfig({ - agents: { - defaults: { - memorySearch: { - provider: "gemini", - }, - }, - }, - }); + const cfg = configWithDefaultProvider("gemini"); const resolved = resolveMemorySearchConfig(cfg, "main"); - expect(resolved?.remote?.batch).toEqual({ - enabled: false, - wait: true, - concurrency: 2, - pollIntervalMs: 2000, - timeoutMinutes: 60, - }); + expectDefaultRemoteBatch(resolved); }); it("defaults session delta thresholds", () => { diff --git a/src/agents/model-fallback.probe.test.ts b/src/agents/model-fallback.probe.test.ts index bc8ffe704c7..3015e6ce349 100644 --- a/src/agents/model-fallback.probe.test.ts +++ b/src/agents/model-fallback.probe.test.ts @@ -37,6 +37,19 @@ function makeCfg(overrides: Partial = {}): OpenClawConfig { } as OpenClawConfig; } +function expectFallbackUsed( + result: { result: unknown; attempts: Array<{ reason?: string }> }, + run: { + (...args: unknown[]): unknown; + mock: { calls: unknown[][] }; + }, +) { + expect(result.result).toBe("ok"); + expect(run).toHaveBeenCalledTimes(1); + expect(run).toHaveBeenCalledWith("anthropic", "claude-haiku-3-5"); + expect(result.attempts[0]?.reason).toBe("rate_limit"); +} + describe("runWithModelFallback – probe logic", () => { let realDateNow: () => number; const NOW = 1_700_000_000_000; @@ -95,10 +108,7 @@ describe("runWithModelFallback – probe logic", () => { }); // Should skip primary and use fallback - expect(result.result).toBe("ok"); - expect(run).toHaveBeenCalledTimes(1); - expect(run).toHaveBeenCalledWith("anthropic", "claude-haiku-3-5"); - expect(result.attempts[0]?.reason).toBe("rate_limit"); + expectFallbackUsed(result, run); }); it("probes primary model when within 2-min margin of cooldown expiry", async () => { @@ -201,10 +211,7 @@ describe("runWithModelFallback – probe logic", () => { }); // Should be throttled → skip primary, use fallback - expect(result.result).toBe("ok"); - expect(run).toHaveBeenCalledTimes(1); - expect(run).toHaveBeenCalledWith("anthropic", "claude-haiku-3-5"); - expect(result.attempts[0]?.reason).toBe("rate_limit"); + expectFallbackUsed(result, run); }); it("allows probe when 30s have passed since last probe", async () => { diff --git a/src/agents/models-config.falls-back-default-baseurl-token-exchange-fails.test.ts b/src/agents/models-config.falls-back-default-baseurl-token-exchange-fails.test.ts index f0c7493fe39..dcb42ae8e55 100644 --- a/src/agents/models-config.falls-back-default-baseurl-token-exchange-fails.test.ts +++ b/src/agents/models-config.falls-back-default-baseurl-token-exchange-fails.test.ts @@ -13,6 +13,14 @@ import { ensureOpenClawModelsJson } from "./models-config.js"; installModelsConfigTestHooks({ restoreFetch: true }); +async function readCopilotBaseUrl(agentDir: string) { + const raw = await fs.readFile(path.join(agentDir, "models.json"), "utf8"); + const parsed = JSON.parse(raw) as { + providers: Record; + }; + return parsed.providers["github-copilot"]?.baseUrl; +} + describe("models-config", () => { it("falls back to default baseUrl when token exchange fails", async () => { await withTempHome(async () => { @@ -27,12 +35,7 @@ describe("models-config", () => { await ensureOpenClawModelsJson({ models: { providers: {} } }); const agentDir = path.join(process.env.HOME ?? "", ".openclaw", "agents", "main", "agent"); - const raw = await fs.readFile(path.join(agentDir, "models.json"), "utf8"); - const parsed = JSON.parse(raw) as { - providers: Record; - }; - - expect(parsed.providers["github-copilot"]?.baseUrl).toBe(DEFAULT_COPILOT_API_BASE_URL); + expect(await readCopilotBaseUrl(agentDir)).toBe(DEFAULT_COPILOT_API_BASE_URL); }); }); }); @@ -63,12 +66,7 @@ describe("models-config", () => { await ensureOpenClawModelsJson({ models: { providers: {} } }, agentDir); - const raw = await fs.readFile(path.join(agentDir, "models.json"), "utf8"); - const parsed = JSON.parse(raw) as { - providers: Record; - }; - - expect(parsed.providers["github-copilot"]?.baseUrl).toBe("https://api.copilot.example"); + expect(await readCopilotBaseUrl(agentDir)).toBe("https://api.copilot.example"); }); }); }); diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts b/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts index d12303b613d..1f679816742 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn.lifecycle.test.ts @@ -43,6 +43,32 @@ const waitFor = async (predicate: () => boolean, timeoutMs = 1_500) => { ); }; +async function getDiscordGroupSpawnTool() { + return await getSessionsSpawnTool({ + agentSessionKey: "discord:group:req", + agentChannel: "discord", + }); +} + +async function executeSpawnAndExpectAccepted(params: { + tool: Awaited>; + callId: string; + cleanup?: "delete" | "keep"; + label?: string; +}) { + const result = await params.tool.execute(params.callId, { + task: "do thing", + runTimeoutSeconds: RUN_TIMEOUT_SECONDS, + ...(params.cleanup ? { cleanup: params.cleanup } : {}), + ...(params.label ? { label: params.label } : {}), + }); + expect(result.details).toMatchObject({ + status: "accepted", + runId: "run-1", + }); + return result; +} + describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { let previousFastTestEnv: string | undefined; @@ -92,15 +118,11 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { agentChannel: "whatsapp", }); - const result = await tool.execute("call2", { - task: "do thing", - runTimeoutSeconds: RUN_TIMEOUT_SECONDS, + await executeSpawnAndExpectAccepted({ + tool, + callId: "call2", label: "my-task", }); - expect(result.details).toMatchObject({ - status: "accepted", - runId: "run-1", - }); const child = ctx.getChild(); if (!child.runId) { @@ -154,20 +176,12 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { }), }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "discord:group:req", - agentChannel: "discord", - }); - - const result = await tool.execute("call1", { - task: "do thing", - runTimeoutSeconds: RUN_TIMEOUT_SECONDS, + const tool = await getDiscordGroupSpawnTool(); + await executeSpawnAndExpectAccepted({ + tool, + callId: "call1", cleanup: "delete", }); - expect(result.details).toMatchObject({ - status: "accepted", - runId: "run-1", - }); const child = ctx.getChild(); if (!child.runId) { @@ -240,20 +254,12 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { agentWaitResult: { status: "ok", startedAt: 3000, endedAt: 4000 }, }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "discord:group:req", - agentChannel: "discord", - }); - - const result = await tool.execute("call1b", { - task: "do thing", - runTimeoutSeconds: RUN_TIMEOUT_SECONDS, + const tool = await getDiscordGroupSpawnTool(); + await executeSpawnAndExpectAccepted({ + tool, + callId: "call1b", cleanup: "delete", }); - expect(result.details).toMatchObject({ - status: "accepted", - runId: "run-1", - }); const child = ctx.getChild(); if (!child.runId) { @@ -295,20 +301,12 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { agentWaitResult: { status: "timeout", startedAt: 6000, endedAt: 7000 }, }); - const tool = await getSessionsSpawnTool({ - agentSessionKey: "discord:group:req", - agentChannel: "discord", - }); - - const result = await tool.execute("call-timeout", { - task: "do thing", - runTimeoutSeconds: RUN_TIMEOUT_SECONDS, + const tool = await getDiscordGroupSpawnTool(); + await executeSpawnAndExpectAccepted({ + tool, + callId: "call-timeout", cleanup: "keep", }); - expect(result.details).toMatchObject({ - status: "accepted", - runId: "run-1", - }); await waitFor(() => ctx.calls.filter((call) => call.method === "agent").length >= 2); @@ -333,15 +331,11 @@ describe("openclaw-tools: subagents (sessions_spawn lifecycle)", () => { agentAccountId: "kev", }); - const result = await tool.execute("call-announce-account", { - task: "do thing", - runTimeoutSeconds: RUN_TIMEOUT_SECONDS, + await executeSpawnAndExpectAccepted({ + tool, + callId: "call-announce-account", cleanup: "keep", }); - expect(result.details).toMatchObject({ - status: "accepted", - runId: "run-1", - }); const child = ctx.getChild(); if (!child.runId) { diff --git a/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.test.ts b/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.test.ts index f353da5e7da..5e809e5cca9 100644 --- a/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.test.ts +++ b/src/agents/pi-embedded-helpers.buildbootstrapcontextfiles.test.ts @@ -17,6 +17,12 @@ const makeFile = (overrides: Partial): WorkspaceBootstra missing: false, ...overrides, }); + +const createLargeBootstrapFiles = (): WorkspaceBootstrapFile[] => [ + makeFile({ name: "AGENTS.md", content: "a".repeat(10_000) }), + makeFile({ name: "SOUL.md", path: "/tmp/SOUL.md", content: "b".repeat(10_000) }), + makeFile({ name: "USER.md", path: "/tmp/USER.md", content: "c".repeat(10_000) }), +]; describe("buildBootstrapContextFiles", () => { it("keeps missing markers", () => { const files = [makeFile({ missing: true, content: undefined })]; @@ -60,11 +66,7 @@ describe("buildBootstrapContextFiles", () => { }); it("keeps total injected bootstrap characters under the new default total cap", () => { - const files = [ - makeFile({ name: "AGENTS.md", content: "a".repeat(10_000) }), - makeFile({ name: "SOUL.md", path: "/tmp/SOUL.md", content: "b".repeat(10_000) }), - makeFile({ name: "USER.md", path: "/tmp/USER.md", content: "c".repeat(10_000) }), - ]; + const files = createLargeBootstrapFiles(); const result = buildBootstrapContextFiles(files); const totalChars = result.reduce((sum, entry) => sum + entry.content.length, 0); expect(totalChars).toBeLessThanOrEqual(DEFAULT_BOOTSTRAP_TOTAL_MAX_CHARS); @@ -73,11 +75,7 @@ describe("buildBootstrapContextFiles", () => { }); it("caps total injected bootstrap characters when totalMaxChars is configured", () => { - const files = [ - makeFile({ name: "AGENTS.md", content: "a".repeat(10_000) }), - makeFile({ name: "SOUL.md", path: "/tmp/SOUL.md", content: "b".repeat(10_000) }), - makeFile({ name: "USER.md", path: "/tmp/USER.md", content: "c".repeat(10_000) }), - ]; + const files = createLargeBootstrapFiles(); const result = buildBootstrapContextFiles(files, { totalMaxChars: 24_000 }); const totalChars = result.reduce((sum, entry) => sum + entry.content.length, 0); expect(totalChars).toBeLessThanOrEqual(24_000); diff --git a/src/agents/pi-embedded-runner-extraparams.test.ts b/src/agents/pi-embedded-runner-extraparams.test.ts index 69f2077b063..184f1119480 100644 --- a/src/agents/pi-embedded-runner-extraparams.test.ts +++ b/src/agents/pi-embedded-runner-extraparams.test.ts @@ -109,6 +109,26 @@ describe("applyExtraParamsToAgent", () => { return payload; } + function runAnthropicHeaderCase(params: { + cfg: Record; + modelId: string; + options?: SimpleStreamOptions; + }) { + const { calls, agent } = createOptionsCaptureAgent(); + applyExtraParamsToAgent(agent, params.cfg, "anthropic", params.modelId); + + const model = { + api: "anthropic-messages", + provider: "anthropic", + id: params.modelId, + } as Model<"anthropic-messages">; + const context: Context = { messages: [] }; + void agent.streamFn?.(model, context, params.options ?? {}); + + expect(calls).toHaveLength(1); + return calls[0]?.headers; + } + it("adds OpenRouter attribution headers to stream options", () => { const { calls, agent } = createOptionsCaptureAgent(); @@ -204,50 +224,33 @@ describe("applyExtraParamsToAgent", () => { }); it("merges existing anthropic-beta headers with configured betas", () => { - const { calls, agent } = createOptionsCaptureAgent(); const cfg = buildAnthropicModelConfig("anthropic/claude-sonnet-4-5", { context1m: true, anthropicBeta: ["files-api-2025-04-14"], }); - - applyExtraParamsToAgent(agent, cfg, "anthropic", "claude-sonnet-4-5"); - - const model = { - api: "anthropic-messages", - provider: "anthropic", - id: "claude-sonnet-4-5", - } as Model<"anthropic-messages">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, { - apiKey: "sk-ant-api03-test", - headers: { "anthropic-beta": "prompt-caching-2024-07-31" }, + const headers = runAnthropicHeaderCase({ + cfg, + modelId: "claude-sonnet-4-5", + options: { + apiKey: "sk-ant-api03-test", + headers: { "anthropic-beta": "prompt-caching-2024-07-31" }, + }, }); - expect(calls).toHaveLength(1); - expect(calls[0]?.headers).toEqual({ + expect(headers).toEqual({ "anthropic-beta": "prompt-caching-2024-07-31,fine-grained-tool-streaming-2025-05-14,interleaved-thinking-2025-05-14,files-api-2025-04-14,context-1m-2025-08-07", }); }); it("ignores context1m for non-Opus/Sonnet Anthropic models", () => { - const { calls, agent } = createOptionsCaptureAgent(); const cfg = buildAnthropicModelConfig("anthropic/claude-haiku-3-5", { context1m: true }); - - applyExtraParamsToAgent(agent, cfg, "anthropic", "claude-haiku-3-5"); - - const model = { - api: "anthropic-messages", - provider: "anthropic", - id: "claude-haiku-3-5", - } as Model<"anthropic-messages">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, { headers: { "X-Custom": "1" } }); - - expect(calls).toHaveLength(1); - expect(calls[0]?.headers).toEqual({ "X-Custom": "1" }); + const headers = runAnthropicHeaderCase({ + cfg, + modelId: "claude-haiku-3-5", + options: { headers: { "X-Custom": "1" } }, + }); + expect(headers).toEqual({ "X-Custom": "1" }); }); it("forces store=true for direct OpenAI Responses payloads", () => { @@ -295,27 +298,18 @@ describe("applyExtraParamsToAgent", () => { }, { name: "without config via provider/model hints", - run: () => { - const payload = { store: false }; - const baseStreamFn: StreamFn = (_model, _context, options) => { - options?.onPayload?.(payload); - return {} as ReturnType; - }; - const agent = { streamFn: baseStreamFn }; - - applyExtraParamsToAgent(agent, undefined, "openai-codex", "codex-mini-latest"); - - const model = { - api: "openai-codex-responses", - provider: "openai-codex", - id: "codex-mini-latest", - baseUrl: "https://chatgpt.com/backend-api/codex/responses", - } as Model<"openai-codex-responses">; - const context: Context = { messages: [] }; - - void agent.streamFn?.(model, context, {}); - return payload; - }, + run: () => + runStoreMutationCase({ + applyProvider: "openai-codex", + applyModelId: "codex-mini-latest", + model: { + api: "openai-codex-responses", + provider: "openai-codex", + id: "codex-mini-latest", + baseUrl: "https://chatgpt.com/backend-api/codex/responses", + } as Model<"openai-codex-responses">, + options: {}, + }), }, ])( "does not force store=true for Codex responses (Codex requires store=false) ($name)", diff --git a/src/agents/pi-embedded-runner.google-sanitize-thinking.test.ts b/src/agents/pi-embedded-runner.google-sanitize-thinking.test.ts index 93266a0230d..4e08b49cbd0 100644 --- a/src/agents/pi-embedded-runner.google-sanitize-thinking.test.ts +++ b/src/agents/pi-embedded-runner.google-sanitize-thinking.test.ts @@ -3,11 +3,21 @@ import { SessionManager } from "@mariozechner/pi-coding-agent"; import { describe, expect, it } from "vitest"; import { sanitizeSessionHistory } from "./pi-embedded-runner/google.js"; -type AssistantThinking = { type?: string; thinking?: string; thinkingSignature?: string }; +type AssistantContentBlock = { + type?: string; + text?: string; + thinking?: string; + thinkingSignature?: string; + thought_signature?: string; + thoughtSignature?: string; + id?: string; + name?: string; + arguments?: unknown; +}; function getAssistantMessage(out: AgentMessage[]) { const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as - | { content?: AssistantThinking[] } + | { content?: AssistantContentBlock[] } | undefined; if (!assistant) { throw new Error("Expected assistant message in sanitized history"); @@ -43,6 +53,7 @@ async function sanitizeSimpleSession(params: { sessionId: string; content: unknown[]; modelId?: string; + provider?: string; }) { const sessionManager = SessionManager.inMemory(); const input = [ @@ -59,12 +70,34 @@ async function sanitizeSimpleSession(params: { return sanitizeSessionHistory({ messages: input, modelApi: params.modelApi, + provider: params.provider, modelId: params.modelId, sessionManager, sessionId: params.sessionId, }); } +function geminiThoughtSignatureInput() { + return [ + { type: "text", text: "hello", thought_signature: "msg_abc123" }, + { type: "thinking", thinking: "ok", thought_signature: "c2ln" }, + { + type: "toolCall", + id: "call_1", + name: "read", + arguments: { path: "/tmp/foo" }, + thoughtSignature: '{"id":1}', + }, + { + type: "toolCall", + id: "call_2", + name: "read", + arguments: { path: "/tmp/bar" }, + thoughtSignature: "c2ln", + }, + ]; +} + describe("sanitizeSessionHistory (google thinking)", () => { it("keeps thinking blocks without signatures for Google models", async () => { const assistant = await sanitizeGoogleAssistantWithContent([ @@ -106,29 +139,14 @@ describe("sanitizeSessionHistory (google thinking)", () => { }); it("maps base64 signatures to thinkingSignature for Antigravity Claude", async () => { - const sessionManager = SessionManager.inMemory(); - const input = [ - { - role: "user", - content: "hi", - }, - { - role: "assistant", - content: [{ type: "thinking", thinking: "reasoning", signature: "c2ln" }], - }, - ] as unknown as AgentMessage[]; - - const out = await sanitizeSessionHistory({ - messages: input, + const out = await sanitizeSimpleSession({ modelApi: "google-antigravity", modelId: "anthropic/claude-3.5-sonnet", - sessionManager, sessionId: "session:antigravity-claude", + content: [{ type: "thinking", thinking: "reasoning", signature: "c2ln" }], }); - const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as { - content?: Array<{ type?: string; thinking?: string; thinkingSignature?: string }>; - }; + const assistant = getAssistantMessage(out); expect(assistant.content?.map((block) => block.type)).toEqual(["thinking"]); expect(assistant.content?.[0]?.thinking).toBe("reasoning"); expect(assistant.content?.[0]?.thinkingSignature).toBe("c2ln"); @@ -166,52 +184,15 @@ describe("sanitizeSessionHistory (google thinking)", () => { }); it("strips non-base64 thought signatures for OpenRouter Gemini", async () => { - const sessionManager = SessionManager.inMemory(); - const input = [ - { - role: "user", - content: "hi", - }, - { - role: "assistant", - content: [ - { type: "text", text: "hello", thought_signature: "msg_abc123" }, - { type: "thinking", thinking: "ok", thought_signature: "c2ln" }, - { - type: "toolCall", - id: "call_1", - name: "read", - arguments: { path: "/tmp/foo" }, - thoughtSignature: '{"id":1}', - }, - { - type: "toolCall", - id: "call_2", - name: "read", - arguments: { path: "/tmp/bar" }, - thoughtSignature: "c2ln", - }, - ], - }, - ] as unknown as AgentMessage[]; - - const out = await sanitizeSessionHistory({ - messages: input, + const out = await sanitizeSimpleSession({ modelApi: "openrouter", provider: "openrouter", modelId: "google/gemini-1.5-pro", - sessionManager, sessionId: "session:openrouter-gemini", + content: geminiThoughtSignatureInput(), }); - const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as { - content?: Array<{ - type?: string; - thought_signature?: string; - thoughtSignature?: string; - thinking?: string; - }>; - }; + const assistant = getAssistantMessage(out); expect(assistant.content).toEqual([ { type: "text", text: "hello" }, { type: "thinking", thinking: "ok", thought_signature: "c2ln" }, @@ -232,52 +213,15 @@ describe("sanitizeSessionHistory (google thinking)", () => { }); it("strips non-base64 thought signatures for native Google Gemini", async () => { - const sessionManager = SessionManager.inMemory(); - const input = [ - { - role: "user", - content: "hi", - }, - { - role: "assistant", - content: [ - { type: "text", text: "hello", thought_signature: "msg_abc123" }, - { type: "thinking", thinking: "ok", thought_signature: "c2ln" }, - { - type: "toolCall", - id: "call_1", - name: "read", - arguments: { path: "/tmp/foo" }, - thoughtSignature: '{"id":1}', - }, - { - type: "toolCall", - id: "call_2", - name: "read", - arguments: { path: "/tmp/bar" }, - thoughtSignature: "c2ln", - }, - ], - }, - ] as unknown as AgentMessage[]; - - const out = await sanitizeSessionHistory({ - messages: input, + const out = await sanitizeSimpleSession({ modelApi: "google-generative-ai", provider: "google", modelId: "gemini-2.0-flash", - sessionManager, sessionId: "session:google-gemini", + content: geminiThoughtSignatureInput(), }); - const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as { - content?: Array<{ - type?: string; - thought_signature?: string; - thoughtSignature?: string; - thinking?: string; - }>; - }; + const assistant = getAssistantMessage(out); expect(assistant.content).toEqual([ { type: "text", text: "hello" }, { type: "thinking", thinking: "ok", thought_signature: "c2ln" }, @@ -298,59 +242,19 @@ describe("sanitizeSessionHistory (google thinking)", () => { }); it("keeps mixed signed/unsigned thinking blocks for Google models", async () => { - const sessionManager = SessionManager.inMemory(); - const input = [ - { - role: "user", - content: "hi", - }, - { - role: "assistant", - content: [ - { type: "thinking", thinking: "signed", thinkingSignature: "sig" }, - { type: "thinking", thinking: "unsigned" }, - ], - }, - ] as unknown as AgentMessage[]; - - const out = await sanitizeSessionHistory({ - messages: input, - modelApi: "google-antigravity", - sessionManager, - sessionId: "session:google-mixed-signatures", - }); - - const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as { - content?: Array<{ type?: string; thinking?: string }>; - }; + const assistant = await sanitizeGoogleAssistantWithContent([ + { type: "thinking", thinking: "signed", thinkingSignature: "sig" }, + { type: "thinking", thinking: "unsigned" }, + ]); expect(assistant.content?.map((block) => block.type)).toEqual(["thinking", "thinking"]); expect(assistant.content?.[0]?.thinking).toBe("signed"); expect(assistant.content?.[1]?.thinking).toBe("unsigned"); }); it("keeps empty thinking blocks for Google models", async () => { - const sessionManager = SessionManager.inMemory(); - const input = [ - { - role: "user", - content: "hi", - }, - { - role: "assistant", - content: [{ type: "thinking", thinking: " " }], - }, - ] as unknown as AgentMessage[]; - - const out = await sanitizeSessionHistory({ - messages: input, - modelApi: "google-antigravity", - sessionManager, - sessionId: "session:google-empty", - }); - - const assistant = out.find((msg) => (msg as { role?: string }).role === "assistant") as { - content?: Array<{ type?: string; thinking?: string }>; - }; + const assistant = await sanitizeGoogleAssistantWithContent([ + { type: "thinking", thinking: " " }, + ]); expect(assistant?.content?.map((block) => block.type)).toEqual(["thinking"]); }); diff --git a/src/agents/pi-embedded-subscribe.e2e-harness.ts b/src/agents/pi-embedded-subscribe.e2e-harness.ts index 918685a1844..0c9a9240df0 100644 --- a/src/agents/pi-embedded-subscribe.e2e-harness.ts +++ b/src/agents/pi-embedded-subscribe.e2e-harness.ts @@ -165,6 +165,43 @@ export function emitAssistantTextEnd(params: { }); } +export function emitAssistantLifecycleErrorAndEnd(params: { + emit: (evt: unknown) => void; + errorMessage: string; + provider?: string; + model?: string; +}): void { + const assistantMessage = { + role: "assistant", + stopReason: "error", + errorMessage: params.errorMessage, + ...(params.provider ? { provider: params.provider } : {}), + ...(params.model ? { model: params.model } : {}), + } as AssistantMessage; + params.emit({ type: "message_update", message: assistantMessage }); + params.emit({ type: "agent_end" }); +} + +type LifecycleErrorAgentEvent = { + stream?: unknown; + data?: { + phase?: unknown; + error?: unknown; + }; +}; + +export function findLifecycleErrorAgentEvent( + calls: Array, +): LifecycleErrorAgentEvent | undefined { + for (const call of calls) { + const event = call?.[0] as LifecycleErrorAgentEvent | undefined; + if (event?.stream === "lifecycle" && event?.data?.phase === "error") { + return event; + } + } + return undefined; +} + export function expectFencedChunks(calls: Array, expectedPrefix: string): void { expect(calls.length).toBeGreaterThan(1); for (const call of calls) { diff --git a/src/agents/pi-embedded-subscribe.lifecycle-billing-error.test.ts b/src/agents/pi-embedded-subscribe.lifecycle-billing-error.test.ts index 669bb50c3ec..80819d0f964 100644 --- a/src/agents/pi-embedded-subscribe.lifecycle-billing-error.test.ts +++ b/src/agents/pi-embedded-subscribe.lifecycle-billing-error.test.ts @@ -1,35 +1,36 @@ -import type { AssistantMessage } from "@mariozechner/pi-ai"; import { describe, expect, it, vi } from "vitest"; -import { createStubSessionHarness } from "./pi-embedded-subscribe.e2e-harness.js"; -import { subscribeEmbeddedPiSession } from "./pi-embedded-subscribe.js"; +import { + createSubscribedSessionHarness, + emitAssistantLifecycleErrorAndEnd, + findLifecycleErrorAgentEvent, +} from "./pi-embedded-subscribe.e2e-harness.js"; describe("subscribeEmbeddedPiSession lifecycle billing errors", () => { - it("includes provider and model context in lifecycle billing errors", () => { - const { session, emit } = createStubSessionHarness(); + function createAgentEventHarness(options?: { runId?: string; sessionKey?: string }) { const onAgentEvent = vi.fn(); - - subscribeEmbeddedPiSession({ - session, - runId: "run-billing-error", + const { emit } = createSubscribedSessionHarness({ + runId: options?.runId ?? "run", + sessionKey: options?.sessionKey, onAgentEvent, + }); + return { emit, onAgentEvent }; + } + + it("includes provider and model context in lifecycle billing errors", () => { + const { emit, onAgentEvent } = createAgentEventHarness({ + runId: "run-billing-error", sessionKey: "test-session", }); - const assistantMessage = { - role: "assistant", - stopReason: "error", + emitAssistantLifecycleErrorAndEnd({ + emit, errorMessage: "insufficient credits", provider: "Anthropic", model: "claude-3-5-sonnet", - } as AssistantMessage; + }); - emit({ type: "message_update", message: assistantMessage }); - emit({ type: "agent_end" }); - - const lifecycleError = onAgentEvent.mock.calls.find( - (call) => call[0]?.stream === "lifecycle" && call[0]?.data?.phase === "error", - ); + const lifecycleError = findLifecycleErrorAgentEvent(onAgentEvent.mock.calls); expect(lifecycleError).toBeDefined(); - expect(lifecycleError?.[0]?.data?.error).toContain("Anthropic (claude-3-5-sonnet)"); + expect(lifecycleError?.data?.error).toContain("Anthropic (claude-3-5-sonnet)"); }); }); diff --git a/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts b/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts index a048ab2d6e0..82c968d23a8 100644 --- a/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts +++ b/src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts @@ -3,9 +3,11 @@ import { describe, expect, it, vi } from "vitest"; import { THINKING_TAG_CASES, createStubSessionHarness, + emitAssistantLifecycleErrorAndEnd, emitMessageStartAndEndForAssistantText, expectSingleAgentEventText, extractAgentEventPayloads, + findLifecycleErrorAgentEvent, } from "./pi-embedded-subscribe.e2e-harness.js"; import { subscribeEmbeddedPiSession } from "./pi-embedded-subscribe.js"; @@ -490,24 +492,15 @@ describe("subscribeEmbeddedPiSession", () => { sessionKey: "test-session", }); - const assistantMessage = { - role: "assistant", - stopReason: "error", + emitAssistantLifecycleErrorAndEnd({ + emit, errorMessage: "429 Rate limit exceeded", - } as AssistantMessage; - - // Simulate message update to set lastAssistant - emit({ type: "message_update", message: assistantMessage }); - - // Trigger agent_end - emit({ type: "agent_end" }); + }); // Look for lifecycle:error event - const lifecycleError = onAgentEvent.mock.calls.find( - (call) => call[0]?.stream === "lifecycle" && call[0]?.data?.phase === "error", - ); + const lifecycleError = findLifecycleErrorAgentEvent(onAgentEvent.mock.calls); expect(lifecycleError).toBeDefined(); - expect(lifecycleError?.[0]?.data?.error).toContain("API rate limit reached"); + expect(lifecycleError?.data?.error).toContain("API rate limit reached"); }); }); diff --git a/src/agents/pi-tools.before-tool-call.integration.test.ts b/src/agents/pi-tools.before-tool-call.integration.test.ts index 1f9176cec77..643a14b0338 100644 --- a/src/agents/pi-tools.before-tool-call.integration.test.ts +++ b/src/agents/pi-tools.before-tool-call.integration.test.ts @@ -9,20 +9,35 @@ vi.mock("../plugins/hook-runner-global.js"); const mockGetGlobalHookRunner = vi.mocked(getGlobalHookRunner); -describe("before_tool_call hook integration", () => { - let hookRunner: { - hasHooks: ReturnType; - runBeforeToolCall: ReturnType; +type HookRunnerMock = { + hasHooks: ReturnType; + runBeforeToolCall: ReturnType; +}; + +function installMockHookRunner(params?: { + hasHooksReturn?: boolean; + runBeforeToolCallImpl?: (...args: unknown[]) => unknown; +}) { + const hookRunner: HookRunnerMock = { + hasHooks: + params?.hasHooksReturn === undefined + ? vi.fn() + : vi.fn(() => params.hasHooksReturn as boolean), + runBeforeToolCall: params?.runBeforeToolCallImpl + ? vi.fn(params.runBeforeToolCallImpl) + : vi.fn(), }; + // oxlint-disable-next-line typescript/no-explicit-any + mockGetGlobalHookRunner.mockReturnValue(hookRunner as any); + return hookRunner; +} + +describe("before_tool_call hook integration", () => { + let hookRunner: HookRunnerMock; beforeEach(() => { resetDiagnosticSessionStateForTest(); - hookRunner = { - hasHooks: vi.fn(), - runBeforeToolCall: vi.fn(), - }; - // oxlint-disable-next-line typescript/no-explicit-any - mockGetGlobalHookRunner.mockReturnValue(hookRunner as any); + hookRunner = installMockHookRunner(); }); it("executes tool normally when no hook is registered", async () => { @@ -127,19 +142,14 @@ describe("before_tool_call hook integration", () => { }); describe("before_tool_call hook deduplication (#15502)", () => { - let hookRunner: { - hasHooks: ReturnType; - runBeforeToolCall: ReturnType; - }; + let hookRunner: HookRunnerMock; beforeEach(() => { resetDiagnosticSessionStateForTest(); - hookRunner = { - hasHooks: vi.fn(() => true), - runBeforeToolCall: vi.fn(async () => undefined), - }; - // oxlint-disable-next-line typescript/no-explicit-any - mockGetGlobalHookRunner.mockReturnValue(hookRunner as any); + hookRunner = installMockHookRunner({ + hasHooksReturn: true, + runBeforeToolCallImpl: async () => undefined, + }); }); it("fires hook exactly once when tool goes through wrap + toToolDefinitions", async () => { @@ -191,19 +201,11 @@ describe("before_tool_call hook deduplication (#15502)", () => { }); describe("before_tool_call hook integration for client tools", () => { - let hookRunner: { - hasHooks: ReturnType; - runBeforeToolCall: ReturnType; - }; + let hookRunner: HookRunnerMock; beforeEach(() => { resetDiagnosticSessionStateForTest(); - hookRunner = { - hasHooks: vi.fn(), - runBeforeToolCall: vi.fn(), - }; - // oxlint-disable-next-line typescript/no-explicit-any - mockGetGlobalHookRunner.mockReturnValue(hookRunner as any); + hookRunner = installMockHookRunner(); }); it("passes modified params to client tool callbacks", async () => { diff --git a/src/agents/pi-tools.before-tool-call.test.ts b/src/agents/pi-tools.before-tool-call.test.ts index 3348c3e334d..44cb5dfd69f 100644 --- a/src/agents/pi-tools.before-tool-call.test.ts +++ b/src/agents/pi-tools.before-tool-call.test.ts @@ -121,13 +121,35 @@ describe("before_tool_call loop detection behavior", () => { }; } - it("blocks known poll loops when no progress repeats", async () => { + function createNoProgressProcessFixture(sessionId: string) { const execute = vi.fn().mockResolvedValue({ content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], details: { status: "running", aggregated: "steady" }, }); - const tool = createWrappedTool("process", execute); - const params = { action: "poll", sessionId: "sess-1" }; + return { + tool: createWrappedTool("process", execute), + params: { action: "poll", sessionId }, + }; + } + + function expectCriticalLoopEvent( + loopEvent: DiagnosticToolLoopEvent | undefined, + params: { + detector: "ping_pong" | "known_poll_no_progress"; + toolName: string; + count?: number; + }, + ) { + expect(loopEvent?.type).toBe("tool.loop"); + expect(loopEvent?.level).toBe("critical"); + expect(loopEvent?.action).toBe("block"); + expect(loopEvent?.detector).toBe(params.detector); + expect(loopEvent?.count).toBe(params.count ?? CRITICAL_THRESHOLD); + expect(loopEvent?.toolName).toBe(params.toolName); + } + + it("blocks known poll loops when no progress repeats", async () => { + const { tool, params } = createNoProgressProcessFixture("sess-1"); for (let i = 0; i < CRITICAL_THRESHOLD; i += 1) { await expect(tool.execute(`poll-${i}`, params, undefined, undefined)).resolves.toBeDefined(); @@ -245,12 +267,10 @@ describe("before_tool_call loop detection behavior", () => { ).rejects.toThrow("CRITICAL"); const loopEvent = emitted.at(-1); - expect(loopEvent?.type).toBe("tool.loop"); - expect(loopEvent?.level).toBe("critical"); - expect(loopEvent?.action).toBe("block"); - expect(loopEvent?.detector).toBe("ping_pong"); - expect(loopEvent?.count).toBe(CRITICAL_THRESHOLD); - expect(loopEvent?.toolName).toBe("list"); + expectCriticalLoopEvent(loopEvent, { + detector: "ping_pong", + toolName: "list", + }); }); }); @@ -281,12 +301,7 @@ describe("before_tool_call loop detection behavior", () => { it("emits structured critical diagnostic events when blocking loops", async () => { await withToolLoopEvents(async (emitted) => { - const execute = vi.fn().mockResolvedValue({ - content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], - details: { status: "running", aggregated: "steady" }, - }); - const tool = createWrappedTool("process", execute); - const params = { action: "poll", sessionId: "sess-crit" }; + const { tool, params } = createNoProgressProcessFixture("sess-crit"); for (let i = 0; i < CRITICAL_THRESHOLD; i += 1) { await tool.execute(`poll-${i}`, params, undefined, undefined); @@ -297,12 +312,10 @@ describe("before_tool_call loop detection behavior", () => { ).rejects.toThrow("CRITICAL"); const loopEvent = emitted.at(-1); - expect(loopEvent?.type).toBe("tool.loop"); - expect(loopEvent?.level).toBe("critical"); - expect(loopEvent?.action).toBe("block"); - expect(loopEvent?.detector).toBe("known_poll_no_progress"); - expect(loopEvent?.count).toBe(CRITICAL_THRESHOLD); - expect(loopEvent?.toolName).toBe("process"); + expectCriticalLoopEvent(loopEvent, { + detector: "known_poll_no_progress", + toolName: "process", + }); }); }); }); diff --git a/src/agents/session-write-lock.test.ts b/src/agents/session-write-lock.test.ts index d69e8528502..4bef8a5194a 100644 --- a/src/agents/session-write-lock.test.ts +++ b/src/agents/session-write-lock.test.ts @@ -9,6 +9,18 @@ import { resolveSessionLockMaxHoldFromTimeout, } from "./session-write-lock.js"; +async function expectLockRemovedOnlyAfterFinalRelease(params: { + lockPath: string; + firstLock: { release: () => Promise }; + secondLock: { release: () => Promise }; +}) { + await expect(fs.access(params.lockPath)).resolves.toBeUndefined(); + await params.firstLock.release(); + await expect(fs.access(params.lockPath)).resolves.toBeUndefined(); + await params.secondLock.release(); + await expect(fs.access(params.lockPath)).rejects.toThrow(); +} + describe("acquireSessionWriteLock", () => { it("reuses locks across symlinked session paths", async () => { if (process.platform === "win32") { @@ -45,11 +57,11 @@ describe("acquireSessionWriteLock", () => { const lockA = await acquireSessionWriteLock({ sessionFile, timeoutMs: 500 }); const lockB = await acquireSessionWriteLock({ sessionFile, timeoutMs: 500 }); - await expect(fs.access(lockPath)).resolves.toBeUndefined(); - await lockA.release(); - await expect(fs.access(lockPath)).resolves.toBeUndefined(); - await lockB.release(); - await expect(fs.access(lockPath)).rejects.toThrow(); + await expectLockRemovedOnlyAfterFinalRelease({ + lockPath, + firstLock: lockA, + secondLock: lockB, + }); } finally { await fs.rm(root, { recursive: true, force: true }); } @@ -130,11 +142,11 @@ describe("acquireSessionWriteLock", () => { await expect(fs.access(lockPath)).resolves.toBeUndefined(); // Old release handle must not affect the new lock. - await lockA.release(); - await expect(fs.access(lockPath)).resolves.toBeUndefined(); - - await lockB.release(); - await expect(fs.access(lockPath)).rejects.toThrow(); + await expectLockRemovedOnlyAfterFinalRelease({ + lockPath, + firstLock: lockA, + secondLock: lockB, + }); } finally { warnSpy.mockRestore(); await fs.rm(root, { recursive: true, force: true }); diff --git a/src/agents/skills-install-fallback.test.ts b/src/agents/skills-install-fallback.test.ts index db0f826e99e..c946c45f7c4 100644 --- a/src/agents/skills-install-fallback.test.ts +++ b/src/agents/skills-install-fallback.test.ts @@ -3,12 +3,13 @@ import os from "node:os"; import path from "node:path"; import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { installSkill } from "./skills-install.js"; +import { + hasBinaryMock, + runCommandWithTimeoutMock, + scanDirectoryWithSummaryMock, +} from "./skills-install.test-mocks.js"; import { buildWorkspaceSkillStatus } from "./skills-status.js"; -const runCommandWithTimeoutMock = vi.fn(); -const scanDirectoryWithSummaryMock = vi.fn(); -const hasBinaryMock = vi.fn(); - vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), })); @@ -69,6 +70,17 @@ async function writeSkillWithInstaller( return writeSkillWithInstallers(workspaceDir, name, [{ id: "deps", kind, ...extra }]); } +function mockBinaryAvailability(availableBinaries: string[]) { + const available = new Set(availableBinaries); + hasBinaryMock.mockImplementation((bin: string) => available.has(bin)); +} + +function getAptGetCalls() { + return runCommandWithTimeoutMock.mock.calls.filter( + (call) => Array.isArray(call[0]) && (call[0] as string[]).includes("apt-get"), + ); +} + describe("skills-install fallback edge cases", () => { let workspaceDir: string; @@ -99,18 +111,7 @@ describe("skills-install fallback edge cases", () => { it("apt-get available but sudo missing/unusable returns helpful error for go install", async () => { // go not available, brew not available, apt-get + sudo are available, sudo check fails - hasBinaryMock.mockImplementation((bin: string) => { - if (bin === "go") { - return false; - } - if (bin === "brew") { - return false; - } - if (bin === "apt-get" || bin === "sudo") { - return true; - } - return false; - }); + mockBinaryAvailability(["apt-get", "sudo"]); // sudo -n true fails (no passwordless sudo) runCommandWithTimeoutMock.mockResolvedValueOnce({ @@ -136,23 +137,13 @@ describe("skills-install fallback edge cases", () => { ); // Verify apt-get install was NOT called - const aptCalls = runCommandWithTimeoutMock.mock.calls.filter( - (call) => Array.isArray(call[0]) && (call[0] as string[]).includes("apt-get"), - ); + const aptCalls = getAptGetCalls(); expect(aptCalls).toHaveLength(0); }); it("status-selected go installer fails gracefully when apt fallback needs sudo", async () => { // no go/brew, but apt and sudo are present - hasBinaryMock.mockImplementation((bin: string) => { - if (bin === "go" || bin === "brew") { - return false; - } - if (bin === "apt-get" || bin === "sudo") { - return true; - } - return false; - }); + mockBinaryAvailability(["apt-get", "sudo"]); runCommandWithTimeoutMock.mockResolvedValueOnce({ code: 1, @@ -176,18 +167,7 @@ describe("skills-install fallback edge cases", () => { it("handles sudo probe spawn failures without throwing", async () => { // go not available, brew not available, apt-get + sudo appear available - hasBinaryMock.mockImplementation((bin: string) => { - if (bin === "go") { - return false; - } - if (bin === "brew") { - return false; - } - if (bin === "apt-get" || bin === "sudo") { - return true; - } - return false; - }); + mockBinaryAvailability(["apt-get", "sudo"]); runCommandWithTimeoutMock.mockRejectedValueOnce( new Error('Executable not found in $PATH: "sudo"'), @@ -204,26 +184,13 @@ describe("skills-install fallback edge cases", () => { expect(result.stderr).toContain("Executable not found"); // Verify apt-get install was NOT called - const aptCalls = runCommandWithTimeoutMock.mock.calls.filter( - (call) => Array.isArray(call[0]) && (call[0] as string[]).includes("apt-get"), - ); + const aptCalls = getAptGetCalls(); expect(aptCalls).toHaveLength(0); }); it("uv not installed and no brew returns helpful error without curl auto-install", async () => { // uv not available, brew not available, curl IS available - hasBinaryMock.mockImplementation((bin: string) => { - if (bin === "uv") { - return false; - } - if (bin === "brew") { - return false; - } - if (bin === "curl") { - return true; - } - return false; - }); + mockBinaryAvailability(["curl"]); const result = await installSkill({ workspaceDir, diff --git a/src/agents/skills-install.download.test.ts b/src/agents/skills-install.download.test.ts index bc1652e97e4..763316ff83b 100644 --- a/src/agents/skills-install.download.test.ts +++ b/src/agents/skills-install.download.test.ts @@ -59,7 +59,7 @@ const TAR_GZ_TRAVERSAL_BUFFER = Buffer.from( function mockArchiveResponse(buffer: Uint8Array): void { fetchWithSsrFGuardMock.mockResolvedValue({ - response: new Response(buffer, { status: 200 }), + response: new Response(new Blob([buffer]), { status: 200 }), release: async () => undefined, }); } diff --git a/src/agents/skills-install.test-mocks.ts b/src/agents/skills-install.test-mocks.ts new file mode 100644 index 00000000000..a999994a3de --- /dev/null +++ b/src/agents/skills-install.test-mocks.ts @@ -0,0 +1,32 @@ +import { vi } from "vitest"; + +export const runCommandWithTimeoutMock = vi.fn(); +export const scanDirectoryWithSummaryMock = vi.fn(); +export const fetchWithSsrFGuardMock = vi.fn(); +export const hasBinaryMock = vi.fn(); + +export function runCommandWithTimeoutFromMock(...args: unknown[]) { + return runCommandWithTimeoutMock(...args); +} + +export function fetchWithSsrFGuardFromMock(...args: unknown[]) { + return fetchWithSsrFGuardMock(...args); +} + +export function hasBinaryFromMock(...args: unknown[]) { + return hasBinaryMock(...args); +} + +export function scanDirectoryWithSummaryFromMock(...args: unknown[]) { + return scanDirectoryWithSummaryMock(...args); +} + +export async function mockSkillScannerModule( + importOriginal: () => Promise, +) { + const actual = await importOriginal(); + return { + ...actual, + scanDirectoryWithSummary: scanDirectoryWithSummaryFromMock, + }; +} diff --git a/src/agents/skills-install.test.ts b/src/agents/skills-install.test.ts index 803d261647c..03c14808ba6 100644 --- a/src/agents/skills-install.test.ts +++ b/src/agents/skills-install.test.ts @@ -3,9 +3,10 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { withTempWorkspace } from "./skills-install.download-test-utils.js"; import { installSkill } from "./skills-install.js"; - -const runCommandWithTimeoutMock = vi.fn(); -const scanDirectoryWithSummaryMock = vi.fn(); +import { + runCommandWithTimeoutMock, + scanDirectoryWithSummaryMock, +} from "./skills-install.test-mocks.js"; vi.mock("../process/exec.js", () => ({ runCommandWithTimeout: (...args: unknown[]) => runCommandWithTimeoutMock(...args), diff --git a/src/agents/tool-call-id.test.ts b/src/agents/tool-call-id.test.ts index ce4e5dd614d..19e2625d686 100644 --- a/src/agents/tool-call-id.test.ts +++ b/src/agents/tool-call-id.test.ts @@ -48,6 +48,20 @@ function expectCollisionIdsRemainDistinct( return { aId: a.id as string, bId: b.id as string }; } +function expectSingleToolCallRewrite( + out: AgentMessage[], + expectedId: string, + mode: "strict" | "strict9", +): void { + const assistant = out[0] as Extract; + const toolCall = assistant.content?.[0] as { id?: string }; + expect(toolCall.id).toBe(expectedId); + expect(isValidCloudCodeAssistToolId(toolCall.id as string, mode)).toBe(true); + + const result = out[1] as Extract; + expect(result.toolCallId).toBe(toolCall.id); +} + describe("sanitizeToolCallIdsForCloudCodeAssist", () => { describe("strict mode (default)", () => { it("is a no-op for already-valid non-colliding IDs", () => { @@ -84,15 +98,8 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => { const out = sanitizeToolCallIdsForCloudCodeAssist(input); expect(out).not.toBe(input); - - const assistant = out[0] as Extract; - const toolCall = assistant.content?.[0] as { id?: string }; // Strict mode strips all non-alphanumeric characters - expect(toolCall.id).toBe("callitem123"); - expect(isValidCloudCodeAssistToolId(toolCall.id as string, "strict")).toBe(true); - - const result = out[1] as Extract; - expect(result.toolCallId).toBe(toolCall.id); + expectSingleToolCallRewrite(out, "callitem123", "strict"); }); it("avoids collisions when sanitization would produce duplicate IDs", () => { @@ -159,15 +166,8 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => { const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict"); expect(out).not.toBe(input); - - const assistant = out[0] as Extract; - const toolCall = assistant.content?.[0] as { id?: string }; // Strict mode strips all non-alphanumeric characters - expect(toolCall.id).toBe("whatsapplogin17687998415271"); - expect(isValidCloudCodeAssistToolId(toolCall.id as string, "strict")).toBe(true); - - const result = out[1] as Extract; - expect(result.toolCallId).toBe(toolCall.id); + expectSingleToolCallRewrite(out, "whatsapplogin17687998415271", "strict"); }); it("avoids collisions with alphanumeric-only suffixes", () => { @@ -183,6 +183,24 @@ describe("sanitizeToolCallIdsForCloudCodeAssist", () => { }); describe("strict9 mode (Mistral tool call IDs)", () => { + it("is a no-op for already-valid 9-char alphanumeric IDs", () => { + const input = [ + { + role: "assistant", + content: [{ type: "toolCall", id: "abc123XYZ", name: "read", arguments: {} }], + }, + { + role: "toolResult", + toolCallId: "abc123XYZ", + toolName: "read", + content: [{ type: "text", text: "ok" }], + }, + ] as unknown as AgentMessage[]; + + const out = sanitizeToolCallIdsForCloudCodeAssist(input, "strict9"); + expect(out).toBe(input); + }); + it("enforces alphanumeric IDs with length 9", () => { const input = [ { diff --git a/src/agents/tool-loop-detection.test.ts b/src/agents/tool-loop-detection.test.ts index 19cf950efc3..2a356f73209 100644 --- a/src/agents/tool-loop-detection.test.ts +++ b/src/agents/tool-loop-detection.test.ts @@ -45,6 +45,36 @@ function recordSuccessfulCall( }); } +function recordRepeatedSuccessfulCalls(params: { + state: SessionState; + toolName: string; + toolParams: unknown; + result: unknown; + count: number; + startIndex?: number; +}) { + const startIndex = params.startIndex ?? 0; + for (let i = 0; i < params.count; i += 1) { + recordSuccessfulCall( + params.state, + params.toolName, + params.toolParams, + params.result, + startIndex + i, + ); + } +} + +function createNoProgressPollFixture(sessionId: string) { + return { + params: { action: "poll", sessionId }, + result: { + content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], + details: { status: "running", aggregated: "steady" }, + }, + }; +} + function recordSuccessfulPingPongCalls(params: { state: SessionState; readParams: { path: string }; @@ -248,11 +278,7 @@ describe("tool-loop-detection", () => { it("applies custom thresholds when detection is enabled", () => { const state = createState(); - const params = { action: "poll", sessionId: "sess-custom" }; - const result = { - content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], - details: { status: "running", aggregated: "steady" }, - }; + const { params, result } = createNoProgressPollFixture("sess-custom"); const config: ToolLoopDetectionConfig = { enabled: true, warningThreshold: 2, @@ -264,17 +290,27 @@ describe("tool-loop-detection", () => { }, }; - for (let i = 0; i < 2; i += 1) { - recordSuccessfulCall(state, "process", params, result, i); - } + recordRepeatedSuccessfulCalls({ + state, + toolName: "process", + toolParams: params, + result, + count: 2, + }); const warningResult = detectToolCallLoop(state, "process", params, config); expect(warningResult.stuck).toBe(true); if (warningResult.stuck) { expect(warningResult.level).toBe("warning"); } - recordSuccessfulCall(state, "process", params, result, 2); - recordSuccessfulCall(state, "process", params, result, 3); + recordRepeatedSuccessfulCalls({ + state, + toolName: "process", + toolParams: params, + result, + count: 2, + startIndex: 2, + }); const criticalResult = detectToolCallLoop(state, "process", params, config); expect(criticalResult.stuck).toBe(true); if (criticalResult.stuck) { @@ -285,11 +321,7 @@ describe("tool-loop-detection", () => { it("can disable specific detectors", () => { const state = createState(); - const params = { action: "poll", sessionId: "sess-no-detectors" }; - const result = { - content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], - details: { status: "running", aggregated: "steady" }, - }; + const { params, result } = createNoProgressPollFixture("sess-no-detectors"); const config: ToolLoopDetectionConfig = { enabled: true, detectors: { @@ -299,9 +331,13 @@ describe("tool-loop-detection", () => { }, }; - for (let i = 0; i < CRITICAL_THRESHOLD; i += 1) { - recordSuccessfulCall(state, "process", params, result, i); - } + recordRepeatedSuccessfulCalls({ + state, + toolName: "process", + toolParams: params, + result, + count: CRITICAL_THRESHOLD, + }); const loopResult = detectToolCallLoop(state, "process", params, config); expect(loopResult.stuck).toBe(false); @@ -309,15 +345,14 @@ describe("tool-loop-detection", () => { it("warns for known polling no-progress loops", () => { const state = createState(); - const params = { action: "poll", sessionId: "sess-1" }; - const result = { - content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], - details: { status: "running", aggregated: "steady" }, - }; - - for (let i = 0; i < WARNING_THRESHOLD; i += 1) { - recordSuccessfulCall(state, "process", params, result, i); - } + const { params, result } = createNoProgressPollFixture("sess-1"); + recordRepeatedSuccessfulCalls({ + state, + toolName: "process", + toolParams: params, + result, + count: WARNING_THRESHOLD, + }); const loopResult = detectToolCallLoop(state, "process", params, enabledLoopDetectionConfig); expect(loopResult.stuck).toBe(true); @@ -330,15 +365,14 @@ describe("tool-loop-detection", () => { it("blocks known polling no-progress loops at critical threshold", () => { const state = createState(); - const params = { action: "poll", sessionId: "sess-1" }; - const result = { - content: [{ type: "text", text: "(no new output)\n\nProcess still running." }], - details: { status: "running", aggregated: "steady" }, - }; - - for (let i = 0; i < CRITICAL_THRESHOLD; i += 1) { - recordSuccessfulCall(state, "process", params, result, i); - } + const { params, result } = createNoProgressPollFixture("sess-1"); + recordRepeatedSuccessfulCalls({ + state, + toolName: "process", + toolParams: params, + result, + count: CRITICAL_THRESHOLD, + }); const loopResult = detectToolCallLoop(state, "process", params, enabledLoopDetectionConfig); expect(loopResult.stuck).toBe(true); diff --git a/src/agents/tools/cron-tool.test.ts b/src/agents/tools/cron-tool.test.ts index 1c19f16f243..d1a1bb429bc 100644 --- a/src/agents/tools/cron-tool.test.ts +++ b/src/agents/tools/cron-tool.test.ts @@ -15,6 +15,19 @@ vi.mock("../agent-scope.js", () => ({ import { createCronTool } from "./cron-tool.js"; describe("cron tool", () => { + function readGatewayCall(index = 0): { method?: string; params?: Record } { + return ( + (callGatewayMock.mock.calls[index]?.[0] as + | { method?: string; params?: Record } + | undefined) ?? { method: undefined, params: undefined } + ); + } + + function readCronPayloadText(index = 0): string { + const params = readGatewayCall(index).params as { payload?: { text?: string } } | undefined; + return params?.payload?.text ?? ""; + } + async function executeAddAndReadDelivery(params: { callId: string; agentSessionKey: string; @@ -37,6 +50,39 @@ describe("cron tool", () => { return call?.params?.delivery; } + async function executeAddAndReadSessionKey(params: { + callId: string; + agentSessionKey: string; + jobSessionKey?: string; + }): Promise { + const tool = createCronTool({ agentSessionKey: params.agentSessionKey }); + await tool.execute(params.callId, { + action: "add", + job: { + name: "wake-up", + schedule: { at: new Date(123).toISOString() }, + ...(params.jobSessionKey ? { sessionKey: params.jobSessionKey } : {}), + payload: { kind: "systemEvent", text: "hello" }, + }, + }); + const call = readGatewayCall(); + const payload = call.params as { sessionKey?: string } | undefined; + return payload?.sessionKey; + } + + async function executeAddWithContextMessages(callId: string, contextMessages: number) { + const tool = createCronTool({ agentSessionKey: "main" }); + await tool.execute(callId, { + action: "add", + contextMessages, + job: { + name: "reminder", + schedule: { at: new Date(123).toISOString() }, + payload: { kind: "systemEvent", text: "Reminder: the thing." }, + }, + }); + } + beforeEach(() => { callGatewayMock.mockClear(); callGatewayMock.mockResolvedValue({ ok: true }); @@ -156,40 +202,22 @@ describe("cron tool", () => { callGatewayMock.mockResolvedValueOnce({ ok: true }); const callerSessionKey = "agent:main:discord:channel:ops"; - const tool = createCronTool({ agentSessionKey: callerSessionKey }); - await tool.execute("call-session-key", { - action: "add", - job: { - name: "wake-up", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "systemEvent", text: "hello" }, - }, + const sessionKey = await executeAddAndReadSessionKey({ + callId: "call-session-key", + agentSessionKey: callerSessionKey, }); - - const call = callGatewayMock.mock.calls[0]?.[0] as { - params?: { sessionKey?: string }; - }; - expect(call?.params?.sessionKey).toBe(callerSessionKey); + expect(sessionKey).toBe(callerSessionKey); }); it("preserves explicit job.sessionKey on add", async () => { callGatewayMock.mockResolvedValueOnce({ ok: true }); - const tool = createCronTool({ agentSessionKey: "agent:main:discord:channel:ops" }); - await tool.execute("call-explicit-session-key", { - action: "add", - job: { - name: "wake-up", - schedule: { at: new Date(123).toISOString() }, - sessionKey: "agent:main:telegram:group:-100123:topic:99", - payload: { kind: "systemEvent", text: "hello" }, - }, + const sessionKey = await executeAddAndReadSessionKey({ + callId: "call-explicit-session-key", + agentSessionKey: "agent:main:discord:channel:ops", + jobSessionKey: "agent:main:telegram:group:-100123:topic:99", }); - - const call = callGatewayMock.mock.calls[0]?.[0] as { - params?: { sessionKey?: string }; - }; - expect(call?.params?.sessionKey).toBe("agent:main:telegram:group:-100123:topic:99"); + expect(sessionKey).toBe("agent:main:telegram:group:-100123:topic:99"); }); it("adds recent context for systemEvent reminders when contextMessages > 0", async () => { @@ -206,30 +234,15 @@ describe("cron tool", () => { }) .mockResolvedValueOnce({ ok: true }); - const tool = createCronTool({ agentSessionKey: "main" }); - await tool.execute("call3", { - action: "add", - contextMessages: 3, - job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "systemEvent", text: "Reminder: the thing." }, - }, - }); + await executeAddWithContextMessages("call3", 3); expect(callGatewayMock).toHaveBeenCalledTimes(2); - const historyCall = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: unknown; - }; + const historyCall = readGatewayCall(0); expect(historyCall.method).toBe("chat.history"); - const cronCall = callGatewayMock.mock.calls[1]?.[0] as { - method?: string; - params?: { payload?: { text?: string } }; - }; + const cronCall = readGatewayCall(1); expect(cronCall.method).toBe("cron.add"); - const text = cronCall.params?.payload?.text ?? ""; + const text = readCronPayloadText(1); expect(text).toContain("Recent context:"); expect(text).toContain("User: Discussed Q2 budget"); expect(text).toContain("Assistant: We agreed to review on Tuesday."); @@ -243,29 +256,15 @@ describe("cron tool", () => { })); callGatewayMock.mockResolvedValueOnce({ messages }).mockResolvedValueOnce({ ok: true }); - const tool = createCronTool({ agentSessionKey: "main" }); - await tool.execute("call5", { - action: "add", - contextMessages: 20, - job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "systemEvent", text: "Reminder: the thing." }, - }, - }); + await executeAddWithContextMessages("call5", 20); expect(callGatewayMock).toHaveBeenCalledTimes(2); - const historyCall = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { limit?: number }; - }; + const historyCall = readGatewayCall(0); expect(historyCall.method).toBe("chat.history"); - expect(historyCall.params?.limit).toBe(10); + const historyParams = historyCall.params as { limit?: number } | undefined; + expect(historyParams?.limit).toBe(10); - const cronCall = callGatewayMock.mock.calls[1]?.[0] as { - params?: { payload?: { text?: string } }; - }; - const text = cronCall.params?.payload?.text ?? ""; + const text = readCronPayloadText(1); expect(text).not.toMatch(/Message 1\\b/); expect(text).not.toMatch(/Message 2\\b/); expect(text).toContain("Message 3"); @@ -287,12 +286,9 @@ describe("cron tool", () => { // Should only call cron.add, not chat.history expect(callGatewayMock).toHaveBeenCalledTimes(1); - const cronCall = callGatewayMock.mock.calls[0]?.[0] as { - method?: string; - params?: { payload?: { text?: string } }; - }; + const cronCall = readGatewayCall(0); expect(cronCall.method).toBe("cron.add"); - const text = cronCall.params?.payload?.text ?? ""; + const text = readCronPayloadText(0); expect(text).not.toContain("Recent context:"); }); @@ -462,42 +458,22 @@ describe("cron tool", () => { it("does not infer delivery when mode is none", async () => { callGatewayMock.mockResolvedValueOnce({ ok: true }); - - const tool = createCronTool({ agentSessionKey: "agent:main:discord:dm:buddy" }); - await tool.execute("call-none", { - action: "add", - job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "agentTurn", message: "hello" }, - delivery: { mode: "none" }, - }, + const delivery = await executeAddAndReadDelivery({ + callId: "call-none", + agentSessionKey: "agent:main:discord:dm:buddy", + delivery: { mode: "none" }, }); - - const call = callGatewayMock.mock.calls[0]?.[0] as { - params?: { delivery?: { mode?: string; channel?: string; to?: string } }; - }; - expect(call?.params?.delivery).toEqual({ mode: "none" }); + expect(delivery).toEqual({ mode: "none" }); }); it("does not infer announce delivery when mode is webhook", async () => { callGatewayMock.mockResolvedValueOnce({ ok: true }); - - const tool = createCronTool({ agentSessionKey: "agent:main:discord:dm:buddy" }); - await tool.execute("call-webhook-explicit", { - action: "add", - job: { - name: "reminder", - schedule: { at: new Date(123).toISOString() }, - payload: { kind: "agentTurn", message: "hello" }, - delivery: { mode: "webhook", to: "https://example.invalid/cron-finished" }, - }, + const delivery = await executeAddAndReadDelivery({ + callId: "call-webhook-explicit", + agentSessionKey: "agent:main:discord:dm:buddy", + delivery: { mode: "webhook", to: "https://example.invalid/cron-finished" }, }); - - const call = callGatewayMock.mock.calls[0]?.[0] as { - params?: { delivery?: { mode?: string; channel?: string; to?: string } }; - }; - expect(call?.params?.delivery).toEqual({ + expect(delivery).toEqual({ mode: "webhook", to: "https://example.invalid/cron-finished", }); diff --git a/src/agents/tools/message-tool.test.ts b/src/agents/tools/message-tool.test.ts index 77d4441ae1e..b7d5fe29961 100644 --- a/src/agents/tools/message-tool.test.ts +++ b/src/agents/tools/message-tool.test.ts @@ -32,6 +32,14 @@ function mockSendResult(overrides: { channel?: string; to?: string } = {}) { } satisfies MessageActionRunResult); } +function getToolProperties(tool: ReturnType) { + return (tool.parameters as { properties?: Record }).properties ?? {}; +} + +function getActionEnum(properties: Record) { + return (properties.action as { enum?: string[] } | undefined)?.enum ?? []; +} + describe("message tool agent routing", () => { it("derives agentId from the session key", async () => { mockSendResult(); @@ -149,9 +157,8 @@ describe("message tool schema scoping", () => { config: {} as never, currentChannelProvider: "telegram", }); - const properties = - (tool.parameters as { properties?: Record }).properties ?? {}; - const actionEnum = (properties.action as { enum?: string[] } | undefined)?.enum ?? []; + const properties = getToolProperties(tool); + const actionEnum = getActionEnum(properties); expect(properties.components).toBeUndefined(); expect(properties.buttons).toBeDefined(); @@ -179,9 +186,8 @@ describe("message tool schema scoping", () => { config: {} as never, currentChannelProvider: "discord", }); - const properties = - (tool.parameters as { properties?: Record }).properties ?? {}; - const actionEnum = (properties.action as { enum?: string[] } | undefined)?.enum ?? []; + const properties = getToolProperties(tool); + const actionEnum = getActionEnum(properties); expect(properties.components).toBeDefined(); expect(properties.buttons).toBeUndefined(); diff --git a/src/agents/tools/slack-actions.test.ts b/src/agents/tools/slack-actions.test.ts index fffeb528a13..550b9b5a1da 100644 --- a/src/agents/tools/slack-actions.test.ts +++ b/src/agents/tools/slack-actions.test.ts @@ -49,6 +49,30 @@ describe("handleSlackAction", () => { } as OpenClawConfig; } + function createReplyToFirstContext(hasRepliedRef: { value: boolean }) { + return { + currentChannelId: "C123", + currentThreadTs: "1111111111.111111", + replyToMode: "first" as const, + hasRepliedRef, + }; + } + + async function resolveReadToken(cfg: OpenClawConfig): Promise { + readSlackMessages.mockClear(); + readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); + await handleSlackAction({ action: "readMessages", channelId: "C1" }, cfg); + const opts = readSlackMessages.mock.calls[0]?.[1] as { token?: string } | undefined; + return opts?.token; + } + + async function resolveSendToken(cfg: OpenClawConfig): Promise { + sendSlackMessage.mockClear(); + await handleSlackAction({ action: "sendMessage", to: "channel:C1", content: "Hello" }, cfg); + const opts = sendSlackMessage.mock.calls[0]?.[2] as { token?: string } | undefined; + return opts?.token; + } + beforeEach(() => { vi.clearAllMocks(); }); @@ -285,12 +309,7 @@ describe("handleSlackAction", () => { const cfg = { channels: { slack: { botToken: "tok" } } } as OpenClawConfig; sendSlackMessage.mockClear(); const hasRepliedRef = { value: false }; - const context = { - currentChannelId: "C123", - currentThreadTs: "1111111111.111111", - replyToMode: "first" as const, - hasRepliedRef, - }; + const context = createReplyToFirstContext(hasRepliedRef); // First message should be threaded await handleSlackAction( @@ -322,12 +341,7 @@ describe("handleSlackAction", () => { const cfg = { channels: { slack: { botToken: "tok" } } } as OpenClawConfig; sendSlackMessage.mockClear(); const hasRepliedRef = { value: false }; - const context = { - currentChannelId: "C123", - currentThreadTs: "1111111111.111111", - replyToMode: "first" as const, - hasRepliedRef, - }; + const context = createReplyToFirstContext(hasRepliedRef); await handleSlackAction( { @@ -521,32 +535,21 @@ describe("handleSlackAction", () => { const cfg = { channels: { slack: { botToken: "xoxb-1", userToken: "xoxp-1" } }, } as OpenClawConfig; - readSlackMessages.mockClear(); - readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); - await handleSlackAction({ action: "readMessages", channelId: "C1" }, cfg); - const opts = readSlackMessages.mock.calls[0]?.[1] as { token?: string } | undefined; - expect(opts?.token).toBe("xoxp-1"); + expect(await resolveReadToken(cfg)).toBe("xoxp-1"); }); it("falls back to bot token for reads when user token missing", async () => { const cfg = { channels: { slack: { botToken: "xoxb-1" } }, } as OpenClawConfig; - readSlackMessages.mockClear(); - readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); - await handleSlackAction({ action: "readMessages", channelId: "C1" }, cfg); - const opts = readSlackMessages.mock.calls[0]?.[1] as { token?: string } | undefined; - expect(opts?.token).toBeUndefined(); + expect(await resolveReadToken(cfg)).toBeUndefined(); }); it("uses bot token for writes when userTokenReadOnly is true", async () => { const cfg = { channels: { slack: { botToken: "xoxb-1", userToken: "xoxp-1" } }, } as OpenClawConfig; - sendSlackMessage.mockClear(); - await handleSlackAction({ action: "sendMessage", to: "channel:C1", content: "Hello" }, cfg); - const opts = sendSlackMessage.mock.calls[0]?.[2] as { token?: string } | undefined; - expect(opts?.token).toBeUndefined(); + expect(await resolveSendToken(cfg)).toBeUndefined(); }); it("allows user token writes when bot token is missing", async () => { @@ -555,10 +558,7 @@ describe("handleSlackAction", () => { slack: { userToken: "xoxp-1", userTokenReadOnly: false }, }, } as OpenClawConfig; - sendSlackMessage.mockClear(); - await handleSlackAction({ action: "sendMessage", to: "channel:C1", content: "Hello" }, cfg); - const opts = sendSlackMessage.mock.calls[0]?.[2] as { token?: string } | undefined; - expect(opts?.token).toBe("xoxp-1"); + expect(await resolveSendToken(cfg)).toBe("xoxp-1"); }); it("returns all emojis when no limit is provided", async () => { diff --git a/src/agents/tools/web-search.ts b/src/agents/tools/web-search.ts index 3f1c585ea6c..c3a5d7692d0 100644 --- a/src/agents/tools/web-search.ts +++ b/src/agents/tools/web-search.ts @@ -468,6 +468,12 @@ function resolveSiteName(url: string | undefined): string | undefined { } } +async function throwWebSearchApiError(res: Response, providerLabel: string): Promise { + const detailResult = await readResponseText(res, { maxBytes: 64_000 }); + const detail = detailResult.text; + throw new Error(`${providerLabel} API error (${res.status}): ${detail || res.statusText}`); +} + async function runPerplexitySearch(params: { query: string; apiKey: string; @@ -508,9 +514,7 @@ async function runPerplexitySearch(params: { }); if (!res.ok) { - const detailResult = await readResponseText(res, { maxBytes: 64_000 }); - const detail = detailResult.text; - throw new Error(`Perplexity API error (${res.status}): ${detail || res.statusText}`); + return throwWebSearchApiError(res, "Perplexity"); } const data = (await res.json()) as PerplexitySearchResponse; @@ -558,9 +562,7 @@ async function runGrokSearch(params: { }); if (!res.ok) { - const detailResult = await readResponseText(res, { maxBytes: 64_000 }); - const detail = detailResult.text; - throw new Error(`xAI API error (${res.status}): ${detail || res.statusText}`); + return throwWebSearchApiError(res, "xAI"); } const data = (await res.json()) as GrokSearchResponse; diff --git a/src/test-utils/auth-token-assertions.ts b/src/test-utils/auth-token-assertions.ts new file mode 100644 index 00000000000..606f0bae883 --- /dev/null +++ b/src/test-utils/auth-token-assertions.ts @@ -0,0 +1,13 @@ +import { expect } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; + +export function expectGeneratedTokenPersistedToGatewayAuth(params: { + generatedToken?: string; + authToken?: string; + persistedConfig?: OpenClawConfig; +}) { + expect(params.generatedToken).toMatch(/^[0-9a-f]{48}$/); + expect(params.authToken).toBe(params.generatedToken); + expect(params.persistedConfig?.gateway?.auth?.mode).toBe("token"); + expect(params.persistedConfig?.gateway?.auth?.token).toBe(params.generatedToken); +}