Developer workflow: dependencies as dependencies, comparing a git revision, and finishing the merge - #52
Merged
Merged
Conversation
Phase 1, commit 1 of specs/2026-08-09-developer-workflow. Four ecosystems, one shape out: package-lock.json (v1's nested tree and v2/v3's flat `packages` map), pnpm-lock.yaml, yarn.lock v1 and go.sum. No package manager is consulted and no registry is contacted — every fact reported comes out of the file in front of the reader. The interesting field is `direct`. npm v2/v3 and pnpm record which packages the project actually ASKED for (the root entry, the importers block); yarn v1 and go.sum do not, so those report knowsDirect: false and claim nothing rather than guessing. npm v1 is in that group too: its top level is the HOISTED tree, so depth 0 does not mean direct. Matching a direct name against every install path was wrong, and this repo's own lockfile caught it: a package installed TWICE — once at the root and once nested under a dependency wanting another version — was direct at both paths, reporting 33 direct against a manifest declaring 30. @emnapi/core, @emnapi/runtime and globals are each installed twice here. Only `node_modules/<name>` counts now, and the nested copy is the transitive dependency it actually is. A lockfile is untrusted input (rule 6): the parse is capped at 50k entries, malformed content returns null rather than throwing, and pnpm's YAML is read with the same maxAliasCount guard structuralDiff uses so an anchor bomb cannot expand. Verified against this repo's own package-lock.json: 783 packages, 30 direct — exactly its 11 production plus 19 dev dependencies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1, commit 2 of specs/2026-08-09-developer-workflow. PHP's lockfile is the odd shape of the five: two ARRAYS rather than a map, and `packages` vs `packages-dev` is how it records dev — so `dev` is known here even though `direct` is not. What composer.json asked for lives in composer.json, which is a different file, so knowsDirect is false and nothing is guessed. A licence is a LIST in this ecosystem, because a package may be dual-licensed; both are shown rather than the first one winning. `v1.2.3` and a `dev-main` branch pin are both real versions here and both survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1, commit 3 of specs/2026-08-09-developer-workflow. diffLocks classifies every package that moved as added, removed, bumped or downgraded, carries the semver step of each, and splits the count into what the project ASKED for and what came along with it. Rows sort direct first, because that is the part a reader can act on. Three things it refuses to get wrong: - A name can be installed at two versions AT ONCE. Comparing a version per name would invent a bump that never happened, so each side is a SET of versions per name; what both sides hold did not move, and only the leftovers pair up. - `1.10.0` is not older than `1.9.0`. Direction comes from the parsed triple, never from the string. - Below 1.0.0 the MINOR is the breaking position, and a lockfile is full of packages that never left 0.x — so 0.1.0 to 0.2.0 reports as major. `knowsDirect` is only claimed when BOTH sides recorded it; one side guessing is the same as not knowing. Validated against this repo's own history: package-lock.json between v0.4.10 and HEAD is 15 insertions and 13 deletions of text, which this reads as 4 packages bumped, one of them direct — mermaid 11.16.0 to 11.16.1, a patch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1, commit 4 of specs/2026-08-09-developer-workflow. A lockfile IS json, so it must not go through resolveAdapter and hide the raw file. It becomes a semantic KIND instead — semanticKind() gains 'deps' — which keeps the text and structure views one toggle away and reuses the routing diagram and grid already use. Deps outranks tree deliberately: json is what a lockfile is, but a 780-key structural tree is not what anyone opened it for. Both sides must parse AND be the same ecosystem, so a package-lock against a go.sum is refused rather than compared as if it meant something. Two extractions, forced by diffStore sitting exactly on its 747-line ratchet and paid for rather than waived: - lockPairOf moves to utils/lockfile/lockPair.js, so the store holds the question and the util holds the work - structureDiff's body moves to utils/structurePair.js for the same reason — pure comparison plumbing that was never store logic, and it takes structureAdapter and diffStructures out of the store's imports Also fixes an orphaned comment: the six lines describing comparableKind sat above semanticKind, having been left behind when that getter was split out. They now sit above the getter they describe. The store gained no net lines. The cap was not raised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1, commit 5 of specs/2026-08-09-developer-workflow. One row per package that moved, what the reader asked for first, and the carried packages folded away until wanted — they are the bulk of any lockfile move and rarely why anyone opened it. Status is the row's LEFT EDGE rather than a wash: a row already carries a red version and a green one, and a third tint behind them reads as neither. Row height comes from --control-h and the chips from --chip-h, so a long package name cannot make one row taller than its neighbours. Every colour is a role the diff panes already use. The semver step is rendered from a map of LITERAL key ids rather than a key built from the value: a template-assembled key is invisible to check:i18n, which caught it. Driven against two real lockfiles from this repo's own history — 10,222 lines against 10,224 — and it renders one row: mermaid, asked for, 11.16.0 to 11.16.1, patch. Three carried packages behind the fold. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1, commit 6 of specs/2026-08-09-developer-workflow — phase 1 closes. The seed pair lives in two DIRECTORIES, not two filenames: a lockfile is recognised by its exact name, so `package-lock.before.json` would never have been read as one. Two checkouts is also what the real situation looks like. e2e/deps.spec.mjs drives the whole path through a real launch — main reads the file, the name routes it, and the view that takes over is neither Monaco nor the structure tree. It asserts the fold (one asked-for row, two carried behind a press) and that the text is still one toggle away, because the point is to summarise the file, not to hide it. README gains a Dependencies row; the glossary gains lockfile and the direct/transitive distinction. npm run check green: 3132 tests, coverage 95.33/88.36/95.80/96.41. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2, commit 1 of specs/2026-08-09-developer-workflow. READ ONLY, and the vocabulary is two subcommands — `rev-parse` and `show`. Nothing here writes, stages or commits, and no subcommand that can reach the network is reachable from it, which is what keeps rule 1 intact: a subprocess that cannot open a socket does not weaken the offline guarantee. The fence, because a repository someone cloned is untrusted input and repo-local config has been an execution vector before: - execFile with a FIXED argv, never a shell - the repo root is computed HERE, never accepted from the renderer - a revision is validated against a narrow pattern and can never begin with `-`; --end-of-options catches anything that somehow did - a path may not be absolute and may not contain `..` - every invocation carries core.hooksPath=, core.fsmonitor= (a command git will SPAWN), core.editor=true and protocol.ext.allow=never The environment needed a second pass. The comment claimed a clean one while the code spread process.env wholesale, which keeps GIT_DIR, GIT_INDEX_FILE and GIT_ALTERNATE_OBJECT_DIRECTORIES — each of which points git at something other than the repository main chose. gitEnv now drops every inherited GIT_*, and three tests hold it there. Proven against this repository: the root resolves, HEAD~1 resolves to a 40-character sha, `show v0.4.10:package.json` returns the manifest as it stood at that tag — version 0.4.9, because the version-sync commit lands after the tag — and an unknown revision fails with git's own message rather than a crash. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…h grammar Phase 2, commit 2 of specs/2026-08-09-developer-workflow. Two IPC handlers and no more: git:root asks whether a path is inside a repository, git:show reads one file as it stood at one revision. The renderer names a REVISION and a path — never a command, never a cwd, never a git argument — and main resolves the repository FROM the file it was given, so a renderer cannot point git at a directory of its choosing. A refusal inside the fence (an unsafe revision, a path climbing out) comes back as `refused` without the reason: the renderer does not need to learn which of its inputs the fence disliked. splitRevisionArg is the CLI half, and it is pure so the grammar is testable without a repo. Only the FIRST colon separates, so a path may contain its own; `C:/Users/x/a.js` stays a path, because a single letter before the colon is a Windows drive and not a revision; and a revision the fence would refuse never becomes one here either. Blobs are capped at 32 MB — the point where the streamed reader takes over anyway, and a revision is not a reason to hold more in memory than a file would be. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2, commit 3 of specs/2026-08-09-developer-workflow — phase 2 closes. `diffbro compare HEAD~1:src/app.js src/app.js` now works: parseCompare recognises the revision:path shape and leaves it unresolved, and the launch path turns it into a real file BEFORE the renderer hears about the command at all. Everything downstream sees two ordinary paths and nothing else has to know git was involved — the same shape gitTool's difftool launcher already uses, writing into the same temp prefix so one sweep clears both. A failure refuses the whole comparison rather than opening half of it, and says which of the three things went wrong on stderr. Those messages come from a map of literal key ids, not a key built from the error code — check:i18n caught the assembled version, as it did in phase 1. e2e builds a real two-commit repository, drives the real CLI against it, and asserts the diff shows 3 becoming 9 — the old side read out of git, the new one off disk. A second test proves an unknown revision opens nothing at all. docs/security.md gains the fence beside the offline guarantee, because that is the rule a subprocess comes nearest; docs/ipc-security.md gains the two handlers; README's Terminal row gains the grammar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3, commit 1 of specs/2026-08-09-developer-workflow. A file as git leaves it mid-merge: stable text with conflict regions between, each carrying both sides and — in diff3 style — the ancestor that says which side actually changed. Pure, and it COMPOSES the resolved file rather than editing text in place, so nothing can half-apply. Two refusals it makes deliberately: - Markers that do not close return null. A file someone was editing by hand is not a conflict file, and guessing which side the remainder belongs to would silently drop the other. - composeMerge returns null while any region is undecided. Writing a half-resolved file would hand git one with markers still in it, which is worse than not writing at all. The file's own shape survives: CRLF stays CRLF and a trailing newline stays, because a merge tool that reformats the file it resolves is a merge tool nobody trusts twice. Empty stable runs are dropped — a file that opens on a conflict has nothing above it, and an empty segment is not something anyone renders. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3, commit 2 of specs/2026-08-09-developer-workflow. This is the line docs/roadmap.md parked as "a decision, not code", and it is crossed deliberately: the app already registered as `git mergetool`, so it already took the job. It is now able to finish it. The write is as narrow as the surface can make it. Main remembers the $MERGED path from the LAUNCH argv; the renderer sends the resolved TEXT and nothing else, so there is no argument through which it could name a file — the same shape clipboard:writeFile uses. With no mergetool launch there is no path, so merge:write does nothing at all whatever arrives, and a session is spent once used: a second write would be a second file. Six tests hold that, including the two that matter — nothing is written without a launch, and a non-string is refused rather than coerced. `diffbro mergetool LOCAL REMOTE MERGED` is the verb git calls, in git's own argument order. Routing it also collapsed the verb ladder in routeCommand into a table, which is what let clipboard-save and the two new arrivals sit side by side under the complexity cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 3, commit 3 of specs/2026-08-09-developer-workflow. The merge view is a BaseDialog over the two sides git handed over: one card per conflict, four choices each — ours, theirs, both, neither — with "ours everywhere" and "theirs everywhere" for the common case. Save is disabled until every conflict has an answer, because a half-resolved file still has markers in it. trustExitCode flips to true, and the reason it may is the new launcher. The app is single-instance, so a launch returns as soon as the running window has been told; a script that exited there would tell git the merge was resolved before anyone had looked at it. gitMergeTool.js polls $MERGED's modification time and only then exits 0. That is what makes the flag honest rather than optimistic. An existing test asserted trustExitCode=false and had to change — it encoded the old contract, that Diff Bro never writes $MERGED. Its replacement says why the answer is now true, and two more hold the wait itself. The extraction of gitMergeTool.js was forced by gitTool.js's 250-line cap and then by a real import cycle the structure guard caught: the shared MARK now lives in its own module, so neither launcher imports the other. e2e builds a repository with a genuine merge conflict, drives `mergetool` against it, resolves it in the dialog, and asserts the file git is left holding is `one/theirs/three` with no markers — and that Save refuses while anything is undecided. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…gises Phase 3, commit 4 of specs/2026-08-09-developer-workflow — the spec closes. docs/security.md gains "Writing a merged file", because a write over a file the user already had deserves its own section rather than a footnote in the IPC table; ipc-security.md gains merge:write beside the git pair. The Settings copy is corrected rather than left standing. It used to say "Diff Bro doesn't write the merged file, so git still asks you whether the merge worked" — that sentence was the evidence for this whole phase, and it is no longer true. The roadmap gains a Developer workflow track and "Comparing more" loses three-way merge to it. The board is hand-authored, so it was edited in the same change and rendered offscreen to check: the new card collided with the rail and had no footer, which only a render shows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two agents read and drove this branch. Four blocking findings, two of them verified data loss with bytes on the table. BLOCKING — a binary conflict was destroyed by one click. git calls the mergetool for binary conflicts too and leaves them with no markers. Reading $MERGED as UTF-8 turned ff fe into ef bf bd ef bf bd; zero conflicts read as "nothing left to decide", which ENABLED Save, and trustExitCode=true then told git it worked. mergeGuards.js refuses a NUL in the first 8 KB — the same sniff files.js already uses — and refuses a file with no markers at all, because that is not "already resolved", it is a file this tool has no business rewriting. Save is additionally disabled when there is nothing to decide. BLOCKING — a mixed-EOL conflict wrote the markers back. git writes LF markers into a CRLF file; picking one line ending for the whole file found ZERO conflicts in one, told the user there was nothing to resolve, and wrote the unresolved file back. Every line now keeps the ending it arrived with, so the round trip is byte for byte. BLOCKING — git:show was an arbitrary-file-read primitive. It took a renderer-supplied path, used its dirname as git's cwd, and bypassed the allowlist files.js exists to enforce — for anything committed in any repository on the machine. It was also DEAD: nothing called it, because the shipped feature is the CLI. Deleted, along with the doc rows that asserted a fence it did not have. BLOCKING — `git mergetool` opened with both panes empty. The merge route returned before allowCliPath, so file:read refused both sides; the refusal is a resolved value, not a throw, so the user was not even told. HIGH — cancelling left the write armed and hung git. endMerge ran only after a successful write, so an abandoned session kept $MERGED live for the process lifetime and a later write still landed. Cancel now spends the session and releases the launcher. The launcher watches a SENTINEL rather than $MERGED's own mtime: a resolution writing identical bytes changed neither size nor timestamp, and `ls -l` only resolves to the minute — either way it waited for ever. It is bounded now too. MEDIUM — version text failed the 4.5 floor on 14 of 20 themes (nord 1.80) and the bumped keyline was invisible on 8 light grounds (contrast 1.35): raw --danger-border/--success-text are 3:1 NON-TEXT roles, and --warning-bg is a background role. Both now use the --dg-* diff roles and the color-mix ui.css already uses for the status band. The deps viewer is registered in theme-sweep's SURFACES, so this is measured next time rather than reviewed. Also: lockDiff sorted versions as STRINGS, pairing 1.10.0 with 1.9.0 and reporting a downgrade that never happened; registerGitTool clobbered a pre-existing merge launcher the difftool target was guarded against; a refused revision path failed silently into the crash log; DepsStatusBand dereferenced a field its validator did not require; mergeInvocation was byte-identical to invocation; four comment walls restating docs and one stale claim about `--` that the code never used. Red-verified: reverting the two guards makes the new e2e fail with the dialog open on a binary file. npm run check green — 3201 tests, coverage 95.01/88.03/95.15/96.08. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI's Linux e2e shard caught what the macOS host did not: `diffbro
compare HEAD~1:app.json app.json` opened nothing at all, and said
nothing either.
Staging the blob was the wrong shape. It landed in TMPDIR, and `file:read`
refuses anything under userData however it was obtained — which is where
E2E puts TMPDIR. The renderer's readFile then answered {error:
'not-permitted'}, a RESOLVED value, so compareFromCli's try/catch never
fired and no notice was raised: the same silent-refusal shape the audit
found in the merge route.
Fixing the allowlist would have been fixing the symptom. A temp copy has
to live somewhere the renderer may read, and it would then outlive the
comparison with nothing to tell it the tab had closed — the staging and
its sweep were both solving a problem that need not exist. Main already
holds the content; it hands over a loaded FILE, exactly as the merge
command hands over the conflicted text. No temp directory, no sweep, no
allowlist entry.
The side is named `app.json @ HEAD~1` now, so two revisions of one file
are told apart in the slot and the tab title.
Also: deps.spec hand-rolled the View menu click, which misses when the
toolbar folds View into the overflow at the container's window width. It
uses the setViewOption fixture, like every other spec that touches it.
Verified in the Docker Linux env, which is where both failures
reproduced: all 8 tests across the three new specs green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three findings from a developer-experience investigation, in one spec (
specs/2026-08-09-developer-workflow/plan.md), each landed in its own commits. The thesis is the one the app already lives by — compare meaning, not lines — pointed at the three artifacts a developer actually spends the day on.1 · Dependencies as dependencies
A lockfile pair reads as the packages that moved, not the four thousand lines it is written in: added, removed, bumped or downgraded, the semver step of each, and — the part that matters — which of them you asked for rather than got carried along. Five ecosystems:
package-lock.json(v1 and v2/v3),pnpm-lock.yaml,yarn.lock,go.sum,composer.lock. Nothing is fetched; every fact comes out of the file in front of you.Formats that do not record which packages were asked for say so rather than guessing. Verified against this repo's own lockfile — 783 packages, 30 direct, exactly its 11 production plus 19 dev dependencies — and against its history:
package-lock.jsonbetween v0.4.10 and HEAD is 15 insertions and 13 deletions of text, which this reads as 4 bumped, one of them direct: mermaid 11.16.0 → 11.16.1, a patch.2 · Comparing against a revision
diffbro compare HEAD~1:src/app.js src/app.js. The old side comes straight out of the repository, so you never have to produce a copy first.Behind a real fence, because a repository someone cloned is untrusted input:
execFilewith fixed argv and never a shell;rev-parseandshoware the whole vocabulary, so nothing that reaches the network is callable; the repo root is computed in main; revisions are validated and followed by--end-of-options;core.hooksPath=,core.fsmonitor=andprotocol.ext.allow=neveron every invocation; and every inheritedGIT_*is dropped, becauseGIT_DIRand friends point git somewhere else entirely.3 · Finishing the merge
gitTool.jshad registered DiffBro as git'smergetooland then admitted, in its own settings copy, that it could not do the job: "Diff Bro doesn't write the merged file, so git still asks you whether the merge worked."It writes it now. This crosses the documented "DiffBro never writes files" line, deliberately and at the maintainer's instruction — the argument being that the app had already taken the job. The write is as narrow as the surface can make it: main holds the
$MERGEDpath from the launch argv, the renderer sends resolved text and never a path, and one launch permits one write.trustExitCodeflips totrue, which is honest only because the new launcher waits for a sentinel before exiting.The audit caught four blocking bugs
Two agents — one reading, one driving the app — returned do not ship. All fixed in
fa16f2b:$MERGEDas UTF-8 turnedff feintoef bf bd ef bf bd, zero conflicts read as "nothing left to decide" so Save was enabled, andtrustExitCodethen told git it worked.git:showwas an arbitrary-file-read primitive bypassing thefiles.jsallowlist — and dead code. Deleted, with the doc claims that asserted a fence it did not have.git mergetoolopened with both panes empty, because the merge route returned beforeallowCliPath.Plus: cancelling left the write armed and hung git; version text failed the contrast floor on 14 of 20 themes (nord 1.80) and the bumped keyline was invisible on 8 light grounds (contrast 1.35); lockDiff sorted versions as strings and called 1.10.0 → 1.9.0 a downgrade.
Verification
npm run checkgreen: 3201 tests, coverage 95.01 / 88.03 / 95.15 / 96.08 over the 93 / 86 / 92 / 95 floors, theme depth ok on 20 themes.gitTool.jswas split when it hit its 250-line cap,diffStore.jsstayed exactly on its 747 by extractingstructurePair.js, and a real import cycle was broken with a shared constant rather than baselined./bin/shonly), and a fullgit mergetoolthrough a globally-registered launcher, which would mutate the developer's git config.🤖 Generated with Claude Code