Defer the COW branch decision past process exit - #219
Merged
Conversation
congwang-mk
force-pushed
the
defer-branch-action
branch
from
September 10, 2026 00:22
d6fdac0 to
40e9c81
Compare
A sandbox's COW branch was committed, aborted, or kept only inside Sandbox::drop, so a caller had no way to look at what a run changed and then decide (issue #215: an MCP tool-call proxy wants to hold a tool's writes pending an external approval). Keep left the upper behind with nothing to act on, and dry_run showed changes that could never be kept. Defer makes wait() leave the branch in the sandbox for commit()/abort(), with pending() and upper_dir() to inspect it, and dropping an undecided sandbox preserves the branch like Keep so nothing is published without a decision. wait() now settles the branch itself, reading the change set into RunResult::changes first, which makes dry_run a plain run with both actions set to Abort, so dry_run and DryRunResult are removed. Signed-off-by: Cong Wang <cwang@multikernel.io>
Sandbox::dry_run is gone from the core: a dry run is a run whose branch actions are both Abort, and every RunResult now carries the change list. The flag keeps its behaviour and output format, it just sets the actions and prints result.changes instead of calling a dedicated entry point. Signed-off-by: Cong Wang <cwang@multikernel.io>
The C ABI mirrors the core change: the branch action setters accept 3 for Defer, sandlock_result_* gains the change list accessors that the removed sandlock_dry_run_result_* type used to carry, and a handle whose run exited under Defer stays alive for sandlock_handle_commit/abort, with sandlock_handle_pending and sandlock_handle_upper_dir to inspect it. Freeing a pending handle preserves the branch, so a binding that crashes mid-decision never publishes. Header regenerated with cbindgen. Signed-off-by: Cong Wang <cwang@multikernel.io>
wait() frees the native handle on its way out, and that free is what used to run the merge, so Python needs to know up front that the caller wants to decide: DEFER keeps the handle parked on the Sandbox until commit()/abort() frees it, and pending/upper_dir expose it meanwhile. A pending sandbox refuses another run, and leaving the context preserves an undecided branch. Every Result now carries changes, which makes the old dry_run() a run with on_exit=ABORT, so it and DryRunResult go away. Signed-off-by: Cong Wang <cwang@multikernel.io>
Run frees its handle before returning, so it rejects Defer outright; the disposition lives on Process, whose Wait keeps the handle when the run exited under Defer and exposes Pending/UpperDir/Commit/Abort. Close on a pending Process preserves the branch. DryRun is removed along with the C entry points it wrapped; a dry run is Run with OnExit set to Abort and the Changes now on every Result. Signed-off-by: Cong Wang <cwang@multikernel.io>
congwang-mk
force-pushed
the
defer-branch-action
branch
from
September 10, 2026 16:39
40e9c81 to
a9e3c6b
Compare
The change kind was derived from the whiteout set, so any tool that unlinks before it writes (sed -i, mv over a file, rm then recreate) reported the path as Added while its old bytes were still in the workdir. A caller diffing upper_dir against the workdir to review a deferred run then compared the new file against nothing. Modified now means the path exists on both sides, regardless of how it got there. Directories the run created were skipped by the upper walk even though commit creates them, so an empty directory was invisible to the caller deciding whether to commit. Signed-off-by: Cong Wang <cwang@multikernel.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #215.
A sandbox's COW branch was committed, aborted, or kept only inside
Sandbox::drop, so a caller could never look at what a run changed and then decide.Keepleft the upper behind with nothing to act on, anddry_runshowed changes that could never be kept.What changes
BranchAction::Deferleaves the branch in the sandbox afterwait().Sandbox::pending(),upper_dir(),commit(), andabort()inspect and dispose of it.on_errorkeeps its own action, soon_exit = Defer, on_error = Abortsends only successful runs to a decision.RunResultcarrieschanges, read from the branch before the exit action is applied. Empty without a workdir.dry_runandDryRunResultare removed. A dry run is a run with both actions set toAbortplusresult.changes. The CLI--dry-runflag keeps its behaviour and output format on top of that.Keep, so nothing is ever published without a decision and nothing is lost.wait()now settles the branch itself. TheCommitmerge runs underspawn_blockingso a contended workdir lock no longer stalls an async worker;Droponly sees a branch a cancelledwait()left behind.TransactionrejectsDeferon a stage, alongside the actions it already rejects.The same surface is mirrored in the C ABI (
sandlock_handle_{pending,upper_dir,commit,abort},sandlock_result_change*, discriminant 3), Python (BranchAction.DEFER,Result.changes,sandbox.pending/upper_dir/commit()/abort(), also acrosspopen), and Go (BranchActionDefer,Result.Changes,Process.{Pending,UpperDir,Commit,Abort};Runrejects Defer because it frees its handle before returning).Semantics worth knowing
commit()blocks up to 5s on a workdir another sandbox is merging into. On timeout it returns an error and the branch is preserved, path in the message.Committoday, only later.Modifiedmeans the file was opened for writing and exists in the workdir. Contents are not compared, and the doc comment now says so.Testing
Core lib (743), core integration, CLI, FFI, Python (417), and Go suites pass. One integration test,
test_chroot_hardlink_into_a_branch_is_refused, fails on this host both onmainand here because the chroot rootfs prints "Cross-device link" where the test expects glibc's "Invalid cross-device link".