Distillation

Date: Sep 17, 2026

• 🔴 (08:41) User supplied completed security review task ses_f52fef029ffe2a2CGDN98jGF2v, which returned DO-NOT-MERGE with 3 MUST-FIX findings.
• 🔴 (08:41) User stated recovery only verifies that the referenced assistant is the latest assistant and never checks whether that assistant already has time.completed, finish, or an error (packages/core/src/session/recovery.ts:147-165).
• 🔴 (08:41) Security finding 1: createLLMEventPublisher.failAssistant commits SessionEvent.Step.Failed through input.eventCommit at packages/core/src/session/runner/publish-llm-event.ts:222-239, renewing the claim without changing its "unknown" phase; the claim becomes "safe" only later at packages/core/src/session/runner/llm.ts:513-518 after being marked "unknown" at lines 434-440. A crash in that interval leaves a completed assistant with a known provider failure and an expired "unknown" execution claim.
• 🔴 (08:41) Security finding 1 impact: recovery can emit Step.Recovered, create a continuation, overwrite the settled assistant with a restart error, append recovery text, and awaken "continue", turning a terminal provider failure into another paid model/tool call without a new user request and corrupting the durable meaning of Step.Failed.
• 🔴 (08:41) Security finding 1 required fix: distinguish incomplete unknown work from an assistant already settled by Step.Failed; preferably commit the terminal event and safe execution state in one transaction, otherwise inspect the completed projection and clear/reconcile the claim without creating a continuation.
• 🔴 (08:41) Security finding 2: concurrent interrupts can bypass the cleanup barrier. Every caller independently transitions the same process-local claim to "cancelling" in packages/core/src/session/execution/local.ts:169-175, while SessionRunCoordinator.interrupt at packages/core/src/session/run-coordinator.ts:99-125 neither detects nor joins an interruption already in progress.
• 🔴 (08:41) Security finding 2 race: drain finalization removes the claim from activeClaims at packages/core/src/session/execution/local.ts:100-103; before the first caller starts settleCancellation, a second interrupt can see no local claim, finalize the coordinator entry, and start pending work at packages/core/src/session/run-coordinator.ts:109-120. The durable unique claim blocks ownership but can still let the coordinator consume a coalesced wake or explicit resume before cleanup, causing the successor to disappear until periodic redrive; cleanupExit is also propagated only after successor startup at lines 108-122.
• 🔴 (08:41) Security finding 2 required fix: concurrent interrupts must join one interruption operation, and the coordinator must not release or start a successor until the single durable cleanup reaches its authoritative result.
• 🔴 (08:41) Security finding 3: packages/core/schema.json:4-7 declares snapshot ID 3bc49670-06d1-4eba-9a45-8c8128bdcf6f with predecessor d4ff43d2-c4f9-442c-bada-a7d488b42fd2, but the immutable base snapshot is ID f14a9b18-8207-487e-a3d3-227e629ba9ad with previous ID 169a0f0f-d58f-479f-b024-fa1c7b9a09db; candidate predecessor d4ff43d2-c4f9-442c-bada-a7d488b42fd2 is absent from the frozen tree.
• 🔴 (08:41) Security finding 3 required fix: regenerate snapshot and migration artifacts directly from the stated immutable base, or include and review the missing predecessor transition, so 20260914170650_session-recovery is provably the sole schema transition from the base.
• 🔴 (08:41) Security review verified frozen patch SHA-256 a2bce01664538243cb8ba16c6ca8f0364c6f79463ad6677567044a1b3e1e74fe, SEA SHA-256 753b05afb86195246b211237ca5b11e26a7c08da863d70a711da2c93ee453c8a, and worktree HEAD base ab9408c81cc9916177641ae8e9f08624fc27b5b9; patch and worktree each had exactly 34 changed paths, all worktree blobs matched patch postimages, and reverse applicability confirmed the patch was already applied to the base.
• 🔴 (08:41) Security review verified: exact Session/execution/owner/live-lease predicates in packages/core/src/session/execution/claim.ts:27-33; normal cancellation waits for runner/finalizer settlement and transactionally publishes interruption before clearing only the exact live claim at packages/core/src/session/execution/local.ts:129-165; explicit resume records intent before acquisition at packages/core/src/session.ts:427-430; recovery-owned inputs are excluded at packages/core/src/session/input.ts:164-188; conflicting exact retries are rejected before arming intent at lines 62-67; takeover uses exact stale-owner/expiry predicates at packages/core/src/session/recovery.ts:83-99; public events expose execution identity but not owner identity, credentials, tool secrets, or extra prompt content; dependency direction remains Schema → Core → Server; generated event unions/OpenAPI consistently contain the 3 new events; recovery and ordinary redrive scans are bounded and rotate retry/fresh work.
• 🔴 (08:41) Security review retained accepted residual risks: final synchronous ownership-check-to-external-call window, process-local interruption routing, and bounded wall-clock skew. It ran no tests, builds, services, or generators; the SEA hash established supplied artifact identity, not reproducible source-to-binary provenance.
• 🟡 (08:41) Assistant invalidated frozen v20 because the resumed reviewer had inspected rejected v19 but its 3 findings still mapped to v20 code. Planned revised gates, in order: 1. make terminal Step.Failed recovery safe, 2. serialize concurrent interruption and durable cleanup, 3. restore schema snapshot ancestry to the frozen base, 4. run full package validation and isolated artifact restart smoke, 5. freeze a revised candidate and obtain fresh independent correctness and security approval. Existing v20 reviews could provide additional findings but could not approve changed source.
• 🟡 (08:41) Task state after invalidating v20: Make Step.Failed and safe claim settlement atomic was in_progress; Serialize concurrent interruption and durable cleanup, Restore schema snapshot ancestry to frozen base, Run full package validation and isolated artifact restart smoke, and Freeze revised candidate and obtain independent correctness and security approval were all pending, each with high priority.
• 🟡 (08:41) Assistant rejected blindly making Step.Failed → safe atomic because a failed Step may still have local tool fibers or unresolved durable tools. Chosen approach: recovery-aware terminal reconciliation that preserves an already-completed assistant’s known failure, settles only unresolved tools, and clears the claim without enqueueing "continue"; synthetic continuation was rejected because it would overwrite the known failure and trigger another model call.
• 🟡 (08:42) Assistant refined the terminal-failure algorithm: publish SessionEvent.Tool.Failed only for pending/running tools on an already-completed assistant; intermediate tool events renew the exact claim, and the final tool event atomically clears it. If no unresolved tools remain, clear the claim immediately. A crash midway retries only still-unresolved tools, preserving idempotency and never rewriting the original Step.Failed projection.
• 🟡 (08:43) Assistant began implementing terminal-failure reconciliation in startup recovery with the explicit constraints that it preserve the original assistant error, emit only missing tool outcomes, clear the exact claim in the final tool-event transaction, and create neither a synthetic prompt nor a continuation.
• 🟡 (08:44) packages/core/src/session/recovery.ts was modified for the terminal-failure reconciliation work; no completed regression result or validation result was reported in this segment.