fix(session): restore safe deletion retries and historical GC

[snowykr]

What

Fix four runtime-reproduced session cleanup regressions from #6316/#6335/#6339/#6341, without reactivating the reverted #6349 owner producer:

1. Owner-free legacy-only SDK deletion: do not require or initialize an unrelated v2 owner scope for either initial deletion or durable restart reconciliation.
2. False artifact completion: carry an explicit verified artifactsRemoved fact in storage owner-pending results. A pre-artifact refusal preserves the original artifact tree/absence authority and the helper's prepared journal instead of falsely advancing completion.
3. Sticky initial writer capture refusal: retry only the diagnostic-only, evidence-less, demonstrably pre-effect writer state. Reauthenticate the original transcript and retain original artifact authority; already-captured owner evidence remains immutable.
4. Historical workspace GC: authenticate the stored namespace for an empty journal inventory when the old workspace is gone. Any retirement journal still requires the existing full workspace/owner authority and fails closed otherwise.

Also correct the affected receipt producers to omit absent owner fields in memory as well as JSON, so same-Broker retries obey the same strict decoder as restart retries. No decoder relaxation or corrupt-receipt migration.

Why / bug adjudication

All four original failures reproduced again on fresh dev 380f4aaabb28aa853fd4087849ee0a27fd8a19d0 in a dedicated worktree before implementation. They are not intentional owner authentication refusals: they impose unrelated owner requirements on owner-free paths or record/replay facts that never occurred.

The regression/safety tests were run with only these product fixes reversed: 19 scenarios, 15 failed / 4 passed. The fixes were then reapplied. Additional same-Broker post-artifact guard cases cover both owned and legacy-only sessions.

Safety boundaries

• Preserve exact transcript hash/inode/link/parent and original artifact tree/absence authority.
• Do not recapture owner evidence after effects, adopt reappeared artifacts, swallow unknown owner fields, bypass sibling authentication, or synthesize native success.
• Persist new capture evidence and actual owner dispositions before subsequent effects.
• Retain public cleanup_pending for genuinely scrubbed but retained native namespaces.
• Empty historical discovery verifies canonical binding bytes, path/digest/platform, scope/protocol identities, configured-root security and identity before and after verification; it creates no writer/recovery authority and does not skip sibling inventories.
• Windows/queue findings that were only statically identified in the retrospective audit are outside this bounded PR.

Testing

Final verification on the submitted source, after refreshing the local native addon:

• 216 passed, 1 platform skip, 0 failed across 14 relevant files: SDK owner deletion, owner storage, managed cleanup API/coordinator, GC journal/retirement/retention/e2e, owner wire validation, SDK directory, resident lifecycle, session-id, broker lifecycle cleanup, and ACP delete wire contracts.
• New regressions use real managed staging/replacement subprocesses, same-Broker and restart retries, byte-identical transcript replacement, canonical artifact reappearance, actual preflight refusal, deleted-workspace dry-run/prune, malformed bindings/protocol symlinks, and genuine owner journals.
• External configured-root tests use genuine native verification and an actual parent replacement preserving the original sessions subtree. The POSIX chmod test explicitly skips Windows because it is not an ACL test; the replacement test preserves platform-specific fail-closed errors.
• bun --cwd=packages/coding-agent run check passed (vendor verification, Biome, and package TypeScript check).
• Changed-file Biome check and git diff --check passed.
• bun run build:native passed. Its generated local diagnostic digest is not included in this PR.

Existing package diagnostics remain visible and unchanged: three warnings (type-only import in state-writer, agent-session file size, unused diagnostic-matrix variable) and one String.raw info diagnostic. Native build retains the existing nix future-compatibility warnings.

Verification limits

A broader process-heavy SDK e2e/directory union timed out at 180 seconds, and the complete session-directory file timed out at 90 seconds without a complete verdict. Neither is reported as passing or attributed to these fixes. Full repository aggregate checks/test suites and Windows runtime execution were not completed. Hosted CI and maintainer review remain required.

