From e91aa0657e5e70a2328bf51b9f92acd02bb2d201 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 12 Jan 2026 17:48:08 +0000 Subject: [PATCH] fix: add copilot tests and lint fixes --- src/agents/models-config.providers.ts | 2 +- src/agents/models-config.test.ts | 195 +++++++++++++++++- src/agents/pi-embedded-helpers.ts | 5 +- src/agents/pi-embedded-runner.guard.test.ts | 12 +- src/agents/pi-embedded-runner.ts | 14 +- .../pi-extensions/transcript-sanitize.ts | 5 +- .../session-tool-result-guard-wrapper.ts | 6 +- src/agents/session-tool-result-guard.test.ts | 8 +- src/agents/session-tool-result-guard.ts | 3 +- 9 files changed, 214 insertions(+), 36 deletions(-) diff --git a/src/agents/models-config.providers.ts b/src/agents/models-config.providers.ts index 181da9e27..f62502ca4 100644 --- a/src/agents/models-config.providers.ts +++ b/src/agents/models-config.providers.ts @@ -34,7 +34,7 @@ const MOONSHOT_DEFAULT_COST = { function normalizeApiKeyConfig(value: string): string { const trimmed = value.trim(); const match = /^\$\{([A-Z0-9_]+)\}$/.exec(trimmed); - return match ? match[1] : trimmed; + return match?.[1] ?? trimmed; } function resolveEnvApiKeyVarName(provider: string): string | undefined { diff --git a/src/agents/models-config.test.ts b/src/agents/models-config.test.ts index 641536255..936aa6c02 100644 --- a/src/agents/models-config.test.ts +++ b/src/agents/models-config.test.ts @@ -35,7 +35,7 @@ const MODELS_CONFIG: ClawdbotConfig = { describe("models config", () => { it("auto-injects github-copilot provider when token is present", async () => { - await withTempHome(async () => { + await withTempHome(async (home) => { const previous = process.env.COPILOT_GITHUB_TOKEN; process.env.COPILOT_GITHUB_TOKEN = "gh-token"; @@ -54,11 +54,10 @@ describe("models config", () => { })); const { ensureClawdbotModelsJson } = await import("./models-config.js"); - const { resolveClawdbotAgentDir } = await import("./agent-paths.js"); - await ensureClawdbotModelsJson({ models: { providers: {} } }); + const agentDir = path.join(home, "agent-default-base-url"); + await ensureClawdbotModelsJson({ models: { providers: {} } }, agentDir); - const agentDir = resolveClawdbotAgentDir(); const raw = await fs.readFile( path.join(agentDir, "models.json"), "utf8", @@ -77,6 +76,114 @@ describe("models config", () => { }); }); + it("prefers COPILOT_GITHUB_TOKEN over GH_TOKEN and GITHUB_TOKEN", async () => { + await withTempHome(async () => { + const previous = process.env.COPILOT_GITHUB_TOKEN; + const previousGh = process.env.GH_TOKEN; + const previousGithub = process.env.GITHUB_TOKEN; + process.env.COPILOT_GITHUB_TOKEN = "copilot-token"; + process.env.GH_TOKEN = "gh-token"; + process.env.GITHUB_TOKEN = "github-token"; + + try { + vi.resetModules(); + + const resolveCopilotApiToken = vi.fn().mockResolvedValue({ + token: "copilot", + expiresAt: Date.now() + 60 * 60 * 1000, + source: "mock", + baseUrl: "https://api.copilot.example", + }); + + vi.doMock("../providers/github-copilot-token.js", () => ({ + DEFAULT_COPILOT_API_BASE_URL: + "https://api.individual.githubcopilot.com", + resolveCopilotApiToken, + })); + + const { ensureClawdbotModelsJson } = await import("./models-config.js"); + + await ensureClawdbotModelsJson({ models: { providers: {} } }); + + expect(resolveCopilotApiToken).toHaveBeenCalledWith( + expect.objectContaining({ githubToken: "copilot-token" }), + ); + } finally { + process.env.COPILOT_GITHUB_TOKEN = previous; + process.env.GH_TOKEN = previousGh; + process.env.GITHUB_TOKEN = previousGithub; + } + }); + }); + + it("uses the first github-copilot profile when env tokens are missing", async () => { + await withTempHome(async (home) => { + const previous = process.env.COPILOT_GITHUB_TOKEN; + const previousGh = process.env.GH_TOKEN; + const previousGithub = process.env.GITHUB_TOKEN; + delete process.env.COPILOT_GITHUB_TOKEN; + delete process.env.GH_TOKEN; + delete process.env.GITHUB_TOKEN; + + try { + vi.resetModules(); + + const agentDir = path.join(home, "agent-profiles"); + await fs.mkdir(agentDir, { recursive: true }); + await fs.writeFile( + path.join(agentDir, "auth-profiles.json"), + JSON.stringify( + { + version: 1, + profiles: { + "github-copilot:alpha": { + type: "token", + provider: "github-copilot", + token: "alpha-token", + }, + "github-copilot:beta": { + type: "token", + provider: "github-copilot", + token: "beta-token", + }, + }, + }, + null, + 2, + ), + ); + + const resolveCopilotApiToken = vi.fn().mockResolvedValue({ + token: "copilot", + expiresAt: Date.now() + 60 * 60 * 1000, + source: "mock", + baseUrl: "https://api.copilot.example", + }); + + vi.doMock("../providers/github-copilot-token.js", () => ({ + DEFAULT_COPILOT_API_BASE_URL: + "https://api.individual.githubcopilot.com", + resolveCopilotApiToken, + })); + + const { ensureClawdbotModelsJson } = await import("./models-config.js"); + + await ensureClawdbotModelsJson({ models: { providers: {} } }, agentDir); + + expect(resolveCopilotApiToken).toHaveBeenCalledWith( + expect.objectContaining({ githubToken: "alpha-token" }), + ); + } finally { + if (previous === undefined) delete process.env.COPILOT_GITHUB_TOKEN; + else process.env.COPILOT_GITHUB_TOKEN = previous; + if (previousGh === undefined) delete process.env.GH_TOKEN; + else process.env.GH_TOKEN = previousGh; + if (previousGithub === undefined) delete process.env.GITHUB_TOKEN; + else process.env.GITHUB_TOKEN = previousGithub; + } + }); + }); + it("does not override explicit github-copilot provider config", async () => { await withTempHome(async () => { const previous = process.env.COPILOT_GITHUB_TOKEN; @@ -129,6 +236,42 @@ describe("models config", () => { }); }); + it("falls back to default baseUrl when token exchange fails", async () => { + await withTempHome(async () => { + const previous = process.env.COPILOT_GITHUB_TOKEN; + process.env.COPILOT_GITHUB_TOKEN = "gh-token"; + + try { + vi.resetModules(); + + vi.doMock("../providers/github-copilot-token.js", () => ({ + DEFAULT_COPILOT_API_BASE_URL: "https://api.default.test", + resolveCopilotApiToken: vi.fn().mockRejectedValue(new Error("boom")), + })); + + const { ensureClawdbotModelsJson } = await import("./models-config.js"); + const { resolveClawdbotAgentDir } = await import("./agent-paths.js"); + + await ensureClawdbotModelsJson({ models: { providers: {} } }); + + const agentDir = resolveClawdbotAgentDir(); + 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.default.test", + ); + } finally { + process.env.COPILOT_GITHUB_TOKEN = previous; + } + }); + }); + it("uses agentDir override auth profiles for copilot injection", async () => { await withTempHome(async (home) => { const previous = process.env.COPILOT_GITHUB_TOKEN; @@ -197,6 +340,50 @@ describe("models config", () => { } }); }); + + it("skips writing models.json when no env token or profile exists", async () => { + await withTempHome(async (home) => { + const previous = process.env.COPILOT_GITHUB_TOKEN; + const previousGh = process.env.GH_TOKEN; + const previousGithub = process.env.GITHUB_TOKEN; + const previousMinimax = process.env.MINIMAX_API_KEY; + const previousMoonshot = process.env.MOONSHOT_API_KEY; + delete process.env.COPILOT_GITHUB_TOKEN; + delete process.env.GH_TOKEN; + delete process.env.GITHUB_TOKEN; + delete process.env.MINIMAX_API_KEY; + delete process.env.MOONSHOT_API_KEY; + + try { + vi.resetModules(); + const { ensureClawdbotModelsJson } = await import("./models-config.js"); + + const agentDir = path.join(home, "agent-empty"); + const result = await ensureClawdbotModelsJson( + { + models: { providers: {} }, + }, + agentDir, + ); + + await expect( + fs.stat(path.join(agentDir, "models.json")), + ).rejects.toThrow(); + expect(result.wrote).toBe(false); + } finally { + if (previous === undefined) delete process.env.COPILOT_GITHUB_TOKEN; + else process.env.COPILOT_GITHUB_TOKEN = previous; + if (previousGh === undefined) delete process.env.GH_TOKEN; + else process.env.GH_TOKEN = previousGh; + if (previousGithub === undefined) delete process.env.GITHUB_TOKEN; + else process.env.GITHUB_TOKEN = previousGithub; + if (previousMinimax === undefined) delete process.env.MINIMAX_API_KEY; + else process.env.MINIMAX_API_KEY = previousMinimax; + if (previousMoonshot === undefined) delete process.env.MOONSHOT_API_KEY; + else process.env.MOONSHOT_API_KEY = previousMoonshot; + } + }); + }); let previousHome: string | undefined; beforeEach(() => { diff --git a/src/agents/pi-embedded-helpers.ts b/src/agents/pi-embedded-helpers.ts index 7cd1817c3..384286e1f 100644 --- a/src/agents/pi-embedded-helpers.ts +++ b/src/agents/pi-embedded-helpers.ts @@ -12,16 +12,13 @@ import { } from "../auto-reply/thinking.js"; import type { ClawdbotConfig } from "../config/config.js"; import { formatSandboxToolPolicyBlockedMessage } from "./sandbox.js"; +import { repairToolUseResultPairing } from "./session-transcript-repair.js"; import { isValidCloudCodeAssistToolId, sanitizeToolCallId, sanitizeToolCallIdsForCloudCodeAssist, } from "./tool-call-id.js"; import { sanitizeContentBlocksImages } from "./tool-images.js"; -import { - repairToolUseResultPairing, - sanitizeToolUseResultPairing, -} from "./session-transcript-repair.js"; import type { WorkspaceBootstrapFile } from "./workspace.js"; export type EmbeddedContextFile = { path: string; content: string }; diff --git a/src/agents/pi-embedded-runner.guard.test.ts b/src/agents/pi-embedded-runner.guard.test.ts index 4e1816b81..08a0c7c9f 100644 --- a/src/agents/pi-embedded-runner.guard.test.ts +++ b/src/agents/pi-embedded-runner.guard.test.ts @@ -1,7 +1,6 @@ -import { describe, expect, it } from "vitest"; - import type { AgentMessage } from "@mariozechner/pi-agent-core"; import { SessionManager } from "@mariozechner/pi-coding-agent"; +import { describe, expect, it } from "vitest"; import { guardSessionManager } from "./session-tool-result-guard-wrapper.js"; import { sanitizeToolUseResultPairing } from "./session-transcript-repair.js"; @@ -34,9 +33,10 @@ describe("guardSessionManager integration", () => { "assistant", ]); expect((messages[1] as { toolCallId?: string }).toolCallId).toBe("call_1"); - expect( - sanitizeToolUseResultPairing(messages).map((m) => m.role), - ).toEqual(["assistant", "toolResult", "assistant"]); + expect(sanitizeToolUseResultPairing(messages).map((m) => m.role)).toEqual([ + "assistant", + "toolResult", + "assistant", + ]); }); }); - diff --git a/src/agents/pi-embedded-runner.ts b/src/agents/pi-embedded-runner.ts index 9a2949872..f0960e295 100644 --- a/src/agents/pi-embedded-runner.ts +++ b/src/agents/pi-embedded-runner.ts @@ -117,11 +117,8 @@ import { makeToolPrunablePredicate } from "./pi-extensions/context-pruning/tools import { toToolDefinitions } from "./pi-tool-definition-adapter.js"; import { createClawdbotCodingTools } from "./pi-tools.js"; import { resolveSandboxContext } from "./sandbox.js"; +import { guardSessionManager } from "./session-tool-result-guard-wrapper.js"; import { sanitizeToolUseResultPairing } from "./session-transcript-repair.js"; -import { - guardSessionManager, - type GuardedSessionManager, -} from "./session-tool-result-guard-wrapper.js"; import { applySkillEnvOverrides, applySkillEnvOverridesFromSnapshot, @@ -1672,8 +1669,7 @@ export async function runEmbeddedPiAgent(params: { model, }); - const toolResultGuard = - installSessionToolResultGuard(sessionManager); + const toolResultGuard = guardSessionManager(sessionManager); const { builtInTools, customTools } = splitSdkTools({ tools, @@ -1727,7 +1723,7 @@ export async function runEmbeddedPiAgent(params: { session.agent.replaceMessages(limited); } } catch (err) { - toolResultGuard.flushPendingToolResults(); + toolResultGuard.flushPendingToolResults?.(); session.dispose(); await sessionLock.release(); throw err; @@ -1759,7 +1755,7 @@ export async function runEmbeddedPiAgent(params: { enforceFinalTag: params.enforceFinalTag, }); } catch (err) { - toolResultGuard.flushPendingToolResults(); + toolResultGuard.flushPendingToolResults?.(); session.dispose(); await sessionLock.release(); throw err; @@ -1857,7 +1853,7 @@ export async function runEmbeddedPiAgent(params: { ACTIVE_EMBEDDED_RUNS.delete(params.sessionId); notifyEmbeddedRunEnded(params.sessionId); } - sessionManager.flushPendingToolResults?.(); + toolResultGuard.flushPendingToolResults?.(); session.dispose(); await sessionLock.release(); params.abortSignal?.removeEventListener?.("abort", onAbort); diff --git a/src/agents/pi-extensions/transcript-sanitize.ts b/src/agents/pi-extensions/transcript-sanitize.ts index 5ac8333ac..81af767e3 100644 --- a/src/agents/pi-extensions/transcript-sanitize.ts +++ b/src/agents/pi-extensions/transcript-sanitize.ts @@ -14,10 +14,7 @@ import type { } from "@mariozechner/pi-coding-agent"; import { isGoogleModelApi } from "../pi-embedded-helpers.js"; -import { - repairToolUseResultPairing, - sanitizeToolUseResultPairing, -} from "../session-transcript-repair.js"; +import { repairToolUseResultPairing } from "../session-transcript-repair.js"; import { sanitizeToolCallIdsForCloudCodeAssist } from "../tool-call-id.js"; export default function transcriptSanitizeExtension(api: ExtensionAPI): void { diff --git a/src/agents/session-tool-result-guard-wrapper.ts b/src/agents/session-tool-result-guard-wrapper.ts index 4e11dc9fd..1fc253d75 100644 --- a/src/agents/session-tool-result-guard-wrapper.ts +++ b/src/agents/session-tool-result-guard-wrapper.ts @@ -14,7 +14,10 @@ export type GuardedSessionManager = SessionManager & { export function guardSessionManager( sessionManager: SessionManager, ): GuardedSessionManager { - if (typeof (sessionManager as GuardedSessionManager).flushPendingToolResults === "function") { + if ( + typeof (sessionManager as GuardedSessionManager).flushPendingToolResults === + "function" + ) { return sessionManager as GuardedSessionManager; } @@ -23,4 +26,3 @@ export function guardSessionManager( guard.flushPendingToolResults; return sessionManager as GuardedSessionManager; } - diff --git a/src/agents/session-tool-result-guard.test.ts b/src/agents/session-tool-result-guard.test.ts index ed0d3959a..07cdd6618 100644 --- a/src/agents/session-tool-result-guard.test.ts +++ b/src/agents/session-tool-result-guard.test.ts @@ -1,7 +1,7 @@ -import { describe, expect, it } from "vitest"; +import type { AgentMessage } from "@mariozechner/pi-agent-core"; import { SessionManager } from "@mariozechner/pi-coding-agent"; -import type { AgentMessage } from "@mariozechner/pi-agent-core"; +import { describe, expect, it } from "vitest"; import { installSessionToolResultGuard } from "./session-tool-result-guard.js"; @@ -110,9 +110,7 @@ describe("installSessionToolResultGuard", () => { "toolResult", // synthetic for call_b "assistant", // text ]); - expect( - (messages[2] as { toolCallId?: string }).toolCallId, - ).toBe("call_b"); + expect((messages[2] as { toolCallId?: string }).toolCallId).toBe("call_b"); expect(guard.getPendingIds()).toEqual([]); }); diff --git a/src/agents/session-tool-result-guard.ts b/src/agents/session-tool-result-guard.ts index b672318a2..c37d0b2e9 100644 --- a/src/agents/session-tool-result-guard.ts +++ b/src/agents/session-tool-result-guard.ts @@ -94,7 +94,8 @@ export function installSessionToolResultGuard(sessionManager: SessionManager): { }; // Monkey-patch appendMessage with our guarded version. - sessionManager.appendMessage = guardedAppend as SessionManager["appendMessage"]; + sessionManager.appendMessage = + guardedAppend as SessionManager["appendMessage"]; return { flushPendingToolResults,