Merge pull request #309 from Ark0N/fix/home-order-followups

Home-screen ordering follow-ups: live stamps, one numbering, restart-proof recency

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<void> {
     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 {

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();

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 };
   },

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}`, {

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

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]');
+  });
+});

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', () => {

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);
+  });
+});