Critic review

Two independent bounded critic lanes reviewed the actual patch and both finished OKAY after corrections. Their blocking findings led to additional same-process receipt tests and configured-root security/replacement tests. These are advisory code reviews, not maintainer approval.

Risk classification

• low-risk
• regression-risk
• high-risk — authenticated deletion authority and durable recovery state; bounded changes, explicit negative tests, no owner-producer activation

Approval

Target is dev. One approving GitHub review from a write-access maintainer is required on the current head; no merge is requested or performed by this change.

• Target branch is dev
• Full root bun run check passes (not claimed; targeted package gate passed)
• Tested locally as specified above
• Changelog fragment added under packages/coding-agent/changelog.d/
• Required maintainer approval matches the exact current PR head
• Risk classification matches the review path

[snowykr]

Advisory critic review receipt

Reviewed submitted source head: 532e4746a84a9fa0b45a5e6018f4e82bbde674fd (base 380f4aaabb28aa853fd4087849ee0a27fd8a19d0). The three commits preserve the reviewed product/test content; generated native diagnostics are excluded from the PR.

Two independent critic slices finished OKAY after resolving their blocking findings:

• SDK deletion/storage: explicit pre/post-artifact completion, conditional owner authority for legacy-only deletion, bounded initial writer recapture, immutable evidence and actual native dispositions. Review found that absent owner fields were own undefined properties in in-memory receipts, poisoning same-Broker recovery while JSON restart masked it. All affected owner receipt producers now omit absent claims; regression tests cover held/released writers, same-Broker/restart alias recovery, and post-artifact canonical reappearance for both owned and legacy-only sessions. Strict decoding was not relaxed.
• GC discovery/coordination: authenticated journal-free historical scopes and prepared-journal preservation. Review found the external configured-parent security/identity gap. The helper now verifies and revalidates that root without repair; tests cover valid external roots, actual POSIX permission failure, and an actual parent replacement preserving the sessions subtree after genuine native verification. Any owner-retirement journal still requires full workspace authority.

Parent verification: 216 passed / 1 platform skip / 0 failed across 14 relevant test files, refreshed native build, coding-agent package check, changed-file Biome and patch whitespace checks. Broader process-heavy runs timed out and Windows runtime was not exercised, as documented in the PR. Existing warnings were neither suppressed nor altered.

This is an advisory agent code review, not an approving maintainer GitHub review. A write-access maintainer must approve the exact current head before merge; no merge was performed.

[snowykr]

Affected-path CI correction — head fb3db234cefa1572b4315b3195cdb8cd74efd4d7

The failed plan job stopped before test/planner execution at Verify PR head contains exact base: its immutable event base was bddbf37ede6264bf7722ba04b3281f95cbacc774, which was not an ancestor of the initially submitted head. I reproduced the same failing git merge-base --is-ancestor locally.

Integrated the exact latest dev base with a merge commit, without force-pushing, rewriting the reviewed fixes, or modifying any CI guard/workflow. The nine PR product/test/changelog files are byte-identical to their previously reviewed versions. The exact-base ancestor and whitespace checks now pass; GitHub confirms the new head and has started a fresh run.

Local CI-equivalent validation against this exact head/base:

• Canonical PR affected plan generated and successfully validated against its source SHA and SHA-256 digest; exact nine changed paths retained.
• Released changelog history guard and changed-path relevance passed.
• All 11 selected affected test shards ran through ci-dev-affected.ts --task: 376 passed, 9 existing platform skips, 0 failed. This includes the complete lifecycle e2e shard: 152 passed / 1 skip, completed in 84.64 seconds, resolving the earlier incomplete local e2e evidence.
• Selected package check, CLI version/help/native-worker smoke, and compiled coding-agent binary build all passed through the same task runner.
• Current darwin-arm64 native producer/build and native artifact provenance verification passed. The Linux baseline/modern producer and Windows-specific execution are hosted-runner checks, not claimed to have run on macOS.
• Existing diagnostics remain visible. Generated build output, native digest and local canonical-plan/evidence files are not committed.

