Skip to content

repository: honor --dry-run in repository delete - #174

Open
jonathanmcmichael wants to merge 1 commit into
EpicGames:mainfrom
jonathanmcmichael:fix/repository-delete-dry-run
Open

repository: honor --dry-run in repository delete#174
jonathanmcmichael wants to merge 1 commit into
EpicGames:mainfrom
jonathanmcmichael:fix/repository-delete-dry-run

Conversation

@jonathanmcmichael

Copy link
Copy Markdown

What

lore repository delete --dry-run now actually performs a dry run instead
of deleting the repository. The CLI also reports "Repository would be
deleted" rather than "Repository deleted successfully" when --dry-run is
set.

Why

lore-revision/src/repository/delete.rs::delete() never checked
dry_run — it called repository_service.delete(id) unconditionally.
Every other destructive command in this codebase (revision commit,
branch push/switch, clone) gates its mutating call behind
globals.dry_run() / execution_context().globals().dry_run(); delete
was simply missing that check. This is a safety-flag correctness bug: a
user relying on --dry-run before a destructive operation gets no warning
and no protection.

How

  • lore-revision/src/repository/delete.rs: wrap the
    repository_service.delete(id) call in
    if !execution_context().globals().dry_run(), following the same pattern
    already used in repository/clone.rs and branch/push.rs in this crate.
    The repository name/ID is still resolved and validated either way, so a
    dry run still surfaces a bad URL/name the same way a real delete would.
  • lore-client/src/cli/commands/repository.rs: the delete callback's
    success message now distinguishes dry_run (prints "Repository would be
    deleted") from a real delete ("Repository deleted successfully"),
    matching the message style already used by revision commit
    ("Previewing commit" vs "Committing").

Testing

  • cargo +nightly fmt --all
  • cargo clippy -p lore-revision -p lore-client --all-targets -- -D warnings --no-deps (full-workspace clippy currently fails on an unrelated pre-existing issue in lore-revision/src/util/fs.rs, not touched by this PR)
  • cargo test -p lore-revision -p lore-client
  • No test included in this PR. There was no existing test coverage for
    repository delete at all, and none of this crate's tests spin up a
    real server, so covering this properly meant adding new test
    infrastructure rather than a single test function. That's split out
    into a companion PR (in-process gRPC server harness + dry-run/real-delete
    tests for repository::delete::delete()) so it can be reviewed on its
    own.

Notes for reviewers

Small, mechanical, non-security change — no auth/crypto paths touched.

AI assistance disclosure

This change (code and this PR description) was developed with Claude (Anthropic) assistance. I reviewed and tested the change myself and am responsible for its correctness.

repository::delete never checked dry_run and always called the real
delete RPC. Gate it behind execution_context().globals().dry_run(),
matching the pattern already used in repository/clone.rs and
branch/push.rs. Also fix the CLI success message so a dry run reports
"would be deleted" instead of falsely claiming the repository was
deleted.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Jonathan McMichael <jonathanmc@dpr.com>
@github-actions github-actions Bot added area:client CLI area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals labels Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:client CLI area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals

Development

Successfully merging this pull request may close these issues.

1 participant