Skip to content

Add confirm dialog when deleting keys #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karl-sjogren merged 1 commit into from
Dec 30, 2025
Merged

Add confirm dialog when deleting keys #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Karls.BetterSecretsTool/Tool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,17 @@ private void ShowSecretJson(ISecretsStore secretStore) {

private void RemoveSecret(ISecretsStore secretStore) {
var key = SelectKey(secretStore, "[grey]Select a secret to delete:[/]");

var response = _console.Prompt(
new TextPrompt<string>($"[grey]Are you sure you want to delete [red]{Markup.Escape(key)}[/]? ([green]y[/]/[red]n[/]):[/]")
.AllowEmpty());
Comment on lines +219 to +220

Copilot AI Dec 30, 2025

Copy link

Choose a reason for hiding this comment

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

The confirmation logic treats an empty response (just pressing Enter) as a cancellation, which may not be intuitive for users. When users press Enter without typing anything, it's unclear whether they meant to confirm or cancel the deletion. Consider either:

  1. Requiring an explicit response (remove AllowEmpty() and provide a default value), or
  2. Treating empty responses as "no" explicitly by checking for empty string in the confirmation logic, or
  3. Using a Yes/No selection prompt instead of a text prompt for clearer user intent.
Suggested change
new TextPrompt<string>($"[grey]Are you sure you want to delete [red]{Markup.Escape(key)}[/]? ([green]y[/]/[red]n[/]):[/]")
.AllowEmpty());
new TextPrompt<string>($"[grey]Are you sure you want to delete [red]{Markup.Escape(key)}[/]? ([green]y[/]/[red]n[/]):[/]"));

Copilot uses AI. Check for mistakes.
var confirmed = response.Equals("y", StringComparison.OrdinalIgnoreCase) ||
response.Equals("yes", StringComparison.OrdinalIgnoreCase);

if(!confirmed) {
return;
}

secretStore.Remove(key);
secretStore.Save();
}
Expand Down
4 changes: 3 additions & 1 deletion test/Karls.BetterSecretsTool.Tests/ConsoleUIIntegrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ public void RemoveSecret_WhenUserRemovesSecret_SecretIsDeleted() {
secretsStore.Set("SecretToRemove", "SomeValue");
secretsStore.Save();

// Simulate: D (delete secret), select the key (Enter to select first), then Q (quit)
// Simulate: D (delete secret), select the key (Enter to select first), confirm (y), then Q (quit)
console.Input.PushText("D");
console.Input.PushKey(ConsoleKey.Enter); // Select first (only) item
console.Input.PushTextWithEnter("y"); // Confirm deletion

Copilot AI Dec 30, 2025

Copy link

Choose a reason for hiding this comment

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

There is no test coverage for the cancellation scenario when a user declines to delete a secret (e.g., by entering "n" or "no"). Add a test case that verifies the secret is not deleted when the user cancels the deletion confirmation.

Copilot uses AI. Check for mistakes.
console.Input.PushText("Q");

// Act
Expand Down Expand Up @@ -165,6 +166,7 @@ public void AddEditRemove_FullWorkflow_SecretsAreManagedCorrectly() {
// Step 4: Remove SecondKey (first in sorted list, so just press Enter)
console.Input.PushText("D");
console.Input.PushKey(ConsoleKey.Enter); // Select first item (SecondKey)
console.Input.PushTextWithEnter("y"); // Confirm deletion

// Step 5: Quit
console.Input.PushText("Q");
Expand Down
4 changes: 3 additions & 1 deletion test/Karls.BetterSecretsTool.Tests/EndToEndIntegrationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,10 @@ public void Run_WithProjectDirectory_RemoveSecret_SecretIsDeleted() {
console.Interactive();
var tool = CreateRealTool(console);

// Simulate: D (delete secret), select the key (Enter to select first), then Q (quit)
// Simulate: D (delete secret), select the key (Enter to select first), confirm (y), then Q (quit)
console.Input.PushText("D");
console.Input.PushKey(ConsoleKey.Enter); // Select first (only) item
console.Input.PushTextWithEnter("y"); // Confirm deletion

Copilot AI Dec 30, 2025

Copy link

Choose a reason for hiding this comment

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

There is no test coverage for the cancellation scenario when a user declines to delete a secret (e.g., by entering "n" or "no"). Add a test case that verifies the secret is not deleted when the user cancels the deletion confirmation.

Copilot uses AI. Check for mistakes.
console.Input.PushText("Q");

// Act
Expand Down Expand Up @@ -196,6 +197,7 @@ public void Run_WithProjectDirectory_FullWorkflow_SecretsAreManagedCorrectly() {
// Step 4: Remove SecondKey (first in sorted list, so just press Enter)
console.Input.PushText("D");
console.Input.PushKey(ConsoleKey.Enter); // Select first item (SecondKey)
console.Input.PushTextWithEnter("y"); // Confirm deletion

// Step 5: Quit
console.Input.PushText("Q");
Expand Down