[CCColastic]

HUMAN:

the blockage happens when the workspace POST waits for a file lock already held by another process. That wait blocks the event-loop thread, delaying other requests.

---

AGENT:

Why

A workspace mutation waiting for another process to release .workspaces.lock currently blocks the server's event loop, so even /alive stops responding. Run the synchronous mutation handlers in FastAPI's worker pool so the update can wait safely while unrelated requests continue.

Summary

• Change the four workspace/parent mutation routes from async def to def, preserving the existing lock, atomic transaction, and HTTP contract.
• Add default-suite regression coverage for all four mutations, queued updates, liveness during contention, and correct results after release.
• Add maintained F17.lock-responsive live verification for all four mutations and both missing-item deletes, with an external holder, kernel-confirmed waiters, byte-integrity checks, and cleanup.

Issue Number

Fixes #5974

How to Test

uv run pytest -q tests/agent_server/test_workspaces_router.py \
  tests/agent_server/test_workspace_router.py \
  tests/agent_server/test_workspace_cookie_auth.py
.venv/bin/python .agents/skills/verify-agent-server/scripts/control_agent_server.py \
  map run --file F17 --only F17.lock-responsive --fresh --repeat 2 --record

Verified in the real sdk-audit Linux Docker container, Python 3.13.16, SDK/Agent Server 1.54.0, based on main 6e7a8532ef4426da08b5c06dd57cf4795303be01. No model calls.

• Before: all four new regression cases fail; the maintained live check fails twice, with all six mutation cases timing out on /alive after a confirmed lock wait.
• After: 43 related tests pass. The live check passes twice; all 36 probes to /alive, workspace GET, and /server_info answer 200 while mutations remain pending. Slowest response: 2.52 ms against a 2 s budget.
• Registry bytes remain unchanged while locked; after release, all mutations return their original 200/404 results and persist exactly the expected lists.
• All 22 selected F17 recipes pass, including same-server concurrency, two servers sharing one registry, the TypeScript client, auth, validation, error responses, and persistence.
• Generated OpenAPI documents are identical before and after. Per-file pre-commit checks, including Pyright, and feature-map validation pass. All lock holders and test servers were cleaned up.

Detailed runtime evidence and the sanitized archive are in the temporary evidence note.

Video/Screenshots

API measurements, kernel wait confirmations, HTTP transcripts, and test reports are included in the evidence archive linked above.

Design Doc

Small scheduling fix; the evidence note describes the execution boundary and before/after behavior. No public schema or persistence migration.

Type

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

Notes

