From 954a9ac26a3159447f86e02b1e6b3f66f7f505a0 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Mon, 17 Aug 2026 15:59:17 +0200 Subject: [PATCH] fix: date a working row by its turn, not by the session age `TuiSessionRow` declared `lastSubmitAt`/`inputTokens`/`outputTokens`, `stateSince()` ordered the WORKING group by the first of them and `renderRowLines()` painted the other two, but nothing ever filled any of them in: the unified list carries none, and the `session:updated` payload that does was discarded (an event only schedules a refetch). So a running turn was dated by its SESSION's creation instead. Measured against the live server before the fix: w65 (created 21h ago, turn started one minute earlier) outranked w67 (created 15 minutes ago, turn started five minutes earlier), the reverse of the rule docs/tui.md states, and the elapsed column read `21h` for a turn a minute old. The token column was unreachable code for the same reason. `fetchLiveSessionMetrics()` reads the three fields from `GET /api/sessions` and `applyLiveMetrics()` folds them onto the rows. That route answers from the server's cached LIGHT state (no terminal buffers): 10-20ms measured, against the ~550ms the unified list in the same `Promise.all` already costs, so it is cheap enough to ride every refresh. It is best-effort like the approvals and tmux reads beside it, because losing the anchor is better than losing the list. A ZERO is treated as unknown rather than merged: `stateSince()` reads `lastSubmitAt ?? createdAt` and 0 is not nullish, so a merged 0 would date every never-submitted session to the epoch. The snapshot path gets the same merge, or `codeman tui --list` would number the WORKING group differently from the dashboard that `codeman tui ` indexes into. Verified live: working rows now show 28m/8m (turn age, tokens 280.5k/65.2k) where they showed 21h/34m and no tokens. The e2e assertion fails on master's wiring with `[*] 10m` against a session that pressed Enter one minute ago. Co-Authored-By: Claude Opus 5 (1M context) --- src/tui/tui-app.ts | 44 ++++++++++++++++++++++--- src/tui/tui-client.ts | 52 +++++++++++++++++++++++++++-- test/tui/tui-app.test.ts | 65 +++++++++++++++++++++++++++++++++++-- test/tui/tui-client.test.ts | 28 ++++++++++++++++ test/tui/tui-e2e.test.ts | 25 ++++++++++++++ 5 files changed, 205 insertions(+), 9 deletions(-) diff --git a/src/tui/tui-app.ts b/src/tui/tui-app.ts index c600acb3..25f1eb96 100644 --- a/src/tui/tui-app.ts +++ b/src/tui/tui-app.ts @@ -58,6 +58,7 @@ import { TuiClient, type TuiApprovalAnswer, type TuiEventStream, + type TuiLiveSessionMetrics, type TuiPlanUsage, type TuiQuickStartOptions, type TuiTmuxSession, @@ -455,6 +456,35 @@ export function applyMuxNames(sessions: readonly TuiSessionRow[], tmux: readonly }); } +/** + * Fold the live-only counters onto the rows the unified list produced: the + * pane's last Enter (which dates a running turn and orders the WORKING group) + * and the token totals the wide layout shows. + * + * A ZERO is treated as "unknown" rather than merged, and that is the whole + * reason this is not a spread: `stateSince()` reads `lastSubmitAt ?? createdAt`, + * and 0 is not nullish, so merging a 0 would date every never-submitted session + * to the epoch and sort it as the oldest turn on the list. The join is on the + * FULL id, since both sides come from the same server. + */ +export function applyLiveMetrics( + sessions: readonly TuiSessionRow[], + metrics: readonly TuiLiveSessionMetrics[] +): TuiSessionRow[] { + if (metrics.length === 0) return sessions.map((session) => ({ ...session })); + const byId = new Map(metrics.map((entry) => [entry.sessionId, entry])); + return sessions.map((session) => { + const live = byId.get(session.sessionId); + if (!live) return { ...session }; + return { + ...session, + ...(live.lastSubmitAt ? { lastSubmitAt: live.lastSubmitAt } : {}), + ...(live.inputTokens ? { inputTokens: live.inputTokens } : {}), + ...(live.outputTokens ? { outputTokens: live.outputTokens } : {}), + }; + }); +} + const STATE_TONE: Record = { 'blocked-permission': 'err', 'blocked-question': 'err', @@ -762,12 +792,15 @@ class TuiApp { private async refreshConnected(): Promise { try { - const [sessions, approvals, tmux] = await Promise.all([ + const [sessions, approvals, tmux, metrics] = await Promise.all([ this.client.fetchUnifiedSessions(UNIFIED_LIMIT), this.client.fetchApprovals().catch(() => []), this.client.enumerateTmuxSessions().catch(() => [] as TuiTmuxSession[]), + // Best-effort like the other two: without it a running turn is dated by + // its session's creation, which is worse than the list going stale. + this.client.fetchLiveSessionMetrics().catch(() => [] as TuiLiveSessionMetrics[]), ]); - this.model.replaceSessions(applyMuxNames(sessions, tmux)); + this.model.replaceSessions(applyLiveMetrics(applyMuxNames(sessions, tmux), metrics)); this.model.setApprovals(approvals); this.noteApprovals(approvals); if (this.pendingSelectId && this.model.select(this.pendingSelectId)) this.pendingSelectId = null; @@ -1684,12 +1717,15 @@ async function snapshot(client: TuiClient): Promise { model.replaceSessions(tmuxRowsToSessions(await client.enumerateTmuxSessions())); return { kind: 'ok', rows: model.rows(), degraded: true }; } - const [sessions, approvals, tmux] = await Promise.all([ + const [sessions, approvals, tmux, metrics] = await Promise.all([ client.fetchUnifiedSessions(UNIFIED_LIMIT), client.fetchApprovals().catch(() => []), client.enumerateTmuxSessions().catch(() => [] as TuiTmuxSession[]), + client.fetchLiveSessionMetrics().catch(() => [] as TuiLiveSessionMetrics[]), ]); - model.replaceSessions(applyMuxNames(sessions, tmux)); + // Same merge as the dashboard, so `--list`'s numbers stay the numbers + // `codeman tui ` takes: the WORKING group's order depends on it. + model.replaceSessions(applyLiveMetrics(applyMuxNames(sessions, tmux), metrics)); model.setApprovals(approvals); return { kind: 'ok', rows: model.rows(), degraded: false }; } diff --git a/src/tui/tui-client.ts b/src/tui/tui-client.ts index e9261c1d..38ded631 100644 --- a/src/tui/tui-client.ts +++ b/src/tui/tui-client.ts @@ -31,9 +31,12 @@ * - Plan usage has no route of its own: the last-known snapshot rides * `GET /api/status` as `planUsage` (`web/plan-usage-latest.ts`) and updates * arrive as `session:statusTelemetry` SSE frames. - * - The unified list carries no token counters or turn-start stamp - * (`TuiSessionRow.lastSubmitAt`), so those stay unset here; the app layer - * merges them from live session state when it wants them. + * - The unified list carries no token counters and no turn-start stamp + * (`TuiSessionRow.lastSubmitAt`), which the WORKING group is ordered by, so + * `fetchLiveSessionMetrics()` reads them from `GET /api/sessions` and + * `applyLiveMetrics()` (tui-app) folds them onto the rows. That route answers + * from the server's cached LIGHT state (no terminal buffers, ~10ms), which is + * what makes it cheap enough to ride every refresh. * * @module tui/tui-client */ @@ -147,6 +150,22 @@ export interface TuiQuickStartResult { caseName?: string; } +/** + * The live-only fields the unified list does not carry, per session id. + * + * `lastSubmitAt` is the one the dashboard cannot do without: it is the pane's + * last Enter, which is what a WORKING row's elapsed column shows and what the + * WORKING group is sorted by. Without it a running turn is dated by the + * SESSION's creation instead, so a day-old session that started a turn a minute + * ago outranks one that has been working for an hour. + */ +export interface TuiLiveSessionMetrics { + sessionId: string; + lastSubmitAt?: number; + inputTokens?: number; + outputTokens?: number; +} + /** Init snapshot, narrowed to the two facts the dashboard header shows. */ export interface TuiInitState { version?: string; @@ -507,6 +526,33 @@ export class TuiClient { return data?.sessions ?? []; } + /** + * The turn-start stamp and token counters for every LIVE session. + * + * `GET /api/sessions` is the light state (`getLightSessionsState()`, itself + * cached server-side): no terminal buffers, so this is a ~10ms read next to + * the unified list's transcript scan. Rows are narrowed to the three fields + * the dashboard actually merges, and a row with no usable id is dropped + * rather than folded in under an empty key. + */ + async fetchLiveSessionMetrics(): Promise { + const data = await this.requestData< + Array<{ id?: unknown; lastSubmitAt?: unknown; inputTokens?: unknown; outputTokens?: unknown }> + >('GET', '/api/sessions'); + if (!Array.isArray(data)) return []; + const rows: TuiLiveSessionMetrics[] = []; + for (const entry of data) { + if (!entry || typeof entry.id !== 'string' || entry.id === '') continue; + rows.push({ + sessionId: entry.id, + ...(typeof entry.lastSubmitAt === 'number' ? { lastSubmitAt: entry.lastSubmitAt } : {}), + ...(typeof entry.inputTokens === 'number' ? { inputTokens: entry.inputTokens } : {}), + ...(typeof entry.outputTokens === 'number' ? { outputTokens: entry.outputTokens } : {}), + }); + } + return rows; + } + async fetchApprovals(): Promise { const data = await this.requestData<{ approvals?: ApprovalItem[] }>('GET', '/api/approvals'); return data?.approvals ?? []; diff --git a/test/tui/tui-app.test.ts b/test/tui/tui-app.test.ts index f2403c3c..e5652151 100644 --- a/test/tui/tui-app.test.ts +++ b/test/tui/tui-app.test.ts @@ -11,6 +11,7 @@ */ import { describe, it, expect } from 'vitest'; import { + applyLiveMetrics, applyMuxNames, buildListLines, confirmAccepts, @@ -27,9 +28,9 @@ import { tmuxRowsToSessions, tmuxSocketFromEnv, } from '../../src/tui/tui-app.js'; -import { createTuiModel } from '../../src/tui/tui-model.js'; +import { createTuiModel, stateSince } from '../../src/tui/tui-model.js'; import { glyphsFor } from '../../src/tui/tui-render.js'; -import type { TuiTmuxSession } from '../../src/tui/tui-client.js'; +import type { TuiLiveSessionMetrics, TuiTmuxSession } from '../../src/tui/tui-client.js'; import type { TuiConfirmState, TuiRow, TuiSessionRow } from '../../src/tui/tui-types.js'; const GLYPHS = glyphsFor('unicode'); @@ -339,6 +340,66 @@ describe('applyMuxNames', () => { }); }); +describe('applyLiveMetrics', () => { + const metrics: TuiLiveSessionMetrics[] = [ + { sessionId: 'aaaa1111', lastSubmitAt: 5_000, inputTokens: 900, outputTokens: 100 }, + { sessionId: 'bbbb2222', lastSubmitAt: 0, inputTokens: 0, outputTokens: 0 }, + ]; + + it('folds the turn stamp and the token totals onto the matching row', () => { + const rows = applyLiveMetrics( + [ + { sessionId: 'aaaa1111', sources: ['live'] }, + { sessionId: 'cccc3333', sources: ['live'] }, + ], + metrics + ); + expect(rows[0]).toMatchObject({ lastSubmitAt: 5_000, inputTokens: 900, outputTokens: 100 }); + // No live counterpart (a history row): nothing to fold, nothing invented. + expect(rows[1].lastSubmitAt).toBeUndefined(); + expect(rows[1].inputTokens).toBeUndefined(); + }); + + it('treats a zero as unknown, so a never-submitted session is not dated to the epoch', () => { + const [merged] = applyLiveMetrics([{ sessionId: 'bbbb2222', sources: ['live'], createdAt: 1_000 }], metrics); + expect(merged.lastSubmitAt).toBeUndefined(); + expect(merged.inputTokens).toBeUndefined(); + expect(stateSince('working', merged)).toBe(1_000); + }); + + it('copies rather than mutating its input, and survives an empty metrics list', () => { + const input: TuiSessionRow[] = [{ sessionId: 'aaaa1111', sources: ['live'] }]; + const rows = applyLiveMetrics(input, []); + expect(rows[0]).not.toBe(input[0]); + expect(rows[0].lastSubmitAt).toBeUndefined(); + expect(input[0].lastSubmitAt).toBeUndefined(); + }); + + it('orders the WORKING group by when the turn started, not by session age', () => { + // The regression this merge exists for: `old` was created a day before + // `fresh` but started its turn a minute AFTER it, so `fresh` has been + // working longer and has to lead. Without the merge both fall back to + // createdAt and `old` wins. + const unified: TuiSessionRow[] = [ + { sessionId: 'old00000', name: 'old', sources: ['live'], isWorking: true, createdAt: 1_000 }, + { sessionId: 'fresh000', name: 'fresh', sources: ['live'], isWorking: true, createdAt: 500_000 }, + ]; + const turns: TuiLiveSessionMetrics[] = [ + { sessionId: 'old00000', lastSubmitAt: 900_000 }, + { sessionId: 'fresh000', lastSubmitAt: 600_000 }, + ]; + + const before = createTuiModel(); + before.replaceSessions(unified); + expect(before.rows().map((r) => r.session.name)).toEqual(['old', 'fresh']); + + const after = createTuiModel(); + after.replaceSessions(applyLiveMetrics(unified, turns)); + expect(after.rows().map((r) => r.session.name)).toEqual(['fresh', 'old']); + expect(after.rows()[0].since).toBe(600_000); + }); +}); + describe('buildListLines', () => { it('numbers rows in the dashboard order, so `tui ` and `tui --list` agree', () => { const model = createTuiModel(); diff --git a/test/tui/tui-client.test.ts b/test/tui/tui-client.test.ts index 5e3e4029..db4cdf7f 100644 --- a/test/tui/tui-client.test.ts +++ b/test/tui/tui-client.test.ts @@ -63,6 +63,17 @@ const defaultResponder: Responder = (req, res) => { data: { sessions: [{ sessionId: 'abc', name: 'w1-codeman', sources: ['live'] }], total: 1 }, }); } + if (url === '/api/sessions' || url.startsWith('/api/sessions?')) { + // The light state, plus the two rows the narrowing has to discard. + return sendJson(res, 200, { + success: true, + data: [ + { id: 'abc', lastSubmitAt: 4000, inputTokens: 900, outputTokens: 100, status: 'busy' }, + { id: '', lastSubmitAt: 7000 }, + { id: 'zzz', lastSubmitAt: '5', inputTokens: null }, + ], + }); + } if (url.startsWith('/api/approvals')) { return sendJson(res, 200, { success: true, @@ -318,6 +329,23 @@ describe('TuiClient remaining API surface', () => { expect(recorded[0].url).toBe('/api/sessions/abc/terminal?tail=4096'); }); + it('reads the turn stamp and token counters off the light session state', async () => { + const metrics = await client().fetchLiveSessionMetrics(); + expect(recorded[0].url).toBe('/api/sessions'); + // Narrowed to the three fields, keyed by id: a row with no usable id is + // dropped rather than folded in under an empty key, and a field of the + // wrong type is left absent rather than merged as a string. + expect(metrics).toEqual([ + { sessionId: 'abc', lastSubmitAt: 4000, inputTokens: 900, outputTokens: 100 }, + { sessionId: 'zzz' }, + ]); + }); + + it('reports an unreadable session list as empty rather than throwing', async () => { + responder = (_req, res) => sendJson(res, 200, { success: true, data: { sessions: 'not an array' } }); + await expect(client().fetchLiveSessionMetrics()).resolves.toEqual([]); + }); + it('starts sessions through quick-start', async () => { const result = await client().quickStart({ caseName: 'x', mode: 'claude', parentSessionId: 'abc' }); expect(result.sessionId).toBe('new-1'); diff --git a/test/tui/tui-e2e.test.ts b/test/tui/tui-e2e.test.ts index 62c0e4bc..41caa78a 100644 --- a/test/tui/tui-e2e.test.ts +++ b/test/tui/tui-e2e.test.ts @@ -53,6 +53,13 @@ let sessions: UnifiedSessionItem[] = []; let approvals: ApprovalItem[] = []; /** Terminal buffers the preview pane polls, by session id. */ const terminals = new Map(); +/** + * The LIGHT session state (`GET /api/sessions`), which is where the turn stamp + * and the token counters come from: the unified list carries neither, so w2-beta + * is deliberately a session created 10 minutes ago whose turn started one minute + * ago. A row dated by the wrong one of those reads `10m` instead of `1m`. + */ +let liveState: Array> = []; /** Everything the TUI posted, so a test can assert on the exact body. */ const answered: Array<{ id: string; body: Record }> = []; const inputs: Array<{ sessionId: string; body: Record }> = []; @@ -184,6 +191,10 @@ function resetSessions(): void { lastActivityAt: NOW - 3_600_000, }, ]; + liveState = [ + { id: BETA, status: 'busy', lastSubmitAt: NOW - 60_000, inputTokens: 42_000, outputTokens: 3_200 }, + { id: ALPHA, status: 'idle', lastSubmitAt: NOW - 60_000 }, + ]; } let server: http.Server; @@ -264,6 +275,9 @@ beforeAll(async () => { return sendJson(res, { success: true, data: { version: '9.9.9', planUsage: PLAN_USAGE } }); } if (url.startsWith('/api/sessions/unified')) return sendJson(res, { success: true, data: { sessions } }); + if (url === '/api/sessions' || url.startsWith('/api/sessions?')) { + return sendJson(res, { success: true, data: liveState }); + } const previewFor = sessionRoute(url, 'terminal'); if (previewFor) { @@ -424,6 +438,17 @@ describe('codeman tui (under a pty)', () => { expect(index('RECENT')).toBeLessThan(index('w3-gamma')); }); + it('dates a working row by its turn, not by the session age', () => { + // w2-beta was created 10 minutes ago and pressed Enter one minute ago, and + // only `GET /api/sessions` knows the second number. `1m` proves the merge + // ran end to end; `10m` would mean the row fell back to createdAt. + const beta = rowFor(output, 'w2-beta'); + expect(beta).toMatch(/\b1m\b/); + expect(beta).not.toMatch(/\b10m\b/); + // The token column rides the same read. + expect(beta).toContain('45.2k'); + }); + it('shows the header facts and only the keys that work', () => { const lines = frameLines(output); expect(lines[0]).toContain('codeman');