From 7bdbb444e6814d2a4bcfba34da16567f499613e7 Mon Sep 17 00:00:00 2001 From: David Garson Date: Wed, 28 Jan 2026 20:33:48 -0700 Subject: [PATCH] fix: tool fixes and sdk logging reduction --- .../claude-agent-sdk/error-handling.test.ts | 20 +- .../claude-agent-sdk/provider-config.ts | 92 +---- .../claude-agent-sdk/sdk-agent-runtime.ts | 13 +- src/agents/claude-agent-sdk/sdk-hooks.ts | 10 + src/agents/claude-agent-sdk/sdk-loader.ts | 33 +- src/agents/claude-agent-sdk/sdk-runner.ts | 385 ++---------------- .../sdk-session-transcript.ts | 3 +- src/agents/claude-agent-sdk/system-prompt.ts | 13 +- .../claude-agent-sdk/tool-bridge.test.ts | 207 +++++++++- src/agents/claude-agent-sdk/tool-bridge.ts | 141 ++++++- .../claude-agent-sdk/tool-bridge.types.ts | 55 ++- src/agents/claude-agent-sdk/types.ts | 4 +- src/agents/cli-credentials.ts | 175 +------- src/agents/main-agent-runtime-factory.ts | 2 - src/agents/pi-agent-runtime.ts | 10 - src/agents/unified-agent-runner.test.ts | 18 +- src/agents/unified-agent-runner.ts | 16 +- .../reply/agent-runner-execution.ts | 11 +- 18 files changed, 467 insertions(+), 741 deletions(-) diff --git a/src/agents/claude-agent-sdk/error-handling.test.ts b/src/agents/claude-agent-sdk/error-handling.test.ts index a2209019e..b9a553f99 100644 --- a/src/agents/claude-agent-sdk/error-handling.test.ts +++ b/src/agents/claude-agent-sdk/error-handling.test.ts @@ -26,6 +26,10 @@ describe("error-handling", () => { beforeEach(() => { vi.clearAllMocks(); vi.useFakeTimers(); + // Mock Math.random to eliminate jitter in retry backoff delays. + // The withRetry function adds 0-20% random jitter to delays, which causes + // tests using exact timer advancement to timeout when jitter > 0. + vi.spyOn(Math, "random").mockReturnValue(0); }); afterEach(() => { @@ -241,9 +245,8 @@ describe("error-handling", () => { const promise = withRetry(fn, options); - // Advance timers for retries - await vi.advanceTimersByTimeAsync(100); - await vi.advanceTimersByTimeAsync(200); + // Run all timers to completion (handles all retries) + await vi.runAllTimersAsync(); const result = await promise; @@ -275,13 +278,14 @@ describe("error-handling", () => { retryOn: ["rate_limit"], }; - const promise = withRetry(fn, options); + // Attach rejection handler BEFORE running timers to avoid unhandled rejection + const promise = withRetry(fn, options).catch((e) => e); - // Advance timers for all retries - await vi.advanceTimersByTimeAsync(100); - await vi.advanceTimersByTimeAsync(200); + // Run all timers to completion (exhausts all retries) + await vi.runAllTimersAsync(); - await expect(promise).rejects.toMatchObject({ status: 429 }); + const error = await promise; + expect(error).toMatchObject({ status: 429 }); expect(fn).toHaveBeenCalledTimes(3); // Initial + 2 retries }); diff --git a/src/agents/claude-agent-sdk/provider-config.ts b/src/agents/claude-agent-sdk/provider-config.ts index a8b1b2a2a..ce58818e7 100644 --- a/src/agents/claude-agent-sdk/provider-config.ts +++ b/src/agents/claude-agent-sdk/provider-config.ts @@ -16,47 +16,6 @@ */ import type { SdkProviderConfig, SdkProviderEnv } from "./types.js"; -import { createSubsystemLogger } from "../../logging/subsystem.js"; - -const log = createSubsystemLogger("agents/claude-agent-sdk"); - -/** - * Mask a token for logging - shows length and first/last 4 chars. - */ -function maskToken(token: string | undefined): string { - if (!token) return "(empty)"; - if (token.length <= 12) return `${"*".repeat(token.length)} (length: ${token.length})`; - const first = token.slice(0, 4); - const last = token.slice(-4); - return `${first}...${"*".repeat(Math.min(8, token.length - 8))}...${last} (length: ${token.length})`; -} - -/** - * Log the resolved provider config with masked credentials. - */ -function logProviderConfig(config: SdkProviderConfig, source: string): void { - const envKeys = config.env ? Object.keys(config.env) : []; - const maskedEnv: Record = {}; - - if (config.env) { - for (const [key, value] of Object.entries(config.env)) { - if (key.includes("KEY") || key.includes("TOKEN") || key.includes("SECRET")) { - maskedEnv[key] = maskToken(value); - } else { - maskedEnv[key] = value ?? "(undefined)"; - } - } - } - - log.debug("[CCSDK-PROVIDER] Provider config resolved", { - source, - providerName: config.name, - envKeys, - maskedEnv, - model: config.model, - maxTurns: config.maxTurns, - }); -} /** * Build provider config for direct Anthropic API access. @@ -81,15 +40,10 @@ export function buildAnthropicSdkProvider(apiKey: string): SdkProviderConfig { * env vars set in their system environment, that's their configuration choice. */ export function buildClaudeCliSdkProvider(): SdkProviderConfig { - log.debug("[CCSDK-PROVIDER] buildClaudeCliSdkProvider called - using SDK native auth"); - - const config: SdkProviderConfig = { + return { name: "Claude CLI (SDK native)", env: {}, }; - - logProviderConfig(config, "sdk-native-auth"); - return config; } /** @@ -127,23 +81,11 @@ export function buildZaiSdkProvider( env.ANTHROPIC_DEFAULT_OPUS_MODEL = options.opusModel; } - const config: SdkProviderConfig = { + return { name: "z.AI", env, model: options?.defaultModel, }; - - log.debug("[CCSDK-PROVIDER] Built z.AI provider config", { - hasBaseUrl: Boolean(options?.baseUrl), - baseUrl: options?.baseUrl ?? "(default)", - defaultModel: options?.defaultModel ?? "(sdk default)", - haikuModel: options?.haikuModel ?? "(sdk default)", - sonnetModel: options?.sonnetModel ?? "(sdk default)", - opusModel: options?.opusModel ?? "(sdk default)", - authTokenMasked: maskToken(authToken), - }); - - return config; } /** @@ -182,22 +124,11 @@ export function buildOpenRouterSdkProvider( env.ANTHROPIC_DEFAULT_OPUS_MODEL = options.opusModel; } - const config: SdkProviderConfig = { + return { name: "OpenRouter (Anthropic-compatible)", env, model: options?.defaultModel, }; - - log.debug("[CCSDK-PROVIDER] Built OpenRouter provider config", { - baseUrl: env.ANTHROPIC_BASE_URL, - defaultModel: options?.defaultModel ?? "(sdk default)", - haikuModel: options?.haikuModel ?? "(sdk default)", - sonnetModel: options?.sonnetModel ?? "(sdk default)", - opusModel: options?.opusModel ?? "(sdk default)", - apiKeyMasked: maskToken(apiKey), - }); - - return config; } /** @@ -249,48 +180,33 @@ export function resolveProviderConfig(options?: { baseUrl?: string; useCliCredentials?: boolean; }): SdkProviderConfig { - log.debug("[CCSDK-PROVIDER] resolveProviderConfig called", { - hasApiKey: Boolean(options?.apiKey), - apiKeyMasked: maskToken(options?.apiKey), - hasAuthToken: Boolean(options?.authToken), - authTokenMasked: maskToken(options?.authToken), - baseUrl: options?.baseUrl ?? "(default)", - useCliCredentials: options?.useCliCredentials ?? true, - }); - // 1. Explicit API key from moltbot config takes precedence if (options?.apiKey) { - log.debug("[CCSDK-PROVIDER] Using explicit API key from moltbot config"); const config = buildAnthropicSdkProvider(options.apiKey); if (options.baseUrl && config.env) { config.env.ANTHROPIC_BASE_URL = options.baseUrl; } - logProviderConfig(config, "explicit-api-key"); return config; } // 2. Explicit auth token from moltbot config (for z.AI, custom endpoints, etc.) if (options?.authToken) { - log.debug("[CCSDK-PROVIDER] Using explicit auth token from moltbot config"); const env: SdkProviderEnv = { ANTHROPIC_AUTH_TOKEN: options.authToken, }; if (options.baseUrl) { env.ANTHROPIC_BASE_URL = options.baseUrl; } - const config: SdkProviderConfig = { + return { name: "Anthropic (auth token)", env, }; - logProviderConfig(config, "explicit-auth-token"); - return config; } // 3. Default: SDK native auth // Let the SDK use its internal credential resolution (keychain → OAuth flow). // We inherit the full parent process env - if the user has ANTHROPIC_API_KEY or // ANTHROPIC_AUTH_TOKEN set, that's their configuration choice. - log.debug("[CCSDK-PROVIDER] Using SDK native auth (no explicit credentials configured)"); const cliConfig = buildClaudeCliSdkProvider(); if (options?.baseUrl && cliConfig.env) { cliConfig.env.ANTHROPIC_BASE_URL = options.baseUrl; diff --git a/src/agents/claude-agent-sdk/sdk-agent-runtime.ts b/src/agents/claude-agent-sdk/sdk-agent-runtime.ts index b1d666380..fe5294893 100644 --- a/src/agents/claude-agent-sdk/sdk-agent-runtime.ts +++ b/src/agents/claude-agent-sdk/sdk-agent-runtime.ts @@ -214,14 +214,13 @@ export function createCcSdkAgentRuntime(context?: CcSdkAgentRuntimeContext): Age ? `${params.provider ?? "anthropic"}/${params.model}` : (ccsdkOpts.modelTiers?.sonnet ?? context?.ccsdkConfig?.models?.sonnet ?? "default"); - log.info("Starting CCSDK Agent session", { + log.debug("Starting CCSDK Agent session", { sessionId: params.sessionId, runId: params.runId, provider: providerConfig.name, model: effectiveModel, agentId, thinkLevel: params.thinkLevel, - verboseLevel: params.verboseLevel, }); // Load conversation history from session file if not provided @@ -316,6 +315,7 @@ export function createCcSdkAgentRuntime(context?: CcSdkAgentRuntimeContext): Age verboseLevel: mapVerboseLevel(params.verboseLevel), timezone: resolveTimezone(effectiveConfig), skills: extractSkillNames(params.skillsSnapshot), + skillsPrompt: params.skillsSnapshot?.prompt, // ─── CCSDK options from ccsdkOptions bag ──────────────────────────────── hooksEnabled: ccsdkOpts.hooksEnabled ?? context?.ccsdkConfig?.hooksEnabled, @@ -347,15 +347,6 @@ export function createCcSdkAgentRuntime(context?: CcSdkAgentRuntimeContext): Age onAgentEvent: params.onAgentEvent, }); - log.debug("[CCSDK-RUNTIME] Callbacks passed to runSdkAgent", { - hasOnPartialReply: Boolean(params.onPartialReply), - hasOnBlockReply: Boolean(params.onBlockReply), - hasOnBlockReplyFlush: Boolean(params.onBlockReplyFlush), - hasOnReasoningStream: Boolean(params.onReasoningStream), - hasOnToolResult: Boolean(params.onToolResult), - hasOnAgentEvent: Boolean(params.onAgentEvent), - }); - // Persist session transcript for multi-turn continuity. if (params.sessionFile) { if (sdkResult.completedToolCalls && sdkResult.completedToolCalls.length > 0) { diff --git a/src/agents/claude-agent-sdk/sdk-hooks.ts b/src/agents/claude-agent-sdk/sdk-hooks.ts index 7f4a4d278..42768774e 100644 --- a/src/agents/claude-agent-sdk/sdk-hooks.ts +++ b/src/agents/claude-agent-sdk/sdk-hooks.ts @@ -11,6 +11,9 @@ import { sanitizeToolResult, } from "../pi-embedded-subscribe.tools.js"; import { normalizeToolName } from "../tool-policy.js"; +import { createSubsystemLogger } from "../../logging/subsystem.js"; + +const log = createSubsystemLogger("agents/claude-agent-sdk/sdk-hooks"); function isRecord(value: unknown): value is Record { return !!value && typeof value === "object" && !Array.isArray(value); @@ -152,6 +155,13 @@ export function buildMoltbotSdkHooks(params: { const normalized = normalizeSdkToolName(rawName, params.mcpServerName); const error = extractToolErrorMessage(record ?? input); + // Log tool failures at warn level so they appear in logs + log.warn("Tool execution failed (via hook)", { + tool: normalized.name, + toolCallId: typeof toolUseId === "string" ? toolUseId : undefined, + error: error?.slice(0, 500) ?? "Unknown error", + }); + params.emitEvent("tool", { phase: "result", name: normalized.name, diff --git a/src/agents/claude-agent-sdk/sdk-loader.ts b/src/agents/claude-agent-sdk/sdk-loader.ts index ab59d49c8..5b2c5c8e5 100644 --- a/src/agents/claude-agent-sdk/sdk-loader.ts +++ b/src/agents/claude-agent-sdk/sdk-loader.ts @@ -61,54 +61,29 @@ export function isSdkAvailable(): boolean { */ export async function loadClaudeAgentSdk(): Promise { if (sdkModule) { - log.debug("[CCSDK-LOADER] Returning cached SDK module"); return sdkModule; } if (loadAttempted && loadError) { - log.debug("[CCSDK-LOADER] Previous load attempt failed, re-throwing cached error"); throw loadError; } loadAttempted = true; - log.debug("[CCSDK-LOADER] Attempting to load Claude Agent SDK"); try { // Dynamic import to avoid bundling issues - // The SDK package name is resolved at runtime const moduleName = "@anthropic-ai/claude-agent-sdk"; - const loadStart = Date.now(); sdkModule = (await import(/* @vite-ignore */ moduleName)) as ClaudeAgentSdkModule; - const loadDuration = Date.now() - loadStart; - // Log SDK module details for debugging - const moduleKeys = Object.keys(sdkModule); - const hasQuery = typeof sdkModule.query === "function"; - - log.info("[CCSDK-LOADER] Claude Agent SDK loaded successfully", { - loadDurationMs: loadDuration, - hasQueryFunction: hasQuery, - moduleExports: moduleKeys.slice(0, 10), - totalExports: moduleKeys.length, - }); - - if (!hasQuery) { - log.error("[CCSDK-LOADER] SDK loaded but query function is missing!", { - moduleKeys, - queryType: typeof sdkModule.query, - }); + if (typeof sdkModule.query !== "function") { + log.error("SDK loaded but query function is missing"); } return sdkModule; } catch (err) { - const errorMessage = err instanceof Error ? err.message : String(err); - const errorStack = err instanceof Error ? err.stack : undefined; - - log.error("[CCSDK-LOADER] Failed to load Claude Agent SDK", { - error: errorMessage, - stack: errorStack, + log.error("Failed to load Claude Agent SDK", { + error: err instanceof Error ? err.message : String(err), }); - loadError = new Error( "Claude Agent SDK not installed. Install with: npm install @anthropic-ai/claude-agent-sdk", ); diff --git a/src/agents/claude-agent-sdk/sdk-runner.ts b/src/agents/claude-agent-sdk/sdk-runner.ts index d511d4543..8f1693b53 100644 --- a/src/agents/claude-agent-sdk/sdk-runner.ts +++ b/src/agents/claude-agent-sdk/sdk-runner.ts @@ -143,90 +143,6 @@ const DEFAULT_MAX_EXTRACTED_CHARS = 120_000; const DEFAULT_MCP_SERVER_NAME = "moltbot"; const DEFAULT_MAX_TURNS = 50; -// --------------------------------------------------------------------------- -// Debug helpers -// --------------------------------------------------------------------------- - -/** - * Mask a token for logging - shows length and first/last 4 chars. - */ -function maskToken(token: string | undefined): string { - if (!token) return "(empty)"; - if (token.length <= 12) return `${"*".repeat(token.length)} (length: ${token.length})`; - const first = token.slice(0, 4); - const last = token.slice(-4); - return `${first}...${"*".repeat(Math.min(8, token.length - 8))}...${last} (length: ${token.length})`; -} - -/** - * Log authentication-related environment variables (masked). - */ -function logAuthEnvVars(env: Record, prefix: string): void { - const authKeys = [ - "ANTHROPIC_API_KEY", - "ANTHROPIC_AUTH_TOKEN", - "ANTHROPIC_BASE_URL", - "CLAUDE_CODE_USE_BEDROCK", - "CLAUDE_CODE_USE_VERTEX", - ]; - const authEnv: Record = {}; - for (const key of authKeys) { - if (env[key]) { - authEnv[key] = key.includes("KEY") || key.includes("TOKEN") ? maskToken(env[key]) : env[key]; - } - } - log.debug(`${prefix} Auth environment variables`, { - authEnv, - totalEnvKeys: Object.keys(env).length, - }); -} - -/** - * Detect potential auth-related error patterns in response text. - */ -function detectAuthErrorPatterns(text: string): { - hasAuthError: boolean; - patterns: string[]; -} { - const patterns: string[] = []; - const lowerText = text.toLowerCase(); - - // Common auth error patterns - if (lowerText.includes("unauthorized") || lowerText.includes("401")) { - patterns.push("unauthorized/401"); - } - if (lowerText.includes("invalid api key") || lowerText.includes("invalid_api_key")) { - patterns.push("invalid_api_key"); - } - if (lowerText.includes("authentication") && lowerText.includes("failed")) { - patterns.push("authentication_failed"); - } - if (lowerText.includes("expired") && (lowerText.includes("token") || lowerText.includes("key"))) { - patterns.push("expired_credentials"); - } - if (lowerText.includes("permission") && lowerText.includes("denied")) { - patterns.push("permission_denied"); - } - if (lowerText.includes("rate limit") || lowerText.includes("ratelimit")) { - patterns.push("rate_limit"); - } - if ( - lowerText.includes("quota") && - (lowerText.includes("exceeded") || lowerText.includes("limit")) - ) { - patterns.push("quota_exceeded"); - } - // Check for suspiciously short responses that might indicate auth issues - if (text.length >= 100 && text.length <= 200) { - patterns.push("suspicious_short_response"); - } - - return { - hasAuthError: patterns.length > 0, - patterns, - }; -} - // --------------------------------------------------------------------------- // Model tier environment helpers // --------------------------------------------------------------------------- @@ -299,13 +215,7 @@ function normalizeModelNameForProvider(modelName: string, providerType: SdkProvi case "anthropic-native": // Strip "anthropic/" prefix for direct Anthropic API if (hasAnthropicPrefix) { - const stripped = modelName.slice("anthropic/".length); - log.debug("[CCSDK-MODEL] Stripped anthropic/ prefix for native API", { - original: modelName, - normalized: stripped, - providerType, - }); - return stripped; + return modelName.slice("anthropic/".length); } return modelName; @@ -313,13 +223,7 @@ function normalizeModelNameForProvider(modelName: string, providerType: SdkProvi // OpenRouter expects "anthropic/model-name" format // Keep as-is if already prefixed, add prefix if not if (!hasAnthropicPrefix && modelName.startsWith("claude-")) { - const prefixed = `anthropic/${modelName}`; - log.debug("[CCSDK-MODEL] Added anthropic/ prefix for OpenRouter", { - original: modelName, - normalized: prefixed, - providerType, - }); - return prefixed; + return `anthropic/${modelName}`; } return modelName; @@ -328,14 +232,10 @@ function normalizeModelNameForProvider(modelName: string, providerType: SdkProvi // The model name should come from config already in z.AI format // Warn if we see Anthropic-formatted model names - likely a config issue if (hasAnthropicPrefix || modelName.startsWith("claude-")) { - log.warn( - "[CCSDK-MODEL] z.AI provider detected but model name looks like Anthropic format", - { - modelName, - providerType, - hint: "For z.AI, configure model tiers with z.AI model names (e.g., 'glm-4.7'), not Anthropic names", - }, - ); + log.warn("z.AI provider detected but model name looks like Anthropic format", { + modelName, + hint: "For z.AI, configure model tiers with z.AI model names (e.g., 'glm-4.7'), not Anthropic names", + }); } // Pass through unchanged return modelName; @@ -343,11 +243,6 @@ function normalizeModelNameForProvider(modelName: string, providerType: SdkProvi case "bedrock": case "vertex": // Bedrock/Vertex have their own model ID formats - // For now, pass through - may need specific handling later - log.debug("[CCSDK-MODEL] Passing model name through for cloud provider", { - modelName, - providerType, - }); return modelName; default: @@ -373,11 +268,6 @@ function buildModelTierEnv( if (!modelTiers) return env; const providerType = detectProviderType(providerConfig); - log.debug("[CCSDK-MODEL] Building model tier env", { - providerType, - providerName: providerConfig?.name, - rawTiers: modelTiers, - }); if (modelTiers.haiku) { env.ANTHROPIC_DEFAULT_HAIKU_MODEL = normalizeModelNameForProvider( @@ -593,46 +483,6 @@ export async function runSdkAgent(params: SdkRunnerParams): Promise 0 ? ` (skipped: ${bridgeResult.skippedTools.join(", ")})` @@ -832,11 +682,6 @@ export async function runSdkAgent(params: SdkRunnerParams): Promise = {}; - // Log inherited auth env vars BEFORE we override them - log.debug("[CCSDK-ENV] Checking inherited auth env vars from parent process", { - hasAnthropicApiKey: Boolean(process.env.ANTHROPIC_API_KEY), - inheritedApiKeyMasked: maskToken(process.env.ANTHROPIC_API_KEY), - hasAnthropicAuthToken: Boolean(process.env.ANTHROPIC_AUTH_TOKEN), - inheritedAuthTokenMasked: maskToken(process.env.ANTHROPIC_AUTH_TOKEN), - hasAnthropicBaseUrl: Boolean(process.env.ANTHROPIC_BASE_URL), - inheritedBaseUrl: process.env.ANTHROPIC_BASE_URL ?? "(unset)", - claudeCodeEntrypoint: process.env.CLAUDE_CODE_ENTRYPOINT ?? "(unset)", - claudeCodeIndicator: process.env.CLAUDECODE ?? "(unset)", - }); - // Inherit parent process environment. for (const [key, value] of Object.entries(process.env)) { if (value !== undefined) sdkEnv[key] = value; @@ -891,10 +725,6 @@ export async function runSdkAgent(params: SdkRunnerParams): Promise 0) { - log.debug("[CCSDK-ENV] Applying model tier env vars", { modelTierEnv }); - } for (const [key, value] of Object.entries(modelTierEnv)) { if (value !== undefined) sdkEnv[key] = value; } - // NOTE: We intentionally do NOT set CLAUDE_CONFIG_DIR. - // Setting it to a custom directory breaks auth because the SDK scopes - // credential resolution (including keychain lookups) to that directory. - // The user's `claude login` credentials are associated with ~/.claude, - // so we must let the SDK use the default config directory. - // - // TODO: If per-agent session isolation is needed, we may need to: - // 1. Symlink auth state from ~/.claude to the agent's config dir, OR - // 2. Use a different mechanism for session isolation that doesn't affect auth - if (params.ccsdkConfigDir) { - log.debug("[CCSDK-ENV] Ignoring ccsdkConfigDir to preserve auth - using default ~/.claude", { - ignoredConfigDir: params.ccsdkConfigDir, - }); - } - - // Log final auth env state - logAuthEnvVars(sdkEnv, "[CCSDK-ENV] Final SDK"); - // Always pass the full environment to the SDK. sdkOptions.env = sdkEnv; @@ -950,7 +759,6 @@ export async function runSdkAgent(params: SdkRunnerParams): Promise @@ -1024,8 +820,6 @@ export async function runSdkAgent(params: SdkRunnerParams): Promise= DEFAULT_MAX_EXTRACTED_CHARS) { truncated = true; - log.debug(`Truncated after ${extractedChars} chars`); break; } } } - - // Log when the stream ends naturally (not via break from result/abort/truncate) - log.debug("SDK event stream ended", { - eventCount, - extractedChars, - aborted, - truncated, - hasResultText: Boolean(resultText), - chunksCount: chunks.length, - }); } catch (err) { if (timeoutId) clearTimeout(timeoutId); @@ -1428,49 +1159,7 @@ export async function runSdkAgent(params: SdkRunnerParams): Promise { /** * Skills enricher. + * + * Prefers the full skillsPrompt (which includes descriptions) over just names. */ const skillsEnricher: PromptEnricher = (ctx) => { + // Prefer full skills prompt with descriptions + if (ctx.skillsPrompt?.trim()) { + return ctx.skillsPrompt.trim(); + } + // Fallback to skill names list if (!ctx.skills?.length) return null; return `Available Moltbot skills: ${ctx.skills.join(", ")}`; }; @@ -189,6 +198,7 @@ export function buildSystemPromptAdditionsFromParams(params: { messageChannel?: string; channelHints?: string; skills?: string[]; + skillsPrompt?: string; senderId?: string | null; senderName?: string | null; senderUsername?: string | null; @@ -202,6 +212,7 @@ export function buildSystemPromptAdditionsFromParams(params: { channel: params.messageChannel, channelHints: params.channelHints, skills: params.skills, + skillsPrompt: params.skillsPrompt, senderId: params.senderId, senderName: params.senderName, senderUsername: params.senderUsername, diff --git a/src/agents/claude-agent-sdk/tool-bridge.test.ts b/src/agents/claude-agent-sdk/tool-bridge.test.ts index b6707d5a2..2a84c5289 100644 --- a/src/agents/claude-agent-sdk/tool-bridge.test.ts +++ b/src/agents/claude-agent-sdk/tool-bridge.test.ts @@ -1,4 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { Type } from "@sinclair/typebox"; import type { AgentToolResult } from "@mariozechner/pi-agent-core"; @@ -18,6 +19,8 @@ vi.mock("../tool-policy.js", () => ({ import { extractJsonSchema, + extractZodCompatibleSchema, + createZodCompatibleSchema, convertToolResult, wrapToolHandler, mcpToolName, @@ -101,6 +104,112 @@ describe("tool-bridge", () => { }); }); + describe("createZodCompatibleSchema", () => { + it("creates schema with parse/safeParse methods", () => { + const typeboxSchema = { + type: "object", + properties: { + name: { type: "string" }, + }, + required: ["name"], + }; + + const zodSchema = createZodCompatibleSchema(typeboxSchema as any); + + expect(typeof zodSchema.parse).toBe("function"); + expect(typeof zodSchema.safeParse).toBe("function"); + expect(typeof zodSchema.safeParseAsync).toBe("function"); + expect(zodSchema._def.typeName).toBe("ZodObject"); + }); + + it("parse returns data for valid input", () => { + const typeboxSchema = Type.Object({ + count: Type.Number(), + }); + + const zodSchema = createZodCompatibleSchema(typeboxSchema); + const result = zodSchema.parse({ count: 42 }); + + expect(result).toEqual({ count: 42 }); + }); + + it("safeParse returns success for valid input", () => { + const typeboxSchema = Type.Object({ + active: Type.Boolean(), + }); + + const zodSchema = createZodCompatibleSchema(typeboxSchema); + const result = zodSchema.safeParse({ active: true }); + + expect(result.success).toBe(true); + expect(result.data).toEqual({ active: true }); + }); + + it("safeParse returns error for invalid input", () => { + const typeboxSchema = Type.Object({ + value: Type.String(), + }); + + const zodSchema = createZodCompatibleSchema(typeboxSchema); + // Missing required 'value' property + const result = zodSchema.safeParse({}); + + expect(result.success).toBe(false); + expect(result.error).toBeDefined(); + }); + + it("safeParseAsync returns promise", async () => { + const typeboxSchema = Type.Object({}); + + const zodSchema = createZodCompatibleSchema(typeboxSchema); + const result = await zodSchema.safeParseAsync({}); + + expect(result.success).toBe(true); + }); + }); + + describe("extractZodCompatibleSchema", () => { + it("returns Zod-compatible schema for tool with parameters", () => { + const tool = { + name: "test_tool", + parameters: { + type: "object", + properties: { + query: { type: "string" }, + }, + }, + } as unknown as AnyAgentTool; + + const schema = extractZodCompatibleSchema(tool); + + expect(schema).toBeDefined(); + expect(typeof schema?.parse).toBe("function"); + expect(typeof schema?.safeParse).toBe("function"); + }); + + it("returns undefined for tool without parameters", () => { + const tool = { + name: "no_params_tool", + parameters: undefined, + } as unknown as AnyAgentTool; + + const schema = extractZodCompatibleSchema(tool); + + expect(schema).toBeUndefined(); + }); + + it("returns undefined for tool with null parameters", () => { + const tool = { + name: "null_params_tool", + parameters: null, + } as unknown as AnyAgentTool; + + const schema = extractZodCompatibleSchema(tool); + + expect(schema).toBeUndefined(); + }); + }); + describe("convertToolResult", () => { it("converts text content blocks", () => { const result: AgentToolResult = { @@ -203,6 +312,9 @@ describe("tool-bridge", () => { }); describe("wrapToolHandler", () => { + // Mock extra object that MCP SDK passes to handlers + const mockExtra = { signal: new AbortController().signal }; + it("executes tool and converts result", async () => { const mockExecute = vi.fn().mockResolvedValue({ content: [{ type: "text", text: "Tool output" }], @@ -214,18 +326,18 @@ describe("tool-bridge", () => { } as unknown as AnyAgentTool; const handler = wrapToolHandler(tool); - const result = await handler({ input: "test" }); + const result = await handler({ input: "test" }, mockExtra); expect(mockExecute).toHaveBeenCalledWith( expect.stringContaining("mcp-bridge-test_tool"), { input: "test" }, - undefined, + mockExtra.signal, undefined, ); expect(result.content).toEqual([{ type: "text", text: "Tool output" }]); }); - it("passes abort signal to tool execute", async () => { + it("uses signal from extra when provided", async () => { const mockExecute = vi.fn().mockResolvedValue({ content: [{ type: "text", text: "Done" }], }); @@ -235,14 +347,36 @@ describe("tool-bridge", () => { execute: mockExecute, } as unknown as AnyAgentTool; - const controller = new AbortController(); - const handler = wrapToolHandler(tool, controller.signal); - await handler({}); + const extraController = new AbortController(); + const handler = wrapToolHandler(tool); + await handler({}, { signal: extraController.signal }); expect(mockExecute).toHaveBeenCalledWith( expect.any(String), {}, - controller.signal, + extraController.signal, + undefined, + ); + }); + + it("falls back to shared abortSignal when extra.signal is undefined", async () => { + const mockExecute = vi.fn().mockResolvedValue({ + content: [{ type: "text", text: "Done" }], + }); + + const tool = { + name: "fallback-signal-tool", + execute: mockExecute, + } as unknown as AnyAgentTool; + + const sharedController = new AbortController(); + const handler = wrapToolHandler(tool, sharedController.signal); + await handler({}, {} as any); + + expect(mockExecute).toHaveBeenCalledWith( + expect.any(String), + {}, + sharedController.signal, undefined, ); }); @@ -256,7 +390,7 @@ describe("tool-bridge", () => { } as unknown as AnyAgentTool; const handler = wrapToolHandler(tool); - const result = await handler({}); + const result = await handler({}, mockExtra); expect(result.isError).toBe(true); expect(result.content[0]).toMatchObject({ @@ -276,7 +410,7 @@ describe("tool-bridge", () => { } as unknown as AnyAgentTool; const handler = wrapToolHandler(tool); - const result = await handler({}); + const result = await handler({}, mockExtra); expect(result.isError).toBe(true); expect(result.content[0]).toMatchObject({ @@ -298,7 +432,7 @@ describe("tool-bridge", () => { const onToolUpdate = vi.fn(); const handler = wrapToolHandler(tool, undefined, onToolUpdate); - await handler({}); + await handler({}, mockExtra); expect(onToolUpdate).toHaveBeenCalledWith( expect.objectContaining({ @@ -323,7 +457,7 @@ describe("tool-bridge", () => { const handler = wrapToolHandler(tool, undefined, onToolUpdate); // Should not throw even if callback fails - const result = await handler({}); + const result = await handler({}, mockExtra); expect(result.content[0]).toMatchObject({ type: "text", @@ -344,8 +478,8 @@ describe("tool-bridge", () => { } as unknown as AnyAgentTool; const handler = wrapToolHandler(tool); - await handler({}); - await handler({}); + await handler({}, mockExtra); + await handler({}, mockExtra); expect(toolCallIds[0]).not.toBe(toolCallIds[1]); expect(toolCallIds[0]).toContain("mcp-bridge-unique_id_tool"); @@ -381,21 +515,21 @@ describe("tool-bridge", () => { describe("bridgeMoltbotToolsSync", () => { // Helper to create a class-like McpServer mock (required because implementation uses `new`) - function createMockMcpServerClass(toolFn: ReturnType) { + function createMockMcpServerClass(registerToolFn: ReturnType) { return class MockMcpServer { - tool = toolFn; + registerTool = registerToolFn; constructor(_opts: { name: string; version: string }) { // Constructor receives options } }; } - it("registers tools with MCP server", () => { + it("registers tools with MCP server using registerTool", () => { const registeredTools: string[] = []; - const toolFn = vi.fn((name: string) => { + const registerToolFn = vi.fn((name: string) => { registeredTools.push(name); }); - const MockMcpServer = createMockMcpServerClass(toolFn); + const MockMcpServer = createMockMcpServerClass(registerToolFn); const tools = [ { @@ -421,6 +555,39 @@ describe("tool-bridge", () => { expect(result.toolCount).toBe(2); expect(result.registeredTools).toEqual(["tool_one", "tool_two"]); expect(result.skippedTools).toEqual([]); + expect(registerToolFn).toHaveBeenCalledTimes(2); + }); + + it("passes config with inputSchema to registerTool", () => { + const registerToolFn = vi.fn(); + const MockMcpServer = createMockMcpServerClass(registerToolFn); + + const tools = [ + { + name: "schema_tool", + description: "Tool with schema", + parameters: { type: "object", properties: { query: { type: "string" } } }, + execute: vi.fn(), + }, + ] as unknown as AnyAgentTool[]; + + bridgeMoltbotToolsSync({ + name: "test-server", + tools, + McpServer: MockMcpServer as any, + }); + + expect(registerToolFn).toHaveBeenCalledWith( + "schema_tool", + expect.objectContaining({ + description: "Tool with schema", + inputSchema: expect.objectContaining({ + parse: expect.any(Function), + safeParse: expect.any(Function), + }), + }), + expect.any(Function), + ); }); it("skips tools with empty names", () => { @@ -458,10 +625,10 @@ describe("tool-bridge", () => { }); it("handles tool registration errors gracefully", () => { - const toolFn = vi.fn((name: string) => { + const registerToolFn = vi.fn((name: string) => { if (name === "bad_tool") throw new Error("Registration failed"); }); - const MockMcpServer = createMockMcpServerClass(toolFn); + const MockMcpServer = createMockMcpServerClass(registerToolFn); const tools = [ { name: "good_tool", execute: vi.fn() }, diff --git a/src/agents/claude-agent-sdk/tool-bridge.ts b/src/agents/claude-agent-sdk/tool-bridge.ts index 93ed4d81e..611f1d60d 100644 --- a/src/agents/claude-agent-sdk/tool-bridge.ts +++ b/src/agents/claude-agent-sdk/tool-bridge.ts @@ -18,6 +18,8 @@ */ import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import type { TSchema } from "@sinclair/typebox"; +import { Value } from "@sinclair/typebox/value"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { normalizeToolName } from "../tool-policy.js"; @@ -25,6 +27,7 @@ import type { AnyAgentTool } from "../tools/common.js"; import type { McpCallToolResult, McpContentBlock, + McpRequestHandlerExtra, McpSdkServerConfig, McpServerConstructor, McpServerLike, @@ -33,9 +36,85 @@ import type { const log = createSubsystemLogger("agents/claude-agent-sdk/tool-bridge"); // --------------------------------------------------------------------------- -// Schema conversion: TypeBox → JSON Schema (passthrough) +// Schema conversion: TypeBox → Zod-compatible schema // --------------------------------------------------------------------------- +/** + * Interface for Zod-compatible schema that the MCP SDK expects. + * The SDK checks for `parse` and `safeParse` methods to identify valid schemas. + */ +export interface ZodCompatibleSchema { + /** Synchronous parse - throws on validation failure */ + parse(data: unknown): unknown; + /** Safe synchronous parse - returns success/error result */ + safeParse(data: unknown): { success: true; data: unknown } | { success: false; error: unknown }; + /** Async version of safeParse (used by MCP SDK) */ + safeParseAsync( + data: unknown, + ): Promise<{ success: true; data: unknown } | { success: false; error: unknown }>; + /** Mark this as a Zod-like schema instance (SDK checks for _def or _zod) */ + _def: { typeName: string; description?: string }; +} + +/** + * Wrap a TypeBox schema to make it Zod-compatible for the MCP SDK. + * + * The MCP SDK's McpServer.tool() checks `isZodTypeLike()` which requires: + * - `parse` method (function) + * - `safeParse` method (function) + * + * This wrapper uses TypeBox's Value module to perform validation. + */ +export function createZodCompatibleSchema(typeboxSchema: TSchema): ZodCompatibleSchema { + return { + parse(data: unknown): unknown { + // Decode handles coercion (string→number, etc.) and throws on failure + return Value.Decode(typeboxSchema, data); + }, + + safeParse( + data: unknown, + ): { success: true; data: unknown } | { success: false; error: unknown } { + try { + // First check if valid + if (!Value.Check(typeboxSchema, data)) { + // Get the first validation error for a useful message + const errors = [...Value.Errors(typeboxSchema, data)]; + const firstError = errors[0]; + return { + success: false, + error: { + message: firstError?.message ?? "Validation failed", + path: firstError?.path ?? "", + issues: errors.map((e) => ({ message: e.message, path: e.path })), + }, + }; + } + // Decode to apply any transformations + const decoded = Value.Decode(typeboxSchema, data); + return { success: true, data: decoded }; + } catch (err) { + return { + success: false, + error: err instanceof Error ? { message: err.message } : { message: String(err) }, + }; + } + }, + + async safeParseAsync( + data: unknown, + ): Promise<{ success: true; data: unknown } | { success: false; error: unknown }> { + return this.safeParse(data); + }, + + // Mimic Zod v3's internal structure so isZodSchemaInstance returns true + _def: { + typeName: "ZodObject", + description: (typeboxSchema as { description?: string }).description, + }, + }; +} + /** * Extract a clean JSON Schema object from a TypeBox schema. * @@ -57,6 +136,20 @@ export function extractJsonSchema(tool: AnyAgentTool): Record { } } +/** + * Create a Zod-compatible schema from a tool's TypeBox parameters. + * Falls back to a permissive schema if the tool has no parameters. + */ +export function extractZodCompatibleSchema(tool: AnyAgentTool): ZodCompatibleSchema | undefined { + const schema = tool.parameters as TSchema | undefined; + if (!schema || typeof schema !== "object") { + // Return undefined to indicate no input schema + // The MCP SDK will then call handler(extra) instead of handler(args, extra) + return undefined; + } + return createZodCompatibleSchema(schema); +} + // --------------------------------------------------------------------------- // Result conversion: AgentToolResult → McpCallToolResult // --------------------------------------------------------------------------- @@ -132,7 +225,12 @@ export function convertToolResult(result: AgentToolResult): McpCallTool * * Differences bridged: * - Moltbot: `execute(toolCallId, params, signal?, onUpdate?)` - * - MCP: `handler(args) → Promise` + * - MCP: `handler(args, extra) → Promise` + * + * When using registerTool() with inputSchema, the MCP SDK calls + * the handler with (args, extra) where: + * - args: the validated tool input parameters + * - extra: RequestHandlerExtra with signal, sessionId, _meta, etc. * * Notable: MCP handlers have no native streaming update callback. * We pass the shared `abortSignal` (if provided) and create an `onUpdate` @@ -142,10 +240,13 @@ export function wrapToolHandler( tool: AnyAgentTool, abortSignal?: AbortSignal, onToolUpdate?: OnToolUpdateCallback, -): (args: Record) => Promise { +): (args: Record, extra: McpRequestHandlerExtra) => Promise { const normalizedName = normalizeToolName(tool.name); - return async (args: Record): Promise => { + return async ( + args: Record, + extra: McpRequestHandlerExtra, + ): Promise => { // Generate a synthetic toolCallId. The Claude Agent SDK doesn't expose its // internal tool call ID to MCP handlers, so we create a unique one for // internal tracking/logging. This is safe because Moltbot tools only use @@ -160,6 +261,9 @@ export function wrapToolHandler( argsPreview: JSON.stringify(args).slice(0, 500), }); + // Use the signal from extra if available, fall back to the shared abortSignal + const signal = extra?.signal ?? abortSignal; + // Create an onUpdate callback that forwards to the bridge callback. const onUpdate = onToolUpdate ? (update: unknown) => { @@ -172,7 +276,7 @@ export function wrapToolHandler( : undefined; try { - const result = await tool.execute(toolCallId, args, abortSignal, onUpdate); + const result = await tool.execute(toolCallId, args, signal, onUpdate); return convertToolResult(result); } catch (err) { // Propagate AbortError so the SDK runner can handle cancellation. @@ -309,10 +413,20 @@ export async function bridgeMoltbotToolsToMcpServer(options: BridgeOptions): Pro } try { - const jsonSchema = extractJsonSchema(tool); + const inputSchema = extractZodCompatibleSchema(tool); const handler = wrapToolHandler(tool, options.abortSignal, options.onToolUpdate); - server.tool(toolName, tool.description ?? `Moltbot tool: ${toolName}`, jsonSchema, handler); + // Use registerTool() which properly handles inputSchema. + // The MCP SDK will call handler(args, extra) when inputSchema is set, + // and validate the args against the schema before calling the handler. + server.registerTool( + toolName, + { + description: tool.description ?? `Moltbot tool: ${toolName}`, + inputSchema, + }, + handler, + ); registered.push(toolName); } catch (err) { @@ -364,9 +478,18 @@ export function bridgeMoltbotToolsSync( } try { - const jsonSchema = extractJsonSchema(tool); + const inputSchema = extractZodCompatibleSchema(tool); const handler = wrapToolHandler(tool, options.abortSignal, options.onToolUpdate); - server.tool(toolName, tool.description ?? `Moltbot tool: ${toolName}`, jsonSchema, handler); + + server.registerTool( + toolName, + { + description: tool.description ?? `Moltbot tool: ${toolName}`, + inputSchema, + }, + handler, + ); + registered.push(toolName); } catch (err) { const message = err instanceof Error ? err.message : String(err); diff --git a/src/agents/claude-agent-sdk/tool-bridge.types.ts b/src/agents/claude-agent-sdk/tool-bridge.types.ts index ee54f020e..92b4d1d93 100644 --- a/src/agents/claude-agent-sdk/tool-bridge.types.ts +++ b/src/agents/claude-agent-sdk/tool-bridge.types.ts @@ -103,20 +103,59 @@ export type SdkResultEvent = { }; // --------------------------------------------------------------------------- -// MCP Server tool registration (shape of McpServer.tool() from the SDK) +// MCP Server tool registration (shape of McpServer from the SDK) // --------------------------------------------------------------------------- +/** + * Extra context passed to tool handlers by the MCP SDK. + * Contains abort signal, session info, and other runtime context. + */ +export interface McpRequestHandlerExtra { + signal: AbortSignal; + sessionId?: string; + _meta?: Record; + requestInfo?: { method: string; params?: unknown }; + [key: string]: unknown; +} + +/** + * Tool handler signature when inputSchema is provided. + * The SDK calls handler(args, extra) where args is the validated input. + */ +export type McpToolHandler = ( + args: Record, + extra: McpRequestHandlerExtra, +) => Promise; + +/** + * Configuration object for registerTool(). + */ +export interface McpToolConfig { + title?: string; + description?: string; + /** Zod-compatible schema with parse/safeParse methods */ + inputSchema?: { + parse(data: unknown): unknown; + safeParse(data: unknown): { success: boolean; data?: unknown; error?: unknown }; + safeParseAsync?(data: unknown): Promise<{ success: boolean; data?: unknown; error?: unknown }>; + _def?: Record; + }; + outputSchema?: unknown; + annotations?: Record; + _meta?: Record; +} + /** * Minimal interface for the McpServer class from `@modelcontextprotocol/sdk`. - * We only use the `tool()` registration method and the constructor. + * We use registerTool() which properly handles Zod-compatible inputSchema. */ export interface McpServerLike { - tool( - name: string, - description: string, - inputSchema: Record, - handler: (args: Record) => Promise, - ): void; + /** + * Register a tool with explicit configuration. + * The inputSchema must be Zod-compatible (have parse/safeParse methods) + * for the MCP SDK to properly validate args and pass them to the handler. + */ + registerTool(name: string, config: McpToolConfig, handler: McpToolHandler): void; } /** diff --git a/src/agents/claude-agent-sdk/types.ts b/src/agents/claude-agent-sdk/types.ts index 53554cc05..82f10d0f9 100644 --- a/src/agents/claude-agent-sdk/types.ts +++ b/src/agents/claude-agent-sdk/types.ts @@ -148,8 +148,10 @@ export type SdkRunnerParams = { messageChannel?: string; /** Channel-specific hints. */ channelHints?: string; - /** Available Moltbot skills. */ + /** Available Moltbot skills (names only, for system prompt context). */ skills?: string[]; + /** Full formatted skills prompt from skillsSnapshot (preferred over skills). */ + skillsPrompt?: string; /** Sender identifier. */ senderId?: string | null; /** Sender display name. */ diff --git a/src/agents/cli-credentials.ts b/src/agents/cli-credentials.ts index d1e08f764..2573affd1 100644 --- a/src/agents/cli-credentials.ts +++ b/src/agents/cli-credentials.ts @@ -12,18 +12,6 @@ import { resolveUserPath } from "../utils.js"; const log = createSubsystemLogger("agents/auth-profiles"); const CLAUDE_CLI_CREDENTIALS_RELATIVE_PATH = ".claude/.credentials.json"; - -/** - * Mask a token for logging - shows length and first/last 4 chars. - * Example: "sk-ant-api03-abc...xyz" (length: 108) - */ -function maskToken(token: string | undefined): string { - if (!token) return "(empty)"; - if (token.length <= 12) return `${"*".repeat(token.length)} (length: ${token.length})`; - const first = token.slice(0, 4); - const last = token.slice(-4); - return `${first}...${"*".repeat(Math.min(8, token.length - 8))}...${last} (length: ${token.length})`; -} const CODEX_CLI_AUTH_FILENAME = "auth.json"; const QWEN_CLI_CREDENTIALS_RELATIVE_PATH = ".qwen/oauth_creds.json"; @@ -205,25 +193,15 @@ function readQwenCliCredentials(options?: { homeDir?: string }): QwenCliCredenti function readClaudeCliKeychainCredentials( execSyncImpl: ExecSyncFn = execSync, ): ClaudeCliCredential | null { - log.debug("[CCSDK-AUTH] Attempting to read credentials from macOS Keychain", { - service: CLAUDE_CLI_KEYCHAIN_SERVICE, - account: CLAUDE_CLI_KEYCHAIN_ACCOUNT, - }); - try { const result = execSyncImpl( `security find-generic-password -s "${CLAUDE_CLI_KEYCHAIN_SERVICE}" -w`, { encoding: "utf8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"] }, ); - log.debug("[CCSDK-AUTH] Keychain returned data", { - rawLength: result.length, - }); - const data = JSON.parse(result.trim()); const claudeOauth = data?.claudeAiOauth; if (!claudeOauth || typeof claudeOauth !== "object") { - log.debug("[CCSDK-AUTH] Keychain data missing claudeAiOauth object"); return null; } @@ -232,30 +210,12 @@ function readClaudeCliKeychainCredentials( const expiresAt = claudeOauth.expiresAt; if (typeof accessToken !== "string" || !accessToken) { - log.debug("[CCSDK-AUTH] Keychain accessToken invalid or missing"); return null; } if (typeof expiresAt !== "number" || expiresAt <= 0) { - log.debug("[CCSDK-AUTH] Keychain expiresAt invalid", { expiresAt }); return null; } - const expiresDate = new Date(expiresAt); - const isExpired = expiresAt < Date.now(); - const msUntilExpiry = expiresAt - Date.now(); - - log.debug("[CCSDK-AUTH] Keychain credentials parsed successfully", { - source: "keychain", - hasAccessToken: Boolean(accessToken), - accessTokenMasked: maskToken(accessToken), - hasRefreshToken: Boolean(refreshToken), - refreshTokenMasked: maskToken(refreshToken), - expiresAt: expiresDate.toISOString(), - isExpired, - msUntilExpiry, - type: refreshToken ? "oauth" : "token", - }); - if (typeof refreshToken === "string" && refreshToken) { return { type: "oauth", @@ -272,10 +232,7 @@ function readClaudeCliKeychainCredentials( token: accessToken, expires: expiresAt, }; - } catch (err) { - log.debug("[CCSDK-AUTH] Keychain read failed", { - error: err instanceof Error ? err.message : String(err), - }); + } catch { return null; } } @@ -291,10 +248,6 @@ function readClaudeCliKeychainCredentials( function readClaudeCliLinuxSecretServiceCredentials( execSyncImpl: ExecSyncFn = execSync, ): ClaudeCliCredential | null { - log.debug("[CCSDK-AUTH] Attempting to read credentials from Linux Secret Service", { - service: CLAUDE_CLI_KEYCHAIN_SERVICE, - }); - try { // secret-tool lookup returns the secret value for matching attributes const result = execSyncImpl(`secret-tool lookup service "${CLAUDE_CLI_KEYCHAIN_SERVICE}"`, { @@ -304,18 +257,12 @@ function readClaudeCliLinuxSecretServiceCredentials( }); if (!result || !result.trim()) { - log.debug("[CCSDK-AUTH] Linux Secret Service returned empty result"); return null; } - log.debug("[CCSDK-AUTH] Linux Secret Service returned data", { - rawLength: result.length, - }); - const data = JSON.parse(result.trim()); const claudeOauth = data?.claudeAiOauth; if (!claudeOauth || typeof claudeOauth !== "object") { - log.debug("[CCSDK-AUTH] Linux Secret Service data missing claudeAiOauth object"); return null; } @@ -324,30 +271,12 @@ function readClaudeCliLinuxSecretServiceCredentials( const expiresAt = claudeOauth.expiresAt; if (typeof accessToken !== "string" || !accessToken) { - log.debug("[CCSDK-AUTH] Linux Secret Service accessToken invalid or missing"); return null; } if (typeof expiresAt !== "number" || expiresAt <= 0) { - log.debug("[CCSDK-AUTH] Linux Secret Service expiresAt invalid", { expiresAt }); return null; } - const expiresDate = new Date(expiresAt); - const isExpired = expiresAt < Date.now(); - const msUntilExpiry = expiresAt - Date.now(); - - log.debug("[CCSDK-AUTH] Linux Secret Service credentials parsed successfully", { - source: "linux-secret-service", - hasAccessToken: Boolean(accessToken), - accessTokenMasked: maskToken(accessToken), - hasRefreshToken: Boolean(refreshToken), - refreshTokenMasked: maskToken(refreshToken), - expiresAt: expiresDate.toISOString(), - isExpired, - msUntilExpiry, - type: refreshToken ? "oauth" : "token", - }); - if (typeof refreshToken === "string" && refreshToken) { return { type: "oauth", @@ -364,10 +293,7 @@ function readClaudeCliLinuxSecretServiceCredentials( token: accessToken, expires: expiresAt, }; - } catch (err) { - log.debug("[CCSDK-AUTH] Linux Secret Service read failed", { - error: err instanceof Error ? err.message : String(err), - }); + } catch { return null; } } @@ -381,10 +307,6 @@ function readClaudeCliLinuxSecretServiceCredentials( function readClaudeCliWindowsCredentialManagerCredentials( execSyncImpl: ExecSyncFn = execSync, ): ClaudeCliCredential | null { - log.debug("[CCSDK-AUTH] Attempting to read credentials from Windows Credential Manager", { - target: CLAUDE_CLI_WINDOWS_TARGET, - }); - try { // PowerShell script to read from Windows Credential Manager // Uses the .NET CredentialManager API via Add-Type @@ -442,18 +364,12 @@ function readClaudeCliWindowsCredentialManagerCredentials( ); if (!result || !result.trim()) { - log.debug("[CCSDK-AUTH] Windows Credential Manager returned empty result"); return null; } - log.debug("[CCSDK-AUTH] Windows Credential Manager returned data", { - rawLength: result.length, - }); - const data = JSON.parse(result.trim()); const claudeOauth = data?.claudeAiOauth; if (!claudeOauth || typeof claudeOauth !== "object") { - log.debug("[CCSDK-AUTH] Windows Credential Manager data missing claudeAiOauth object"); return null; } @@ -462,30 +378,12 @@ function readClaudeCliWindowsCredentialManagerCredentials( const expiresAt = claudeOauth.expiresAt; if (typeof accessToken !== "string" || !accessToken) { - log.debug("[CCSDK-AUTH] Windows Credential Manager accessToken invalid or missing"); return null; } if (typeof expiresAt !== "number" || expiresAt <= 0) { - log.debug("[CCSDK-AUTH] Windows Credential Manager expiresAt invalid", { expiresAt }); return null; } - const expiresDate = new Date(expiresAt); - const isExpired = expiresAt < Date.now(); - const msUntilExpiry = expiresAt - Date.now(); - - log.debug("[CCSDK-AUTH] Windows Credential Manager credentials parsed successfully", { - source: "windows-credential-manager", - hasAccessToken: Boolean(accessToken), - accessTokenMasked: maskToken(accessToken), - hasRefreshToken: Boolean(refreshToken), - refreshTokenMasked: maskToken(refreshToken), - expiresAt: expiresDate.toISOString(), - isExpired, - msUntilExpiry, - type: refreshToken ? "oauth" : "token", - }); - if (typeof refreshToken === "string" && refreshToken) { return { type: "oauth", @@ -502,10 +400,7 @@ function readClaudeCliWindowsCredentialManagerCredentials( token: accessToken, expires: expiresAt, }; - } catch (err) { - log.debug("[CCSDK-AUTH] Windows Credential Manager read failed", { - error: err instanceof Error ? err.message : String(err), - }); + } catch { return null; } } @@ -518,70 +413,38 @@ export function readClaudeCliCredentials(options?: { }): ClaudeCliCredential | null { const platform = options?.platform ?? process.platform; - log.debug("[CCSDK-AUTH] readClaudeCliCredentials called", { - platform, - allowKeychainPrompt: options?.allowKeychainPrompt, - homeDir: options?.homeDir ?? "(default)", - }); - // Try platform-specific secure credential storage first if (options?.allowKeychainPrompt !== false) { let secureStoreCreds: ClaudeCliCredential | null = null; let source = ""; if (platform === "darwin") { - log.debug("[CCSDK-AUTH] Attempting macOS Keychain read"); secureStoreCreds = readClaudeCliKeychainCredentials(options?.execSync); source = "macos-keychain"; } else if (platform === "linux") { - log.debug("[CCSDK-AUTH] Attempting Linux Secret Service read"); secureStoreCreds = readClaudeCliLinuxSecretServiceCredentials(options?.execSync); source = "linux-secret-service"; } else if (platform === "win32") { - log.debug("[CCSDK-AUTH] Attempting Windows Credential Manager read"); secureStoreCreds = readClaudeCliWindowsCredentialManagerCredentials(options?.execSync); source = "windows-credential-manager"; } if (secureStoreCreds) { - log.info("[CCSDK-AUTH] Successfully read credentials from secure storage", { - type: secureStoreCreds.type, - source, - accessTokenMasked: - secureStoreCreds.type === "oauth" - ? maskToken(secureStoreCreds.access) - : maskToken(secureStoreCreds.token), - }); + log.debug("read claude cli credentials", { source, type: secureStoreCreds.type }); return secureStoreCreds; } - - if (source) { - log.debug("[CCSDK-AUTH] Secure storage read returned null, falling back to file", { source }); - } } // Fall back to file-based credentials const credPath = resolveClaudeCliCredentialsPath(options?.homeDir); - log.debug("[CCSDK-AUTH] Attempting to read credentials from file", { - path: credPath, - }); - const raw = loadJsonFile(credPath); if (!raw || typeof raw !== "object") { - log.debug("[CCSDK-AUTH] Credentials file not found or invalid", { - path: credPath, - rawType: typeof raw, - }); return null; } const data = raw as Record; const claudeOauth = data.claudeAiOauth as Record | undefined; if (!claudeOauth || typeof claudeOauth !== "object") { - log.debug("[CCSDK-AUTH] Credentials file missing claudeAiOauth", { - path: credPath, - topLevelKeys: Object.keys(data), - }); return null; } @@ -590,41 +453,21 @@ export function readClaudeCliCredentials(options?: { const expiresAt = claudeOauth.expiresAt; if (typeof accessToken !== "string" || !accessToken) { - log.debug("[CCSDK-AUTH] Credentials file accessToken invalid", { - path: credPath, - }); return null; } if (typeof expiresAt !== "number" || expiresAt <= 0) { - log.debug("[CCSDK-AUTH] Credentials file expiresAt invalid", { - path: credPath, - expiresAt, - }); return null; } - const expiresDate = new Date(expiresAt); - const isExpired = expiresAt < Date.now(); - const msUntilExpiry = expiresAt - Date.now(); + const credType = typeof refreshToken === "string" && refreshToken ? "oauth" : "token"; + log.debug("read claude cli credentials", { source: "file", type: credType }); - log.info("[CCSDK-AUTH] Successfully read credentials from file", { - source: "file", - path: credPath, - accessTokenMasked: maskToken(accessToken as string), - hasRefreshToken: Boolean(refreshToken), - refreshTokenMasked: maskToken(refreshToken as string | undefined), - expiresAt: expiresDate.toISOString(), - isExpired, - msUntilExpiry, - type: refreshToken ? "oauth" : "token", - }); - - if (typeof refreshToken === "string" && refreshToken) { + if (credType === "oauth") { return { type: "oauth", provider: "anthropic", access: accessToken, - refresh: refreshToken, + refresh: refreshToken as string, expires: expiresAt, }; } @@ -728,7 +571,6 @@ export function writeClaudeCliLinuxSecretServiceCredentials( ); if (!existingResult || !existingResult.trim()) { - log.debug("[CCSDK-AUTH] No existing Linux Secret Service entry to update"); return false; } @@ -826,7 +668,6 @@ export function writeClaudeCliWindowsCredentialManagerCredentials( ); if (!existingResult || !existingResult.trim()) { - log.debug("[CCSDK-AUTH] No existing Windows Credential Manager entry to update"); return false; } diff --git a/src/agents/main-agent-runtime-factory.ts b/src/agents/main-agent-runtime-factory.ts index 7600a6f57..a57f0edbb 100644 --- a/src/agents/main-agent-runtime-factory.ts +++ b/src/agents/main-agent-runtime-factory.ts @@ -55,8 +55,6 @@ export async function createAgentRuntime( ): Promise { const runtimeKind = forceKind ?? resolveAgentRuntimeKind(config, agentId); - log.debug("Creating agent runtime", { agentId, runtime: runtimeKind, forced: !!forceKind }); - if (runtimeKind === "ccsdk") { // Dynamically import to avoid loading SDK when not needed const { createCcSdkAgentRuntime, isSdkAvailable } = await import("./claude-agent-sdk/index.js"); diff --git a/src/agents/pi-agent-runtime.ts b/src/agents/pi-agent-runtime.ts index 91a0c1d66..756745bb6 100644 --- a/src/agents/pi-agent-runtime.ts +++ b/src/agents/pi-agent-runtime.ts @@ -6,11 +6,8 @@ */ import type { AgentRuntime, AgentRuntimeRunParams, AgentRuntimeResult } from "./agent-runtime.js"; -import { createSubsystemLogger } from "../logging/subsystem.js"; import { runEmbeddedPiAgent } from "./pi-embedded.js"; -const log = createSubsystemLogger("agents/pi-runtime"); - /** * Create a Pi Agent runtime instance. * @@ -23,13 +20,6 @@ export function createPiAgentRuntime(): AgentRuntime { displayName: "Pi Agent", async run(params: AgentRuntimeRunParams): Promise { - log.info("Starting Pi Agent session", { - sessionId: params.sessionId, - runId: params.runId, - provider: params.provider ?? "default", - model: params.model ?? "default", - }); - // Extract Pi-specific options from the options bag const piOpts = params.piOptions ?? {}; diff --git a/src/agents/unified-agent-runner.test.ts b/src/agents/unified-agent-runner.test.ts index b40a52b5c..1780eb6ac 100644 --- a/src/agents/unified-agent-runner.test.ts +++ b/src/agents/unified-agent-runner.test.ts @@ -3,17 +3,21 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import type { MoltbotConfig } from "../config/config.js"; import type { AgentRuntime, AgentRuntimeResult } from "./agent-runtime.js"; +// Mock run functions - defined separately to avoid unbound-method lint errors +const mockPiRun = vi.fn(); +const mockCcsdkRun = vi.fn(); + // Mock dependencies const mockPiRuntime: AgentRuntime = { kind: "pi", displayName: "Pi Agent", - run: vi.fn(), + run: mockPiRun, }; const mockCcsdkRuntime: AgentRuntime = { kind: "ccsdk", displayName: "Claude Code SDK", - run: vi.fn(), + run: mockCcsdkRun, }; vi.mock("./main-agent-runtime-factory.js", () => ({ @@ -114,8 +118,8 @@ describe("unified-agent-runner", () => { beforeEach(() => { vi.clearAllMocks(); vi.mocked(createAgentRuntime).mockResolvedValue(mockPiRuntime); - vi.mocked(mockPiRuntime.run).mockResolvedValue(successResult); - vi.mocked(mockCcsdkRuntime.run).mockResolvedValue(successResult); + mockPiRun.mockResolvedValue(successResult); + mockCcsdkRun.mockResolvedValue(successResult); }); afterEach(() => { @@ -130,7 +134,7 @@ describe("unified-agent-runner", () => { const result = await runAgentWithUnifiedFailover(baseParams); expect(result.runtime).toBe("pi"); - expect(mockPiRuntime.run).toHaveBeenCalled(); + expect(mockPiRun).toHaveBeenCalled(); }); it("uses ccsdk as primary when configured", async () => { @@ -143,7 +147,7 @@ describe("unified-agent-runner", () => { const result = await runAgentWithUnifiedFailover(baseParams); expect(result.runtime).toBe("ccsdk"); - expect(mockCcsdkRuntime.run).toHaveBeenCalled(); + expect(mockCcsdkRun).toHaveBeenCalled(); }); }); @@ -329,7 +333,7 @@ describe("unified-agent-runner", () => { const abortError = new Error("Aborted"); abortError.name = "AbortError"; - vi.mocked(mockPiRuntime.run).mockRejectedValue(abortError); + mockPiRun.mockRejectedValue(abortError); await expect(runAgentWithUnifiedFailover(baseParams)).rejects.toThrow("Aborted"); }); diff --git a/src/agents/unified-agent-runner.ts b/src/agents/unified-agent-runner.ts index 52730a19d..2bdd41041 100644 --- a/src/agents/unified-agent-runner.ts +++ b/src/agents/unified-agent-runner.ts @@ -38,7 +38,7 @@ import { } from "./auth-profiles.js"; import { isReasoningTagProvider } from "../utils/provider-utils.js"; -const log = createSubsystemLogger("agents/unified-runner"); +const log = createSubsystemLogger("agent-runner"); /** Model candidate for fallback. */ type ModelCandidate = { @@ -233,7 +233,7 @@ export async function runAgentWithUnifiedFailover( let attemptIndex = 0; const totalAttempts = runtimeChain.length * modelCandidates.length; - log.info("Starting unified agent run", { + log.info("Starting Agent run", { sessionId: params.sessionId, runId: params.runId, agentId, @@ -267,8 +267,6 @@ export async function runAgentWithUnifiedFailover( continue; } - log.debug("Trying runtime", { runtime: runtimeKind, displayName: runtime.displayName }); - // Model-inner loop for (const candidate of modelCandidates) { attemptIndex += 1; @@ -295,14 +293,6 @@ export async function runAgentWithUnifiedFailover( } try { - log.debug("Attempting model", { - runtime: runtimeKind, - provider: candidate.provider, - model: candidate.model, - attempt: attemptIndex, - total: totalAttempts, - }); - // Notify model selection before attempt (wrapped in try/catch to be resilient) try { await params.onModelSelected?.({ @@ -346,7 +336,7 @@ export async function runAgentWithUnifiedFailover( : { enforceFinalTag: effectiveEnforceFinalTag }, }); - log.info("Unified agent run completed", { + log.debug("Agent run completed", { sessionId: params.sessionId, runId: params.runId, runtime: runtimeKind, diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index a94db7310..629d1bdb1 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -5,10 +5,7 @@ import { runCliAgent } from "../../agents/cli-runner.js"; import { getCliSessionId } from "../../agents/cli-session.js"; import { isCliProvider } from "../../agents/model-selection.js"; import { runEmbeddedPiAgent } from "../../agents/pi-embedded.js"; -import { - runAgentWithUnifiedFailover, - type UnifiedAgentRunResult, -} from "../../agents/unified-agent-runner.js"; +import { runAgentWithUnifiedFailover } from "../../agents/unified-agent-runner.js"; import { isCompactionFailureError, isContextOverflowError, @@ -240,11 +237,7 @@ export async function runAgentTurnWithFallback(params: { : ("plain" as const); })(); - let unifiedResult: UnifiedAgentRunResult; - logVerbose( - `[CCSDK-EXEC] Calling runAgentWithUnifiedFailover. hasOnBlockReply=${Boolean(params.opts?.onBlockReply)}, blockStreamingEnabled=${params.blockStreamingEnabled}, hasBlockReplyPipeline=${Boolean(params.blockReplyPipeline)}, hasOnReasoningStream=${Boolean(params.opts?.onReasoningStream)}, shouldStartOnReasoning=${params.typingSignals.shouldStartOnReasoning}`, - ); - unifiedResult = await runAgentWithUnifiedFailover({ + const unifiedResult = await runAgentWithUnifiedFailover({ // Core params sessionId: params.followupRun.run.sessionId, sessionKey: params.sessionKey,