The earlier timeout limitations do not apply to the now-completed affected lifecycle e2e shard. Full unrelated repository tests, the separate entire session-directory file, and Windows execution are still not claimed as local passes. Hosted CI is in progress; this comment does not assert its aggregate result or maintainer approval.

[probepark]

Code review (CI still running on fb3db23; approval decision follows once CI finishes).

Review (head fb3db23, gajae-reviewer on behalf of probepark)

CI: pending — 19 checks pass (incl. check:@gajae-code/coding-agent, native-build, gjc-state-gates, 6 targeted test shards); still running: sdk-broker-lifecycle-e2e, sdk-broker-restart, task-artifact-owner-storage, notifications-live-stream, session-manager-resident-cache, ts-build:coding-agent, Windows dev:doctor. No failures so far.
Scope: +602 / -62, 9 files — reviewable source 4 files (+224/-55): src/sdk/broker/lifecycle.ts, src/session/internal/managed-session-scope.ts, src/session/internal/managed-task-owner-cleanup.ts, src/session/session-storage.ts; tests 4 files (+372/-7); changelog fragment.
Conventions: changelog fragment packages/coding-agent/changelog.d/verified-session-cleanup-regressions.md present; no generated files; no released CHANGELOG section touched; no console.* in coding-agent; no mock.module; new vi.spyOn uses are restored (afterEach(vi.restoreAllMocks) in gc-task-artifact-owner-retirement.test.ts:53, finally { …mockRestore() } in sdk-task-artifact-owner-deletion.test.ts); labels none.

Checked:

• lifecycle.ts:6416-6418 — owner-free replay now returns before deriving owner scope/context, so a legacy-only receipt with zero taskArtifactOwner* fields no longer needs a v2 scope. Fresh path (:6552-6568) only builds owner scope when the transcript has an owner locator; a locator parse error becomes a capture error instead of an authority failure. inspectProtocol becomes optional and is only consumed behind owner-evidence checks (session-storage.ts:2612).
• lifecycle.ts:6442-6494 — the writer_not_quiescent retry is gated on: phase artifacts, artifactsRemoved !== true, the only owner field being taskArtifactOwnerCleanupError, no detached/retained path, no planned path or .removing sibling present (non-ENOENT lstat errors count as occupied, so it fails closed), and an unchanged transcript parent dev/ino. captureTaskArtifactOwnerEvidence (:1083-1115) re-checks the original transcript identity and sha256 before capture. An empty capture result fails closed.
• session-storage.ts:2701-2702 — the pre-artifact sibling refusal now reports artifactsRemoved === true instead of an implied completion. lifecycle.ts:8454-8455 sends !artifactsRemoved to publishTaskArtifactOwnerPending, which keeps artifactsAbsentAtAuthorization and the artifact tree authority. managed-task-owner-cleanup.ts:362 stops GC from publishing artifacts_removed for the same case.
• lifecycle.ts:7917-7945 — cleanupWithFreshOwnerPayload deletes every owner key and then re-adds only the truthy ones, so same-Broker receipts in memory match the strict decoder used on restart. The owner-pending receipt (:7952), the fresh-retirement receipt (:8027) and the artifact-completion receipt (:8339) all use it, which replaces the explicit undefined fields.
• managed-session-scope.ts:3003-3083, :3130-3144 — the cwd_missing historical scope checks the platform, the digest-derived directory name, the configured-root native security and dev/ino before and after, and the binding bytes and receipts dir identity read twice. It only continues when no retirement journal exists. When a journal exists it falls through to the existing managed_gc_scope_authority_mismatch throw.

Notes (non-blocking):

