fix(gitlab): avoid /raw_diffs 404 on GitLab < 17.9

[puemos]

Summary

fetch_mr_diff used to shell out to glab mr diff --raw. Recent glab releases implement --raw by calling GitLab's /projects/:id/merge_requests/:iid/raw_diffs endpoint, which first shipped in GitLab 17.9 (archived 17.8 docs show it absent; archived 17.9 docs show it present). On older self-hosted instances — the reporter in #12 was on GitLab CE 17.8.6 — that endpoint returns 404 Not Found and the diff fetch aborts:

Could not obtain raw diff: 404 Not Found

This PR replaces glab mr diff --raw with a single glab api --paginate --output ndjson call to the long-standing /diffs endpoint and reconstructs the diff --git framing that the downstream unidiff-based DiffIndex parser requires.

Closes #12.

Why --raw existed in the first place

Without --raw, glab mr diff prints a unified diff without the diff --git a/... b/... separator headers between files. DiffIndex (wrapping unidiff) hard-requires those headers to split the stream into per-file views — without them, the parser sees the whole thing as one big diff and only the first file is recognised. Commit 015ca0b originally added --raw to fix exactly that.

This PR takes the other road: drop --raw entirely, use the structured JSON endpoint, and synthesise the framing in lareview. That keeps DiffIndex happy and works on every GitLab version from at least 17.x onward (the /diffs endpoint predates both 17.8 and 17.9, though GitLab's docs don't cite the exact introduction version).

The fix

src/infra/vcs/gitlab.rs::fetch_mr_diff — rewritten

One glab api --paginate --output ndjson invocation hitting:

projects/:encoded/merge_requests/:iid/diffs?per_page=20

The endpoint returns one JSON object per changed file with old_path, new_path, a_mode, b_mode, new_file, deleted_file, renamed_file, and diff. A new synthesize_unified_diff helper wraps each entry in the framing the parser expects — diff --git, --- /dev/null / +++ b/... for adds, rename from / rename to for renames, etc. — then appends the raw hunks from the JSON verbatim.

src/infra/cli/diff.rs::DiffSource::GitLabMr — deduplicated

There was a second, verbatim copy of the same glab mr diff --raw shell-out in the CLI entry point — same bug, independent code path. It now calls gitlab::fetch_mr_diff via the existing block_on helper. The two paths can't drift apart again.

Why --paginate --output ndjson specifically

glab api --paginate walks the server's Link: rel="next" header internally, collapsing what would otherwise be N sequential subprocess spawns (one per page) into a single invocation. This was explicitly flagged by review and is the right mitigation here.

The catch: glab's default output mode doesn't transform multi-page responses — it concatenates the raw bodies back-to-back with no separator, so for an endpoint returning JSON arrays you get [...p1...][...p2...] which is not a valid single JSON array (confirmed by glab's own REST pagination test: api_test.go#L556 asserts the output is literally {"page":1}{"page":2}{"page":3}).

--output ndjson is the supported programmatic-consumption mode — it emits each array element as one JSON document per line, which serde_json::Deserializer::into_iter::<GlabMrChange>() stream-parses cleanly.

On per_page preservation: glab's addPerPage only injects its default (100) when the URL has no per_page, and subsequent pages use the server's Link: rel="next" URL verbatim — which echoes our per_page=20 back into every paged request. Verified in api_test.go which asserts per_page=100 appears only on the first request when the user supplied nothing.

Why per_page=20

It's the documented default for this endpoint per the GitLab REST API reference. It also incidentally sidesteps a GitLab 17.8.x upstream bug: per_page > 30 triggers NoMethodError: undefined method 'page' for PaginatedMergeRequestDiff in the /diffs controller (the high-per_page path swaps to a pre-paginated collection that doesn't respond to Kaminari's .page while the controller calls it anyway). The bug is not called out in the docs, because bugs by definition aren't documented. 20 is both spec-compliant and safe on 17.8.x, so we pin it.

Verification

1. Mock integration test — tests/gitlab_diff_fetch_integration.rs installs a fake glab on PATH that exits non-zero on any --raw invocation, asserts the Rust code passes both --paginate and --output ndjson (so dropping the perf optimization re-trips the review comment), answers with the canned multi-file ndjson payload, and checks fetch_mr_diff's output parses into the expected two files via DiffIndex::new.

2. Real GitLab 17.8.6 via Docker — during development I stood up a manual harness (not in this PR) that booted gitlab/gitlab-ce:17.8.6-ce.0, seeded a multi-file MR via REST, and ran fetch_mr_metadata + fetch_mr_diff against it through a real glab. Red leg (pre-fix source) reported Could not obtain raw diff: 404 Not Found verbatim matching #12. Green leg (post-fix source, same container) reported diff --git headers: 2, diff bytes: 6553, both files parsed. The real run is also what surfaced the per_page > 30 upstream bug and drove the per_page=20 pin; the mock test alone would not have caught that. The harness code was stripped from the branch after verification since it's not shippable.

3. Build + lint + full suite — cargo test (201 tests), cargo fmt -- --check, cargo clippy --all-targets --all-features -- -D warnings, all clean.

Docs grounding

Every version-specific claim in this PR and in the code comments is grounded in an authoritative source, not in E2E observation:

• /raw_diffs first shipping in 17.9: 17.8 archived docs (absent) vs 17.9 archived docs (present).
• /diffs per_page default of 20: current endpoint reference.
• glab api --paginate semantics (walks Link: rel="next", stops when absent): pagination.go::findNextPage.
• glab api --output ndjson emitting one element per line: api.go source + api_test.go line 556.
• per_page preservation across --paginate pages: addPerPage injects default only when absent.

Test plan

• cargo test --test gitlab_diff_fetch_integration
• cargo test
• cargo fmt --check && cargo clippy --all-targets --all-features -- -D warnings
• Manual smoke against any GitLab < 17.9 instance (the only code path this change meaningfully improves over the old --raw approach from the reporter's perspective). If you don't have one handy, a GitLab ≥ 17.9 instance confirms the fix doesn't regress newer servers.

Non-goals

• push_review / push_feedback against GitLab still shell out to glab api the same way as before — not touched here.
• Binary diffs are passed through with empty diff bodies (GitLab's own representation), matching pre-existing behaviour — not independently exercised by the test.
• Direct HTTP (reqwest) as a replacement for glab is not pursued; --paginate --output ndjson already collapses the subprocess-per-page cost to 1, and keeping glab as the single VCS auth surface avoids introducing a second "way of talking to GitLab" that could drift.