Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 115 additions & 0 deletions packages/protocol/__tests__/event-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -527,6 +527,121 @@ describe('EventStore', () => {
});
});

describe('session state machine', () => {
const sid = 'state-test-session';

beforeEach(() => {
store.upsertSession({ sessionId: sid });
});

it('sets and gets session state', () => {
store.setSessionState(sid, 'CREATED', { clientId: 'c1' });
expect(store.getSessionState(sid)).toBe('CREATED');
});

it('returns null for unknown session', () => {
expect(store.getSessionState('nonexistent')).toBeNull();
});

it('includes state in session metadata', () => {
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1' });
const meta = store.getSession(sid);
expect(meta?.state).toBe('ACTIVE');
expect(meta?.lastStateChange).toBeGreaterThan(0);
});

it('defaults to ENDED for existing sessions after migration', () => {
// New sessions get DEFAULT 'ENDED' from migration
const meta = store.getSession(sid);
expect(meta?.state).toBe('ENDED');
});

it('allows valid CREATED → STARTING transition', () => {
store.setSessionState(sid, 'CREATED', { clientId: 'c1', force: true });
store.setSessionState(sid, 'STARTING', { clientId: 'c1' });
expect(store.getSessionState(sid)).toBe('STARTING');
});

it('allows valid ACTIVE → DETACHED transition', () => {
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', force: true });
store.setSessionState(sid, 'DETACHED', { clientId: 'c1' });
expect(store.getSessionState(sid)).toBe('DETACHED');
});

it('allows ENDED → CREATED for resume', () => {
store.setSessionState(sid, 'ENDED', { clientId: 'c1', force: true });
store.setSessionState(sid, 'CREATED', { clientId: 'c1' });
expect(store.getSessionState(sid)).toBe('CREATED');
});

it('allows CREATED → ENDED for early failure', () => {
store.setSessionState(sid, 'CREATED', { clientId: 'c1', force: true });
store.setSessionState(sid, 'ENDED', { clientId: 'c1' });
expect(store.getSessionState(sid)).toBe('ENDED');
});

it('warns but does not block invalid transitions (Phase 1)', () => {
const messages: string[] = [];
const logger = { info: (msg: string) => messages.push(msg) };
const s = new EventStore(':memory:', logger);
s.upsertSession({ sessionId: 'warn-test' });
s.setSessionState('warn-test', 'ACTIVE', { clientId: 'c1', force: true });
// ACTIVE → CREATED is invalid
s.setSessionState('warn-test', 'CREATED', { clientId: 'c1' });
// Should still succeed (Phase 1 — warn only)
expect(s.getSessionState('warn-test')).toBe('CREATED');
// Should have logged the invalid transition
expect(messages.some((m) => m.includes('invalid'))).toBe(true);
s.close();
});

it('force flag bypasses validation', () => {
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', force: true });
// ACTIVE → CREATED is invalid but force bypasses
store.setSessionState(sid, 'CREATED', { clientId: 'c1', force: true });
expect(store.getSessionState(sid)).toBe('CREATED');
});

it('tracks full lifecycle: CREATED → STARTING → ACTIVE → ENDED', () => {
store.setSessionState(sid, 'CREATED', { clientId: 'c1', force: true });
store.setSessionState(sid, 'STARTING', { clientId: 'c1' });
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1' });
store.setSessionState(sid, 'ENDED', { clientId: 'c1', reason: 'completed' });
expect(store.getSessionState(sid)).toBe('ENDED');
});

it('tracks detach/reattach cycle', () => {
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', force: true });
store.setSessionState(sid, 'DETACHED', { clientId: 'c1', reason: 'transport_close' });
expect(store.getSessionState(sid)).toBe('DETACHED');
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', reason: 'reattach' });
expect(store.getSessionState(sid)).toBe('ACTIVE');
});

it('tracks suspend/resume cycle', () => {
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', force: true });
store.setSessionState(sid, 'SUSPENDED', { clientId: 'c1', reason: 'ios_background' });
expect(store.getSessionState(sid)).toBe('SUSPENDED');
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', reason: 'resume' });
expect(store.getSessionState(sid)).toBe('ACTIVE');
});

it('tracks closeout flow', () => {
store.setSessionState(sid, 'ACTIVE', { clientId: 'c1', force: true });
store.setSessionState(sid, 'CLOSING', { clientId: 'c1', reason: 'detach_ttl' });
store.setSessionState(sid, 'ENDED', { clientId: 'c1', reason: 'closeout_complete' });
expect(store.getSessionState(sid)).toBe('ENDED');
});

it('updates lastStateChange timestamp on each transition', () => {
store.setSessionState(sid, 'CREATED', { clientId: 'c1', force: true });
const meta1 = store.getSession(sid);
store.setSessionState(sid, 'STARTING', { clientId: 'c1' });
const meta2 = store.getSession(sid);
expect(meta2!.lastStateChange).toBeGreaterThanOrEqual(meta1!.lastStateChange!);
});
});

