Repository navigation
fix(session): close owner-aware shared reader safety boundary - #6320
Conversation
Protocol inspection needs complete strict owner target and receipt readers, but decoding their records must not enable legacy destructive consumers. Share target, pending-history and completion validation while rejecting owner claims before legacy effects, including header-patch races and missing-transcript replay. Native storage retains its exact identity fence. Lore-id: 807c2f41 Constraint: parsed owner metadata and inspection are not effect authority Constraint: owner producers and complete cleanup coordination remain off Tested: 138 passing focused tests, 8 existing skips, 565 assertions Tested: three upstream stale-tombstone and successor-race regressions Tested: coding-agent vendor Biome TypeScript and diff checks Not-tested: final protocol inspector and complete owner consumer installation Not-tested: aggregate closure; unchanged parent over-limit reaper test failed Confidence: high Scope-risk: narrow Reversibility: revert
Preserve both independent no-effects and legacy-fence tests while satisfying the repository import gate. Lore-id: 978e3fa1 Tested: exact prefix212pass8skip1111assertions; source-check failure was import ordering, repaired with Biome Confidence: high Scope-risk: bounded Reversibility: revert
Shared cleanup readers must never turn incomplete inventories or malformed owner claims into deletion permission. Record the whole-root fail-closed discovery rationale and the reader-boundary safety fix. Lore-id: 518c349b Constraint: no consumer/producer activation or metadata-as-authority Tested: Biome targeted files Not-tested: fresh exact-head runtime qualification pending Confidence: high Scope-risk: bounded Reversibility: revert
probepark
left a comment
There was a problem hiding this comment.
Review (head 22d1bac, gajae-reviewer on behalf of probepark)
CI: green. Every planned check passed at this head, including check:@gajae-code/coding-agent, the managed-gc-retirement-journal shard, Virtual integration validation and Windows dev:doctor. The approve gate returned ALLOW with no pending, failed or need-local items.
Scope: +510 / -179, 3 files. Reviewable production code is packages/coding-agent/src/session/internal/managed-session-scope.ts (+369/-178). The test (+138/-1) and the changelog fragment were excluded by ocr.
Conventions: release fragment changelog.d/task-owner-shared-reader-boundary.md present, shared CHANGELOG untouched. No generated files, no labels, no console.*, no new Worker.
Notable:
managed-session-scope.ts:3058:retiredTargetsnow turns aSyntaxErrorintodurability_failedwhere it used to returnundefined. Combined with the new rethrow intombstonePathContaining(:4356), one unparsable tombstone file now stopsreconcileManagedTombstonesand everydeleteManagedSessionCandidatein the scope, where before it was skipped. This matches the stated fail-closed intent and tombstones are published throughpublishManagedTombstone, so it is not blocking. The changelog fragment only mentions owner claims, though, so a user who hits this will get no explanation from it.assertLegacyOwnerFreeTranscript(:3088) callsreadSnapshotSyncon the whole transcript. It now runs at eachdeleteSessionVerifiedWithFencestage plus the pre-lock and post-lock target loops, so one deletion of a large legacy transcript reads it 3–5 times. This is correct, but it is a cost on the owner-free legacy path. A bounded header-only read would be enough if the locator can only appear in the header or a header_patch.
Checked and clean: parseRetiredTargetRecord (:3107) rejects every taskArtifactOwner* key except a strictly parsed taskArtifactOwnerDeletionEvidence whose sessionId matches. parseCleanupReceiptRecord only allows taskArtifactOwnerTranscriptDeleted===true when the target carries deletion evidence. cleanupCompleted and publishCleanupCompleted refuse owner targets before any effect. The pending-receipt refactor moves the old checks unchanged and adds only the owner clause. The new tests restore spies through vi.restoreAllMocks() and reset ManagedSessionScopeTestHooks.beforeVerifiedDelete in afterEach.
Blocking: none
PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:22bd1e99f8e0dc78516e33666eb16afa989e0d53c1cbfbc34f08ef7dd67ad265 reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;owner-claim-parsers-read;pending-receipt-refactor-diffed;spy-restore-checked
Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:22bd1e99f8e0dc78516e33666eb16afa989e0d53c1cbfbc34f08ef7dd67ad265 reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;owner-claim-parsers-read;pending-receipt-refactor-diffed;spy-restore-checked
Scope
Split #6240 after external readonly prerequisite #6316 installed. Base
b37ba33f1b516d4ceac200ed5c08e0254b08c130; exact head22d1bac844b086a5d6854422fdc6630bfee4fba6. Owned and actual dev production review cost 547 additions+deletions (Scope369+178); tests139 and release fragment3 excluded. No extraction/caller/generated production exclusions.Complete shared target, pending-receipt and completed-receipt readers retain strict owner evidence, contiguous attempts, original deletion proof and native outcome matching. Malformed/unsupported owner metadata cannot become absence and reconstruct a destructive cleanup target. Direct/reconcile, canonical-absence and header-patch races refuse owner effects before complete consumers install. Owner-free legacy behavior and the native exact-identity fence/error mapping remain intact. No placeholders, compatibility fallback or effect authority from decoded DTOs.
Whole-root fail-closed discovery is intentional: a malformed/unauthenticated v2 sibling prevents a partial inventory from proving live sibling absence. The explicit source comment addresses #6316's nonblocking review note. Automatic owner production and post-move task admission remain OFF. Protocol/scanner/managed/SDK/GC effects install separately; this PR does not enable them.
Exact-head verification
Separate QA checkout clean before/after at
22d1bac844b086a5d6854422fdc6630bfee4fba6:bun test packages/coding-agent/test/managed-gc-retirement-journal.test.ts packages/coding-agent/test/managed-gc-retirement-codec.test.ts packages/coding-agent/test/task-artifact-owner-transcript.test.ts packages/coding-agent/test/session-storage.test.ts packages/coding-agent/test/gc-disk-retention.test.ts packages/coding-agent/test/task-artifact-owner-retirement.test.ts packages/coding-agent/test/task-artifact-owner-access.test.ts: exit0, 212 passed / 8 existing platform skips / 0 failed / 1111 assertions, seven real files. Runner refuses nonexistent test paths.bun --cwd=packages/coding-agent run check: exit0 at this exact head; full vendor/Biome/typecheck, baseline2 warnings/1 info retained, no suppression.Genuine current-source native0.18.6 addon SHA256
ecca056f0c63eca5139c674999a86ca8f4c32926cb18b4c9804efd0d00c32030, source SHA25626555fb71debdd40e0cdb70811847765452facd4cbb4107b646df7cc1a25dcaf. Native crates tree identical to the successful ci-profile source buildb1fbdafa94f0ad68825d2d46590ba4ad81e013e0; truthful generated descriptor already externally installed through #6314. No descriptor rewrite during QA, version/digest waiver, released-artifact substitution, timeout increase or new skip.Darwin local execution, not full cumulative task-admission/Rust/API/platform proof. The separate inherited over-limit 50,001-entry reaper exact-QA20s failure remains outside these focused green claims; no performance candidate delivery or gate waiver. Prior #6316 Linux bug/old changes-request retained; its fixed exact47a approved by write-access probepark review5405416016, code-event37191477305 success, externally merged by Yeachan-Heo c2adb3d. That approval/CI does not transfer here.
Preservation and delivery
Protected original HEAD
1a12e32f6bbcb5c4a469dd480b4b5dacb37dc95d/ 44 staged files preserved; protected reference HEAD7e5f7ef2a745bc1c3d012362f00be9a441bdeb76/ sourceHashsha256:0ef698e426d34102d9be3e0c250a880059221bfa77c0ace2351bea28c97528abpreserved. Release fragment included; shared CHANGELOG untouched.Fresh exact-head code-event CI and approving write-access maintainer review required. Leader does not merge. No final cohort, critic or whole-goal completion claim.