fix(event): fall back to a byte-copy receipt when capability linkat is unsupported (macOS) - #309
Conversation
…s unsupported archive_validated_file hardlinks the validated predecessor through /proc/self/fd/N (Linux) or /dev/fd/N (elsewhere) with AT_SYMLINK_FOLLOW. Linux procfs permits materializing that link; macOS fdescfs answers with EPERM, so publication failed on every Darwin host and cargo test -p st2 --lib could not pass there since #300. Classify EPERM/ENOSYS/EOPNOTSUPP from the capability linkat as platform-unsupported and install the receipt as a staged, fsynced, rename_noreplace byte copy instead; every other error stays a hard failure. The caller's readback proves whichever receipt won carries the validated bytes, so supersession semantics are unchanged. The tradeoff is inode identity: a crash between copy and conditional unlink leaves the retained inbox entry in place until revalidation. A debug-only TEST_FORCE_ARCHIVE_RECEIPT_COPY switch exercises the fallback on every platform; the new ding supersession test asserts receipt bytes match and no staging files leak into the archive. Fixes #308
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ff31f6fb1c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(debug_assertions)] | ||
| if TEST_FORCE_ARCHIVE_RECEIPT_COPY.load(Ordering::Relaxed) { | ||
| return true; |
There was a problem hiding this comment.
Bypass linkat when the copy fallback is forced
When capability linkat succeeds, as it normally does on Linux CI, this flag is never consulted because capability_link_unsupported is called only inside the result < 0 branch. Consequently, archive_copy_fallback_preserves_supersede_ownership_without_staging_leftovers passes through the original hardlink path rather than exercising write_archive_receipt_copy, leaving the new fallback untested on supported platforms; check the force flag before attempting linkat or otherwise force that call to take the copy path.
Useful? React with 👍 / 👎.
|
Correction to the Verification section: |
|
Rerun of the complete suite on this branch, macOS arm64: Fully green — the earlier single failure was the documented pre-existing parallel-load flake, which passes here and on unfixed main in isolation. |
Fixes #308
Problem
archive_validated_file(src/event.rs) archives a validated predecessor by hardlinking through the open-file descriptor path —/proc/self/fd/Non Linux,/dev/fd/Nelsewhere — withlinkat(AT_SYMLINK_FOLLOW). Linux procfs materializes that link against the underlying inode. macOS fdescfs does not support it and answersEPERM, so since #300 every Darwin host fails publication, andcargo test -p st2 --libcannot pass there:Minimal repro (verified Linux OK / macOS EPERM): https://github.com/schickling-repros/2026-08-st2-linkat-devfd-eperm-on-macos
Goal
Publication must succeed on platforms where capability linkat is unsupported, without changing supersession semantics where it works today.
Decisions
EPERM/ENOSYS/EOPNOTSUPPfrom the capabilitylinkatas platform-unsupported; everything else remains a hard error so real failures surface instead of being silently degraded.create_newtemp in the archive dir (.st2-archive-{pid}-{n}, O_NOFOLLOW), write validated bytes from the fd, fsync, install via existingrename_noreplace; an install race resolves like the existingAlreadyExistspath. The caller's readback byte-comparison proves whichever receipt won carries exactly the validated bytes, so downstream ownership logic is unchanged.TEST_FORCE_ARCHIVE_RECEIPT_COPYatomic rather than an env var: process-global env mutation isunsafeunder edition 2024 and racy across parallel tests; the atomic keeps the fallback deterministically exercisable on every platform.Verification
cargo test -p st2 --lib→ 343 passed; 0 failed, including the newarchive_copy_fallback_preserves_supersede_ownership_without_staging_leftovers.codex_app_server::process_group_cleanup_reaps_a_native_launcher_descendant,ParseIntError { kind: Empty }) does not touch this code path; evidence that it fails identically on unfixed9b1a3b4bfollows in a comment.Complexity
One fallback helper + one classification helper; no new dependencies, no API changes.
References