Skip to content

feat(acp): let codeg-server own agent lifecycle instead of the browser tab - #1124

Open
Mistic92 wants to merge 1 commit into
spacering-net:mainfrom
Mistic92:feat/server-owned-agent-lifecycle
Open

Mistic92 wants to merge 1 commit into
spacering-net:mainfrom
Mistic92:feat/server-owned-agent-lifecycle

Conversation

@Mistic92

Copy link
Copy Markdown

Why

On a server deployment (codeg-server on a VM or in Docker, used from several devices) agents keep dying whenever the user looks away from them. Leave the web UI open on a phone, lock the screen for a few minutes, come back, and every conversation's agent has been killed and shows as disconnected. In a desktop browser tab the same agents live for hours.

The cause is that agent lifetime is tied to the browser tab:

  • The backend idle sweep ends any idle agent (status Connected, nothing in flight) that has not been touched for CODEG_ACP_IDLE_TIMEOUT_SECS, 180 s by default.
  • An open tab touches its connections every 30 s. A desktop browser only throttles a background tab's timers to about once a minute, so its agents survive.
  • A mobile browser freezes the page entirely when the screen locks or the user switches apps. The touches stop, and three minutes later every idle agent is killed at once.
  • Closing a conversation tab, a preview tab being replaced, and the frontend's own 60 s sweep also call acp_disconnect. Any device can therefore end an agent another device is using.

In a month of logs from one server deployment this sweep ended about 350 agents. Almost all of them were between turns, not mid-turn: a turn in flight was already protected. But the CLI process also holds things a tab cannot see, and they die with it. A pending ScheduleWakeup (a self-paced /loop) is not counted by the sweep at all.

What changes

The server owns the agent; a browser tab is only a view of it.

1. Idle policy per runtime (acp/idle_sweep.rs, ConnectionManager::sweep_idle_with)

  • Desktop: unchanged. 180 s idle timeout, no cap.
  • codeg-server: idle agents are kept for 2 hours instead of 3 minutes.
  • Cap on idle agents (server default 8). Each agent is a CLI process tree, often with its own MCP servers, so cost is bounded by a cap instead of a short clock tied to a tab. Past the cap, the agents idle the longest are put to sleep first, and only those no open tab is touching. An agent being watched is never put to sleep by the cap.
  • Headless owners keep the short timeout in both runtimes: automation runs, codeg-cli runs and one-shot generations. No tab ever comes back to them, and they must not hold cap slots.
  • All existing exemptions still apply: turn in flight, pending permission, background work, Remote Control, running delegation.
  • Configuration: CODEG_ACP_IDLE_TIMEOUT_SECS keeps its meaning (0 disables the sweep); only the server default changes. New CODEG_ACP_MAX_IDLE_AGENTS (0 = no cap).

"Put to sleep" is the same disconnect as before: the CLI process ends, the session stays resumable through session/load.

2. Closing a view detaches instead of killing (serverOwnsAgentLifecycle())

In web mode, and in a desktop window bound to a remote server, these paths now only detach the view (new release action) and never call acp_disconnect:

  • closing a conversation tab (unmount cleanup)
  • a preview tab being replaced
  • the frontend idle sweep
  • the provider unmounting

Reopening the conversation attaches to the same running agent. Explicit user actions are unchanged: switching agent or folder, Reconnect, and restart-to-apply after a settings change.

In the local desktop app every path behaves exactly as before.

3. A conversation whose agent went to sleep resumes on its own

When the agent behind the open, active conversation goes away (put to sleep after a long idle stretch, or a server restart), the conversation reconnects and resumes the same session as soon as the page is visible. The user no longer sees a dimmed composer and has to find the Reconnect button first.

This is limited to a connection that went away while the view was open, and to at most once per minute, so an agent that keeps dying cannot become a respawn loop.

4. A pending ScheduleWakeup keeps its agent alive (acp/background_watch.rs)

The transcript watcher now counts a pending wakeup as outstanding work, from the ScheduleWakeup ack's toolUseResult.scheduledFor:

  • It is cleared when the wakeup fires (a user record with turnOrigin: "scheduled") or when the loop is stopped (stop: true).
  • It expires on its own deadline: the scheduled time plus 5 minutes of slack, so a wakeup that never fires cannot pin the connection.
  • While work is pending, the watcher re-sends its count at least every half keep-alive max age. A wakeup can be scheduled close to an hour out, past the window in which the sweep exemption would otherwise lapse.
  • A pending wakeup alone stays on the idle poll cadence.

