fix(agent): record the view a sandboxed session forked from - #158
Merged
Conversation
SessionEnd refuses to finalize or attest after a failed flush-record, so unrecorded work stays visible and recoverable. It decided whether there was anything to protect by asking `session.parent_view.is_some()` — a stand-in, because only the branch that opens a real repository set a parent view. The stand-in excluded sandboxes, which are real working trees holding real uncommitted work, and which had no parent view for an unrelated reason. A failed flush there finalized and attested anyway. Ask the question directly instead. The repository is already opened a few lines above to align the view, so this costs nothing but keeping the answer. Repo-less orchestrator tests still take the old path, which is what that branch was for.
A session started inside a sandbox reached the ledger with `parent_view: null`, so `session show` and anything grouping runs by where they came from had nothing to say about work that plainly came from somewhere. It was the only kind of session with that hole. The hole was a side effect, not a decision. SessionStart takes a separate branch in a sandbox — it adopts the provisioned view and does NOT fork or switch, because `create_view_from` would inject a spurious view into the canonical graph and `set_current_view` would clobber the real user's current view. The parent was only ever set by the branch that forks, so skipping the fork skipped the fact as well. Nothing about recording it requires forking. `sandbox create --from <base>` already stored the fork as the view's parent in the canonical graph, so this is a read of something atomic wrote itself: no fork, no switch, nothing written. It reports the IMMEDIATE parent, matching what the non-sandbox branch records, so a sandbox forked from a draft names that draft rather than the nearest shared ancestor. A root view has no parent and the field stays empty, which is the honest answer. Verified against a built binary, not just the unit tests. Same repo, same `sandbox create --from dev`, same session-start payload: 0.14.0 parent_view: null this parent_view: "dev" and a sandbox forked from `feature-x` reports `feature-x`. The existing sandbox test asserted `parent_view.is_none()` under the heading "must not fork a child view", which conflated not forking with having nothing to say about the fork. Only the first is an invariant, and the canonical-graph assertions beneath it are what enforce it.
CI builds with `RUSTFLAGS: -Dwarnings`, so `unused_mut` failed the Test job on all three platforms. The test forks no view, so it never needed a mutable handle.
vinceblock99
approved these changes
Aug 11, 2026
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
2 participants
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.
A session started inside a sandbox reached the ledger with
parent_view: null— the only kind of session with that hole.Not a decision: SessionStart skips the fork in a sandbox (correctly — forking would inject a view into the canonical graph, switching would clobber the user's current view), and the parent was only ever set by the branch that forks.
Recording it needs no fork.
sandbox create --from <base>already stored the parent on the view, so this reads back what atomic wrote. Reports the immediate parent, matching the non-sandbox branch; a root view stays empty.Same repo, same
--from dev, same session-start payload:parent_viewnull"dev"Look here: the first commit changes error-path behaviour. SessionEnd decided whether it had a worktree to protect by asking
parent_view.is_some()— a stand-in that excluded sandboxes. It now asks directly, reusing the repo handle opened just above. Separated so it is reviewable on its own.The existing sandbox test asserted
parent_view.is_none()under "must not fork a child view"; the no-fork invariant is still asserted by the canonical-graph checks below it. Two tests added.cargo test --workspace,clippy -D warnings,fmt --checkall pass.