From 4612de49c4c2fd1db48b09ba2019b91cf1ea1728 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Sat, 12 Sep 2026 01:55:11 +0200 Subject: [PATCH] =?UTF-8?q?(session-state):=20one=20owner=20per=20remote?= =?UTF-8?q?=20row=20=E2=80=94=20local-pty=20while=20attached,=20the=20adap?= =?UTF-8?q?ter=20otherwise?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #273. Measured on v0.0.76: 53 response-ready flickers in 2 min on an attached remote session, and a 20 s busy tail after the process died. The remote adapter's watch, seed and decay writers now no-op while the row is attached, and the adapter's projection does not repaint an attached row; on detach the adapter takes over from the current state, cancels the decay, clears busy at once and floors the seed window so a stale remoteActiveAt cannot re-arm it. clearUnread carries its via. --- .ai/contexts/session-state.md | 46 +++++++++- public/app.js | 2 +- public/remote-activity-ui.js | 25 +++++- public/session-activity.js | 4 +- test/remote-row-ownership.test.js | 132 ++++++++++++++++++++++++++++ test/remote-session-adapter.test.js | 3 +- 6 files changed, 203 insertions(+), 9 deletions(-) create mode 100644 test/remote-row-ownership.test.js diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index f793368b..87330d1c 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -143,10 +143,48 @@ per remote session id in `remoteSessionStates` (a `Map`, pruned in (`cli-busy`/`response-ready`) `applyActivityClasses` produces for local-pty, but computed from the adapter's own snapshot instead of the local-pty Maps. +### Row ownership: attached vs unattached (issue #273) + +An attached remote row (a tab open on it) is owned by the local-pty path — +OSC busy/idle/attention/response-ready, exactly like a local session. An +unattached remote row is owned by the remote-ssh adapter — busy from the +watch channel and seed, decayed, never response-ready (no PTY to confirm a +turn ended). Only one side may write a given row at a time; `attached` +(above) is the arbiter, not a cosmetic fact. + +Concretely, in `public/remote-activity-ui.js`: `markRemoteBusy`/ +`decayRemoteBusy` (fed by remote-watch, remote-seed and the decay timer) are +a no-op while `remoteState(id).snapshot().attached` is true, and +`projectRemoteState` refuses to paint the row at all while attached — both +guards exist because two independent things write the row: the shared +`sessionBusyState`/`responseReadySessions` Maps (`setActivity`, called from +inside `markRemoteBusy`/`decayRemoteBusy`) and the adapter's own private +`createSessionState('remote-ssh')` snapshot (painted by `applyRemoteDescriptor` +on every render, regardless of what wrote busy last). Without both guards a +descriptor-only render (no new remote event at all) can still repaint a +stale, adapter-held `busy` value over whatever the local-pty path just wrote, +which is what produced issue #273's ~53-cycle response-ready flap on an +attached, otherwise-idle row. + +`setRemoteAttached(id, false)` — the true→false handoff (tab closed or the +local ssh attach's own `pty.exit`) — clears busy at once instead of waiting +out whatever decay was in flight (previously up to 20s), and records a +per-session floor (`remoteSeedFloors`) so a +`seedRemoteActivity` call carrying the *same*, now-stale `remoteActiveAt` +(the process died; nothing refreshes it) cannot re-arm busy immediately +after the handoff. A genuinely newer `remoteActiveAt` — real activity that +resumes after detach — still arms busy normally. + +Mutation-proven: stripping the three `attached`/`snapshot.attached` guards in +`markRemoteBusy`/`decayRemoteBusy`/`projectRemoteState` turns the response- +ready-flap replay in `test/remote-row-ownership.test.js` red; disabling the +`setRemoteAttached` handoff block turns the pty.exit/20s-tail replay in the +same file red. + **`setActivity()`/the Maps in `session-activity.js` are still fed for remote -ids in parallel** (`markRemoteBusy`/`decayRemoteBusy` call both). Two readers -were not migrated onto the adapter in this step, so removing the dual-feed -would regress them: +ids in parallel** (`markRemoteBusy`/`decayRemoteBusy` call both, when not +attached). Two readers were not migrated onto the adapter in this step, so +removing the dual-feed would regress them: - `sidebar.js`'s `buildSessionItem` reads `sessionBusyState`/ `responseReadySessions`/`attentionSessions` directly at initial paint. - `app.js`'s grid-card busy dot (`updateRunningIndicators`'s `gridCards` @@ -451,7 +489,7 @@ the others assert on the dot/slot element itself, only on row classes and | `busy` / `attention` (OSC 0 / 9) | yes | never | wired via the watch channel (transcript writes), not OSC — OSC-while-attached is not wired | | `transcriptTouched(at)` | yes | yes (only signal) — `onLocalTranscriptActivity` | yes — `onRemoteActivityEvent`/`markRemoteBusy` | | `descriptorStatus(status, at)` / `liveness` | yes | yes — `seedLocalTranscriptDescriptor`, from `sessionMap`'s `status`/`statusUpdatedAt` (see "The local-transcript adapter" above for why this widens the issue's original "no (no live CLI)") | yes (`main.js:539` → `applyRemoteDescriptor`) | -| `attached` | reserved, unused | reserved, unused | yes — `setRemoteAttached`, driven by the per-row `activePtyIds` transition | +| `attached` | reserved, unused | reserved, unused | yes — `setRemoteAttached`, driven by the per-row `activePtyIds` transition; also the row-ownership arbiter since #273 (see "Row ownership" above) | | `subagentSpawned` / `subagentCompleted` | yes — via `detectSubagentTransitions()` IPC | yes (issue #247) — `onLocalTranscriptSubagentActivity`, gated on the parent having no PTY | yes (issue #247) — `onRemoteActivityEvent({kind:'subagent'})`, attributed by `subagentParentFromParts()` | An adapter without a PTY must never claim `waitingForInput` or `responseReady` diff --git a/public/app.js b/public/app.js index 9b0b2229..bbbb721b 100644 --- a/public/app.js +++ b/public/app.js @@ -305,7 +305,7 @@ function trackActivity(sessionId, data) { } function clearNotifications(sessionId) { - clearUnread(sessionId); + clearUnread(sessionId, 'clearNotifications'); if (window.ATRACE && attentionSessions.has(sessionId)) window.atrace('store.mutate', sessionId, { map: 'attentionSessions', op: 'delete', from: true, to: false, fn: 'clearNotifications' }); attentionSessions.delete(sessionId); setNeedsAttention(sessionItemEl(sessionId), false); diff --git a/public/remote-activity-ui.js b/public/remote-activity-ui.js index 7fc72e62..e3b6d282 100644 --- a/public/remote-activity-ui.js +++ b/public/remote-activity-ui.js @@ -10,6 +10,9 @@ const remoteAgentsDecayTimers = new Map(); // remote-ssh adapter: one persistent state per remote session id — see .ai/contexts/session-state.md const remoteSessionStates = new Map(); +// seed staleness floor per session, set at the attached handoff (#273) — see .ai/contexts/session-state.md +const remoteSeedFloors = new Map(); + function remoteState(sessionId) { let state = remoteSessionStates.get(sessionId); if (!state) { @@ -19,8 +22,11 @@ function remoteState(sessionId) { return state; } +// An attached row is owned by the local-pty path (#273) — see .ai/contexts/session-state.md function projectRemoteState(sessionId) { - applyStateClasses(sessionId, remoteState(sessionId).snapshot()); + const snapshot = remoteState(sessionId).snapshot(); + if (snapshot.attached) return; + applyStateClasses(sessionId, snapshot); } function clearRemoteActivityTimer(sessionId) { @@ -40,8 +46,10 @@ function clearRemoteAgentsTimer(sessionId) { } // Maps still fed in parallel for sidebar initial paint and grid dot — see session-state.md "migration status" +// An attached row is owned by the local-pty path (#273): a no-op here. function markRemoteBusy(sessionId, via, at) { const state = remoteState(sessionId); + if (state.snapshot().attached) return; state.apply({ type: 'transcriptTouched', at: at || Date.now(), source: via }); state.apply({ type: 'busy', active: true }); setActivity(sessionId, true, via); @@ -49,8 +57,10 @@ function markRemoteBusy(sessionId, via, at) { } // silence is "stopped writing", not "response ready" — see .ai/contexts/session-cache.md ("Remote hosts — busy spinner") +// An attached row is owned by the local-pty path (#273): a no-op here. function decayRemoteBusy(sessionId) { const state = remoteState(sessionId); + if (state.snapshot().attached) return; state.apply({ type: 'busy', active: false, armReady: false }); setActivity(sessionId, false, 'remote-decay', { armReady: false }); projectRemoteState(sessionId); @@ -96,6 +106,9 @@ function pruneRemoteActivityTimers() { for (const sessionId of remoteSessionStates.keys()) { if (!sessionItemEl(sessionId)) remoteSessionStates.delete(sessionId); } + for (const sessionId of remoteSeedFloors.keys()) { + if (!sessionItemEl(sessionId)) remoteSeedFloors.delete(sessionId); + } } function onRemoteActivityEvent(payload) { @@ -124,10 +137,18 @@ function applyRemoteDescriptor(session) { } // attached = a PTY/ssh attach exists for this row (activePtyIds signal from app.js) +// true->false handoff clears busy at once and floors stale reseeds (#273) — see .ai/contexts/session-state.md function setRemoteAttached(sessionId, attached) { if (!attached && !remoteSessionStates.has(sessionId)) return; // nothing recorded yet, nothing to clear const state = remoteState(sessionId); + const wasAttached = state.snapshot().attached; state.apply({ type: 'attached', value: attached }); + if (wasAttached && !attached) { + clearRemoteActivityTimer(sessionId); + remoteSeedFloors.set(sessionId, Date.now()); + state.apply({ type: 'busy', active: false, armReady: false }); + setActivity(sessionId, false, 'remote-attach-handoff', { armReady: false }); + } projectRemoteState(sessionId); } @@ -151,6 +172,8 @@ function seedRemoteActivity(session) { if (!Number.isFinite(session.remoteActiveAt)) return; const sessionId = session.sessionId; + const floor = remoteSeedFloors.get(sessionId); + if (floor !== undefined && session.remoteActiveAt <= floor) return; // stale — see setRemoteAttached, #273 const remaining = session.remoteActiveAt + PIP_DECAY_MS - Date.now(); if (remaining <= 0) return; markRemoteBusy(sessionId, 'remote-seed', session.remoteActiveAt); diff --git a/public/session-activity.js b/public/session-activity.js index 237b8024..ec8e271c 100644 --- a/public/session-activity.js +++ b/public/session-activity.js @@ -59,8 +59,8 @@ function setActivity(sessionId, active, via, opts) { applyActivityClasses(sessionId); } -function clearUnread(sessionId) { - if (window.ATRACE && responseReadySessions.has(sessionId)) window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'delete', from: true, to: false, fn: 'clearUnread' }); +function clearUnread(sessionId, via) { + if (window.ATRACE && responseReadySessions.has(sessionId)) window.atrace('store.mutate', sessionId, { map: 'responseReadySessions', op: 'delete', from: true, to: false, fn: 'clearUnread', via }); responseReadySessions.delete(sessionId); applyActivityClasses(sessionId); } diff --git a/test/remote-row-ownership.test.js b/test/remote-row-ownership.test.js new file mode 100644 index 00000000..c9d452bf --- /dev/null +++ b/test/remote-row-ownership.test.js @@ -0,0 +1,132 @@ +// Issue #273: an attached remote row is owned by the local-pty path. The +// remote-ssh adapter (remote-seed/remote-watch/remote-decay) must not write +// sessionBusyState/responseReadySessions or repaint the row while attached, +// and a detach/pty.exit handoff must clear busy at once instead of waiting +// out the 20s decay. See .ai/contexts/session-state.md ("The remote-ssh +// adapter"). Same eval-in-jsdom technique as test/remote-session-adapter.test.js. + +'use strict'; +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { JSDOM } = require('jsdom'); + +const STATE_SRC = path.join(__dirname, '..', 'public', 'session-state.js'); +const DOM_SRC = path.join(__dirname, '..', 'public', 'session-activity-dom.js'); +const ACTIVITY_SRC = path.join(__dirname, '..', 'public', 'session-activity.js'); +const SRC = path.join(__dirname, '..', 'public', 'remote-activity-ui.js'); + +function setup(sessionIds = ['s1']) { + const items = sessionIds + .map(id => `
`) + .join(''); + const dom = new JSDOM(`${items}`, + { url: 'http://localhost/', runScripts: 'outside-only' }); + const { window } = dom; + + Object.defineProperty(window, 'activeSessionId', { value: null, writable: true, configurable: true }); + + let onRemoteActivityCb = null; + Object.defineProperty(window, 'api', { + value: { onRemoteActivity: (cb) => { onRemoteActivityCb = cb; } }, + writable: true, configurable: true, + }); + + const scheduled = []; + let nextId = 1; + Object.defineProperty(window, 'setTimeout', { + value: (fn, ms) => { + const handle = { id: nextId++, fn, ms, cleared: false }; + scheduled.push(handle); + return handle.id; + }, + writable: true, configurable: true, + }); + Object.defineProperty(window, 'clearTimeout', { + value: (id) => { + const h = scheduled.find(s => s.id === id); + if (h) h.cleared = true; + }, + writable: true, configurable: true, + }); + + const ctx = dom.getInternalVMContext(); + vm.runInContext(fs.readFileSync(STATE_SRC, 'utf8'), ctx, { filename: STATE_SRC }); + vm.runInContext(fs.readFileSync(DOM_SRC, 'utf8'), ctx, { filename: DOM_SRC }); + vm.runInContext(fs.readFileSync(ACTIVITY_SRC, 'utf8'), ctx, { filename: ACTIVITY_SRC }); + vm.runInContext(fs.readFileSync(SRC, 'utf8'), ctx, { filename: SRC }); + + const call = (fnName, ...args) => vm.runInContext( + `${fnName}(${args.map((a) => JSON.stringify(a)).join(',')})`, ctx + ); + const read = (expr) => vm.runInContext(expr, ctx); + + return { + window, + document: window.document, + item: (id) => window.document.querySelector(`.session-item[data-session-id="${id}"]`), + emit: (payload) => onRemoteActivityCb(payload), + snapshot: (id) => vm.runInContext(`remoteState(${JSON.stringify(id)}).snapshot()`, ctx), + setRemoteAttached: (id, attached) => call('setRemoteAttached', id, attached), + seedRemoteActivity: (session) => call('seedRemoteActivity', session), + reconcileBusyState: (entries, sinceSeq) => vm.runInContext( + `reconcileBusyState(${JSON.stringify(entries)}, ${JSON.stringify(sinceSeq)})`, ctx + ), + sessionBusyState: read('sessionBusyState'), + responseReadySessions: read('responseReadySessions'), + scheduled, + pending: () => scheduled.filter(h => !h.cleared), + destroy: () => window.close(), + }; +} + +// Symptom 1 (trace-2026-09-12.md, session 1cc1ca36): reconcileBusyState (the +// local-pty path, from the attached PTY's OSC titles) sets the row idle and +// arms response-ready; remote-seed/remote-watch must not then delete it and +// force busy back on. +test('symptom 1 (#273): a remote-watch event must not overwrite an attached row the local-pty path just idled', () => { + const t = setup(['s1']); + t.setRemoteAttached('s1', true); + + // The local-pty path, via reconcileBusyState: busy, then idle+response-ready. + t.reconcileBusyState([{ sessionId: 's1', busy: true }], 0); + t.reconcileBusyState([{ sessionId: 's1', busy: false }], 999999); + assert.equal(t.responseReadySessions.has('s1'), true, 'precondition: local-pty armed response-ready'); + assert.ok(t.item('s1').classList.contains('response-ready'), 'precondition: painted response-ready'); + + // remote-watch fires next (a transcript write the host reported). + t.emit({ sessionId: 's1', at: Date.now() }); + + assert.equal(t.responseReadySessions.has('s1'), true, 'remote-watch must not delete responseReadySessions while attached'); + assert.equal(t.sessionBusyState.get('s1'), false, 'remote-watch must not force busy while attached'); + assert.ok(t.item('s1').classList.contains('response-ready'), 'the row must stay response-ready, not flap back to busy'); + assert.ok(!t.item('s1').classList.contains('cli-busy')); + t.destroy(); +}); + +// Symptom 2 (trace-2026-09-12.md, throwaway sessions): pty.exit of a remote +// attach fires almost immediately when the host process dies, but the row +// stayed busy for ~20s until remote-decay finally cleared it. +test('symptom 2 (#273): pty.exit of a remote attach clears busy at once, no 20s tail', () => { + const t = setup(['s1']); + // Busy accrues on the remote-ssh side (with a live decay timer) before the + // tab attaches to it — the adapter's own internal state is left busy. + t.emit({ sessionId: 's1', at: Date.now() }); + t.setRemoteAttached('s1', true); + assert.ok(t.item('s1').classList.contains('cli-busy'), 'precondition: row shows busy'); + + const remoteActiveAt = Date.now() - 5000; // last known activity before the death, captured before the handoff + t.setRemoteAttached('s1', false); // pty.exit of the local ssh attach + + assert.equal(t.snapshot('s1').busy, false, 'busy must clear at the handoff instant, not wait for the 20s decay'); + assert.ok(!t.item('s1').classList.contains('cli-busy'), 'the row must stop looking busy within one render'); + assert.equal(t.pending().length, 0, 'no decay timer should be left pending to repaint busy later'); + + // A subsequent render re-seeds from the session's own (now-stale, unchanged + // since the process died) remoteActiveAt — must not re-arm the row busy. + t.seedRemoteActivity({ sessionId: 's1', remoteAlias: 'planificator', remoteActiveAt }); + assert.ok(!t.item('s1').classList.contains('cli-busy'), 'a stale seed window must not re-arm busy after the handoff'); + t.destroy(); +}); diff --git a/test/remote-session-adapter.test.js b/test/remote-session-adapter.test.js index 44ff2c02..8b417abb 100644 --- a/test/remote-session-adapter.test.js +++ b/test/remote-session-adapter.test.js @@ -138,10 +138,11 @@ test('applyRemoteDescriptor is a no-op for a session with no remoteAlias', () => t.destroy(); }); -test('setRemoteAttached toggles the attached flag without affecting busy/liveness', () => { +test('setRemoteAttached(true) toggles the attached flag without affecting busy/liveness; setRemoteAttached(false) hands ownership back — see test/remote-row-ownership.test.js (#273) for the full handoff', () => { const t = setup(['s1']); t.setRemoteAttached('s1', true); assert.equal(t.snapshot('s1').attached, true); + assert.equal(t.snapshot('s1').busy, false, 'attaching must not touch busy'); t.setRemoteAttached('s1', false); assert.equal(t.snapshot('s1').attached, false);