fix: report session-workspace failures faithfully and prime inline files atomically

api/src/api/v2-checkpoint-binding.test.ts

@@ -0,0 +1,123 @@
+import { afterAll, afterEach, beforeAll, describe, expect, test } from 'bun:test';
+import express from 'express';
+import * as fsp from 'fs/promises';
+import type { Server } from 'http';
+import * as os from 'os';
+import * as path from 'path';
+import { config } from '../config';
+import { bindSessionWorkspace, resetSessionWorkspaceStateForTests } from '../session-workspace';
+import v2Router from './v2';
+
+/**
+ * The checkpoint/restore routes bind the session straight off the header, and
+ * the control plane acts on WHICH way that bind failed: a missing header is a
+ * caller error, while a rejected bind means this runner is pinned to a
+ * different session and must be recycled. Both used to answer with the same
+ * generic 409, so a real conflict was invisible and the VM stayed in service.
+ */
+
+let server: Server;
+let baseUrl: string;
+let packageDir: string;
+const savedSessionWorkspaceEnabled = config.session_workspace_enabled;
+
+beforeAll(async () => {
+  packageDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'checkpoint-binding-'));
+  const app = express();
+  app.use('/api/v2', v2Router);
+  await new Promise<void>((resolve) => {
+    server = app.listen(0, '127.0.0.1', () => resolve());
+  });
+  const address = server.address();
+  baseUrl = `http://127.0.0.1:${typeof address === 'object' && address ? address.port : 0}`;
+});
+
+afterAll(async () => {
+  await new Promise<void>((resolve) => server.close(() => resolve()));
+  await fsp.rm(packageDir, { recursive: true, force: true });
+});
+
+afterEach(() => {
+  config.session_workspace_enabled = savedSessionWorkspaceEnabled;
+  resetSessionWorkspaceStateForTests();
+});
+
+const checkpoint = (headers: Record<string, string> = {}) =>
+  fetch(`${baseUrl}/api/v2/session/checkpoint`, { headers });
+
+const restore = (headers: Record<string, string> = {}) =>
+  fetch(`${baseUrl}/api/v2/session/restore`, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/x-gtar', ...headers },
+    body: 'not-a-real-archive',
+  });
+
+describe('checkpoint/restore session binding', () => {
+  test('a missing header stays a plain 409 without the recycle signal', async () => {
+    config.session_workspace_enabled = true;
+
+    for (const response of [await checkpoint(), await restore()]) {
+      expect(response.status).toBe(409);
+      const body = (await response.json()) as { error?: string; message?: string };
+      expect(body.message).toBe('Missing runtime session header');
+      /* A caller that simply forgot the header must NOT trigger a VM recycle. */
+      expect(body.error).toBeUndefined();
+    }
+  });
+
+  test('a bind rejected by a different bound session reports the recycle signal', async () => {
+    config.session_workspace_enabled = true;
+    expect(bindSessionWorkspace({ runtimeSessionId: 'rt_already_bound' })).toBeDefined();
+
+    for (const response of [
+      await checkpoint({ 'X-Runtime-Session-Id': 'rt_other' }),
+      await restore({ 'X-Runtime-Session-Id': 'rt_other' }),
+    ]) {
+      expect(response.status).toBe(409);
+      const body = (await response.json()) as { error?: string; message?: string };
+      /* Same code /execute returns for this condition, so an older service
+       * fronting a newer runner still recycles the VM. */
+      expect(body.error).toBe('session_workspace_dirty');
+      expect(body.message).toBe('Runner is bound to a different runtime session');
+    }
+  });
+
+  test('a malformed header is a 400, not a session conflict', async () => {
+    config.session_workspace_enabled = true;
+
+    const response = await checkpoint({ 'X-Runtime-Session-Id': 'not a valid id!' });
+    expect(response.status).toBe(400);
+    const body = (await response.json()) as { error?: string; message?: string };
+    expect(body.error).toBeUndefined();
+    expect(body.message).toContain('malformed');
+  });
+
+  test('a duplicated header is a 400 rather than an unhandled throw', async () => {
+    config.session_workspace_enabled = true;
+
+    /* Node joins repeated occurrences of this header into one comma-separated
+     * string rather than surfacing an array, so the value fails the id pattern
+     * instead of the appears-once check. Either way it must be a caller error,
+     * never an unhandled throw or a session-conflict signal. */
+    const response = await fetch(`${baseUrl}/api/v2/session/checkpoint`, {
+      headers: [
+        ['X-Runtime-Session-Id', 'rt_one'],
+        ['X-Runtime-Session-Id', 'rt_two'],
+      ] as unknown as HeadersInit,
+    });
+    expect(response.status).toBe(400);
+    const body = (await response.json()) as { error?: string; message?: string };
+    expect(body.error).toBeUndefined();
+    expect(body.message).toMatch(/malformed|exactly once/);
+  });
+
+  test('a matching header binds and proceeds past the gate', async () => {
+    config.session_workspace_enabled = true;
+    expect(bindSessionWorkspace({ runtimeSessionId: 'rt_same' })).toBeDefined();
+
+    const response = await checkpoint({ 'X-Runtime-Session-Id': 'rt_same' });
+    /* Whatever the handler then does, it must not be rejected by the gate. */
+    expect(response.status).not.toBe(409);
+    expect(response.status).not.toBe(400);
+  });
+});

