feat(afs): implement the mount and unmount daemon routes - #701
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the daemon-side afs.mount lifecycle and routing for AFS sessions, adding an out-of-process NFS export helper (coven-afs-serve) plus startup orphan recovery, while keeping afsMount disabled by default behind an explicit opt-in.
Changes:
- Add
POST /api/v1/afs/sessions/:id/mountandDELETE /api/v1/afs/sessions/:id/mountroutes, plus capability detection forafsMount. - Introduce
crates/coven-cli/src/afs_mount.rsto manage mount/unmount, export process spawning, record persistence, and orphan sweeping at daemon startup. - Add the
coven-afs-serveper-mount export binary (behind themountfeature) and document the new routes and gating behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/daemon/socket-api.md | Documents new mount/unmount endpoints and clarifies afsMount remains off by default behind an opt-in. |
| crates/coven-cli/src/main.rs | Wires in the new afs_mount module. |
| crates/coven-cli/src/daemon.rs | Runs AFS mount orphan recovery during daemon startup. |
| crates/coven-cli/src/api.rs | Adds DELETE routing for mount, implements afs.mount, and updates afsMount capability reporting. |
| crates/coven-cli/src/afs.rs | Adds mount/unmount errors and store methods, and surfaces mount state in session view. |
| crates/coven-cli/src/afs_mount.rs | Implements mount lifecycle, record persistence, process management, and orphan sweeping. |
| crates/coven-afs/src/bin/coven-afs-serve.rs | Adds out-of-process export helper with stdout handshake + stdin control. |
| crates/coven-afs/Cargo.toml | Registers the new helper binary and its feature-scoped dependency. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+176
to
+178
| let point = mount_point(coven_home, id); | ||
| prepare_mount_point(&point).map_err(AfsError::Internal)?; | ||
|
|
Comment on lines
+50
to
+52
| /// The on-disk record, written before the route answers so that a daemon which | ||
| /// dies immediately after mounting still leaves a sweepable trace. | ||
| /// |
Comment on lines
+360
to
+365
| fn reclaim(coven_home: &Path, record: &MountRecord) { | ||
| let point = PathBuf::from(&record.mount_point); | ||
| let _ = unmount_path(&point); | ||
| let _ = std::fs::remove_dir(&point); | ||
| let _ = std::fs::remove_file(record_path(coven_home, &record.session_id)); | ||
| } |
Wires `afs.mount` end to end: POST spawns an export, mounts it, and rotates the token; DELETE takes it down; startup reclaims what a dead daemon left. The daemon does not serve NFS itself. Each mount is backed by a child `coven-afs-serve` process that owns exactly one export. Out-of-process because `coven-cli` has no tokio and should not grow an RPC stack for one optional macOS backend, because a panicking export must not take the daemon with it, and because orphan recovery wants a real pid to sweep rather than a thread to guess about. The token reaches the helper over a pipe, never argv, and the helper refuses to run with a terminal on stdout so a hand-run invocation cannot paint a live credential into scrollback. Mount rotates the gate as soon as `mount_nfs` returns, closing the window where the token sat in argv. Every failure path tears the export back down; a half-mounted session that left a listener running is precisely the orphan DESIGN.md §7 is about. `afsMount` stays false by default. The export serves a session's delta, not the merged base+delta view §3.2 specifies, so a mount today would show only files the session had already changed — bead coven-vlw. macOS daemons can set COVEN_AFS_MOUNT_EXPERIMENTAL=1 to exercise the lifecycle meanwhile. Two semantics worth stating. Unmount is idempotent: unmounting an unmounted session is the state the caller asked for, and §3.4 has no code for "was not mounted". It also does not require an open session, so a session committed while mounted can still be taken down; an unknown session is still 404. And the on-disk record carries no port and no token — it is a cleanup hint, not a credential store, and COVEN_HOME is not a secret directory. Closes: coven-dts Refs: coven-vlw Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com>
BunsDev
force-pushed
the
feat/dts-mount-routes
branch
from
August 9, 2026 23:45
e23a3b5 to
ea90c07
Compare
All three are review findings on #701 and all three are real. A non-empty mount point answered `afs.unavailable` (500) while the comment beside it cited §3.4. It is `afs.mount_busy` (409); prepare_mount_point now returns the contract's error and the test asserts the status, not just that something failed. The mount record was documented as written before the route answers so a daemon dying immediately after mounting still leaves a sweepable trace, and was in fact written after the mount. Dying in between left a live mount no later daemon knew about. It is now written first and torn down if the mount fails. The asymmetry is the point: a record without a mount is debris the sweep clears harmlessly, a mount without a record is stranded. reclaim() removed the record even when umount failed, discarding the only hint a later sweep had. That needed a distinction the code did not have: umount fails both when a mount is stuck and when there was never a mount, and those want opposite handling. still_mounted() compares the point's device against its parent's, so a record survives only while something is genuinely mounted. The sweep leaves such records for the next start, and unmount reports afs.mount_busy naming the obstruction rather than claiming a success it did not achieve. Refs: coven-dts Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Val Alexander <68980965+BunsDev@users.noreply.github.com>
This was referenced Aug 10, 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
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.
Implements
afs.mount(DESIGN.md §3.2, §3.3, §7):POST …/mountspawns anexport, mounts it, and rotates the token;
DELETE …/mounttakes it down;daemon startup reclaims mounts a dead daemon left behind.
Closes
coven-dts.Out-of-process exports
The daemon does not serve NFS itself. Each mount is backed by a child
coven-afs-serveprocess owning exactly one export. Three reasons, all ofwhich outweigh the extra binary:
coven-clihas no tokio and should not grow an RPC stack for one optionalmacOS backend.
The helper's handshake is
portthentokenon stdout, and it acceptsrotate/quiton stdin. The token never touches argv, and the helperrefuses to run with a terminal on stdout — hand-running it must not paint a
live credential into scrollback or a captured log.
Mount rotates the gate the moment
mount_nfsreturns, closing the windowwhere the token sat in argv (
ps). Every failure path tears the export backdown: a half-mounted session that left a listener running is exactly the
orphan §7 exists to prevent.
afsMount stays false
AfsNfsexports a singleAgentFs, not the merged base+delta view §3.2specifies —
OverlayFsis path-level and NFS drives 15 inode-level calls, sothere is no unified inode namespace to serve. A mount today would show only
files the session had already changed. That is filed as coven-vlw and
blocks flipping the capability.
Until then
afsMountisfalse,POST …/mountanswersafs.mount_unsupported, and a macOS daemon can setCOVEN_AFS_MOUNT_EXPERIMENTAL=1to exercise the lifecycle.Semantics worth review
Unmount is idempotent — unmounting an unmounted session returns 200,
because that is the state the caller asked for and the §3.4 table has no code
for "was not mounted". It also does not require an open session, so a session
committed while mounted can still be taken down. An unknown session is still
404, not a cheerful no-op.
The record carries no port and no token — session id, mount point, backend,
owner pid, timestamp. It is a cleanup hint, not a credential store, and
COVEN_HOMEis not a secret directory. A test asserts the serialized recordcontains neither.
Orphan recovery is non-fatal. A mount that cannot be reclaimed is untidy;
refusing to boot over it would take the daemon down for a session nobody asked
about. Deltas are never touched — unreviewed work is not garbage.
Verification
Workspace
cargo test --lockedgreen (1876 in the daemon binary), clippy cleanworkspace-wide and under
--features mount, fmt clean, secret scan clean. Thehelper's handshake was exercised live:
port/token/rotated, clean exit.Route and registry tests cover what does not need a real mount: the backend is
off without the opt-in, a live vs. dead owner decides whether a record counts as
mounted, the sweep reclaims dead owners and leaves live ones, unparseable
records are dropped, a non-empty mount point is refused, unmount is idempotent,
DELETE is routed for
mountalone, and the view/record serializations exposenothing connectable.
🤖 Generated with Claude Code