feat(#576): durable per-scope sandbox — P3 scope durability + RO-layer content hash + reaper

middleware/migrations/0044_sandbox_registry.sql

@@ -0,0 +1,29 @@
+-- Issue #576 P3 — durable per-scope sandbox registry.
+--
+-- Bookkeeping `DockerSandboxBackend`'s deterministic container naming does
+-- not give for free: last-used timestamps for the idle reaper (reaper.ts),
+-- the RO-layer content hash last synced into each scope's sandbox
+-- (contentHash.ts), and — once a second backend exists — a scope->backend
+-- routing table. One row per LIVE scope sandbox; a reaped or torn-down
+-- scope's row is deleted, not soft-deleted (nothing here is an audit log).
+--
+-- Migration numbering note: 0040-0042 are occupied by concurrent
+-- credential-work PRs (#769/#772) as of this migration's authoring, with an
+-- observed numbering collision at 0040 (three files) unrelated to #576.
+-- 0044 was chosen with margin over that, per the phase-4b plan's guidance
+-- to leave room for in-flight PRs.
+
+CREATE TABLE IF NOT EXISTS sandbox_registry (
+  scope_key      TEXT PRIMARY KEY,
+  backend        TEXT NOT NULL,
+  sandbox_ref    TEXT NOT NULL,
+  profile        JSONB NOT NULL,
+  ro_layer_hash  TEXT,
+  created_at     TIMESTAMPTZ NOT NULL DEFAULT now(),
+  last_used_at   TIMESTAMPTZ NOT NULL DEFAULT now()
+);
+
+-- The reaper sweeps by last_used_at; an index keeps that scan cheap once
+-- the table has more than a handful of rows.
+CREATE INDEX IF NOT EXISTS sandbox_registry_last_used_at_idx
+  ON sandbox_registry (last_used_at);

middleware/packages/harness-sandbox/package.json

@@ -19,6 +19,9 @@
     "typecheck": "tsc --noEmit",
     "clean": "rm -rf dist"
   },
+  "peerDependencies": {
+    "pg": "^8.13.0"
+  },
   "engines": {
     "node": ">=20"
   },

middleware/packages/harness-sandbox/src/contentHash.ts

@@ -0,0 +1,73 @@
+import { createHash } from 'node:crypto';
+
+/**
+ * Issue #576 P3 — content-hash fingerprinting for a sandbox's read-only
+ * layer (per the issue: "Read-only layers (org files, skills) are
+ * materialized by content-hash fingerprint, only re-synced on change").
+ *
+ * `#576` is deliberately the SUBSTRATE, not the consumer: what actually goes
+ * into a scope's RO layer (org files, skills — the issue names both) is a
+ * separate concept the issue explicitly says "several other qm concepts …
+ * build on" this sandbox, not something #576 itself decides. So this module
+ * exports the mechanism — hash a file set, sync only the files whose
+ * content changed since the last synced hash — for a future consumer to
+ * call with real content. Calling it with a hand-built `files` map is a
+ * fully exercised, real code path in `contentHash.test.ts`; there is no
+ * unwired surface here — `syncReadOnlyLayer` calls `Sandbox.write` for
+ * real, and the "only when the hash changed" behavior is exactly what is
+ * under test.
+ */
+
+/**
+ * Deterministic content hash of a file set. Order-independent (sorted keys)
+ * and byte-stable for a given `files` value — the same set of paths and
+ * contents always hashes identically, which is what makes "only re-sync on
+ * change" possible: a caller persists this string (e.g. in
+ * `SandboxRegistry.roLayerHash`) and compares it next time before writing
+ * anything.
+ */
+export function computeContentHash(files: Readonly<Record<string, string>>): string {
+  const hash = createHash('sha256');
+  for (const path of Object.keys(files).sort()) {
+    hash.update(path, 'utf8');
+    hash.update('\0', 'utf8');
+    hash.update(files[path] as string, 'utf8');
+    hash.update('\0', 'utf8');
+  }
+  return hash.digest('hex');
+}
+
+export interface SyncReadOnlyLayerResult {
+  readonly hash: string;
+  /** False when `previousHash` already matched — nothing was written. */
+  readonly synced: boolean;
+  /** Paths actually written this call. Empty when `synced` is false. */
+  readonly writtenPaths: readonly string[];
+}
+
+/**
+ * Materialize `files` into `sandbox` — but only when their combined content
+ * hash differs from `previousHash`. On a genuine change, every file in the
+ * set is (re)written; a partial diff (only the changed files) is
+ * deliberately NOT attempted here — the issue's design point is skipping
+ * the sync ENTIRELY when nothing changed, not minimizing bytes written on
+ * a real change, and per-file diffing would need a per-file hash map this
+ * module does not carry (a legitimate future refinement, not a shortfall
+ * of this primitive's stated contract).
+ */
+export async function syncReadOnlyLayer(
+  sandbox: { write(relativePath: string, content: string): Promise<{ ok: boolean }> },
+  files: Readonly<Record<string, string>>,
+  previousHash: string | undefined,
+): Promise<SyncReadOnlyLayerResult> {
+  const hash = computeContentHash(files);
+  if (hash === previousHash) {
+    return { hash, synced: false, writtenPaths: [] };
+  }
+  const writtenPaths: string[] = [];
+  for (const path of Object.keys(files).sort()) {
+    const result = await sandbox.write(path, files[path] as string);
+    if (result.ok) writtenPaths.push(path);
+  }
+  return { hash, synced: true, writtenPaths };
+}

middleware/packages/harness-sandbox/src/dockerSandbox.ts

@@ -3,6 +3,7 @@ import { createHash } from 'node:crypto';
 import type { AgentComputerProfile } from './agentComputerProfile.js';
 import { execDockerViaSpawn, type DockerExec } from './dockerExec.js';
 import { clampSandboxPathPosix } from './pathGuard.js';
+import type { SandboxRegistry } from './sandboxRegistry.js';
 import type {
   Sandbox,
   SandboxBackend,
@@ -46,6 +47,18 @@ export interface DockerSandboxBackendOptions {
   readonly workDir?: string;
   /** Test seam — see `dockerExec.ts`. Defaults to the real `docker` CLI. */
   readonly execDocker?: DockerExec;
+  /**
+   * #576 P3 — optional durable bookkeeping. Omitted (the default): this
+   * backend behaves EXACTLY as P1/P2 shipped it — deterministic container
+   * naming is the only durability, `provision()` never touches anything
+   * beyond Docker itself. Provided: `provision()` additionally records
+   * (scope → container name) with a `lastUsedAt` the reaper can act on, and
+   * re-attaches via the registry's stored `sandboxRef` rather than
+   * recomputing the deterministic name — the seam a future non-deterministic
+   * backend (one where Docker/the platform assigns the id) would need, kept
+   * exercised now via the Docker backend's own deterministic case.
+   */
+  readonly registry?: SandboxRegistry;
 }
 
 const DEFAULT_IMAGE = 'alpine:3.20';
@@ -67,21 +80,35 @@ export class DockerSandboxBackend implements SandboxBackend {
    *  than re-probing Docker every time. Cross-process durability comes from
    *  the deterministic container name, not from this map. */
   private readonly live = new Map<string, DockerSandbox>();
+  private readonly registry: SandboxRegistry | undefined;
 
   constructor(options: DockerSandboxBackendOptions = {}) {
     this.image = options.image ?? DEFAULT_IMAGE;
     this.workDir = options.workDir ?? DEFAULT_WORK_DIR;
     this.execDocker = options.execDocker ?? execDockerViaSpawn;
+    this.registry = options.registry;
   }
 
   async provision(args: {
     readonly scopeKey: string;
     readonly profile: AgentComputerProfile;
   }): Promise<Sandbox> {
     const cached = this.live.get(args.scopeKey);
-    if (cached) return cached;
+    if (cached) {
+      // #576 P3: even a process-local cache hit is real usage — the reaper
+      // must not treat a scope as idle while its sandbox is actively being
+      // reused just because Docker itself wasn't consulted this call.
+      if (this.registry) await this.registry.touch(args.scopeKey, new Date());
+      return cached;
+    }
+
+    // #576 P3: a registered scope re-attaches via its STORED reference
+    // rather than recomputing the deterministic name — the seam a future
+    // non-deterministic backend needs, exercised here even though this
+    // backend's own name is always recomputable.
+    const registered = this.registry ? await this.registry.get(args.scopeKey) : undefined;
+    const name = registered?.sandboxRef ?? containerNameFor(args.scopeKey);
 
-    const name = containerNameFor(args.scopeKey);
     const exists = await this.containerExists(name);
     if (!exists) {
       await this.runContainer(name, args.profile);
@@ -92,6 +119,16 @@ export class DockerSandboxBackend implements SandboxBackend {
       await this.exec(['start', name], { timeoutMs: 15_000 });
     }
 
+    if (this.registry) {
+      await this.registry.upsert({
+        scopeKey: args.scopeKey,
+        backend: 'docker',
+        sandboxRef: name,
+        profile: args.profile,
+        now: new Date(),
+      });
+    }
+
     const sandbox = new DockerSandbox({
       id: name,
       scopeKey: args.scopeKey,

middleware/packages/harness-sandbox/src/index.ts

@@ -30,3 +30,21 @@ export { execDockerViaSpawn } from './dockerExec.js';
 
 export type { DockerSandboxBackendOptions } from './dockerSandbox.js';
 export { DockerSandboxBackend } from './dockerSandbox.js';
+
+export type { SyncReadOnlyLayerResult } from './contentHash.js';
+export { computeContentHash, syncReadOnlyLayer } from './contentHash.js';
+
+export type {
+  SandboxRegistry,
+  SandboxRegistryEntry,
+  SandboxRegistryUpsertInput,
+} from './sandboxRegistry.js';
+export { InMemorySandboxRegistry } from './sandboxRegistry.js';
+
+export { PostgresSandboxRegistry } from './postgresSandboxRegistry.js';
+
+export type {
+  ReapOrphanedSandboxesOptions,
+  ReapOrphanedSandboxesResult,
+} from './reaper.js';
+export { reapOrphanedSandboxes } from './reaper.js';

middleware/packages/harness-sandbox/src/postgresSandboxRegistry.ts

@@ -0,0 +1,93 @@
+import type { Pool } from 'pg';
+
+import type { AgentComputerProfile } from './agentComputerProfile.js';
+import type {
+  SandboxRegistry,
+  SandboxRegistryEntry,
+  SandboxRegistryUpsertInput,
+} from './sandboxRegistry.js';
+
+/**
+ * Postgres-backed `SandboxRegistry` — migration `0044_sandbox_registry.sql`.
+ * Schema is intentionally narrow (see that file's header for the numbering
+ * note). `profile` round-trips through `JSONB` verbatim; nothing here
+ * validates its shape beyond `JSON.parse` succeeding — the caller (P2's
+ * `execute` tool wiring, or a future consumer) owns the `AgentComputerProfile`
+ * contract, this store just persists whatever it is handed.
+ */
+export class PostgresSandboxRegistry implements SandboxRegistry {
+  constructor(private readonly pool: Pool) {}
+
+  async get(scopeKey: string): Promise<SandboxRegistryEntry | undefined> {
+    const result = await this.pool.query<RegistryRow>(
+      `SELECT scope_key, backend, sandbox_ref, profile, ro_layer_hash, created_at, last_used_at
+       FROM sandbox_registry WHERE scope_key = $1`,
+      [scopeKey],
+    );
+    const row = result.rows[0];
+    return row ? rowToEntry(row) : undefined;
+  }
+
+  async upsert(input: SandboxRegistryUpsertInput): Promise<SandboxRegistryEntry> {
+    const result = await this.pool.query<RegistryRow>(
+      `INSERT INTO sandbox_registry (scope_key, backend, sandbox_ref, profile, ro_layer_hash, created_at, last_used_at)
+       VALUES ($1, $2, $3, $4::jsonb, $5, $6, $6)
+       ON CONFLICT (scope_key) DO UPDATE SET
+         backend = EXCLUDED.backend,
+         sandbox_ref = EXCLUDED.sandbox_ref,
+         profile = EXCLUDED.profile,
+         ro_layer_hash = COALESCE(EXCLUDED.ro_layer_hash, sandbox_registry.ro_layer_hash),
+         last_used_at = EXCLUDED.last_used_at
+       RETURNING scope_key, backend, sandbox_ref, profile, ro_layer_hash, created_at, last_used_at`,
+      [
+        input.scopeKey,
+        input.backend,
+        input.sandboxRef,
+        JSON.stringify(input.profile),
+        input.roLayerHash ?? null,
+        input.now.toISOString(),
+      ],
+    );
+    return rowToEntry(result.rows[0] as RegistryRow);
+  }
+
+  async touch(scopeKey: string, now: Date): Promise<void> {
+    await this.pool.query(`UPDATE sandbox_registry SET last_used_at = $2 WHERE scope_key = $1`, [
+      scopeKey,
+      now.toISOString(),
+    ]);
+  }
+
+  async delete(scopeKey: string): Promise<void> {
+    await this.pool.query(`DELETE FROM sandbox_registry WHERE scope_key = $1`, [scopeKey]);
+  }
+
+  async listAll(): Promise<readonly SandboxRegistryEntry[]> {
+    const result = await this.pool.query<RegistryRow>(
+      `SELECT scope_key, backend, sandbox_ref, profile, ro_layer_hash, created_at, last_used_at FROM sandbox_registry`,
+    );
+    return result.rows.map(rowToEntry);
+  }
+}
+
+interface RegistryRow {
+  scope_key: string;
+  backend: string;
+  sandbox_ref: string;
+  profile: AgentComputerProfile;
+  ro_layer_hash: string | null;
+  created_at: Date;
+  last_used_at: Date;
+}
+
+function rowToEntry(row: RegistryRow): SandboxRegistryEntry {
+  return {
+    scopeKey: row.scope_key,
+    backend: row.backend,
+    sandboxRef: row.sandbox_ref,
+    profile: row.profile,
+    ...(row.ro_layer_hash !== null ? { roLayerHash: row.ro_layer_hash } : {}),
+    createdAt: new Date(row.created_at),
+    lastUsedAt: new Date(row.last_used_at),
+  };
+}

middleware/packages/harness-sandbox/src/reaper.ts

@@ -0,0 +1,72 @@
+import type { SandboxRegistry } from './sandboxRegistry.js';
+
+/**
+ * Issue #576 P3 — reaper for orphaned (idle, non-persistent) sandboxes.
+ *
+ * ## The #709/#710 clock-race lesson, applied here
+ *
+ * `ipv6-bind-ipv4-dial-test-flake-pr703` / the #709 reaper-test race taught
+ * this codebase that an idle-timeout check must anchor "now" to a clock
+ * INDEPENDENT of the row being checked — never derive "now" from the same
+ * field (or a sibling row's copy of it) you are comparing against, because
+ * that makes the comparison self-referential and racy (a row updated
+ * between "compute now" and "compare" moves the goalposts).
+ *
+ * `reapOrphanedSandboxes` therefore takes `now` as a REQUIRED, externally
+ * supplied parameter (a plain `Date`, never `new Date()` computed inside
+ * this function, never derived from `entries`). The caller's own clock is
+ * the anchor; the entries are the thing being checked. Tests assert this
+ * directly: an entry catalog and an independently-chosen `now` are passed
+ * in, never the other way around.
+ *
+ * ## What counts as orphaned
+ *
+ * `profile.persistent === true` sandboxes are NEVER reaped by idle time —
+ * that is the entire point of `persistent`. Only non-persistent entries
+ * whose `lastUsedAt` is older than `now - idleThresholdMs` are candidates.
+ */
+export interface ReapOrphanedSandboxesOptions {
+  readonly registry: SandboxRegistry;
+  /** Tears down the backend-specific sandbox for a given `sandboxRef`. */
+  readonly teardown: (sandboxRef: string) => Promise<void>;
+  /** The clock anchor — see the module doc. Required, never defaulted to
+   *  `new Date()` internally. */
+  readonly now: Date;
+  readonly idleThresholdMs: number;
+}
+
+export interface ReapOrphanedSandboxesResult {
+  readonly reapedScopeKeys: readonly string[];
+  /** Scope keys whose teardown call threw — left in the registry so a
+   *  retry can find them again rather than losing track of a sandbox that
+   *  may still be running. */
+  readonly failedScopeKeys: readonly string[];
+}
+
+export async function reapOrphanedSandboxes(
+  options: ReapOrphanedSandboxesOptions,
+): Promise<ReapOrphanedSandboxesResult> {
+  const entries = await options.registry.listAll();
+  const cutoff = options.now.getTime() - options.idleThresholdMs;
+
+  const reapedScopeKeys: string[] = [];
+  const failedScopeKeys: string[] = [];
+
+  for (const entry of entries) {
+    if (entry.profile.persistent) continue;
+    if (entry.lastUsedAt.getTime() >= cutoff) continue;
+
+    try {
+      await options.teardown(entry.sandboxRef);
+      await options.registry.delete(entry.scopeKey);
+      reapedScopeKeys.push(entry.scopeKey);
+    } catch {
+      // Best-effort: a teardown failure must not abort the sweep for the
+      // remaining entries, and the registry row is deliberately LEFT so the
+      // next sweep retries it rather than the sandbox becoming untracked.
+      failedScopeKeys.push(entry.scopeKey);
+    }
+  }
+
+  return { reapedScopeKeys, failedScopeKeys };
+}