fix(shared-writer): make blocker/label/relation mutations idempotent (#600)#605
Merged
Conversation
…600) SharedWriter::{add,remove}_{blocker,label,relation} now short-circuit before write_commit_push when no change is needed, avoiding the empty git commit that previously failed with a confusing "git commit […] in cache failed:" error. (Git emits the "nothing to commit" hint on stdout, which git_commit_in_cache_with_args drops — see #601.) Method signatures change Result<()> -> Result<bool> (true = mutated, false = no-op). commands::{deps,label,relate} writer branches use the new bool to print differentiated "Dependency already exists" / "Label '<x>' already exists" / "Issues X and Y are already related" messages, matching what their non-writer counterparts already did. Tightened test_add_label_idempotent (was swallowing the bug with `let _ =`) and added regression tests for the five sibling mutations. Also fixes 20 pre-existing clippy::pedantic warnings in lock_check.rs (11x map_unwrap_or on identical pattern), container.rs, compaction.rs, sentinel/collect.rs, and sync/tests.rs so cargo clippy -D warnings passes again. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 15, 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.
Summary
Fixes GH#600.
SharedWriter::{add,remove}_{blocker,label,relation}now short-circuit beforewrite_commit_pushwhen no change is needed, socrosslink issue block <a> <b>exits 0 when<a>is already blocked by<b>(previously failed with a confusinggit commit […] in cache failed:error caused by the empty index path — see #601 for the underlying stdout-capture bug).Changes
shared_writer/mutations.rs): All six mutations gain an early-return guard that loads the issue, checks for the no-op condition, and returns without touching git. Signatures changeResult<()>→Result<bool>(true= mutated,false= no-op short-circuit).commands/{deps,label,relate}.rs): Writer branches now use the new bool to print differentiated messages (Dependency already exists,Label 'X' already exists on issue …,Issues X and Y are already related, plus symmetric remove-not-present messages) — matching what the non-writer branches already did.shared_writer/tests.rs): Tightenedtest_add_label_idempotent(it was usinglet _ =to swallow the bug, with a// duplicate -- may error on empty commitcomment). Added five new regression tests:test_remove_label_idempotent,test_{add,remove}_blocker_idempotent,test_{add,remove}_relation_idempotent.clippy::pedanticwarnings that were already ondevelop, socargo clippy --lib --bin crosslink --tests -- -D warningspasses again.lock_check.rshad 11 instances of the samemap(|s| !s.success()).unwrap_or(true)pattern; others touchcontainer.rs,compaction.rs,sentinel/collect.rs,sync/tests.rs.Why
Result<bool>instead of silent short-circuitThe reported defect is about exit code, but the writer-branch UX was also inconsistent with the non-writer branch (which already returns
boolviadb.add_dependencyetc. and printsDependency already existsaccordingly). Threading the bool through gives the writer-branch the same honest feedback rather than silently printing "Issue X is now blocked by Y" on a no-op.Out of scope
git_commit_in_cache_with_argsreports empty error messages for the most common failure mode #601 (stdout dropped fromgit_commit_in_cache_with_argserror messages) — fixing this would partially closecrosslink issue blockexits non-zero when the blocker is already set #600 symptoms but is its own bug.crosslink integrity hydration --repairdeletes SQLite rows when the JSON files do not have them #602,add_blockerflow is not transactional across JSON / git / SQLite #604 (hydration / transactionality) — listed as related incrosslink issue blockexits non-zero when the blocker is already set #600 but separate concerns.Test plan
cargo test --lib— 1888 passedcargo test --bin crosslink— 2864 passed (includes the new 6 idempotency tests and the existingtest_block_duplicate/test_add_duplicate_label/test_add_duplicate_relationthat exercise the non-writer branch)cargo clippy --lib --bin crosslink --tests -- -D warnings— cleancrosslink issue block L2 L1twice in a row → both exit 0, second printsDependency already exists🤖 Generated with Claude Code