Tool approvals dangle silently and channel prompts stay live after a session restart

[Aaronontheweb]

Summary

When a tool-approval prompt is outstanding and the session actor restarts (redeploy/crash) before the user responds, the approval is left half-finished: the approver is never told the prompt survived, and on the channel side the Approve/Deny buttons stay live-looking but dead. Root cause is an incomplete implementation from #1219 — the spec-required explicit approval-turn state machine was added but is write-only (never consulted), so the recovered-pending lifecycle was never wired up.

Symptoms

• Restart with a pending approval: the session silently stays pending; the approver gets no notification that the prompt is still valid/clickable.
• Abandonment (a new user message supersedes the prompt): the channel buttons are never disabled — ToolBatchAbandoned is journal-internal, and the PromptExpired nack explicitly skips the channel redraw, so a dead prompt stays clickable. A user can click it repeatedly and just get "expired" text each time while the buttons remain.
• No expiry: approvalTimeout = Timeout.InfiniteTimeSpan; cleanup is purely lazy (only when a new message or a click happens to trigger it).

Root cause

PR #1219 ("persist approval turn context") correctly persisted the turn context and the parked command (so a post-restart redrive is faithful), but the intended approval-turn state machine (NoActive / Running / WaitingForApprovals / RecoveredWaitingForApprovals / Redriving / Abandoning) was written but never read — decisions run off the scattered _pendingToolInteractions / _resolvedToolApprovals dictionaries instead. The write-only ApprovalTurnState hierarchy is still present in src/Netclaw.Actors/Sessions/ToolApprovalState.cs (the standalone removal PR #1508 was closed so the cleanup folds into this larger fix — the dead code is the fossil this issue tracks finishing).

Concretely, on recovery both abandon paths gate on _pendingToolInteractions.Count == 0 (AbandonResolvedToolBatchAfterRecovery, HasInterruptedToolBatchAfterRecovery in LlmSessionActor.cs), so a recovered pending approval falls through and is never announced or cleaned up.

Spec gap

openspec/specs/session-state-machine/spec.md ("Approval turn state is explicit") requires handling to consult explicit state "instead of inferring the turn solely from … scattered pending dictionaries" — the implementation does the opposite. The archived change openspec/changes/archive/2026-06-04-redesign-session-approval-state-machine/tasks.md marks items 2.1–2.4 [x] despite the state never being consulted.

Proposed direction (decisions from design discussion)

• Rebuild the approval-turn state as a load-bearing ApprovalTurnCoordinator that owns the dictionaries — one source of truth that handling consults, so it can't drift into a write-only breadcrumb again.
• Policy: keep-clickable, no timeout — approvals keep their infinite lifetime and stay clickable across restarts.
• On recovery with a pending approval: emit a new lifecycle output ("survived restart, still valid"); keep the prompt clickable.
• On abandon/expiry: emit a lifecycle output so all three channels (Slack/Discord/Mattermost) disable the dead prompt's buttons.
• Correct the synced specs via a new OpenSpec change (complete-tool-approval-prompt-lifecycle) and fix the falsely-checked tasks.

A detailed implementation plan (coordinator design, the ~40 dictionary-inference sites to migrate, the new ToolInteractionUpdate output, per-channel handlers, spec deltas, and a full test matrix) is in the first comment below.

Verified fine (not part of the fix)

• The in-flight WaitForApprovalAsync is cleanly cancelled on every actor-stop path (direct via toolExecutionCt; sub-agent bridge transitively via cancellation registration + child supervision) — no leak, so only a regression test is warranted.
• The command + trust context are already persisted (ToolBatchStarted.AssistantMessage + ToolApprovalRequested), so post-restart redrive is faithful — no data-recovery work needed.

References

