Repository navigation
macOS：Commit 只刷 export 卷；可写 open 与 seal 走同一把目录 lane；Busy 结果保真 - #647
Merged
yifanxuaaa merged 23 commits intoNov 14, 2026
Merged
Conversation
When no LAYERFS_NFS_FLUSH helper is configured, flush_export previously called nix::unistd::sync(), forcing a host-wide sync(2) across every writable mount on macOS before each Workspace Commit. Add a narrow macOS FFI wrapper layerfs_bridge::process::sync_volume_wait calling Darwin's sync_volume_np(path, SYNC_VOLUME_WAIT) (SYNC_VOLUME_WAIT = 0x02 from macOS SDK <unistd.h>; checked alongside <sys/mount.h>) inside the bridge's existing audited process/darwin.rs module, and call it from flush_export on macOS. If sync_volume_np returns an error, log a warning and fall back to nix::unistd::sync() so client writeback flush is never skipped. Non-macOS targets and LAYERFS_NFS_FLUSH overrides keep their existing behavior. Tests: unit test sync_volume_wait on an existing temporary directory and a missing path in layerfs-bridge::process_identity, and test default export flush plus missing-directory fallback in layerfs-nfs::mounter. Deleted tests: none. Production LOC: 156995 -> 157027 (delta +32) Scope: first-party core/engine/control/API (core 39787, engine 73145 -> 73177, control 38513, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent bdeb9e9 vs committed tree (engine/crates/layerfs-bridge/src/process/darwin.rs +18, engine/crates/layerfs-bridge/src/process/mod.rs +0, engine/crates/layerfs-daemon/src/control/export.rs +14; tests excluded). Co-authored-by: ImmuneFOMO <rikiton.shift@gmail.com>
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Writer::acquire returned an empty lock on macOS, so every writer of the shared Store file ran with SQLite's busy_timeout=0 and met another process's write window as Busy (Ephemeral-AI-Lab#534, Ephemeral-AI-Lab#647). Widen the lane's cfg to macOS: acquire and release now flock the Store's parent directory there, an independent description that leaves SQLite's own main/WAL/SHM descriptors and fcntl locks untouched. Port the same admission to the two points that write outside a transaction. A writable Session takes the lane for its open: the PRAGMA and schema statements of an open write, and another opener now waits for it instead of failing. And the idle WAL checkpoint's try_acquire takes the same non-blocking branch on macOS as on Linux (LockExclusiveNonblock, EWOULDBLOCK -> Ok(None)), because the port must never turn that skip into a blocking wait while the caller holds its Session turn; Ephemeral-AI-Lab#570 merged into a base with this function would have blocked there. A Store open refused for Busy now answers OpenRefusal::Busy (SDK code Busy) instead of Failed, since nothing was attempted. Tests: writer_admission's cross-process and open-time cases and idle_checkpoint's held-lane case run on macOS too (cfg widened; the child-side socket is made blocking again, as Ephemeral-AI-Lab#570 does), and three_processes_open_and_reserve_concurrently is new: three children open the same Store at once and make twenty reservations each, all succeeding. Deleted tests: none. Production LOC: 157027 -> 157039 (delta +12) Scope: first-party core/engine/control/API (core 39787, engine 73177 -> 73188, control 38513 -> 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent 1c0a6fd vs committed tree (engine/crates/layerfs-persistence/src/backend/sqlite/lane/writer_admission.rs -5, engine/crates/layerfs-persistence/src/backend/sqlite/session/connection.rs +8, engine/crates/layerfs-store-service/src/store_service.rs +8, control/crates/layerfs-store/src/service.rs +1; tests excluded).
The commit path's file-digest port wraps a failed record_file_digests as WorkspaceError::Service(HistoryError). The original-cause classification only looked inside that carrier for an owner job and for an Arc<PortError>, so a HistoryError fell through to CommitError::Workspace(Service(..)), whose uncertain() is true: a definite pre-effect refusal (SQLite's Busy at BEGIN IMMEDIATE, no statement attempted, no row written) was answered as an unknown outcome and the daemon kept that control slot for good (Ephemeral-AI-Lab#540's mechanism, Ephemeral-AI-Lab#647's first trigger). Move the carrier's downcast chain into store::service_cause and add the typed HistoryError: a Busy keeps its own class as CommitError::History, which the control boundary already answers as ControlCode::Busy and whose uncertain() is false, so settle_failure resolves the capture and the slot returns. Every other carrier value is unchanged. Tests: commit_service_busy.rs classifies the exact value the digest port produces (WorkspaceError::Service(HistoryError::Busy)) as CommitError::History(Busy) with uncertain() false, keeps UnknownOutcome unknown, and leaves a non-product carrier as a Workspace service failure. Deleted tests: none. Production LOC: 157039 -> 157052 (delta +13) Scope: first-party core/engine/control/API (core 39787, engine 73188 -> 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent c5d5df2 vs committed tree (engine/crates/layerfs-store-service/src/store/captured.rs -6, engine/crates/layerfs-store-service/src/store/commit_types.rs +18, engine/crates/layerfs-store-service/src/store/mod.rs +1; tests excluded).
commit_pipeline's construction_failure_abandons_the_save reproduced exactly the value Ephemeral-AI-Lab#647 misread: a held Store write lock refuses the new chunked file's digest write during construction, which the digest port carries as WorkspaceError::Service(HistoryError::Busy). It asserted that wrapper, i.e. the old classification. Assert the Commit's first cause as it now is - CommitError::History(HistoryError::Busy) - and that it is not an unknown outcome, so the test keeps checking the same refused write with the class the control boundary answers Busy from. Tests: the same three commit_pipeline cases. Deleted tests: none. Production LOC: 157052 -> 157052 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent 4f9a966 vs committed tree; only the external test engine/crates/layerfs-store-service/tests/commit_pipeline.rs changes (tests excluded).
Ports the flock and Busy-open wording of the Ephemeral-AI-Lab#534 review fix (Ephemeral-AI-Lab#570) to the current tree, without its 0700 directory, kernel-guard and SDK changes: - docs/architecture/engine/store/README.md: the cross-process writer row names the Store parent directory flock for Linux and macOS; the writer_admission.rs test bullet covers macOS child processes, concurrent opens and reservations, and keeps Cancelled on a signal Linux-only, as the test itself is. - docs/architecture/engine/store/backends.md: Writer::acquire step, the one-writer rule and the Cancelled row say Linux and macOS; the paragraph explains the separate descriptor, writable Session::connect taking the lane, and that only a writer bypassing the lane meets SQLite's Busy. - docs/architecture/engine/store/sqlite.md: Session::connect takes the writer lock when writable and releases it after profile::check; the lane holds the directory lock on both platforms. - docs/architecture/engine/store/lifecycle.md: the idle checkpoint's try_acquire lane and the closing paragraph say Linux and macOS. - docs/architecture/engine/store-calls/service.md: open_direct reports Busy("Store open: ..") before any effect, else Failed("Store open: .."). - Comments only in lane.rs, session/checkpoint.rs and records.rs: the publication lane, the skipped idle checkpoint and Cancelled are Linux and macOS, not Linux alone. Seal is unchanged: Session::seal takes the lane under cfg(target_os = "linux") and the lifecycle page keeps saying so. Deleted tests: none. Production LOC: 157052 -> 157052 (delta +0) Core 39787 -> 39787 (+0); engine 73201 -> 73201 (+0); control 38514 -> 38514 (+0); api 5550 -> 5550 (+0); reference 0 -> 0 (+0); dsh 0 -> 0 (+0). reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 -B tools/production_loc.py --detail on exact parent 1df9185 and the staged tree.
Session::seal is the sole owner's offline handoff: it checkpoints the WAL, closes the one connection and removes the sidecars. On Linux it has always held the writers' advisory lane (a flock on the Store's directory) across that sequence; the block was still gated to Linux, so a macOS seal ran with no advisory ownership while another opener could write. Widen the gate to any(linux, macos), as writer_admission, the publication lane and the idle checkpoint already are. Off those two platforms the binding is unchanged. Tests: layerfs-persistence writer_admission (12), idle_checkpoint (4), seal_allocation (5), serverless_store (5) on macOS. Deleted tests: none. Production LOC: 157052 -> 157052 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent 1df9185 vs committed tree; only comment lines are added to lifecycle/seal.rs, so the production total is unchanged.
…che test a_running_call_keeps_its_store_open_against_a_push_or_remove keeps one call running by holding the Store directory's writer lane itself, then checks that store.remove and store.push are refused InUse while a lease is held. Since the lane exists on macOS as well, enable the case there instead of skipping it. Tests: layerfs-api store_open_cache, 2 passed on macOS (the case was gated to Linux before). Deleted tests: none. Production LOC: 157052 -> 157052 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent c0bd28b vs committed tree; only the external test api/crates/layerfs-api/tests/store_open_cache.rs changes (tests excluded).
Port the two kernel_guard cases that the macOS writer admission makes true, without the Store-directory 0700 changes they were originally written next to (this tree has no private-directory guard, so the fixture's default directory mode is enough): - open_direct_reports_busy_when_locked_and_failed_when_corrupt: another process holds BEGIN IMMEDIATE, so the open is refused OpenRefusal::Busy with "Store open: Store busy before effect" before any effect; a corrupt Store file is still OpenRefusal::Failed. - three_processes_open_direct_and_commit_concurrently plus its child helper open_concurrent_child: three processes open the same Store directly at one barrier and each commits twenty times; all opens wait instead of failing and each branch ends with exactly twenty commits. The file's OpenRefusal/StoreService import is now unconditional because the macOS cases use both. Tests: layerfs-store-service kernel_guard, 4 passed on macOS. Deleted tests: none. Production LOC: 157052 -> 157052 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent 42aa0ca vs committed tree; only the external test engine/crates/layerfs-store-service/tests/kernel_guard.rs changes (tests excluded).
c0bd28b widened Session::seal to any(linux, macos), but the two Seal flow boxes still said Linux-only: lifecycle.md's "## Seal" block read "Linux: take the directory writer lock" and seal-and-install.md's Init flow read "Linux: take the cross-process writer hold". Both now say the hold is taken on Linux and macOS, matching lifecycle/seal.rs. This corrects 368b108, whose message states that seal is unchanged and that Session::seal takes the lane under cfg(target_os = "linux"). That was true of its base 1df9185 and stopped being true when c0bd28b merged with it at 0711348; the commit message itself is left as written and this one carries the correction. Two remaining Linux-only wordings go with it: store_open_cache.rs's module doc ("(Linux, where a writer waits on its Store directory's lock)") and the glossary's Publish lane row ("the flock on the Store's directory on Linux"). kernel_guard's three-process barrier socket was 101 bytes of Darwin's 103-byte sun_path limit under the default 48-byte TMPDIR, so any longer TMPDIR reached SUN_LEN before the test could run. The fixture label shrinks to "co" and the socket to "s" (78 bytes with a 57-byte TMPDIR, verified there), with the reason in a comment; open_concurrent_child now carries the same any(linux, macos) gate as its only caller. No test assertion changes. Tests: layerfs-store-service kernel_guard (4 passed) on macOS, and again with a 57-byte TMPDIR; layerfs-api store_open_cache (2 passed); layerfs-persistence writer_admission (12), seal_allocation (5), serverless_store (5); cargo clippy -D warnings --tests for layerfs-store-service and layerfs-api; cargo fmt --all -- --check; tools/check_product_boundary.py (1343 production files). Deleted tests: none. Production LOC: 157052 -> 157052 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at parent 0711348 vs the committed tree; only docs and external tests change, so the production total is unchanged. Merge commit 0711348 ("Merge docs sync for macOS writer admission") carries no Production LOC line. Its history is not rewritten; the record is here instead: Production LOC: 157052 -> 157052 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 73201, control 38514, api 5550, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the merge's first parent 97a02e1 (157052) and at its merged tree 32df2a9 (157052); 368b108's side touches records.rs, lane.rs and checkpoint.rs in comments only, and that tree also counts 157052, so the merge changes no production line.
jiangkoumo
changed the title
Contributor
|
Ran my scripts on 7d2bfbd on the M3 Max, macOS 26.6.2, all agents starting at once. All 850 commits went through with no errors, and no daemon or mount was left after the runs: flushcheck passes 5 of 5. In the first run on a fresh home all 5 agents opened the Store too (on bdeb9e9, 4 of 5 got Busy there) |
Contributor
|
Review (static, at 7d2bfbd). Approve after small fixes. Flushing only the export volume with To fix
Smaller
|
Brings the review base up to the current upstream head so the author's comments are addressed against the tree that will land, not the 7d2bfbd base the static review was taken at. Production LOC: 157052 -> 160034 (delta +2982) Scope: first-party core/engine/control/API (core 39787, engine 73201 -> 74475, control 38514 -> 39796, api 5550 -> 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the merge's first parent 7d2bfbd (157052) and at the merged tree f6276e957 (160034); all of the delta is the merged upstream side, which this branch does not modify.
…rivate Review of Ephemeral-AI-Lab#647, items 1 and 2. Bounded waits (item 1). three_processes_open_and_reserve_concurrently (layerfs-persistence) and three_processes_open_direct_and_commit_concurrently (layerfs-store-service) each collected their children with Child::wait_with_output(), which has no deadline, so a lane deadlock would hang the test instead of failing it. Both now call one wait_bounded helper per test crate: it polls try_wait against a 60 s deadline (the per-invocation ceiling is 120 s), kills and reaps the child and panics with the child's label when the deadline expires, and otherwise reads the pipes of a child that has already exited, so no wait can outlive a live child. Private Store fixture (item 2). installed::Fixture created its Store directory with the process umask (0755 under the CI umask of 022). Ephemeral-AI-Lab#663's kernel guard refuses any Store directory that holds a group or other bit, so once both land the fixture's Store answered Unsupported and the three-process children panicked. Both fixtures now create it with mode 0700 through one private_directory helper: layerfs-store-service's tests/support/installed_store (which layerfs-daemon tests also include by path) covers Fixture::built_with and Fixture::cloned, and layerfs-daemon's own fixture covers Fixture::built. The kernel source directory and the overlay directories keep their previous modes; the guard looks only at the Store file's own directory. Tests: layerfs-persistence writer_admission 13 passed, heads 1, idle_checkpoint 4, serverless_store 5, snapshot 1, concurrent_first_installs 1, sqlite_lock_custody 2, disposable_startup_sync 1; layerfs-store-service kernel_guard 4 (both lane platforms on Linux), commit_service_busy 3, commit_pipeline 3, commit_reclaim 2, write_refusals 3, sdk_method_golden 1 under umask 022; layerfs-api store_open_cache 2; cargo +1.85.1 clippy -D warnings --tests for layerfs-persistence, layerfs-store-service and layerfs-daemon; fmt --check clean for every file this commit touches. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the merge commit 95d0aef (160034) and at this commit's tree (160034); only external tests change, so the production total is unchanged.
Review of Ephemeral-AI-Lab#647, smaller item: pre-existing on Linux and now reachable on macOS because this branch moved a seal onto the directory lane there. Session::seal released the directory writer lock after the checkpoint and the close, but before checking that no -wal, -shm or -journal sidecar remains. An opener waiting on the same lane was admitted in that gap, created -wal for its own writable session, and the check then answered Refused for a Store that had sealed correctly. The release now follows the sidecar check, so an opener that takes the lane is admitted only after the sealed Store is known to have no sidecar, and the check can only observe another process's sidecar (or a remaining file), never one an admitted opener just made. Nothing else about the seal changes: a found sidecar still answers Refused, that early return drops the retained State owner, which closes the descriptor and releases the lock, and the checkpoint, the close, the macOS persistent-WAL clear and the unused-extent release keep their order. The Seal flows in lifecycle.md and seal-and-install.md now show the check before the release. Tests: the seal's own paths on Linux - serverless_store 5 passed (including seal_refuses_retained_provider_without_effect and disposable_create_publish_seal_and_reopen_all_layouts), disposable_startup_sync 1, heads 1, snapshot 1, concurrent_first_installs 1, sqlite_lock_custody 2, idle_checkpoint 4, writer_admission 13; layerfs-store-service kernel_guard 4, commit_service_busy 3, commit_pipeline 3 (each fixture seals a Store); layerfs-api store_open_cache 2; clippy -D warnings --tests for layerfs-persistence, layerfs-store-service and layerfs-daemon. NOT_RUN: every macOS-only path here (seal_allocation, the macOS lane and sync_volume_np coverage) needs a Darwin host. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the parent 523f7fd (160034) and at this commit's tree (160034); the only production file here is lifecycle/seal.rs, which counts 79 production lines before and after (it gains 4 comment lines and moves existing code), so the total is unchanged.
Review of Ephemeral-AI-Lab#647, item 4 and the Linux names in the smaller list. Three engine pages still described the default pre-Commit flush as `sync(2)`: daemon/control-calls.md's Commit row, kernel-routes/coherence.md's writeback box and kernel-routes/nfs.md's Mount box. They now say what the branch does: with no LAYERFS_NFS_FLUSH helper, macOS flushes only the export directory's volume with `sync_volume_np(<dir>, SYNC_VOLUME_WAIT)` and falls back to `sync(2)` if that call fails, and Linux keeps `sync(2)`. The daemon's own module doc in control/export.rs said "`sync(2)` by default" the same way and is corrected with them. The Busy open code was missing from the two call-level tables a reader checks first. flows/call.md row 9a now names the lane a writable open takes (Linux and macOS) and its failure row adds `Busy` for an open that meets SQLite's busy lock before any effect; api/call-table/errors.md adds that case to the `Busy` catalog row (with store_service.rs as its source) and to the uncertain-outcome table, where a Busy open is a known no-effect refusal like the other Busy cases. Tests: none run - documentation and a doc comment only. `cargo +1.85.1 --locked fmt --check` unaffected; the pages were checked against control/export.rs `flush_default`/`flush_export`, store_service.rs `open_refusal` and control/crates/layerfs-store/src/service.rs `open`. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the parent 9030b8e (160034) and at this commit's tree (160034); the only production file touched is control/export.rs, and only its module doc comment changes, so no production line moves.
Review of Ephemeral-AI-Lab#647, smaller item: default_flush_syncs_volume_and_falls_back_ when_directory_is_missing wrote to the export through its own NFS RPC client, so the host mount client held no writeback for either flush to push and no part of the test could see which call ran. It only ever observed that a Commit succeeds in both cases, which is what its title denied. The test is renamed to default_flush_commits_with_the_export_volume_and_ without_it and says what it checks: a Commit with the live export directory (the default `sync_volume_np` succeeding) and one after the directory is removed (that call answering ENOENT, so `flush_default` falls back to `sync(2)`) both run the Commit and succeed instead of failing on the flush. The comment also names where the flush itself is covered instead: `layerfs- bridge`'s process_identity test for `sync_volume_wait`'s Ok/Err contract, and the single fallback branch in export.rs `flush_default`. No assertion changes. Tests: NOT_RUN on this host - the file, like the rest of the native macOS client coverage, is `#![cfg(target_os = "macos")]`, so nothing in it compiles on a Linux box and the rename is verified only by review. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the parent 59b143f (160034) and at this commit's tree (160034); only an external macOS-only test changes.
The call flow's step-10 failure row still said "SQLite busy (no cross-process wait on macOS)". This branch is what removed that exception: a writable open and every write transaction on macOS take the Store directory's `flock`, so the busy lock is left for a writer that bypasses the lane (a raw SQLite connection, or any process that does not go through Writer::acquire). The row now says that, matching engine/store/backends.md and engine/store/README.md, which the branch already rewrote. Tests: none run - a table cell only. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the parent 2d4bac0 (160034) and at this commit's tree (160034); documentation only.
Review item 2, corrected. 523f7fd made the whole installed::Fixture directory 0700 so the Store inside it would satisfy Ephemeral-AI-Lab#663's guard. That directory is also where the daemon harness puts its mount points and where daemon tests run `bash` as another uid: in the mount lane, 24 of 45 binaries then failed with EACCES from `bash_in`'s chdir into the mount, because the parent no longer let that uid traverse. The fixture now keeps everything it owns in `directory` at the process umask (0755 under the CI umask) - kernel source, overlay template, mount points - and puts the sealed and installed Store files in `directory/store` at 0700. The store-service fixture exposes it as `Fixture::store_directory()`, and kernel_guard's corrupt copy moves there: Ephemeral-AI-Lab#663's guard checks the Store file's own parent, which is now the private directory, while every path a test reaches as another uid keeps its old mode. The daemon's own fixture gets the same split as a field. Both three-process cases now own their children as one group instead of waiting on each in turn: wait_bounded becomes Children::wait_all, with one deadline for the whole set (60 s, under the per-invocation ceiling), a Drop that stops any child still running, and a failure that names the child which hung together with what the group wrote to stderr. The barrier handshake is bounded too: the parent's accepted streams and the child's socket get read and write timeouts, so a child that connects and never writes `R` fails the case instead of blocking the accept loop. With this, the remaining review items are in the same tree: api/call-table's Busy open also reaches the 2.0 manual (versioned/2.0.0/calls.md), engine AGENTS.md states that macOS holds the parent-directory lane over short writer jobs and writable opens so only a bypassing writer keeps SQLite's Busy, the coherence box is re-aligned, and the mounter test's comment no longer implies the fallback branch is observed by a test of its own. Tests: layerfs-persistence writer_admission 13, heads 1, idle_checkpoint 4, serverless_store 5, snapshot 1, concurrent_first_installs 1, sqlite_lock_custody 2, disposable_startup_sync 1; layerfs-store-service kernel_guard 4, commit_service_busy 3, commit_pipeline 3, commit_reclaim 2, write_refusals 3, sdk_method_golden 1; layerfs-api store_open_cache 2 - 46 passed, 0 failed. cargo +1.85.1 clippy -D warnings --tests for layerfs-persistence, layerfs-store-service and layerfs-daemon; fmt --check clean for every file here. Mount lane, selected: the 26 binaries that failed at 523f7fd pass 26/26 with this fixture layout (/data/work/ci-logs/pr647-r2-r, 05:44). Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py at the parent 180ef59 (160034) and at this commit's tree (160034); this commit touches external tests, docs and one test-support module only.
jiangkoumo
added a commit
to jiangkoumo/layerfs
that referenced
this pull request
Nov 13, 2026
Ephemeral-AI-Lab#570 P1 review) Review r663.md, points 1-3 and the smaller ones: - Point 1, the merge with Ephemeral-AI-Lab#647: its `installed::Fixture` made the Store's folder with plain `std::fs::create_dir` (0755), so its new `open_direct` tests reached this branch's guard's `Unsupported` before Busy, corrupt or concurrent (`tests/support/installed_store.rs:48,119`). Both constructors now make that folder `0700` through one `private_directory`. Verified by merging `origin/fix/578-sync-volume` into a throwaway worktree: before, `kernel_guard`'s two new tests failed with `Unsupported(... has mode 0775 ...)`; after, all 5 passed. Whichever PR lands second needs this folder to be `0700`. - Point 2: the restore's install-entry check from 82eb09f is unchanged. `install` checks the target's folder before `private_dir`, `hard_link` and `rename`, under the caller's home lock (`control/crates/layerfs-store/src/backup/restore.rs:166`). - Smaller: the claim step (a `0700` `mkdir`, ownership only when this call's own `mkdir` succeeded) is now one product function, `install::make_private_directory`, used by `prepare` and by `store.create`'s `make_empty`. This removes the second copy of the ownership rule and lets an unprivileged test drive both branches and the failed `mkdir`; the `install_directory` guard cases still need the daemons' user. - Smaller: doctor now reports a folder that is a symlink as a link, and its remedy no longer `chmod`s through the link into the target (the 0777 it reported was the link's own mode). A `store/<name>` link is scanned too, so a Store folder the daemon refuses is reported rather than skipped. - Smaller: docs say the macOS `HostProcess` Rust and Git homes are the caller's own writable ones, so a native command can change the host's toolchain. - Not split: the env passthrough (a80592c) stays in this PR; the review calls the split optional. - Point 3 (the breaking change in the PR description) is in the handoff report, which no call or code change can carry. Production LOC: 160145 -> 160194 (delta +49) Scope and method: first-party production Rust/SQL/JavaScript in core, engine, control and api; tests, fixtures, tools, docs, manifests/lockfiles and generated artifacts excluded. Counted with tools/production_loc.py --detail on the exported parent tree (this commit's parent 3f039a7) and this tree: engine 74500 -> 74506 (+6), control 39882 -> 39925 (+43); core 39787 and api 5976 unchanged. Reference absent (retired R9c; last counted 65,417 at tree 498dd19).
…derr on timeout Completes review item 1 (F1) in both three-process concurrency tests: - Children::wait_all polls the group together, stops the whole group (kill + wait) as soon as any child fails or the 60 s group deadline expires, and includes every child's status and stderr in the timeout or failure panic. - Children::abort stops the whole group and attaches every child's stderr when the barrier accept, read, or write fails or times out. - Child sockets also set a 10 s write timeout alongside the 30 s read timeout; writer_admission's barrier accept uses 10 s like kernel_guard. Tests: writer_admission (13 passed), kernel_guard (4 passed); rustfmt --check clean. Deleted tests: none. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py --detail at 30c0e56 (160034) and this staged tree (160034); external tests only.
Upstream Ephemeral-AI-Lab#671 rebuilt the architecture documentation: 10 pages this branch had edited were deleted and their content rewritten into rebuilt pages. Conflicts, all documentation (no source conflict): - Accept the deletion of docs/architecture/api/call-table/errors.md, engine/kernel-routes/coherence.md, engine/project-init/seal-and-install.md, engine/store-calls/service.md, engine/store/backends.md and engine/store/lifecycle.md. - engine/daemon/control-calls.md, engine/kernel-routes/nfs.md, engine/store/README.md, engine/store/sqlite.md, flows/call.md and glossary.md: keep upstream's rebuilt structure and carry this branch's macOS statement into the surviving section (the NFS Commit row, the NFS mount diagram, the Store limits table, the Lane paragraph). - engine/AGENTS.md: keep upstream's pinned blob links, take this branch's macOS lane sentence. The text whose page upstream deleted (the export-volume flush, the writer lane on macOS, the seal's lane hold, the Busy open) is ported into the rebuilt pages in the commit that follows, per the task's instruction to keep the port separate. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py --detail on the first parent 5534c3e (160034) and on this merge's tree (160034). Upstream/layerfs_v2_0_0 (de1d77e) alone is 159977; the branch's own production delta over that base is +57, contributed by earlier commits, not by this merge. No tests deleted by this merge.
… seal notes into the rebuilt pages Upstream Ephemeral-AI-Lab#671 replaced the pages this branch had edited, so the substantive text is carried into their rebuilt successors; only this PR's content moves, and the new structure is kept. - engine/kernel-routes/README.md: the NFS "Durability of a reply" row now names the Commit flush: the flush helper when set, else the export volume's own `sync_volume_np(<dir>, SYNC_VOLUME_WAIT)` on macOS and `sync(2)` elsewhere, with `sync(2)` also when that call fails. - engine/store-calls/README.md: `open_direct` answers `Busy("Store open: ..")` or `Failed(..)`, with a paragraph on why a `Busy` open is a definite pre-effect refusal and what bypasses the lane. - engine/store/sqlite.md: the Lane paragraph merges upstream's description of the lock (a separate directory descriptor, the macOS `fcntl` reason, `Cancelled`, `LaneHold` drop) with this branch's lane: `flock` on the parent directory on Linux and macOS, taken by a writable open, and only a bypassing writer keeps exact `Busy`. The lane diagram, the idle checkpoint and the lifecycle block say "Linux and macOS" / "release the writer lock". - engine/store/README.md: the direct-open row keeps macOS apfs/hfs; a cross-process-writers row and the invariant say Linux and macOS; `Cancelled` leaves "Linux"; the `writer_admission` demonstration no longer claims its cross-process cases are Linux only. - engine/project-init/README.md: seal takes the lane before the checkpoint and releases it after the sidecar check, and says why (a waiting opener could otherwise create a `-wal` and make a sealed Store Refused). - api/calls/errors-and-events.md: `Busy` covers a writable open that met SQLite's busy lock before any effect, and stays a known outcome. - flows/call.md: hop 7a names the writable open's lane, hop 8 the writer on both hosts. Production LOC: 160034 -> 160034 (delta 0) Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19). Method: tools/production_loc.py --detail at the parent 19a6398 (160034) and at this commit's tree (160034). Documentation only.
The seal and install steps gained two lines ("take the writer lane",
"release the writer lane"), which left the paired `install_file` column one
space off. Realign both, content unchanged.
Production LOC: 160034 -> 160034 (delta 0)
Scope: first-party core/engine/control/API (core 39787, engine 74475, control 39796, api 5976, dsh 0; reference absent, retired R9c, last counted 65,417 at tree 498dd19).
Method: tools/production_loc.py --detail at the parent e24c1b4 (160034) and this commit's tree (160034). Documentation only.
Collaborator
Author
|
谢谢这轮审阅。四条要修、五条较小意见都处理了；起点
验证：全量 fast 1902 passed（26 skipped）、slow 148 passed（11 skipped）、mount 45/0； |
jiangkoumo
changed the title
Contributor
|
Ran the macOS parts on 19aaeb1, M3 Max, macOS 26.6.2, For the lane timing, same agent scripts as last time, all agents starting at once on a fresh home. 850 of 850 commits went through with no errors, and no daemon or mount was left: flushcheck passes 5 of 5 |
A failed sync_volume_np no longer falls back to sync(2) and runs the Commit: on macOS sync(2) may return before the writes are flushed, so the Commit could capture without the client's buffered writes and hide the error. The default flush now refuses the Commit as Failed at commit:flush, as a failed flush helper already does, and leaves the Workspace idle. Linux keeps sync(2). Tests: layerfs-nfs mounter's second case now expects the refusal (renamed default_flush_commits_with_the_export_volume_and_refuses_without_it). Docs: kernel-routes/README.md and daemon/control-calls.md state the refusal. Production LOC: 160034 -> 160034 (delta 0); scope core 39787, engine 74475, control 39796, api 5976; reference absent (retired R9c; last counted 65,417 at tree 498dd19); method tools/production_loc.py on the parent and the committed tree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…#663, Ephemeral-AI-Lab#642) into fix/578-sync-volume Conflict resolved in engine/crates/layerfs-store-service/tests/support/installed_store.rs: this branch's fixture layout is kept (the Store in a private directory/store, the fixture root left traversable for mounts), and the duplicate private_directory helper Ephemeral-AI-Lab#663 added is dropped. Production LOC (first parent 11b30f0): 160034 -> 160621 (delta +587, the base's changes brought in); scope core 39787, engine 74475 -> 74765, control 39796 -> 40093, api 5976; reference absent (retired R9c; last counted 65,417 at tree 498dd19); method tools/production_loc.py. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Review record
Reviewer: Claude Opus 5.5 (independent-context review, then spot-checked) |
yifanxuaaa
added a commit
that referenced
this pull request
Nov 14, 2026
Brings #656 up to the v2 tip (64 commits). Conflicts and how they were resolved: - writer_admission.rs, lane.rs: both sides enabled the macOS directory lane identically; took base (#647). - lease.rs: base #666 already treats a stale lease its holder dropped as released (remove_stale); took base. The PR's lease_race test still passes and is kept. - connection.rs: took base's open rule (#647): a writable open takes the lane and releases it after the profile check; a read-only open takes none. Kept the PR's ConnectionSlot and State.path so the last-close custody in session/lifecycle.rs still applies. - seal.rs: kept the PR's checked close through ConnectionSlot (a failed close restores the connection) and base's order: the lane is held through the sidecar check and released after it. - glossary.md: took base (already says Linux and macOS). - docs: #671 deleted api/sdk/machine/{README,stop}.md and engine/store/backends.md. Their PR content moved to api/sdk/machine.md (machine.stop force), api/sdk/sandbox.md (native forced close) and engine/store/sqlite.md (last-close lane row). Deleted test (it asserts the PR's read-only-open admission, which base's #647 rule does not have): writer_admission.rs another_process_admission_precedes_read_only_connection_open. The last-close admission test is kept and passes. Checks (macOS, toolchain 1.85.1, registered worktree via tools/worktree.py): - Clippy --all-targets -D warnings: layerfs-persistence; layerfs-api (machine, postgresql), -sandbox, -machine, -control, -platform, -store; layerfs-cli (postgresql), layerfs-daemon: clean. - Tests: persistence writer_admission 13, sqlite_lock_custody 2; store lease_race 1, lease_stale 1; platform machine_force 9, machine_lima 8; cli capabilities 4, stream_framed 3; api (machine, postgresql) machine_* and sandbox_* 30 across 17 suites, sandbox_capacity 8: all passed. - rustfmt clean on the resolved files. cargo fmt --check still flags api/crates/layerfs-cli/tests/concurrent_reads.rs, which comes from base unchanged. - Not run: machine_force_native (needs a matching private daemon), Linux and Docker E2E. Durable: NOT_RUN - disabled by owner until explicit reauthorization. Production LOC: 159204 -> 161104 (delta +1900), first parent eda9c04 against the merged tree; core 39787 -> 39787, engine 74384 -> 74810, control 39465 -> 40527, api 5568 -> 5980. The delta is the base's 64 commits; against layerfs_v2_0_0 (160621) the PR now adds +483. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: tools/production_loc.py (blob 35e4a6d) --root <git archive snapshot> --json on both trees. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
yifanxuaaa
added a commit
that referenced
this pull request
Nov 14, 2026
…l Store writes safe (#656) Adds an explicit `force` option to `machine.stop` and makes `sandbox.close --force` work on `native` without a Store or daemon. Both stop the runtime through its provider or its own records, never through the machine's worker, and keep disks, Stores, saved history and records. - machine.stop --force: Lima `limactl stop --force`, Docker stops only this home's worker container, native ends only this home's recorded Sandbox runtimes (the host stays `ready`). Stopped Sandboxes are recorded `gone`, unconfirmed ones `unresolved`; uncertain outcomes leave an uncertain op, never replayed or reported as success. On native a Sandbox is uncertain only once its stop was sent; a refusal before that is certain, the stop goes on to the next Sandbox, and the answer lists refused_sandboxes. Record-keeping failures are reported in custody_errors beside the stop's own refusal. A forced stop may settle an uncertain stop this process left or one whose process ended. - Forced native sandbox.close: no Store open or daemon call; Busy while another live process holds native; a runtime that stopped before a later failure is recorded stopped (detail.runtime_stopped). - Machine `not_held` gates on Store, Sandbox and Workspace calls while a machine operation is in progress or unresolved. - Store (SQLite, macOS and Linux): the Store directory lane also covers a writable session's last close; read-only and diagnostic closes take no lane. A failed lane wait closes with checkpoint-on-close off (no second attempt). - Docker machine.stop refuses a container that is not a LayerFS worker (InvalidInput). - Sandbox record: new machine_stopped and machine_stop_unknown fields; "v" stays 1. Tier: P1; gate: yes (risk:store, risk:format, Store-access gate). Owner approved and accepted review items A, N1 and 10; review records on the PR (Fable 5.2, independent context). Docs: api/sdk/machine.md, api/sdk/sandbox.md, engine/store/sqlite.md, control/sandboxes/README.md, glossary.md. Durable: NOT_RUN - disabled by owner until explicit reauthorization. Checks at the merged tree: Clippy --all-targets -D warnings clean on all changed crates (macOS and Linux); macOS 57 tests and live machine_force_native passed; Linux 38 tests passed; Linux FUSE mount lane 45/45 binaries, 116 tests passed. Not run: live Docker/Lima providers. Deleted tests: writer_admission another_process_admission_precedes_read_only_connection_open (base #647 keeps read-only opens off the lane). Production LOC: 160621 -> 161187 (delta +566), first parent layerfs_v2_0_0 27920d3 against the merged tree; core 39787 -> 39787, engine 74765 -> 74818, control 40093 -> 40602, api 5976 -> 5980. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: tools/production_loc.py (blob 35e4a6d) --root <git archive snapshot> --json. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jiangkoumo
added a commit
to jiangkoumo/layerfs
that referenced
this pull request
Nov 14, 2026
Clean merge: no code or documentation conflict, so no resolution was needed. The upstream side brings Ephemeral-AI-Lab#678, Ephemeral-AI-Lab#679/Ephemeral-AI-Lab#680, Ephemeral-AI-Lab#663, Ephemeral-AI-Lab#642, Ephemeral-AI-Lab#681 and Ephemeral-AI-Lab#647. Production LOC: 160158 -> 160802 (delta +644) Scope and method: first-parent comparison at 67559fd vs this merge tree via python3 -B tools/production_loc.py over first-party product Rust/SQL/JavaScript only (reference absent, retired R9c).
jiangkoumo
added a commit
to jiangkoumo/layerfs
that referenced
this pull request
Nov 14, 2026
Brings the Ephemeral-AI-Lab#136 helper reaping work onto upstream after the reviewer's first pass: Ephemeral-AI-Lab#656 force-stop of machines and native Sandboxes, Ephemeral-AI-Lab#647 macOS export-volume flush and exact Busy, Ephemeral-AI-Lab#682 image-loaded Sandboxes, Ephemeral-AI-Lab#681 GC design, Ephemeral-AI-Lab#642 one-read NFS records. Conflicts were documentation only, and are resolved into upstream's structure: - engine/daemon/control-calls.md: upstream's macOS `sync_volume_np` flush row kept; this branch's Attach (mount table read once, `tracked`), Unmount (mount-table settlement), ForceUnmount (finishing an export kept at Detach) and Helper run (retained queue) rows merged into upstream's table. - engine/kernel-routes/nfs.md: upstream's Commit line kept inside this branch's Attach/Unmount/ForceUnmount diagram. Code merged without conflicts; Ephemeral-AI-Lab#656 touches Sandbox/machine force-stop and the Store lane, not the NFS export teardown, so the two do not overlap in the data path. `cargo +1.85.1 check --locked --all-targets` on the changed crates passes at this tree. Durable: NOT_RUN - disabled by owner until explicit reauthorization. Production LOC: 160218 -> 163746 (delta +3528); first parent b49690d. core 39787 -> 39787, engine 74627 -> 75078, control 39828 -> 42804, api 5976 -> 6077. Reference absent (retired R9c; last counted 65,417 at tree 498dd19). Method: python3 tools/production_loc.py --root <git archive of b49690d> and --root <the merged working tree>.
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.
Tier: P1 (engine/crates/layerfs-persistence, layerfs-store-service, layerfs-bridge, layerfs-daemon) ; risks: risk:store, risk:security (process/**, new sync_volume_np FFI), risk:deps (nix dev-dependency target widened to macOS), perf (backend/sqlite/lane) ; gate: yes: risk:store, risk:security, engine/AGENTS.md ; de-escalated: none
Docs: daemon/control-calls, kernel-routes/{README,nfs}, store/{README,sqlite}, project-init/README, store-calls/README, flows/call, api/calls/errors-and-events, glossary, engine/AGENTS.md, versioned/2.0.0/calls.md
Durable: NOT_RUN — disabled by owner until explicit reauthorization
Change by the reviewer (
11b30f0d76), superseding scope item 1's fallback: a failedsync_volume_npno longer falls back tosync(2). The Commit is refused asFailedatcommit:flush, as a failed flush helper is, and the Workspace is left idle. On macOSsync(2)may return before writes are flushed, so the fallback could commit without the client's buffered writes. Themountertest now expects that refusal.Visible change: a writable open that hits SQLite busy before any effect now reaches the SDK as
Busy(wasFailed).范围
sync(2)）。未配置LAYERFS_NFS_FLUSH帮手时，macOS 走
sync_volume_np(<export 目录>, SYNC_VOLUME_WAIT)，只把该卷的写回刷到完成；调用失败（例如目录已不在，
ENOENT）时记录 warning 并回退sync(2)，不会跳过客户端写回。Linux 保持
sync(2)。这不是对 Disposable backing 的 fsync。flock在 macOS 也生效（与 Linux同一把 lane、同一段代码）。行为变化：macOS 上可写 open（
Session::connect）与每个写事务一样阻塞等待这把 lane（毫秒级，作者实测 3–19 ms），两个 LayerFS opener 因此排队，而不是撞
SQLite busy。seal 在 macOS 上也取同一把 lane，并一直持有到 sidecar 检查之后。
（典型是绕过 lane 的裸 SQLite 写入者），现在是 definite 的
OpenRefusal::Busy，到 SDK 的 code从
Failed变为Busy，调用方可以安全重复。对外可见的变化。service_cause把 service port 里的HistoryError按原类型还原为CommitError::History，因此每个变体都保留自身分类（不只是Busy）；不确定性仍只来自HistoryError::UnknownOutcome。kill+wait 整组 + 握手读写有超时 + 失败/超时附全部子进程 stderr”；
installed::Fixture把 Store 放进directory/store（0700），fixture 根目录保持 0755，既满足 fix(store): Store 目录 0700、macOS 原生命令透传 Rust/Git 路径、失败只回收本次创建的目录（#570 补全） #663 的守卫，又让 mount 点对非 root uid 可穿越；macOS 客户端 flush 测试不再声称能观察
到回退调用；文档同步到上游 2.0 docs: delete most docs and reorganize by layer (core, engine, control, api) #671 重建后的新页（
daemon/control-calls、kernel-routes/{README,nfs}、store/{README,sqlite}、project-init/README、store-calls/README、flows/call、api/calls/errors-and-events、glossary、engine/AGENTS.md、versioned/2.0.0/calls.md）。数字（本次真实执行）
de1d77e13159977 → 本 PR 相对上游净 +57 生产行（14 个
src/文件）；control/export.rs952 物理行 / 838 生产行（上限 999）。19aaeb1b5）：fast 1902 passed（26 skipped）、slow 148 passed（11 skipped）、mount 45/0。
--workspace --all-targets -D warnings干净；fmt --all --check只剩上游既有的api/crates/layerfs-cli/tests/concurrent_reads.rs:16，本 PR 触及的文件全部干净。writer_admission13/13、kernel_guard4/4，各连跑 3 次全通过。已知限制
Unsupported，无回退。sync_volume_np、seal_allocation、mounter 测试、macOS lane 时序）在本次Linux 环境是 NOT_RUN，需要 Darwin 主机补跑。
control/export.rs余量已不多，与 fix(daemon): 原生 NFS 卸载超时后回收 helper 并完成 teardown（layerfs-project#136） #644/fix(nfs): spill AppleDouble companion bodies to disk and expand workspace capacity to 65,536 #636 合流后先拆模块再加功能。