Skip to content

feat(cli): Add prune confirmation.#3469

Merged
c-r33d merged 3 commits into
mainfrom
confirm-deletion
May 13, 2026
Merged

feat(cli): Add prune confirmation.#3469
c-r33d merged 3 commits into
mainfrom
confirm-deletion

Conversation

@c-r33d
Copy link
Copy Markdown
Contributor

@c-r33d c-r33d commented May 13, 2026

Proposed Changes

  1. Ask users to confirm the deletion of a specific policy construct before executing the deletion. User is given the choice to: skip, abort, confirm.
  2. Refactor the result passed into the summary from a string to enum.

Checklist

  • I have added or updated unit tests
  • I have added or updated integration tests (if appropriate)
  • I have added or updated documentation

Testing Instructions

Summary by CodeRabbit

  • New Features

    • Interactive per-item confirmation for prune-plan deletions (confirm / skip / abort).
    • New "Skipped" status for items users skip; summaries reflect skipped counts and reasons.
    • Prune summaries now report typed results (success, failure, aborted) and distinguish executed vs. dry-run.
  • Tests

    • Added tests covering interactive confirmation flows, skipped-item handling, abort behavior, and summary rendering.

Review Change Stack

@c-r33d c-r33d requested a review from a team as a code owner May 13, 2026 12:49
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e65fa4df-bb22-4126-8b67-df05a1dbeeca

📥 Commits

Reviewing files that changed from the base of the PR and between 5a103d7 and b5b7bfc.

📒 Files selected for processing (6)
  • otdfctl/cmd/migrate/prune/namespaced_policy.go
  • otdfctl/migrations/namespacedpolicy/prune_commit_confirmation.go
  • otdfctl/migrations/namespacedpolicy/prune_commit_confirmation_test.go
  • otdfctl/migrations/namespacedpolicy/prune_plan.go
  • otdfctl/migrations/namespacedpolicy/prune_review.go
  • otdfctl/migrations/namespacedpolicy/prune_summary_test.go

📝 Walkthrough

Walkthrough

This PR adds interactive per-item prune-delete confirmation with skip/abort choices, a typed PruneSummaryResult enum (success/failure/aborted), a PruneStatusSkipped status with a skipped-by-user reason, and updates command flow and summary rendering to use the new typed API and executed flag.

Changes

Prune interactive confirmation and typed results

Layer / File(s) Summary
Schema and type definitions
otdfctl/migrations/namespacedpolicy/prune_plan.go, otdfctl/migrations/namespacedpolicy/prune_summary.go
Adds PruneStatusSkipped and PruneStatusReasonTypeSkippedByUser; introduces PruneSummaryResult type with success, failure, and aborted constants.
Prune summary rendering API refactoring
otdfctl/migrations/namespacedpolicy/prune_summary.go
Changes RenderNamespacedPolicyPruneSummary to accept executed bool and typed PruneSummaryResult; replaces untyped wrapper; propagates executed across summarizers and handles PruneStatusSkipped in output and classification.
Prune deletion confirmation implementation
otdfctl/migrations/namespacedpolicy/prune_commit_confirmation.go, otdfctl/migrations/namespacedpolicy/prune_review.go
Adds ConfirmPrunePlanDeletes and helpers to prompt per-item delete confirm/skip/abort across all plan item types; marks skipped items with skipped-by-user reason; refactors some generic helpers to non-generic forms used by the confirmation flow.
Confirmation unit tests
otdfctl/migrations/namespacedpolicy/prune_commit_confirmation_test.go
Adds tests covering confirm, skip, abort, ignored non-delete items, per-construct prompting, and decision callback behavior.
Prune command refactoring to use typed results
otdfctl/cmd/migrate/prune/namespaced_policy.go
Extracts interactive commit flow, maps aborts to PruneSummaryResultAborted, writes typed success/failure/aborted summaries, and updates summary writer to accept executed+typed result.
Execution and summary test updates
otdfctl/migrations/namespacedpolicy/prune_execute_test.go, otdfctl/migrations/namespacedpolicy/prune_summary_test.go
Extends test fixtures with PruneStatusSkipped entries and verifiers to assert nil Execution for skipped items; migrates tests to RenderNamespacedPolicyPruneSummary(plan, executed, result) and adds a test ensuring skipped deletes render as “Skipped” with correct counts and reason.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • alkalescent
  • elizabethhealy

Poem

🐰 A rabbit hops through plans with care,
Prompting whether to skip, delete, or spare—
Skipped items get a gentle reason,
Typed results mark success, fail, or treason,
Commit or abort, the summary's fair.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main feature addition: adding interactive user confirmation before deleting policy constructs during prune operations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch confirm-deletion

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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 confirmation flow for the policy prune operation, allowing users to make granular decisions on which policy constructs to delete. It also improves the internal handling of operation results by introducing a dedicated enum, ensuring more robust and maintainable reporting in the CLI summary output.

