fix(runner): applied-state identity and safe teardown (lifecycle steps 1-2)

The environment now owns its applied state; park and repark no longer
accept request-derived fingerprints, so the approval-stale-config bug is
unrepresentable. Four named teardown reasons with a parkable allowlist:
a config change stops the sandbox instead of deleting it, and true
incompatibility still destroys with the reconnect pointer cleared.
Revision id, version, and draft flag leave the fingerprint, so a commit
with identical content keeps the warm session.

Author: mmabrouk

services/runner/src/engines/sandbox_agent/applied-state.ts

@@ -0,0 +1,88 @@
+/**
+ * `AppliedEnvironmentState` — what an environment ACTUALLY has installed.
+ *
+ * LIFECYCLE MIGRATION, STEP 2. This is the smallest change that kills the stale-config bug class.
+ *
+ * The bug. Until now the pool stored a `configFingerprint` its CALLER supplied, and the caller
+ * supplied the INCOMING request's fingerprint. On the ordinary path that is harmless, because the
+ * dispatch already proved the incoming and parked configurations equal. On the approval-resume
+ * path it is not: that branch never compares configurations, so the re-park stamped a
+ * configuration the environment had never applied. The next turn then read that stamp, found a
+ * match, and continued warm on an environment running something else.
+ *
+ * The fix is structural, not a patch. A request says what somebody WANTED. Only the environment
+ * knows what it GOT. So the environment owns its applied state, and the pool reads it. There is
+ * no fingerprint parameter left for a caller to stamp, which makes the bug unrepresentable rather
+ * than merely fixed.
+ *
+ * Scope note. This slice carries one facet, `configFingerprint`, because that is the facet the
+ * pool compares today. The richer shape in the lifecycle design (sandbox, runtime, mounts,
+ * workspace, and harness-session facets, each with its own generation) arrives with the
+ * reconciliation router. `generation` is here from the start so a later facet split has a counter
+ * to build on.
+ */
+
+/**
+ * The state an environment has successfully installed. Read-only to everyone except
+ * `commitApplied`.
+ */
+export interface AppliedEnvironmentState {
+  /**
+   * Increments on every successful commit. It is a monotonic counter for logs and for tests, and
+   * it never re-enters environment identity. A test asserts that two commits of the same
+   * fingerprint still advance it, so "nothing changed" and "we re-applied" stay distinguishable.
+   */
+  readonly generation: number;
+  /**
+   * The canonical hash of the configuration this environment actually runs. It is stamped from a
+   * SUCCESSFUL acquire, never from an incoming request.
+   */
+  readonly configFingerprint: string;
+}
+
+/**
+ * The structural contract the pool needs. It is deliberately minimal: the pool must stay
+ * engine-agnostic, so it constrains its environment type to this shape rather than importing the
+ * engine.
+ */
+export interface AppliedStateOwner {
+  readonly appliedState: AppliedEnvironmentState;
+}
+
+/**
+ * A mutable holder for one environment's applied state.
+ *
+ * `commitApplied` is the ONLY way to advance it. Every caller must be a lifecycle action that
+ * already succeeded. Committing before the action succeeds recreates the bug this module exists
+ * to remove.
+ */
+export class AppliedState implements AppliedStateOwner {
+  #generation: number;
+  #configFingerprint: string;
+
+  constructor(configFingerprint: string) {
+    this.#generation = 1;
+    this.#configFingerprint = configFingerprint;
+  }
+
+  get appliedState(): AppliedEnvironmentState {
+    // A fresh object each read, so a caller cannot hold a reference and mutate it later.
+    return {
+      generation: this.#generation,
+      configFingerprint: this.#configFingerprint,
+    };
+  }
+
+  /**
+   * Record a lifecycle action that ALREADY SUCCEEDED.
+   *
+   * Call this after the action, never before and never instead of it. A `setModel` that threw, a
+   * workspace refresh that failed halfway, or a session reopen that lost its native history must
+   * leave applied state exactly where it was. That is the partial-reconciliation rule from the
+   * lifecycle design, and it is why this method takes a result rather than a desired value.
+   */
+  commitApplied(result: { configFingerprint: string }): void {
+    this.#generation += 1;
+    this.#configFingerprint = result.configFingerprint;
+  }
+}

services/runner/src/engines/sandbox_agent/environment-setup.ts

