Repository navigation
fix(session): pin managed reaper identity without weakening native guards - #6329
Conversation
Hash the descriptor exclusively owned by exact regular matching without changing identity, no-follow or post-hash name validation. Genuine full-cardinality setup now also asserts all poison files are removed. This isolated experiment is NOT qualified: the unchanged 20-second acceptance still timed out at 20.766 seconds. Lore-id: 9c8043e1 Constraint: no timeout, cardinality, fsync or assertion waiver Tested: genuine native build and generated digest Tested: native identity boundary suites, 32 pass and 2 platform skips Tested: native package check and strict Rust gates Not-tested: full-cardinality timing acceptance remains red Confidence: medium Scope-risk: narrow Reversibility: revert
Re-observing the same parent for each file wastes work and can adopt a replacement root mid-pass. Pin only immutable identity scalars while native validation remains mandatory for each effect. Preserve lookup and final-parent symlink failure accounting; snapshots never grant deletion permission. Lore-id: a5738b0c Constraint: preserve native no-follow, byte, identity and fsync boundaries Constraint: keep all 50001 files and unchanged 20-second acceptance Tested: full session-directory and native identity union, 121 pass and 17 existing skips, 711 assertions Tested: coding-agent vendor, Biome and TypeScript checks Not-tested: current-dev integration and non-Darwin execution Confidence: high Scope-risk: narrow Reversibility: revert
The new dev base backmerged release 0.18.7. Rebuild the addon from this exact Rust tree rather than relabeling older 0.18.6 bytes; record only the genuinely generated descriptor. Lore-id: 2c86e950 Constraint: no manual trusted metadata stamping or old native byte reuse Tested: real current-source native build Tested: full session-directory plus native identity suites Tested: coding-agent package and strict Rust gates Confidence: high Scope-risk: narrow Reversibility: revert
probepark
left a comment
There was a problem hiding this comment.
Review (head 3c91cbd, gajae-reviewer on behalf of probepark) — re-requested 2026-10-04T14:32:39Z
CI: green — Dev CI run https://github.com/Yeachan-Heo/gajae-code/actions/runs/37209652356 complete: Affected path validation aggregate success (15:01:27Z), including check:@gajae-code/coding-agent, check:@gajae-code/natives, rust-check, rust-test, cargo-build, native-build, session-directory.test.ts, path-identity-posix.test.ts, Windows dev:doctor/session-path, RSS compare, gjc-state-gates. Virtual integration validation success. 0 failures. Approve gate (gajae-approve-gate.py, fresh read): ALLOW, check coverage present for both touched packages.
Scope: +429 / -42, 6 files — crates/pi-natives (path_identity.rs), packages/coding-agent (managed-session-storage.ts, session-directory.test.ts), packages/natives/native/diagnostic-artifact.json, two changelog.d fragments. Production lines ≈101.
Conventions: changelog.d fragments present for coding-agent (Fixed) and natives (Changed); no released CHANGELOG section edited; no generated-file hand edits (models.json / schemas / docs-index untouched); no labels; no console.*, no mock.module().
Notable:
crates/pi-natives/src/path_identity.rs:5980— wrapping the owned fd inFile::from_raw_fdmakes the close RAII-driven on every return path (including the earlyfstatfailure and theS_IFREGOk(false)exit), replacing the manuallibc::closeafter the closure. No double close:digest_fd's dup is gone and nothing else closesfd. Post-hash no-follow name check is unchanged. Clean.packages/coding-agent/src/session/internal/managed-session-storage.ts:951,1031— the parent dev/ino is captured once per invocation, only after the first eligible candidate, and passed as an expectation. Native still does the no-follow walk and parent/file identity check on each unlink. A mid-pass replacement now returnsparent_mismatch, which is not inisBenignReaperRace, so it counts as a failure (no silent adoption). The replacement/symlink/diagnostic-EACCES/ENOENT tests atsession-directory.test.tscover both the sync and async paths with real renames, and every spy is restored infinally.- Note (non-blocking):
managed-session-storage.ts:1062await exactReaperUnlinkSync(...)awaits a non-promise. The old async helper already called the sync native, so behaviour is unchanged. Dropping theawait, or keeping a thin async wrapper, would make the intent explicit. - Note (non-blocking):
diagnostic-artifact.jsonpins only the darwin-arm64 addon hash from the author's localbuild:native. Other platforms rely on CI's native-build, which passed.
Blocking: none
Body verdict line: PR body verdict line count=0, not updated.
Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:65efa421e6da7834c78730a76fec1fbbd4918d605d60dada00333eb2d4859e85 reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;rust-fd-raii-checked;parent-pin-per-pass-checked;spy-restore-checked
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:65efa421e6da7834c78730a76fec1fbbd4918d605d60dada00333eb2d4859e85 reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;rust-fd-raii-checked;parent-pin-per-pass-checked;spy-restore-checked
|
Downstream integration evidence on exact dev d9a5178, using a freshly built isolated 0.18.7 native addon: the real |
Summary
Resolve the full-cardinality managed-remnant reaper regression without weakening deletion authority or extending its deadline.
Exact source and budget
Base dev
5197b1e9effb71345be8a1035912efaa5c46a2e8, containing external #6326 and release 0.18.7 backmerge #6327.Exact HEAD
3c91cbd7f663cdc2989acdf3ffcfe75486c92450.Owned AND actual dev production review lines 101: Rust 7+6, managed storage 55+31, generated production descriptor 1+1. Tests 362+4 and changelog fragments 4 excluded. Total 6 files +429/-42. Independently safe without persistent-owner production or task admission activation.
Verification
Genuine
bun run build:nativeon this 0.18.7 source, exit0; 115 generated exports. No old-byte reuse/manual trusted metadata stamping:8d1c6b2dcaafc1889eeb5719c74075834d0088ba903bffdf1beac71606a933252617654e11e48dd58b5b591f5d0f0b21913ad75302bc6edbf66135e5cf27fd0dFull directory file, including real alias/hardlink and native identity cases, passed—not just a narrowed timeout check. Two additional clean exact-head full-cardinality executions both passed the original deadline: suite wall time 16.96s and 19.59s, each 1 pass / 11 assertions. Existing registered skips remain visible. The second run has limited timing margin; no claim of arbitrary contention resilience.
Retained failed experiments: bounded setup alone 31.246s in exact QA; owned-descriptor hash alone 20.799s; synchronous aging 20.054s plus timed-out cleanup race; 1024 setup 20.207s. None were relabeled green or delivered. Full outputs and immutable heads are preserved in the durable goal ledger. This proposal adds actual parent pinning and was re-integrated/rebuilt/tested on release-current dev.
Limits and preservation
Darwin runtime only; Linux/Windows/junction execution requires fresh CI, not assumed from compilation or local results. Existing 2 package warnings/1 info and Rust nix future-incompatibility warning retained. No whole-root writer linearizability, FD revocation, owner physical reclamation from scrub, MCP/LSP policy, move authority, or final task-after-move repair claim.
Protected original HEAD
1a12e32f6bbcb5c4a469dd480b4b5dacb37dc95dwith its 44 staged files and reference #62407e5f7ef2a745bc1c3d012362f00be9a441bdeb76are untouched. Owner production/admission remains OFF; remaining deletion consumers, real retained execution and final frozen cohort/critic audit remain required. Require fresh exact code-event CI and write-access maintainer review. Leader does not merge.