Skip to content

fix(uploads): probe the workspace bounded before a prompt upload - #605

Merged
Ark0N merged 1 commit into
Ark0N:masterfrom
JDProfresh:fix/paste-image-bounded-probe
Oct 13, 2026
Merged

Ark0N merged 1 commit into
Ark0N:masterfrom
JDProfresh:fix/paste-image-bounded-probe

Conversation

@JDProfresh

Copy link
Copy Markdown
Contributor

Problem

POST /api/sessions/:id/paste-image writes the upload under session.workingDir (through ensureUploadDir) with no bounded check first. If that workspace sits on a network mount that has stopped answering, the lstat/mkdir inside ensureUploadDir is an unbounded async call that holds one of libuv's few threadpool workers (4 by default, shared with every other fs, dns.lookup and crypto call) until the mount answers. A handful of uploads onto a dead mount can drain the pool and stall the whole server.

Cause and precedent

This is the #516 rule: a user-chosen path is probed BOUNDED, never with a sync or unbounded async fs call. #574 round one bounded the hourly upload sweep and the per-session delete with probePathKind() plus pastCap, but the upload REQUEST path was left on a raw ensureUploadDir. POST /api/sessions already does the right thing one function over: a single probePathKind(workingDir, { pastCap: true }) at entry that refuses an unknown mount. The paste route just never got the same guard.

Fix

  • Probe session.workingDir with probePathKind(..., { pastCap: true }) right after the remote-session 400 and before anything reads the body or touches the workspace. Refuse only unknown (the mount is not answering) through describeUnknownPath('Workspace', ...) with OPERATION_FAILED, exactly as the create route does. absent and not-a-directory keep ensureUploadDir's own errors, since a live session's workspace should be neither.
  • pastCap: true because this is one path at the user's explicit request, so unrelated dead mounts must not refuse it; it still stops at the ceiling that keeps one worker free.
  • The probe is a preflight, not a wrapper around the write. A mount that dies between the probe and ensureUploadDir is the same residual race POST /api/sessions accepts: a pre-check cannot bound the write that follows it, and a second probe would only narrow, never close, that window. I kept one probe at entry and stated the race in the code comment and the invariants rather than chase it with more checks.

Tests

  • test/routes/session-routes.test.ts (paste-image block): one test pins probePathKind(session.workingDir, { pastCap: true }) at the call site, with the mock cleared right before the request so only that upload's call can satisfy the pin. Another drives an unknown result and asserts the exact describeUnknownPath message, the 422 / OPERATION_FAILED envelope, and that nothing is written. Each guard was shown to fail with it removed: drop the probe and both go red; flip pastCap to false and the pin goes red; change the 'Workspace' label and the message assertion goes red.
  • test/routes/session-create-unreachable-path.test.ts: added a paste-image case to the existing dead-mount harness (a hung stat on a simulated unreachable mount). It asserts the route answers well inside the block window and does no synchronous fs on the dead mount, the same shape as the create and quick-start cases already there.
  • The full suite passes apart from one failure on my run, session-input-wait's wall-clock "blocks for its full timeout", which passed 3 of 3 on its own; it is a timing flake on my host, unrelated to this diff.

Verified

The dead-mount test reproduces the stall directly: with the mount's stat hung, the route returns the "not responding" refusal promptly instead of parking a worker, and records no synchronous fs touch. The probe pattern this copies has shipped on POST /api/sessions in released builds. It also runs in my dev install's 1.41.0 build together with the other open PRs of this batch, where image, video and document uploads behave as before on a healthy workspace.

Note

The Attach button and video uploads PR I'm sending right after this one adds a second caller of the same ensureUploadDir, so the entry probe covers it too. Both add to the Prompt uploads paragraph in CLAUDE.md and the paste-image test block; I'll rebase whichever lands second.

The paste-image route wrote under session.workingDir via ensureUploadDir
with no bounded probe, so an upload onto a stalled network mount held one
of libuv's few threadpool workers until the mount answered. Probe the
workspace with probePathKind past the stall cap before the first disk
touch and refuse an unknown mount through describeUnknownPath, the way
POST /api/sessions already does.
@Ark0N
Ark0N merged commit 6bbfc44 into Ark0N:master Oct 13, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Oct 13, 2026
…-roll video uploads

Conflicts resolved against #605 (and #602's test mock):
- CLAUDE.md Prompt uploads: #606's rewrite is kept and #605's bounded
  workspace probe sentence follows the remote (SSH) refusal.
- test/routes/session-routes.test.ts: the remote-pane-history and
  bounded-path-probe mocks are kept beside #606's _resetPasteRateBuckets
  import, and both PRs' paste-image tests are kept.

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

Ark0N commented Oct 13, 2026

Copy link
Copy Markdown
Owner

Thanks @JDProfresh, shipped in 1.41.1, merged just before #606 so the entry probe covers the video path too. No changes at merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants