Repository navigation
fix(uploads): probe the workspace bounded before a prompt upload - #605
Merged
Merged
Conversation
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
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>
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
POST /api/sessions/:id/paste-imagewrites the upload undersession.workingDir(throughensureUploadDir) with no bounded check first. If that workspace sits on a network mount that has stopped answering, thelstat/mkdirinsideensureUploadDiris an unbounded async call that holds one of libuv's few threadpool workers (4 by default, shared with every otherfs,dns.lookupandcryptocall) 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
fscall. #574 round one bounded the hourly upload sweep and the per-session delete withprobePathKind()pluspastCap, but the upload REQUEST path was left on a rawensureUploadDir.POST /api/sessionsalready does the right thing one function over: a singleprobePathKind(workingDir, { pastCap: true })at entry that refuses anunknownmount. The paste route just never got the same guard.Fix
session.workingDirwithprobePathKind(..., { pastCap: true })right after the remote-session 400 and before anything reads the body or touches the workspace. Refuse onlyunknown(the mount is not answering) throughdescribeUnknownPath('Workspace', ...)withOPERATION_FAILED, exactly as the create route does.absentand not-a-directory keepensureUploadDir's own errors, since a live session's workspace should be neither.pastCap: truebecause 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.ensureUploadDiris the same residual racePOST /api/sessionsaccepts: 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 pinsprobePathKind(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 anunknownresult and asserts the exactdescribeUnknownPathmessage, the 422 /OPERATION_FAILEDenvelope, and that nothing is written. Each guard was shown to fail with it removed: drop the probe and both go red; flippastCaptofalseand 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 hungstaton a simulated unreachable mount). It asserts the route answers well inside the block window and does no synchronousfson the dead mount, the same shape as the create and quick-start cases already there.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
stathung, the route returns the "not responding" refusal promptly instead of parking a worker, and records no synchronousfstouch. The probe pattern this copies has shipped onPOST /api/sessionsin 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.