🧹 fix: Await Job Cleanup and Measure Retained Resources

.github/workflows/ci.yml

@@ -37,6 +37,9 @@ jobs:
       - name: Sandbox-runner liveness checks
         run: tests/sandbox_runner_healthcheck.sh
 
+      - name: Sandbox-runner metrics discovery and network policy
+        run: tests/sandbox_runner_metrics.sh
+
       - name: Bridge pairing rollout safety
         run: tests/bridge_pairing_rollout.sh
 
@@ -121,6 +124,17 @@ jobs:
       - name: Bun tests
         run: bun run test
 
+      - name: File-heavy workspace cleanup on tmpfs
+        run: |
+          docker run --rm --user 0 \
+            --tmpfs /tmp:rw,size=1g \
+            --mount "type=bind,source=$GITHUB_WORKSPACE,target=/work,readonly" \
+            --workdir /work/api \
+            --env SANDBOX_CLEANUP_TMPFS_TEST=1 \
+            --env SANDBOX_LOG_LEVEL=error \
+            oven/bun:1.3.14-debian \
+            bun test src/cleanup.integration.test.ts
+
   service-unit-tests:
     name: Service Unit Tests
     runs-on: ubuntu-latest

api/README.md

@@ -139,3 +139,70 @@ validation behavior.
 Roll out this sandbox behavior before enabling timeout forwarding in the
 service's plain `/exec` handler. Older sandboxes reject caps above their local
 runtime limit; older services remain compatible with updated sandboxes.
+
+### Runner memory after cleanup
+
+`Job.cleanup()` emits `Post-cleanup resources` after workspace removal and UID
+release (or quarantine). `/execute` now waits for cleanup before sending success
+or execution-failure responses. Artifact upload still finishes before cleanup.
+Persistent session workspaces and their pinned UIDs are intentionally preserved;
+failed disposable cleanup still quarantines the directory and retains its UID
+for the existing retry path. This change does not alter those policies.
+
+The following metrics use only fixed, low-cardinality labels:
+
+| Metric suffix (prefix `codeapi_sandbox_`) | Meaning |
+| --- | --- |
+| `post_cleanup_memory_bytes{kind}` | `current`, `anon`, `file`, `shmem` at the visible cgroup v2 mount root |
+| `post_cleanup_tmp_used{resource}` | `/tmp` allocated `bytes`, allocated `inodes`, and `is_tmpfs` (0 or 1) |
+| `post_cleanup_workspaces{kind}` | Remaining `disposable`, `session`, and `other` entries in `/tmp/sandbox` |
+| `cleanup_total{mode,outcome}` | Disposable/session cleanup attempts: `removed`, `preserved`, `retained`, `error` |
+| `post_cleanup_sample_success{source}` | Whether `memory`, `tmp`, or `workspaces` was readable on the last sample |
+| `post_cleanup_timestamp_seconds` | When the last cleanup sample was taken |
+
+These are runner-wide **last-cleanup samples**, not live gauges or per-job
+memory attribution. Other jobs can still be running. Reaper-only changes are
+visible on the next job cleanup, not immediately. Interpret the workspace counts
+alongside active executions and the sample timestamp. Unavailable sources are
+reported as unavailable and their old gauge values are removed, never replaced
+with a healthy-looking zero. The structured log also includes active UID slots
+and the number of retained cleanup retries. Synthetic jobs update metrics while
+suppressing successful per-job logs as before.
+
+The visible cgroup root covers the API and sibling NsJail cgroups. The old
+`Post-execution memory` log instead samples `/proc/self/cgroup` **before** job
+cleanup, so it can have a different scope. `file` includes `shmem`; do not add
+them or assume all `file` memory is reclaimable page cache.
+
+`statfs` measures allocation, including deleted-but-open files on the sampled
+mount, without walking user files. The runner's 1 GiB `/tmp` mount is distinct
+from each jail's 20 MiB `/tmp` mount. Its ceiling does not cap all container
+memory. If workspace counts fall but shmem does not, investigate descendant
+processes and retained mounts as well as files. A stable API-process FD count
+alone cannot exclude those cases. Memory requests and HPA settings are unchanged.
+
+#### Cleanup stress tests
+
+Ordinary unit tests cover metrics, unavailable sources, session/disposable
+classification and response ordering. The opt-in stress tests must run in an
+**isolated test container**, never on an active runner:
+
+```bash
+# From api/, with /tmp mounted as tmpfs and per-job chown available:
+SANDBOX_CLEANUP_TMPFS_TEST=1 bun test src/cleanup.integration.test.ts
+
+# Additionally requires the runner's NsJail binary, spec-guard, config,
+# and normal namespace/cgroup permissions. Run this file alone.
+NSJAIL_CONFIG=/sandbox_api/config/sandbox.cfg \
+SANDBOX_CLEANUP_NSJAIL_TEST=1 bun test src/cleanup.integration.test.ts
+```
+
+The first test repeats real Job priming and cleanup with large payloads and many
+small files, checking tmpfs bytes/inodes and UID slots after every iteration.
+CI runs it in a dedicated Bun container with a 1 GiB `/tmp` tmpfs. It does not
+execute NsJail. The second test uses real NsJail jobs covering normal completion,
+timeout and output overflow, including a detached child holding an unlinked file.
+It asserts no processes remain under the job UID before removing its workspace,
+then checks workspace removal and post-cleanup tmpfs allocation. This test is
+skipped unless explicitly enabled; a unit-test pass is not proof of namespace
+teardown on the production kernel.