api/src/api/v2-session-binding.test.ts

@@ -7,6 +7,7 @@ import * as path from 'path';
 import { config } from '../config';
 import { Job } from '../job';
 import { loadPackage } from '../runtime';
+import { ValidationError } from '../validation';
 import {
   getBoundSessionWorkspace,
   resetSessionWorkspaceStateForTests,
@@ -130,4 +131,87 @@ describe('per-request session binding', () => {
       Job.prototype.cleanup = originalCleanup;
     }
   });
+
+  test('a request with nothing runnable is rejected BEFORE anything is primed', async () => {
+    config.session_workspace_enabled = true;
+    config.require_execution_manifest = false;
+
+    const originalPrime = Job.prototype.prime;
+    const originalExecute = Job.prototype.execute;
+    const originalCleanup = Job.prototype.cleanup;
+
+    let primed = false;
+    Job.prototype.prime = async function trackPrime(): Promise<void> {
+      primed = true;
+    };
+    Job.prototype.execute = async function executeWithoutSandbox() {
+      return {} as Awaited<ReturnType<Job['execute']>>;
+    };
+    Job.prototype.cleanup = async function cleanupWithoutFilesystem(): Promise<void> {};
+
+    try {
+      /* A lone `.dirkeep` plus a binary input satisfied the old gate (it only
+       * asked for "some utf8 file"), so the request primed its writes into the
+       * session workspace and only then failed the stricter check inside
+       * execute. Session cleanup deliberately preserves the workspace, so those
+       * writes stayed visible to the next execution. */
+      const response = await fetch(`${baseUrl}/api/v2/execute`, {
+        method: 'POST',
+        headers: {
+          'Content-Type': 'application/json',
+          'X-Runtime-Session-Id': 'rt_nothing_runnable',
+        },
+        body: JSON.stringify({
+          language: testLanguage,
+          version: testVersion,
+          files: [
+            { name: '.dirkeep', content: '' },
+            { name: 'blob.bin', content: 'AA==', encoding: 'base64' },
+          ],
+        }),
+      });
+
+      expect(response.status).toBe(400);
+      const body = (await response.json()) as { error?: string; message?: string };
+      expect(body.error).toBeUndefined();
+      expect(body.message).toContain('runnable source file');
+      /* The point of the fix: the workspace was never touched. */
+      expect(primed).toBe(false);
+    } finally {
+      Job.prototype.prime = originalPrime;
+      Job.prototype.execute = originalExecute;
+      Job.prototype.cleanup = originalCleanup;
+    }
+  });
+
+  test('a post-prime failure still reports the workspace as dirty', async () => {
+    config.session_workspace_enabled = true;
+    config.require_execution_manifest = false;
+
+    const originalPrime = Job.prototype.prime;
+    const originalExecute = Job.prototype.execute;
+    const originalCleanup = Job.prototype.cleanup;
+
+    Job.prototype.prime = async function primeWithoutFilesystem(): Promise<void> {};
+    Job.prototype.execute = async function executeFailingAfterPrime() {
+      throw new ValidationError('files must include at least one runnable source file');
+    };
+    Job.prototype.cleanup = async function cleanupWithoutFilesystem(): Promise<void> {};
+
+    try {
+      /* Once priming has written to the workspace, ANY later failure leaves
+       * state the next execute must not inherit silently — so the dirty signal
+       * outranks the 400 here, even for a validation error. */
+      const response = await execute('rt_dirty_after_prime');
+      expect(response.status).toBe(409);
+      expect(await response.json()).toEqual({
+        error: 'session_workspace_dirty',
+        message: 'Session workspace must be restored before another execute',
+      });
+    } finally {
+      Job.prototype.prime = originalPrime;
+      Job.prototype.execute = originalExecute;
+      Job.prototype.cleanup = originalCleanup;
+    }
+  });
 });

api/src/api/v2.ts

@@ -8,6 +8,7 @@ import {
   Job,
   SessionWorkspaceDirtyError,
   ValidationError,
+  hasRunnableSource,
   validateFilePath,
 } from '../job';
 import { EXECUTION_MANIFEST_HEADER, ExecutionManifestError, type ExecutionManifestClaims } from '../execution-manifest';
