feat(cli): Add interactive review for prune plans#3421
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces an interactive review capability for prune plans, enabling users to safely manage the deletion of legacy policy objects following a migration. It establishes a robust planning mechanism to classify objects based on their usage and migration status, ensuring that only safe-to-delete objects are pruned while providing clear feedback for unresolved or blocked items. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. The legacy code must fade away, To make the new path clear today. With prune and plan we check the state, And choose the objects to delete. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughRefactors OpenTDF policy migration code by introducing new policy formatting utilities, adding interactive review functionality for prune plans, implementing String() methods for prune plan types, and relocating summary helper functions. The changes span formatting, review, and plan management across the namespacedpolicy package. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The diff spans multiple new files with substantial logic (interactive review with ~224 LOC), refactored string formatting utilities, and cross-file reorganization of helper functions. While test-heavy and well-structured, the interactive review logic requires careful understanding of prune plan item handling, user interaction flow, and status transitions across multiple item types. Possibly related PRs
Suggested labels
Suggested reviewers
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 docstrings
🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![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 implements a pruning system for namespaced policies, enabling the identification and removal of legacy policy objects post-migration. It introduces the PrunePlanner, which categorizes objects as deleted, blocked, or unresolved, alongside an interactive review tool for manual decision-making. The PR also includes comprehensive testing for the pruning logic and refactors several utility functions for policy formatting and object identification into shared locations. Feedback identifies several redundant nil checks in the PrunePlanner implementation that should be removed to improve code clarity.
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@otdfctl/migrations/namespacedpolicy/prune_plan.go`:
- Around line 38-42: The PruneStatusReason struct incorrectly embeds
fmt.Stringer causing an extra JSON field; remove the embedded fmt.Stringer from
the struct definition (PruneStatusReason) so its JSON shape no longer contains a
Stringer member, and if stringification is required implement a String() method
on PruneStatusReason (or provide a helper) instead of embedding the interface;
update any references expecting the embedded field to call the new String()
method or helper.
In `@otdfctl/migrations/namespacedpolicy/prune_planner.go`:
- Around line 183-186: Align nil-checking patterns between the loops over
sourceActions and planSubjectConditionSets by using the same guard form; update
the loop that populates planSubjectConditionSets to mirror the explicit check
used in planActions (i.e., check both source == nil || source.GetId() == "") so
both loops use the same nil-safe style; modify the loop that builds
planSubjectConditionSets (and any similar loops at lines 234-237) to reference
the same condition and keep the protobuf getter usage unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0637e001-5a7d-4d3b-9387-7a5866fd5067
📒 Files selected for processing (15)
otdfctl/migrations/namespacedpolicy/migration_review.gootdfctl/migrations/namespacedpolicy/migration_review_test.gootdfctl/migrations/namespacedpolicy/plan_utils.gootdfctl/migrations/namespacedpolicy/planner.gootdfctl/migrations/namespacedpolicy/policy_formatting.gootdfctl/migrations/namespacedpolicy/policy_formatting_test.gootdfctl/migrations/namespacedpolicy/prune_plan.gootdfctl/migrations/namespacedpolicy/prune_plan_test.gootdfctl/migrations/namespacedpolicy/prune_planner.gootdfctl/migrations/namespacedpolicy/prune_planner_test.gootdfctl/migrations/namespacedpolicy/prune_review.gootdfctl/migrations/namespacedpolicy/prune_review_test.gootdfctl/migrations/namespacedpolicy/resolved.gootdfctl/migrations/namespacedpolicy/retrieve.gootdfctl/migrations/namespacedpolicy/summary.go
💤 Files with no reviewable changes (2)
- otdfctl/migrations/namespacedpolicy/retrieve.go
- otdfctl/migrations/namespacedpolicy/summary.go
Benchmark results, click to expandBenchmark authorization.GetDecisions Results:
Benchmark authorization.v2.GetMultiResourceDecision Results:
Benchmark Statistics
Bulk Benchmark Results
TDF3 Benchmark Results:
|
|
🤖 I have created a release *beep* *boop* --- ## [0.32.0](opentdf/platform@otdfctl/v0.31.0...otdfctl/v0.32.0) (2026-05-19) ### Features * **cli:** Add better unit testing. ([opentdf#3378](opentdf#3378)) ([3ad33dc](opentdf@3ad33dc)) * **cli:** Add interactive review for prune plans ([opentdf#3421](opentdf#3421)) ([c11680b](opentdf@c11680b)) * **cli:** Add prune confirmation. ([opentdf#3469](opentdf#3469)) ([c6d47ec](opentdf@c6d47ec)) * **cli:** Add prune planner. ([opentdf#3411](opentdf#3411)) ([3e294e6](opentdf@3e294e6)) * **cli:** Add prune summary information ([opentdf#3456](opentdf#3456)) ([c900c53](opentdf@c900c53)) * **cli:** add sensitive flag annotation to DocFlag ([opentdf#3457](opentdf#3457)) ([98f48d2](opentdf@98f48d2)) * **cli:** Confirm and execute pruning of legacy objects ([opentdf#3458](opentdf#3458)) ([24c09dd](opentdf@24c09dd)) * **cli:** Print report on failure ([opentdf#3365](opentdf#3365)) ([05a4473](opentdf@05a4473)) * **cli:** Sort parameters. ([opentdf#3478](opentdf#3478)) ([73ad878](opentdf@73ad878)) * **policy:** Add FQN to RegisteredResourceValues ([opentdf#3446](opentdf#3446)) ([3199583](opentdf@3199583)) * **policy:** Add resource mapping group FQNs ([opentdf#3447](opentdf#3447)) ([6a0b3c6](opentdf@6a0b3c6)) ### Bug Fixes * **cli:** Prune was not classifying multi-namespaced RRs properly. ([opentdf#3488](opentdf#3488)) ([eae8645](opentdf@eae8645)) * **cli:** support json profile output ([opentdf#3448](opentdf#3448)) ([61f194c](opentdf@61f194c)) * **deps:** bump github.com/opentdf/platform/lib/identifier from 0.3.0 to 0.4.0 in /otdfctl ([opentdf#3367](opentdf#3367)) ([aa23179](opentdf@aa23179)) * **deps:** bump github.com/opentdf/platform/protocol/go from 0.27.0 to 0.28.0 in /otdfctl ([opentdf#3419](opentdf#3419)) ([c80374f](opentdf@c80374f)) * **deps:** bump github.com/opentdf/platform/sdk from 0.16.0 to 0.17.0 in /otdfctl ([opentdf#3397](opentdf#3397)) ([bb9fcd6](opentdf@bb9fcd6)) * **deps:** bump go.opentelemetry.io/otel from 1.40.0 to 1.41.0 in /otdfctl ([opentdf#3400](opentdf#3400)) ([5631c37](opentdf@5631c37)) * **deps:** bump module protocol/go to v0.30.0 throughout ([opentdf#3459](opentdf#3459)) ([8eaa502](opentdf@8eaa502)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: opentdf-automation[bot] <149537512+opentdf-automation[bot]@users.noreply.github.com> Co-authored-by: Chris Reed <87077975+c-r33d@users.noreply.github.com>
2 participants
Branch Summary
Branch: prune-plan-review
What Changed
Interactive Prune Review Behavior
The prune review flow now prompts for unresolved prune items across:
For each unresolved item, the user can:
Choosing delete now clears the prune reason.
Formatting
Added reusable formatting for policy-related prompt details:
Target refs now render as structured fields, including empty values:
id="target-1" (namespace="https://example.com")
id="" (namespace="")
Summary by CodeRabbit
Release Notes
New Features
Refactor