• fix(sessions): persist approval turn context #1219 (introduced the persistence + the write-only state)
• Closed refactor(sessions): extract CurrentTurnScope + drop dead _approvalTurnState (superseded by #1508) #1502 (CurrentTurnScope attempt, dropped), refactor(sessions): remove dead _approvalTurnState breadcrumb #1508 (dead _approvalTurnState removal, folded here)
• openspec/specs/{session-state-machine,session-resume,tool-approval-gates}/spec.md

[Aaronontheweb]

Implementation plan (design detail)

Captured from the planning session; the "Step 0" housekeeping step is already done (PR #1508 closed). Policy and shape are as agreed: keep-clickable / no timeout, load-bearing coordinator, full end-to-end.

---

Finish the tool-approval prompt lifecycle (complete what #1219 left write-only)

Context

PR #1219 ("persist approval turn context") set out to make tool approvals survive a session
restart. It correctly persisted the turn context and the full parked command (the raw tool
args live in ToolBatchStarted.AssistantMessage; the approval/authorization projection lives
in ToolApprovalRequested), so a post-restart redrive replays the exact original call under
the restored trust context. But it left the intended approval-turn state machine write-only,
and the recovered-pending lifecycle was never finished. Three confirmed gaps:

1. Spec violation (the core): openspec/specs/session-state-machine/spec.md requires the
   actor to maintain an explicit approval-turn state that "approval response handling SHALL
   consult … instead of inferring the turn solely from … scattered pending dictionaries." The
   implementation does the opposite — behavior is driven off _pendingToolInteractions /
   _resolvedToolApprovals, and the discriminated-union state was never read (it was deleted
   as dead code; the same fossil flagged this bug). The archived change's tasks.md 2.1–2.4
   are checked [x] despite the state never being consulted.
2. Recovered-pending approvals dangle silently: both recovery-abandon paths gate on
   _pendingToolInteractions.Count == 0, so a pending approval restored on restart is never
   announced. The approver is never told the prompt survived; it's only cleaned up lazily if a
   new message arrives.
3. Dead-button channel prompts: ToolBatchAbandoned is journal-internal — no output
   reaches the channel — and PromptExpired nacks explicitly skip the redraw. So Slack /
   Discord / Mattermost prompts keep live-looking dead buttons forever after a supersede
   or restart.

Decisions (set by the user):

• Policy: notify-only / keep-clickable, NO timeout. Approvals keep their infinite lifetime
  and stay clickable across restarts. The fix is making the lifecycle explicit, communicated,
  and non-stale — not bounding it.
• Rebuild the approval-turn state machine as load-bearing (consult it, don't infer).
• Full end-to-end (session + protocol + all three channels + spec + tests).

Intended outcome: finish #1219 — a load-bearing approval-turn state, a recovery notification
that keeps the prompt clickable, channel prompts that disable on abandon/expiry, the synced
specs corrected, and the falsely-checked tasks closed for real.

Step 0 — housekeeping

• Close PR refactor(sessions): remove dead _approvalTurnState breadcrumb #1508 (the standalone _approvalTurnState dead-code removal). Its deletion is
  incorporated into this work (the coordinator below replaces the dictionaries and removes
  the dead union as its prep commit).
• Branch off current dev. Track this as a new OpenSpec change — /opsx-new
complete-tool-approval-prompt-lifecycle (do NOT reopen the immutable archived change). The
  proposal notes that fix(sessions): persist approval turn context #1219 left the state write-only and that tasks.md 2.1–2.4 were marked
  done while handling stayed dictionary-driven.

Part A — Session: a load-bearing ApprovalTurnCoordinator

New ApprovalTurnCoordinator in src/Netclaw.Actors/Sessions/ToolApprovalState.cs that
privately owns _pendingToolInteractions, _resolvedToolApprovals, an explicit
ApprovalTurnPhase (NoActive / Running / WaitingForApprovals / RecoveredWaitingForApprovals
/ Redriving / Abandoning), and a _recovered provenance bit. The actor stops touching raw
dictionaries (LlmSessionActor.cs:80-81 → private readonly ApprovalTurnCoordinator _approvalTurn = new();)
and instead calls intent-named mutators (RecordRequested/RecordResolved/RecordAbandoned/
BeginRedrive/CompleteTurn/Reset/MarkRecovered) and queries (Status, HasUnresolvedBatch,
TryGetPending/Resolved, LatestPendingFor, HasSiblingPending, ShouldAbandonResolvedOnRecovery,
IsInterruptedOnRecovery). This is option (a) from design — one object, one writer — which
makes the #1219 drift structurally impossible (a union written next to the dicts is what
failed). RecoveredWaitingForApprovals is a stored bit, not derived, because today
"recovered" is only implied by the fragile phase==Ready && pending>0 cross-invariant.

Ship as two commits to de-risk the ~40 touched sites:

1. Mechanical extraction — coordinator wraps the two dicts; every call site swapped 1:1 to a
   behavior-preserving named query; the dead _approvalTurnState union removed. No logic
   change. (Decision-site table from design covers all ~40, incl. recovery gates 2465-2469,
   AbandonResolved* 3985-4016, snapshot gate 3396-3402, redrive 4069-4127, the
   Apply* handlers, FailCurrentTurn.) Preserve the load-bearing _currentTurnContext
   restore (3343-3344) and redrive bind (4118) byte-for-byte.
2. Behavior — add RecoveredWaitingForApprovals / Abandoning branching + the emits below.

Core fix (recovered-pending):

• RecoveryCompleted: after TransitionTo(Ready), _approvalTurn.MarkRecovered() when there
  are pending approvals → stamps RecoveredWaitingForApprovals.
• JoinSession gate (2465-2469): switch on Status. RecoveredWaitingForApprovals →
  emit the refresh output (below) point-to-point to the joining subscriber and DO NOT
  abandon; ResolvedAwaitingExec/Interrupted → abandon as today. Post-restart click
  already redrives correctly under the restored TurnContext (verified — HandleToolInteraction ResponseWhenIdle → RedriveToolBatchForApproval); only re-route it through BeginRedrive.

Abandon path: funnel the 5 abandon sites (supersede 2205-2236, idle-with-resolved
425-431, AbandonResolved 3996, AbandonInterrupted 4008, redrive-failed 3979-3981)
through one AbandonApprovalBatch(reason) helper that sets Abandoning, persists
ToolBatchAbandoned + heals the transcript as today, and emits the cancel output keyed by
the abandoned call ids (captured before the reset).

Wire-compat: none. The coordinator is transient (rebuilt by replaying existing
ToolApprovalRequested/Resolved/ToolBatchAbandoned); no event/snapshot shape changes;
SaveSnapshotIfSafe still excludes approval state. Add a debug invariant in TransitionTo
asserting legal ApprovalTurnPhase↔SessionPhase pairings (fail loudly on drift).

Part B — Protocol: one new output

Add to src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs (single type + status enum —
both moments share the CallId correlation, delivery semantics, and channel handler shape):

enum ToolInteractionLifecycle { RefreshedAfterRestart, Abandoned, Expired }
record ToolInteractionUpdate : SessionOutput
{ IReadOnlyList<ToolCallId> CallIds; ToolInteractionLifecycle Status; string? Reason; }

Emit with OutputFilter.None (must bypass the narrow per-channel filters, like
ToolInteractionRequest). No persisted/proto surface (INoSerializationVerificationNeeded).
The only wire surface is SignalR/TUI: add SessionOutputTypes.ToolInteractionUpdate + DTO
fields + ToDto/FromDto in SessionOutputDtoMapper.cs/SessionOutputDto.cs (additive;
existing Unknown fallback keeps it crash-safe). Payload carries CallIds only — the channel
already journaled ToolName/DisplayText per CallId in PendingApprovalPromptTracked.

Part C — Channels: disable / refresh the prompt

All three binding actors share the shape (a SessionOutput switch + a
TryResolveApprovalPromptAsync that edits the message). For each
(SlackThreadBindingActor.cs:~1040/1558, DiscordSessionBindingActor.cs:~1128/974,
MattermostSessionBindingActor.cs:~1106/953):

• Add case ToolInteractionUpdate → a handler that, per CallId, locates the tracked prompt
  (reuse PendingApprovalPromptTracked).
• Generalize TryResolveApprovalPromptAsync with a kind param: Abandoned/Expired → edit
  the original message and REMOVE the action controls (+ a superseded/expired banner) and
  persist PendingApprovalPromptCleared; RefreshedAfterRestart → edit in place, KEEP the
  buttons, keep tracking; if no tracked message id replayed, fall back to re-posting a
  fresh interactive prompt (never a silent no-op, never a duplicate when the id is known).
• Extract the shared correlate-iterate-clear loop into a small helper next to
  src/Netclaw.Actors/Channels/PendingApprovalPromptState.cs; keep the transport edit per-channel.

Part D — OpenSpec deltas (via the change from Step 0)

Delta-update three synced specs (drafted scenarios available from design):

• session-state-machine — approval-turn state is now CONSULTED; drives the refresh/cancel
  emits.
• session-resume — "Recovered pending approval notifies the approver and stays clickable"
  (+ re-notify on every reactivation).
• tool-approval-gates — "Channel approval prompts reflect abandonment and expiry."
  Then /opsx-sync → /opsx-verify → /opsx-archive at the end.

Part E — ApprovalChannel

No code change: design verified the in-flight WaitForApprovalAsync is cleanly cancelled on
every stop path (direct via toolExecutionCt; sub-agent bridge transitively via the
cancellation registration + child supervision). Add only a regression test (Part F-f) and
a code comment documenting the cancellation chain, since a future refactor could silently
re-introduce a leak.

Part F — Test strategy (how we test it)

No-timeout means paths are event-driven → mostly deterministic without a clock. Follow the
ApprovalRehydrationTests pattern (IdleTimeout = Zero, simulate passivation via Sys.Stop
then respawn). Use Akka TestScheduler (HOCON akka.scheduler.implementation = TestScheduler

• AdvanceScheduler, per GatewayLifecycleContractTests) only for the channel passivation-
  deferral assertion.

Session (ApprovalRehydrationTests.cs / new ApprovalPromptLifecycleTests.cs):

• (a) recover-with-pending → ToolInteractionUpdate{RefreshedAfterRestart} to the joiner →
  click redrives the parked tool + turn completes.
• (b) recover → passivate → stop → respawn ×2 → refresh re-emitted each time (idempotent, no
  orphan).
• (c) new-message supersede → {Abandoned} emitted, parked tool does NOT re-run, history healed.
• (c2) unrecoverable context → {Expired} emitted (augments the expired TextOutput).
• (e) sub-agent parent-mediated approval across a parent restart → stale click handled as
  Expired, parent re-requests (documents the PersistApprovalState==false divergence).
• (f) Sys.Stop with an outstanding approval wait → PostStop cancels it (no hung task / no
  DeadLetter pileup).

Channels (Channels/Contracts/SessionBindingContractTests.cs, runs for all three):

• (d) Abandoned/Expired update → one edit with controls REMOVED + cleared-prompt journal.
• (d2) Refresh → edit with controls RETAINED, still tracked.
• (d3) cold-spawn refresh edits via persisted PromptId with buttons retained.
• (d4) refresh with empty binding journal → re-posts a fresh prompt.
• (d5, TestScheduler) pending prompt defers passivation.
  Harness add: let RecordingSessionPipeline push an out-of-band ToolInteractionUpdate; assert
  buttons present/absent per channel.

Native smoke (scripts/smoke/run-smoke.sh): one Slack scenario for end-to-end "abandon →
buttons visibly disabled" (the transport accepting the edit is the one thing fakes can't
prove). Secondary to the deterministic tests.

Verification (Definition of Done)

• dotnet build; full Netclaw.Actors.Tests (Sessions + channel contract matrix) green;
  20× stress the recovery tests for determinism.
• dotnet slopwatch analyze (no new violations); ./scripts/Add-FileHeaders.ps1 -Verify.
• /opsx-verify passes; specs synced; archived-change note added; tasks closed honestly.
• System Skills Sync: approval-prompt lifecycle is operator-visible behavior — update
  feeds/skills/.system/files/netclaw-operations/SKILL.md (approval/tool guidance) + bump
  version. Eval suite: run ./evals/run-evals.sh if tool-approval guidance changed.
• One Slack native smoke scenario for the disable-on-abandon flow.

Critical files

• src/Netclaw.Actors/Sessions/ToolApprovalState.cs (new ApprovalTurnCoordinator)
• src/Netclaw.Actors/Sessions/LlmSessionActor.cs (consume coordinator; emit on recovery-join /
  supersede / abandon / expired)
• src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs (ToolInteractionUpdate)
• src/Netclaw.Actors/Protocol/SessionOutputDtoMapper.cs + SessionOutputDto.cs (SignalR/TUI)
• src/Netclaw.Channels.{Slack,Discord,Mattermost}/*SessionBindingActor.cs + *ApprovalBlockBuilder
• src/Netclaw.Actors/Channels/PendingApprovalPromptState.cs (shared correlation helper)
• Tests: src/Netclaw.Actors.Tests/Sessions/ApprovalRehydrationTests.cs,
  src/Netclaw.Actors.Tests/Channels/Contracts/SessionBindingContractTests.cs,
  ParentSessionApprovalBridgeTests.cs
• OpenSpec: new complete-tool-approval-prompt-lifecycle change → deltas for
  session-state-machine, session-resume, tool-approval-gates

Phasing (suggested)

1. OpenSpec change (proposal/design/tasks) via /opsx-new + /opsx-ff.
2. PR 1 — Part A (coordinator, 2 commits) + Part B (output) + session tests (a–f).
3. PR 2 — Part C (channel handlers) + DTO mapping + channel contract tests (d*) + smoke.
4. /opsx-sync + /opsx-verify + /opsx-archive; skill/eval updates.