Highlights

  • Interactive Prune Confirmation: Added a confirmation step for users to review and individually confirm, skip, or abort the deletion of policy constructs during the prune process.
  • Enum-based Result Handling: Refactored the prune summary result handling from a string-based approach to a type-safe enum (PruneSummaryResult), improving consistency across the CLI output.
New Features

🧠 You can now enable Memory (public preview) 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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 policy tree is overgrown and tall, We prune the branches, one and all. But wait, confirm before you delete, To keep the data safe and neat.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions
Copy link
Copy Markdown
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 144.564322ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 68.656667ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 343.007369ms
Throughput 291.54 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 34.678159464s
Average Latency 344.679496ms
Throughput 144.18 requests/second

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces an interactive confirmation flow for pruning namespaced policies, allowing users to confirm, skip, or abort individual delete actions. It includes updates to the summary rendering logic to support a 'skipped' status and refactors the prune execution flow to handle these interactive confirmations. I have identified a high-severity issue regarding missing definitions for several interfaces and constants used in the new confirmation logic, which will cause compilation failures. Additionally, I recommend refactoring the error handling in the interactive commit flow to return errors to the caller rather than exiting directly within the helper function to improve testability and reduce code duplication.

Comment thread otdfctl/migrations/namespacedpolicy/prune_commit_confirmation.go
Comment thread otdfctl/cmd/migrate/prune/namespaced_policy.go Outdated
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@otdfctl/migrations/namespacedpolicy/prune_plan.go`:
- Line 30: Add the missing message constant corresponding to
PruneStatusReasonTypeSkippedByUser so it matches the existing pattern used for
other reasons; create a new constant (named consistently with the other message
constants, e.g., PruneStatusReasonMessageSkippedByUser) and assign it the
human-readable message for the "SkippedByUser" reason, then ensure any
switch/lookup that maps PruneStatusReasonType to message constants includes this
new symbol.
🪄 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: 112fc5ba-3aba-44de-80cf-482de83664af

📥 Commits

Reviewing files that changed from the base of the PR and between 37870f4 and 5a103d7.

📒 Files selected for processing (7)
  • otdfctl/cmd/migrate/prune/namespaced_policy.go
  • otdfctl/migrations/namespacedpolicy/prune_commit_confirmation.go
  • otdfctl/migrations/namespacedpolicy/prune_commit_confirmation_test.go
  • otdfctl/migrations/namespacedpolicy/prune_execute_test.go
  • otdfctl/migrations/namespacedpolicy/prune_plan.go
  • otdfctl/migrations/namespacedpolicy/prune_summary.go
  • otdfctl/migrations/namespacedpolicy/prune_summary_test.go

Comment thread otdfctl/migrations/namespacedpolicy/prune_plan.go
@github-actions
Copy link
Copy Markdown
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 191.912214ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 103.69831ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 425.410689ms
Throughput 235.07 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 44.783317859s
Average Latency 445.860621ms
Throughput 111.65 requests/second

Comment thread otdfctl/cmd/migrate/prune/namespaced_policy.go Outdated
Comment thread otdfctl/migrations/namespacedpolicy/prune_commit_confirmation.go Outdated
Comment thread otdfctl/migrations/namespacedpolicy/prune_commit_confirmation.go Outdated
elizabethhealy
elizabethhealy previously approved these changes May 13, 2026
Copy link
Copy Markdown
Member

@elizabethhealy elizabethhealy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! just left some nits

@c-r33d c-r33d enabled auto-merge May 13, 2026 15:49
@github-actions
Copy link
Copy Markdown
Contributor

Benchmark results, click to expand

Benchmark authorization.GetDecisions Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 187.050978ms

Benchmark authorization.v2.GetMultiResourceDecision Results:

Metric Value
Approved Decision Requests 1000
Denied Decision Requests 0
Total Time 96.074755ms

Benchmark Statistics

Name № Requests Avg Duration Min Duration Max Duration

Bulk Benchmark Results

Metric Value
Total Decrypts 100
Successful Decrypts 100
Failed Decrypts 0
Total Time 398.766725ms
Throughput 250.77 requests/second

TDF3 Benchmark Results:

Metric Value
Total Requests 5000
Successful Requests 5000
Failed Requests 0
Concurrent Requests 50
Total Time 42.010002413s
Average Latency 418.346433ms
Throughput 119.02 requests/second

@github-actions
Copy link
Copy Markdown
Contributor

⚠️ Govulncheck found vulnerabilities ⚠️

The following modules have known vulnerabilities:

  • examples
  • otdfctl
  • sdk
  • service
  • lib/fixtures
  • tests-bdd

See the workflow run for details.

@c-r33d c-r33d added this pull request to the merge queue May 13, 2026
Merged via the queue into main with commit c6d47ec May 13, 2026
40 checks passed
@c-r33d c-r33d deleted the confirm-deletion branch May 13, 2026 16:14
JBCongdon pushed a commit to JBCongdon/platform that referenced this pull request May 24, 2026
🤖 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants