[P1] permission prompt = locked inline radios (one approval path) - #53
Conversation
Replace the centred Tool Approval modal with SPEC §3.10 inline radios driven by production ApprovalState. lock_v2 permission-prompt scenes now use the real builder instead of synthetic radios(). Co-authored-by: Mathis <echobt@users.noreply.github.com>
Capture live MockTerminal frames for permission-prompt (40x12, 120x40) and permission-prompt-hover (120x40) so Designer can QA the inline radios. Signed designer boards under 40x12/120x40 are unchanged. Co-authored-by: Mathis <echobt@users.noreply.github.com>
Move permission-prompt 'e' and cancel-reject out of the two over-budget handlers so quality.py reports no regressions. Co-authored-by: Mathis <echobt@users.noreply.github.com>
Greptile SummaryThis change moves tool approvals into the session view and adds numbered permission choices. The new flow can approve operations without showing their complete details, labels a tool-wide persistent grant as though it applies only to the displayed command, and can leave authorization behavior out of sync with the selected permission mode. It also does not return users to Help, Questions, or subagent conversations after completing an approval. Confidence Score: 0/5Not safe to merge until approval disclosure, persistent-approval scope, and active permission-policy synchronization are corrected. The view-restoration issue is non-blocking but should be addressed to avoid interrupting user workflows. Three independently reproduced high-impact failures affect authorization disclosure, authorization scope, and enforcement of the selected permission setting. Two directly weaken authorization boundaries. Files Needing Attention: src/cortex-tui/src/interactive/builders/approval.rs, src/cortex-tui/src/runner/event_loop/modal.rs, src/cortex-tui/src/runner/event_loop/actions.rs, src/cortex-tui/src/permissions/mod.rs, and src/cortex-tui/src/app/methods.rs
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| pub fn build_permission_prompt(approval: &ApprovalState) -> InteractiveState { | ||
| let command = permission_command_line(approval); | ||
| let always = permission_always_label(&command); | ||
| let items = vec![ | ||
| InteractiveItem::new("once", PERMISSION_ONCE_LABEL) | ||
| .with_description("run this command once") | ||
| .with_shortcut('1'), | ||
| InteractiveItem::new("always", always) | ||
| .with_description("remember for this project") | ||
| .with_shortcut('2'), | ||
| InteractiveItem::new("edit", PERMISSION_EDIT_LABEL) | ||
| .with_description("edit before running") | ||
| .with_shortcut('3'), | ||
| InteractiveItem::new("no", PERMISSION_NO_LABEL) | ||
| .with_description("reject") | ||
| .with_shortcut('4'), | ||
| ]; | ||
| InteractiveState::new( | ||
| PERMISSION_PROMPT_TITLE, | ||
| items, | ||
| InteractiveAction::Custom(PERMISSION_PROMPT_ACTION.into()), | ||
| ) | ||
| .with_prompt_focus() |
There was a problem hiding this comment.
The inline approval panel shows its title and four choices, but not the pending tool arguments or diff_preview. It exposes command text only indirectly in the “always allow” choice, while collapsed shell summaries truncate commands after 50 characters. A harmful command suffix or file diff can therefore remain undisclosed when the user approves an operation.
How this was verified: The rendered approval state contains no argument or diff fields, and shell summaries truncate commands longer than 50 characters.
Artifacts
- Generated and executed script that inspects the exact approval builder and collapsed shell-summary paths, then runs focused Rust tests; it provides reproducible confirmation of the claimed behavior.
- Observed output from the executed validation script, including cited source lines, passing assertions, and passing focused Rust tests; it confirms context omission and 50-character shell-command truncation.
| "always" => { | ||
| let _ = self.handle_action(KeyAction::ApproveAlways).await; | ||
| } |
There was a problem hiding this comment.
Option 2 says it will always allow the displayed command snippet, but this branch dispatches ApproveAlways, which stores only the tool name. Approving an npm install request therefore allows every later shell invocation to bypass permission prompts, including unrelated destructive commands. Scope the exception to the command and project named by the option, or clearly disclose the broader grant.
How this was verified: Selecting option 2 persists only the tool name, and subsequent permission decisions bypass prompts for that tool name.
Artifacts
- Runs the focused before/after permission and TUI prompt checks while recording their commands, paths, outputs, and exit codes; it provides repeatable validation.
- Shows the command-snippet label, modal option-2 dispatch at lines 972-974, and a passing runtime test that high-risk Bash requires approval before an always allowance exists; the baseline prompts.
- Shows `ApproveAlways` storing only `tool_name`, the tool-name-only bypass gate, and passing runtime tests for always-allowed bypass and the displayed npm-install snippet; the allowance is tool-wide.
| "permissions-picker" => { | ||
| self.app_state.permission_mode = match item_id.as_str() { | ||
| "ro" => crate::permissions::PermissionMode::High, | ||
| "smart" => crate::permissions::PermissionMode::Medium, | ||
| "full" => crate::permissions::PermissionMode::Low, | ||
| _ => return false, | ||
| }; | ||
| let label = match item_id.as_str() { | ||
| "ro" => "Read-only", | ||
| "smart" => "Smart", | ||
| "full" => "Full access", | ||
| _ => "Permissions", | ||
| }; | ||
| self.app_state.toasts.info(format!("Permissions: {label}")); | ||
| return false; |
There was a problem hiding this comment.
Selecting a /permissions option updates only app_state.permission_mode, while tool authorization reads permission_manager.mode. This branch never calls sync_permission_mode(). For example, switching from Full access to Read-only can leave the previous permissive policy active even though the UI reports “Permissions: Read-only,” allowing later tools to run under the wrong policy. Synchronize the permission manager before closing the picker.
| pub fn open_permission_prompt(&mut self) { | ||
| if self.view == AppView::Approval { | ||
| self.go_back(); | ||
| } | ||
| if self.view != AppView::Session { | ||
| self.set_view(AppView::Session); | ||
| } | ||
| if let Some(ref approval) = self.pending_approval { | ||
| let interactive = crate::interactive::builders::build_permission_prompt(approval); | ||
| self.enter_interactive_mode(interactive); | ||
| } |
There was a problem hiding this comment.
If an approval arrives while the user is in Help, Questions, or a subagent conversation, this code switches to Session. Approving or rejecting leaves the user there instead of returning to their prior view. This is non-blocking, but it interrupts the user’s current task and strands the prior view in navigation state.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Artifacts
- The authored shell script builds and runs a small external Rust harness against the public TUI API for Help, Questions, and subagent origins, so the transition behavior is exercised without changing application source.
- The executed pre-completion run shows each origin becomes Session when the approval prompt opens, while previous_view records the origin.
- The executed completion run shows both approve and reject leave all three origins in Session with interactive mode disabled, confirming the prior view is not restored.
2 participants
Summary
Replace the centred modal Tool Approval path with the locked inline numbered radios under the command (SPEC §3.10 / lock
permission-prompt). One production approval path only.Chrome: inky + green selection
#1F49451:1 with the Designer lock. Composer placeholder isChoose an option above;>is dim while the prompt owns focus.lock_v2permission-promptandpermission-prompt-hovernow come from productionApprovalState(request_tool_approval), not syntheticradios()stand-ins.Runtime permission-prompt PNGs are on tip
96c4067for Designer QA. Signed designer boards indocs/media/tui-lock-v2/{40x12,120x40}/were not overwritten. Tip2cc5b25is a behavior-neutral complexity split only (no chrome / PNG regen).Keep draft until Designer cli reviews shots. Do not undraft for the quality-gate fix.
Source/dependency policy (CI fix)
python scripts/readiness/quality.py --base e77d80ca52751208aa82fd907cc53a620c3c4dfd→ 0 regressions.handle_interactive_keyhandle_key_eventHelpers:
handle_non_search_char,try_permission_prompt_edit,reject_pending_approval_and_exit_interactive(all ≤ 25). Permission-prompt frames stay pixel-identical.Runtime PNGs (Designer cli)
Download from the branch (or
ghraw):docs/media/tui-lock-v2/runtime/40x12/permission-prompt.png4df3824135de4f2f42dca845c91d2c10b24ca0f188a36766df473ffc8a419d77docs/media/tui-lock-v2/runtime/120x40/permission-prompt.png1380a9d505f1a08ccdb6106be4dc5ad6d921f201a7d3354fd112f8bd2980fc4fdocs/media/tui-lock-v2/runtime/120x40/permission-prompt-hover.pngcc682e964de037b9e697df80ef2c2e0d285f4f538761ccf9194fbdd94580ba9fSampling: exact
#1F4945is present on the focused>(37 px each). Selection bar is charcoal#262626; hover row is#1A1A1A. Zero#A78BFA/ cyan. Inky#000, text#F5F5F5, dim#6B7280../scripts/render-tui-lock-v2.shhas no scene filter; only these three frames were rasterised and committed (full unique-PNG pack check not re-run).What changed
request_tool_approvalstays on Session and opensbuild_permission_prompt(&ApprovalState)— SPEC copy:1 Yes, run once2 Yes, always allow {snippet} in this project3 Edit command4 No — tell Cortex what to do insteadAppView::Approvalno longer paints the centred modal;ApprovalViewis a dead-path no-op.card_handleroverlay path is a no-op (one approval path).permissions-picker,sandbox-deny,question,plan-confirm,clear-confirm./clearnow confirms first (clear-confirm)./permissionsuses the SPEC 3-option picker.'e'→ Edit and Esc-reject live in private helpers so the quality gate does not regress vse77d80ca.Designer cli — how to recapture
Scene ids (lock v2):
permission-prompt#1F4945, dim composer>permission-prompt-hoverpermissions-pickersandbox-denyquestionplan-confirmclear-confirmCapture runtime frames (does not overwrite signed PNG packs):
./scripts/render-tui-lock-v2.sh # → docs/media/tui-lock-v2/runtime/{40x12,120x40}/Single viewport:
Compare against:
docs/media/tui-lock-v2/SPEC.md§3.10docs/media/tui-lock-v2/index.md(permission-prompt,permission-prompt-hover)docs/media/tui-lock-v2/{40x12,120x40}/permission-prompt.pngCheck: numbered radios under the command, inky chrome, selection
#1F4945, no violet, no inverted wash, placeholderChoose an option above.Test plan
cargo fmt --all -- --check./scripts/clippy.sh -p cortex-tui --libpermission_prompt*(4)lock_v2(12) including uniqueness (77 / 31)card_handler(23)builders::approval(5)views::approval(3)request_tool_approval(1)ux_contract(12) — real approval request → numbered rows at 40×12 and 120×40; keys1-4, ↑↓, Enter, Esccargo audit— not re-run in this agent; no new depse77d80ca—quality.py0 regressions / 0 policy failuresAttestation (required)
I attest that:
approve/reject/ApproveAlways. No secrets, tokens, or keyring dumps are in the change.ApprovalState; dead-path modal tests assert no"Tool Approval Required". Handler unit test covers'e'only on the permission prompt. No mock-success..envfiles are included.Risk
/clearnow requires confirm (clear-confirm) — small behavior change.QuestionPromptViewmay still be a centred path; sandbox-deny / question Custom handlers currently close only. Follow-up if Designer wants those wired to live question/sandbox APIs.Follow-ups (out of scope if they balloon)
QuestionPromptView/ sandbox-deny to the same inline radios instead of close-only Custom handlers.