Repository navigation
fix(agent-server): answer uploads onto a directory or below a file with 400 - #5931
Merged
enyst merged 2 commits intoOct 14, 2026
Merged
Conversation
…th 400 _upload_file mapped every non-HTTP exception to 500, so uploading onto an existing directory ([Errno 21]) or to a path whose parent is a file ([Errno 17] / [Errno 20]) looked like a server fault. Map IsADirectoryError to 400 "Path is a directory" and FileExistsError/NotADirectoryError to 400 "A parent of the path is not a directory", matching create_directory, and drop the F14.upload-onto-directory known-bug marker. Fixes #5930 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014MKndVj7XWvRQmvW9w2bDg
Contributor
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Contributor
Coverage Report •
|
||||||||||||||||||||
…d route Fixes #5930 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014MKndVj7XWvRQmvW9w2bDg
all-hands-bot
approved these changes
Oct 14, 2026
all-hands-bot
left a comment
all-hands-bot
left a comment
Collaborator
There was a problem hiding this comment.
This review was posted by an AI agent (OpenHands).
Scope
In scope. The change belongs in openhands-agent-server: it edits the canonical REST handler _upload_file in openhands-agent-server/openhands/agent_server/file_router.py, which this repository owns per the cross-repo boundaries in AGENTS.md. No product/architecture decision is needed — the linked issue #5930 (labels bug, priority:low, ready-for-dev) already fixed the intended status code as 400, consistent with create_directory in the same file.
Findings
No material correctness, security, or compatibility defects found.
Verified against current head 0f38cdba:
- The two new
exceptbranches are disjoint and correctly typed.IsADirectoryErrormaps to 400Path is a directory;(FileExistsError, NotADirectoryError)maps to 400A parent of the path is not a directory. Neither class is a subclass of the other (IsADirectoryError/NotADirectoryErrorderive directly fromOSError), so branch ordering is not load-bearing. - The raised exception types match the actual code paths: on this Python (3.13),
mkdir(parents=True, exist_ok=True)raisesFileExistsErrorwhen the direct parent is a file andNotADirectoryErrorwhen an ancestor further up is one, whileopen(target, "wb")raisesIsADirectoryErrorfor a directory target. Every other failure still falls through to the 500 branch, so genuine I/O faults remain server errors. - The conversation-scoped route mounts the same handler, so the single fix covers
POST /api/conversations/{id}/file/uploadas well. - No compatibility concern: the route's non-2xx contract is unchanged, and both callers (
RemoteWorkspace.file_uploadin the Python SDK andfileUploadinclients/typescript) already treat any non-2xx as a failed upload.
Acceptance criteria in #5930 are all met: directory target to 400 with the directory unchanged; direct-parent file to 400 with the file unchanged; deeper ancestor-file to 400; non-empty, cause-identifying detail; non-path failures still 500; conversation-scoped route returns the same 400; success behavior unchanged.
Verification
- GitHub Actions for head
0f38cdba: all required checks green (agent-server-tests,agent-server-api,pre-commit,sdk-tests,integration-test, etc.). - Ran the focused suites locally on the checked-out head:
uv run pytest tests/agent_server/test_file_router.py-> 95 passed;uv run pytest tests/agent_server/test_runtime_router.py-> 7 passed. The new tests drive the real FastAPI app (TestClient / ASGITransport), assert 400 plus the exactdetail, and assert nothing on disk changed.
✅ APPROVED
enyst
deleted the
claude/umbrella-bug-fixes-tgz9ai-upload-onto-directory
branch
October 14, 2026 20:24
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.
HUMAN:
This PR proposes a fix for a bug uncovered during the first full agentic run of
verify-agent-server.AGENT:
I'm an AI agent (Claude Code) helping Engel Nyst (@enyst) with project maintenance.
Requested by Engel · project thread
False
Decision neededWhich status code a wrong upload destination returns. I implemented 400. The issue triage asks for 400, and
POST /api/file/create_directoryalready answers the same collisions (existing non-directory, parent is a file) with 400Path exists and is not a directory. The alternative is 409 Conflict when the target is an existing directory. With 409, upload and create_directory would disagree unless create_directory changed too. Neither choice needs a client change, because both clients (RemoteWorkspace.file_uploadin the Python SDK andfileUploadinclients/typescript) treat any non-2xx as a failed upload.Why
Uploading with
POST /api/file/uploadonto an existing directory, or to a path whose parent is a regular file, returned500 Failed to upload file: [Errno 21] Is a directory/[Errno 17] File exists. Callers could not tell a mistake in their own path from a server fault.Summary
_upload_filenow mapsIsADirectoryErrorto 400Path is a directory, andFileExistsError/NotADirectoryError(whichmkdir(parents=True)raises when a parent is a file) to 400A parent of the path is not a directory. Every other failure still returns 500. The conversation-scoped routePOST /api/conversations/{id}/file/uploadmounts the same handler.F14.upload-onto-directoryno longer has its# bugmarkers and now asserts the exact 400 details.Issue Number
Fixes #5930
How to Test
uv run pytest tests/agent_server/test_file_router.py: 94 passed, 1 skipped.uv run pytest tests/agent_server/test_runtime_router.py: 7 passed. Both new tests fail on main (500) and pass here.AGENT_SERVER_VERIFY_HOME=/var/tmp/avs-upload-onto-directory):control-agent-server map run --file F14 --only F14.upload-onto-directory --fresh --repeat 2 --checkout <main checkout>->xfailx2;xpassx2;control-agent-server map checkok (34 files, 935 ids), andmap run --only F14.upload-onto-directory --fresh->pass.uv run pre-commit run --files <changed files>: all passed.Video/Screenshots
n/a
Type
Notes
Expect small merge overlaps with two open PRs that also touch
_upload_fileand the F14 map: #5856 (uploadfile_size) and #5902 (atomic overwrite). This change only addsexceptbranches, so it combines cleanly with both. With the atomic write, a directory target raisesIsADirectoryErrorat the replace step, and this change maps it the same way.🐳 Agent Server images for this PR — GHCR package, pull/run commands, and all pushed tags (click to expand)
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdkpython-node-runtimepython-node-runtimepython-node-runtimegolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:0f38cdb-pythonRun
All tags pushed for this build
About Multi-Architecture Support
0f38cdb-python) is a multi-arch manifest supporting both amd64 and arm640f38cdb-python-amd64) are also available if needed