fix(session): remove Darwin replacement predecessors - #3799
Conversation
Managed transcript rewrites were explicitly rejected on macOS after exact replacement hardening. Use a Darwin-only flock-authorized atomic exchange and retain the predecessor as durable recovery evidence. Lore-id: darwin-managed-replace Constraint: preserve Linux and Windows replacement paths Scope-risk: medium Reversibility: revertable Tested: cargo check -p pi-natives; bun --cwd=packages/natives run check:types; bun --cwd=packages/coding-agent run check:types; bun test packages/coding-agent/test/darwin-managed-replace.test.ts
The Darwin replacement regression test previously only proved that a replacement artifact existed.\n\nAssert that the exchanged predecessor retains the prior transcript bytes.\n\nLore-id: darwin-managed-replace-test\nScope-risk: low\nReversibility: revertable\nTested: bun test packages/coding-agent/test/darwin-managed-replace.test.ts
The new Darwin native implementation must satisfy the repository Rust formatter.\n\nLore-id: darwin-managed-replace-format\nScope-risk: low\nReversibility: revertable\nTested: cargo fmt --check; cargo check -p pi-natives; bun test packages/coding-agent/test/darwin-managed-replace.test.ts
Darwin append and direct replacement paths must share the exact-exchange admission lock.\n\nAvoid re-running the stale expected-identity fence after a successful exchange, retain optional loader validation context, and document the shipped surface.\n\nLore-id: darwin-managed-replace-review\nConstraint: preserve Linux and Windows paths\nScope-risk: medium\nReversibility: revertable\nTested: bun --cwd=packages/coding-agent run lint; bun --cwd=packages/natives run lint; bun test packages/coding-agent/test/darwin-managed-replace.test.ts; bun test packages/natives/test/memory-guard-native.test.ts; bun --cwd=packages/coding-agent run check:types; bun --cwd=packages/natives run check:types; cargo fmt --check; cargo check -p pi-natives
The generated declaration must match the formatted native documentation.\n\nLore-id: darwin-managed-replace-generated\nScope-risk: low\nReversibility: revertable\nTested: bun --cwd=packages/natives run build; bun --cwd=packages/natives run check:types
Managed append must fail closed while the exact replacement lock is held.\n\nLore-id: darwin-managed-replace-admission-test\nScope-risk: low\nReversibility: revertable\nTested: bun test packages/coding-agent/test/darwin-managed-replace.test.ts; bun --cwd=packages/coding-agent run lint; bun --cwd=packages/coding-agent run check:types
Managed removal and consume must not race the Darwin exact replacement exchange.\n\nLore-id: darwin-managed-replace-removal\nConstraint: preserve Linux and Windows paths\nScope-risk: medium\nReversibility: revertable\nTested: bun test packages/coding-agent/test/darwin-managed-replace.test.ts; bun --cwd=packages/coding-agent run lint; bun --cwd=packages/coding-agent run check:types
A completed managed transcript rewrite left old transcript payloads and receipt state in the session directory.\n\nThe authenticated native exchange now removes the verified predecessor before syncing the parent, while failed cleanup remains explicit and recoverable. Lore-id: darwin-managed-replace-cleanup Constraint: preserve exact-exchange admission and durable publication Scope-risk: medium Reversibility: revertable Tested: bun run build:native; bun --cwd=packages/coding-agent run check; bun test packages/coding-agent/test/darwin-managed-replace.test.ts; bun test packages/natives/test/path-identity-posix.test.ts
The macOS-only replacement path was not checked by Linux CI, leaving its safety annotations and lint requirements incomplete.\n\nMake its descriptor ownership and syscall preconditions explicit so platform checks exercise the same quality bar. Lore-id: darwin-managed-replace-lint Scope-risk: low Reversibility: revertable Tested: bun run check:rs; bun run build:native; bun --cwd=packages/coding-agent run check; bun test packages/coding-agent/test/darwin-managed-replace.test.ts; bun test packages/natives/test/path-identity-posix.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74805be197
ℹ️ 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".
| ); | ||
| }, | ||
| }; | ||
| if parent.st_dev as u64 != parent_dev || parent.st_ino as u64 != parent_ino { |
There was a problem hiding this comment.
When the managed parent is renamed or replaced after openDarwinReplacementAuth succeeds, this reopens parent_path and compares it only with the caller-supplied identity, which the TypeScript caller captures after opening the authorization. The reopened directory can therefore be a different inode from handle.parent_fd, so the exchange proceeds in a directory where the admission lock is not held and can race another append, removal, or replacement. Compare the reopened parent directly with handle.parent_dev and handle.parent_ino before touching either filename.
Useful? React with 👍 / 👎.
| const auth = openManagedDarwinReplacementAuth(root, parent); | ||
| if (!auth) throw new Error("managed_replace_exact_unavailable"); | ||
| const operationId = randomUUID(); | ||
| const parentIdentity = fs.lstatSync(parent, { bigint: true }); |
There was a problem hiding this comment.
On Darwin, if the parent disappears or becomes unreadable immediately after the authorization is acquired, this lstatSync throws before execution enters the try/finally beginning below. The native object then retains its exclusive flock until nondeterministic garbage collection or process teardown, causing subsequent append, removal, and replacement attempts in that directory to fail with migration_busy. Begin the try immediately after acquiring auth so every setup failure closes it deterministically.
Useful? React with 👍 / 👎.
| renameatx_np( | ||
| parent_fd, | ||
| source_name.as_ptr(), | ||
| parent_fd, | ||
| destination_name.as_ptr(), | ||
| RENAME_SWAP | RENAME_NOFOLLOW_ANY, | ||
| ) |
There was a problem hiding this comment.
When another same-user process ignores or does not yet implement the new advisory lock, it can replace either regular-file name after darwin_named_identity validates it and before this pathname-based syscall runs. renameatx_np is not tied to the descriptors or identities that were checked, so it can exchange an unvalidated successor; the post-check then reports committed_unproven, but the destination has already been mutated and may contain the substituted file. The exchange must make the preflight identities authoritative at the mutation boundary rather than relying solely on the cooperative flock.
Useful? React with 👍 / 👎.
|
CLOSE_NOT_ADMITTED Author gate / ledgerPASS. Exact branch state
Contract decisionCurrent
This PR introduces a parallel Darwin-only exchange API and removes that durable recovery evidence. In particular, it performs Additional admission blockers:
Existing owner boundaryNo new mutation lane is admitted. The existing session/Darwin batch remains the sole owner of replacement-predecessor cleanup semantics. A future proposal must begin with a concrete issue demonstrating a current post-#3760 defect, preserve or formally replace every crash-recovery invariant above, and receive architecture approval before native/TypeScript implementation. No source was mutated, no branch was pushed, no workflow was rerun or cancelled, and no merge action was taken. Signed: GJC — existing session/Darwin batch triage owner, 2026-08-03 |
2 participants
Summary
Verification
Normalizing native addon filename: pi_natives.darwin-arm64.node → pi_natives.darwin-arm64.node
Generated 89 explicit ESM exports in index.js, fixed 9 const enums in index.d.ts
Build complete.