Merge branch 'pr-1553' into integration/validation-batch
# Conflicts: # src/services/worker/session/SessionCompletionHandler.ts
This commit is contained in:
@@ -889,6 +889,16 @@ export class SessionStore {
|
|||||||
`).run(memorySessionId, sessionDbId);
|
`).run(memorySessionId, sessionDbId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
markSessionCompleted(sessionDbId: number): void {
|
||||||
|
const nowEpoch = Date.now();
|
||||||
|
const nowIso = new Date(nowEpoch).toISOString();
|
||||||
|
this.db.prepare(`
|
||||||
|
UPDATE sdk_sessions
|
||||||
|
SET status = 'completed', completed_at = ?, completed_at_epoch = ?
|
||||||
|
WHERE id = ?
|
||||||
|
`).run(nowIso, nowEpoch, sessionDbId);
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Ensures memory_session_id is registered in sdk_sessions before FK-constrained INSERT.
|
* Ensures memory_session_id is registered in sdk_sessions before FK-constrained INSERT.
|
||||||
* This fixes Issue #846 where observations fail after worker restart because the
|
* This fixes Issue #846 where observations fail after worker restart because the
|
||||||
|
|||||||
@@ -40,7 +40,8 @@ export class SessionRoutes extends BaseRouteHandler {
|
|||||||
super();
|
super();
|
||||||
this.completionHandler = new SessionCompletionHandler(
|
this.completionHandler = new SessionCompletionHandler(
|
||||||
sessionManager,
|
sessionManager,
|
||||||
eventBroadcaster
|
eventBroadcaster,
|
||||||
|
dbManager.getSessionStore()
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -11,12 +11,14 @@
|
|||||||
|
|
||||||
import { SessionManager } from '../SessionManager.js';
|
import { SessionManager } from '../SessionManager.js';
|
||||||
import { SessionEventBroadcaster } from '../events/SessionEventBroadcaster.js';
|
import { SessionEventBroadcaster } from '../events/SessionEventBroadcaster.js';
|
||||||
|
import { SessionStore } from '../../sqlite/SessionStore.js';
|
||||||
import { logger } from '../../../utils/logger.js';
|
import { logger } from '../../../utils/logger.js';
|
||||||
|
|
||||||
export class SessionCompletionHandler {
|
export class SessionCompletionHandler {
|
||||||
constructor(
|
constructor(
|
||||||
private sessionManager: SessionManager,
|
private sessionManager: SessionManager,
|
||||||
private eventBroadcaster: SessionEventBroadcaster
|
private eventBroadcaster: SessionEventBroadcaster,
|
||||||
|
private sessionStore: SessionStore
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -24,6 +26,9 @@ export class SessionCompletionHandler {
|
|||||||
* Used by DELETE /api/sessions/:id and POST /api/sessions/:id/complete
|
* Used by DELETE /api/sessions/:id and POST /api/sessions/:id/complete
|
||||||
*/
|
*/
|
||||||
async completeByDbId(sessionDbId: number): Promise<void> {
|
async completeByDbId(sessionDbId: number): Promise<void> {
|
||||||
|
// Persist completion to database before in-memory cleanup (fix for #1532)
|
||||||
|
this.sessionStore.markSessionCompleted(sessionDbId);
|
||||||
|
|
||||||
// Delete from session manager (aborts SDK agent via SIGTERM)
|
// Delete from session manager (aborts SDK agent via SIGTERM)
|
||||||
await this.sessionManager.deleteSession(sessionDbId);
|
await this.sessionManager.deleteSession(sessionDbId);
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,66 @@
|
|||||||
|
/**
|
||||||
|
* Tests for SessionStore.markSessionCompleted (fix for #1532)
|
||||||
|
*
|
||||||
|
* Mock Justification: NONE (0% mock code)
|
||||||
|
* - Uses real SQLite with ':memory:' - tests actual SQL and schema
|
||||||
|
*/
|
||||||
|
import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
|
||||||
|
import { SessionStore } from '../../../src/services/sqlite/SessionStore.js';
|
||||||
|
|
||||||
|
describe('SessionStore.markSessionCompleted', () => {
|
||||||
|
let store: SessionStore;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
store = new SessionStore(':memory:');
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
store.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('sets status to completed and records completed_at timestamps', () => {
|
||||||
|
const before = Date.now();
|
||||||
|
const id = store.createSDKSession('session-1', 'project', 'prompt');
|
||||||
|
|
||||||
|
store.markSessionCompleted(id);
|
||||||
|
|
||||||
|
const row = store.db.prepare(
|
||||||
|
'SELECT status, completed_at, completed_at_epoch FROM sdk_sessions WHERE id = ?'
|
||||||
|
).get(id) as { status: string; completed_at: string; completed_at_epoch: number };
|
||||||
|
|
||||||
|
expect(row.status).toBe('completed');
|
||||||
|
expect(row.completed_at).toBeTruthy();
|
||||||
|
expect(row.completed_at_epoch).toBeGreaterThanOrEqual(before);
|
||||||
|
expect(row.completed_at_epoch).toBeLessThanOrEqual(Date.now());
|
||||||
|
});
|
||||||
|
|
||||||
|
it('leaves other sessions unaffected', () => {
|
||||||
|
const id1 = store.createSDKSession('session-a', 'project', 'prompt');
|
||||||
|
const id2 = store.createSDKSession('session-b', 'project', 'prompt');
|
||||||
|
|
||||||
|
store.markSessionCompleted(id1);
|
||||||
|
|
||||||
|
const row2 = store.db.prepare(
|
||||||
|
'SELECT status, completed_at FROM sdk_sessions WHERE id = ?'
|
||||||
|
).get(id2) as { status: string; completed_at: string | null };
|
||||||
|
|
||||||
|
expect(row2.status).toBe('active');
|
||||||
|
expect(row2.completed_at).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not throw when called on a non-existent session id', () => {
|
||||||
|
expect(() => store.markSessionCompleted(99999)).not.toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('completed_at is a valid ISO timestamp', () => {
|
||||||
|
const id = store.createSDKSession('session-iso', 'project', 'prompt');
|
||||||
|
store.markSessionCompleted(id);
|
||||||
|
|
||||||
|
const row = store.db.prepare(
|
||||||
|
'SELECT completed_at FROM sdk_sessions WHERE id = ?'
|
||||||
|
).get(id) as { completed_at: string };
|
||||||
|
|
||||||
|
expect(() => new Date(row.completed_at).toISOString()).not.toThrow();
|
||||||
|
expect(new Date(row.completed_at).getTime()).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user