feat(vfs): session persistent-volume lifecycle API + local-dir backend (RIG-3278) - #898
Open
rigel-mintaka wants to merge 3 commits into
Open
feat(vfs): session persistent-volume lifecycle API + local-dir backend (RIG-3278)#898rigel-mintaka wants to merge 3 commits into
rigel-mintaka wants to merge 3 commits into
Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Compass engineering docs preview: https://compass-runner-rig-3278-vfs.compass-eng-docs.pages.dev Deployed from |
rigel-mintaka
added a commit
that referenced
this pull request
Sep 5, 2026
…rability Additive review-fold on the P2 W1 session-volume backend (`go/internal/vfs`), folding the findings from the PR #898 review round. The TOCTOU fix under review was traced sound and is unchanged; these harden the seams around it. - HIGH: eachVolume now requires the `.compass-vfs-meta` marker dir to treat an entry as a volume, making volume identity structural rather than positional. Without it the first ReconcileOrphans would stamp W2's sibling snapshot store (frozen record: "a sibling subtree under the base dir keyed by VolumeSnapshotID") closed and Expire would delete it. Crash orphans keep their marker, so genuine-orphan reconciliation is unaffected. Regression test TestReaperIgnoresNonVolumeDirs. - MED: Attach joins the lock-release error into both previously-dropping error paths (a dropped release leaves the flock held for the process lifetime → every later Expire skips that volume forever). The typed ErrVolumeNotFound stays errors.Is-detectable through the join. - MED: ReconcileOrphans doc-contract strengthened to a normative startup-only precondition (must complete before the manager serves any Attach, not merely before the first Expire) — the interleaving that would stamp a running session's volume closed is a caller/wiring error. - MED: the close-stamp write is now durable, not just atomic — fsync the staged file before rename and fsync the metadata dir after, so a host crash cannot lose an IntentSuspended stamp (which would fail WRONG, reaping a suspended session's tree). - MED: the load-bearing race gate compares the kernel's full MAJ:MIN:INO field string (read from the held lock's own /proc/locks row, keyed by inode+pid) instead of the inode number alone, closing a cross-device false-match that could degrade the deterministic interleaving into a false green. - MED: the volumeRoot lock-namespace suffix guard gains regression coverage (bad-id table asserts errors.Is ErrInvalidSessionID). - LOW: readStamp G304 nolint reworded to the property true on every caller path; the race-gate poll gets a paced tick (bounded poll, not a timing assumption). Findings held for follow-up: the interface-shape fork (Stamp/ReconcileOrphans off the frozen VolumeManager) → RIG-3309 (Matt); reap/lock hardening (ctx-interruptible acquire + atomic rename-then-delete reap + orphan-lock reclaim) → RIG-3310. Refs RIG-3278 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-runner/rig-3278-vfs-volume-lifecycle
branch
from
September 5, 2026 20:14
4887132 to
0493af9
Compare
…d (RIG-3278) W1 of the P2 persistent-session-volume design: the `go/internal/vfs` seam that owns a session's volume lifecycle Runner-side, so a session's working tree and derived state (`target/`, `node_modules`, build caches) survive suspend / resume / eviction instead of dying with the container. ## What lands - **`vfs.go`** — the `VolumeManager` interface (`CreateVolume`, `Lookup`, `Attach`, `Snapshot`, `Archive`, `Restore`, `Expire`), the value types that cross it (`Volume`, `VolumeSnapshotID`, `ArchiveRef`, `CloseIntent`), and the package's typed errors. Layered to mirror the elastic-compute seam in `internal/compute`, so a later network-volume backend replaces the local one without touching a caller. - **`localvolume.go`** — the P2 `LocalManager` backend: one directory subtree per session (`<baseDir>/<sessionID>`) on the box's fast local storage, under an operator-configured base dir. Vendor-neutral, no storage fabric (Global Constraint 1). - **`localvolume_test.go`** — hermetic tests over every invariant and eligibility state; no wall-clock waits. ## Load-bearing mechanism: the close-stamp The backend holds three invariants over a small JSON close-stamp written by teardown (`Stamp`) and read by the reaper (`Expire`): - **(a)** `Attach` clears the stamp before returning the path, so a reopened closed-but-unexpired session never carries a past-deadline stamp into its new life. - **(b)** `Expire` takes a per-volume advisory file lock and re-reads the stamp under it, so a volume is never reaped in the window between the reaper reading a stamp and a concurrent `Attach` clearing it. `Attach` takes the same lock around its clear and **re-verifies volume existence under it** — an `Attach` blocked behind the `Expire` that reaped the volume returns `ErrVolumeNotFound` (a cold-materialize signal) rather than a path to a deleted tree. The lock file lives **outside** the volume root, on a stable inode a reap cannot unlink, which is what makes that under-lock verdict authoritative across reap+recreate. - **(c)** The stamp carries close-vs-suspend intent, supplied by the caller: a suspended session's volume is never eligible however old, because D4's suspend uses the same stop+remove teardown path a close does. Crash reconciliation (`ReconcileOrphans`) stamps every unstamped volume closed at discovery, so a crash between container-remove and stamp-write fails safe (reaped one window late) rather than open (a volume the reaper can never see). It needs no Server query and by design cannot want one — the Runner is authoritative for live-session truth. `Snapshot` / `Archive` / `Restore` are declared in the interface now (so their consumers land no interface change) but return honest not-implemented sentinels: `Snapshot`'s real body is W2's; `Archive` / `Restore`'s consumer is D4's cold-idle (OQ-2). Spec-impact: none. Refs RIG-3278 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…rability Additive review-fold on the P2 W1 session-volume backend (`go/internal/vfs`), folding the findings from the PR #898 review round. The TOCTOU fix under review was traced sound and is unchanged; these harden the seams around it. - HIGH: eachVolume now requires the `.compass-vfs-meta` marker dir to treat an entry as a volume, making volume identity structural rather than positional. Without it the first ReconcileOrphans would stamp W2's sibling snapshot store (frozen record: "a sibling subtree under the base dir keyed by VolumeSnapshotID") closed and Expire would delete it. Crash orphans keep their marker, so genuine-orphan reconciliation is unaffected. Regression test TestReaperIgnoresNonVolumeDirs. - MED: Attach joins the lock-release error into both previously-dropping error paths (a dropped release leaves the flock held for the process lifetime → every later Expire skips that volume forever). The typed ErrVolumeNotFound stays errors.Is-detectable through the join. - MED: ReconcileOrphans doc-contract strengthened to a normative startup-only precondition (must complete before the manager serves any Attach, not merely before the first Expire) — the interleaving that would stamp a running session's volume closed is a caller/wiring error. - MED: the close-stamp write is now durable, not just atomic — fsync the staged file before rename and fsync the metadata dir after, so a host crash cannot lose an IntentSuspended stamp (which would fail WRONG, reaping a suspended session's tree). - MED: the load-bearing race gate compares the kernel's full MAJ:MIN:INO field string (read from the held lock's own /proc/locks row, keyed by inode+pid) instead of the inode number alone, closing a cross-device false-match that could degrade the deterministic interleaving into a false green. - MED: the volumeRoot lock-namespace suffix guard gains regression coverage (bad-id table asserts errors.Is ErrInvalidSessionID). - LOW: readStamp G304 nolint reworded to the property true on every caller path; the race-gate poll gets a paced tick (bounded poll, not a timing assumption). Findings held for follow-up: the interface-shape fork (Stamp/ReconcileOrphans off the frozen VolumeManager) → RIG-3309 (Matt); reap/lock hardening (ctx-interruptible acquire + atomic rename-then-delete reap + orphan-lock reclaim) → RIG-3310. Refs RIG-3278 Co-authored-by: Matt Wilkinson <matt@rigel.build>
…mp clear Second review round (ReviewW1Fold) on the W1 session-volume backend graded 0 high / 3 medium / 2 low and verified six of the eight prior folds correct by mutation. This folds the two mechanical mediums; the third medium is a structural fork filed for Matt (RIG-3311), and the two lows defer to W2. - NEW-1 (medium): stampOrphanLocked was the one lock-bearing mutator missing the requireVolumeRoot guard Attach and Stamp already carry. A volume reaped between eachVolume's marker stat and the pass's lock acquisition would be resurrected by writeStamp's os.MkdirAll, after which Lookup succeeds on a dead session and provision warm-attaches an empty shell instead of cold-materializing — silently defeating the not-found-is-an-observable-signal contract. Guarded the same way its siblings are: a typed not-found is swallowed (a reaped volume is not an orphan to stamp), anything else surfaces. Regression: TestReconcileOrphansDoesNotResurrectAReapedRoot (mutation-verified non-vacuous). - NEW-3 (medium): FOLD-4 made the stamp WRITE durable but left the CLEAR non-durable. clearStamp was a bare os.Remove whose unlink could sit in the writeback window; a crash shortly after an Attach loses the clear, the past-deadline stamp reappears with its original timestamp, and the next Expire reaps a just-re-attached volume. Mirrored the write's durability: fsync the containing metadata dir after a successful remove, reusing the existing syncDir helper. Driver-verified on go-1.26.6: gofmt clean, build+vet ok, package tests ok, golangci-lint --build-tags microvm 0 issues, nilaway clean, race suite (incl. the new regression) 20/20 under -race. Refs RIG-3278 Co-authored-by: Matt Wilkinson <matt@rigel.build>
rigel-mintaka
force-pushed
the
compass-runner/rig-3278-vfs-volume-lifecycle
branch
from
September 5, 2026 22:34
b57c894 to
82602e1
Compare
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.
W1 of the P2 persistent-session-volume design: the
go/internal/vfsseam that owns a session's volume lifecycle Runner-side, so a session's working tree and derived state (target/,node_modules, build caches) survive suspend / resume / eviction instead of dying with the container.What lands
vfs.go— theVolumeManagerinterface (CreateVolume,Lookup,Attach,Snapshot,Archive,Restore,Expire), the value types that cross it (Volume,VolumeSnapshotID,ArchiveRef,CloseIntent), and the package's typed errors. Layered to mirror the elastic-compute seam ininternal/compute, so a later network-volume backend replaces the local one without touching a caller.localvolume.go— the P2LocalManagerbackend: one directory subtree per session (<baseDir>/<sessionID>) on the box's fast local storage, under an operator-configured base dir. Vendor-neutral, no storage fabric (Global Constraint 1).localvolume_test.go— hermetic tests over every invariant and eligibility state; no wall-clock waits.Load-bearing mechanism: the close-stamp
The backend holds three invariants over a small JSON close-stamp written by teardown (
Stamp) and read by the reaper (Expire):Attachclears the stamp before returning the path, so a reopened closed-but-unexpired session never carries a past-deadline stamp into its new life.Expiretakes a per-volume advisory file lock and re-reads the stamp under it, so a volume is never reaped in the window between the reaper reading a stamp and a concurrentAttachclearing it.Attachtakes the same lock around its clear and re-verifies volume existence under it — anAttachblocked behind theExpirethat reaped the volume returnsErrVolumeNotFound(a cold-materialize signal) rather than a path to a deleted tree. The lock file lives outside the volume root, on a stable inode a reap cannot unlink, which is what makes that under-lock verdict authoritative across reap+recreate.Crash reconciliation (
ReconcileOrphans) stamps every unstamped volume closed at discovery, so a crash between container-remove and stamp-write fails safe (reaped one window late) rather than open (a volume the reaper can never see). It needs no Server query and by design cannot want one — the Runner is authoritative for live-session truth.Snapshot/Archive/Restoreare declared in the interface now (so their consumers land no interface change) but return honest not-implemented sentinels:Snapshot's real body is W2's;Archive/Restore's consumer is D4's cold-idle (OQ-2).Spec-impact: none. Refs RIG-3278
Co-authored-by: Matt Wilkinson matt@rigel.build