From 1d55f96e01a476accbf078feed716516e6705bfc Mon Sep 17 00:00:00 2001 From: Hernan Javier Ardila Sanchez Date: Fri, 29 May 2026 06:44:22 +0200 Subject: [PATCH] fix(combo): preserve system messages during context handoff summary generation (#2865) Integrated into release/v3.8.6. --- open-sse/services/contextHandoff.ts | 28 ++++++++-- tests/unit/context-handoff.test.ts | 52 +++++++++++++++++++ tests/unit/dockerignore-docs-coverage.test.ts | 2 +- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/open-sse/services/contextHandoff.ts b/open-sse/services/contextHandoff.ts index e592764561..d65a550cdb 100644 --- a/open-sse/services/contextHandoff.ts +++ b/open-sse/services/contextHandoff.ts @@ -230,16 +230,36 @@ function formatMessagesForPrompt(messages: MessageLike[]): string { .join("\n\n"); } -function selectMessagesForSummary(messages: MessageLike[], maxMessages: number): MessageLike[] { - const recentMessages = messages.slice(-maxMessages); +export function selectMessagesForSummary(messages: MessageLike[], maxMessages: number): MessageLike[] { + const validMessages = messages.filter((m) => m && typeof m === "object"); + const system = validMessages.filter( + (m) => typeof m.role === "string" && (m.role === "system" || m.role === "developer") + ); + const nonSystem = validMessages.filter( + (m) => typeof m.role !== "string" || (m.role !== "system" && m.role !== "developer") + ); + + const recentMessages = [...system, ...nonSystem.slice(-maxMessages)]; let working = [...recentMessages]; - while (working.length > 1) { + while (working.length > system.length + 1) { const history = formatMessagesForPrompt(working); if (estimateTokens(history) <= MAX_HISTORY_TOKENS_FOR_SUMMARY) { return working; } - working = working.slice(1); + working = [...system, ...working.slice(system.length + 1)]; + } + + const fallbackHistory = formatMessagesForPrompt(working); + if (estimateTokens(fallbackHistory) > MAX_HISTORY_TOKENS_FOR_SUMMARY) { + // If there are system messages, return them so the caller can still produce context. + // If there are no system messages (system=[]), fall back to the single most-recent + // non-system message rather than returning [] which would silently drop the handoff. + if (system.length > 0) { + return system; + } + const lastNonSystem = nonSystem[nonSystem.length - 1]; + return lastNonSystem ? [lastNonSystem] : []; } return working; diff --git a/tests/unit/context-handoff.test.ts b/tests/unit/context-handoff.test.ts index 99e79a88a4..86eeb23024 100644 --- a/tests/unit/context-handoff.test.ts +++ b/tests/unit/context-handoff.test.ts @@ -368,3 +368,55 @@ test("context handoff DB module upserts and deletes active handoffs", () => { handoffDb.deleteHandoff("sess-db", "relay-combo"); assert.equal(handoffDb.getHandoff("sess-db", "relay-combo"), null); }); + +test("selectMessagesForSummary filters falsy values and preserves system/developer messages", () => { + const messages: (contextHandoff.MessageLike | null | undefined | false)[] = [ + null, + undefined, + { role: "system", content: "System 1" }, + false, + { role: "user", content: "User 1" }, + { role: "developer", content: "Dev 1" }, + { role: "assistant", content: "Assistant 1" }, + { role: "user", content: "User 2" }, + ]; + + const selected = contextHandoff.selectMessagesForSummary( + messages as contextHandoff.MessageLike[], + 2 + ); + + assert.equal(selected.length, 4); + assert.equal(selected[0].role, "system"); + assert.equal(selected[1].role, "developer"); + assert.equal(selected[2].role, "assistant"); + assert.equal(selected[3].role, "user"); + assert.equal(selected[3].content, "User 2"); +}); + +test("selectMessagesForSummary with no system messages and oversized single remaining message still produces non-empty selection", () => { + // Build a single very large non-system message that exceeds MAX_HISTORY_TOKENS_FOR_SUMMARY + // (token estimator is ~4 chars/token, so 8000 tokens ≈ 32000 chars). + const hugeContent = "x".repeat(40000); + const messages: contextHandoff.MessageLike[] = [ + { role: "user", content: "first message" }, + { role: "assistant", content: hugeContent }, + ]; + + const selected = contextHandoff.selectMessagesForSummary(messages, 10); + + // The function must return at least one message rather than [] so the handoff is not silently dropped. + assert.ok(selected.length > 0, "expected at least one message to be selected"); + + // formatMessagesForPrompt on the result must produce a non-empty string + // (guards the regression: previously returned [] → empty historyText → handoff skipped). + const historyText = selected + .map((m, i) => { + const role = typeof m.role === "string" ? m.role : "unknown"; + const content = typeof m.content === "string" ? m.content.trim() : ""; + return content ? `[${i + 1}] ${role.toUpperCase()}:\n${content}` : ""; + }) + .filter(Boolean) + .join("\n\n"); + assert.ok(historyText.length > 0, "historyText must be non-empty so the handoff is generated"); +}); diff --git a/tests/unit/dockerignore-docs-coverage.test.ts b/tests/unit/dockerignore-docs-coverage.test.ts index 7934d27f1a..c3fb6176ea 100644 --- a/tests/unit/dockerignore-docs-coverage.test.ts +++ b/tests/unit/dockerignore-docs-coverage.test.ts @@ -154,7 +154,7 @@ test("#2348 .dockerignore keeps every doc the in-product viewer needs", () => { test("#2348 .dockerignore still excludes the heavy i18n tree", () => { const parsed = parseDockerignore(fs.readFileSync(DOCKERIGNORE, "utf8")); - const heavy = "docs/i18n/pt-BR/docs/AUTO-COMBO.md"; + const heavy = "docs/i18n/pt-BR/docs/routing/AUTO-COMBO.md"; assert.ok( isIgnored(heavy, parsed), `${heavy} should be excluded from Docker context but is not — image size will balloon`