Repository navigation
fix(minimald): run daemon tasks in the session working-directory layout - #1910
Conversation
`min task run` and the in-box `min run` path built their sandbox with the bound-directory layout, so the task's working directory was the daemon's internal tree path and `PWD` leaked it. Route both through the session layout instead, mounting the tree at `/workbench` and home at `/home`, matching what the interactive session sees. Add a `WdLayout` choice to `mctx::env::EnvArgs` and thread it through `make_env`/`make_env_with_network`. `task_producer` and the in-box run path request `WdLayout::Session`; `mip run` and the other callers keep the bound-directory default. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
[inbox-patch:clobber] This PR would revert recently merged workSimulating this merge shows it undoing commits that landed on the base branch within the last 3 day(s):
Before merging, restore the reverted region to its state on the base branch. The rest of this PR is unaffected. If a listed commit is one this PR intends to supersede, say so and merge — this check reports, it does not block. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. 📝 WalkthroughWalkthroughEnvironment construction now supports bound-directory and session layouts. Session task callers pass the session home and working paths when creating an environment. ChangesSession Working-Directory Layout
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant TaskSetup as minimald task setup
participant Context as Context::make_env_with_network
participant Env as Env::build
participant Sandbox as sandbox configuration
TaskSetup->>Context: Pass WdLayout::Session with home and working paths
Context->>Env: Forward WdLayout through EnvArgs
Env->>Sandbox: Configure session directories
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The task paths match the intended session layout, and no actionable merge-blocking issue remains after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
A rabbit hops through paths anew Comment |
A daemon-run task now uses the session layout, which dropped the task's patch mappings: read-only patches became writable through the read-write home, missing sources went unreported, and absolute paths vanished. WdSetup::Session now carries fs_mappings. sandbox2 validates, creates targets for and mounts them after the /home and /workbench binds, and mctx retargets a mapping inside the session home or tree under /home or /workbench. Document the layout's environment in the tasks reference.
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none: |
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABU1JnKg
Summary
min task runand the in-boxmin runpath built their sandbox with thebound-directory layout, so a task's working directory was the daemon's
internal tree path and
PWDleaked it. This routes both through the sessionlayout instead, mounting the tree at
/workbenchand home at/home—matching what the interactive session sees.
Changes
WdLayoutchoice tomctx::env::EnvArgs(BoundDirdefault,Session { home, working }) and branchEnv::buildon it via a singleapply_wd_layouthelper.wd_layoutthroughmake_env(forwardsBoundDir) andmake_env_with_network(new parameter).task_producer(exec.rs) and the in-boxmin runpath (env.rs) requestWdLayout::Sessionfrom the session'shome/workingpaths.mip runand the remaining callers keep the bound-directory default.Behavior notes
WdSetup::Sessionnow carriesfs_mappings;sandbox2 validates them (a missing read-only source is still an error, a
read-write one is still created), creates their targets and mounts them
after the
/homeand/workbenchbinds, so a read-only mapping inside theread-write session home stays read-only. mctx retargets a mapping inside the
session home or tree to the matching path under
/homeor/workbench;any other absolute path (e.g.
/var/run/docker.sock) keeps its own path.session's
command_env:LANG=C.UTF-8with noLC_ALL;XDG_CONFIG_HOME/XDG_DATA_HOME/XDG_STATE_HOMEmove from thestate_key-backed/state/home,/state/data,/state/stateinto theshared session home (
/home/.config,/home/.local/share,/home/.local/state);PATHgains/home/.local/bin;PS1andSHELLare set.
state_keystill backsXDG_CACHE_HOME(/state/cache).Documented in
docs/reference/tasks.md.mip runis unchanged.Verification
cargo build --workspace --locked— passescargo fmt --all --check— passescargo test -p mctx --locked— 31 passed, 3 ignored (includes newwd_layout_selects_session_or_bound_dirtest)cargo clippy --workspace --locked -- -D warnings— blocked by 3pre-existing
needless_borrows_for_generic_argserrors incrates/args/src/lib.rs(unrelated to this change)cargo test --workspace --locked— blocked by pre-existing failures incrates/checkouts(git clone environment) andcrates/minimald(networknamespace tests), unrelated to this change
live-session: native-daemon-e2e — runs on this PR; not ru 8000 nnable in the fixer sandbox