Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69f91d03b6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| let request = ApprovalRequest::Exec { | ||
| id: ev.call_id, | ||
| id: ev.effective_approval_id(), |
There was a problem hiding this comment.
Keep replay-tracking key aligned with approval response ID
handle_exec_approval_now now sets ApprovalRequest::Exec.id to ev.effective_approval_id(), but pending interactive replay still indexes exec approvals by ev.call_id and later clears them using outbound Op::ExecApproval.id (codex-rs/tui/src/app/pending_interactive_replay.rs, note_event at lines 113-119 and note_outbound_op at lines 60-67). When approval_id differs from call_id (subcommand approvals), approving in the TUI no longer removes the tracked pending prompt, so should_replay_snapshot_event (lines 219-222) can replay an already-resolved approval after a thread/agent switch before TurnComplete arrives.
Useful? React with 👍 / 👎.
Why
ExecApprovalRequestEventcan carry a distinctapproval_idfor subcommand approvals, including theexecve-intercepted zsh-fork path.The session registers the pending approval callback under
approval_idwhen one is present, butChatWidgetwas stashingcall_idin the approval modal state. When the user approved the command in the TUI, the response was sent back with the wrong identifier, so the pending approval could not be matched and the approval callback would not resolve.Note
approval_idwas introduced in #12051.What changed
tui/src/chatwidget.rs,ChatWidgetnow usesExecApprovalRequestEvent::effective_approval_id()when constructingApprovalRequest::Exec.unified_execapprovals, whereapproval_idis absent and the effective id still falls back tocall_id.approval_id, the TUI now sends back the same key thatSession::request_command_approval()registered.Verification
Session::request_command_approval()registers the pending callback underapproval_id.unwrap_or(call_id).ChatWidgetnow emitsOp::ExecApprovalwith that same effective id.