Monitor is already covered by the adapter's async-task channel. CronCreate is not handled here: I found no real transcript showing its record shapes, and did not want to guess them.

Not in this PR

  • Web Push notifications ("agent finished / needs permission") for using the web UI from a phone. The PWA already exists; push needs VAPID keys, stored subscriptions and a sender.
  • Agents surviving a restart of codeg-server.

Tests

  • Rust:
    • idle_sweep: policy defaults and env parsing for both runtimes.
    • manager: the server policy keeps an unwatched agent until its long timeout; headless owners are reclaimed on the short timeout; the cap picks the longest-idle unwatched agents, never touches a watched one, and ignores busy ones.
    • background_watch: a wakeup counts until it fires, a later one replaces it, stop clears it, a long-overdue one is not counted, it expires on its own deadline, and it does not force the 1 s poll.
  • Frontend:
    • use-connection-lifecycle.unmount.test.ts: server mode releases instead of disconnecting (idle or busy), keeps the view across a transient remount, auto-resumes when the agent goes away, waits for a hidden page to be shown, does not respawn twice within the interval, and never auto-resumes on desktop.
    • acp-connections-context.test.tsx: in server mode the preview release, release(), the frontend idle sweep and the provider unmount detach without acp_disconnect.
    • Existing lifecycle tests are pinned to local-desktop semantics, so they keep covering the unchanged desktop paths.

🤖 Generated with Claude Code

…r tab

On a server deployment used from several devices, agents died whenever
the user looked away: a phone freezes the page when the screen locks,
the 30 s keepalive stops, and the 180 s idle sweep ends every idle agent.
Closing a tab on any device also ended agents another device was using.

- Idle policy per runtime: desktop unchanged (180 s, no cap); the server
  keeps idle agents for 2 h, bounded by a cap of 8 idle agents (longest
  idle and unwatched first). Headless owners (automation, codeg-cli,
  one-shot) keep the short timeout. New CODEG_ACP_MAX_IDLE_AGENTS.
- In web mode closing a view, replacing a preview tab, the frontend idle
  sweep and the provider unmount detach the view instead of calling
  acp_disconnect.
- A conversation whose agent went away while it was open resumes its
  session on its own once the page is visible (at most once a minute).
- A pending ScheduleWakeup counts as outstanding background work until
  it fires, is stopped, or passes its deadline plus 5 minutes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@xintaofei

Copy link
Copy Markdown
Collaborator

@Mistic92 thank you for this, and for the very clear write-up. The problem is real: a phone that locks its screen should not cost the user their agents. The month of logs and the breakdown of who calls acp_disconnect made it easy to follow. The backend half is in good shape. The per-runtime sweep policy, keeping headless owners on the short timeout, a cap that never touches a watched agent, and ScheduleWakeup accounting that expires on its own deadline all read well. The tests pin each of those rules down.

We'd like to work through one design question before merging, though. It is about who owns an agent once a view only detaches, and it needs a decision rather than a small patch. There is also one smaller issue in the auto-resume that can be fixed in place.

Summary

  1. After release(), reopening a saved conversation whose agent is still alive attaches this client as a viewer of its own agent. Viewers lose restart-to-apply and the other owner-only controls. This one needs a design decision.
  2. Auto-resume only reacts to a "disconnected" status. When the re-attach after a server restart or a frozen page reports the connection gone first, the entry is removed instead, and the conversation does not resume on its own. This one is fixable in place.
  3. A few smaller notes.

1. A reopened conversation comes back as a viewer

Ownership in codeg is decided on the client. The surface that spawned a connection owns it; another surface that finds a live connection for the same conversation attaches as a viewer. When no local entry can be reused, opening a saved conversation first asks the server for a live connection. If one exists and no local entry owns it, the client takes the viewer path:

const localOwnerKey = discovered
? localOwnerKeyOf(discovered.connection_id)
: null
if (discovered && localOwnerKey !== contextKey) {
const attached = await connectAsViewer(
contextKey,
discovered.connection_id,
agentType,
nextWorkingDir
)
// Attached (or superseded) — done. Otherwise the connection died
// between discovery and the attach, so fall through and spawn one
// rather than leaving a viewer bound to a dead id.
if (attached) return
}

With this PR, release() removes the local owner entry but leaves the agent running on the server:

