fix(tui): harden approval queue handling

[chilu18]

Summary

• process queued approval requests in FIFO order instead of reverse order
• send cancel/abort decisions for queued approval requests instead of silently dropping them on Esc
• show the built-in Read Only permission preset on non-Windows too

Validation

• cargo +1.93.0 test -p codex-tui queued_requests_are_processed_fifo -- --exact
• cargo +1.93.0 test -p codex-tui ctrl_c_aborts_and_clears_queue -- --exact
• cargo +1.93.0 test -p codex-tui approvals_popup_includes_read_only_preset -- --exact

Local Rust validation is currently blocked on this host by root filesystem capacity during codex-rs dependency compilation; see issue #79 for the environment constraint.

[chilu18]

OpenCTO Autonomous PR Review (2026-03-16T15:36:58.461Z)

Decision: approve

The PR improves the approval queue handling by processing requests in FIFO order, properly cancelling queued approvals on Esc, and adding a missing permission preset. It includes targeted tests validating these changes. Although local Rust validation is currently blocked due to environment constraints, the submitted test coverage and logical changes are sufficient for approval.

Concerns: none identified.

[greptile-apps]

Greptile Summary

This PR hardens the approval queue in the TUI by fixing three related issues: switching from a Vec (LIFO) to a VecDeque (FIFO) so requests are presented in the order they arrive, sending explicit abort/cancel decisions for every item still in the queue when the user presses Ctrl+C or Esc (instead of silently dropping them), and removing the Windows-only guard on the "Read Only" permission preset so it is available on all platforms.

Changes

• approval_overlay.rs: queue field changed from Vec<ApprovalRequest> to VecDeque<ApprovalRequest>; push → push_back, pop → pop_front throughout; new cancel_request() helper dispatches the correct abort/cancel op for each request variant; on_ctrl_c now drains the queue through cancel_request instead of calling queue.clear(); two new unit tests validate FIFO ordering and the abort-count on Ctrl+C.
• chatwidget.rs: Removed the let include_read_only = cfg!(target_os = "windows") guard and the corresponding continue that filtered out the read-only preset on non-Windows platforms.
• chatwidget/tests.rs: New approvals_popup_includes_read_only_preset integration test (not platform-gated) that asserts "Read Only" appears in the rendered popup on every OS.

Notable observation
Aborting queued Exec requests via cancel_request delegates to handle_exec_decision, which emits both an ExecApproval abort op and an InsertHistoryCell event. Commands that were queued but never presented to the user will therefore produce history entries on Ctrl+C. This is worth documenting as intentional if it is the desired audit-trail behaviour (see inline comment).

Confidence Score: 4/5

• Safe to merge; all three fixes are logically correct and covered by new tests — one minor UX side-effect worth confirming with the author.
• The FIFO ordering change is straightforward and correct (VecDeque push_back/pop_front). The on_ctrl_c abort-dispatch logic is equivalent to the old variant-matching code but now also correctly propagates cancellations to queued items. The Read Only preset change is a one-line guard removal with a matching test. The only open question is whether InsertHistoryCell should fire for every queued-but-never-shown Exec request on abort — this is a UX design decision rather than a correctness issue, hence score 4 rather than 5.
• sheri-ml/codex-rs/tui/src/bottom_pane/approval_overlay.rs — specifically the cancel_request → handle_exec_decision path and whether history cells for unseen queued commands are intentional.

Important Files Changed

Filename	Overview
sheri-ml/codex-rs/tui/src/bottom_pane/approval_overlay.rs	Core fix: replaces Vec (LIFO) with VecDeque (FIFO) for the approval queue and adds proper abort/cancel dispatch for all queued requests on Ctrl+C; also refactors the on_ctrl_c handler to use current_request instead of current_variant (both are equivalent). Logic is correct and the new tests validate both behaviors.
sheri-ml/codex-rs/tui/src/chatwidget.rs	Removes the Windows-only guard on the "read-only" permission preset so it renders on all platforms; the deleted include_read_only = cfg!(target_os = "windows") variable and the associated continue are cleanly excised without any side-effects.
sheri-ml/codex-rs/tui/src/chatwidget/tests.rs	Adds approvals_popup_includes_read_only_preset integration test (not gated to Windows) that exercises open_approvals_popup and asserts "Read Only" appears in the rendered popup; correctly validates the cross-platform preset change.

Sequence Diagram