• lifecycle.ts:8374-8378 — transcriptPhaseCleanup used to clear taskArtifactOwnerTranscriptDeleted explicitly. It now depends on the local ownerTranscriptDeleted, which is only reset when the outcome is completed (:8017). The replay path at :8225 probably makes this unreachable with true. Consider adding one assertion in the payload_retired same-Broker case so the receipt shape is pinned.
• The PR body says the SDK e2e/directory union timed out locally. The sdk-broker-lifecycle-e2e and sdk-broker-restart shards are still running here, and they cover exactly that. The approval decision waits for them.

Blocking: none found in code review (step 3). Approval is held only because CI is still running.

[snowykr]

Hosted CI verification complete

Exact current head: fb3db234cefa1572b4315b3195cdb8cd74efd4d7.

Dev CI run 37290885011 now passes:

• Affected path validation / plan — success (the originally failing exact-base gate).
• All selected affected task shards, including full lifecycle e2e, package check, CLI smoke, native build and compiled TS/binary build — success.
• Affected path validation / evidence producer and protected Affected path validation aggregate — success.
• Windows dev:doctor + session-path regression — success on the hosted Windows runner.
• gjc-state-gates aggregate and Virtual integration validation — success.

Local evidence remains 376 passed / 9 existing platform skips / 0 failed across all 11 selected affected test shards, plus package/smoke/build/plan/provenance guards. The initial exact-base failure was repaired by including the immutable dev base, not by relaxing validation or changing product code.

All current checks are completed with success or intentional not-applicable skips. This supersedes the prior in-progress CI note. No maintainer approval or merge is claimed.

[probepark]

Review (head fb3db23, gajae-reviewer on behalf of probepark)

CI finished; no blocking findings. The code review for this exact head is #6357 (review).

CI: green. The approve gate (gajae-approve-gate.py) returned ALLOW on fb3db23 with 0 pending, 0 failed and 0 need-local, and check:@gajae-code/coding-agent is covered. The sdk-broker-lifecycle-e2e / sdk-broker-restart shards that were still running at the code review (the PR body says they timed out locally) have now passed in Dev CI run 37290885011.
Scope: +602 / -62, 9 files. Reviewable source is 4 files (+224/-55) under packages/coding-agent/src/{sdk/broker,session}; the rest is tests and a changelog fragment.
Conventions: changelog fragment present, no generated files, no released CHANGELOG section touched, labels none.
Blocking: none.
Note (non-blocking, carried from the code review): pin the taskArtifactOwnerTranscriptDeleted receipt shape in the payload_retired same-Broker case (lifecycle.ts:8374-8378).

PR body verdict line count=0, not updated. Suggested verdict line: gajae.pr-review-verdict.v1 merge-approved sha256:e618a0fba58cbfb27ea17ad34d6c6266bf429b580fbd97dbc3f5a610a50df2f3 reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;code-review-5412782094-blocking0

Verdict: gajae.pr-review-verdict.v1 merge-approved sha256:e618a0fba58cbfb27ea17ad34d6c6266bf429b580fbd97dbc3f5a610a50df2f3 reviewer:human reviewer-id:probepark evidence:ci-green;approve-gate-allow;code-review-5412782094-blocking0

[Yeachan-Heo]

Merged into dev as 076d77c3.

• probepark approved the exact head fb3db23.
• CI on that head: 28 passed, 0 failed.
• The branch was 0 commits behind dev.
• Since it touches src/sdk/** and src/session/**, I ran test/session-manager/, sdk-broker*, session-* and the task-artifact-owner suites locally, with and without it, on the same host:
  • with this PR: 1791 passed / 14 failed
  • unmodified dev: 1771 passed / 14 failed
  • it fixed binds session.delete to the requested session header and configured storage root
  • the only failure without a match on dev, delayed lifecycle host cleanup does not unregister a file-only same-generation successor, passed 3/3 run alone, so it's intermittent
• typescript-edit-benchmark: 15 passed, 0 failed.

—
[repo owner's gaebal-gajae (clawdbot) 🦞]