if (conn.isViewer || keepAgent) {
// Viewer teardown: drop our read-only attachment WITHOUT
// `acpDisconnect` — the backend connection belongs to another client,
// and disconnecting it would kill the owner's agent mid-turn. Mirrors
// detachDelegationChild. The owner's own disconnect / the idle sweep
// governs the connection's real lifetime.
// A `release` takes the same path for an owner: the agent belongs to
// the server, which puts it to sleep once it has been idle long
// enough.
teardownAttachSubscription(contextKey)
releaseConnectionRoute(conn.connectionId, contextKey)
pendingUnmappedEventsRef.current.delete(conn.connectionId)
lastActivityRef.current.delete(contextKey)
dispatch({ type: "CONNECTION_REMOVED", contextKey })
return true
}

So in server mode, after release() removes the local owner entry, reopening a saved conversation attaches as a viewer whenever discovery finds the surviving connection. Viewers receive the keepalive touches too, so while those touches continue, the agent does not reach its idle timeout and the user can stay in this restricted state while using the conversation:

const consider = (contextKey: string) => {
if (seen.has(contextKey)) return
seen.add(contextKey)
const conn = storeRef.current.connections.get(contextKey)
if (!conn) return
if (conn.status === "disconnected" || conn.status === "error") return
// Broker-owned children come and go on the parent's schedule and are
// released by `detachDelegationChild`; settling one here would fight
// that lifecycle.
if (conn.isDelegationChild) return
toTouch.push({ contextKey, connectionId: conn.connectionId })
}
if (currentActiveKey) consider(currentActiveKey)
for (const contextKey of currentOpenTabKeys) consider(contextKey)
for (const { contextKey, connectionId } of toTouch) {
void isConnectionLiveOnBackend(connectionId).then((live) => {
if (!live) markConnectionGone(contextKey, connectionId)
})
}
}, CONNECTION_KEEPALIVE_INTERVAL_MS)
return () => clearInterval(timer)
}, [heldOpenKeys, isConnectionLiveOnBackend, markConnectionGone])

A viewer is not read-only: it can still send prompts and cancel a turn. But several controls are gated on local ownership, so in that state:

That conflicts with "Explicit user actions are unchanged" in the description. Before this PR, closing an idle owned tab in web mode ended the agent, so reopening spawned a fresh one that this client owned. To be fair, this viewer state was already possible after a page reload, because nothing calls acp_disconnect on unload. This PR also introduces it after releasing and reopening a saved conversation. And the default server idle timeout rises from three minutes to two hours, subject to earlier reclamation under the cap, so part 1 on its own also lets the reload case last much longer.

Reproduction (vitest, added to src/contexts/acp-connections-context.test.tsx)

This uses the file's existing mocks plus the serverLifecycle switch your PR adds. It passes on the PR head, which confirms the behaviour above.

describe("server-owned lifecycle: release then reopen", () => {
  it("reopening a released conversation demotes this client to a viewer", async () => {
    serverLifecycle.owned = true
    h.acpFindConnectionForConversation.mockResolvedValue(null)
    await mountProvider()
    await act(async () => {
      await h.actions!.connect(TAB, "claude_code", "/tmp/x", "sess-1", 42)
    })
    expect(h.store!.getConnection(TAB)?.isViewer).toBe(false)
    await act(async () => {
      await h.actions!.release(TAB)
    })
    expect(h.acpDisconnect).not.toHaveBeenCalled()
    h.acpFindConnectionForConversation.mockResolvedValue({
      connection_id: "spawned-conn",
      event_seq: 3,
    })
    await act(async () => {
      await h.actions!.connect(TAB, "claude_code", "/tmp/x", "sess-1", 42)
    })
    expect(h.store!.getConnection(TAB)?.isViewer).toBe(true)
    let restarted: boolean | undefined
    await act(async () => {
      restarted = await h.actions!.reapplyConfig(TAB)
    })
    expect(restarted).toBe(false)
    expect(h.acpDisconnect).not.toHaveBeenCalled()
  })
})

Possible directions. We don't have a firm preference yet and would rather agree on one with you before you write code:

  • Ownership tracked on the server. The server would hold an owner identity per connection, with atomic claim and release. An owner's release gives ownership up, while a viewer's only detaches. A client opening an unowned connection claims it instead of attaching as a viewer, and the atomic claim keeps two clients from both adopting it. Ownership could also expire, which would cover a reload or a page the phone discarded. Renewal would then need to tell owner heartbeats apart from viewer keepalives, since today every web client spawns under the same "web" label and a touch carries only the connection id. This keeps today's multi-device protection, but we'd need to agree on the claim and lease behaviour first.
  • No ownership gate on explicit actions in server mode. Any view may restart or stop the agent, perhaps with a confirmation when a turn is in flight. It is simpler, but one device can then interrupt a turn another device started.
  • Remember released connections locally. The provider keeps the ids it released and re-adopts them on reopen. This only covers closing and reopening a tab within the same page, not a reload or another device, so on its own it is probably not enough.

