feat(minimal): add bulk session destruction by jtnkminimal · Pull Request #811 · gominimal/minimal

jtnkminimal · 2026-07-17T17:40:54Z

Summary by CodeRabbit

New Features
- Enhanced the destroy CLI to terminate either one specified session or all active sessions in a single command.
- Added interactive yes/no confirmation for bulk destruction (configurable via force), while supporting non-interactive/forced execution.
Bug Fixes
- Bulk destruction now stops and reports when any session removal fails.
- “Destroy all” now succeeds cleanly when there are no active sessions.
Tests
- Expanded CLI integration coverage for both single-session and all-sessions destroy flows.

coderabbitai · 2026-07-17T17:41:28Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9d4a332a-7955-4b2c-9471-ca918087b9c3

📥 Commits

Reviewing files that changed from the base of the PR and between ae0bfa8 and 3810341.

📒 Files selected for processing (1)

crates/minimal/src/lib.rs

🚧 Files skipped from review as they are similar to previous changes (1)

crates/minimal/src/lib.rs

📝 Walkthrough

Walkthrough

The destroy CLI now accepts either a session identifier or --all. All-session destruction lists active sessions, optionally prompts with default-no confirmation, supports --force, and reports failed destruction RPCs. Confirmation callers and integration tests were updated.

Changes

Session destruction

Layer / File(s)	Summary
Destroy command flow `crates/minimal/src/lib.rs`	`DestroyArgs` supports optional sessions, `--all`, and `--force`; command helpers implement single-session and all-session destruction with confirmation and failure handling. Confirmation callers now specify their defaults.
Destroy flow integration tests `crates/minimal/tests/cli.rs`	Existing single-session tests use the optional argument shape, and new tests cover deleting all sessions and handling no active sessions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

gominimal/inbox/306 — Covers the requested all-session enumeration, confirmation, force mode, and per-session destruction.
gominimal/minimal/805 — Covers the requested all-session destroy functionality.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant cmd_destroy
  participant Daemon
  User->>cmd_destroy: Select --all
  cmd_destroy->>Daemon: ListSessions
  cmd_destroy->>User: Request confirmation unless forced
  cmd_destroy->>Daemon: DestroySession for each active session
  Daemon-->>cmd_destroy: Return destruction results

Poem

A bunny found sessions lined in a row,
“Destroy them all?” with a cautious no.
Say yes, or force the burrow clear,
Each vanished session brings rabbit cheer.
Tests hop after, neat and bright.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding bulk session destruction in minimal.
Docstring Coverage	✅ Passed	Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.
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.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

crates/minimal/tests/cli.rs (1)
313-350: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add parser-level tests for the new CLI contract.

These tests construct DestroyArgs directly, so they do not validate clap’s new constraints. Cover no target, SESSION --all, and --force without --all to prevent argument-metadata regressions.
🤖 Prompt for 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.

In `@crates/minimal/tests/cli.rs` around lines 313 - 350, Add parser-level tests
for the DestroyArgs CLI contract, covering rejection of no target, rejection
when SESSION is combined with --all, and rejection when --force is used without
--all. Exercise clap parsing rather than constructing DestroyArgs directly, and
keep the existing runtime tests unchanged.

🤖 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.

Nitpick comments:
In `@crates/minimal/tests/cli.rs`:
- Around line 313-350: Add parser-level tests for the DestroyArgs CLI contract,
covering rejection of no target, rejection when SESSION is combined with --all,
and rejection when --force is used without --all. Exercise clap parsing rather
than constructing DestroyArgs directly, and keep the existing runtime tests
unchanged.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 54ddbf60-757e-4044-a200-0d45d82d144d

📥 Commits

Reviewing files that changed from the base of the PR and between 2d0e5e6 and ae0bfa8.

📒 Files selected for processing (2)

crates/minimal/src/lib.rs
crates/minimal/tests/cli.rs

twitchyliquid64 · 2026-07-17T17:42:58Z

+    #[arg(long)]
+    pub all: bool,
+    /// Skip confirmation when destroying all sessions
+    #[arg(long, short, requires = "all")]


If a user did --all, do we really need a confirmation?

I feel a confirm is a nice to have for now, we can remove if it gets annoying but I'd say we're the ones that will be killing everything, most folks will have pets running not cattle.

twitchyliquid64 · 2026-07-17T17:43:55Z

 }

+/// Prompt for confirmation, defaulting to no.
+fn confirm_default_no(question: &str) -> Result<bool, anyhow::Error> {


Assuming we are keeping the confirmation: use the existing confirm() function, add parameters for the default no etc as you need to

jtnkminimal · 2026-07-17T18:12:12Z

Addressed in 38103415: the existing confirm helper now accepts an explicit default, renders [Y/n] or [y/N], and all existing call sites pass their intended default. The duplicate confirm_default_no helper has been removed.

feat(minimal): add bulk session destruction

ae0bfa8

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

twitchyliquid64 reviewed Jul 17, 2026

View reviewed changes

refactor(minimal): parameterize confirmation default

3810341

twitchyliquid64 enabled auto-merge (squash) July 17, 2026 18:16

twitchyliquid64 approved these changes Jul 17, 2026

View reviewed changes

twitchyliquid64 merged commit 02ea2f4 into main Jul 17, 2026
28 checks passed

twitchyliquid64 deleted the amp-codex-5.5-test-805-destroy-all branch July 17, 2026 18:20

jtnkminimal mentioned this pull request Jul 17, 2026

Add min destroy all command #805

Closed

This was referenced Jul 29, 2026

fix: model session destroy target as a required arg group #1043

Merged

feat(minimal,minimald): confirm single-session destroy with the workspace delta #1148

Merged

Navigation Menu

Uh oh!

feat(minimal): add bulk session destruction - #811

feat(minimal): add bulk session destruction#811
twitchyliquid64 merged 2 commits into
mainfrom
amp-codex-5.5-test-805-destroy-all

jtnkminimal commented Jul 17, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

twitchyliquid64 Jul 17, 2026

Uh oh!

jtnkminimal Jul 17, 2026

Uh oh!

twitchyliquid64 Jul 17, 2026

Uh oh!

jtnkminimal commented Jul 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jtnkminimal commented Jul 17, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

twitchyliquid64 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

jtnkminimal Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

twitchyliquid64 Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

jtnkminimal commented Jul 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jtnkminimal commented Jul 17, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 17, 2026 •

edited

Loading