Skip to content
Merged
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
11 changes: 6 additions & 5 deletions frontend/src/client-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ import { eventBus } from './lib/event-bus-singleton';
import { getPreferredModel } from './lib/model-preference';

/**
* Transport selector — set localStorage 'mitzo:transport' to 'sse' to use
* SSE + HTTP POST instead of WebSocket. Default is 'ws'.
* Transport selector — SSE + HTTP POST is the default transport (Transport SSOT P0).
* This is an intentional flip from WS-default per the transport-ssot design doc.
* Set localStorage 'mitzo:transport' to 'ws' to fall back to WebSocket.
*
* Toggle from console: localStorage.setItem('mitzo:transport', 'sse'); location.reload();
* Revert: localStorage.removeItem('mitzo:transport'); location.reload();
* Force WS: localStorage.setItem('mitzo:transport', 'ws'); location.reload();
* Revert SSE: localStorage.removeItem('mitzo:transport'); location.reload();
*/
const useSSE = typeof window !== 'undefined' && localStorage.getItem('mitzo:transport') === 'sse';
const useSSE = typeof window !== 'undefined' && localStorage.getItem('mitzo:transport') !== 'ws';

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.

🟡 regressions: Changing the default transport from WS to SSE is a user-visible behavioral change. Any client with no mitzo:transport localStorage key will silently switch from WebSocket to SSE on upgrade. If SSE has gaps (e.g., the parser only logs session_state_changed rather than acting on it), this could regress existing users. Ensure the SSE path has been validated end-to-end before merging, or gate behind a more explicit opt-in.

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.

🟡 regressions: Flipping the default transport from WS to SSE is a behavioral change that affects all clients on upgrade. Users with mitzo:transport unset in localStorage (the common case) will silently switch from WebSocket to SSE. If the SSE transport has any feature gaps or bugs, this becomes a production regression with no server-side rollback — each client must manually set localStorage('mitzo:transport', 'ws'). Consider whether the SSE path has been validated at parity with WS before making it the default, or gate this behind a more visible opt-in (e.g., server-sent feature flag).


const sseConfig: SseConnectionConfig | undefined = useSSE
? {
Expand Down
42 changes: 42 additions & 0 deletions packages/client/__tests__/protocol-parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,48 @@ describe('boot_context', () => {
});
});

// ─── Session state (Transport SSOT P0) ──────────────────────────────────────

describe('session_state_changed', () => {
it('produces no message actions (P0: observability only)', () => {
const r = parseServerMessage(
{
type: 'session_state_changed',
sessionId: 'sid-1',
state: 'running',
internalState: 'ACTIVE',
timestamp: 1234567890,
},
makeState(),
makeCallbacks(),
POOL_KEY,
);
expect(r.messagesActions).toHaveLength(0);
});

it('logs via console.debug', () => {
const spy = vi.spyOn(console, 'debug').mockImplementation(() => {});
parseServerMessage(
{
type: 'session_state_changed',
sessionId: 'sid-1',
state: 'idle',
internalState: 'ENDED',
timestamp: 1234567890,
},
makeState(),
makeCallbacks(),
POOL_KEY,
);
expect(spy).toHaveBeenCalledWith('[mitzo] session_state_changed', {
sessionId: 'sid-1',
state: 'idle',
internalState: 'ENDED',
});
spy.mockRestore();
});
});

// ─── Subagent cancellation ───────────────────────────────────────────────────

