Skip to content

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

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

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
Contributor

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
@mjansson mjansson added the ready-to-import Approved by Epic staff for import into Lore label Aug 31, 2026
@epic-lore-bot epic-lore-bot Bot added imported Imported into Lore for internal review and removed ready-to-import Approved by Epic staff for import into Lore labels Aug 31, 2026
@epic-lore-bot

epic-lore-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

Imported as Lore CR-497.

epic-lore-bot Bot pushed a commit that referenced this pull request Aug 31, 2026
## Summary

`lore repository delete` now respects the `--dry-run` global flag. Previously, `delete()` in `lore-revision` called `repository_service.delete(id)` unconditionally, ignoring `dry_run` entirely - a correctness gap compared to every other destructive command in the codebase.

- `lore-revision/src/repository/delete.rs`: wraps the `repository_service.delete(id)` call in `if !execution_context().globals().dry_run()`, following the same pattern used in `clone.rs` and `branch/push.rs`. Repository name/ID resolution still runs in dry-run mode, so bad URLs are still caught.
- `lore-client/src/cli/commands/repository.rs`: the success callback now checks `dry_run` and prints "Repository would be deleted" instead of "Repository deleted successfully" when the flag is set.

No existing `repository delete` tests exist, so leaving this as manual test for now.

## Test Plan

Ran a manual `repository delete --dry-run` and verified the output. 

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

```
Imported-PR: #174
Imported-From: 904944a
Imported-Base: 4d563ea
Imported-Merge: 0d7c3a0
Imported-Merge-Strategy: verbatim
Imported-Merged-Paths: 0
Imported-Author: Jonathan McMichael (jonathanmcmichael)
Signed-off-by: Jonathan McMichael <jonathanmc@dpr.com>
GH-URL: #174
```

Lore-RevId: 845
Lore-Signature: d4c5e021989a0d0844b24819f65994fb31f5f0e9bc3d98b969f3d002042841fd
@epic-lore-bot

epic-lore-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

Closed by mirrored commit 046de55.

@epic-lore-bot epic-lore-bot Bot closed this Aug 31, 2026
@epic-lore-bot epic-lore-bot Bot added the merged Merged into Lore codebase label Aug 31, 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 imported Imported into Lore for internal review merged Merged into Lore codebase

Development

Successfully merging this pull request may close these issues.

2 participants