diff --git a/.ai/contexts/session-state.md b/.ai/contexts/session-state.md index 0b2628ae..80acd851 100644 --- a/.ai/contexts/session-state.md +++ b/.ai/contexts/session-state.md @@ -189,6 +189,88 @@ per remote session id in `remoteSessionStates` (a `Map`, pruned in `projectLocalPtyState`, see "The local-pty adapter" below), just fed from the remote-ssh adapter's own snapshot instead. +### A parent's busy decay shortens while a subagent is running (issue #284) + +A Task-tool invocation typically appends to the parent's own top-level +transcript (recording the tool_use/tool_result around the spawn) at almost +the same moment it appends to the subagent's own file. On the remote-ssh +adapter this used to mean a plain `busy` edge could win the icon rung over +`agentsBusy` for its full 20s decay, even though the top-level agent was +really just idle waiting on the subagent — visibly different from a local-pty +row, which reflects the OSC-driven busy edge instantly and clears it just as +fast. + +**First attempt (reverted in review): a 3s coincidence window keyed off the +`agentsBusy` false→true edge.** Two adversarial-review findings killed it. +First, a genuinely busy parent that spawns a subagent lost `busy` outright: +every touch inside the post-spawn window was swallowed and never re-applied, +so a parent writing every ~1s could sit at `agentsBusy` even while it was +still producing output itself. Second, the edge only fires once — a second +subagent spawned while the first was still running had no edge to key off, +`remoteSubagentSpawnAt` stayed stale, and its coincident parent-file touch +armed the full 20s `busy` decay again: the original bug, for the ordinary +sequential-agents case. + +**Current design: no window, no edge — the decay *length* itself depends on +`agentsBusy`.** `remoteBusyDecayMs(sessionId)` (`public/remote-activity-ui.js`) +returns `SUBAGENT_PARENT_DECAY_MS` (3000ms) when `agentsBusy` is true, +`PIP_DECAY_MS` (20000ms) otherwise; `onRemoteActivityEvent`'s plain-touch +branch arms the decay with whichever value applies at that instant. Every +`busy` touch still applies `busy:true` unconditionally — nothing is ever +swallowed or synchronously cleared. `markRemoteSubagentBusy` runs on **every** +subagent touch, not only the first: if a busy decay is currently pending with +more than `SUBAGENT_PARENT_DECAY_MS` left, it reschedules that pending timer +down to 3s from now (`remoteActivityDecayRemaining`, tracked as `fireAt` on +`remoteActivityDecayTimers`'s entries) — it never touches `busy` itself, +only how soon its decay fires. The outcome: a parent genuinely still writing +while a subagent runs re-touches at the ~1s IPC throttle, always inside the +3s window, so `busy` never lapses — it keeps the same violet-tinted spinner a +local row would show in the same situation +(`.has-busy-agents .session-icon--busy::before`). A parent that only +bookends the spawn (one touch at spawn, one at completion) has its single +post-spawn touch decay in 3s instead of 20s, landing on `agentsBusy` — the +same rung as the local row — within a few seconds instead of up to 20. A +parent with **no** subagent keeps the full 20s decay unchanged (a long silent +tool call must not read as idle). `PRIORITY` in `session-state.js` is +unchanged — this only changes which decay duration a renderer-side timer +picks, never the priority ladder. + +**`seedRemoteActivity` (cold-start / rebuild paint) is migrated too, not just +the live-touch path.** Its own arm used to stay `PIP_DECAY_MS`-based +regardless of `agentsBusy`, computing `remaining = remoteActiveAt + +PIP_DECAY_MS - now`. `renderProjects()` calls `seedRemoteActivity` on every +full sidebar rebuild, and a rebuild is itself commonly provoked by the +subagent's own writes — so a parent whose short decay had *already* fired +got put back on the animated busy rung for up to 20s at the very next +rebuild (measured: touch t=0, spawn t=1000 reschedules the decay to fire at +t=4000, busy correctly false at t=4000, then a rebuild at t=5000 re-armed +busy for another ~15s). Fixed by using `remoteBusyDecayMs(sessionId)` in that +same arithmetic (`remaining = remoteActiveAt + remoteBusyDecayMs(sessionId) - +now`) — with `agentsBusy` true the seed window is 3s from `remoteActiveAt` +instead of 20s, so a seed older than that does nothing, exactly mirroring +what the live-touch path already does. A seed with `agentsBusy` false is +byte-identical to before (`remoteBusyDecayMs` returns `PIP_DECAY_MS`), which +is why `test/dom-sidebar-remote-activity-pip.test.js` (no subagent in any of +its fixtures) needed no changes. + +**Accepted trade-off, not a bug: with a subagent running, a genuinely busy +parent can visibly flap between the `busy` and `agentsBusy` rungs.** If the +parent's own transcript stays silent for more than `SUBAGENT_PARENT_DECAY_MS` +(3s) — a long tool call — its `busy` decays to `agentsBusy` until the next +write brings it back to `busy`. Both rungs are violet-tinted +(`.has-busy-agents .session-icon--busy::before` / `.session-icon--agents-busy::before`), +so the visible change is animation only (spinner vs. static diamond), not a +color or row-class change. This is deliberate: the alternative — decaying at +the full `PIP_DECAY_MS` (20s) whenever `agentsBusy` is true — is exactly +issue #284's original symptom, a parent idling on `busy` long after it +stopped producing output. **The local-pty row has no equivalent gap**: its +busy signal is the OSC title stream, edge-triggered on the CLI's own +idle/busy transitions rather than decayed from silence, so it never flaps +while genuinely idle-but-subagent-running. This asymmetry between local and +remote is a known, accepted consequence of the remote-ssh adapter having no +edge-triggered signal to key off — only transcript touches — not an +oversight to fix later. + ### Row ownership: attached vs unattached (issue #273) An attached remote row (a tab open on it) is owned by the local-pty path — diff --git a/.ai/contexts/subagent-observability.md b/.ai/contexts/subagent-observability.md index 976eeadf..7393a188 100644 --- a/.ai/contexts/subagent-observability.md +++ b/.ai/contexts/subagent-observability.md @@ -93,6 +93,55 @@ This is the **#1 fork-specific feature** (upstream PR #47 still pending). It per prune until PR #137. Keep cross-file names distinct; `test/dom-grid-sidebar-prune-collision.test.js` pins the pair. +## The child row's own `.running` dot, for every source (issue #285) + +`activeSubagentsByParent` (above) used to be fed only by the local +`subagent-spawned`/`subagent-completed` IPC pair, so a remote or +local-transcript subagent's own row never got `.running` — not even after a +full rebuild — even though both sources' `kind:'subagent'` payload already +carries `agentId` (`remote-activity.js`, `local-transcript-activity.js`). +`noteSubagentActivity(parentSessionId, agentId)` (`public/sidebar.js`) is now +the single write path into that map: the local IPC handler, `onRemoteActivityEvent`'s +subagent branch (`remote-activity-ui.js`) and `onLocalTranscriptSubagentActivity` +(`local-transcript-adapter.js`) all call it, so all three share the same 60s +TTL/decay convention `isSubagentActive`/`pruneStaleSubagents` already +implement — no second, independently-tuned decay was added. + +A suspected second cause (`reflectSubagentRunningState`'s DOM lookup, +`subagentDomId(parent, agentId)` vs. the row's own id) turned out not to +reproduce: every subagent row's `sessionId` is built by +`subagentSessionId(parent, agentId)` = `'sub:'+parent+':'+agentId` +(`read-session-file.js`), for local, remote-mirrored and legacy-layout rows +alike, so `'si-' + session.sessionId` and `subagentDomId(parent, agentId)` +are byte-identical by construction. `test/dom-sidebar-subagent-running.test.js` +already pinned this for the local IPC path with no rebuild; the actual gap +was purely the missing feed, not the lookup. + +One landmine found while wiring this: `reflectSubagentRunningState` also +repaints the **parent's** icon slot via `paintSessionIcon`, which paints from +the local-pty adapter's snapshot (`snapshotForLocal` auto-vivifies a +`localPtyState` entry). Calling it for a remote/local-transcript parent would +have overwritten the icon the remote-ssh/local-transcript adapter's own +`projectRemoteState`/`projectLocalTranscriptState` had just painted, with an +empty local-pty snapshot. + +**Corrected in review**: the first pass skipped that repaint whenever +`remoteSessionStates.has(parentSessionId)` was true — wrong, because +`setRemoteAttached` never deletes the entry, so it stays true for the rest of +the row's life whether or not a tab is currently attached. An **attached** +remote row is owned by the local-pty path (#273: OSC busy/idle governs it, +exactly like an ordinary local row), so it still needs this repaint — with +the old check it never got one on a live subagent toggle, leaving +`has-busy-agents` set on the row while the icon itself stayed +`session-icon--idle` until the next full rebuild. The guard is now +`isRemoteRowOwned(parentSessionId)` (`remote-activity-ui.js`) — remote state +exists **and** is not attached — matching the exact ownership test #273 +already established (`projectRemoteState`, `markRemoteBusy`, +`decayRemoteBusy` all gate on `snapshot().attached` the same way). +`localTranscriptStates` needs no equivalent check: there is no "attached" +state for that kind — `localTranscriptPtyTakeover` deletes the entry outright +once a PTY takes the row over, so `.has()` alone stays correct. + ## Attribution across sources (issue #247) `.has-busy-agents` used to light only for a parent reachable from diff --git a/eslint.config.js b/eslint.config.js index 2da2def3..dd9edff6 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -179,6 +179,8 @@ const rendererCrossFileGlobals = { stopBeforeArchive: 'readonly', // read by sidebar.js's parentHasActiveSubagent() — see .ai/contexts/subagent-observability.md remoteSessionStates: 'readonly', + // see .ai/contexts/subagent-observability.md + isRemoteRowOwned: 'readonly', // public/local-transcript-adapter.js (local-transcript adapter, see .ai/contexts/session-state.md) localTranscriptPtyTakeover: 'readonly', pruneLocalTranscriptTimers: 'readonly', @@ -186,6 +188,8 @@ const rendererCrossFileGlobals = { // public/sidebar.js, consumed by session-activity-dom.js's snapshotForLocal // (see .ai/contexts/session-state.md, "The icon slot (step 3b)") parentHasActiveSubagent: 'readonly', + // public/sidebar.js — see .ai/contexts/subagent-observability.md + noteSubagentActivity: 'readonly', // Third-party renderer libs loaded as