fix(compaction): pass model through runtime for safeguard summaries (#17864)
* fix(compaction): pass model through runtime to fix ctx.model undefined Fixes #3479 Root cause: extensionRunner.initialize() is never called in compact.ts workflow, leaving ctx.model undefined. Compaction safeguard checks ctx.model and returns fallback summary immediately without attempting LLM summarization. Changes: 1. Pass model through compaction safeguard runtime registry (same pattern as maxHistoryShare) 2. Fall back to runtime.model when ctx.model is undefined 3. Add once-per-session warning when both models are missing (prevents log spam) 4. Add regression test for runtime.model fallback This follows the established runtime registry pattern rather than attempting to call extensionRunner.initialize() (which is SDK-internal and not meant for direct access). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test: add comprehensive tests for compaction-safeguard model fallback Add integration tests to verify the model fallback behavior: - Test runtime.model fallback when ctx.model is undefined (compact.ts workflow) - Test fallback summary when both ctx.model and runtime.model are undefined - Test contextWindowTokens runtime storage/retrieval - Test combined runtime values (maxHistoryShare + contextWindowTokens + model) These tests verify the fix for issue #3479 where compaction fails due to ctx.model being undefined in the compact.ts workflow. The runtime registry pattern allows model to be passed when extensionRunner.initialize() is not called, ensuring summarization works in all code paths. Related: PR #17864 * fix(test): adapt compaction-safeguard tests to upstream type changes - Add baseUrl to Model mock objects (now required by Model<Api>) - Add explicit Model<Api> annotation to prevent provider string widening - Cast modelRegistry mock through unknown (ModelRegistry expanded) - Use non-null assertion for compactionHandler (TypeScript strict) - Type compaction result explicitly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Compaction: add changelog credit for model fallback fix * Update CHANGELOG.md * Update CHANGELOG.md --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Vincent Koc <vincentkoc@ieee.org>
This commit is contained in:
@@ -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.
|
||||
|
||||
|
||||
@@ -81,6 +81,7 @@ export function buildEmbeddedExtensionFactories(params: {
|
||||
setCompactionSafeguardRuntime(params.sessionManager, {
|
||||
maxHistoryShare: compactionCfg?.maxHistoryShare,
|
||||
contextWindowTokens: contextWindowInfo.tokens,
|
||||
model: params.model,
|
||||
});
|
||||
factories.push(compactionSafeguardExtension);
|
||||
}
|
||||
|
||||
@@ -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<Api>;
|
||||
};
|
||||
|
||||
const registry = createSessionManagerRuntimeRegistry<CompactionSafeguardRuntimeValue>();
|
||||
|
||||
@@ -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<Api> = {
|
||||
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<Api> = {
|
||||
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<Api> = {
|
||||
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<unknown>;
|
||||
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<ExtensionContext>;
|
||||
|
||||
// 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<unknown>;
|
||||
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<ExtensionContext>;
|
||||
|
||||
// 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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<object>();
|
||||
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 ?? [];
|
||||
|
||||
Reference in New Issue
Block a user