Fix #34: include permission fix hints in blocked command errors - #57
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI
participant PermissionCheck as Permission<br/>Evaluator
participant SuggestionEngine as Suggestion<br/>Generator
participant Output as Error/Report<br/>Output
User->>CLI: Execute command
CLI->>PermissionCheck: evaluate_command_policy()
PermissionCheck->>PermissionCheck: Determine if command blocked
alt Command Blocked
PermissionCheck->>SuggestionEngine: suggested_wildcard_prefix_rule()
SuggestionEngine->>SuggestionEngine: Extract first 2 tokens
SuggestionEngine-->>PermissionCheck: Option<String>
PermissionCheck->>PermissionCheck: Populate suggested_rule field
end
PermissionCheck-->>CLI: CommandPolicyDecision
CLI->>Output: Format error message + hint
Output-->>User: "blocked by policy... (hint: add '...')"
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Tutti Choir check-in: I’m now owning the review loop on this PR. I’ll wait for CodeRabbit + CI to fully settle, then I’ll resolve every CodeRabbit finding with either a code fix + tests or a technical rebuttal with evidence before merge. |
|
Tutti Choir update: CI + CodeQL are fully green, and CodeRabbit produced no actionable review findings/threads to resolve on this revision. Branch protection is still blocking merge due to required-review policy, and auto-merge is disabled on this repo. |
|
Tutti Choir handoff: all checks are green and CodeRabbit has no unresolved findings, but merge is still blocked by required approval from a write reviewer. I’ve requested review from @nutt-adam so this can be merged immediately once approved. |
|
Good catch — re-running to get an explicit completion marker on this head commit.\n\n@coderabbitai review |
|
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/cli/permissions.rs (1)
200-206: Deduplicate bysuggested_rule, not only by raw command.Different blocked commands that map to the same prefix (e.g.
cargo run --aandcargo run --b) can currently produce duplicatesuggested_ruleentries inblocked. Consider tracking aseen_rulesset to keep suggest output cleaner.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/permissions.rs` around lines 200 - 206, Currently duplicates are filtered only by seen_commands (seen_commands.insert(cmd.clone())) which still allows multiple PermissionSuggestion entries with the same suggested_rule; change the logic in the loop that builds blocked (where seen_commands, blocked, PermissionSuggestion, decision, cmd, suggested_rule are used) to also track a seen_rules set (e.g., HashSet<String>) and only push a PermissionSuggestion when both the raw cmd and the computed decision.suggested_rule (use the unwrap_or_else branch that formats "{cmd} *") are not already seen; insert into seen_rules the final suggested_rule string when adding to blocked to avoid duplicate suggestions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/permissions.rs`:
- Around line 88-92: The formatting of the error-message construction around the
`format!` call is not rustfmt-compliant; update the block that builds `message`
(the `message` variable, `decision.command`, `decision.suggested_rule`, and the
`TuttiError::ConfigValidation` return) to follow rustfmt style — either run
`cargo fmt --all` or reflow the `format!`/`push_str` invocations so they are
properly indented and wrapped (for example break long `format!` into multiple
lines or use `format!` with a single concatenated expression) then re-run `cargo
fmt --check` to confirm the fix.
In `@src/permissions/mod.rs`:
- Around line 78-85: The function suggested_wildcard_prefix_rule currently has a
public signature returning Option<String>; either make it internal by removing
pub (change to fn suggested_wildcard_prefix_rule(...)) or change its public API
to follow the project contract by returning Result<String, TuttiError> (pub fn
suggested_wildcard_prefix_rule(command_line: &str) -> Result<String,
TuttiError>) — on error return an appropriate TuttiError when tokens.len() < 2
and on success return Ok(format!("{} {} *", tokens[0], tokens[1])); update
callers accordingly to handle the Result.
---
Nitpick comments:
In `@src/cli/permissions.rs`:
- Around line 200-206: Currently duplicates are filtered only by seen_commands
(seen_commands.insert(cmd.clone())) which still allows multiple
PermissionSuggestion entries with the same suggested_rule; change the logic in
the loop that builds blocked (where seen_commands, blocked,
PermissionSuggestion, decision, cmd, suggested_rule are used) to also track a
seen_rules set (e.g., HashSet<String>) and only push a PermissionSuggestion when
both the raw cmd and the computed decision.suggested_rule (use the
unwrap_or_else branch that formats "{cmd} *") are not already seen; insert into
seen_rules the final suggested_rule string when adding to blocked to avoid
duplicate suggestions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6425dcb8-3098-44cf-bdd4-01a343c00693
📒 Files selected for processing (3)
src/automation/mod.rssrc/cli/permissions.rssrc/permissions/mod.rs
|
Addressed CodeRabbit thread: made internal () to align with public API contract guidance. Pushed in 960b55c.\n\n@coderabbitai review |
|
Correction to previous comment (shell ate inline formatting):\n\nAddressed CodeRabbit thread by changing suggested_wildcard_prefix_rule from public to internal scope.\nCommit: 960b55c\n\n@coderabbitai review |
|
✅ Actions performedReview triggered.
|
|
✅ Actions performedReview triggered.
|
1 participant
Summary
evaluate_command_policybased on the first two command tokens (e.g.cargo test *)tt permissions checkstderr + returned CLI error textValidation
cargo test evaluate_command_policy_blocks_when_rule_missingcargo test evaluate_command_policy_suggests_rule_from_first_two_tokens_onlycargo test command_policy_block_closed_fails_stepcargo test suggest_reports_blocked_commands_including_nested_workflow_and_deduplicatescargo testCloses #34.
Summary by CodeRabbit