diff --git a/src/session.ts b/src/session.ts index 17aac27c..42dcac62 100644 --- a/src/session.ts +++ b/src/session.ts @@ -135,6 +135,14 @@ const MUX_STARTUP_DELAY_MS = 300; /** Delay before declaring session idle after last output (2 seconds) */ const IDLE_DETECTION_DELAY_MS = 2000; +// How long after construction a RECOVERED session's wire activity stamp keeps +// its restored previous-run value. Recovery attaches every pane at boot and the +// attach repaint arrives as ordinary PTY output; without this window that +// repaint would overwrite every restored stamp within the same second, which is +// exactly the restart flattening the restore exists to prevent. Real actions +// (input, task assignment, respawn) always stamp through it. +const WIRE_ACTIVITY_SETTLE_MS = 15_000; + // Note: Auto-compact/clear timing constants moved to session-auto-ops.ts /** Graceful shutdown delay when stopping session (100ms) */ @@ -392,6 +400,12 @@ export class Session extends EventEmitter { private _textOutput = new BufferAccumulator(MAX_TEXT_OUTPUT_SIZE, TEXT_OUTPUT_TRIM_SIZE); private _errorBuffer: string = ''; private _lastActivityAt: number; + // Display twin of _lastActivityAt, reported by toState()/the getter. It can + // lag behind on recovery: the restored previous-run stamp survives the attach + // repaint (see _markActivity), so a restart does not flatten the home + // screens' quiet ordering. Idle detection never reads it. + private _wireActivityAt: number; + private _wireActivitySettleUntil: number; private _claudeSessionId: string | null = null; private _totalCost: number = 0; private _messages: ClaudeMessage[] = []; @@ -592,6 +606,8 @@ export class Session extends EventEmitter { attachmentHistory?: SessionAttachmentHistoryItem[]; /** Restored wall-clock ms of the pane's last Enter (see `lastSubmitAt`). */ lastSubmitAt?: number; + /** Restored wall-clock ms of the pane's last output (recovery only; see `_wireActivityAt`). */ + lastActivityAt?: number; /** Remote execution metadata for sessions launched through SSH inside local tmux. */ remote?: SessionRemote; /** Docker execution metadata for sessions launched inside a container via local tmux. */ @@ -620,9 +636,18 @@ export class Session extends EventEmitter { // NOW, not `createdAt`: recovery passes the ORIGINAL creation time of a // days-old tmux session, and seeding last-activity from it would report a // freshly re-attached pane as having been silent for days, which the idle - // confirmation reads as "already quiet" and the home screens print as its - // idle duration. For a genuinely new session the two are the same instant. + // confirmation reads as "already quiet". For a genuinely new session the + // two are the same instant. this._lastActivityAt = Date.now(); + // The WIRE copy of the stamp is allowed to be older: recovery threads the + // previous run's value so a restart does not flatten the home screens' + // most-recently-quiet ordering (every stamp otherwise resets to boot time, + // and the attach repaint re-bumps the rest within the same second). The + // settle window in _markActivity() carries the restored value through that + // repaint; the private stamp above stays boot-anchored because the idle + // confirmation reads it as "how long has the pane been quiet". + this._wireActivityAt = config.lastActivityAt || Date.now(); + this._wireActivitySettleUntil = config.lastActivityAt ? Date.now() + WIRE_ACTIVITY_SETTLE_MS : 0; // Set claudeSessionId — when resuming, the Claude conversation ID is the resumed one. this._claudeSessionId = config.resumeSessionId || this.id; // Restored from state.json on boot recovery. start() resets _claudeSessionId @@ -794,7 +819,21 @@ export class Session extends EventEmitter { } get lastActivityAt(): number { - return this._lastActivityAt; + return this._wireActivityAt; + } + + /** + * Stamp activity NOW. The private stamp (idle detection's "how long has the + * pane been quiet") always moves; the wire stamp holds its restored value + * through the post-recovery attach-repaint window unless the activity is a + * real action (input, task assignment, respawn), which always writes through. + */ + private _markActivity(realAction = false): void { + this._lastActivityAt = Date.now(); + if (realAction || Date.now() >= this._wireActivitySettleUntil) { + this._wireActivityAt = this._lastActivityAt; + this._wireActivitySettleUntil = 0; + } } get claudeSessionId(): string | null { @@ -1219,7 +1258,9 @@ export class Session extends EventEmitter { parentSessionId: this._parentSessionId, currentTaskId: this._currentTaskId, createdAt: this.createdAt, - lastActivityAt: this._lastActivityAt, + // The wire twin, not the private stamp: it survives the post-recovery + // attach repaint, so the home screens' quiet ordering survives a restart. + lastActivityAt: this._wireActivityAt, name: this._name, mode: this.mode, autoClearEnabled: this._autoOps.autoClearEnabled, @@ -1585,7 +1626,7 @@ export class Session extends EventEmitter { // BufferAccumulator handles auto-trimming when max size exceeded this._terminalBuffer.append(data); - this._lastActivityAt = Date.now(); + this._markActivity(); this.emit('terminal', data); this.emit('output', data); } @@ -2484,7 +2525,7 @@ export class Session extends EventEmitter { this._messages = []; this._lineBuffer = ''; this._altScreenSeqCarry = ''; - this._lastActivityAt = Date.now(); + this._markActivity(true); } private _clearAllTimers(): void { @@ -3083,7 +3124,7 @@ export class Session extends EventEmitter { // Legacy method for sending input - wraps runPrompt async sendInput(input: string): Promise { this._status = 'busy'; - this._lastActivityAt = Date.now(); + this._markActivity(true); this.runPrompt(input).catch((err) => { const errorMsg = getErrorMessage(err); // Clean up task state so the task queue doesn't get stuck @@ -3091,7 +3132,7 @@ export class Session extends EventEmitter { const taskId = this._currentTaskId; this._currentTaskId = null; this._status = 'idle'; - this._lastActivityAt = Date.now(); + this._markActivity(true); this.emit('taskError', taskId, errorMsg); } else { this._status = 'idle'; @@ -3252,13 +3293,13 @@ export class Session extends EventEmitter { this._textOutput.clear(); this._errorBuffer = ''; this._messages = []; - this._lastActivityAt = Date.now(); + this._markActivity(true); } clearTask(): void { this._currentTaskId = null; this._status = 'idle'; - this._lastActivityAt = Date.now(); + this._markActivity(true); } getOutput(): string { diff --git a/src/web/public/app.js b/src/web/public/app.js index f3b502b9..72af7bba 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -1116,12 +1116,17 @@ class CodemanApp { if (digitMatch) { const idx = parseInt(digitMatch[1], 10) - 1; // Sessions occupy 1..N and web tabs continue from N+1, matching the - // numbers actually painted on the tabs. - if (idx < this.sessionOrder.length) { + // numbers actually painted on the tabs. Resolve through the same + // live-session projection the render paints: sessionOrder can + // transiently hold a dead id (delete raced against the order sync), + // and raw indexing then names the wrong tab for every key to its + // right, web tabs included. + const live = this.sessionOrder.filter((id) => this.sessions.has(id)); + if (idx < live.length) { e.preventDefault(); - this.selectSession(this.sessionOrder[idx]); + this.selectSession(live[idx]); } else { - const webIdx = idx - this.sessionOrder.length; + const webIdx = idx - live.length; const webId = (this.webviewOrder || [])[webIdx]; if (webId) { e.preventDefault(); diff --git a/src/web/public/mobile-overview.js b/src/web/public/mobile-overview.js index eef976c3..a8d2f969 100644 --- a/src/web/public/mobile-overview.js +++ b/src/web/public/mobile-overview.js @@ -118,14 +118,16 @@ Object.assign(CodemanApp.prototype, { * A WORKING pane is the opposite: it repaints about once a second, so its * last-activity stamp is always "now" and would report every running turn as * 0m. The turn's own start is the pane's last Enter (`lastSubmitAt`), which is - * persisted server-side and therefore survives a Codeman restart. A session - * that has never submitted has no anchor at all, and gets no stamp rather than - * a made-up one. + * persisted server-side and therefore survives a Codeman restart. A working + * session with NO submit stamp falls back to `lastActivityAt`, because that is + * exactly what `sessionActivityAnchor` (constants.js) sorts it by: a row must + * never be ranked by a number it does not show. * * @returns {{key: string, at: number}|null} */ _mobileOverviewSince(state, session) { - const at = state === 'working' ? Number(session.lastSubmitAt) || 0 : Number(session.lastActivityAt) || 0; + const activeAt = Number(session.lastActivityAt) || 0; + const at = state === 'working' ? Number(session.lastSubmitAt) || activeAt : activeAt; if (!at) return null; return { key: MOBILE_OVERVIEW_SINCE_LABEL[state] || state, at }; }, diff --git a/src/web/routes/hook-event-routes.ts b/src/web/routes/hook-event-routes.ts index d59d7c39..440b18c9 100644 --- a/src/web/routes/hook-event-routes.ts +++ b/src/web/routes/hook-event-routes.ts @@ -148,6 +148,11 @@ export function registerHookEventRoutes( ...safeData, ...(approvalId && { approvalId }), }); + // Full state ride-along, same shape as the working/idle handlers: the home + // screens rank the blocked group on lastActivityAt, and without this a + // permission prompt raised after page load kept ranking by whatever stamp + // the browser loaded with. Debounced, so a hook burst costs one broadcast. + ctx.broadcastSessionStateDebounced(sessionId); // Send push notifications for hook events ctx.sendPushNotifications(`hook:${event}`, { diff --git a/src/web/server.ts b/src/web/server.ts index 62f1acec..49a4a503 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -2657,6 +2657,11 @@ export class WebServer extends EventEmitter { // the launch conversation until the user types again, even though // the re-attached CLI is on a post-`/clear` one. lastSubmitAt: savedState?.lastSubmitAt, + // The pane's last output, previous run's value. Without it every + // restart restamped all sessions "now" (constructor + the attach + // repaint within the same second), flattening the home screens' + // most-recently-quiet ordering to tab order after each deploy. + lastActivityAt: savedState?.lastActivityAt, // Remote SSH metadata must round-trip on recovery: without it the // attach cwd falls back to the (nonexistent-locally) remote path and // respawn rebuilds a LOCAL command, breaking the pane and silently diff --git a/test/home-sessions.test.ts b/test/home-sessions.test.ts index fd5e19ad..8ef9d8c8 100644 --- a/test/home-sessions.test.ts +++ b/test/home-sessions.test.ts @@ -270,3 +270,75 @@ describe('home sessions column: wiring', () => { expect(html.indexOf('src="mobile-overview.js"')).toBeGreaterThan(html.indexOf('src="constants.js"')); }); }); + +describe('home screens: one order, one numbering', () => { + it('produces the same order on the rail and the phone overview for one input', () => { + // Both surfaces claim to share CodemanSessionOrder. Nothing used to assert + // they actually produce one order for one input, so a future local sort in + // either builder would silently split them. The rail is one list; the phone + // splits NEEDS YOU / CURRENT, so rail order must equal the concatenation. + const fixture = [ + { id: 'blocked-new', lastActivityAt: 5_000 }, + { id: 'idle-old', lastActivityAt: 3_000 }, + { id: 'run-new', status: 'busy', lastSubmitAt: 8_000, lastActivityAt: 9_500 }, + { id: 'blocked-old', lastActivityAt: 1_000 }, + { id: 'run-old', status: 'busy', lastSubmitAt: 2_000, lastActivityAt: 9_600 }, + { id: 'idle-new', lastActivityAt: 9_000 }, + ]; + const pendingHooks = new Map([ + ['blocked-new', new Set(['permission_prompt'])], + ['blocked-old', new Set(['permission_prompt'])], + ]); + const sessionOrder = fixture.map((s) => s.id); + const app = loadHomeSessionsApp({ + sessions: sessionMap(fixture), + sessionOrder, + cases: CASES, + pendingHooks, + }); + + const railIds = app.buildHomeSessionRows().map((r: any) => r.id); + const model = app.buildMobileOverviewModel({ + sessions: app.sessions, + cases: CASES, + sessionOrder, + pendingHooks, + }); + const phoneIds = [...model.needsYou, ...model.current].map((r: any) => r.id); + + expect(railIds).toEqual(phoneIds); + // And the shared order is the documented one: blocked longest-first, then + // running longest-first, then quiet newest-first. + expect(railIds).toEqual(['blocked-old', 'blocked-new', 'run-old', 'run-new', 'idle-new', 'idle-old']); + }); + + it('numbers rows over the LIVE projection when sessionOrder holds a dead id', () => { + // sessionOrder can transiently contain a deleted session (delete raced the + // order sync). The strip paints numbers over live sessions only, and the + // Alt+digit handler resolves through the same projection, so the rail must + // number alpha=1, beta=2 with no hole where the ghost sits. + const app = loadHomeSessionsApp({ + sessions: sessionMap([{ id: 'alpha' }, { id: 'beta' }]), + sessionOrder: ['ghost', 'alpha', 'beta'], + cases: CASES, + }); + expect(app.buildHomeSessionRows().map((r: any) => [r.id, r.orderIndex])).toEqual([ + ['alpha', 0], + ['beta', 1], + ]); + }); + + it('Alt+digit resolves through the live-session projection in app.js', () => { + // Static guard for the handler half of the invariant above: the digit + // branch must filter sessionOrder against live sessions before indexing, + // for sessions AND for the web-tab continuation. + const appJs = readFileSync(resolve(PUBLIC, 'app.js'), 'utf8'); + const start = appJs.indexOf('^Digit([1-9])$'); + expect(start).toBeGreaterThan(-1); + const branch = appJs.slice(start, start + 1200); + expect(branch).toContain('this.sessionOrder.filter((id) => this.sessions.has(id))'); + expect(branch).toContain('idx < live.length'); + expect(branch).toContain('idx - live.length'); + expect(branch).not.toContain('this.sessionOrder[idx]'); + }); +}); diff --git a/test/mobile-overview.test.ts b/test/mobile-overview.test.ts index 222674d0..3059a2d7 100644 --- a/test/mobile-overview.test.ts +++ b/test/mobile-overview.test.ts @@ -277,15 +277,28 @@ describe('mobile overview model', () => { expect(rows.i.createdAt).toBe(now - 7200_000); }); - it('leaves the stamp off rather than inventing an anchor', () => { + it('falls back to the sort anchor for a working row with no submit stamp', () => { + // A session that has never submitted has no turn start to measure from, but + // `sessionActivityAnchor` still RANKS it by lastActivityAt. The stamp must + // show that same number rather than nothing: a row sorted by a value it + // does not display reads as randomly placed. + const now = Date.now(); const app = loadOverviewApp(); const model = app.buildMobileOverviewModel({ - // A session that has never submitted has no turn start to measure from. - sessions: [session({ id: 'w', status: 'busy', lastActivityAt: Date.now() })], + sessions: [session({ id: 'w', status: 'busy', lastActivityAt: now })], + cases: CASES, + }); + expect(model.current[0].since).toEqual({ key: 'working', at: now }); + expect(model.current[0].createdAt).toBe(0); + }); + + it('still leaves the stamp off when there is no anchor at all', () => { + const app = loadOverviewApp(); + const model = app.buildMobileOverviewModel({ + sessions: [session({ id: 'w', status: 'busy' })], cases: CASES, }); expect(model.current[0].since).toBeNull(); - expect(model.current[0].createdAt).toBe(0); }); it('formats a moment as "ago" and a span as a bare duration', () => { diff --git a/test/session-activity.test.ts b/test/session-activity.test.ts index a95337f9..f67f45e3 100644 --- a/test/session-activity.test.ts +++ b/test/session-activity.test.ts @@ -246,3 +246,45 @@ describe('Session interactive idle detection', () => { expect(events).toEqual([]); }); }); + +describe('wire activity stamp across recovery', () => { + // The stamp both home screens sort the quiet group on. Recovery restores the + // previous run's value, and the settle window keeps the boot attach repaint + // (ordinary PTY output, arriving within seconds of construction) from + // restamping every session "now": measured live, a restart left 17 of 17 + // sessions with an identical lastActivityAt, which flattens the ordering to + // tab order after every deploy. + const OLD = 1_700_000_000_000; + const restored = () => + new Session({ workingDir: '/tmp', mode: 'claude', lastActivityAt: OLD } as ConstructorParameters< + typeof Session + >[0]); + + it('restores the previous-run stamp and holds it through attach-repaint output', () => { + const session = restored(); + expect(session.lastActivityAt).toBe(OLD); + (session as unknown as SessionInternals)._handleTerminalOutput('attach repaint bytes'); + expect(session.lastActivityAt).toBe(OLD); + expect(session.toState().lastActivityAt).toBe(OLD); + }); + + it('a real action writes through the settle window', () => { + const session = restored(); + session.assignTask('t1'); + expect(session.lastActivityAt).toBeGreaterThan(OLD); + }); + + it('output after the window moves the stamp normally', () => { + const session = restored(); + (session as unknown as { _wireActivitySettleUntil: number })._wireActivitySettleUntil = Date.now() - 1; + (session as unknown as SessionInternals)._handleTerminalOutput('real output'); + expect(session.lastActivityAt).toBeGreaterThan(OLD); + }); + + it('a fresh session has no window: first output stamps immediately', () => { + const before = Date.now(); + const session = new Session({ workingDir: '/tmp', mode: 'claude' }); + (session as unknown as SessionInternals)._handleTerminalOutput('x'); + expect(session.lastActivityAt).toBeGreaterThanOrEqual(before); + }); +});