fix(shared-writer,sync): include stdout in git cache-failure errors (#601)

[dollspace-gay]

Summary

Fixes GH#601. git_{commit_in_cache_with_args,in_cache} (both in shared_writer/core.rs and sync/core.rs) captured only output.stderr when bailing on a non-zero exit. But git commit's most common failure status — "nothing to commit, working tree clean" — is written to stdout, leaving stderr empty. The user saw Error: git commit […] in cache failed: with nothing after the colon and no way to diagnose what went wrong.

The fix is a single-shape change applied to all four sites: include output.stdout, output.stderr, and the exit status in the bail message.

Side effect: four dead-code guards in the push path revive

The push paths in core.rs have four substring-matching guards intended to treat "nothing to commit" as a clean no-op:

if err_str.contains("nothing to commit") || err_str.contains("no changes added") {
    return Ok(PushOutcome::Pushed);
}

…at lines 318, 416, 506, 976. They never fired on main, because err_str was built from empty stderr. After this fix, the substring lives in err_str and the guards do what they were written to do — so future no-op commit paths that don't have a per-callsite short-circuit (like the one added for #600) still degrade cleanly instead of bubbling a noisy empty error.

New bail format

git commit ["-m", "..."] in cache failed (exit status: 1):
stdout: On branch crosslink/hub
nothing to commit, working tree clean
stderr:

Same treatment applied to all four sites ({commit_,}in_cache × {shared_writer,sync}) — the same stdout-vs-stderr trap would mask any future failure whose diagnostic lands on stdout (pre-commit hook output, server-side push rejections that bubble back through stdout, etc.).

Out of scope

The issue notes an optional defense-in-depth: detect the no-op explicitly via git diff --cached --quiet before invoking git commit. Skipped here because:

1. The issue explicitly marks it optional.
2. The substring guards now fire correctly (the side effect above), which is the same outcome the defense-in-depth provides.
3. It adds an extra git invocation on every write-path call.

Worth a separate PR if the team wants structural robustness against git's wording changing across versions/locales.

Test plan

• cargo test --lib --bin crosslink — 2866 passed (2 new regression tests)
• cargo clippy --lib --bin crosslink --tests -- -D warnings — clean
• New tests:
  • test_git_commit_in_cache_includes_stdout_on_failure — asserts the "nothing to commit" string surfaces in the bail message
  • test_git_in_cache_includes_stdout_on_failure — asserts both stdout: and stderr: labels appear in non-commit git errors too
• Manual repro from the issue: crosslink issue create "a" -q; crosslink issue create "b" -q; crosslink issue block L2 L1 twice. With fix(shared-writer): make blocker/label/relation mutations idempotent (#600) #605 (the crosslink issue block exits non-zero when the blocker is already set #600 fix) merged this hits the per-callsite short-circuit; without it, the second call now reports a meaningful error including "nothing to commit, working tree clean" rather than an empty failed: .

Related

• crosslink issue block exits non-zero when the blocker is already set #600 (PR fix(shared-writer): make blocker/label/relation mutations idempotent (#600) #605) — user-facing manifestation. Closed by per-callsite idempotency. This PR closes the underlying diagnostic-loss bug, which would have masked any similar future failure mode.

🤖 Generated with Claude Code