Repository navigation
Refuse an album name and a flag pair that cannot mean anything - #44
Merged
Merged
Conversation
Two ways `imogen album` accepted an argument it should have refused, both the write-side counterpart of #38's reading. `album update -d "new text" --clear-description` parsed happily, `albums::update` took the clearing branch, and the description asked for was discarded without a word, exit 0. `edit` has carried `conflicts_with = "clear_description"` since #32; `album update` declares the same pair and did not, so the same contradiction was a refusal in one command and a silent loss in the other. The fix is that attribute, not a second way of saying it: the inconsistency between the two was the defect. `album create ""` — or " ", or `--name` the same on `update` — wrote an album no reference could ever pick again. `albums_for` refuses an empty reference when reading, and no non-empty one matches an empty name, so `album show`, `add`, `remove`, `delete` and `--album` were all shut out of it and only its id was left. `album_name` is the mirror of that guard on the side that writes, refusing before anything reaches the wire. It trims as well as refuses, because `named_album` compares the *stored* name trimmed: a name written with padding would otherwise answer to something it is not spelled as, and `album create " Trip "` would leave an album a listing cannot tell from the one `upload --album Trip` makes. `album_or_create` already created under the trimmed name; the two doors into an album's name now agree. On `update` the refusal comes before the album is even looked up, so a rename that could never be undone by name stops the command whatever else it was asked to change. Closes #41 Closes #43 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The write-side half of the album hardening #42 /
4a3cc7adid for reading. Twoarguments
imogen albumaccepted and should not have.#41 —
-dand--clear-descriptiontogetheralbum update Holiday -d "new text" --clear-descriptionparsed,albums::updatetook the clearing branch, and the description asked for was dropped without a word,
exit 0.
edithas hadconflicts_with = "clear_description"on the same pair since#32.
album updatenow declares it the same way — the inconsistency between the twocommands was the defect, so it is the same attribute rather than a second way of
saying it.
#43 — a name no reference can pick
album create ""," ", oralbum update --namewith either, wrote an albumreachable only by its id:
albums_forrefuses an empty reference when reading, andno non-empty reference matches an empty name, so
album show,add,remove,deleteand--albumwere all shut out of it.context::album_nameis the mirror of that read-side guard, refusing beforeanything reaches the wire. It trims as well, because
named_albumcompares thestored name trimmed: a padded name would otherwise answer to something it is not
spelled as, and
album create " Trip "would leave an album a listing cannot tellfrom the one
upload --album Tripmakes.album_or_createalready created underthe trimmed name, so the two doors into an album's name now agree — creation and
lookup settle on the same string.
On
updatethe refusal runs beforefind_album, so a rename that could never beundone by name stops the command whatever else it was asked to change.
Filing into an album is still a name and not a search: nothing here touches
named_album's tiers.Tests
Written first; all five failed on
mainbefore the fix.mainwithcli::an_album_description_cannot_be_both_written_and_removedalbums::an_album_cannot_be_created_without_a_namealbums::an_album_cannot_be_renamed_to_nothingalbums::a_new_album_is_created_under_the_trimmed_name" Trip\n"albums::a_rename_is_stored_trimmed_too" Trip\n"The two refusal tests assert
stub.calls().is_empty()— the refusal is before thewire, not a rejection from the server. The parse test also asserts each flag alone
still parses, so a
conflicts_withnaming the wrong id cannot pass unnoticed.src/commands/albums.rsgains its own stub server, ascontext.rsandcommands/people.rseach carry one; no shared test component was introduced.Both audiences
The refusal is a plain
anyhowerror down the one pathmain::reportalreadyrenders twice. Checked against the release binary with an unreachable server, which
was never contacted:
Both exit 1.
album update -d … --clear-descriptionis clap's own refusal, exit 2.Verify
cargo test --locked && cargo fmt --check && cargo clippy --all-targets --locked -- -D warnings && cargo build --release --locked— green. 154 unit + 4 integrationtests pass.
Nothing on the wire changes and the SDK pin does not move.
Closes #41
Closes #43
🤖 Generated with Claude Code