describe('subagent_cancelled', () => {
Expand Down
9 changes: 9 additions & 0 deletions packages/client/src/protocol-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,15 @@ export function parseServerMessage(
callbacks.onSessionRenamed?.(msg.name as string);
break;

case 'session_state_changed':

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: The session_state_changed case is a silent no-op with a comment referencing 'Phase 1'. Consider logging via console.debug or a callbacks hook so the event is observable during development, and to validate the message is actually received. [fixable]

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.

🟡 missing_tests: The new session_state_changed case in parseServerMessage has no test coverage in the protocol-parser test suite. Even though it's currently console-only, a test verifying it doesn't throw (and doesn't produce spurious actions) would prevent regressions when Phase 1 adds real behavior. [fixable]

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: The session_state_changed handler uses console.debug for production observability. If the project already has a structured logging pattern for the client package (e.g., via a logger utility), consider using that instead for consistency. If not, console.debug is fine for P0. [fixable]

// P0: log for observability, no UI action yet (Phase 1 will bind to running state)
console.debug('[mitzo] session_state_changed', {
sessionId: msg.sessionId,
state: msg.state,
internalState: msg.internalState,
});
break;

case 'message_start':
result.messagesActions.push({
type: 'MESSAGE_START',
Expand Down
209 changes: 209 additions & 0 deletions packages/protocol/__tests__/event-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,215 @@ describe('EventStore', () => {
});
});

describe('toClientState mapping (via session_state_changed events)', () => {
const sid = 'client-state-test';

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

it('maps CREATED to idle', () => {
store.setSessionState(sid, 'CREATED', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('idle');
expect(stateEvent?.payload.internalState).toBe('CREATED');
});

it('maps STARTING to running', () => {
store.setSessionState(sid, 'STARTING', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('running');
});

it('maps ACTIVE to running', () => {
store.setSessionState(sid, 'ACTIVE', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('running');
});

it('maps DETACHED to idle', () => {
store.setSessionState(sid, 'DETACHED', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('idle');
});

it('maps SUSPENDED to idle', () => {
store.setSessionState(sid, 'SUSPENDED', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('idle');
});

it('maps CLOSING to idle', () => {
store.setSessionState(sid, 'CLOSING', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('idle');
});

it('maps ENDED to idle', () => {
store.setSessionState(sid, 'ENDED', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.state).toBe('idle');
});

it('includes timestamp in event payload', () => {
const before = Date.now();
store.setSessionState(sid, 'ACTIVE', { force: true });
const events = store.getSessionEvents(sid);
const stateEvent = events.find((e) => e.type === 'session_state_changed');
expect(stateEvent?.payload.timestamp).toBeGreaterThanOrEqual(before);
});
});

describe('setSessionState syncs is_active', () => {
const sid = 'is-active-sync-test';

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

it('sets is_active=true for ACTIVE', () => {
store.setSessionState(sid, 'ACTIVE', { force: true });
expect(store.getSession(sid)!.isActive).toBe(true);
});

it('sets is_active=true for DETACHED', () => {
store.setSessionState(sid, 'DETACHED', { force: true });
expect(store.getSession(sid)!.isActive).toBe(true);
});

it('sets is_active=true for SUSPENDED', () => {
store.setSessionState(sid, 'SUSPENDED', { force: true });
expect(store.getSession(sid)!.isActive).toBe(true);
});

it('sets is_active=false for ENDED', () => {
store.setSessionState(sid, 'ENDED', { force: true });
expect(store.getSession(sid)!.isActive).toBe(false);
});

it('sets is_active=false for CLOSING', () => {
store.setSessionState(sid, 'CLOSING', { force: true });
expect(store.getSession(sid)!.isActive).toBe(false);
});

it('sets is_active=true for CREATED', () => {
store.setSessionState(sid, 'CREATED', { force: true });
expect(store.getSession(sid)!.isActive).toBe(true);
});

it('sets is_active=true for STARTING', () => {
store.setSessionState(sid, 'STARTING', { force: true });
expect(store.getSession(sid)!.isActive).toBe(true);
});
});

describe('recoverStaleSessions', () => {
it('transitions ACTIVE sessions to ENDED', () => {
store.upsertSession({ sessionId: 'active-1' });
store.setSessionState('active-1', 'ACTIVE', { force: true });

const count = store.recoverStaleSessions();

expect(count).toBe(1);
expect(store.getSessionState('active-1')).toBe('ENDED');
});

it('transitions STARTING sessions to ENDED', () => {
store.upsertSession({ sessionId: 'starting-1' });
store.setSessionState('starting-1', 'STARTING', { force: true });

store.recoverStaleSessions();

expect(store.getSessionState('starting-1')).toBe('ENDED');
});

it('transitions DETACHED sessions to ENDED', () => {
store.upsertSession({ sessionId: 'detached-1' });
store.setSessionState('detached-1', 'DETACHED', { force: true });

store.recoverStaleSessions();

expect(store.getSessionState('detached-1')).toBe('ENDED');
});

it('transitions SUSPENDED sessions to ENDED', () => {
store.upsertSession({ sessionId: 'suspended-1' });
store.setSessionState('suspended-1', 'SUSPENDED', { force: true });

store.recoverStaleSessions();

expect(store.getSessionState('suspended-1')).toBe('ENDED');
});

it('transitions CLOSING sessions to ENDED', () => {
store.upsertSession({ sessionId: 'closing-1' });
store.setSessionState('closing-1', 'CLOSING', { force: true });

store.recoverStaleSessions();

expect(store.getSessionState('closing-1')).toBe('ENDED');
});

it('does not touch ENDED sessions', () => {
store.upsertSession({ sessionId: 'ended-1' });
store.setSessionState('ended-1', 'ENDED', { force: true });

const count = store.recoverStaleSessions();

expect(count).toBe(0);
expect(store.getSessionState('ended-1')).toBe('ENDED');
});

it('does not touch CREATED sessions', () => {
store.upsertSession({ sessionId: 'created-1' });
store.setSessionState('created-1', 'CREATED', { force: true });

const count = store.recoverStaleSessions();

expect(count).toBe(0);
expect(store.getSessionState('created-1')).toBe('CREATED');
});

it('returns correct count for multiple stale sessions', () => {
store.upsertSession({ sessionId: 'stale-1' });
store.upsertSession({ sessionId: 'stale-2' });
store.upsertSession({ sessionId: 'ok-1' });
store.setSessionState('stale-1', 'ACTIVE', { force: true });
store.setSessionState('stale-2', 'DETACHED', { force: true });
store.setSessionState('ok-1', 'ENDED', { force: true });

const count = store.recoverStaleSessions();

expect(count).toBe(2);
});

it('emits session_state_changed events for recovered sessions', () => {
store.upsertSession({ sessionId: 'recover-1' });
store.setSessionState('recover-1', 'ACTIVE', { force: true });

// Clear events from setup
const beforeCount = store.getSessionEvents('recover-1').length;

store.recoverStaleSessions();

const events = store.getSessionEvents('recover-1');
// Should have new session_state_changed event from recovery
const recoveryEvent = events
.slice(beforeCount)
.find((e) => e.type === 'session_state_changed');
expect(recoveryEvent).toBeDefined();
expect(recoveryEvent?.payload.state).toBe('idle');
expect(recoveryEvent?.payload.internalState).toBe('ENDED');
});
});

describe('close', () => {
it('is safe to call multiple times', () => {
store.close();
Expand Down
Loading
Loading