Merge pull request #1686 from ousamabenyounes/fix/issue-1633

fix: expose summaryStored in session status to detect silent summary loss (#1633)
This commit is contained in:
Alex Newman
2026-04-14 18:41:58 -07:00
committed by GitHub
5 changed files with 70 additions and 5 deletions
+17 -5
View File
@@ -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 <summary> tags (#1633)', {
sessionId,
waitedMs: Date.now() - waitStart
});
break;
}
break;
}
} catch {
// Worker may be busy — keep polling
+3
View File
@@ -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 {
@@ -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();
@@ -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
});
});
@@ -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 = `
<summary>
<request>user asked to fix bug</request>
<investigated>looked at auth module</investigated>
<learned>JWT tokens were expiring</learned>
<completed>fixed expiry check</completed>
<next_steps>write tests</next_steps>
</summary>
`;
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 <summary> tags,
// storeObservations skips summary and returns summaryId=null.
mockStoreObservations.mockImplementation(() => ({
observationIds: [],
summaryId: null,
createdAtEpoch: 1700000000000,
} as StorageResult));
const session = createMockSession();
// Response with no <summary> block — LLM failed to produce structured output
const responseText = '<skip_summary/>';
await processAgentResponse(responseText, session, mockDbManager, mockSessionManager, mockWorker, 0, null, 'TestAgent');
expect(session.lastSummaryStored).toBe(false);
});
});
});