`crosslink issue block` exits non-zero when the blocker is already set

[vringar]

Summary

Calling crosslink issue block <a> <b> a second time — when <a> is already
blocked by <b> — fails with a git commit […] in cache failed: error and
exit code 1, even though the desired post-state is already present. The command
is not idempotent.

The root cause is in SharedWriter::add_blocker: the closure always returns a
WriteSet containing the issue JSON, even when no field was modified. That
write then drives a git commit with no staged changes, which git reports as
a failure with exit code 1.

Reproducer

crosslink issue create "first"  -q   # → L1
crosslink issue create "second" -q   # → L2
crosslink issue block L2 L1          # succeeds: commits a real diff
crosslink issue block L2 L1          # fails with "git commit [...] failed: "
echo $?                              # → 1

Offending code

add_blocker only updates the in-memory issue when the blocker is missing, but
the JSON is serialized and returned in every case — so an unchanged JSON is
still handed to write_commit_push, which calls git add + git commit.

crosslink/crosslink/src/shared_writer/mutations.rs

Lines 545 to 568 in 12eb7b9

	pub fn add_blocker(&self, db: &Database, issue_id: i64, blocking_issue_id: i64) -> Result<()> {
	let blocker_uuid = self.resolve_uuid(blocking_issue_id, db)?;
	let _ = self.write_commit_push(
	|writer| {
	let mut issue = writer.load_issue_by_id(issue_id, db)?;
	if !issue.blockers.contains(&blocker_uuid) {
	issue.blockers.push(blocker_uuid);
	issue.updated_at = Utc::now();
	}
	let json = serde_json::to_vec_pretty(&issue)?;
	let rel_path = writer.issue_rel_path(&issue.uuid);
	Ok(WriteSet {
	files: vec![(rel_path, json)],
	counters: None,
	use_git_rm: false,
	})
	},
	&format!("block issue #{issue_id} on #{blocking_issue_id}"),
	)?;
	self.hydrate_with_retry(db);
	Ok(())
	}

The command dispatch in commands::deps::block also does not detect the
already-blocked case before invoking the writer:

crosslink/crosslink/src/commands/deps.rs

Lines 8 to 39 in 12eb7b9

	pub fn block(
	db: &Database,
	writer: Option<&SharedWriter>,
	issue_id: i64,
	blocker_id: i64,
	) -> Result<()> {
	// Check if both issues exist
	db.require_issue(issue_id)?;
	db.require_issue(blocker_id)?;
	if issue_id == blocker_id {
	bail!("An issue cannot block itself");
	}
	if let Some(w) = writer {
	w.add_blocker(db, issue_id, blocker_id)?;
	println!(
	"Issue {} is now blocked by {}",
	format_issue_id(issue_id),
	format_issue_id(blocker_id)
	);
	} else if db.add_dependency(issue_id, blocker_id)? {
	println!(
	"Issue {} is now blocked by {}",
	format_issue_id(issue_id),
	format_issue_id(blocker_id)
	);
	} else {
	println!("Dependency already exists");
	}
	Ok(())
	}

Note that the non-writer branch (line 29-37) does handle the "already exists"
case via db.add_dependency returning false, printing
"Dependency already exists". Only the writer branch is broken.

Why it matters

Any automation that idempotently re-applies block relations (e.g. a script
computing transitive predecessors, or a re-run after a partial failure) hits
this. In a downstream project of mine, a script that re-asserted some
relations as part of fixed-point iteration produced ~10 non-zero exits across
~50 invocations — and the script's shell-level error handling masked which
relations actually persisted to the event log.

The same shape will affect add_relation (mutations.rs:610-633) and
add_label-style mutations that share the unconditional-serialize pattern.

Suggested fix

Short-circuit before write_commit_push:

pub fn add_blocker(&self, db: &Database, issue_id: i64, blocking_issue_id: i64) -> Result<()> {
    let blocker_uuid = self.resolve_uuid(blocking_issue_id, db)?;
    let issue = self.load_issue_by_id(issue_id, db)?;
    if issue.blockers.contains(&blocker_uuid) {
        return Ok(());  // already blocked — nothing to do
    }
    // ... existing write_commit_push path ...
}

Equivalently, the closure inside write_commit_push could signal "no diff" via
the WriteSet shape and have the writer skip the commit step.

Either way, the user-visible contract should be: crosslink issue block exits
0 when the post-state is blocked-by(<a>, <b>), regardless of whether that
required a new event.

Related

• git_commit_in_cache_with_args reports empty error messages for the most common failure mode #601 — git_commit_in_cache_with_args drops stdout when reporting failures.
  That is what makes this bug surface as a noisy git commit […] failed:
  with no detail rather than a clear "already blocked" status. Fixing git_commit_in_cache_with_args reports empty error messages for the most common failure mode #601
  alone would partially close the user-visible symptoms here even without
  any add_blocker change.
• crosslink integrity hydration --repair deletes SQLite rows when the JSON files do not have them #602 — integrity hydration --repair is destructive when SQLite drifts
  from the JSON files. That is the bug that turns this transient failure
  into permanent data loss in some configurations.
• add_blocker flow is not transactional across JSON / git / SQLite #604 — add_blocker flow is not transactional across JSON / git / SQLite.
  The structural reason the partial-failure modes leave inconsistent state in
  the first place.