From 8807b3ff6d04d2dce4a156210d3be4d9849c3405 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Fri, 19 Jun 2026 18:42:39 -0400 Subject: [PATCH] COD-131 sync tab order across devices via server state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tab reordering (drag-and-drop + Ctrl+Shift+{/}) persisted only to localStorage (codeman-session-order), so each device kept its own private order. Add server-side persistence so the order follows the user across devices, live. Takes the issue's recommended default (a): one global order, server authoritative, localStorage as offline fallback. - session-order.ts (new, pure + unit-tested): normalizeSessionOrder (coerce to string[], drop empty/non-string, dedup) and mergeSessionOrder (the pushing device's order wins; ids the device hadn't loaded fall to the end in their existing relative order, never dropped — graceful for closed/remote/parked sessions absent on that device). - AppState.sessionOrder?: string[]; StateStore get/setSessionOrder + the field added to buildPartialJson() (the incremental serializer whitelists fields, so without this the value never reached disk / survived a restart). - PUT /api/session-order (session-routes): parse -> merge -> persist -> broadcast session:orderChanged; getLightState() init snapshot now carries sessionOrder so a fresh load/reconnect restores it. - SSE event session:orderChanged registered in sse-events.ts + constants.js. - app.js: handleInit seeds localStorage from the server snapshot before syncSessionOrder(); saveSessionOrder() also PUTs to the server (debounced 400ms, covers drag + both keyboard moves); _onSessionOrderChanged adopts a remote order and re-renders (no-op-guarded to avoid echo flicker). Verified (orchestrator re-ran all gates): tsc 0, lint 0, frontend-syntax + prettier clean, build ok; session-order + session-order-routes + state-store 56/56. Functional round-trip on an isolated beta: PUT {a,b,c} -> status snapshot reflects it; merge PUT {c,a} vs {a,b,c} -> {c,a,b} (b preserved at end); malformed payload rejected with a clean 400; sessionOrder persisted to state.json and survived a restart. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit 79415f2fdfbdf3fbe362a063534e7f84c553eefb) --- src/session-order.ts | 68 ++++++++++++ src/state-store.ts | 14 +++ src/types/app-state.ts | 2 + src/web/public/app.js | 41 ++++++- src/web/public/constants.js | 3 + src/web/routes/session-routes.ts | 14 +++ src/web/schemas.ts | 5 + src/web/server.ts | 1 + src/web/sse-events.ts | 6 ++ test/mocks/mock-route-context.ts | 8 ++ test/routes/session-order-routes.test.ts | 129 +++++++++++++++++++++++ test/session-order.test.ts | 74 +++++++++++++ 12 files changed, 364 insertions(+), 1 deletion(-) create mode 100644 src/session-order.ts create mode 100644 test/routes/session-order-routes.test.ts create mode 100644 test/session-order.test.ts diff --git a/src/session-order.ts b/src/session-order.ts new file mode 100644 index 00000000..21a2ba62 --- /dev/null +++ b/src/session-order.ts @@ -0,0 +1,68 @@ +/** + * @fileoverview Pure helpers for the global session tab-order (COD-131). + * + * Tab order (drag-and-drop reorder + Ctrl+Shift+{/}) is persisted server-side + * so it follows the user across devices. The server is authoritative; the + * browser's localStorage (`codeman-session-order`) is the offline fallback. + * + * These helpers are pure (no IO) so they can be unit-tested in isolation and + * reused by both the PUT /api/session-order route and the StateStore accessor. + * + * - `normalizeSessionOrder` coerces arbitrary input into a clean string[] + * (non-empty strings only, deduped with first occurrence winning). + * - `mergeSessionOrder` lets the pushing device's order win, while preserving + * any server-only ids the pushing device didn't know about — they fall to the + * END in their existing relative order, never dropped. + */ + +/** + * Coerce arbitrary input into a clean ordered list of session ids: + * keep only non-empty strings and dedup (first occurrence wins). + * + * @param order - unknown input (expected to be a string[], but defensive) + * @returns a normalized string[] (empty array for non-array / all-junk input) + */ +export function normalizeSessionOrder(order: unknown): string[] { + if (!Array.isArray(order)) { + return []; + } + const seen = new Set(); + const result: string[] = []; + for (const entry of order) { + if (typeof entry !== 'string' || entry.length === 0) { + continue; + } + if (seen.has(entry)) { + continue; + } + seen.add(entry); + result.push(entry); + } + return result; +} + +/** + * Merge an incoming order from a pushing device with the existing server order. + * + * The incoming order wins; any ids present in `existing` but NOT in `incoming` + * are appended at the END, preserving their relative order. This is the + * "server-only ids the pushing device didn't know about fall to the end, never + * dropped" rule. + * + * Both arguments are normalized first, so callers may pass raw input safely. + * + * @param incoming - the order the pushing device wants + * @param existing - the current server-side order + * @returns the merged, normalized order + */ +export function mergeSessionOrder(incoming: string[], existing: string[]): string[] { + const normalizedIncoming = normalizeSessionOrder(incoming); + const incomingSet = new Set(normalizedIncoming); + const merged = [...normalizedIncoming]; + for (const id of normalizeSessionOrder(existing)) { + if (!incomingSet.has(id)) { + merged.push(id); + } + } + return merged; +} diff --git a/src/state-store.ts b/src/state-store.ts index 1cb6378e..792e4a21 100644 --- a/src/state-store.ts +++ b/src/state-store.ts @@ -278,6 +278,9 @@ export class StateStore { if (this.state.cronJobRuns) { parts.push(`"cronJobRuns":${JSON.stringify(this.state.cronJobRuns)}`); } + if (this.state.sessionOrder) { + parts.push(`"sessionOrder":${JSON.stringify(this.state.sessionOrder)}`); + } return `{${parts.join(',')}}`; } @@ -630,6 +633,17 @@ export class StateStore { this.save(); } + /** Returns the global tab order (ordered sessionIds), [] if unset. COD-131. */ + getSessionOrder(): string[] { + return this.state.sessionOrder ?? []; + } + + /** Persists the global tab order (ordered sessionIds) and triggers a debounced save. COD-131. */ + setSessionOrder(order: string[]): void { + this.state.sessionOrder = order; + this.save(); + } + /** Resets all state to initial values and saves immediately. */ reset(): void { this.state = createInitialState(); diff --git a/src/types/app-state.ts b/src/types/app-state.ts index e0b39838..4d16ddcd 100644 --- a/src/types/app-state.ts +++ b/src/types/app-state.ts @@ -116,6 +116,8 @@ export interface AppState { cronJobs?: Record; /** Scheduled job run history, keyed by run ID. */ cronJobRuns?: Record; + /** Global tab order shared across devices (ordered list of sessionIds) — COD-131 */ + sessionOrder?: string[]; } // ========== Default Configuration ========== diff --git a/src/web/public/app.js b/src/web/public/app.js index c27c7f48..c6017815 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -287,6 +287,9 @@ const _SSE_HANDLER_MAP = [ // Clipboard [SSE_EVENTS.CLIPBOARD_WRITE, '_onClipboardWrite'], + + // Session order (global tab order sync, COD-131) + [SSE_EVENTS.SESSION_ORDER_CHANGED, '_onSessionOrderChanged'], ]; @@ -2956,6 +2959,13 @@ class CodemanApp { // on another device). try { localStorage.removeItem('codeman-tab-meta'); } catch {} + // COD-131: server is authoritative for global tab order. Seed localStorage + // from the server snapshot (if present) so syncSessionOrder() reconciles + // against the cross-device order rather than this device's stale local copy. + if (Array.isArray(data.sessionOrder) && data.sessionOrder.length) { + try { localStorage.setItem('codeman-session-order', JSON.stringify(data.sessionOrder)); } catch {} + } + // Sync sessionOrder with current sessions (preserve order, add new, remove stale) this.syncSessionOrder(); @@ -3520,13 +3530,42 @@ class CodemanApp { } } - // Save session order to localStorage + // Save session order to localStorage and (debounced) sync to the server so it + // follows the user across devices (COD-131). localStorage stays the offline + // fallback; the server is authoritative and echoes back via SSE. saveSessionOrder() { try { localStorage.setItem('codeman-session-order', JSON.stringify(this.sessionOrder)); } catch { // Ignore storage errors } + const order = [...this.sessionOrder]; + this._debouncedCall('saveSessionOrderServer', () => { + fetch('/api/session-order', { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ order }), + }).catch(() => {}); + }, 400); + } + + // COD-131: another device (or our own debounced push) reordered tabs. Adopt the + // server order as the new base and reconcile to our currently-open sessions. + // Guard against no-op churn so an echo of our own push doesn't flicker the tabs. + _onSessionOrderChanged(data) { + if (!data || !Array.isArray(data.order)) return; + try { + localStorage.setItem('codeman-session-order', JSON.stringify(data.order)); + } catch { + // Ignore storage errors + } + const before = JSON.stringify(this.sessionOrder); + this.syncSessionOrder(); + // Only re-render when the reconciled order actually changed (avoids flicker + // when the broadcast is just an echo of the order we already have). + if (JSON.stringify(this.sessionOrder) !== before) { + this._fullRenderSessionTabs(); + } } // Set up drag-and-drop handlers on tab elements diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 1751bfe3..3826feda 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -474,6 +474,9 @@ const SSE_EVENTS = { CASE_LINKED: 'case:linked', CASE_DELETED: 'case:deleted', CASE_ORDER_CHANGED: 'case:order-changed', + + // Session order (global tab order sync) + SESSION_ORDER_CHANGED: 'session:orderChanged', }; // ═══════════════════════════════════════════════════════════════ diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 6da2cf52..202996d7 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -35,7 +35,9 @@ import { QuickRunSchema, QuickStartSchema, InteractiveStartSchema, + SessionOrderUpdateSchema, } from '../schemas.js'; +import { mergeSessionOrder } from '../../session-order.js'; import { autoConfigureRalph, CASES_DIR, @@ -280,6 +282,18 @@ export function registerSessionRoutes( return ctx.getLightSessionsState(); }); + // ========== Session Tab Order (global sync, COD-131) ========== + + app.put('/api/session-order', async (req): Promise> => { + const { order } = parseBody(SessionOrderUpdateSchema, req.body, 'Invalid session order'); + // Server is authoritative but never drops ids it knows about that the + // pushing device hadn't loaded yet — those fall to the end (mergeSessionOrder). + const merged = mergeSessionOrder(order, ctx.store.getSessionOrder()); + ctx.store.setSessionOrder(merged); + ctx.broadcast(SseEvent.SessionOrderChanged, { order: merged }); + return { success: true, data: { order: merged } }; + }); + // ========== Session Creation ========== app.post('/api/sessions', async (req) => { diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 91485a45..e1bd7c55 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -769,6 +769,11 @@ export const CaseOrderSchema = z.object({ order: z.array(z.string().regex(/^[a-zA-Z0-9_-]+$/, 'Invalid case name format')), }); +/** PUT /api/session-order — global tab order (ordered sessionIds), COD-131 */ +export const SessionOrderUpdateSchema = z.object({ + order: z.array(z.string()), +}); + /** POST /api/auth/revoke */ export const RevokeSessionSchema = z.object({ sessionToken: z.string().min(1).max(200).optional(), diff --git a/src/web/server.ts b/src/web/server.ts index 393b5a35..791d5921 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1769,6 +1769,7 @@ export class WebServer extends EventEmitter { timestamp: now, inputCjkForm: process.env.INPUT_CJK_FORM?.toUpperCase() === 'ON', planUsage: getLatestPlanUsage(), // last-known plan-usage telemetry, for the header chip on fresh load + sessionOrder: this.store.getSessionOrder(), // global tab order, synced across devices (COD-131) }; this.cachedLightState = { data: result, timestamp: now }; diff --git a/src/web/sse-events.ts b/src/web/sse-events.ts index 6fc419d6..4a7d8878 100644 --- a/src/web/sse-events.ts +++ b/src/web/sse-events.ts @@ -370,6 +370,9 @@ export const CaseDeleted = 'case:deleted' as const; /** Case ordering changed. */ export const CaseOrderChanged = 'case:order-changed' as const; +/** Global session tab order changed (synced across devices). COD-131. */ +export const SessionOrderChanged = 'session:orderChanged' as const; + // ─── Namespace Re-export ───────────────────────────────────────────────────── /** @@ -551,4 +554,7 @@ export const SseEvent = { CaseLinked, CaseDeleted, CaseOrderChanged, + + // Session order (global tab order sync) + SessionOrderChanged, } as const; diff --git a/test/mocks/mock-route-context.ts b/test/mocks/mock-route-context.ts index e7dedc62..14aa1ea1 100644 --- a/test/mocks/mock-route-context.ts +++ b/test/mocks/mock-route-context.ts @@ -21,6 +21,10 @@ export function createMockRouteContext(options?: { sessionId?: string }) { const sessions = new Map(); sessions.set(sessionId, session); + // Stateful backing for the global tab order (COD-131) so route tests can + // assert that setSessionOrder() actually persists what the handler computed. + let sessionOrder: string[] = []; + return { // -- SessionPort -- sessions, @@ -65,6 +69,10 @@ export function createMockRouteContext(options?: { sessionId?: string }) { load: vi.fn(), incrementSessionsCreated: vi.fn(), setConfig: vi.fn(), + getSessionOrder: vi.fn(() => sessionOrder), + setSessionOrder: vi.fn((order: string[]) => { + sessionOrder = order; + }), getAggregateStats: vi.fn(() => ({ totalInputTokens: 0, totalOutputTokens: 0, totalCost: 0 })), getGlobalStats: vi.fn(() => ({ sessionsCreated: 0 })), getDailyStats: vi.fn(() => []), diff --git a/test/routes/session-order-routes.test.ts b/test/routes/session-order-routes.test.ts new file mode 100644 index 00000000..f7dbba7f --- /dev/null +++ b/test/routes/session-order-routes.test.ts @@ -0,0 +1,129 @@ +/** + * @fileoverview Tests for PUT /api/session-order (global tab-order sync, COD-131). + * + * Uses app.inject() — no real HTTP ports needed. + * Asserts the uniform envelope contract: + * SUCCESS -> 2xx, { success: true, data: { order } } + * ERROR -> 4xx/5xx, { success: false, error, errorCode } + * and that the order is persisted to the (mock) StateStore + broadcast over SSE. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; +import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js'; +import { installRouteErrorHandler } from '../../src/web/route-error-handler.js'; +import { ApiErrorCode, httpStatusForErrorCode } from '../../src/types.js'; + +// registerSessionRoutes pulls in session.js which can shell out; stub the bits +// that would touch the OS at import/registration time. None are needed by the +// session-order handler itself, but the module imports them. +vi.mock('node:child_process', async (orig) => { + const actual = await orig(); + return { ...actual, execFile: vi.fn(), spawn: vi.fn() }; +}); + +import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; + +interface LocalHarness { + app: FastifyInstance; + ctx: MockRouteContext; +} + +async function buildHarness(): Promise { + const app = Fastify({ logger: false }); + await app.register(fastifyCookie); + + const ctx = createMockRouteContext(); + registerSessionRoutes(app, ctx as unknown as Parameters[1]); + + // Mirror production's uniform-envelope preSerialization hook (server.ts). + app.addHook('preSerialization', (req, reply, payload: unknown, done) => { + if (!req.url.startsWith('/api')) return done(null, payload); + if (payload === null || typeof payload !== 'object') return done(null, payload); + const p = payload as { success?: unknown; errorCode?: unknown }; + if (p.success === false) { + if (reply.statusCode === 200 && typeof p.errorCode === 'string') { + reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode)); + } + return done(null, payload); + } + if (p.success === true) return done(null, payload); + return done(null, { success: true, data: payload }); + }); + + installRouteErrorHandler(app); + await app.ready(); + return { app, ctx }; +} + +describe('PUT /api/session-order', () => { + let harness: LocalHarness; + + beforeEach(async () => { + harness = await buildHarness(); + }); + + afterEach(async () => { + await harness.app.close(); + }); + + it('persists the order and returns it in the envelope', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/session-order', + payload: { order: ['a', 'b', 'c'] }, + }); + + expect(res.statusCode).toBe(200); + const body = res.json(); + expect(body).toEqual({ success: true, data: { order: ['a', 'b', 'c'] } }); + // Persisted to the store. + expect(harness.ctx.store.setSessionOrder).toHaveBeenCalledWith(['a', 'b', 'c']); + expect(harness.ctx.store.getSessionOrder()).toEqual(['a', 'b', 'c']); + // Broadcast over SSE. + expect(harness.ctx.broadcast).toHaveBeenCalledWith('session:orderChanged', { order: ['a', 'b', 'c'] }); + }); + + it('preserves a server-only id (unknown to the pushing device) at the end', async () => { + // Seed the store with an order containing a server-only id "z". + harness.ctx.store.setSessionOrder(['a', 'z', 'b']); + (harness.ctx.broadcast as ReturnType).mockClear(); + + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/session-order', + payload: { order: ['b', 'a'] }, + }); + + expect(res.statusCode).toBe(200); + const body = res.json(); + // Incoming order wins, server-only "z" falls to the end. + expect(body).toEqual({ success: true, data: { order: ['b', 'a', 'z'] } }); + expect(harness.ctx.store.getSessionOrder()).toEqual(['b', 'a', 'z']); + expect(harness.ctx.broadcast).toHaveBeenCalledWith('session:orderChanged', { order: ['b', 'a', 'z'] }); + }); + + it('normalizes junk input (dedup + drop empties) before persisting', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/session-order', + payload: { order: ['a', 'a', '', 'b'] }, + }); + + expect(res.statusCode).toBe(200); + expect(res.json()).toEqual({ success: true, data: { order: ['a', 'b'] } }); + }); + + it('rejects a non-array order with a 4xx envelope', async () => { + const res = await harness.app.inject({ + method: 'PUT', + url: '/api/session-order', + payload: { order: 'nope' }, + }); + + expect(res.statusCode).toBeGreaterThanOrEqual(400); + const body = res.json(); + expect(body.success).toBe(false); + expect(harness.ctx.store.setSessionOrder).not.toHaveBeenCalled(); + }); +}); diff --git a/test/session-order.test.ts b/test/session-order.test.ts new file mode 100644 index 00000000..0e8f012e --- /dev/null +++ b/test/session-order.test.ts @@ -0,0 +1,74 @@ +/** + * @fileoverview Unit tests for the pure session-order helpers + * (normalizeSessionOrder, mergeSessionOrder) used by global tab-order sync (COD-131). + */ +import { describe, it, expect } from 'vitest'; +import { normalizeSessionOrder, mergeSessionOrder } from '../src/session-order.js'; + +describe('normalizeSessionOrder', () => { + it('keeps a clean array unchanged', () => { + expect(normalizeSessionOrder(['a', 'b', 'c'])).toEqual(['a', 'b', 'c']); + }); + + it('dedups, first occurrence wins', () => { + expect(normalizeSessionOrder(['a', 'b', 'a', 'c', 'b'])).toEqual(['a', 'b', 'c']); + }); + + it('drops empty strings', () => { + expect(normalizeSessionOrder(['a', '', 'b', ' '])).toEqual(['a', 'b', ' ']); + expect(normalizeSessionOrder([''])).toEqual([]); + }); + + it('drops non-string entries', () => { + expect(normalizeSessionOrder(['a', 1, null, undefined, {}, 'b', true])).toEqual(['a', 'b']); + }); + + it('returns [] for non-array input', () => { + expect(normalizeSessionOrder(undefined)).toEqual([]); + expect(normalizeSessionOrder(null)).toEqual([]); + expect(normalizeSessionOrder('abc')).toEqual([]); + expect(normalizeSessionOrder(42)).toEqual([]); + expect(normalizeSessionOrder({ 0: 'a' })).toEqual([]); + }); + + it('returns [] for empty array', () => { + expect(normalizeSessionOrder([])).toEqual([]); + }); +}); + +describe('mergeSessionOrder', () => { + it('incoming order wins', () => { + expect(mergeSessionOrder(['c', 'a', 'b'], ['a', 'b', 'c'])).toEqual(['c', 'a', 'b']); + }); + + it('appends server-only ids (not in incoming) at the end, preserving their relative order', () => { + expect(mergeSessionOrder(['a', 'b'], ['x', 'a', 'y', 'b', 'z'])).toEqual(['a', 'b', 'x', 'y', 'z']); + }); + + it('empty incoming yields the existing order (normalized)', () => { + expect(mergeSessionOrder([], ['a', 'b', 'c'])).toEqual(['a', 'b', 'c']); + }); + + it('empty existing yields the incoming order (normalized)', () => { + expect(mergeSessionOrder(['a', 'b', 'c'], [])).toEqual(['a', 'b', 'c']); + }); + + it('both empty yields empty', () => { + expect(mergeSessionOrder([], [])).toEqual([]); + }); + + it('normalizes both args (dedup + drop junk) before merging', () => { + expect(mergeSessionOrder(['a', 'a', '', 'b'], ['b', 'c', 'c', ''])).toEqual(['a', 'b', 'c']); + }); + + it('does not duplicate an id present in both', () => { + expect(mergeSessionOrder(['a', 'b'], ['b', 'a'])).toEqual(['a', 'b']); + }); + + it('handles non-array / junk inputs defensively', () => { + // @ts-expect-error testing runtime robustness against bad input + expect(mergeSessionOrder(null, ['a', 'b'])).toEqual(['a', 'b']); + // @ts-expect-error testing runtime robustness against bad input + expect(mergeSessionOrder(['a'], 'nope')).toEqual(['a']); + }); +});