api/src/api/v2-cleanup.test.ts

@@ -0,0 +1,189 @@
+import { afterAll, afterEach, beforeAll, expect, test } from 'bun:test';
+import express from 'express';
+import { mkdtemp, rm, writeFile } from 'fs/promises';
+import type { Server } from 'http';
+import { tmpdir } from 'os';
+import { join } from 'path';
+import { config } from '../config';
+import { Job } from '../job';
+import { loadPackage } from '../runtime';
+import { ValidationError } from '../validation';
+import router from './v2';
+
+let server: Server;
+let url: string;
+let directory: string;
+const language = 'cleanup-order-test';
+const originalPrime = Job.prototype.prime;
+const originalExecute = Job.prototype.execute;
+const originalUpload = Job.prototype.uploadGeneratedFiles;
+const originalCleanup = Job.prototype.cleanup;
+const requireManifest = config.require_execution_manifest;
+
+beforeAll(async () => {
+  directory = await mkdtemp(join(tmpdir(), 'cleanup-order-'));
+  await writeFile(
+    join(directory, 'pkg-info.json'),
+    JSON.stringify({ language, version: '1.0.0', aliases: [] })
+  );
+  loadPackage(directory);
+  const app = express();
+  app.use(router);
+  await new Promise<void>(resolve => {
+    server = app.listen(0, '127.0.0.1', () => resolve());
+  });
+  const address = server.address();
+  url = `http://127.0.0.1:${
+    typeof address === 'object' && address ? address.port : 0
+  }/execute`;
+});
+afterAll(async () => {
+  await new Promise<void>(resolve => server.close(() => resolve()));
+  await rm(directory, { recursive: true, force: true });
+});
+afterEach(() => {
+  Job.prototype.prime = originalPrime;
+  Job.prototype.execute = originalExecute;
+  Job.prototype.uploadGeneratedFiles = originalUpload;
+  Job.prototype.cleanup = originalCleanup;
+  config.require_execution_manifest = requireManifest;
+});
+
+for (const outcome of [
+  'success',
+  'prime_failure',
+  'execution_failure',
+  'validation_failure',
+] as const) {
+  test(`${outcome} waits for cleanup before sending a response and cleans exactly once`, async () => {
+    config.require_execution_manifest = false;
+    const events: string[] = [];
+    let releaseCleanup!: () => void;
+    const cleanupGate = new Promise<void>(resolve => {
+      releaseCleanup = resolve;
+    });
+    let markCleanupStarted!: () => void;
+    const cleanupStarted = new Promise<void>(resolve => {
+      markCleanupStarted = resolve;
+    });
+    Job.prototype.prime = async function () {
+      events.push('prime');
+      if (outcome === 'prime_failure')
+        throw new Error('scripted prime failure');
+    };
+    Job.prototype.execute = async function () {
+      events.push('execute');
+      if (outcome === 'execution_failure')
+        throw new Error('scripted execution failure');
+      if (outcome === 'validation_failure')
+        throw new ValidationError('scripted validation failure');
+      return { files: [{ id: 'artifact', name: 'result.txt' }] } as Awaited<
+        ReturnType<Job['execute']>
+      >;
+    };
+    Job.prototype.uploadGeneratedFiles = async function () {
+      events.push('upload');
+      return new Set(['artifact']);
+    };
+    Job.prototype.cleanup = async function () {
+      events.push('cleanup-start');
+      markCleanupStarted();
+      await cleanupGate;
+      events.push('cleanup-end');
+    };
+    let responseArrived = false;
+    const pending = fetch(url, {
+      method: 'POST',
+      headers: { 'Content-Type': 'application/json' },
+      body: JSON.stringify({
+        language,
+        version: '1.0.0',
+        files: [{ name: 'main.txt', content: 'test' }],
+      }),
+    }).then(response => {
+      responseArrived = true;
+      return response;
+    });
+    try {
+      await Promise.race([
+        cleanupStarted,
+        pending.then(response => {
+          throw new Error(`Response preceded cleanup: ${response.status}`);
+        }),
+      ]);
+      // Give an incorrectly early res.json() time to reach the client.
+      await new Promise(resolve => setTimeout(resolve, 30));
+      expect(responseArrived).toBe(false);
+    } finally {
+      releaseCleanup();
+      const response = await pending;
+      expect(response.status).toBe(
+        outcome === 'success'
+          ? 200
+          : outcome === 'validation_failure'
+          ? 400
+          : 500
+      );
+      await response.text();
+    }
+    expect(events.filter(event => event === 'cleanup-start')).toHaveLength(1);
+    expect(events[events.length - 1]).toBe('cleanup-end');
+    if (outcome === 'success')
+      expect(events).toEqual([
+        'prime',
+        'execute',
+        'upload',
+        'cleanup-start',
+        'cleanup-end',
+      ]);
+  });
+}
+
+test('client disconnect does not clean a workspace while execution is still running', async () => {
+  config.require_execution_manifest = false;
+  let releaseExecution!: () => void;
+  const executionGate = new Promise<void>(resolve => {
+    releaseExecution = resolve;
+  });
+  let markExecuting!: () => void;
+  const executing = new Promise<void>(resolve => {
+    markExecuting = resolve;
+  });
+  let markCleaned!: () => void;
+  const cleaned = new Promise<void>(resolve => {
+    markCleaned = resolve;
+  });
+  let cleanupCount = 0;
+  Job.prototype.prime = async function () {};
+  Job.prototype.execute = async function () {
+    markExecuting();
+    await executionGate;
+    return {} as Awaited<ReturnType<Job['execute']>>;
+  };
+  Job.prototype.cleanup = async function () {
+    cleanupCount++;
+    markCleaned();
+  };
+  const controller = new AbortController();
+  const pending = fetch(url, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/json' },
+    signal: controller.signal,
+    body: JSON.stringify({
+      language,
+      version: '1.0.0',
+      files: [{ name: 'main.txt', content: 'test' }],
+    }),
+  }).catch(() => undefined);
+  try {
+    await executing;
+    controller.abort();
+    await pending;
+    await new Promise(resolve => setTimeout(resolve, 30));
+    expect(cleanupCount).toBe(0);
+  } finally {
+    releaseExecution();
+    await cleaned;
+  }
+  expect(cleanupCount).toBe(1);
+});

api/src/api/v2.ts

@@ -588,9 +588,13 @@ router.post('/execute', express.json({ limit: config.execute_body_limit }), asyn
         }
       }
 
+      /* Upload must finish before cleanup, and cleanup must settle before
+       * acknowledging completion. Failed removals retain a quarantined UID. */
+      await cleanupHandler();
       metricsOutcome = 'success';
       return res.status(200).json(result);
     } catch (error) {
+      await cleanupHandler();
       /* Deliberately BEFORE the ValidationError branch below: once priming has
        * completed, the workspace has been written to, so any later failure —
        * including a validation one — leaves state the next execute must not