fix(server): tolerate existing workspace dirs

[Nikhil (shadowfax92)]

Summary

• Adds a shared server-side ensureDirectory helper that keeps recursive mkdir behavior but tolerates EEXIST when the requested target already exists as a directory.
• Routes BrowserOS runtime/workspace directory setup and workspace-producing filesystem tools through the helper.
• Adds regression coverage for the Windows OneDrive-style EEXIST parent-directory case.

Fixes #974

Test plan

• bun run typecheck from packages/browseros-agent/apps/server
• bun --env-file=.env.development test ./tests/lib/ensure-directory.test.ts ./tests/lib/agents/acpx-runtime-context.test.ts ./tests/tools/filesystem/write.test.ts from packages/browseros-agent/apps/server

[github-actions]

✅ Tests passed — 1233/1237

Suite	Passed	Failed	Skipped
✅ agent	80/80	0	0
✅ build	9/9	0	0
✅ eval	93/93	0	0
✅ server-agent	266/266	0	0
✅ server-api	205/205	0	0
✅ server-browser	4/4	0	0
✅ server-integration	9/10	0	1
✅ server-lib	245/245	0	0
✅ server-root	60/63	0	3
✅ server-skills	31/31	0	0
✅ server-tools	231/231	0	0

View workflow run

[greptile-apps]

Greptile Summary

This PR introduces a shared ensureDirectory helper that wraps mkdir({ recursive: true }) and gracefully tolerates EEXIST when the target path already exists as a directory — fixing a Windows/OneDrive quirk where Node.js raises EEXIST for an intermediate parent even with recursive: true.

• All mkdir({ recursive: true }) call-sites across acpx-runtime-context.ts, browseros-dir.ts, write.ts, and page-actions.ts are replaced with the new helper; the changes are mechanical and correct.
• The helper uses dependency injection (deps.mkdir / deps.stat) for testability and correctly re-throws EEXIST when the target exists as a non-directory (e.g. a file), avoiding silent data-loss scenarios.
• Test coverage handles the core cases (create new dirs, existing-dir EEXIST tolerance, file-at-path rejection) but is missing a case that documents the known limitation: when EEXIST originates from a parent and the target does not yet exist, stat throws and the original EEXIST is rethrown unchanged.

Confidence Score: 4/5

Safe to merge; the helper correctly narrows tolerance to genuine directory-already-exists cases and all call-sites are mechanical substitutions.

The implementation is sound for its stated purpose. The only gaps are a missing test for the EEXIST-from-parent-but-target-is-new scenario and the fact that statExistingDirectory swallows the secondary error when rethrowing — both affect debuggability rather than correctness on any currently exercised path.

The new ensure-directory.ts and its test file warrant a second look around the statExistingDirectory error-swallowing and the incomplete mock coverage for the Windows EEXIST edge case.

Important Files Changed

Filename	Overview
packages/browseros-agent/apps/server/src/lib/ensure-directory.ts	New helper wrapping mkdir({ recursive: true }) with EEXIST tolerance; injects deps for testability. Logic is sound but the EEXIST-when-target-is-new edge case silently rethrows.
packages/browseros-agent/apps/server/tests/lib/ensure-directory.test.ts	Good coverage for the main paths; the Windows/OneDrive test uses fully-mocked deps but is missing a case where stat itself throws (target doesn't exist yet), leaving the failure mode undocumented.
packages/browseros-agent/apps/server/src/lib/agents/acpx-runtime-context.ts	All seven mkdir({ recursive: true }) call-sites replaced cleanly with ensureDirectory; mkdir import removed.
packages/browseros-agent/apps/server/src/lib/browseros-dir.ts	Six ensureBrowserosDir mkdir calls replaced with ensureDirectory; straightforward mechanical change.
packages/browseros-agent/apps/server/src/tools/filesystem/write.ts	Single mkdir replaced with ensureDirectory; no logic change.
packages/browseros-agent/apps/server/src/tools/page-actions.ts	Single mkdir replaced with ensureDirectory for the download temp-dir base; no logic change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["ensureDirectory(path)"] --> B["mkdir(path, { recursive: true })"]
    B --> C{Success?}
    C -- yes --> D[Return — directory ready]
    C -- "throws EEXIST" --> E{isAlreadyExistsError?}
    E -- no --> F[Re-throw error]
    E -- yes --> G["stat(path)"]
    G --> H{stat succeeded?}
    H -- "throws (e.g. ENOENT)" --> I[Re-throw original EEXIST]
    H -- yes --> J{isDirectory?}
    J -- no --> K[Re-throw original EEXIST]
    J -- yes --> D

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
packages/browseros-agent/apps/server/tests/lib/ensure-directory.test.ts:33-56
**Missing test: EEXIST for brand-new target (stat throws ENOENT)**

The mocked `stat` always returns `isDirectory: true`, so the test only covers the case where the target directory already exists. It leaves unverified what happens when Windows/OneDrive throws EEXIST for an intermediate parent but the target itself doesn't exist yet — in that path, `stat(target)` throws `ENOENT`, `statExistingDirectory` catches it and rethrows the original EEXIST, and the directory is never created. Adding a test where `stat` throws would document this known limitation and prevent future maintainers from assuming `ensureDirectory` is a universal fix for all Windows EEXIST variants.

### Issue 2 of 2
packages/browseros-agent/apps/server/src/lib/ensure-directory.ts:36-39
**Swallowed stat error obscures root cause**

When `stat(path)` throws (e.g. `ENOENT` because the target doesn't exist), `statExistingDirectory` discards that error and rethrows the original `EEXIST` from `mkdir`. A caller or operator debugging a failed workspace-creation on Windows will see `EEXIST` with no signal that the real problem is `ENOENT` on the target. Augmenting the original error with a `cause` or a short message before rethrowing would make the failure much easier to diagnose without changing the externally visible error code.

_{Reviews (1): Last reviewed commit: "fix(server): tolerate existing workspace..." | Re-trigger Greptile}