Skip to content

Commit 473dfc2

Browse files
tanshanshansteipete
tanshanshan
and
steipete
authored
fix(compaction): add circuit breaker to stop token burn when summarizer unavailable (#86900)
* fix(compaction): stop repeated staged fallbacks Co-authored-by: tanshanshan <[email protected]> * fix(compaction): preserve degraded summary state * refactor(compaction): keep result state internal * fix(compaction): cancel incomplete circuit-open runs * chore: refresh PR checks --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent e228a3c commit 473dfc2

4 files changed

Lines changed: 281 additions & 87 deletions

File tree

‎src/agents/agent-hooks/compaction-safeguard.test.ts‎

Lines changed: 123 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ vi.mock("../compaction.js", async () => {
3434

3535
const mockSummarizeInStages = vi.mocked(compactionModule.summarizeInStages);
3636

37+
function summaryResult(text: string) {
38+
return { kind: "summary" as const, text };
39+
}
40+
3741
const {
3842
collectToolFailures,
3943
formatToolFailuresSection,
@@ -1442,7 +1446,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
14421446

14431447
it("uses structured instructions when summarizing dropped history chunks", async () => {
14441448
mockSummarizeInStages.mockReset();
1445-
mockSummarizeInStages.mockResolvedValue("mock summary");
1449+
mockSummarizeInStages.mockResolvedValue(summaryResult("mock summary"));
14461450

14471451
const sessionManager = stubSessionManager();
14481452
const model = createAnthropicModelFixture();
@@ -1499,7 +1503,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
14991503

15001504
it("caps summarization reserve tokens to the model output limit", async () => {
15011505
mockSummarizeInStages.mockReset();
1502-
mockSummarizeInStages.mockResolvedValue("mock summary");
1506+
mockSummarizeInStages.mockResolvedValue(summaryResult("mock summary"));
15031507

15041508
const sessionManager = stubSessionManager();
15051509
const model = createAnthropicModelFixture({
@@ -1538,7 +1542,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
15381542

15391543
it("adds Copilot IDE headers to built-in compaction summarization", async () => {
15401544
mockSummarizeInStages.mockReset();
1541-
mockSummarizeInStages.mockResolvedValue("mock summary");
1545+
mockSummarizeInStages.mockResolvedValue(summaryResult("mock summary"));
15421546

15431547
const sessionManager = stubSessionManager();
15441548
const model = createAnthropicModelFixture({
@@ -1584,7 +1588,7 @@ describe("compaction-safeguard recent-turn preservation", () => {
15841588

15851589
it("does not retry summaries unless quality guard is explicitly enabled", async () => {
15861590
mockSummarizeInStages.mockReset();
1587-
mockSummarizeInStages.mockResolvedValue("summary missing headings");
1591+
mockSummarizeInStages.mockResolvedValue(summaryResult("summary missing headings"));
15881592

15891593
const sessionManager = stubSessionManager();
15901594
const model = createAnthropicModelFixture();
@@ -1633,20 +1637,22 @@ describe("compaction-safeguard recent-turn preservation", () => {
16331637
it("retries when generated summary misses headings even if preserved turns contain them", async () => {
16341638
mockSummarizeInStages.mockReset();
16351639
mockSummarizeInStages
1636-
.mockResolvedValueOnce("latest ask status")
1640+
.mockResolvedValueOnce(summaryResult("latest ask status"))
16371641
.mockResolvedValueOnce(
1638-
[
1639-
"## Decisions",
1640-
"Keep current flow.",
1641-
"## Open TODOs",
1642-
"None.",
1643-
"## Constraints/Rules",
1644-
"Follow rules.",
1645-
"## Pending user asks",
1646-
"latest ask status",
1647-
"## Exact identifiers",
1648-
"None.",
1649-
].join("\n"),
1642+
summaryResult(
1643+
[
1644+
"## Decisions",
1645+
"Keep current flow.",
1646+
"## Open TODOs",
1647+
"None.",
1648+
"## Constraints/Rules",
1649+
"Follow rules.",
1650+
"## Pending user asks",
1651+
"latest ask status",
1652+
"## Exact identifiers",
1653+
"None.",
1654+
].join("\n"),
1655+
),
16501656
);
16511657

16521658
const sessionManager = stubSessionManager();
@@ -1733,32 +1739,36 @@ describe("compaction-safeguard recent-turn preservation", () => {
17331739
mockSummarizeInStages.mockReset();
17341740
mockSummarizeInStages
17351741
.mockResolvedValueOnce(
1736-
[
1737-
"## Decisions",
1738-
"Keep current flow.",
1739-
"## Open TODOs",
1740-
"None.",
1741-
"## Constraints/Rules",
1742-
"Follow rules.",
1743-
"## Pending user asks",
1744-
"latest ask status",
1745-
"## Exact identifiers",
1746-
"None.",
1747-
].join("\n"),
1742+
summaryResult(
1743+
[
1744+
"## Decisions",
1745+
"Keep current flow.",
1746+
"## Open TODOs",
1747+
"None.",
1748+
"## Constraints/Rules",
1749+
"Follow rules.",
1750+
"## Pending user asks",
1751+
"latest ask status",
1752+
"## Exact identifiers",
1753+
"None.",
1754+
].join("\n"),
1755+
),
17481756
)
17491757
.mockResolvedValueOnce(
1750-
[
1751-
"## Decisions",
1752-
"Keep current flow.",
1753-
"## Open TODOs",
1754-
"None.",
1755-
"## Constraints/Rules",
1756-
"Follow rules.",
1757-
"## Pending user asks",
1758-
"older context",
1759-
"## Exact identifiers",
1760-
"None.",
1761-
].join("\n"),
1758+
summaryResult(
1759+
[
1760+
"## Decisions",
1761+
"Keep current flow.",
1762+
"## Open TODOs",
1763+
"None.",
1764+
"## Constraints/Rules",
1765+
"Follow rules.",
1766+
"## Pending user asks",
1767+
"older context",
1768+
"## Exact identifiers",
1769+
"None.",
1770+
].join("\n"),
1771+
),
17621772
);
17631773

17641774
const sessionManager = stubSessionManager();
@@ -1822,8 +1832,8 @@ describe("compaction-safeguard recent-turn preservation", () => {
18221832
const oversizedHistorySummary = "history detail ".repeat(MAX_COMPACTION_SUMMARY_CHARS);
18231833
const splitTurnPrefixSummary = "split-turn prefix context that must survive capping";
18241834
mockSummarizeInStages
1825-
.mockResolvedValueOnce(oversizedHistorySummary)
1826-
.mockResolvedValueOnce(splitTurnPrefixSummary)
1835+
.mockResolvedValueOnce(summaryResult(oversizedHistorySummary))
1836+
.mockResolvedValueOnce(summaryResult(splitTurnPrefixSummary))
18271837
.mockRejectedValueOnce(new Error("retry transient failure"));
18281838

18291839
const sessionManager = stubSessionManager();
@@ -1949,18 +1959,20 @@ describe("compaction-safeguard recent-turn preservation", () => {
19491959
it("re-distills prior summaries on the LLM path instead of preserving them verbatim", async () => {
19501960
mockSummarizeInStages.mockReset();
19511961
mockSummarizeInStages.mockResolvedValue(
1952-
[
1953-
"## Decisions",
1954-
"Condensed prior context with latest status.",
1955-
"## Open TODOs",
1956-
"None.",
1957-
"## Constraints/Rules",
1958-
"Preserve identifiers.",
1959-
"## Pending user asks",
1960-
"latest ask status",
1961-
"## Exact identifiers",
1962-
"None.",
1963-
].join("\n"),
1962+
summaryResult(
1963+
[
1964+
"## Decisions",
1965+
"Condensed prior context with latest status.",
1966+
"## Open TODOs",
1967+
"None.",
1968+
"## Constraints/Rules",
1969+
"Preserve identifiers.",
1970+
"## Pending user asks",
1971+
"latest ask status",
1972+
"## Exact identifiers",
1973+
"None.",
1974+
].join("\n"),
1975+
),
19641976
);
19651977

19661978
const sessionManager = stubSessionManager();
@@ -2011,14 +2023,62 @@ describe("compaction-safeguard recent-turn preservation", () => {
20112023
expect(JSON.stringify(messages[0])).toContain("Old duplicated section");
20122024
});
20132025

2026+
it("preserves the prior summary when staged summarization returns a generic fallback", async () => {
2027+
mockSummarizeInStages.mockReset();
2028+
mockSummarizeInStages.mockResolvedValue({
2029+
kind: "generic-fallback",
2030+
text: "Context contained 4 messages. Summary unavailable due to size limits.",
2031+
});
2032+
2033+
const sessionManager = stubSessionManager();
2034+
const model = createAnthropicModelFixture();
2035+
setCompactionSafeguardRuntime(sessionManager, {
2036+
model,
2037+
recentTurnsPreserve: 0,
2038+
});
2039+
2040+
const compactionHandler = createCompactionHandler();
2041+
const mockContext = createCompactionContext({
2042+
sessionManager,
2043+
getApiKeyMock: vi.fn().mockResolvedValue("test-key"),
2044+
});
2045+
const event = {
2046+
preparation: {
2047+
messagesToSummarize: [{ role: "user", content: "latest ask status", timestamp: 1 }],
2048+
turnPrefixMessages: [],
2049+
firstKeptEntryId: "entry-1",
2050+
tokensBefore: 1_500,
2051+
fileOps: {
2052+
read: [],
2053+
edited: [],
2054+
written: [],
2055+
},
2056+
settings: { reserveTokens: 4_000 },
2057+
previousSummary: "## Goal\nKnown context that must survive the outage.",
2058+
isSplitTurn: false,
2059+
},
2060+
customInstructions: "",
2061+
signal: new AbortController().signal,
2062+
};
2063+
2064+
const result = (await compactionHandler(event, mockContext)) as {
2065+
cancel?: boolean;
2066+
compaction?: { summary?: string };
2067+
};
2068+
2069+
expect(result.cancel).not.toBe(true);
2070+
expect(result.compaction?.summary).toContain("Known context that must survive the outage.");
2071+
expect(result.compaction?.summary).toContain("Summary unavailable due to size limits.");
2072+
});
2073+
20142074
it("falls back to LLM when provider throws a provider-side AbortError with signal not aborted", async () => {
20152075
// Reproduce the undici AbortError("This operation was aborted") shape that
20162076
// arrives when the compaction provider's HTTP connection drops mid-stream while
20172077
// the caller has NOT yet fired their abort signal. Before the fix,
20182078
// isAbortError() matched this shape so tryProviderSummarize rethrew and the
20192079
// extension runner swallowed the error — the LLM fallback path was skipped.
20202080
mockSummarizeInStages.mockReset();
2021-
mockSummarizeInStages.mockResolvedValue("llm fallback summary");
2081+
mockSummarizeInStages.mockResolvedValue(summaryResult("llm fallback summary"));
20222082

20232083
const providerAbortErr = Object.assign(new Error("This operation was aborted"), {
20242084
name: "AbortError",
@@ -2225,7 +2285,7 @@ describe("compaction-safeguard extension model fallback", () => {
22252285
// neither apiKey nor headers. `ok: true` must be trusted so compaction runs
22262286
// instead of wedging every message with a false "no credentials" cancel.
22272287
mockSummarizeInStages.mockReset();
2228-
mockSummarizeInStages.mockResolvedValue("mock summary");
2288+
mockSummarizeInStages.mockResolvedValue(summaryResult("mock summary"));
22292289

22302290
const sessionManager = stubSessionManager();
22312291
const model = createAnthropicModelFixture({ provider: "amazon-bedrock" });
@@ -2450,7 +2510,7 @@ describe("compaction-safeguard double-compaction guard", () => {
24502510

24512511
it("falls back to visible custom session branch entries before writing an empty boundary", async () => {
24522512
mockSummarizeInStages.mockReset();
2453-
mockSummarizeInStages.mockResolvedValue("branch summary");
2513+
mockSummarizeInStages.mockResolvedValue(summaryResult("branch summary"));
24542514

24552515
const now = Date.now();
24562516
const sessionManager = {
@@ -2539,7 +2599,7 @@ describe("compaction-safeguard double-compaction guard", () => {
25392599

25402600
it("does not replay inter-session sessions_send branch turns as fallback history", async () => {
25412601
mockSummarizeInStages.mockReset();
2542-
mockSummarizeInStages.mockResolvedValue("branch summary");
2602+
mockSummarizeInStages.mockResolvedValue(summaryResult("branch summary"));
25432603

25442604
const now = Date.now();
25452605
const sessionManager = {
@@ -2607,7 +2667,7 @@ describe("compaction-safeguard double-compaction guard", () => {
26072667
{ toolName: "functions.sessions_send", expectedRoles: ["user", "assistant"] },
26082668
])("preserves unfinished inter-session work after a $toolName result", async (scenario) => {
26092669
mockSummarizeInStages.mockReset();
2610-
mockSummarizeInStages.mockResolvedValue("unfinished branch summary");
2670+
mockSummarizeInStages.mockResolvedValue(summaryResult("unfinished branch summary"));
26112671

26122672
const now = Date.now();
26132673
const sessionManager = {
@@ -2694,7 +2754,7 @@ describe("compaction-safeguard double-compaction guard", () => {
26942754

26952755
it("keeps source-session sends as inert status history", async () => {
26962756
mockSummarizeInStages.mockReset();
2697-
mockSummarizeInStages.mockResolvedValue("completed send summary");
2757+
mockSummarizeInStages.mockResolvedValue(summaryResult("completed send summary"));
26982758

26992759
const now = Date.now();
27002760
const sessionManager = {
@@ -2810,7 +2870,7 @@ describe("compaction-safeguard double-compaction guard", () => {
28102870

28112871
it("preserves completed historical inter-session turns outside the active tail", async () => {
28122872
mockSummarizeInStages.mockReset();
2813-
mockSummarizeInStages.mockResolvedValue("historical branch summary");
2873+
mockSummarizeInStages.mockResolvedValue(summaryResult("historical branch summary"));
28142874

28152875
const now = Date.now();
28162876
const sessionManager = {
@@ -2902,7 +2962,7 @@ describe("compaction-safeguard double-compaction guard", () => {
29022962

29032963
it("recovers user and assistant branch turns when compaction preparation has only tool output", async () => {
29042964
mockSummarizeInStages.mockReset();
2905-
mockSummarizeInStages.mockResolvedValue("branch summary with visible turns");
2965+
mockSummarizeInStages.mockResolvedValue(summaryResult("branch summary with visible turns"));
29062966

29072967
const now = Date.now();
29082968
const sessionManager = {

‎src/agents/agent-hooks/compaction-safeguard.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ async function summarizeViaLLM(params: {
397397
messages: params.messages,
398398
previousSummary: params.previousSummary,
399399
});
400-
return compactionSafeguardDeps.summarizeInStages({
400+
const result = await compactionSafeguardDeps.summarizeInStages({
401401
messages,
402402
model: params.model,
403403
apiKey: params.apiKey,
@@ -410,6 +410,14 @@ async function summarizeViaLLM(params: {
410410
summarizationInstructions: params.summarizationInstructions,
411411
previousSummary: undefined,
412412
});
413+
if (result.kind === "summary") {
414+
return result.text;
415+
}
416+
417+
// A generic fallback means redistillation never happened. Preserve the
418+
// known summary verbatim so a temporary model outage cannot erase it.
419+
const previousSummary = params.previousSummary?.trim();
420+
return previousSummary ? `${previousSummary}\n\n${result.text}` : result.text;
413421
}
414422

415423
/**

0 commit comments

Comments
 (0)