describe('close', () => {
it('is safe to call multiple times', () => {
store.close();
Expand Down
80 changes: 79 additions & 1 deletion packages/protocol/src/event-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import type {
StoredEvent,
SessionMeta,
SessionSearchResult,
SessionState,
EventStoreLogger,
} from './types.js';

// Re-export types for consumer convenience
export type { StoredEvent, SessionMeta, SessionSearchResult, EventStoreLogger };
export type { StoredEvent, SessionMeta, SessionSearchResult, SessionState, EventStoreLogger };

const noopLogger: EventStoreLogger = { info() {} };

Expand Down Expand Up @@ -45,6 +46,8 @@ interface SessionRow {
closed_by: string | null;
last_speaker: string | null;
last_speaker_at: number | null;
state: string | null;
last_state_change: number | null;
created_at: number;
updated_at: number;
}
Expand Down Expand Up @@ -75,6 +78,16 @@ const SCHEMA = `
);
`;

const VALID_TRANSITIONS: Record<SessionState, SessionState[]> = {
CREATED: ['STARTING', 'ENDED'],
STARTING: ['ACTIVE', 'ENDED'],
ACTIVE: ['DETACHED', 'SUSPENDED', 'CLOSING', 'ENDED'],
DETACHED: ['ACTIVE', 'SUSPENDED', 'CLOSING', 'ENDED'],
SUSPENDED: ['ACTIVE', 'ENDED'],
CLOSING: ['ENDED'],
ENDED: ['CREATED'],
};

export class EventStore {
private db: Database.Database | null;
private log: EventStoreLogger;
Expand All @@ -91,6 +104,8 @@ export class EventStore {
recordUsage: Database.Statement;
updateLastSpeaker: Database.Statement;
getAttentionSessions: Database.Statement;
setSessionState: Database.Statement;
getSessionState: Database.Statement;
};

constructor(dbPath: string, logger?: EventStoreLogger) {
Expand All @@ -106,6 +121,7 @@ export class EventStore {
this.migrateWorktreeTracking(db);
this.migrateCloseTracking(db);
this.migrateAttentionTracking(db);
this.migrateSessionState(db);

this.log.info('EventStore initialized', { dbPath });

Expand Down Expand Up @@ -160,6 +176,14 @@ export class EventStore {
ORDER BY last_speaker_at DESC
LIMIT 10`,
),
setSessionState: db.prepare(
`UPDATE sessions SET
state = ?,
last_state_change = ?,
updated_at = unixepoch('now', 'subsec') * 1000
WHERE session_id = ?`,
),
getSessionState: db.prepare('SELECT state FROM sessions WHERE session_id = ?'),
};
}

Expand Down Expand Up @@ -244,6 +268,19 @@ export class EventStore {
}
}

private migrateSessionState(db: Database.Database): void {
const columns = db.prepare("PRAGMA table_info('sessions')").all() as Array<{ name: string }>;
const columnNames = new Set(columns.map((c) => c.name));
if (!columnNames.has('state')) {
db.exec("ALTER TABLE sessions ADD COLUMN state TEXT DEFAULT 'ENDED'");
this.log.info('migrated sessions table: added state');
}
if (!columnNames.has('last_state_change')) {
db.exec('ALTER TABLE sessions ADD COLUMN last_state_change INTEGER');
this.log.info('migrated sessions table: added last_state_change');
}
}

close(): void {
if (this.db) {
this.db.close();
Expand Down Expand Up @@ -488,6 +525,45 @@ export class EventStore {
return (rows as SessionRow[]).map(rowToSession);
}

/** Set session lifecycle state. Warns on invalid transitions but does not block (Phase 1). */
setSessionState(
sessionId: string,
newState: SessionState,
opts?: { clientId?: string; reason?: string; force?: boolean },
): void {
const current = this.getSession(sessionId);
const fromState = (current?.state as SessionState) ?? null;
const now = Date.now();

if (fromState && !opts?.force) {
const allowed = VALID_TRANSITIONS[fromState];
if (!allowed?.includes(newState)) {
this.log.info('invalid session state transition (warn-only)', {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 style: Invalid state transitions are logged at info level. In production, info logs are high-volume and these warnings could be lost. Consider using warn level (would require expanding the EventStoreLogger interface) or at minimum adding a distinguishing prefix/field (e.g., level: 'warn' in the meta object) so they're filterable. [fixable]

sessionId,
fromState,
toState: newState,
clientId: opts?.clientId,
reason: opts?.reason,
});
}
}

this.stmts.setSessionState.run(newState, now, sessionId);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 unsafe_assumptions: setSessionState executes an UPDATE statement that silently does nothing if the session row doesn't exist in the database (no matching session_id). The method still logs 'session state transition' as if it succeeded. For new sessions where upsertSession hasn't been called yet, or if called with a stale/typo'd sessionId, the state write is silently lost. Consider checking changes on the run result. [fixable]


this.log.info('session state transition', {
sessionId,
fromState,
toState: newState,
clientId: opts?.clientId,
reason: opts?.reason,
});
}

getSessionState(sessionId: string): SessionState | null {
const row = this.stmts.getSessionState.get(sessionId) as { state: string | null } | undefined;
return (row?.state as SessionState) ?? null;
}

recordUsage(
sessionId: string,
usage: {
Expand Down Expand Up @@ -575,6 +651,8 @@ function rowToSession(row: SessionRow): SessionMeta {
closedBy: (row.closed_by as SessionMeta['closedBy']) ?? null,
lastSpeaker: (row.last_speaker as SessionMeta['lastSpeaker']) ?? null,
lastSpeakerAt: row.last_speaker_at ?? null,
state: (row.state as SessionMeta['state']) ?? null,
lastStateChange: row.last_state_change ?? null,
createdAt: row.created_at,
updatedAt: row.updated_at,
};
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type {
ImageAttachment,
Session,
SessionClosedBy,
SessionState,
StoredEvent,
SessionMeta,
SessionSearchResult,
Expand Down
11 changes: 11 additions & 0 deletions packages/protocol/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,15 @@ export interface ImageAttachment {

export type SessionClosedBy = 'user' | 'auto' | 'abandoned';

export type SessionState =
| 'CREATED'
| 'STARTING'
| 'ACTIVE'
| 'DETACHED'
| 'SUSPENDED'
| 'CLOSING'
| 'ENDED';

export interface Session {
id: string;
summary: string;
Expand Down Expand Up @@ -206,6 +215,8 @@ export interface SessionMeta {
closedBy: SessionClosedBy | null;
lastSpeaker: 'user' | 'assistant' | null;
lastSpeakerAt: number | null;
state: SessionState | null;
lastStateChange: number | null;
createdAt: number;
updatedAt: number;
}
Expand Down
Loading
Loading