diff --git a/CHANGELOG.md b/CHANGELOG.md index 0755d0af5..8598d8a90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ Docs: https://docs.openclaw.ai - Security/CI: add pre-commit security hook coverage for private-key detection and production dependency auditing, and enforce those checks in CI alongside baseline secret scanning. Thanks @vincentkoc. - Skills/Python: harden skill script packaging and validation edge cases (self-including `.skill` outputs, CRLF frontmatter parsing, strict `--days` validation, and safer image file loading), with expanded Python regression coverage. Thanks @vincentkoc. - Config/Write: apply `unsetPaths` with immutable path-copy updates so config writes never mutate caller-provided objects, and harden `openclaw config get/set/unset` path traversal by rejecting prototype-key segments and inherited-property traversal. (#24134) thanks @frankekn. +- Agents/Compaction: pass model metadata through the embedded runtime so safeguard summarization can run when `ctx.model` is unavailable, avoiding repeated `"Summary unavailable due to context limits"` fallback summaries. (#3479) Thanks @battman21, @hanxiao and @vincentkoc. - Agents/Overflow: detect additional provider context-overflow error shapes (including `input length` + `max_tokens` exceed-context variants) so failures route through compaction/recovery paths instead of leaking raw provider errors to users. (#9951) Thanks @echoVic and @Glucksberg. - Agents/Failover: treat HTTP 502/503/504 errors as failover-eligible transient timeouts so fallback chains can switch providers/models during upstream outages instead of retrying the same failing target. (#20999) Thanks @taw0002 and @vincentkoc. diff --git a/src/agents/pi-embedded-runner/extensions.ts b/src/agents/pi-embedded-runner/extensions.ts index cdaa47b09..fc0e76acd 100644 --- a/src/agents/pi-embedded-runner/extensions.ts +++ b/src/agents/pi-embedded-runner/extensions.ts @@ -81,6 +81,7 @@ export function buildEmbeddedExtensionFactories(params: { setCompactionSafeguardRuntime(params.sessionManager, { maxHistoryShare: compactionCfg?.maxHistoryShare, contextWindowTokens: contextWindowInfo.tokens, + model: params.model, }); factories.push(compactionSafeguardExtension); } diff --git a/src/agents/pi-extensions/compaction-safeguard-runtime.ts b/src/agents/pi-extensions/compaction-safeguard-runtime.ts index df3919cf8..7391e3c1c 100644 --- a/src/agents/pi-extensions/compaction-safeguard-runtime.ts +++ b/src/agents/pi-extensions/compaction-safeguard-runtime.ts @@ -1,8 +1,15 @@ +import type { Api, Model } from "@mariozechner/pi-ai"; import { createSessionManagerRuntimeRegistry } from "./session-manager-runtime-registry.js"; export type CompactionSafeguardRuntimeValue = { maxHistoryShare?: number; contextWindowTokens?: number; + /** + * Model to use for compaction summarization. + * Passed through runtime because `ctx.model` is undefined in the compact.ts workflow + * (extensionRunner.initialize() is never called in that path). + */ + model?: Model; }; const registry = createSessionManagerRuntimeRegistry(); diff --git a/src/agents/pi-extensions/compaction-safeguard.test.ts b/src/agents/pi-extensions/compaction-safeguard.test.ts index 3d5fab422..b5e4915d0 100644 --- a/src/agents/pi-extensions/compaction-safeguard.test.ts +++ b/src/agents/pi-extensions/compaction-safeguard.test.ts @@ -1,10 +1,12 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core"; -import { describe, expect, it } from "vitest"; +import type { Api, Model } from "@mariozechner/pi-ai"; +import type { ExtensionAPI, ExtensionContext } from "@mariozechner/pi-coding-agent"; +import { describe, expect, it, vi } from "vitest"; import { getCompactionSafeguardRuntime, setCompactionSafeguardRuntime, } from "./compaction-safeguard-runtime.js"; -import { __testing } from "./compaction-safeguard.js"; +import compactionSafeguardExtension, { __testing } from "./compaction-safeguard.js"; const { collectToolFailures, @@ -16,6 +18,25 @@ const { SAFETY_MARGIN, } = __testing; +function stubSessionManager(): ExtensionContext["sessionManager"] { + const stub: ExtensionContext["sessionManager"] = { + getCwd: () => "/stub", + getSessionDir: () => "/stub", + getSessionId: () => "stub-id", + getSessionFile: () => undefined, + getLeafId: () => null, + getLeafEntry: () => undefined, + getEntry: () => undefined, + getLabel: () => undefined, + getBranch: () => [], + getHeader: () => null, + getEntries: () => [], + getTree: () => [], + getSessionName: () => undefined, + }; + return stub; +} + describe("compaction-safeguard tool failures", () => { it("formats tool failures with meta and summary", () => { const messages: AgentMessage[] = [ @@ -248,4 +269,212 @@ describe("compaction-safeguard runtime registry", () => { expect(getCompactionSafeguardRuntime(sm1)).toEqual({ maxHistoryShare: 0.3 }); expect(getCompactionSafeguardRuntime(sm2)).toEqual({ maxHistoryShare: 0.8 }); }); + + it("stores and retrieves model from runtime (fallback for compact.ts workflow)", () => { + const sm = {}; + const model: Model = { + id: "claude-opus-4-5", + name: "Claude Opus 4.5", + provider: "anthropic", + api: "anthropic" as const, + baseUrl: "https://api.anthropic.com", + contextWindow: 200000, + maxTokens: 4096, + reasoning: false, + input: ["text"] as const, + cost: { input: 15, output: 75, cacheRead: 0, cacheWrite: 0 }, + }; + setCompactionSafeguardRuntime(sm, { model }); + const retrieved = getCompactionSafeguardRuntime(sm); + expect(retrieved?.model).toEqual(model); + }); + + it("stores and retrieves contextWindowTokens from runtime", () => { + const sm = {}; + setCompactionSafeguardRuntime(sm, { contextWindowTokens: 200000 }); + const retrieved = getCompactionSafeguardRuntime(sm); + expect(retrieved?.contextWindowTokens).toBe(200000); + }); + + it("stores and retrieves combined runtime values", () => { + const sm = {}; + const model: Model = { + id: "claude-opus-4-5", + name: "Claude Opus 4.5", + provider: "anthropic", + api: "anthropic" as const, + baseUrl: "https://api.anthropic.com", + contextWindow: 200000, + maxTokens: 4096, + reasoning: false, + input: ["text"] as const, + cost: { input: 15, output: 75, cacheRead: 0, cacheWrite: 0 }, + }; + setCompactionSafeguardRuntime(sm, { + maxHistoryShare: 0.6, + contextWindowTokens: 200000, + model, + }); + const retrieved = getCompactionSafeguardRuntime(sm); + expect(retrieved).toEqual({ + maxHistoryShare: 0.6, + contextWindowTokens: 200000, + model, + }); + }); +}); + +describe("compaction-safeguard extension model fallback", () => { + it("uses runtime.model when ctx.model is undefined (compact.ts workflow)", async () => { + // This test verifies the root-cause fix: when extensionRunner.initialize() is not called + // (as happens in compact.ts), ctx.model is undefined but runtime.model is available. + const sessionManager = stubSessionManager(); + const model: Model = { + id: "claude-opus-4-5", + name: "Claude Opus 4.5", + provider: "anthropic", + api: "anthropic" as const, + baseUrl: "https://api.anthropic.com", + contextWindow: 200000, + maxTokens: 4096, + reasoning: false, + input: ["text"] as const, + cost: { input: 15, output: 75, cacheRead: 0, cacheWrite: 0 }, + }; + + // Set up runtime with model (mimics buildEmbeddedExtensionPaths behavior) + setCompactionSafeguardRuntime(sessionManager, { model }); + + type CompactionHandler = (event: unknown, ctx: unknown) => Promise; + let compactionHandler: CompactionHandler | undefined; + + // Create a minimal mock ExtensionAPI that captures the handler + const mockApi = { + on: vi.fn((event: string, handler: CompactionHandler) => { + if (event === "session_before_compact") { + compactionHandler = handler; + } + }), + } as unknown as ExtensionAPI; + + // Register the extension + compactionSafeguardExtension(mockApi); + + // Verify handler was registered + expect(compactionHandler).toBeDefined(); + + // Now trigger the handler with mock data + const mockEvent = { + preparation: { + messagesToSummarize: [ + { role: "user", content: "test message", timestamp: Date.now() }, + ] as AgentMessage[], + turnPrefixMessages: [] as AgentMessage[], + firstKeptEntryId: "entry-1", + tokensBefore: 1000, + fileOps: { + read: [], + edited: [], + written: [], + }, + }, + customInstructions: "", + signal: new AbortController().signal, + }; + + const getApiKeyMock = vi.fn().mockResolvedValue(null); + // oxlint-disable-next-line typescript/no-explicit-any + const mockContext = { + model: undefined, // ctx.model is undefined (simulates compact.ts workflow) + sessionManager, + modelRegistry: { + getApiKey: getApiKeyMock, // No API key, should use fallback + }, + } as unknown as Partial; + + // Call the handler and wait for result + // oxlint-disable-next-line typescript/no-non-null-assertion + const result = (await compactionHandler!(mockEvent, mockContext)) as { + compaction?: { summary?: string; firstKeptEntryId?: string }; + }; + const compactionResult = result?.compaction; + + // Verify that compaction returned a result (even with null API key, should use fallback) + expect(compactionResult).toBeDefined(); + expect(compactionResult?.summary).toBeDefined(); + expect(compactionResult?.summary).toContain("Summary unavailable"); + expect(compactionResult?.firstKeptEntryId).toBe("entry-1"); + + // KEY ASSERTION: Prove the fallback path was exercised + // The handler should have called getApiKey with runtime.model (via ctx.model ?? runtime?.model) + expect(getApiKeyMock).toHaveBeenCalledWith(model); + + // Verify runtime.model is still available (for completeness) + const retrieved = getCompactionSafeguardRuntime(sessionManager); + expect(retrieved?.model).toEqual(model); + }); + + it("returns fallback summary when both ctx.model and runtime.model are undefined", async () => { + const sessionManager = stubSessionManager(); + + // Do NOT set runtime.model (both ctx.model and runtime.model will be undefined) + + type CompactionHandler = (event: unknown, ctx: unknown) => Promise; + let compactionHandler: CompactionHandler | undefined; + + const mockApi = { + on: vi.fn((event: string, handler: CompactionHandler) => { + if (event === "session_before_compact") { + compactionHandler = handler; + } + }), + } as unknown as ExtensionAPI; + + compactionSafeguardExtension(mockApi); + + expect(compactionHandler).toBeDefined(); + + const mockEvent = { + preparation: { + messagesToSummarize: [ + { role: "user", content: "test", timestamp: Date.now() }, + ] as AgentMessage[], + turnPrefixMessages: [] as AgentMessage[], + firstKeptEntryId: "entry-1", + tokensBefore: 500, + fileOps: { + read: [], + edited: [], + written: [], + }, + }, + customInstructions: "", + signal: new AbortController().signal, + }; + + const getApiKeyMock = vi.fn().mockResolvedValue(null); + // oxlint-disable-next-line typescript/no-explicit-any + const mockContext = { + model: undefined, // ctx.model is undefined + sessionManager, + modelRegistry: { + getApiKey: getApiKeyMock, // Should NOT be called (early return) + }, + } as unknown as Partial; + + // oxlint-disable-next-line typescript/no-non-null-assertion + const result = (await compactionHandler!(mockEvent, mockContext)) as { + compaction?: { summary?: string; firstKeptEntryId?: string }; + }; + const compactionResult = result?.compaction; + + expect(compactionResult).toBeDefined(); + expect(compactionResult?.summary).toBe( + "Summary unavailable due to context limits. Older messages were truncated.", + ); + expect(compactionResult?.firstKeptEntryId).toBe("entry-1"); + + // Verify early return: getApiKey should NOT have been called when both models are missing + expect(getApiKeyMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/agents/pi-extensions/compaction-safeguard.ts b/src/agents/pi-extensions/compaction-safeguard.ts index 6406c3d8a..a728a4a72 100644 --- a/src/agents/pi-extensions/compaction-safeguard.ts +++ b/src/agents/pi-extensions/compaction-safeguard.ts @@ -22,6 +22,9 @@ import { getCompactionSafeguardRuntime } from "./compaction-safeguard-runtime.js const log = createSubsystemLogger("compaction-safeguard"); const FALLBACK_SUMMARY = "Summary unavailable due to context limits. Older messages were truncated."; + +// Track session managers that have already logged the missing-model warning to avoid log spam. +const missedModelWarningSessions = new WeakSet(); const TURN_PREFIX_INSTRUCTIONS = "This summary covers the prefix of a split turn. Focus on the original request," + " early progress, and any details needed to understand the retained suffix."; @@ -199,8 +202,21 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void { const toolFailureSection = formatToolFailuresSection(toolFailures); const fallbackSummary = `${FALLBACK_SUMMARY}${toolFailureSection}${fileOpsSummary}`; - const model = ctx.model; + // Model resolution: ctx.model is undefined in compact.ts workflow (extensionRunner.initialize() is never called). + // Fall back to runtime.model which is explicitly passed when building extension paths. + const runtime = getCompactionSafeguardRuntime(ctx.sessionManager); + const model = ctx.model ?? runtime?.model; if (!model) { + // Log warning once per session when both models are missing (diagnostic for future issues). + // Use a WeakSet to track which session managers have already logged the warning. + if (!ctx.model && !runtime?.model && !missedModelWarningSessions.has(ctx.sessionManager)) { + missedModelWarningSessions.add(ctx.sessionManager); + console.warn( + "[compaction-safeguard] Both ctx.model and runtime.model are undefined. " + + "Compaction summarization will not run. This indicates extensionRunner.initialize() " + + "was not called and model was not passed through runtime registry.", + ); + } return { compaction: { summary: fallbackSummary, @@ -224,7 +240,6 @@ export default function compactionSafeguardExtension(api: ExtensionAPI): void { } try { - const runtime = getCompactionSafeguardRuntime(ctx.sessionManager); const modelContextWindow = resolveContextWindowTokens(model); const contextWindowTokens = runtime?.contextWindowTokens ?? modelContextWindow; const turnPrefixMessages = preparation.turnPrefixMessages ?? [];