feat(tui): save ask permission rules from approvals#3301
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Thanks @greyfreedom for taking the time to contribute. This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered. Please read |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to persist "ask-only" permission rules directly from the TUI approval view, specifically targeting exec_shell commands. It adds a shortcut ('s') to approve and save the rule, renders a preview of the rule in the approval widget, and handles persistence via the configuration store. The feedback highlights a logical issue where session-only approvals (ApprovedForSession) would incorrectly persist rules permanently. Additionally, it recommends using the established localization system (tr(locale, MessageId::...)) for the new UI hints and status messages instead of hardcoding or manually matching locales.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if matches!( | ||
| event.decision, | ||
| ReviewDecision::Approved | ReviewDecision::ApprovedForSession | ||
| ) && !event.persistent_ask_rules.is_empty() | ||
| && !event.timed_out | ||
| { | ||
| persist_ask_rules_from_approval(app, config, &event.persistent_ask_rules); | ||
| } |
There was a problem hiding this comment.
The condition checks ReviewDecision::ApprovedForSession to decide whether to persist the ask rules. However, 'Approved for Session' is intended to be a temporary, session-only approval. Persisting rules to permissions.toml under this decision contradicts the user's choice of a session-only approval. We should only persist rules when the decision is permanently Approved.
if event.decision == ReviewDecision::Approved
&& !event.persistent_ask_rules.is_empty()
&& !event.timed_out
{
persist_ask_rules_from_approval(app, config, &event.persistent_ask_rules);
}| fn save_ask_rule_hint(locale: Locale) -> &'static str { | ||
| match locale { | ||
| Locale::ZhHans => " s 批准并保存询问规则", | ||
| _ => " s approve + save ask rule", | ||
| } | ||
| } | ||
|
|
||
| fn label_ask_rule_preview(locale: Locale) -> &'static str { | ||
| match locale { | ||
| Locale::ZhHans => "询问规则预览：", | ||
| _ => "Ask rule preview:", | ||
| } | ||
| } |
There was a problem hiding this comment.
The helper functions save_ask_rule_hint and label_ask_rule_preview use manual match locale blocks to return hardcoded translations. This bypasses the established internationalization pattern of using tr(locale, MessageId::...) seen elsewhere in this file (e.g., footer_controls and selection_hint_prefix). To maintain consistency and keep all translations centralized, these strings should be added to the localization system (e.g., MessageId).
| Ok((added, path)) if added > 0 => { | ||
| app.status_message = Some(format!( | ||
| "Saved {added} ask permission rule(s) to {}", | ||
| path.display() | ||
| )); | ||
| } | ||
| Ok((_added, path)) => { | ||
| app.status_message = Some(format!( | ||
| "Ask permission rule already saved in {}", | ||
| path.display() | ||
| )); | ||
| } | ||
| Err(err) => { | ||
| app.status_message = Some(format!("Failed to save ask permission rule: {err:#}")); | ||
| } |
There was a problem hiding this comment.
The status messages set in persist_ask_rules_from_approval are hardcoded in English. Since the TUI supports multiple locales (such as Chinese/ZhHans), these user-facing status messages should be localized using the project's established internationalization/localization patterns rather than being hardcoded.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
2 participants
Summary:
Add an ask-only approval UI action that persists the current shell approval as a
permissions.tomlask rule.Scope:
ToolAskRule::exec_shell(...)proposals forexec_shellapproval promptssshortcut to approve once and save the ask ruleConfigStore::append_ask_rulesConfig.exec_policy_engineafter saving so later turns can use the ruleNot in this slice:
Builds on:
permissions.tomlschema/loading workIssues:
Refs #1186 (partial)
Refs #2242 (partial)
Validation:
cargo fmt --allcargo fmt --all -- --checkcargo check -p codewhale-tui --bin codewhale-tuicargo test -p codewhale-tui --bin codewhale-tui ask_rule -- --nocapturegit diff --checkNote:
cargo clippy -p codewhale-tui --bin codewhale-tui --all-features -- -D warningswas attempted, but currentupstream/mainfails on pre-existing warnings outside this diff, includinguninlined_format_argsandtoo_many_arguments.