feat(file-router): add POST /file/create_directory

[georgeglarson]

HUMAN:

Fully tested and working for me

---

AGENT:

End-to-end evidence (not just unit tests): the agent-server was started from this branch on :18923 and driven over real HTTP through a locally built @openhands/typescript-client with a hand-written createDirectory caller. Verified live: creating a nested directory with a space in the name (confirmed on disk), a repeat call against the same path (idempotent, contents untouched), the new directory immediately visible to GET /api/file/search_subdirs (the folder picker's own listing endpoint), create-over-existing-file → 400, and relative path → 400. Unit: tests/agent_server/test_file_router.py 85/85 (7 new), ruff clean, make test-server-schema passes (OpenAPI determinism + quality + swagger validate).

Why

The OpenHands frontend's workspace folder picker cannot create a directory — there is no directory-creation endpoint anywhere in the file API, so a "New Folder" button has nothing to call (OpenHands/OpenHands#16561). The only stock mechanism that materializes a directory is _upload_file's incidental mkdir of the target's parent, which collapses all failures to 500 and can't clean up after itself (no file-delete endpoint exists).

Summary

• Add POST /api/file/create_directory?path=<abs>: mkdir(parents=True, exist_ok=True) via asyncio.to_thread (same offload pattern as _upload_file), idempotent on repeat calls.
• Typed errors instead of the upload path's bare 500: 400 for a non-absolute path, a path that collides with an existing file, or a path under a file; 403 for permission denied (matching search_subdirs).
• 7 new tests (success, creates missing parents, existing-dir idempotent, relative-path 400, existing-file 400, permission-denied 403 via a real chmod 0o500 with root skip, missing path param).

Issue Number

OpenHands/OpenHands#16561

How to Test

Run the agent-server from this branch, then:

curl -X POST 'http://localhost:8000/api/file/create_directory?path=/tmp/oh-test/new-dir'
curl -X POST 'http://localhost:8000/api/file/create_directory?path=/tmp/oh-test/new-dir'   # idempotent
curl 'http://localhost:8000/api/file/search_subdirs?path=/tmp/oh-test'                       # new-dir listed
curl -X POST 'http://localhost:8000/api/file/create_directory?path=relative/dir'             # 400

Or just uv run pytest tests/agent_server/test_file_router.py (85 tests).

Video/Screenshots

Type

• Bug fix
• Feature
• Refactor
• Breaking change
• Docs / chore

Notes

Purely additive endpoint; no existing contract changes, so no deprecation runway is engaged. Companion typescript-client change (FileClient.createDirectory) is ready and will follow once this lands in an agent-server release, per the release-train dependency. Design alternatives considered and rejected are documented on the linked issue (upload-.keep shim, bash mkdir).

[enyst]

LGTM, thank you for this!

[all-hands-bot]

Code Review: feat(file-router): add POST /file/create_directory

Nice, clean addition. The error semantics are a genuine improvement over _upload_file (which collapses everything to 500), the idempotency is correct, and the test coverage is thorough. A few items below.

---

Should fix

1. os.geteuid() breaks test collection on Windows

tests/agent_server/test_file_router.py:248

@pytest.mark.skipif(
    os.geteuid() == 0, reason="root bypasses directory write permissions"
)

os.geteuid() does not exist on Windows, so this raises AttributeError at collection time and the entire test_file_router.py module errors out on windows-latest (the CI matrix in .github/workflows/tests.yml runs the suite there). Guard it with getattr:

@pytest.mark.skipif(
    getattr(os, "geteuid", lambda: -1)() == 0,
    reason="root bypasses directory write permissions",
)

This is the only use of geteuid in the repo, so it's a one-off fix.

---

Consider

2. PermissionError detail surfaces the resolved path to the client

file_router.py:699

detail=f"Permission denied: {e}",

The OSError string typically embeds the full resolved path. For an API that's callable from a browser/frontend this leaks more than necessary. The same pattern exists in search_subdirs (line 850), so it's not a regression — but since this is a new endpoint it'd be a good moment to return a generic message and log the full exception server-side (which you already do in the 500 branch). Not blocking.

3. lambda inside asyncio.to_thread

file_router.py:687

await asyncio.to_thread(lambda: target_path.mkdir(parents=True, exist_ok=True))

functools.partial(target_path.mkdir, parents=True, exist_ok=True) is the more common idiom in this file's neighborhood (e.g. asyncio.to_thread(f.write, chunk)), avoids a closure, and is marginally friendlier to introspection. Stylistic only.

4. No workspace containment check

The endpoint accepts any absolute path and creates it anywhere the process can write — consistent with _upload_file, _download_file, search_subdirs, and archive_directory, so this is an existing design property rather than something introduced here. Worth noting that the /api/file/* surface has no shared workspace-root confinement; if/when that's hardened it should cover all endpoints uniformly rather than piecemeal.

---

What's good

• Exception hierarchy ordering is correct: (FileExistsError, NotADirectoryError) -> 400, PermissionError -> 403, generic -> 500, with the is_absolute() guard evaluated before the try (so a 400 for a relative path can't be masked by the catch-all).
• Idempotency is real: exist_ok=True + the FileExistsError guard correctly distinguishes "directory already exists" (success) from "file exists where a dir was requested" (400). The test that asserts keep.txt is untouched confirms the no-clobber contract.
• Thread offload mirrors _upload_file, so slow storage won't block the event loop.
• Tests cover the right matrix: success, nested parents, idempotency, relative-path 400, existing-file 400, permission-denied 403 (with a root skip so CI doesn't false-pass), and missing-param 422.
• Purely additive — no existing contract changes, no deprecation runway needed.

---

Verdict

The only thing I'd ask to change before merge is #1 (the Windows collection break). The rest are nits/observations. Once geteuid is guarded, this looks good to go.

---

This review was written by an AI agent (OpenHands) on behalf of the requester.