From 2f19eab9c29179e944d6c6efc20f299d12c97314 Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Fri, 10 Apr 2026 15:06:18 +0000 Subject: [PATCH 1/2] fix: expose summaryStored in session status to detect silent summary loss (#1633) Stop hook polled queueLength===0 as a proxy for summary success, but the queue empties regardless of whether the LLM produced valid tags. Added lastSummaryStored tracking on ActiveSession, surfaced via the /api/sessions/status endpoint, and emit a logger.warn in the Stop hook when summaryStored===false. Generated by Claude Code Vibe coded by ousamabenyounes Co-Authored-By: Claude --- src/cli/handlers/summarize.ts | 15 ++++++- src/services/worker-types.ts | 3 ++ .../worker/agents/ResponseProcessor.ts | 4 ++ .../worker/http/routes/SessionRoutes.ts | 3 ++ .../worker/agents/response-processor.test.ts | 43 +++++++++++++++++++ 5 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/cli/handlers/summarize.ts b/src/cli/handlers/summarize.ts index 55cc6ab1..c178060b 100644 --- a/src/cli/handlers/summarize.ts +++ b/src/cli/handlers/summarize.ts @@ -87,6 +87,7 @@ export const summarizeHandler: EventHandler = { // This keeps the Stop hook alive (120s timeout) so the SDK agent // can finish processing the summary before SessionEnd kills the session. const waitStart = Date.now(); + let summaryStored: boolean | null = null; while ((Date.now() - waitStart) < MAX_WAIT_FOR_SUMMARY_MS) { await new Promise(resolve => setTimeout(resolve, POLL_INTERVAL_MS)); try { @@ -94,11 +95,21 @@ export const summarizeHandler: EventHandler = { timeoutMs: 5000 }); if (statusResponse.ok) { - const status = await statusResponse.json() as { queueLength?: number }; + const status = await statusResponse.json() as { queueLength?: number; summaryStored?: boolean | null }; if ((status.queueLength ?? 0) === 0) { + summaryStored = status.summaryStored ?? null; logger.info('HOOK', 'Summary processing complete', { - waitedMs: Date.now() - waitStart + waitedMs: Date.now() - waitStart, + summaryStored }); + // Warn when the agent processed a summarize request but produced no storable summary. + // This is the silent-failure path described in #1633: queue empties but no summary record exists. + if (summaryStored === false) { + logger.warn('HOOK', 'Summary was not stored: LLM response likely lacked valid tags (#1633)', { + sessionId, + waitedMs: Date.now() - waitStart + }); + } break; } } diff --git a/src/services/worker-types.ts b/src/services/worker-types.ts index dad3444b..1a8557a0 100644 --- a/src/services/worker-types.ts +++ b/src/services/worker-types.ts @@ -43,6 +43,9 @@ export interface ActiveSession { processingMessageIds: number[]; // Tier routing: model override per session based on queue complexity modelOverride?: string; + // Track whether the most recent storage operation persisted a summary record. + // Used by the status endpoint so the Stop hook can detect silent summary loss (#1633). + lastSummaryStored?: boolean; } export interface PendingMessage { diff --git a/src/services/worker/agents/ResponseProcessor.ts b/src/services/worker/agents/ResponseProcessor.ts index 3487764f..06705f52 100644 --- a/src/services/worker/agents/ResponseProcessor.ts +++ b/src/services/worker/agents/ResponseProcessor.ts @@ -126,6 +126,10 @@ export async function processAgentResponse( memorySessionId: session.memorySessionId }); + // Track whether a summary record was stored so the status endpoint can expose this + // to the Stop hook for silent-summary-loss detection (#1633) + session.lastSummaryStored = result.summaryId !== null; + // CLAIM-CONFIRM: Now that storage succeeded, confirm all processing messages (delete from queue) // This is the critical step that prevents message loss on generator crash const pendingStore = sessionManager.getPendingMessageStore(); diff --git a/src/services/worker/http/routes/SessionRoutes.ts b/src/services/worker/http/routes/SessionRoutes.ts index 010e5c35..13828933 100644 --- a/src/services/worker/http/routes/SessionRoutes.ts +++ b/src/services/worker/http/routes/SessionRoutes.ts @@ -672,6 +672,9 @@ export class SessionRoutes extends BaseRouteHandler { status: 'active', sessionDbId, queueLength, + // Expose whether the last storage operation included a summary record. + // The Stop hook uses this to detect silent summary loss when the queue empties (#1633). + summaryStored: session.lastSummaryStored ?? null, uptime: Date.now() - session.startTime }); }); diff --git a/tests/worker/agents/response-processor.test.ts b/tests/worker/agents/response-processor.test.ts index 1638bfdb..bd7b30cb 100644 --- a/tests/worker/agents/response-processor.test.ts +++ b/tests/worker/agents/response-processor.test.ts @@ -683,4 +683,47 @@ describe('ResponseProcessor', () => { ).rejects.toThrow('Cannot store observations: memorySessionId not yet captured'); }); }); + + describe('lastSummaryStored tracking (#1633)', () => { + it('should set lastSummaryStored=true when storage returns a summaryId', async () => { + mockStoreObservations.mockImplementation(() => ({ + observationIds: [], + summaryId: 42, + createdAtEpoch: 1700000000000, + } as StorageResult)); + + const session = createMockSession(); + const responseText = ` + + user asked to fix bug + looked at auth module + JWT tokens were expiring + fixed expiry check + write tests + + `; + + await processAgentResponse(responseText, session, mockDbManager, mockSessionManager, mockWorker, 0, null, 'TestAgent'); + + expect(session.lastSummaryStored).toBe(true); + }); + + it('should set lastSummaryStored=false when storage returns summaryId=null (silent loss path, #1633)', async () => { + // Simulate the silent failure: agent returns no parseable tags, + // storeObservations skips summary and returns summaryId=null. + mockStoreObservations.mockImplementation(() => ({ + observationIds: [], + summaryId: null, + createdAtEpoch: 1700000000000, + } as StorageResult)); + + const session = createMockSession(); + // Response with no block — LLM failed to produce structured output + const responseText = ''; + + await processAgentResponse(responseText, session, mockDbManager, mockSessionManager, mockWorker, 0, null, 'TestAgent'); + + expect(session.lastSummaryStored).toBe(false); + }); + }); }); From edc8535ac11dcd30848b46206dfa73a793e04c43 Mon Sep 17 00:00:00 2001 From: Ousama Ben Younes Date: Sat, 11 Apr 2026 08:16:35 +0000 Subject: [PATCH 2/2] fix: skip queueLength===0 completion branch when session returns 404 --- src/cli/handlers/summarize.ts | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/cli/handlers/summarize.ts b/src/cli/handlers/summarize.ts index c178060b..25a1f227 100644 --- a/src/cli/handlers/summarize.ts +++ b/src/cli/handlers/summarize.ts @@ -94,24 +94,25 @@ export const summarizeHandler: EventHandler = { const statusResponse = await workerHttpRequest(`/api/sessions/status?contentSessionId=${encodeURIComponent(sessionId)}`, { timeoutMs: 5000 }); - if (statusResponse.ok) { - const status = await statusResponse.json() as { queueLength?: number; summaryStored?: boolean | null }; - if ((status.queueLength ?? 0) === 0) { - summaryStored = status.summaryStored ?? null; - logger.info('HOOK', 'Summary processing complete', { - waitedMs: Date.now() - waitStart, - summaryStored + const status = await statusResponse.json() as { queueLength?: number; summaryStored?: boolean | null }; + const queueLength = status.queueLength ?? 0; + // Only treat an empty queue as completion when the session exists (non-404). + // A 404 means the session was not found — not that processing finished. + if (queueLength === 0 && statusResponse.status !== 404) { + summaryStored = status.summaryStored ?? null; + logger.info('HOOK', 'Summary processing complete', { + waitedMs: Date.now() - waitStart, + summaryStored + }); + // Warn when the agent processed a summarize request but produced no storable summary. + // This is the silent-failure path described in #1633: queue empties but no summary record exists. + if (summaryStored === false) { + logger.warn('HOOK', 'Summary was not stored: LLM response likely lacked valid tags (#1633)', { + sessionId, + waitedMs: Date.now() - waitStart }); - // Warn when the agent processed a summarize request but produced no storable summary. - // This is the silent-failure path described in #1633: queue empties but no summary record exists. - if (summaryStored === false) { - logger.warn('HOOK', 'Summary was not stored: LLM response likely lacked valid tags (#1633)', { - sessionId, - waitedMs: Date.now() - waitStart - }); - } - break; } + break; } } catch { // Worker may be busy — keep polling