2. Auto-resume misses the "connection gone" path

The new effect only arms when the status becomes "disconnected":

useEffect(() => {
const previous = resumePrevStatusRef.current
resumePrevStatusRef.current = status
if (status !== "disconnected" || !isActive) {
resumePendingRef.current = false
return
}
if (previous !== "disconnected") resumePendingRef.current = true
if (!serverOwnsAgentLifecycle() || !workingDir) return
const resume = () => {
if (!resumePendingRef.current) return
if (document.visibilityState !== "visible") return
const now = Date.now()
if (now - lastAutoResumeAtRef.current < AUTO_RESUME_MIN_INTERVAL_MS) {
return
}
resumePendingRef.current = false
lastAutoResumeAtRef.current = now
connConnectRef
.current(
agentType,
workingDir,
sessionIdRef.current,
conversationIdRef.current
)
.catch((e: unknown) => {
if (!isExpectedConnectError(e)) {
console.error("[ConnLifecycle] auto-resume:", e)
}
})
}
resume()
document.addEventListener("visibilitychange", resume)
return () => document.removeEventListener("visibilitychange", resume)
}, [status, isActive, workingDir, agentType])

When the WebSocket re-attaches and the server reports the connection gone, the attach handler removes the entry instead of marking it disconnected:

// connection_gone: backend GC'd the connection. Mirror to UI
// so the user sees the conversation tab go away rather than
// staring at stale state forever.
attachSubscriptionsRef.current.delete(contextKey)
// Still in a turn here: the turn ended without this client — a
// connection that ends while attached reports its status first, so
// this is a re-attach that found nothing left (the idle sweep takes
// a connection nobody touches once its turn is over).
const gone = storeRef.current.connections.get(contextKey)
if (gone?.status === "prompting") noteTurnEndedUnseen(gone)
// The composer that survives this is exactly the one whose Reconnect
// button has to resume the session rather than start a new one.
captureIdentityBeforeRemoval(contextKey)
dispatch({ type: "CONNECTION_REMOVED", contextKey })
},

With no entry and no pending connect, useConnection reports null rather than "disconnected" (use-connection.ts#L279), so the effect clears its pending flag and does not resume. This can happen after a codeg-server restart, since the client re-attaches every subscription when its socket comes back and the new server process has no such connection. It can also happen when a frozen page wakes up. If the keepalive probe notices the loss first, it marks the entry "disconnected" and auto-resume runs, subject to visibility and the rate limit. On the removal path the user has to recover by hand: focusing the composer reconnects, because handleFocus treats a missing status as reconnectable (L525-L551), and Reconnect or switching back to the tab also work.

A fix that stays inside your design: also arm the resume when the status goes from a live value to null while the view is active. It should keep the visibility check and the rate limit, and not fire after an intentional teardown or for a view that never had a connection. A test that drives the attach handler's connection_gone detach, rather than a status_changed event, would cover it. The current unmount tests only simulate the "disconnected" transition.

3. Smaller notes (non-blocking)

  • Two devices can both end up as owners. This race exists today, and auto-resume gives it a new trigger. Suppose device A restarts an agent while device B, viewing the same conversation, auto-resumes. Both discovery calls can miss, both reach acp_connect, and the backend's per-session dedup hands both the same connection. There is still only one process, but both clients now consider themselves owners. This mostly matters again once item 1 is settled.
  • The desktop app's built-in web server only gets part of this. A browser connected to it also releases instead of disconnecting, but its backend keeps the desktop idle policy, normally three minutes. Idle retention does not grow there, although auto-resume and wakeup accounting still apply, and the ownership issue in item 1 applies there too. A line in the description or docs would avoid surprises.
  • Thanks for leaving CronCreate out rather than guessing its record shapes. That seems right to us.

Testing on our side

I ran the reproduction above against the PR head. I read the Rust changes (idle_sweep.rs, sweep_idle_with, background_watch.rs) and their tests and did not find problems there, but I did not run the Rust suites.

If I've misread any of this, especially the ownership path, please push back. Once we agree on a direction for item 1, I'm happy to help with the backend side or review a follow-up.

@xintaofei

Copy link
Copy Markdown
Collaborator

codeg work task 506 is done — it made no changes, so nothing was pushed to this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants