Return command output verbatim by default

[tony]

Summary

• Breaking: .run() on the Git/Hg/Svn command classes (and the internal runner) now returns command output verbatim by default — exactly what the VCS printed. It previously stripped each line, dropped blank lines, and removed the trailing newline, so a captured git diff was rejected by git apply, git cat-file blob lost indentation and blank lines, and multi-line stderr was concatenated into one run-on line.
• Add a trim parameter (default False). Pass trim=True for the convenient "bare value" output the old default produced — a single whole-output str.rstrip, never per-line.
• Add trim passthrough on the named Git/Hg/Svn command methods, so the opt-in works everywhere, not only the low-level run().
• Keep downstream-facing scalar APIs bare: GitSync.get_revision() (and rev_parse/rev_list via trim=True) still return a clean value.
• Fix stderr assembly so multi-line errors keep their line breaks in libvcs.exc.CommandError.output.
• Document the decision, measured alternatives, and prior art in ADR 0001; add a changelog entry.

Breaking change

The default return of .run() flips from trimmed to verbatim. Reads that want a bare value opt in with trim=True.

Before — a bare value came back trimmed:

sha = git.run(["rev-parse", "HEAD"])

After — output is verbatim; opt into trimming:

sha = git.run(["rev-parse", "HEAD"], trim=True)

High-level helpers are unaffected: GitSync.get_revision() still returns a bare revision, and downstream vcspull (which strips defensively) passes unchanged — see the companion PR.

The bug, now fixed at the default

Before (on master) — .run(["diff"]) corrupts the patch; git apply rejects it:

@@ -1,5 +1,5 @@
def greet(name):
-    message = "hello, " + name
+    message = "hi, " + name
print(message)
return message

After (this PR) — .run(["diff"]) is byte-identical to git diff and applies cleanly:

@@ -1,5 +1,5 @@
 def greet(name):
-    message = "hello, " + name
+    message = "hi, " + name
 
     print(message)
     return message

Changes by area

• src/libvcs/_internal/run.py: capture stdout/stderr verbatim and decode once; new trim parameter (default False) applies a single whole-output rstrip when set; stderr keeps its line structure; the UTF-8 decode fallback uses errors="backslashreplace".
• src/libvcs/cmd/git.py, cmd/hg.py, cmd/svn.py: forward trim from the named command methods to run(); doctests show clean output via trim=True; the GitRemoteCmd.set_url doctest is now git-version-agnostic; Svn.blame reflects the faithful output.
• src/libvcs/sync/git.py: get_revision() and update_repo's rev_list reads strip to keep bare SHAs.
• tests/: functional tests for verbatim diff/blob fidelity + git apply, the default-preserves-newline contract, and stderr line preservation; sync/cmd tests adapted to verbatim output.
• docs/project/adr/, CHANGES: ADR 0001 (faithful subprocess output capture) wired into the Project docs, plus a changelog entry.

Design decisions

• Verbatim by default, transform at the edge: matches the convention of every tool surveyed in ADR 0001 (pip, uv, mise, mercurial, gitoxide, jujutsu) — capture pristine, trim only where a scalar is wanted. The per-line strip was the outlier and the source of the corruption.
• trim=True is the opt-in for bare values: a single whole-output rstrip, never per-line; high-level sync APIs apply it internally so downstream comparisons stay bare.
• Phase 1 of ADR 0001: the structured CompletedProcess backend (libvcs._internal.subprocess.SubprocessCommand) and retiring console_to_str are Phase 2, deferred to keep this change focused.

Companion PR

• py(deps) libvcs 0.43.0 -> 0.44.0 vcspull#556 (draft) — pins vcspull to this branch via uv.sources; its full suite passes unchanged, confirming the verbatim default is downstream-safe.

Verification

The old per-line strip/filter is gone:

$ rg "line.strip\(\) for line in" src/libvcs/_internal/run.py

(returns zero matches)

Test plan

• test_run_trim_false_preserves_diff — captured diff is byte-identical to git diff and passes git apply --check
• test_run_trim_false_preserves_blob — cat-file blob round-trips byte-for-byte
• test_run_default_preserves_trailing_newline — default .run() returns verbatim output (trailing newline kept); trim=True yields the bare value
• test_run_failure_preserves_stderr_lines — failed command keeps stderr line structure in CommandError.output
• uv run pytest — full suite green
• uv run ruff check . && uv run ruff format --check .
• uv run mypy
• Downstream: vcspull's suite passes against this branch (py(deps) libvcs 0.43.0 -> 0.44.0 vcspull#556)

[codecov]

Codecov Report

❌ Patch coverage is 81.69014% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.19%. Comparing base (163a945) to head (0069b20).

Files with missing lines	Patch %	Lines
src/libvcs/cmd/svn.py	0.00%	8 Missing ⚠️
src/libvcs/cmd/git.py	0.00%	3 Missing and 1 partial ⚠️
src/libvcs/_internal/run.py	88.88%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #538      +/-   ##
==========================================
+ Coverage   60.98%   61.19%   +0.20%     
==========================================
  Files          40       40              
  Lines        6531     6563      +32     
  Branches     1101     1103       +2     
==========================================
+ Hits         3983     4016      +33     
+ Misses       1951     1950       -1     
  Partials      597      597              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tony]

Code review

Found 1 issue:

1. rev_parse (and rev_list) were missed by the new trim passthrough, so trim=True is silently ignored on them. rev_parse declares **kwargs but its self.run(...) call never forwards them, so rev_parse(args="HEAD", trim=True) returns verbatim output with a trailing newline instead of a bare SHA — the documented opt-in is a no-op. The same gap leaves rev_list verbatim, and GitSync.update_repo() feeds that newline-suffixed value straight into reset(pathspec=head_sha), which git rejects (fatal: invalid reference: <sha>). get_revision() got a .strip() fix for exactly this; these two methods did not. The update_repo path is a stash-pop recovery branch not exercised by tests, so the suite stays green.

rev_parse drops the kwargs (no trim/**kwargs forwarded):

libvcs/src/libvcs/cmd/git.py

Lines 2059 to 2063 in 2049c04

	return self.run(
	["rev-parse", *local_flags],
	check_returncode=check_returncode,
	)

The newline-suffixed SHA used as a git pathspec:

libvcs/src/libvcs/sync/git.py

Lines 638 to 643 in 2049c04

	# Stash pop failed: Restore previous state.
	with contextlib.suppress(exc.CommandError):
	self.cmd.reset(
	pathspec=head_sha,
	hard=True,
	quiet=True,

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[tony]

Code review

Fresh re-review at head 788caff. The earlier rev_parse/rev_list trim finding is resolved (both forward **kwargs now, and update_repo strips). Found 2 issues, both in the new ADR:

1. "Alternatives considered" carries branch-internal narrative and a fragile test count (AGENTS.md/CLAUDE.md AI-slop rubric: "Avoid ... fragile file/test counts" and "Do not mention intermediate branch states ... unless users of a published release actually experienced the old state"). Drop the subprocess-trimming branch name and "575 tests at baseline".

libvcs/docs/project/adr/0001-faithful-subprocess-output-capture.md

Lines 88 to 91 in 788caff

	Each candidate was spiked on the `subprocess-trimming` branch on
	2026-06-20 and measured against the test suite (575 tests at baseline)
	and a probe checking whether a captured diff survives `git apply` and

2. The ADR contradicts its own shipped decision on .run(). Phase 1 makes output verbatim by default, but Phase 2 calls .run() -> str a "trimmed convenience facade" and the Risks section says "the facade is defined as rstrip()" — leftover wording from the earlier trimmed-default design.

libvcs/docs/project/adr/0001-faithful-subprocess-output-capture.md

Lines 80 to 82 in 788caff

	callers that want streams, exit code, and exact bytes. Keep
	`.run() -> str` as a trimmed convenience facade layered on top.
	- Retire `libvcs._internal.run.console_to_str` in favor of subprocess's

(also Risks:

libvcs/docs/project/adr/0001-faithful-subprocess-output-capture.md

Lines 141 to 143 in 788caff

	- Behavior drift between the trimmed facade and the structured result.
	Mitigation: the facade is defined as `rstrip()` over the structured
	result's decoded stdout, not a separate code path.

)

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}