@@ -267,11 +268,14 @@ function getJob(
     throw { message: `${language}-${version} runtime is unknown` };
   }
 
-  if (
-    rt.language !== 'file' &&
-    !files.some(file => !file.encoding || file.encoding === 'utf8')
-  ) {
-    throw { message: 'files must include at least one utf8 encoded file' };
+  /* Reject a request with nothing runnable BEFORE anything is primed. This
+   * gate used to count a lone `.dirkeep` as a utf8 source, so such a request
+   * reached prime(), had its files written into the session workspace and its
+   * priming metadata recorded, and only then failed the stricter check in
+   * Job.execute — leaving the rejected request's writes visible to the next
+   * execution. Both sides now ask `hasRunnableSource`. */
+  if (!hasRunnableSource(files, rt.language)) {
+    throw { message: 'files must include at least one runnable source file' };
   }
 
   validateConstraints(body, rt);
@@ -543,6 +547,12 @@ router.post('/execute', express.json({ limit: config.execute_body_limit }), asyn
       metricsOutcome = 'success';
       return res.status(200).json(result);
     } catch (error) {
+      /* 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
+       * inherit silently. Requests with nothing runnable are rejected up front
+       * (see `hasRunnableSource`), so reaching here with a ValidationError
+       * means files really were primed and dirty is the honest answer. */
       if (primeCompleted && job?.markSessionDirty('execution failed after input priming')) {
         metricsOutcome = 'execution_error';
         logger.error({ job: job.uuid, err: error }, 'Session execution left workspace state unknown');
@@ -624,28 +634,55 @@ router.get('/runtimes', (_req: Request, res: Response) => {
  * silently continues with an empty workspace (checkpoint state lost on expiry).
  * Returns false (→ fail closed) when this request carries no valid header, so a
  * headerless/malformed request never operates on a stale prior session. */
-function bindSessionFromHeader(req: Request): boolean {
-  const binding = parseSessionBindingFromHeader(req.headers[RUNTIME_SESSION_ID_HEADER]);
-  if (!binding) return false;
-  /* A REJECTED bind (this runner is already pinned to a different session)
-   * must fail the request too — proceeding would run checkpoint/restore/file
-   * delivery for the requested session against the previously bound session's
-   * workspace. */
-  return bindSessionWorkspace(binding) != null;
+type SessionBindFailure = { status: number; body: Record<string, string> };
+
+/* Distinguishes the two ways a bind can fail, because the control plane acts on
+ * them differently: a missing/malformed header is a caller error, while a
+ * REJECTED bind means this runner is pinned to a DIFFERENT session and must be
+ * recycled. Collapsing both into one generic 409 (as this did) hid the conflict
+ * and left the VM in service. `session_workspace_dirty` is the established
+ * recycle signal — the same code /execute returns for this condition. */
+function bindSessionFromHeader(req: Request): SessionBindFailure | null {
+  let binding;
+  try {
+    binding = parseSessionBindingFromHeader(req.headers[RUNTIME_SESSION_ID_HEADER]);
+  } catch (error) {
+    /* A duplicated or malformed header throws SessionWorkspaceBindingError; it
+     * is a bad request, not a session conflict. */
+    return {
+      status: 400,
+      body: { message: error instanceof Error ? error.message : 'Invalid runtime session header' },
+    };
+  }
+  if (!binding) {
+    return { status: 409, body: { message: 'Missing runtime session header' } };
+  }
+  if (bindSessionWorkspace(binding) == null) {
+    return {
+      status: 409,
+      body: {
+        error: 'session_workspace_dirty',
+        message: 'Runner is bound to a different runtime session',
+      },
+    };
+  }
+  return null;
 }
 
 /* Express 4 (pinned) does NOT auto-forward rejected route-handler promises, so
  * `.catch(next)` is required or a rejection (e.g. session.ownership()) hangs the
  * request and surfaces as an unhandled rejection instead of a 5xx. */
 router.get('/session/checkpoint', (req: Request, res: Response, next: NextFunction) => {
-  if (!bindSessionFromHeader(req)) {
-    return res.status(409).json({ message: 'Missing runtime session header' });
+  const failure = bindSessionFromHeader(req);
+  if (failure) {
+    return res.status(failure.status).json(failure.body);
   }
   return streamSessionCheckpoint(res).catch(next);
 });
 router.post('/session/restore', (req: Request, res: Response, next: NextFunction) => {
-  if (!bindSessionFromHeader(req)) {
-    return res.status(409).json({ message: 'Missing runtime session header' });
+  const failure = bindSessionFromHeader(req);
+  if (failure) {
+    return res.status(failure.status).json(failure.body);
   }
   return restoreSessionCheckpoint(req, res).catch(next);
 });