Skip to content

Default sandbox CWD to /app when directory is unset#623

Merged
phinze merged 1 commit intomainfrom
phinze/fix-app-default-sandbox-controller
Feb 18, 2026
Merged

Default sandbox CWD to /app when directory is unset#623
phinze merged 1 commit intomainfrom
phinze/fix-app-default-sandbox-controller

Conversation

@phinze
Copy link
Contributor

@phinze phinze commented Feb 18, 2026

Follow-up to #621. That PR restored the /app default in the launcher and build server, which fixes new pool creation. But existing pools are persisted in etcd with no directory field in their sandbox spec — the deployment controller happily reuses them, so the launcher default never comes into play.

This adds the /app default in the sandbox controller itself, which is where WithProcessCwd actually gets called. When co.Directory is populated (new pools, apps with custom WORKDIR), it's used as-is. When it's empty (pre-fix pools), we fall back to /app.

Fixes MIR-727

The previous PR (#621) restored /app defaults in the launcher and
build server, but those only take effect when creating new pool
specs. Existing pools persisted in etcd still have no directory
field in their sandbox spec, so the sandbox controller skips
WithProcessCwd entirely and the container boots with CWD=/.

Add the /app default here in the sandbox controller as a safety net.
When co.Directory is populated (new pools, WORKDIR apps), it's used
as-is. When it's empty (pre-fix pools), we fall back to /app.
@phinze phinze requested a review from a team as a code owner February 18, 2026 18:23
@coderabbitai
Copy link

coderabbitai bot commented Feb 18, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

The change modifies the working directory initialization logic for the sandbox container process in controllers/sandbox/sandbox.go. Previously, the process working directory was conditionally applied only when a Directory value was provided. The updated logic now always computes and applies a working directory: defaulting to "/app" when Directory is empty, otherwise using the provided Directory value. This ensures the container process always has a working directory set, removing the previous conditional behavior.


Comment @coderabbitai help to get the list of available commands and usage tips.

@phinze phinze merged commit dabb3a6 into main Feb 18, 2026
10 checks passed
@phinze phinze deleted the phinze/fix-app-default-sandbox-controller branch February 18, 2026 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant