feat: keep a cluster agent's managed memory on its sandbox volume - #1081
Conversation
There was a problem hiding this comment.
Requesting changes for one cluster lifecycle regression. Managed memory correctly moves onto the sandbox volume, but scheduled dreams still call DreamRunner.start() directly after recording the schedule fire. start() now requires an already-bound sandbox before it can snapshot memory, so a normally suspended pool agent loses that scheduled dream instead of waking and running it. The scheduled path needs to bind/wake the sandbox before resolving the memory filesystem (while ordinary console/admin reads can retain the new sandbox-unavailable behavior).
Verification: 261 focused daemon tests and 27 focused web tests passed; daemon, web, and control-plane typechecks passed via their underlying Node entry points; changed files passed ESLint. The focused control-plane integration test could not run because this review environment has no container runtime.
sent by review-bot (Codex · gpt-5.6-sol) · open in session
| // observationally equivalent to never having started a Dream. | ||
| this.assertExecutionAllowed() | ||
| const dir = this.dirFor(agentId) | ||
| const fs = this.fsFor(agentId) |
There was a problem hiding this comment.
Wake/bind the sandbox for scheduled dreams before resolving this filesystem. onDreamScheduleFire() records dreamLastRun and then calls start() directly; unlike the Memory console or session-start path, it has no wake step. A pool agent whose pod was suspended during the usual overnight idle window therefore throws MemorySandboxUnavailableError here, logs a failed start, and does not retry until the next schedule occurrence. That makes scheduled dreaming effectively skip normal idle cluster agents. Please run the scheduled start through an ensureChannel/sandbox-hold seam (or inject an equivalent resolver that can wake for this authorized background job) before taking the snapshot.
|
Tightened the surface as reviewed (head fbc4fc7): |
There was a problem hiding this comment.
Requesting changes because the cluster scheduled-dream lifecycle regression remains on this revision. The new MemoryFs-only refactor is coherent, but onDreamScheduleFire() still records the occurrence and calls DreamRunner.start() directly. start() immediately resolves the managed filesystem, and resolveMemoryFs() throws MemorySandboxUnavailableError when the pool agent's sandbox is suspended. Since an idle cluster agent will normally have no running pod at an overnight dream schedule, the authorized scheduled job is logged as failed and that occurrence is lost until the next schedule. Please wake/bind the sandbox (and hold it for the background job) before resolving/snapshotting its memory; ordinary console/admin reads can retain the intended sandbox-unavailable behavior.
Verification on fbc4fc729dff125b2205e26cf8e5cc03e0b6a998: 269 focused daemon tests passed, the daemon typecheck passed, and every changed TypeScript file passed ESLint.
sent by review-bot (Codex · gpt-5.6-sol) · open in session
|
Thanks—the single-port refactor is clear and is not what blocks approval. At Please wake/bind the sandbox before the snapshot and keep it available for the background dream lifecycle. Ordinary console/admin reads can continue returning sent by |
|
Addressed the review-bot item on scheduled dreams (head 2f2c373): a dream is authorized background work like a turn, so |
|
That lifecycle shape addresses the concern. I can’t verify it yet: GitHub still reports PR head sent by |
fbc4fc7 to
2f2c373
Compare
There was a problem hiding this comment.
The scheduled-dream wake regression is fixed: withMemoryHome binds and holds a suspended cluster sandbox across the snapshot and asynchronous run. Requesting changes for one remaining lifecycle hole in the new background-job bookkeeping.
dutyGroupBusy() delegates to DreamRunner.inFlight(), which only reads active. The run clears active before maybeAutoAdopt(), so shutdown can treat the group as idle, revoke its duty, and release the shim channel while auto-adoption is still reading and swapping the sandbox-backed memory tree. The same block also leaves active and the aborter permanently registered if the second withMemoryHome acquisition/ensureChannel rejects before its callback starts, because cleanup is inside that callback and the outer rejection is swallowed. Please track the whole background-home operation separately from the adoption reservation, expose that whole lifetime to the duty-busy predicate, and perform reservation cleanup from a promise-wide finally.
Verification on 2f2c3739e6a344344fa305507d3b6c3b6144f460: 166 focused dream/K8s/duty/schedule tests passed; daemon typecheck and changed-file ESLint passed.
sent by review-bot (Codex · gpt-5.6-sol) · open in session
| await this.run(dream, files, sources, aborter.signal) | ||
| } finally { | ||
| this.aborters.delete(dream.dreamId) | ||
| if (this.active.get(agentId) === dream.dreamId) this.active.delete(agentId) |
There was a problem hiding this comment.
This clears the only state inFlight() observes before maybeAutoAdopt() runs. During that adoption window, dutyGroupBusy() can return false and shutdown can call releaseClusterSandbox(), which revokes the shim session and discards the driver's hold while adopt() is still swapping the remote memory tree. There is a second cleanup leak on the same promise: if withMemoryHome rejects in ensureSandbox/ensureChannel before entering this callback, this finally never runs and the outer .catch(() => {}) leaves active/aborters stuck. Please keep a separate background-job marker across acquisition → run → auto-adopt, clear it from a promise-wide finally, and have the duty-busy predicate follow that marker.
…lume Managed memory (memory/, .history, channels/, memory-dreams/) of a pool agent lived under the member's ephemeral state root and was lost on every duty move or rollout. Every managed-memory writer and reader is now a directory abstraction over one file-system port (agents/memory-fs.ts): the local port keeps the hardened path walk; a cluster agent's tree lives at <workspace mount>/.agentconnect/memory and is reached through the shim's read capability (memory-fs-channel + a descriptor-bound pod executor), one resolution and sandbox-unavailable while the pod is not bound. Dream staging and adoption run over the same port. A post-turn distillation that finds the tree asleep waits in the memory capture outbox and drains when the sandbox binds again. Refs #1078
…th 503 and the wake code A memory or dream request refused by the daemon with reason sandbox-unavailable is a transient 503 carrying WORKSPACE_SANDBOX_UNAVAILABLE, the code the console wakes the sandbox on, never a 400. Refs #1078
The Memory tab takes the same hook as Files: for a pool-placed agent, or when the file list refuses with the asleep code, the wake is pressed once, a calm starting line replaces the error while the read is polled, and the terminal copy offers Start. Refs #1078
Every managed-memory consumer (memory.ts helpers, the managed provider, the distiller, the dream runner, the CP memory reader) takes a MemoryFs; the string-or-port union is gone. Two implementations, LocalMemoryFs(rootDir) and ShimMemoryFs(channel, rootPath), and one daemon factory (resolveMemoryFs, behind Daemon.memoryFsFor) that picks by placement and may refuse with MemorySandboxUnavailableError. The dispatching provider takes an options object; tests construct LocalMemoryFs explicitly. Refs #1078
A scheduled dream on a pool agent normally fires while its pod is suspended; resolving the memory tree first failed the start and lost the occurrence. A dream is authorized background work like a turn: the runner now brings the memory home up and holds it (ensureChannel + withSandbox under --k8s) around the snapshot and the whole run including auto-adoption, so the idle sweep cannot suspend the pod mid-dream, and an in-flight dream counts as in-flight work for the shutdown release. Console and admin reads keep the sandbox-unavailable refusal. Refs #1078
…led home acquisition The shutdown busy predicate followed the adoption reservation, which is released before auto-adoption, so a drain could revoke the duty and release the shim channel while the swap was still under way; and a home acquisition that rejected before its callback left the reservation and aborter registered forever. A separate background-job marker now spans acquisition, run and auto-adoption and is cleared in a promise-wide finally, inFlight() follows it, and a dream whose home never came up is marked failed. Refs #1078
2f2c373 to
18dba27
Compare
|
Addressed the remaining lifecycle hole (head 18dba27): |
There was a problem hiding this comment.
Approved. The prior lifecycle findings are resolved on 18dba27739e734ab9218a98266191a42ef9e5a2c: scheduled dreams wake and hold a suspended cluster sandbox, the separate background-job marker remains visible to duty shutdown through auto-adoption, and a failed second home acquisition now marks the dream failed and clears its reservation/job state from a promise-wide finally.
Non-blocking follow-up: the job marker is registered after the initial remote snapshot, so shutdown racing that initial snapshot can still release the sandbox and drop that scheduled attempt. That is a narrow shutdown-only window and does not risk a partial store swap, so I would not hold this startup-stage PR for it.
Verification: 168 focused dream/K8s/duty/schedule tests passed; daemon typecheck and changed-file ESLint passed.
sent by review-bot (Codex · gpt-5.6-sol) · open in session
|
Confirmed on The only remaining note is the non-blocking shutdown window during the initial snapshot, before sent by |
Summary
A pool agent's managed memory (
memory/,memory/.history,channels/<key>/…,memory-dreams/<id>/…) was written under the member's ephemeral state root, so a duty move or a rollout lost it (#1078). This PR moves the whole managed tree of a cluster agent onto its sandbox volume — one root beside the checkout,<workspace mount>/.agentconnect/memory, same layout beneath — read and written through the shim's file surface, and makes every managed-memory writer and reader a directory abstraction over one file-system port so a later home is a port swap. Local single-daemon behaviour is unchanged.Closes #1078. Completes the Memory half of #1070 (Files half: #1077).
Decision (from the issue)
MemoryFsimplementation plus a one-directory-per-agent copy, and the rationale for A over B (and over the control-plane database) is recorded in the issue.sandbox-unavailable— one resolution, no fallback to the member's disk (mirrorscreateWorkspaceReader.filesOf). The console wakes the sandbox first, exactly like Files (feat: wake a cluster agent's sandbox from the console instead of explaining #1077).Storage and the file-system port
agents/memory-fs.ts— the ONE portMemoryFs(readFile/writeFile/readdir/mkdir/rename/rm/utimes, root-relative paths, an identitykeyfor the in-process locks and write ledger,subdirfor a channel root), exactly two implementations —LocalMemoryFs(rootDir), which keeps the hardened path walk (component-wise realpath, symlink refusal,O_NOFOLLOW, random exclusive temp + rename, mtime precondition) thatmemory.tsused to inline, andShimMemoryFs(channel, rootPath)— and one factory,resolveMemoryFs(agent, sandboxPlane), behindDaemon.memoryFsFor(agentId): the only place that decides (local port without a plane; the bound sandbox's port under--k8s;MemorySandboxUnavailableErrorwhen the sandbox is not bound). Nostring | MemoryFsunions; no memory consumer receives a directory path.agents/memory.ts, the managed provider, the distiller,agents/dream-runner.tsandcp/memory-reader.tsno longer touchnode:fs; each takes aMemoryFs(the dispatching provider takes aMemoryProviderDepsobject withmemoryFsFor; the localagentDirByAgentsurvives only for native memory and the accepted-skills root, which are not managed memory). History append is now canonicalize-then-rewrite (one write instead of compact/append/compact). Dream staging, the adoption swap (backup, replacement, rename, unchanged-file mtimes), the distill rebase and staged reads all run over the port; a dream's extraction cwd is its input dir in the coordinates of the filesystem that holds it (the pod's, for a cluster agent). A staged skill candidate is materialised locally only for the snapshot walker on accept/review.readcapability beside the workspace ops:shim/memory-fs-channel.ts(payloads shaped for the 256 KiB frame: budgeted slice reads at an offset; writes staged as appended chunks into a sibling temp and committed by one rename carrying the mtime precondition; text as budget-fitted utf8, bytes as base64) andshim/fd-memory-fs.ts(the pod side, descriptor-bound throughwithDescentlike the workspace executor). The daemon side,ShimMemoryFs, is a pass-through that reassembles slices and chunks; the two typed refusals cross the channel as themselves. The memory logic (locks, history, retention, write ledger, dream fence) stays in the daemon process — only primitives cross the wire.K8sRuntimePlane.memoryFsFor(agentId)on the same condition asworkspaceFilesFor;sandboxMemoryRoot(mount);onSandboxBoundhook.daemon.tsresolves the managed root once per call (managedMemoryRoot): local dir, or the bound sandbox port, elseMemorySandboxUnavailableError.memory.ensureis now async and runs after the host is up (a cluster agent's tree is reachable only once the pod is bound); injection was already there.memory_capture_outbox(shared-store-safe) under a synthetic per-agent connection (agents/managed-distill-outbox.ts,withManagedDistill); the pump defers the row without spending attempts while the sandbox is down and drains it when the sandbox binds again on the holder (onSandboxBoundwakes the pump). The outbox contract was narrowed to what the pump needs (MemoryCapturePumpRegistry/MemoryCaptureClient); plugin connections are untouched. Ordinary turns still distill inline as before.BAD_PAYLOAD+reason: sandbox-unavailableon the memory and dream frames; the control plane maps it to 503 withcode: WORKSPACE_SANDBOX_UNAVAILABLEon the memory file, channels, history, write and dream routes (never a 400).Layering
MemoryFsis the file-shaped port for one thing: an agent's managed memory tree (memory/,.history,channels/, dream staging and backups). It answers "where do these files live" — this disk or the agent's sandbox volume — and nothing else; the memory logic (locks, history, retention, write ledger, dream fence) sits above it in the daemon process, and the memory provider dispatch (DispatchingMemoryProvider: managed / native / none / external plugins, per agent) sits above that, deciding which backend serves an agent's recall, capture, tools and console surface.Org knowledge does not go through either. On the daemon it is a separate path: the
findKnowledge/listKnowledge/orgSkillsMCP tools call the control plane directly over theknowledge/search/knowledge/list/org-skillsframes (daemon.ts→CpClient.knowledgeSearch…), dream proposals reach it throughknowledge/suggestion/*, and the console reads it from the CP's own REST surface — search and revision semantics over CP-owned rows, not a file tree, soMemoryFsis the wrong layer for it. If a unification is wanted, it belongs one level up: an org-knowledge contribution torecallForTurnbehind the provider dispatch (a follow-up, not in this PR).Console
MemoryPaneltakessandboxed(pool placement, likeWorkspaceFiles) and wiresuseSandboxWakeon the root read (the file list): the wake is pressed once when the read refuses with the asleep code (or on open for a pool agent), the calm "starting" line replaces the error, the read is polled, and the terminal copy (MEMORY_SANDBOX_ASLEEP_NOTICE) shows with a Start button;unsupportedkeeps the copy without one; a plain 503 presses nothing; non-managed providers never press.Docs
docs/designs/memory-evolution.md§3.2.1 — where the managed tree lives per placement and how the port and shim channel work.docs/designs/k8s-daemon-pool.md§11 — one paragraph on managed memory on the sandbox volume.Test plan
packages/daemon—memory-fs.test.ts(local port semantics and hardening;resolveMemoryFs: local agent →LocalMemoryFs, bound sandbox →ShimMemoryFs, unbound →MemorySandboxUnavailableError; the shim port over a fake requester: the managed tree runs unchanged on a pod root, multi-frame reads and chunked writes, typed refusals, provider + CP reader over the port,sandbox-unavailablewhen unbound, shape query still answers); the descriptor-bound executor and exec-handler routing on Linux (scripts/linux-vitest.sh, run in a container);dream-runner.test.ts(new: stage + adopt on a sandbox root through the port, cwd in pod coordinates, nothing on the member's disk; all 66 existing cases green over the port);managed-distill-outbox.test.ts(deferred without attempts while down, drained once bound, plugin registry untouched);k8s-runtime-plane(memoryFsForonly with a bound channel, root beside the checkout);cp/client-dispatch(memory refusal carries the reason); memory / channel-memory / distiller / provider-dispatch / mcp-ops / session-manager suites green. Full daemon suite: only the pre-existing environment failures.packages/control-plane—test:unit;test:intagents.memory.route.test.ts(new: every memory file route answers 503 +WORKSPACE_SANDBOX_UNAVAILABLEfor a sleeping sandbox).packages/web—MemoryPanel.test.tsx(wake pressed once on the asleep 503, starting line, terminal copy with Start that re-presses,unsupportedwithout Start, plain 503 presses nothing,sandboxedpresses on open, native never presses);sandbox-wake/FilesPanelsuites; full web suite green.pnpm typecheck(daemon, control-plane, web),pnpm lint,pnpm format:check.Notes for review
unavailableRetryMs(5 s) — a couple of cheap store queries; a row that never finds its sandbox expires with the outbox's usual retention.