diff --git a/src/cli/handlers/summarize.ts b/src/cli/handlers/summarize.ts index 55cc6ab1..25a1f227 100644 --- a/src/cli/handlers/summarize.ts +++ b/src/cli/handlers/summarize.ts @@ -87,20 +87,32 @@ 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 { const statusResponse = await workerHttpRequest(`/api/sessions/status?contentSessionId=${encodeURIComponent(sessionId)}`, { timeoutMs: 5000 }); - if (statusResponse.ok) { - const status = await statusResponse.json() as { queueLength?: number }; - if ((status.queueLength ?? 0) === 0) { - logger.info('HOOK', 'Summary processing complete', { + 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 }); - break; } + break; } } catch { // Worker may be busy — keep polling 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 74bd87ef..1f807302 100644 --- a/src/services/worker/agents/ResponseProcessor.ts +++ b/src/services/worker/agents/ResponseProcessor.ts @@ -147,6 +147,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); + }); + }); });