feat(automation): add rollout ergonomics - #21
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (16)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (4)src/**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,jsx,ts,tsx,md,txt,json}📄 CodeRabbit inference engine (Custom checks)
Files:
docs/{operations,workflows}/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/**📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔍 Remote MCP Firecrawl, GitHub CopilotPerfect. Now I have enough information to provide a comprehensive review. Let me compile my findings: Summary of Gathered ContextBased on my investigation of the pull request and related code, I found important information regarding security practices, CLI patterns, and implementation details: 1. SQL Injection Prevention & Parameter Handling,The PR's 2. Dry-Run Pattern Best PracticesAccording to sources on CLI best practices, a critical safety practice for destructive operations is to support a --dry-run flag that simulates the operation without making changes. The PR implements this pattern well— 3. CLI Argument ValidationThe 4. Rollout Safety DesignThe rollout feature explicitly blocks trash rules in the first wave (lines from the diff show: 5. Test CoverageThe PR includes three rollout tests demonstrating key behaviors:
For prune operations, two tests verify:
Sources Used:
🔇 Additional comments (27)
WalkthroughThis PR implements two new automation subcommands: Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Pull request overview
Adds operator-facing ergonomics around automation rollouts and local snapshot lifecycle management, extending the existing automation CLI/service/store layers with read-only rollout readiness checks and a dry-run-by-default pruning command.
Changes:
- Introduces
automation rolloutto validate rules, run readiness checks, preview candidate summaries without persisting a run, and block first-wavetrashrules. - Introduces
automation pruneto report/delete stale local automation snapshots with status filters and--executeto perform deletion. - Updates CLI output error classification, operator docs, and adds store/service test coverage for the new workflows.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/store/automation/write.rs | Adds SQLite-backed prune implementation with matched/deleted counts. |
| src/store/automation/types.rs | Introduces store-layer prune input/report types. |
| src/store/automation/tests.rs | Adds unit tests covering dry-run vs execute pruning behavior and cascade expectations. |
| src/store/automation/mod.rs | Re-exports prune types and write API. |
| src/lib.rs | Adds operation metadata for automation.rollout and automation.prune (JSON error metadata). |
| src/handlers/automation.rs | Wires new CLI subcommands into automation service entrypoints. |
| src/cli.rs | Adds automation rollout and automation prune clap definitions and status enum. |
| src/cli_output/errors.rs | Classifies new automation validation errors for consistent exit codes / JSON error envelopes. |
| src/cli_output/tests.rs | Adds regression test for prune validation error mapping. |
| src/automation/service.rs | Implements rollout readiness/preview flow and prune orchestration (cutoff calculation, status normalization). |
| src/automation/output.rs | Adds plain/JSON output rendering for rollout and prune reports. |
| src/automation/model.rs | Adds request/report models for rollout and prune (including rollout candidate summary). |
| src/automation/mod.rs | Exposes new automation APIs/constants. |
| README.md | Documents example usage for new commands. |
| docs/operations/verification-and-hardening.md | Updates hardening/verification operator flow to include rollout and prune. |
| docs/operations/automation-rules-and-bulk-actions.md | Adds rollout + prune docs, output model notes, and updated safety/ops sequence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7d8fa4a9e
ℹ️ 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 command_plan = rollout_command_plan(&selected_rule_ids, request.limit); |
There was a problem hiding this comment.
Suppress runnable plans while rollout has blockers
When rollout finds blockers, such as a selected trash rule, this still builds command_plan from the blocked selected_rule_ids, so the JSON/plain output contains runnable automation run ... and automation apply ... commands for the very rule the rollout says is unsafe for the first wave. Because the docs present these as exact follow-up commands, an operator copying the plan can bypass the rollout block; omit the run/apply plan or generate it only when blockers is empty.
Useful? React with 👍 / 👎.
Summary
automation rolloutfor read-only readiness, rules validation, candidate preview, command plan output, and first-wavetrashblockingautomation prunefor stale local automation snapshot cleanup with retention/status filters and--executeVerification
rtk test cargo test rollout -- --nocapturertk test cargo test prune_automation_runs -- --nocapturertk test cargo test automation_prune_validation -- --nocapturertk err cargo fmt --checkrtk err cargo clippy --all-targets --all-features -- -D warningsrtk test cargo testrtk err cargo run -- automation rollout --jsonrtk err cargo run -- automation rolloutrtk err cargo run -- automation prune --older-than-days 30 --jsonrtk err cargo run -- automation prune --older-than-days 30rtk test bash -lc 'set +e; cargo run --quiet -- automation prune --older-than-days 0 --json >/tmp/mailroom-prune-invalid.json; status=$?; set -e; [ "$status" -eq 2 ]; jq -e ".success == false and .error.code == \"validation_failed\" and .error.operation == \"automation.prune\"" /tmp/mailroom-prune-invalid.json >/dev/null'rtk err cargo run -- paths --jsonrtk err cargo run -- doctor --json