feat(snippets): compare snippets by dragging them into the diff pane

[mindaugaskasp]

Spec: specs/2026-08-02-compare-snippets/plan.md (11/11 steps).

A sidebar row becomes a drag source; the window drop resolves it and hands the result to the same dropFiles the file path uses.

Why it is small

_place already accepted an object source and receive assigns the whole object to the side, so { path: null, name, content, snippetId } was already a valid side — the same shape paste mode produces. Routing through dropFiles therefore inherits, with no new code: one snippet fills a side and waits, two fill both, and dropping onto a complete unsaved comparison raises the existing replace guard. "Just like any ordinary diff view" needed no work at all — save, clear, export and tabs already apply.

Staying live

A side keeps its snippetId; a watcher re-reads when the snippet changes and feeds back through receive(), which already sets diffSaved = false. So an edit updates the pane and correctly re-dirties the comparison rather than leaving it claiming to match what was saved. The library is never written to — the flow only ever calls load().

Security

• Only the id travels. A drop target outside the app can read a DataTransfer, so the body never goes on it. Asserted by stringifying the whole payload and checking the snippet name appears nowhere — a stronger claim than "we only set the id field".
• The payload is untrusted. dragIdsFrom checks the type before reading, parses defensively, caps at 2 ids of ≤200 chars, and ids are resolved against the store.
• Secret snippets are refused in three places — the row is not draggable, startDrag cancels, and dropSnippets refuses again on arrival, so the guard is not UI-only.

Bug found and fixed while building

The watcher was keyed on updatedAt, a millisecond timestamp — so a snippet created and edited inside the same millisecond left the key unchanged and the pane silently stale, which is precisely the failure the feature exists to prevent. Keyed on the ciphertext iv now: fresh on every re-encrypt, so it changes exactly when the content did.

Verification

• npm run check — 1895 passed, coverage floors unchanged.
• Docker e2e — compare-snippets.spec.mjs, 4 passed, including the live-edit link, driven with a real DataTransfer shared by dragstart and drop.
• Unit: 9 (snippetSource) + 6 (useSnippetDrag) + 5 (useSnippetDiffSync) + 6 (dropSnippets), each written first and watched failing.

For the reviewer

An earlier e2e failure was traced to the test, not the code — it drove Edit → .editor in the view dialog, but Edit opens a separate Edit Snippet dialog and a previewed snippet (claude, mermaid) renders no raw editor. Confirmed pre-existing against a stashed tree.

🤖 Generated with Claude Code

[diff-bro-reviewer]

Changes requested

The routing through dropFiles is the right call and the security posture is sound — id-only payload, three-layer secret refusal, defensive parsing. One real bug, and it is one the new e2e provably cannot catch.

🔴 1 · The drop overlay will never appear for a real snippet drag

onDragEnter decides via carries(e) → snippetIds(e) → dragIdsFrom → transfer.getData(DRAG_TYPE).

Per the HTML drag-and-drop spec, the drag data store is in protected mode during dragenter and dragover: types is readable, but getData() returns the empty string. Data only becomes readable on drop.

So in a real browser drag:

• snippetIds(e) is [] on dragenter
• carries(e) is hasFiles(e) || false, which is false for a snippet drag
• onDragEnter returns early → active never goes true → no overlay
• snippetDrag is never true → the new "Drop a snippet to compare" copy is dead code

The drop itself still works, because onDrop reads after protected mode ends. So the feature functions while its only affordance never appears — the user gets no indication the drag is valid.

Why the e2e misses it: compare-snippets.spec.mjs constructs its own DataTransfer and dispatches synthetic DragEvents. A synthetic event's data store is not in protected mode, so getData works there and the test passes. This is the exact case where a green e2e is not evidence.

Fix: gate dragenter/dragover on types alone — Array.from(e.dataTransfer?.types ?? []).includes(DRAG_TYPE) — and keep dragIdsFrom for drop. Worth a unit test that asserts the enter-guard never calls getData.

🟠 2 · snippetDrag is never reset

onDragLeave and onDrop clear active and depth but leave snippetDrag true. Drag a snippet out without dropping, then drag a file in, and the overlay reads "Drop a snippet to compare" for a file drag. One line in each.

✅ Otherwise verified

• Only the id travels; the test asserting it by stringifying the whole payload is the right shape of assertion.
• Secret refusal at three layers, with the store-side check meaning the guard is not UI-only.
• dropSnippets reuses dropFiles, so the replace guard and one-then-wait cannot drift.
• onDrop was split when it crossed the complexity cap rather than the cap being raised.
• Watcher keyed on the ciphertext iv rather than a millisecond timestamp — the right fix for the bug the description records.

[diff-bro-reviewer]

Both findings resolved — approving

Re-checked against the diff.

1 · protected-mode guard — isSnippetDragType reads types only, and both carries() and snippetDrag now use it; dragIdsFrom is left for drop, where the payload is legitimately readable.

The test is the part worth noting: rather than asserting the outcome, its stub throws if getData is called during enter. That pins the shape of the fix, not just its effect — the guard cannot silently regress into reading the payload again, which is exactly how this bug would come back.

2 · snippetDrag reset — cleared in both onDragLeave (at depth 0) and onDrop.

npm run check green; Docker e2e 4 passed after the change.

Carried forward

The e2e still drives synthetic DragEvents, so it exercises the drop path but not protected mode. That is a real limit of the harness rather than a defect — Playwright cannot produce a native OS drag here — and the unit test now covers the gap it leaves. Worth remembering the next time a green drag e2e is treated as proof.

Approved.

[diff-bro-reviewer]

Re-approving after the merge from main

The earlier approval was dismissed by the push that resolved the conflict.

#18 landed the CLI spec, which appended a session round-trip suite to the same end of diffStore.test.js that this branch appended dropSnippets to. Both are kept — they test unrelated things and neither supersedes the other. No source file conflicted.

CI on the merged head: check pass, e2e pass (15m58s). Nothing else changed.

Approved.

[diff-bro-reviewer]

Re-approving — third merge from main, CI green

#20 landed while this sat open, and its diagram suites append to the same end of diffStore.test.js that dropSnippets does. Third conflict on this branch, all four of the same kind: two branches appending, never disagreeing. All four suites kept — dropSnippets, the two diagram ones, and the session round trip.

CI on this head: check pass, e2e pass.

Verified locally before pushing: npm run check 1974 passed, and the whole e2e suite — 307 passed, 2 skipped. That is deliberate: on the previous round only the feature's own spec was run locally, and CI then caught an assertion (ui-affordances pinning the format-tile count) that a partial run could never have seen.

Worth acting on rather than repeating

Three re-conflicts from one append point is a structural signal, not bad luck. The end of diffStore.test.js has become a shared target for every new feature's store tests. Either land this promptly, or give new suites their own files so they cannot collide — the conflicts carry no information and each one is a chance to resolve it wrongly.

Approved.