Skip to content

feat(diff): --exit-code so olcli diff can gate CI - #57

Merged
aloth merged 2 commits into
aloth:mainfrom
Waynting:feat/diff-exit-code
Sep 11, 2026
Merged

feat(diff): --exit-code so olcli diff can gate CI#57
aloth merged 2 commits into
aloth:mainfrom
Waynting:feat/diff-exit-code

Conversation

@Waynting

@Waynting Waynting commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

The second follow-up left open in #48, alongside --latexdiff. Makes olcli diff usable as a CI gate.

⚠️ Stacked on #56. This branch is cut from feat/latexdiff, so the diff here contains that PR's commit as well. Only 86b078b is new — please merge #56 first, or review this commit-by-commit. It is stacked rather than independent because --exit-code has to cover the failure paths --latexdiff added; implementing it against main would have shipped a known gap and handed you the conflict instead of resolving it myself.

What it does

olcli diff --exit-code             # 0 same, 1 differs, 2 failed
olcli diff --exit-code --name-only # the shape a pipeline usually wants
- name: Fail if the paper on Overleaf has drifted from the repo
  run: olcli diff --exit-code --name-only
  env:
    OVERLEAF_SESSION: ${{ secrets.OVERLEAF_SESSION }}

The 1-vs-2 split is the whole feature

diff(1)'s statuses, not just "non-zero on changes". A pipeline that cannot separate the project differs from the run failed reads an expired session cookie as a content change — and a job that goes red for the wrong reason sends whoever reads the log hunting for a diff that was never computed. That is worse than having no gate.

So all ten failure paths in the command move to 2 together: bad flag combinations, a missing directory, an unresolvable project, a latexdiff that is not installed, a root document that is not on the remote yet, a failed remote compile. If any one of them had stayed at 1, the flag would be quietly lying in exactly the case it exists for.

Without the flag every failure stays 1, which is what every other command exits, so scripts that check olcli diff for success see no change. Verified in both directions:

with --exit-code without
clean tree 0 0
a file differs 1 0
run failed 2 1

SIGINT during --pdf still exits 130, untouched.

The status is set, not exited on

process.exit discards whatever is still buffered on a non-TTY stdout, and olcli diff --exit-code > patch.txt is precisely a large patch going into a pipe. Measured rather than assumed:

$ node -e "process.stdout.write('x'.repeat(1500000)); process.exit(1)" | wc -c
  131072          # truncated at the 128 KB pipe buffer — 91% of the patch gone, mid-hunk
$ node -e "process.stdout.write('x'.repeat(1500000)); process.exitCode = 1" | wc -c
 1500000

So the command sets process.exitCode and returns, letting node flush first. Confirmed against a real project: a 1.5 MB patch redirected to a file arrived complete, trailer line and all, with status 1.

Scope of the gate

It reports on whatever was compared, which settles the two questions I expected you to ask:

  • --file narrows the gate to that file, the way a git diff --exit-code pathspec does. A --file matching nothing is 0 rather than an error — also git's behaviour.
  • --latexdiff is allowed and reports on the project, not on the markup. A changed figure is a real difference even though a marked-up root document cannot show one; the existing No .tex file differs line already says so. I considered rejecting the combination, as --name-only/--file/-U are rejected, but those shape patch output and this does not — keeping it orthogonal is what makes it composable in CI.

One consequence worth stating: a remote-only file counts as a difference, even though plain push leaves it alone. The two sides genuinely do not match, and a gate that passed would be saying they do.

Structure and tests

The decision is a pure function in src/diff.tsdifferencesExitCode(entries) plus the three named statuses — unit-tested with no account and no network, following the rename-plan.ts precedent. Not re-exported from the package root; happy to export it if you would rather it were public.

5 unit tests in test/diff.test.ts (now 84 in npm test, all passing), pinning that unchanged files are not differences — a gate fed an unfiltered compareTrees result must not fire on a project that matches its remote exactly — and that failure stays distinguishable from differences.

8 e2e cases in test/e2e.sh: clean tree, a difference, the unchanged-without-the-flag control, --file narrowing in both directions, the redirected-output case, and both failure statuses. The two failure cases need no network.

Also exercised by hand against a real project: clean pull → 0; an edited file → 1 for plain, --name-only, --file and --latexdiff; --file on an unchanged file → 0; bad flags and a missing directory → 2 with the flag and 1 without. --pdf was not run against anyone's project.

Docs

README.md gains a CI-gate section with the status table and a workflow snippet, SKILL.md a line in the diff section and a tip, and the --help text explains the 2. The changelog entry records why the status is set rather than exited on, since that is the part most likely to be "simplified" later.

Refs #48

Follow-up to aloth#45, left open when the core diff command shipped in 0.10.0.
A unified diff is the right artifact for a developer and the wrong one for
a thesis advisor, who expects the revision marked up in the document.

--latexdiff runs on the two sides diff has already fetched, so the markup
means what the patch output means and costs no extra request. --pdf
compiles it with Overleaf's compiler, so a reviewable PDF needs no local
TeX installation.

Root document detection, argument construction, output naming and failure
interpretation are functions over data in src/latexdiff.ts, unit-tested
with no Overleaf account and no latexdiff binary.

Refs aloth#55
Second follow-up left open by aloth#48, alongside --latexdiff. Reports diff(1)'s
statuses: 0 when nothing differs, 1 when something does, 2 when the run
itself failed.

The 1-vs-2 split is the whole feature. A pipeline that cannot separate "the
project differs" from "the run failed" reads an expired session cookie as a
content change, and a job that goes red for the wrong reason sends whoever
reads the log looking for a diff that was never computed. All ten failure
paths in the command move to 2 together, so no failure can be mistaken for
a difference. Without the flag they stay 1, as in every other command, so
existing scripts are unaffected.

The status is set rather than exited on. process.exit discards whatever is
still buffered on a non-TTY stdout, and `olcli diff --exit-code > patch.txt`
is exactly a large patch going into a pipe: exiting outright truncated a
1.5 MB patch to the 128 KB pipe buffer here, losing 91% of it mid-hunk.

The gate covers whatever was compared, so --file narrows it the way a
git diff --exit-code pathspec does, and under --latexdiff it reports on the
project rather than on the markup - a changed figure is a real difference
even though a marked-up root document cannot show one.

Refs aloth#48
@aloth

aloth commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Built and tested locally: npm ci, lint and build clean, 84 tests passing. Reviewed 86b078b on its own, since the branch carries #56 as well.

The 1-vs-2 split is the part that makes this worth having, and moving all ten failure paths together is what keeps it honest. A gate that reports an expired cookie as a content change is worse than no gate, because the job goes red and the log sends you looking for a diff that was never computed.

Setting exitCode rather than calling process.exit is the detail I would have missed. Measuring it instead of asserting it - 1.5 MB truncated to the 128 KB pipe buffer, 91% gone mid-hunk - is the difference between a claim and a finding.

Stacking this on #56 rather than cutting it from main was the right call, and saying so at the top of the PR is what made it cheap to review.

@aloth
aloth merged commit f8b960e into aloth:main Sep 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants