fix(sessions): persist approval turn context#1219
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Aaronontheweb
left a comment
Aaronontheweb
left a comment
There was a problem hiding this comment.
Self-review notes to make the behavior changes easier to review.
| /// stay on <see cref="MessageSource"/>; this model carries only security and | ||
| /// provenance fields that must survive approval pause and recovery. | ||
| /// </summary> | ||
| public sealed record TurnContext |
There was a problem hiding this comment.
This is the main design change: a turn now has one durable object for who asked, from where, and under what trust boundary. We stop treating live transport metadata as something we can safely reconstruct after restart.
| _currentTurnSource = cmd.Source; | ||
| _currentTrustContext = _trustContextDeriver?.Derive(cmd.Source); | ||
| BindTurnTelemetry(cmd.Source); | ||
| _currentTurnContext = TurnContext.FromMessageSource( |
There was a problem hiding this comment.
We build the turn context once when the user message is accepted. Everything later in the turn, including approval recovery, uses this same context instead of re-deriving trust fields from scattered state.
| repeated string option_keys = 15; | ||
| bool has_third_party_adopted_context = 16; | ||
| repeated string adopted_speaker_ids = 17; | ||
| TurnContextRecordProto turn_context = 18; |
There was a problem hiding this comment.
This is additive persistence only. Old journals still deserialize, but new approval requests carry the original turn context so approval after restart can resume safely.
| Directory = c.Directory | ||
| }) | ||
| .ToArray(), | ||
| TurnContext = _currentTurnContext?.ToRecord(), |
There was a problem hiding this comment.
This is where the approval prompt becomes recoverable. The approval event now stores the original turn context, not just individual copied fields that could drift or be forgotten.
|
|
||
| internal static class ToolApprovalTurnContext | ||
| { | ||
| public static TurnContext? Restore(ToolApprovalRequested evt, out string? failure) |
There was a problem hiding this comment.
Recovery is centralized here on purpose. New events use the persisted turn context; old events go through one legacy compatibility path so fallback behavior stays visible and contained.
| } | ||
|
|
||
| private bool TryRedriveToolBatchAfterApproval(string callId) | ||
| private ApprovalRedriveOutcome TryRedriveToolBatchAfterApproval(string callId) |
There was a problem hiding this comment.
Redrive now has three outcomes: started, deferred, or failed. That distinction matters because a safe wait for sibling approvals should not look the same as a failed recovery.
| _currentTrustContext = _trustContextDeriver?.Derive(synthesized); | ||
| BindTurnTelemetry(synthesized); | ||
| } | ||
| if (pending.TurnContext is not { } turnContext) |
There was a problem hiding this comment.
This replaces the old synthetic MessageSource recovery path. If the original turn context is missing, we do not run the tool under guessed authority.
| catch (ToolApprovalRequiredException approvalEx) | ||
| when (approvalChannel is not null && emitApprovalRequest is not null) | ||
| { | ||
| if (!CanRequestInteractiveApproval(source, turnContext)) |
There was a problem hiding this comment.
A tool call that requires human approval but has no interactive requester now fails immediately. Previously it could emit a prompt that nobody could validly approve.
| TurnContext? turnContext = null) | ||
| { | ||
| var audience = turnSource?.Audience | ||
| var audience = turnContext?.Audience |
There was a problem hiding this comment.
Memory recall now prefers the restored turn context. This keeps recovered approval turns from accidentally using a weaker or default memory audience after restart.
| // no safe default — defaulting a missing source to Personal would silently | ||
| // escalate the job's audience. A null source here is a programming error. | ||
| if (source is null) | ||
| // A background job inherits the submitting turn's authority context. |
There was a problem hiding this comment.
Background jobs inherit the submitting turn's authority context. There is no safe default here because guessing could run a job under the wrong audience or boundary.
1 participant
Summary
Validation
Fixes #1213