Workspace no-op writes (#5849/#5864), other stores (#5632), lock-timeout semantics, and Windows locking remain outside this issue's scope. The existing three no-op-write recipes were not selected. Node/npm were installed only inside the audit container to run the TypeScript prerequisites; no repository dependency changed.

[github-actions]

📁 PR Artifacts Notice

This PR contains a .pr/ directory with temporary PR-specific documents. The directory will be automatically removed after this PR merges to main through a cleanup PR.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[all-hands-bot]

⏳ Review waiting on checks

The current head c7ff25462e62 still has checks that have not finished, so no review conversation was started:

• Agent Server
• Check Docstrings
• Deprecation deadlines
• Persisted settings compatibility checks
• Pre-commit checks
• REST API breakage checks
• Review Thread Gate
• Run tests
• TypeScript client CI
• TypeScript client endpoint audit
• TypeScript client integration tests
• Version bump guard

No action is needed. The scheduled scan retries once every check on the head reports a conclusion.

This is an automated check - no AI was used to generate this comment.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Scope. In-repo and correctly owned. The fix changes four Agent Server handlers in openhands-agent-server/openhands/agent_server/workspaces_router.py (POST/DELETE /api/workspaces, POST/DELETE /api/workspaces/parents) from async def to def, so the blocking fcntl.flock(LOCK_EX) wait inside FileWorkspacesStore.update() runs in FastAPI's worker-thread pool instead of the event loop. That matches this repo's own guidance in openhands-agent-server/AGENTS.md ("move state-lock acquisition into ... a worker-thread boundary") and the precedent set by #4967 / #5632. No product or ownership boundary is crossed.

What I verified on the head (c7ff254)

• The four changed handlers contain no await, so the sync conversion drops no asynchronous work.
• Generated public OpenAPI is byte-identical before and after the conversion (I rebuilt it with the four routes re-wrapped as coroutines): IDENTICAL.
• tests/agent_server/test_workspaces_router.py, test_workspace_router.py, test_workspace_cookie_auth.py: 43 passed. test_openapi_contract.py + test_api_authentication.py: 31 passed.
• The new regression test discriminates the fix rather than merely passing: run against the head handlers re-wrapped as coroutines (base behavior, no tracked file modified), all four test_workspace_lock_wait_keeps_server_responsive cases fail (contended /alive blocks ~8 s); on the head they pass.
• Feature map: map check ok (34 files, 936 ids); map owners --changed origin/main...HEAD maps the changed router to F17-workspaces-registry.md; map coverage ok.
• The committed evidence archive shows the F17.lock-responsive recipe failing on the base revision (ReadTimeout on all six mutation cases) and passing on the fix, driven through the real HTTP entry point with an external lock holder and /proc/locks waiter confirmation. That is authentic before/after runtime evidence, not a forged failure state.

Acceptance criteria. All four mutations (and both missing-item deletes) now leave the event loop free while they wait; the lock, the atomic read-modify-save transaction, the error mappings, and the HTTP models are unchanged; the missing-item deletes keep their 404 contracts. There is no public or persisted compatibility impact (no OpenAPI, schema, or persisted-format change). The pre-existing no-op-write lock acquisition (#5849 / #5864) and Windows locking remain out of scope, as the linked issue scoped them.

Context for the maintainer. Every GitHub Actions check on this head is action_required (fork-PR workflows awaiting approval) — Agent Server, Run tests, Pre-commit checks, REST API breakage checks, TypeScript client checks, etc. have not executed. My local runs cover only the focused suites, so please approve and let those checks run before merge.

✅ APPROVED

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Scope. In-repo and correctly owned. The change converts four Agent Server handlers in openhands-agent-server/openhands/agent_server/workspaces_router.py (POST/DELETE /api/workspaces, POST/DELETE /api/workspaces/parents) from async def to def, so the blocking fcntl.flock(LOCK_EX) wait inside FileWorkspacesStore.update() runs in FastAPI's worker-thread pool instead of on the event loop. This is exactly what openhands-agent-server/AGENTS.md ("Concurrency / async safety": move the synchronous lock wait into a worker-thread boundary) prescribes, and it matches the precedent set for the settings/profile stores. No product or ownership boundary is crossed; issue #5974 is open with bug / priority:medium / ready-for-dev.

What I verified on the head (a1d897d)

• The four converted handlers contain no await/async with/async for (AST check), so the sync conversion drops no asynchronous work. list_workspaces (GET) correctly stays async: it only calls store.load(), which takes no lock and cannot block.
• Generated public OpenAPI is byte-identical before and after (I rebuilt the app with the four routes re-wrapped as coroutines and diffed): IDENTICAL. No REST/OpenAPI contract change.
• Focused suites pass: test_workspaces_router.py, test_workspace_router.py, test_workspace_cookie_auth.py (48 passed); plus test_api.py, test_api_authentication.py (123 passed total) and test_openapi_contract.py (8 passed).
• The new regression test discriminates the fix rather than merely passing. Run against the head handlers re-wrapped as coroutines (base behavior, isolated copy, no tracked file modified), all four test_workspace_lock_wait_keeps_server_responsive cases fail with the contended /alive probe blocked ~8 s; on the head they pass.
• Serialization/integrity is preserved: with the handlers now running concurrently on the worker pool, intra-process mutual exclusion depends on flock. I confirmed empirically that two threads of one process each opening .workspaces.lock separately are still serialized by flock(LOCK_EX), and 16 concurrent POST /api/workspaces calls all returned 200 with all 16 entries persisted — no lost updates.
• Live evidence is authentic and matches this head: the committed archive's baseline runs pin checkout_sha 6e7a8532... (base main) and show all six mutation cases ReadTimeout; the final runs on the same base plus the fix pass all six with /alive, GET /api/workspaces, and /server_info answering 200 in 1.4-2.5 ms while the mutation is still waiting, kernel-confirmed via /proc/locks. The evidenced fixed-router.py is byte-identical to the head router.
• Feature map is consistent: map check ok (34 files, 936 ids), map coverage ok, map owners --changed maps the router to F17-workspaces-registry.md, and the new F17.lock-responsive bullet plus the README 935->936 count agree.

Acceptance criteria. All four mutations (and both missing-item deletes) now leave the event loop free while they wait; the lock, the atomic read-modify-save transaction, the error mappings, and the HTTP models are unchanged; missing-item deletes keep their 404 contract. There is no public or persisted compatibility impact. The pre-existing no-op-write lock acquisition (#5849 / #5864), lock timeouts, and Windows locking remain out of scope, as the linked issue scoped them.

Context for the maintainer. Every substantive GitHub Actions check on this head is action_required (fork-PR workflows awaiting approval) - Agent Server, Run tests, Pre-commit checks, REST API breakage checks, TypeScript client checks, etc. have not executed. My local runs cover only the focused suites; please approve and let those checks run before merge. A single unbounded lock wait now occupies one worker thread rather than the event loop; that is strictly better than the base behavior, and the unbounded-timeout question is explicitly out of scope for this issue.

✅ APPROVED