sequenceDiagram
    participant Agent
    participant ApprovalOverlay
    participant VecDeque as Queue (VecDeque)
    participant AppEventTx

    Agent->>ApprovalOverlay: new(request) → set_current(request)
    Agent->>ApprovalOverlay: enqueue_request(req1) → push_back(req1)
    Agent->>ApprovalOverlay: enqueue_request(req2) → push_back(req2)

    Note over ApprovalOverlay,VecDeque: Normal approval flow (FIFO)
    ApprovalOverlay->>AppEventTx: handle_*_decision(current)
    ApprovalOverlay->>VecDeque: pop_front() → req1
    ApprovalOverlay->>AppEventTx: handle_*_decision(req1)
    ApprovalOverlay->>VecDeque: pop_front() → req2
    ApprovalOverlay->>AppEventTx: handle_*_decision(req2)
    ApprovalOverlay->>VecDeque: pop_front() → empty → done=true

    Note over ApprovalOverlay,VecDeque: Ctrl+C / Esc abort path (new behavior)
    ApprovalOverlay->>AppEventTx: cancel_request(current) → Abort/Cancel op
    loop drain queue
        ApprovalOverlay->>VecDeque: pop_front()
        ApprovalOverlay->>AppEventTx: cancel_request(queued) → Abort/Cancel op
    end
    ApprovalOverlay->>ApprovalOverlay: done = true

Loading

Comments Outside Diff (1)

1. sheri-ml/codex-rs/tui/src/bottom_pane/approval_overlay.rs, line 317-319 (link)

   History cells created for unseen queued requests

   cancel_request for an ApprovalRequest::Exec delegates to handle_exec_decision, which unconditionally emits an InsertHistoryCell event in addition to the ExecApproval abort op:

   fn handle_exec_decision(&self, id: &str, command: &[String], decision: ReviewDecision) {
       let cell = history_cell::new_approval_decision_cell(command.to_vec(), decision.clone());
       self.app_event_tx.send(AppEvent::InsertHistoryCell(cell)); // ← fires for every queued item
       self.app_event_tx.send(AppEvent::CodexOp(Op::ExecApproval { … }));
   }

   This means every Exec request that was sitting in the queue and never displayed to the user will produce a visible "✘ You declined codex to run …" history cell when Ctrl+C is pressed. Whether that is desirable (audit trail) or noise (commands the user never saw) is a product decision, but it is worth explicitly documenting the intent so future readers understand the behaviour is intentional. If only the backend abort signal is needed, the queued-items cancellation path could call a leaner helper that skips the history-cell insertion.

_{Last reviewed commit: 5019873}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5019873858

ℹ️ 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".

[chatgpt-codex-connector]

Restore removed include_read_only binding

Removing include_read_only here leaves open_permissions_popup with an unresolved reference later in the same function (return_to_permissions: !include_read_only), so codex-tui will fail to compile in every environment. This blocks the feature entirely until the flag is reintroduced or that action is rewritten without the variable.

Useful? React with 👍 / 👎.

[chilu18]

OpenCTO Autonomous PR Review (2026-03-17T00:42:57.742Z)

Decision: approve

The PR improves the approval queue handling by enforcing FIFO processing order and properly canceling queued requests on Esc instead of dropping them silently. It also corrects the UI display by including the Read Only permission preset on non-Windows platforms. The changes are supported by targeted tests confirming the intended behavior. Although local Rust validation is currently blocked by environment constraints, the included tests and code changes appear coherent and appropriate.

Concerns:

1. Local Rust validation is not currently possible due to root filesystem capacity issues; ensure full CI runs pass once environment constraints are resolved.

[chilu18]

OpenCTO Autonomous PR Review (2026-03-17T00:53:07.367Z)

Decision: approve

This PR improves the approval queue handling by processing queued requests in FIFO order, correctly cancels/aborts queued approvals on Esc instead of dropping them silently, and adds the Read Only permission preset on non-Windows platforms. The changes are well validated with precise tests, and the code modifications are focused and clear.

Concerns: none identified.

[chilu18]

OpenCTO Autonomous PR Review (2026-03-17T22:31:58.974Z)

Decision: approve

The PR improves the approval queue handling by ensuring FIFO processing, properly handling cancel/abort decisions, and adding support for the Read Only permission preset on non-Windows platforms. All changes are well tested with exact test cases, and the modifications are focused and relevant to the described issues.

Concerns: none identified.