@@ -1,6 +1,7 @@
 import { rmSync } from "node:fs";
 
 import { apiBase } from "../../apiBase.ts";
+import { AppliedState } from "./applied-state.ts";
 
 import { resolveRunSessionId, type AgentRunRequest } from "../../protocol.ts";
 import { type ClientToolOutcome } from "../../responder.ts";
@@ -32,6 +33,7 @@ import {
   type PiModelConfigPlan,
 } from "./pi-model-config.ts";
 import { buildRunPlan } from "./run-plan.ts";
+import { configFingerprint } from "./session-identity.ts";
 import type {
   SandboxAgentDeps,
   SessionEnvironment,
@@ -368,7 +370,16 @@ export async function prepareEnvironmentSetup(
   // so its handler is torn down deterministically and cannot write a result after the turn ends.
   const mcpAbort = new AbortController();
 
+  // LIFECYCLE MIGRATION, STEP 2. The environment owns what it applied. It is seeded from the
+  // request that is building it, because that request IS what this environment installs. Every
+  // later change must go through `commitApplied`, and only after the change succeeds.
+  const applied = new AppliedState(configFingerprint(request));
+
   const environment: SessionEnvironment = {
+    get appliedState() {
+      return applied.appliedState;
+    },
+    commitApplied: (result) => applied.commitApplied(result),
     plan,
     logger,
     deps,

services/runner/src/engines/sandbox_agent/environment.ts

@@ -108,7 +108,10 @@ import {
   routeSessionEventToActiveTurn,
 } from "./session-events.ts";
 import { buildSandboxProvider } from "./provider.ts";
-import { readStoredSandboxPointer } from "./sandbox-reconnect.ts";
+import {
+  markSandboxDestroyed,
+  readStoredSandboxPointer,
+} from "./sandbox-reconnect.ts";
 import type {
   AcquireEnvironmentResult,
   SandboxAgentDeps,
@@ -322,7 +325,15 @@ export async function acquireEnvironment(
         );
       }
     }
-    if (!parked) await environment.sandbox?.destroySandbox().catch(() => {});
+    if (!parked) {
+      // Record the id BEFORE the delete call, and record it even when the call throws. A delete
+      // that failed may still have removed the sandbox, so reconnecting to it is a wasted round
+      // trip either way. See `markSandboxDestroyed`.
+      markSandboxDestroyed(
+        environment.sandbox?.sandboxId ?? plan.sandboxId ?? undefined,
+      );
+      await environment.sandbox?.destroySandbox().catch(() => {});
+    }
     await environment.sandbox?.dispose().catch(() => {});
     // Unmount the durable cwd BEFORE removing the dir: data lives in the store, only the host
     // mountpoint is torn down. If unmount is not CONFIRMED gone, skip the delete: rmSync must

services/runner/src/engines/sandbox_agent/runtime-contracts.ts

@@ -42,6 +42,7 @@ import {
 } from "./session-continuity-durable.ts";
 import { type SessionContinuityStore } from "./session-continuity.ts";
 import { type InstalledMountExpiries } from "./session-identity.ts";
+import type { AppliedEnvironmentState } from "./applied-state.ts";
 import { type TeardownReason } from "./teardown.ts";
 import { uploadToolMcpAssets } from "./tool-mcp-assets.ts";
 import { prepareWorkspace } from "./workspace.ts";
@@ -217,6 +218,17 @@ export function sendLastMessageOnly(opts: RunTurnOptions): boolean {
  * call. Per-turn state rides `currentTurn`, swapped in by `runTurn`.
  */
 export interface SessionEnvironment {
+  /**
+   * What this environment ACTUALLY has installed.
+   *
+   * LIFECYCLE MIGRATION, STEP 2. The pool reads this instead of a fingerprint its caller supplies,
+   * so a request can no longer stamp a configuration the environment never applied. Only
+   * `commitApplied` advances it, and only after a lifecycle action succeeds. See
+   * `applied-state.ts`.
+   */
+  readonly appliedState: AppliedEnvironmentState;
+  /** Record a lifecycle action that already succeeded. The only writer of `appliedState`. */
+  commitApplied: (result: { configFingerprint: string }) => void;
   plan: RunPlan;
   logger: Log;
   deps: SandboxAgentDeps;

services/runner/src/engines/sandbox_agent/sandbox-reconnect.ts

@@ -12,6 +12,44 @@
  */
 import { fetchLatestSessionTurn } from "./session-continuity-durable.ts";
 
+/**
+ * Sandbox ids this runner process has DELETED.
+ *
+ * LIFECYCLE MIGRATION, STEP 1. The stored pointer is the latest turn's `sandbox_id`, and the turns
+ * table is append-only, so there is no pointer row to clear when a destroy deletes the sandbox.
+ * The dead id therefore stays readable until the next turn appends its own row. Reconnecting to it
+ * fails and falls through to a fresh create, which is safe but spends a provider round trip on a
+ * sandbox we ourselves deleted moments earlier.
+ *
+ * This set closes that window inside one runner process: `markSandboxDestroyed` records the id at
+ * the moment of deletion, and `readStoredSandboxPointer` refuses to hand it back.
+ *
+ * What it deliberately does NOT do: it is per-process and in-memory, so another replica, or this
+ * replica after a restart, still reads the stale id and still falls through to a fresh create.
+ * That path was always correct and stays correct. This is a latency fix with a correctness-shaped
+ * name, and treating it as a durable guarantee would be wrong.
+ */
+const destroyedSandboxIds = new Set<string>();
+
+/** Cap the set so a long-lived replica cannot grow it without bound. */
+const DESTROYED_SANDBOX_ID_MAX = 512;
+
+/** Record that this process deleted `sandboxId`, so it never reconnects to it. */
+export function markSandboxDestroyed(sandboxId: string | undefined): void {
+  if (!sandboxId) return;
+  if (destroyedSandboxIds.size >= DESTROYED_SANDBOX_ID_MAX) {
+    // Drop the oldest entry. Losing one only costs the failed-reconnect round trip it saved.
+    const oldest = destroyedSandboxIds.values().next().value;
+    if (oldest !== undefined) destroyedSandboxIds.delete(oldest);
+  }
+  destroyedSandboxIds.add(sandboxId);
+}
+
+/** Test seam: forget every recorded id. */
+export function resetDestroyedSandboxIds(): void {
+  destroyedSandboxIds.clear();
+}
+
 export interface SandboxPointerDeps {
   apiBase?: string;
   authorization: string;
@@ -35,5 +73,11 @@ export async function readStoredSandboxPointer(
   const latest = await fetchLatestSessionTurn(sessionId, undefined, deps);
   const id = latest?.sandbox_id;
   if (typeof id !== "string" || id.length === 0) return undefined;
+  if (destroyedSandboxIds.has(id)) {
+    // This process deleted that sandbox. Reconnecting would fail, so skip straight to a fresh
+    // create. See `destroyedSandboxIds`.
+    deps.log?.(`ignoring pointer to sandbox=${id} destroyed by this runner`);
+    return undefined;
+  }
   return { sandboxId: id };
 }

services/runner/src/engines/sandbox_agent/session-identity.ts

@@ -195,9 +195,16 @@ function canonicalJson(value: unknown): string {
  * every hash, the credential epoch included — each turn's relay uses the INCOMING request's
  * `toolCallback` (see `CredentialEpoch` and `run-turn.ts`), so the parked copy never executes
  * anything.
+ *
+ * LIFECYCLE MIGRATION, STEP 1. The workflow REVISION id, the revision version, and the draft flag
+ * were in this hash and are now out. They are turn METADATA, not environment identity: nothing in
+ * the sandbox, the daemon, the workspace, or the harness session changes when a revision id
+ * changes. Keeping them here meant that committing a revision mid-conversation threw away a warm
+ * sandbox that was still perfectly usable, which is the exact cost this project exists to remove.
+ * They stay in `runContext` for tool binding and observability; they simply no longer decide
+ * whether an environment may be reused.
  */
 export function configFingerprint(request: AgentRunRequest): string {
-  const workflow = request.runContext?.workflow;
   const shape = {
     harness: request.harness ?? null,
     sandbox: request.sandbox ?? null,
@@ -247,13 +254,7 @@ export function configFingerprint(request: AgentRunRequest): string {
     permissions: request.permissions ?? null,
     sandboxPermission: request.sandboxPermission ?? null,
     harnessFiles: request.harnessFiles ?? null,
-    workflowRevision: workflow?.revision
-      ? {
-          id: workflow.revision.id ?? null,
-          version: workflow.revision.version ?? null,
-        }
-      : null,
-    isDraft: workflow?.is_draft ?? null,
+    // No `workflowRevision` and no `isDraft`. See the doc comment above.
   };
   return sha256(canonicalJson(shape));
 }

services/runner/src/engines/sandbox_agent/session-pool.ts

@@ -12,6 +12,7 @@
  * never imports the engine, so it stays a pure map + timer + policy unit. Operators can disable
  * it explicitly with `AGENTA_RUNNER_SESSION_KEEPALIVE=off`.
  */
+import type { AppliedStateOwner } from "./applied-state.ts";
 import type { CredentialEpoch, KeepaliveConfig } from "./session-identity.ts";
 import type { TeardownReason } from "./teardown.ts";
 
@@ -27,10 +28,19 @@ export type SessionState = "busy" | "idle" | "awaiting_approval" | "destroyed";
  * One parked live session. `environment` is opaque to the pool (the engine reads it on a
  * continuation). `teardown` is the engine's complete, idempotent teardown closure.
  */
-export interface LiveSession<E = unknown> {
+export interface LiveSession<E extends AppliedStateOwner = AppliedStateOwner> {
   key: string;
   environment: E;
-  configFingerprint: string;
+  /**
+   * The configuration this session's environment ACTUALLY runs.
+   *
+   * LIFECYCLE MIGRATION, STEP 2. This is a READ-ONLY view of `environment.appliedState`, not a
+   * stored copy. It used to be a field the caller wrote at park time, and the approval-resume
+   * path wrote the INCOMING request's value into it — a configuration the environment had never
+   * applied. Reading through to the environment removes the field a caller could stamp, so that
+   * bug can no longer be written.
+   */
+  readonly configFingerprint: string;
   historyFingerprint: string;
   /**
    * Whether the request this session parked from asserted a transcript beyond its own turn. A
@@ -49,11 +59,15 @@ export interface LiveSession<E = unknown> {
   teardownPromise?: Promise<void>;
 }
 
-/** Fields the caller supplies to park a session (the pool arms the timer and state itself). */
-export interface ParkInput<E> {
+/**
+ * Fields the caller supplies to park a session (the pool arms the timer and state itself).
+ *
+ * There is deliberately NO `configFingerprint` here. The environment owns that, and the pool
+ * reads it. See `LiveSession.configFingerprint`.
+ */
+export interface ParkInput<E extends AppliedStateOwner> {
   key: string;
   environment: E;
-  configFingerprint: string;
   historyFingerprint: string;
   /** See `LiveSession.historyAsserted`. Omitted defaults to the strictest resume check. */
   historyAsserted?: boolean;
@@ -66,7 +80,7 @@ export interface ParkInput<E> {
  * (Node), so check-and-set on a key needs no lock. All teardown routes through the session's
  * one idempotent `teardown`.
  */
-export class SessionPool<E = unknown> {
+export class SessionPool<E extends AppliedStateOwner = AppliedStateOwner> {
   private readonly sessions = new Map<string, LiveSession<E>>();
 
   constructor(
@@ -156,7 +170,6 @@ export class SessionPool<E = unknown> {
   async repark(
     session: LiveSession<E>,
     update: {
-      configFingerprint: string;
       historyFingerprint: string;
       /** See `LiveSession.historyAsserted`. Omitted defaults to the strictest resume check. */
       historyAsserted?: boolean;
@@ -182,7 +195,8 @@ export class SessionPool<E = unknown> {
       this.sessions.set(session.key, session);
     }
     this.clearTimer(session);
-    session.configFingerprint = update.configFingerprint;
+    // No `configFingerprint` assignment. It reads through to the environment, which is the only
+    // thing that knows what it actually applied.
     session.historyFingerprint = update.historyFingerprint;
     session.historyAsserted = update.historyAsserted ?? true;
     session.credentialEpoch = update.credentialEpoch;
@@ -225,10 +239,14 @@ export class SessionPool<E = unknown> {
       return false;
     }
 
+    const environment = input.environment;
     const session: LiveSession<E> = {
       key: input.key,
-      environment: input.environment,
-      configFingerprint: input.configFingerprint,
+      environment,
+      // A getter, not a copy: the pool always reports what the environment currently has applied.
+      get configFingerprint() {
+        return environment.appliedState.configFingerprint;
+      },
       historyFingerprint: input.historyFingerprint,
       historyAsserted: input.historyAsserted ?? true,
       credentialEpoch: input.credentialEpoch,