fix: finalize sessions on normal exit; cleanup hoist; share handler (iter 5)

1. startSessionProcessor success branch now calls completionHandler.
   finalizeSession before removeSessionImmediate. Hooks-disabled installs
   (and any Stop hook that fails before POST /api/sessions/complete) no
   longer leave sdk_sessions rows as status='active' forever. Idempotent
   — a subsequent /api/sessions/complete is a no-op.

2. Hoist SessionRoutes.handleSessionEnd cleanup declaration above the
   closures that reference it (TDZ safety; safe at runtime today but
   fragile if timeout ever shrinks).

3. SessionRoutes now receives WorkerService's shared SessionCompletionHandler
   instead of constructing its own — prevents silent divergence if the
   handler ever becomes stateful.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
2026-04-24 15:03:16 -07:00
parent a3d9c39cc0
commit 02201e64af
3 changed files with 100 additions and 99 deletions

File diff suppressed because one or more lines are too long

View File

@@ -319,7 +319,7 @@ export class WorkerService implements WorkerRef {
// Standard routes (registered AFTER guard middleware)
this.server.registerRoutes(new ViewerRoutes(this.sseBroadcaster, this.dbManager, this.sessionManager));
const sessionRoutes = new SessionRoutes(this.sessionManager, this.dbManager, this.sdkAgent, this.geminiAgent, this.openRouterAgent, this.sessionEventBroadcaster, this);
const sessionRoutes = new SessionRoutes(this.sessionManager, this.dbManager, this.sdkAgent, this.geminiAgent, this.openRouterAgent, this.sessionEventBroadcaster, this, this.completionHandler);
this.server.registerRoutes(sessionRoutes);
// Wire the generator-starter callback now that SessionRoutes exists.
// `setIngestContext` ran in the constructor before routes were
@@ -842,10 +842,16 @@ export class WorkerService implements WorkerRef {
this.startSessionProcessor(session, 'pending-work-restart');
this.broadcastProcessingStatus();
} else {
// Successful completion with no pending work — clean up session
// removeSessionImmediate fires onSessionDeletedCallback → broadcastProcessingStatus()
// Successful completion with no pending work — finalize then drop
// in-memory state. finalizeSession flips sdk_sessions.status to
// 'completed', drains orphaned pendings, broadcasts; idempotent so
// the later POST /api/sessions/complete from the Stop hook is a
// no-op. Without this, hooks-disabled installs (and any session
// whose Stop hook fails before /api/sessions/complete) leave the
// DB row permanently 'active'.
session.restartGuard?.recordSuccess();
session.consecutiveRestarts = 0;
this.completionHandler.finalizeSession(session.sessionDbId);
this.sessionManager.removeSessionImmediate(session.sessionDbId);
}
});

View File

@@ -30,7 +30,6 @@ import { normalizePlatformSource } from '../../../../shared/platform-source.js';
import { RestartGuard } from '../../RestartGuard.js';
export class SessionRoutes extends BaseRouteHandler {
private completionHandler: SessionCompletionHandler;
private spawnInProgress = new Map<number, boolean>();
private crashRecoveryScheduled = new Set<number>();
@@ -41,14 +40,10 @@ export class SessionRoutes extends BaseRouteHandler {
private geminiAgent: GeminiAgent,
private openRouterAgent: OpenRouterAgent,
private eventBroadcaster: SessionEventBroadcaster,
private workerService: WorkerService
private workerService: WorkerService,
private completionHandler: SessionCompletionHandler,
) {
super();
this.completionHandler = new SessionCompletionHandler(
sessionManager,
eventBroadcaster,
dbManager
);
}
/**
@@ -521,6 +516,7 @@ export class SessionRoutes extends BaseRouteHandler {
}
let settled = false;
let timer: ReturnType<typeof setTimeout> | undefined;
const onStored = (evt: SummaryStoredEvent): void => {
if (evt.sessionId !== sessionId) return;
if (settled) return;
@@ -528,19 +524,18 @@ export class SessionRoutes extends BaseRouteHandler {
cleanup();
res.status(200).json({ ok: true, messageId: evt.messageId });
};
const cleanup = (): void => {
if (timer) clearTimeout(timer);
ingestEventBus.off('summaryStoredEvent', onStored);
};
const timer = setTimeout(() => {
timer = setTimeout(() => {
if (settled) return;
settled = true;
cleanup();
res.status(504).json({ ok: false, reason: 'summary_not_stored_in_time' });
}, SessionRoutes.SERVER_SIDE_SUMMARY_TIMEOUT_MS);
const cleanup = (): void => {
clearTimeout(timer);
ingestEventBus.off('summaryStoredEvent', onStored);
};
ingestEventBus.on('summaryStoredEvent', onStored);
// Client aborted (hook process died) — drop the listener immediately so