Merge pull request #605: fix(uploads): probe the workspace bounded before a prompt upload

Conflict resolved in test/routes/session-routes.test.ts: #602's
remote-pane-history mock and #605's bounded-path-probe mock are both kept.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

docs/architecture-invariants.md

@@ -1096,7 +1096,7 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u
 
 Target: 20 sessions, 50 agent windows at 60fps. Limits in `src/config/`: terminal 32MB (see below), text 1MB, messages 1000, max agents 500, max sessions 50, max SSE clients 100. **Terminal history** (`src/config/terminal-history.ts`, COD-80): tmux history-limit 100k lines, PTY buffer 32MB max / 24MB trim (env `CODEMAN_MAX_TERMINAL_BUFFER`/`CODEMAN_TRIM_TERMINAL_TO`; the env-derived trim is clamped ≤75% of max — trim ≥ max would disable `BufferAccumulator` trimming entirely = unbounded memory); browser xterm scrollback stays a separate hardcoded 50k (`DEFAULT_SCROLLBACK` in constants.js — 100k/tab is a mobile-memory hazard). tmux <3.7 allocates history at pane creation, so `createSession()` sets the global default in the same command queue immediately before `new-session`; tmux 3.7+ instead creates the session and targets only that pane, because changing the global option can resize and trim unrelated live panes. A settings change resizes tracked panes only on 3.7+ and otherwise affects future panes; no version can recover lines already evicted. Settings keys `terminalScrollbackLines`/`terminalBufferMaxBytes`/`terminalBufferTrimBytes` are schema-validated but inert (only `tmuxHistoryLimit` is wired); `buffer-limits.ts` re-exports the defaults. Text/message limits are env-overridable too (`CODEMAN_MAX_TEXT_OUTPUT`/`CODEMAN_TRIM_TEXT_TO`/`CODEMAN_MAX_MESSAGES`). **Image upload** (`image-input.js` / `config/buffer-limits.ts`): up to `_maxBatchImages` 20 images/batch (bounded concurrency 3), per-file `MAX_PASTE_IMAGE_BYTES` 50MB (env `CODEMAN_MAX_PASTE_IMAGE_BYTES`); the mobile camera-roll picker auto-downscales to fit before upload. **HEIC paste uploads** (#151): converted server-side to JPEG in a `worker_threads` worker (`web/heic-jpeg-worker.ts`, resourceLimits + 30s timeout) gated by `runWithConversionLimit()`; detection is magic-byte based (covers Android/MIUI HEIFs mislabeled as JPEG); headers declaring > 64MP are rejected 415 BEFORE decode (decompression-bomb guard). Deps: `heic-decode` + `jpeg-js`. Use `LRUMap` for bounded caches, `StaleExpirationMap` for TTL cleanup. Anti-flicker pipeline: `docs/terminal-anti-flicker.md`.
 
-**Prompt uploads live in `<workspace>/.codeman-uploads/`** (`POST /api/sessions/:id/paste-image`; the names live ONLY in `src/web/paste-image-gc.ts`): IN the workspace because that is the only path that resolves identically for a local agent and a container (only the workspace is bind-mounted, at the same absolute path; `~/.codeman` is not); hidden so it stays out of `git status` and the agent's view of the repository; FLAT because a nested `<workspace>/.codeman/` IS the data dir (`join(homedir(), '.codeman')`, `src/config/instance.ts`) when the workspace is the home directory, which nothing refuses; and self-ignoring through a `.gitignore` of `*` written once with `wx`, so a file already there is the user's and stays. The pre-move `.claude-images/` receives nothing but stays readable for one release: `UPLOAD_DIR_NAMES` lists both, the hourly sweep and the delete cleanup in `cleanupSession()` go through `uploadDirs()`, and the image watcher's ignore filter reads the names. ⚠️ `uploadDirs()` is async: the working directory is a user-chosen path, so it is probed BOUNDED first (`probePathKind()`, the #516 rule; `unknown` is skipped and never touched, the hourly sweep keeps the stall cap and `cleanupSession()` passes `pastCap`, since it acts on one path at the user's request), and the lstat and realpath after it are `fs.promises` calls, never sync, because the sweep runs 30 s after boot and hourly over every live session and a linked case on a dead mount would otherwise freeze the whole server. It returns only REAL directories (lstat at the leaf, so a planted `.codeman-uploads -> /other-case/.codeman-uploads` is never listed: `readdir` follows a link to a directory, and the sweep would otherwise age out the other case's uploads through it), none that is or contains `getDataDir()` (contrived, `CODEMAN_INSTANCE=uploads` makes `~/.codeman-uploads` the data dir of a home workspace and `CODEMAN_DATA_DIR` can point inside one, but it is one check next to a recursive delete; one strictly INSIDE the data dir is listed, since it only ever holds uploads and a workspace like the `~/.codeman/app` that `install.sh` clones would otherwise never have its uploads collected), and nothing for a remote (SSH) session, whose `workingDir` is the remote path and would otherwise name a same-named LOCAL directory for the sweep and the delete. The check is made when the directories are listed: a same-user process that swaps a listed directory for a link during the sweep's awaits is accepted (Node has no `openat()`, and that actor already writes anywhere this process can). The route refuses a remote (SSH) session with 400 before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it. Tests: `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, the paste-image block of `test/routes/session-routes.test.ts`.
+**Prompt uploads live in `<workspace>/.codeman-uploads/`** (`POST /api/sessions/:id/paste-image`; the names live ONLY in `src/web/paste-image-gc.ts`): IN the workspace because that is the only path that resolves identically for a local agent and a container (only the workspace is bind-mounted, at the same absolute path; `~/.codeman` is not); hidden so it stays out of `git status` and the agent's view of the repository; FLAT because a nested `<workspace>/.codeman/` IS the data dir (`join(homedir(), '.codeman')`, `src/config/instance.ts`) when the workspace is the home directory, which nothing refuses; and self-ignoring through a `.gitignore` of `*` written once with `wx`, so a file already there is the user's and stays. The pre-move `.claude-images/` receives nothing but stays readable for one release: `UPLOAD_DIR_NAMES` lists both, the hourly sweep and the delete cleanup in `cleanupSession()` go through `uploadDirs()`, and the image watcher's ignore filter reads the names. ⚠️ `uploadDirs()` is async: the working directory is a user-chosen path, so it is probed BOUNDED first (`probePathKind()`, the #516 rule; `unknown` is skipped and never touched, the hourly sweep keeps the stall cap and `cleanupSession()` passes `pastCap`, since it acts on one path at the user's request), and the lstat and realpath after it are `fs.promises` calls, never sync, because the sweep runs 30 s after boot and hourly over every live session and a linked case on a dead mount would otherwise freeze the whole server. It returns only REAL directories (lstat at the leaf, so a planted `.codeman-uploads -> /other-case/.codeman-uploads` is never listed: `readdir` follows a link to a directory, and the sweep would otherwise age out the other case's uploads through it), none that is or contains `getDataDir()` (contrived, `CODEMAN_INSTANCE=uploads` makes `~/.codeman-uploads` the data dir of a home workspace and `CODEMAN_DATA_DIR` can point inside one, but it is one check next to a recursive delete; one strictly INSIDE the data dir is listed, since it only ever holds uploads and a workspace like the `~/.codeman/app` that `install.sh` clones would otherwise never have its uploads collected), and nothing for a remote (SSH) session, whose `workingDir` is the remote path and would otherwise name a same-named LOCAL directory for the sweep and the delete. The check is made when the directories are listed: a same-user process that swaps a listed directory for a link during the sweep's awaits is accepted (Node has no `openat()`, and that actor already writes anywhere this process can). The route refuses a remote (SSH) session with 400 before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it. ⚠️ And because a live session's workspace can go onto a network mount that later stops answering, the route probes `session.workingDir` BOUNDED (`probePathKind(..., { pastCap: true })`) before the first disk touch, refusing an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED) exactly as `POST /api/sessions` does, so a mount already unreachable cannot freeze a libuv worker on every upload (`ensureUploadDir()`'s own lstat/mkdir then start on a reachable path; a mount that dies between the probe and that write is the same residual race `POST /api/sessions` accepts, since a pre-check cannot bound the write that follows it). Tests: `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, the paste-image block of `test/routes/session-routes.test.ts`, `test/routes/session-create-unreachable-path.test.ts`.
 
 ### Process-tree walks are bounded
 

src/web/routes/session-routes.ts

@@ -377,7 +377,9 @@ export function imageMagicMatchesExt(data: Buffer, ext: string): boolean {
  * touched, and a file already there is theirs and stays as it is.
  */
 async function ensureUploadDir(workingDir: string): Promise<string | null> {
-  // workingDir is guaranteed to exist (live session).
+  // workingDir exists (live session) and the caller bounded-probed its mount, so
+  // the lstat/mkdir below normally start on a reachable path; a mount that dies
+  // after that probe is the same residual race POST /api/sessions accepts.
   const uploadDir = join(workingDir, UPLOADS_DIR);
   try {
     const dirStat = await fs.lstat(uploadDir);
@@ -5354,6 +5356,20 @@ export function registerSessionRoutes(
       );
     }
 
+    // The upload is written under session.workingDir (ensureUploadDir below), and
+    // a live session's workspace can sit on a network mount that stopped answering.
+    // Probe it bounded before the first disk touch, as #516 requires for a path the
+    // user named, so one dead mount cannot freeze a libuv threadpool worker per
+    // upload. Only "unknown" (the mount is not answering) is refused here, the way
+    // the create route does; absent / not-a-directory keep ensureUploadDir's own
+    // errors, since a live session's workspace should be neither.
+    if ((await probePathKind(session.workingDir, { pastCap: true })) === 'unknown') {
+      return createErrorResponse(
+        ApiErrorCode.OPERATION_FAILED,
+        describeUnknownPath('Workspace', session.workingDir, { pastCap: true })
+      );
+    }
+
     if (!req.isMultipart()) {
       reply.code(400);
       return createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Expected multipart/form-data');

test/routes/session-create-unreachable-path.test.ts

@@ -1,9 +1,11 @@
 /**
- * @fileoverview Session creation must not freeze the server on a workspace whose
- * network mount has gone away (`POST /api/sessions` with a `workingDir` on it, and
- * `POST /api/quick-start` for a linked case that lives there), and must not treat
- * "did not answer" as "does not exist" (quick-start would scaffold a fresh case
- * over the top of where the real one is mounted).
+ * @fileoverview Request paths that touch a session's workspace must not freeze the
+ * server when its network mount has gone away: `POST /api/sessions` with a
+ * `workingDir` on it, `POST /api/quick-start` for a linked case that lives there,
+ * and `POST /api/sessions/:id/paste-image` for a live session whose mount went away
+ * after creation. Creation must also not treat "did not answer" as "does not exist"
+ * (quick-start would scaffold a fresh case over the top of where the real one is
+ * mounted).
  *
  * A hard mount that stopped answering is simulated two ways, matching how each
  * API behaves on one: a synchronous probe (`existsSync`/`statSync`/`mkdirSync`)
@@ -14,6 +16,7 @@
 import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 import Fastify, { type FastifyInstance } from 'fastify';
 import fastifyCookie from '@fastify/cookie';
+import fastifyMultipart from '@fastify/multipart';
 
 const dead = vi.hoisted(() => {
   // Short probe timeout so a stalled stat costs ~200 ms here. Read at import.
@@ -81,6 +84,7 @@ import { dataPath } from '../../src/config/instance.js';
 
 describe('session creation on an unreachable mount', () => {
   let app: FastifyInstance;
+  let ctx: ReturnType<typeof createMockRouteContext>;
   let scratch: string;
   let warn: ReturnType<typeof vi.spyOn>;
 
@@ -90,7 +94,9 @@ describe('session creation on an unreachable mount', () => {
     scratch = await mkdtemp(join(tmpdir(), 'codeman-unreachable-create-'));
     app = Fastify({ logger: false });
     await app.register(fastifyCookie);
-    registerSessionRoutes(app, createMockRouteContext() as never);
+    await app.register(fastifyMultipart, { limits: { fileSize: 10 * 1024 * 1024, files: 1 } });
+    ctx = createMockRouteContext();
+    registerSessionRoutes(app, ctx as never);
     installRouteErrorHandler(app);
     await app.ready();
   });
@@ -171,4 +177,27 @@ describe('session creation on an unreachable mount', () => {
     expect(body.errorCode).toBe('OPERATION_FAILED');
     expect(body.error).toMatch(/not responding/i);
   });
+
+  it('POST /api/sessions/:id/paste-image answers promptly, not frozen, for a workspace on a dead mount', async () => {
+    // A live session can be created while its workspace is reachable and later have
+    // its mount go away. The probe runs before the multipart body is read, so the
+    // refusal lands without buffering the upload.
+    ctx._session.workingDir = `${dead.root}/project`;
+
+    const started = Date.now();
+    const res = await app.inject({
+      method: 'POST',
+      url: `/api/sessions/${ctx._sessionId}/paste-image`,
+      headers: { 'x-codeman-csrf': '1', 'content-type': 'multipart/form-data; boundary=b' },
+      payload: '--b--\r\n',
+    });
+    const elapsed = Date.now() - started;
+
+    expect(elapsed).toBeLessThan(dead.blockMs - 1_000);
+    expect(dead.syncTouches).toEqual([]);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+    expect(body.errorCode).toBe('OPERATION_FAILED');
+    expect(body.error).toMatch(/not responding/i);
+  });
 });

test/routes/session-routes.test.ts

@@ -61,9 +61,28 @@ vi.mock('../../src/remote-hosts.js', async (orig) => {
 const remoteScrollback = vi.hoisted(() => vi.fn(async (): Promise<string | null> => null));
 vi.mock('../../src/remote-pane-history.js', () => ({ fetchRemoteScrollback: remoteScrollback }));
 
+// Bounded path probe: a live session's workspace can sit on a network mount that
+// stopped answering, and the paste-image route probes it before the first disk
+// touch (#516). Default is call-through so the real probe runs for the other
+// routes here (create / quick-start); the paste-image tests drive the result via
+// pathProbe.kind (reset to null right after, since it is process-wide).
+const pathProbe = vi.hoisted(() => ({
+  kind: null as import('../../src/utils/bounded-path-probe.js').PathProbeKind | null,
+}));
+vi.mock('../../src/utils/bounded-path-probe.js', async (orig) => {
+  const actual = await orig<typeof import('../../src/utils/bounded-path-probe.js')>();
+  return {
+    ...actual,
+    probePathKind: vi.fn((path: string, opts?: { pastCap?: boolean }) =>
+      pathProbe.kind !== null ? Promise.resolve(pathProbe.kind) : actual.probePathKind(path, opts)
+    ),
+  };
+});
+
 import { registerSessionRoutes } from '../../src/web/routes/session-routes.js';
 import { RemoteWakeRegistry, REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS } from '../../src/remote-wake.js';
 import { resolveTerminalHistoryConfig } from '../../src/config/terminal-history.js';
+import { describeUnknownPath, probePathKind } from '../../src/utils/bounded-path-probe.js';
 
 interface LocalHarness {
   app: FastifyInstance;
@@ -450,6 +469,37 @@ describe('session-routes', () => {
       expect(JSON.parse(res.body).success).toBe(false);
       expect(heicConvert).not.toHaveBeenCalled();
     });
+
+    it('probes the session workspace past the stall cap before writing (#516)', async () => {
+      const workDir = await mkdtemp(join(tmpdir(), 'codeman-uploads-'));
+      harness.ctx._session.workingDir = workDir;
+      try {
+        // Clear right before the request so only THIS upload's call can satisfy the pin.
+        vi.mocked(probePathKind).mockClear();
+        const res = await upload('shot.jpg', 'image/jpeg', jpeg);
+        expect(res.statusCode).toBe(200);
+        expect(probePathKind).toHaveBeenCalledWith(workDir, { pastCap: true });
+      } finally {
+        await rm(workDir, { recursive: true });
+      }
+    });
+
+    it('refuses with the describeUnknownPath message, and writes nothing, when the workspace does not answer', async () => {
+      const workDir = await mkdtemp(join(tmpdir(), 'codeman-uploads-'));
+      harness.ctx._session.workingDir = workDir;
+      pathProbe.kind = 'unknown';
+      try {
+        const res = await upload('shot.jpg', 'image/jpeg', jpeg);
+        expect(res.statusCode).toBe(422);
+        const body = JSON.parse(res.body);
+        expect(body.errorCode).toBe('OPERATION_FAILED');
+        expect(body.error).toBe(describeUnknownPath('Workspace', workDir, { pastCap: true }));
+        expect(await readdir(workDir)).toEqual([]);
+      } finally {
+        pathProbe.kind = null;
+        await rm(workDir, { recursive: true });
+      }
+    });
   });
 
   // ========== GET /api/sessions ==========