feat(diff): content-level diff between local files and the remote project - #48
Conversation
…ject `push --dry-run` answers which files would change; there was no way to see what changed inside them. `olcli diff` prints a unified diff of the local tree against the project's current contents. The remote side is fetched fresh on every run. `.olcli.json` records remote paths, never remote contents, so there is no stored snapshot to diff against - comparing "against the last pull" would have meant inventing a content cache rather than reusing one. Fetching fresh is also what makes the diff describe what a subsequent push will overwrite, which is the question the command exists to answer. It costs one request: downloadProject returns the whole project as a single archive, the same call pull and sync already make. `a/` is the remote and `b/` is local, so `+` is content push would upload and `-` is content it would overwrite. Binary files are reported as differing without a patch. Both sides pass through the same ignore layers and dotfile rule, so artifacts sitting on Overleaf are not reported as local deletions. `diff --name-only` and `push --dry-run` deliberately answer different questions: push selects by modification time, diff by content. What they do share - the local walk-and-filter loop, which push and sync each had their own drifting copy of - moves to src/scan.ts. `push --dry-run` now says what it measures and points at `olcli diff`. Comparison, rendering and remote-tree filtering are pure functions in src/diff.ts, unit-tested without an Overleaf account. New dependency `diff` has no dependencies of its own. latexdiff integration follows separately. Refs aloth#45
b73a038 to
14d848e
Compare
Closes #46. The repository had no CI for pull requests. Only publish.yml existed, triggered by tags, so a change was first executed by a machine other than the author's at release time. #48 is a 908-line contribution from outside, and reviewing it without an automated build was the immediate reason to fix this. The new workflow runs on pull requests and on pushes to main: npm ci, lint, build, test, then a check that the build actually produced something. `npm ci` rather than `npm install`, so a pull request whose lockfile disagrees with package.json fails instead of silently resolving something else. The matrix is Node 18 and 24. 18 is the floor declared in engines, 24 is what publish.yml releases with. Testing only one of them would let through a release that satisfies neither its own engines field nor its own publish path. The final step verifies dist/cli.js, dist/mcp.js, dist/remote-helper.js and dist/index.js are non-empty and that `node dist/cli.js --version` runs. tsc exiting zero does not prove entry points were emitted, and dist is the entire published package. test/e2e*.sh are deliberately not run. They drive a real Overleaf account through login, pull, push and compile against a live project. `npm test` globs test/*.test.ts, so the shell suites are already outside it, and the workflow says so in a comment to keep someone from widening the glob later. eslint.config.js is flat config for eslint 9, with typescript-eslint recommended and without type-checked rules. Those need a TypeScript program per run and would turn lint into a second type checker with its own opinions; tsc already runs in CI and is the authority there. `no-explicit-any` is a warning rather than an error, and the count is the reason. 58 occurrences remain, all pre-existing, concentrated in client.ts and cli.ts where untyped JSON comes back from Overleaf. Overleaf publishes no schema for those responses, so each one is a typing decision rather than a mechanical fix. As an error, CI would be red on main from the day it is switched on, which teaches everyone to ignore it. As a warning it stays visible and blocks nothing. Worth revisiting once the count is small enough to clear in one pass. Verified locally: lint, build and test each exit zero, and the workflow parses as YAML.
Resolves the conflicts introduced by three commits that landed on main after this branch was opened: dead-code removal, the new pull request CI with a working lint setup, and the engines correction to Node >=20.18.1. src/cli.ts: both sides of the conflict were unused imports. This branch imports DEFAULT_IGNORE_PATTERNS, IgnoreContext and getLastProject but references none of them outside the import statement, and main had already removed them for exactly that reason. main in turn still imported shouldIgnore and buildTexSiblingSet, which this branch replaced with scanLocalFiles. Resolved against what the merged file actually uses: loadIgnore, four references, and nothing else from ignore.js. CHANGELOG.md: both sections kept as they are. The [Unreleased] entry from this branch and the [0.9.2] entry from main describe different work and are consolidated into a single release entry separately, not here. package-lock.json: regenerated rather than resolved by hand.
|
Thanks for the quick turnaround on this. I pushed a merge of What landed on
How the three conflicts were resolved:
Two notes, neither affecting this PR: The CI is new, and this PR is the first thing to run through it. Both legs pass, Node 20.18.1 and 24. The Beyond the 34 unit tests, the full e2e suite ran against a real Overleaf project: 79 of 79, including the Diff section you added. Merging this now and shipping it as part of 0.10.0. |
#48 landed with its own [Unreleased] section while main carried a [0.9.2] section that was written but never tagged. Neither shipped on its own, so both become one entry. 0.10.0 rather than 0.9.2, because `engines` moved from `>=18` to `>=20.18.1`. That is a breaking change for anyone still on Node 18, even though the runtime never actually worked there, and a patch number would understate it. Waynting gets a Contributors line for `olcli diff`, following the convention already used in 0.4.0 and 0.6.0. The merge commit records the code authorship; this records the credit where the release notes are read.
`olcli diff` landed in #48 as a CLI command only. The MCP server exposes 17 tools covering pull, push, compile and comments, but nothing that answers "what would a push change" - so an assistant either shells out to the CLI or guesses. This closes that gap in a separate commit, because src/diff.ts and src/scan.ts only exist on main after the merge and the tool could not have been written before it. Returns structured data rather than one block of diff text: one entry per changed file with path, status, binary and a unified patch. The other 17 tools return JSON, and an agent should be able to filter by status without parsing output. `name_only` drops the patch text, `file` restricts to a single path, `context` sets hunk width, and the two ignore switches mirror the CLI flags. Semantics follow the command exactly, which matters more than the shape. The remote is fetched fresh on every call, one request via downloadProject, and `remote_fetched_at` is part of every response: a collaborator editing between the call and a later push can still change the outcome, and a tool that hid that would describe something other than what its own name promises. Both sides pass through the same ignore layers, so a freshly pulled directory reports no differences instead of listing every build artifact on Overleaf as a deletion. Remote-only files carry `removed_only_by_push_delete`, since a plain push does not remove them and calling them "deleted" without that flag would misdescribe the effect. Verified over a real MCP handshake rather than by reading the source: initialize followed by tools/list returns 18 tools including diff_project, with project_id and local_dir required. Documentation was also behind reality, independently of this change. docs/MCP.md listed 17 of 18 tools and SKILL.md listed 15: rename_project and plan_project_renames had been missing since they were added. Both files now match the registrations, checked by extracting the server.tool() names and comparing rather than by reading.
Two corrections to the 0.10.0 release notes. #46 was attached to the CI bullet. The issue reports that `npm run lint` fails because eslint is not a dev dependency; the CI workflow was added alongside the fix, not asked for by the issue. The link moves to the lint entry. The contributor line carried both #45 and #48. Existing entries use one link per contribution and #45 is the proposal for the work #48 delivers, so the PR alone is the reference. Multiple links there mean multiple separate contributions, as in the bicheTortue entry.
Resolves the conflict introduced by ten commits that landed on main after this branch was opened: the diff command from aloth#48, dead-code removal, pull request CI with a working lint setup, the engines correction to Node >=20.18.1, and the 0.10.0 release. CHANGELOG.md was the only conflict. Both sections are kept as they are: the Unreleased entry from this branch and the [0.10.0] entry from main describe different work, and consolidating them into a single release entry is a separate step. src/cli.ts and src/client.ts merged cleanly despite both being touched on main, because this branch adds new blocks rather than editing existing ones.
Project creation via CLI and the programmatic API. Author: @mohamedsobhi777. The merge commit on the branch resolves the CHANGELOG conflict with ten commits that landed on main after the PR was opened, including the diff command from #48, pull request CI, and release 0.10.0. Verified before merging: 38 unit tests, lint and build clean, CI green on Node 20.18.1 and 24, and `olcli project create --help` resolves on the merged state.
Draft PR for the core
olcli diffcommand from #45, split as you asked — nolatexdiff, no--pdf, nothing that shells out or touches the compiler.The design question you raised: what is "remote"?
difffetches the remote fresh on every run, and says so. The reasoning turned out to be settled by the data model rather than by preference:.olcli.jsonrecords remote paths (remoteManifest,pushManifest), never remote contents. So "compare against the last pulled state" was not the cheap-and-stable option it looked like in my proposal — there is no snapshot stored to compare against. It would have meant designing and maintaining a content cache, which is a much larger change than the command itself.Fetching fresh is also the semantics that makes the command honest: a diff that does not describe what the push will overwrite is worse than no diff, as you put it.
I should also correct my own sketch in the issue. I wrote that it would "only fetch remote content for files that differ" — that is not possible, since you cannot know which files differ without fetching them. It is also unnecessary:
downloadProjectreturns the entire project as a single archive, the same callpullandsyncalready make. Fetching fresh costs one request, not one per file.The limit is stated rather than hidden: a collaborator editing between
diffandpushcan still change the outcome, so every run prints the fetch time.--name-onlyvspush --dry-runThey answer different questions and should keep disagreeing:
push --dry-runlists files whose modification time is newer than the last pull, because that is whatpushactually uploads.diff --name-onlylists files whose contents differ.A file you touched without editing appears in the first and not the second. Merging the behaviour would make one of them lie. Verified on a real project —
push --dry-runlisted 3 mtime-selected files whilediffreported 4 differing ones, including a remote-only filepushwould not have touched at all.What they genuinely do share is the local walk-and-filter loop. That existed twice already, once in
pushand once insync, and the two copies had drifted (syncguarded against a missing directory,pushdid not).diffwould have made three, so it moves tosrc/scan.tsand all three call it. Same reasoning asrename-plan.tsin 0.9.0.For the discoverability side of the overlap,
push --dry-runnow says what it measures:What the command does
a/is the remote andb/is local, so+is contentpushwould upload and-is content it would overwrite. Reading it the other way round would invert the meaning of every hunk, so the orientation is pinned by a test.Binary handling is exactly what you asked for — reported, never guessed at:
Detection is a NUL byte in the first 8000 bytes, git's heuristic. No per-extension sniffing.
Two things worth flagging because they are decisions, not details:
output.pdfand every stray.auxsitting on Overleaf as a local deletion on every single run. A freshly pulled directory now diffs to "No differences", which is the check that this filtering actually agrees.push, since onlypush --deleteremoves them. Calling them "deleted" without that line would misdescribe what would happen.Archive entries whose names escape the target directory are dropped, consistent with what
pullrefuses to extract (#44).diffwrites nothing, but an entrypullwill not extract is not part of the project as far as that directory is concerned, and showing it would suggest a difference no push could resolve.Dependency
diff@^9.0.0.npm view diff dependenciesis empty — no transitive tree.Structure and tests
Comparison, rendering and remote-tree filtering are pure functions in
src/diff.ts, so they are unit-tested with no Overleaf account and no network: 34 tests acrosstest/diff.test.tsandtest/scan.test.tsunder the existingnpm test. Following therename-plan.tsprecedent, none of it is re-exported from the package root — happy to export it if you would rather it were public.test/e2e.shgains a Diff section between the pull and push tests: clean tree diffs to nothing, a modification shows asMwith the right hunk, a local-only file shows asA, build artifacts are ignored on both sides, and the tree is restored and re-verified clean so the push tests below are unaffected.Also exercised by hand against a real 26-file project: clean pull → no differences; then an edited
.tex, a new file, a deleted figure and a corrupted PDF → all four statuses correct, withpush --dry-runandsync --dry-runstill behaving as before thescan.tsextraction.Follow-ups, not in this PR
--latexdiff/--pdf, as agreed--exit-code, if you wantolcli diffusable as a CI gateRefs #45