Repository navigation
feat(loom): add idle session suspend/wake to free host memory - #388
Conversation
rjpower
left a comment
There was a problem hiding this comment.
Seems fine, I just worry about having to reason about the new suspended state for callers. I'd favor not explicitly calling ensure_awake but rather updating the session retrieval to auto-wake the session as needed?
| @@ -929,6 +942,9 @@ pub(super) async fn send_session( | |||
| Json(req): Json<SendReq>, | |||
| ) -> ApiResult<Json<Value>> { | |||
| let (session, branch) = require_session(&st.db, &key).await?; | |||
There was a problem hiding this comment.
should we update all of the callers to have something like require_active_session?
otherwise i worry that we'll have a bunch of issues where we try to act on a suspended session and other subtle bugs
There was a problem hiding this comment.
I'm fine either way and, honestly, don't have a good enough mental model of the loom codebase to feel confident in my opinions.
My agent is adamantly against it on the grounds that the explicit nature of ensure_awake is better and that the failure mode (it's not awake) means the next thing in line (e.g. require_acp_task) will throw and so it's easily fixed. I can see your point, although I'm not sure it solved the bug problem, given that you have to remember to call require_active_session instead of require_session (and there are lots of calls to require_session that don't need to wake, so you don't want to put it there).
I'll make the change to add require_active_session and we'll see how it feels.
|
This interacted a bit with the changes you made to session communication, so I'm going to update locally and let it bake for a bit before I merge in case something weird happened with the interactions. |
|
Yeah no worries, I trust you'll test it :) |
Suspending stops a session's runtime (agent, terminal, debug shells) while the session, worktree, and branch stay in place; waking replays the adoption machinery and resumes the agent in seconds — a far lighter lever than Archive. - New `suspended` lifecycle status with `sessions.suspend`/`sessions.wake` operations wired to REST, CLI, and MCP; a wake hook at every ingress (send/prompt/channels, automation runs, GitHub triggers, review delivery, debug-shell attach) revives a dormant session transparently, and an opt-in idle reaper policy (`session.idle_suspend_secs`, off by default) suspends quiet sessions under the reaper's guards with an `auto-suspend: disabled` per-session opt-out. - Both lifecycle surfaces (the row ⋯ and Details menus) offer Suspend/Wake directly above Archive/Recover, the conversation foot's progress strip narrates waking/suspending, and interrupted suspending/waking transitions reconcile on the monitor cadence. - Wake and suspend run as server-owned tasks so a dropped request can never strand a published transition, the reaper re-convicts on a fresh row before suspending, and sending into a session mid-suspension 409s on both protocols.
Thirteen review rounds against the original commit produced fixes in five areas: - The terminal lane is deliberately dumb: a suspended terminal session has no queue and no auto-wake, and every ingress — human or agent — refuses with "wake it first" (`sessions.send`, a debug-shell open, the delivery seam used by triggers, Slack replies, and reviews). A channel message to a dormant terminal target publishes (the message is durable in channel history) but never wakes it; a re-publication while the row is awake is the retry. A committed result that cannot reach a dormant terminal parent raises its triage tag instead of waking it. The inbox lane — the one durability mechanism — belongs to ACP alone, whose queued rows it owns exclusively. - Wake semantics: every wake path (manual, ingress, delivery seam) clears its own stale triage by compare-and-clear and never touches an assessment it did not write; wake re-reads its caller's row and releases provably abandoned markers instantly. Every mid-lifecycle state refuses — "retry once it settles" — and a delivery-side wake attempts once, raising the branch's triage with the reason on failure: no settle waits, no retry loops, the next ingress or the operator is the retry. The ACP repair loop leaves deliberate dormancy alone. - Ingress safety: GitHub triggers and Slack replies are protocol-aware — an ACP relay is driven through its live task, never pasted into its JSON-RPC stdin — and neither archives a healthy running ACP session over a transient; review delivery resolves its target through the same shared wake seam; `prompt.create` rejects a terminal row before paying a respawn. - Recovery: interrupted-suspend reconciliation settles the abandoned ACP turn; adapter-open failures leave the relay attachable with a consistent row; the monitor reaps before it suspends so one tick never tears the same session down twice; suspended ACP sessions answer configure/inspect ops with dormancy and its Wake remedy. - ACP adapter startup survives transient races: `session/load` and `session/new` retry while the adapter is still activating (`ServiceUnavailableError`), and an exhausted transient failure preserves the loadable session id (resetting only the cursor and the turn claim) so the next attempt reloads instead of opening a fresh conversation and re-sending the goal. A cancelled prompt's response is awaited before a replacement dispatches, so the adapter cannot reject the new prompt as already active. - The launch's goal turn serializes against queued input: while the goal turn a wake's fresh open starts is live, the inbox holds queued prompts back and starts them as the next turn instead of cancelling the goal turn to start themselves — the 5ijvbegy failure, where the queued prompt immediately cancelled the resumed goal turn and the replacement errored. A `session/load` wake carries no goal at all (history replays; nothing is re-sent), so a queued prompt is simply the first new turn. - The frontend hides the composer for a suspended terminal row (a send would refuse), so `SessionSummaryView` gained `protocol`. Tests: the suspend suite pins the journeys and contracts (round trip and refusals, the uniform terminal refusal across send, shell open, and the delivery seam, the no-wake publication path, every ACP ingress waking its target, the ACP backlog's durability, triage behavior, the monitor hop, and the turn fence); the ACP suite pins the turn-start fence and the fresh-open wake, the adapter-startup retry and cancel-settle contracts, and the goal-turn serialization (the queued prompt starts after the resumed goal turn settles, with no errored turn, and a `session/load` wake injects no second goal turn). The harness scrubs ambient `LOOM_RUNNER`.
2307991 to
5de1235
Compare
|
Alright, this has felt stable over the last week so I'm going to go for it. It's off be default, so you may never encounter any bugs if there are any :-) |
Suspending stops a session's runtime (agent, terminal, debug shells) while the session, worktree, and branch stay in place; waking replays the adoption machinery and resumes the agent in seconds — a far lighter lever than Archive.
suspendedlifecycle status withsessions.suspend/sessions.wakeoperations wired to REST, CLI, and MCP; a wake hook at every ingress (send/prompt/channels, automation runs, GitHub triggers, review delivery, debug-shell attach) revives a dormant session transparently, and an opt-in idle reaper policy (session.idle_suspend_secs, off by default) suspends quiet sessions under the reaper's guards with anauto-suspend: disabledper-session opt-out.