From c6a6a709a13126a9523b560d0c830734f5b349d1 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Mon, 3 Aug 2026 22:13:20 -0700 Subject: [PATCH 1/3] Attribute a clip by the text it handed Obsidian MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The source-URL check tells two clips of different pages apart, but not two clips of the SAME page racing from two Gullet processes: same source, and the claim maps are per-process. tab_clip now returns contentHash — SHA-256 of exactly what it handed over — and a note that does not hash to it was written by some other clip. That identifies the invocation rather than the destination, so it needs no shared state and no marker in the user's notes. Measured before built, in both clip modes and both of the user's real vaults as well as a bare one, at 238 B and 50 kB: Obsidian writes back byte-identical text — no trailing newline added, no line endings rewritten, no truncation. That measurement is what makes a mismatch trustworthy enough to disqualify a note, and if it ever stops holding the symptom is every clip reporting mismatched, which is a verdict of its own precisely so the message can name that possibility. Also from the live run: Obsidian creates a note and fills it a beat later, and an empty file has no parseable source, so the fallback read it as ours. An empty note now never vouches for a clip. The hash is never required — an older extension sends none and attribution falls back to source. The extension hashes with WebCrypto and Gullet with Bun's hasher, so tests/clip-source.test.ts pins them together; nothing else would catch an encoding or digest change. ClipVerifier takes an evidence object rather than four positional arguments, which is the simplification pass deferred from #19. Verified: bun run check (456 tests). --- AGENTS.md | 2 +- gullet/src/clip-verify.ts | 114 +++++++++++++++++------- gullet/src/tools.ts | 17 +++- gullet/tests/clip-verify.test.ts | 146 +++++++++++++++++++++++++------ gullet/tests/tools.test.ts | 40 +++++++++ src/bridge-methods.ts | 24 ++++- src/bridge-protocol.ts | 13 +++ tests/clip-source.test.ts | 32 ++++++- 8 files changed, 326 insertions(+), 62 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8e20441..3be82b7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,7 +47,7 @@ This is a Bun-powered TypeScript WebExtension for Zen Browser, Firefox, and Chro Global Gullet settings live at `${XDG_CONFIG_HOME:-$HOME/.config}/tabglutton/config.json`, with the token in a separate `0600` file by default. The config is deliberately safe to commit: an inline `"token"` key is rejected even when a CLI or environment token would otherwise win. Keep the additive token precedence (`--token` → env → `./.env` → `tokenCommand` → `tokenFile` → default file). `tokenCommand` is bounded and lazy; its timeout/nonzero error, including stderr, goes through `Supervisor.fault()`, and the supervisor retries with backoff so unlocking a secret manager heals the existing MCP session. Do not move command execution ahead of that recoverable startup path. -**A clip is confirmed on disk before anything is closed, because the browser cannot confirm it.** A refused `obsidian://` launch is indistinguishable from a successful one from inside the extension: the ephemeral launch tab sits at `about:blank` with `status: "complete"` in both cases, raises no dialog and fires no error (measured on Firefox 134 with the scheme's `network.protocol-handler.*` prefs both set and unset — the tab samples are byte-identical). So `openObsidianUrl` reports the path it _intended_ to write, a dropped handoff read as success, and `tab_clip({ close: true })` would close the tab over a note that was never saved — a close `undo_close` can reverse, but the clip itself is simply gone. Gullet can answer what the browser cannot, because it runs beside the vault and already reads Obsidian's registry: `clip-verify.ts` resolves the vault directory and looks for the note. Six things are load-bearing. **The close is taken away from the extension**: `clipAndVerify` requests the clip with `close: false`, verifies, and only then calls `tabs_close`, so the undo batch is still written by the one method that owns it and `batchId` keeps its meaning — and that close is allowed to fail. The note is already on disk by then, so a tab that went away or was renumbered during verification comes back as `closed: false` with a reason, never as a failed `tab_clip`: an agent told the clip failed re-clips it, and Obsidian writes the duplicate. **Existence is not proof** — re-clipping a page that was already filed would find the _old_ note and vouch for a handoff that never happened, so the note must have been written since a timestamp sampled before the request, with `CLIP_MTIME_SLACK_MS` absorbing the gap between our clock and the filesystem's. **`obsidian://new` never overwrites**: handed a name that is taken it writes `Note 1.md`, then `Note 2.md` (verified live), and since the extension only ever reports the name it _asked_ for, checking that one path called a landed clip missing the second time a page was filed — `isClipNoteName` matches the numbered variants, by exact string comparison because a note title may contain regex metacharacters. **A note is attributed, not merely counted**: two same-titled pages clipped concurrently ask for the same name and both timestamps precede both writes, so freshness alone let the dropped clip claim the note the other one wrote — and close its tab over nothing. The note says which page it holds, in the `source` property `markdownForClip` writes, so `verifyClip` takes the clipped tab's URL and a note recording a different page never vouches. That needs no shared state, so it holds across the hub/peer processes a claim map cannot reach. `noteSourceUrl` reads the value to the _last_ quote on the line, because the writer escapes `"` and nothing else; the format is duplicated rather than imported (clip-format's import graph reaches browser-typed code, and Gullet has no DOM) and `tests/clip-source.test.ts` pins the two together. Only positive disagreement disqualifies — an unparseable note falls back to freshness, so drift costs a defence rather than failing real clips. Behind that, **one note still vouches for one clip**: the verifier spends each note path+mtime once, which is all that is left when two clips of the _same_ page race. That leaves one accepted residual — two _processes_ clipping one URL simultaneously with a dropped handoff, where nothing on disk separates the requests and the claim maps are not shared. Closing it needs a per-request marker in the user's note or cross-session serialization of the handoff, and neither is worth it for what it costs: the note holds that same page clipped at that same moment, so only drift between two tabs of one URL is lost. **State the guarantee at its real strength**: a `landed` verdict means a fresh note for this page exists, not that this exact extraction is what it holds — and it means nothing at all when the check cannot run, because `unknown` is fail-open and the pre-verification behaviour stands there, dropped handoffs included. The tool description, this file, and any release note say that; "a failed handoff can never cost the user the tab" is false in both directions and has been removed. A content hash returned by `tab_clip` and compared against the file is the one option that would restore a per-invocation guarantee without marking the user's notes — it needs Obsidian's write fidelity (line endings, trailing newline) measured first, because a mismatch has no safe fallback. **Inability to check is never a failure**: an unreadable registry, an unknown vault name, an unlistable folder, or an unstattable note returns `unknown` and behaviour falls back to what it was, the same soft contract as the vault-override check — only `ENOENT` means "not written", which is why `readDir`/`modifiedAt` distinguish absent from unreadable rather than collapsing both to `null`. Only a folder that exists and provably lacks the note is `missing`. For the same reason `parseObsidianVaultEntries` stores the registry's **exact** path and trims only the copy the vault name is derived from: a trailing space is a legal directory name on macOS and Linux, and a trimmed path checks the wrong directory. +**A clip is confirmed on disk before anything is closed, because the browser cannot confirm it.** A refused `obsidian://` launch is indistinguishable from a successful one from inside the extension: the ephemeral launch tab sits at `about:blank` with `status: "complete"` in both cases, raises no dialog and fires no error (measured on Firefox 134 with the scheme's `network.protocol-handler.*` prefs both set and unset — the tab samples are byte-identical). So `openObsidianUrl` reports the path it _intended_ to write, a dropped handoff read as success, and `tab_clip({ close: true })` would close the tab over a note that was never saved — a close `undo_close` can reverse, but the clip itself is simply gone. Gullet can answer what the browser cannot, because it runs beside the vault and already reads Obsidian's registry: `clip-verify.ts` resolves the vault directory and looks for the note. Six things are load-bearing. **The close is taken away from the extension**: `clipAndVerify` requests the clip with `close: false`, verifies, and only then calls `tabs_close`, so the undo batch is still written by the one method that owns it and `batchId` keeps its meaning — and that close is allowed to fail. The note is already on disk by then, so a tab that went away or was renumbered during verification comes back as `closed: false` with a reason, never as a failed `tab_clip`: an agent told the clip failed re-clips it, and Obsidian writes the duplicate. **Existence is not proof** — re-clipping a page that was already filed would find the _old_ note and vouch for a handoff that never happened, so the note must have been written since a timestamp sampled before the request, with `CLIP_MTIME_SLACK_MS` absorbing the gap between our clock and the filesystem's. **`obsidian://new` never overwrites**: handed a name that is taken it writes `Note 1.md`, then `Note 2.md` (verified live), and since the extension only ever reports the name it _asked_ for, checking that one path called a landed clip missing the second time a page was filed — `isClipNoteName` matches the numbered variants, by exact string comparison because a note title may contain regex metacharacters. **A note is attributed, not merely counted**: two same-titled pages clipped concurrently ask for the same name and both timestamps precede both writes, so freshness alone let the dropped clip claim the note the other one wrote — and close its tab over nothing. The note says which page it holds, in the `source` property `markdownForClip` writes, so `verifyClip` takes the clipped tab's URL and a note recording a different page never vouches. That needs no shared state, so it holds across the hub/peer processes a claim map cannot reach. `noteSourceUrl` reads the value to the _last_ quote on the line, because the writer escapes `"` and nothing else; the format is duplicated rather than imported (clip-format's import graph reaches browser-typed code, and Gullet has no DOM) and `tests/clip-source.test.ts` pins the two together. Only positive disagreement disqualifies — an unparseable note falls back to freshness, so drift costs a defence rather than failing real clips. Behind that, **one note still vouches for one clip**: the verifier spends each note path+mtime once, which is all that is left when two clips of the _same_ page race. **The clip's own text is what closes the last gap.** Two _processes_ clipping one URL simultaneously defeat both of the above — same source, and the claim maps are not shared — so `tab_clip` returns `contentHash`, the SHA-256 of exactly what it handed Obsidian, and a note that does not hash to it was written by some other clip. That identifies the invocation rather than the destination, needs no marker in the user's notes, and holds across processes. It rests on a measurement, not an assumption: `obsidian://new` was handed known bytes and the file compared, in both clip modes, at 238 B and 50 kB, in the `test` vault and in both of the user's real (plugin-bearing) vaults — byte-identical every time, no trailing newline added, no line endings rewritten, no truncation. **If that ever stops being true the symptom is every clip reporting `mismatched`**, which is why that verdict exists as its own answer and says so in its message. The hash is never required: an older extension sends none and attribution falls back to `source`. The extension hashes with WebCrypto and Gullet with Bun's hasher, so `tests/clip-source.test.ts` pins the two implementations together — nothing else would catch an encoding or digest change, and the symptom would again be every clip unconfirmed. **An empty note is never a landed clip**: Obsidian creates the file and fills it a beat later (measured live in both real vaults), and without that guard the empty file is exactly the one with no parseable `source`, which the fallback would have read as ours. **State the guarantee at its real strength**: hashed, a `landed` verdict means this exact text is on disk; unhashed, only that a fresh note for this page is; and it means nothing at all when the check cannot run, because `unknown` is fail-open and the pre-verification behaviour stands there, dropped handoffs included. "A failed handoff can never cost the user the tab" is false in both directions and must not come back. **Inability to check is never a failure**: an unreadable registry, an unknown vault name, an unlistable folder, or an unstattable note returns `unknown` and behaviour falls back to what it was, the same soft contract as the vault-override check — only `ENOENT` means "not written", which is why `readDir`/`modifiedAt` distinguish absent from unreadable rather than collapsing both to `null`. Only a folder that exists and provably lacks the note is `missing`. For the same reason `parseObsidianVaultEntries` stores the registry's **exact** path and trims only the copy the vault name is derived from: a trailing space is a legal directory name on macOS and Linux, and a trimmed path checks the wrong directory. The underlying launch bug is [#18](https://github.com/mlsimon734/tabglutton/issues/18) and is **not fixed** by any of this — on a profile without `network.protocol-handler.external.obsidian=true` and `network.protocol-handler.warn-external.obsidian=false`, Firefox still drops the handoff, and the popup's Devour has no sidecar to check it. Note that `bun run start` and `start:firefox` both pass those prefs, so **dev profiles cannot reproduce it**; that is why it went unnoticed. Verification turns the silent failure into a reported one for the bridge path only. diff --git a/gullet/src/clip-verify.ts b/gullet/src/clip-verify.ts index f605879..6114c65 100644 --- a/gullet/src/clip-verify.ts +++ b/gullet/src/clip-verify.ts @@ -20,29 +20,50 @@ import type { ObsidianVaultPaths } from "./obsidian-vaults.js"; * - `landed` the note is on disk. The clip is real. * - `missing` the vault path is known and the note is not there. Refuse to * close, and say so. + * - `mismatched` a fresh note for this page is there, but its text is not what + * we handed over. Refuse to close — but it is a different story from + * `missing` and has to be told differently: either another session's clip of + * the same page landed while ours dropped, or something in the vault rewrites + * notes on create, which would make this the answer for every clip. Guessing + * between them is not our job; naming both is. * - `unknown` the registry could not be read, or names no such vault. Same soft * contract as the vault-override check: inability to verify is never a * failure, so behaviour falls back to what it was before. */ -export type ClipVerdict = "landed" | "missing" | "unknown"; +export type ClipVerdict = "landed" | "missing" | "mismatched" | "unknown"; /** - * `since` is taken before the clip is requested. Mere existence is not proof: - * re-clipping a page that was already filed would find the OLD note and call a - * dropped handoff verified, which is exactly the case this check exists for. The - * note must have been written since we asked. - * - * `sourceUrl` is the clipped tab's URL, as the extension reported it. Freshness - * alone cannot tell two concurrent clips apart — same title, same requested - * name, both timestamps before both writes — so a note whose own `source` names - * a different page never vouches for this clip. Optional: an embedder that - * cannot supply it gets freshness-only verification, as before. + * What a verification has to go on, weakest to strongest. Only `since` is + * required: an embedder that can supply nothing else gets freshness-only + * verification, which is where this check started. */ +export interface ClipEvidence { + /** + * Taken before the clip is requested. Mere existence is not proof: re-clipping + * a page that was already filed would find the OLD note and call a dropped + * handoff verified, which is exactly the case this check exists for. + */ + since: number; + /** + * The clipped tab's URL, as the extension reported it. Freshness cannot tell + * two concurrent clips apart — same title, same requested name, both + * timestamps before both writes — so a note whose own `source` names a + * different page never vouches. + */ + sourceUrl?: string; + /** + * SHA-256 of the exact text handed to Obsidian. Stronger than `sourceUrl`, + * and the only evidence that separates two clips of the *same* page: it + * identifies the invocation rather than the destination. Absent from older + * extensions, so it is never required. + */ + contentHash?: string; +} + export type ClipVerifier = ( vault: string, file: string, - since: number, - sourceUrl?: string, + evidence: ClipEvidence, ) => Promise; /** Obsidian writes asynchronously; give it a moment before calling it missing. */ @@ -73,6 +94,8 @@ export interface ClipVerifierOptions { readDir?: (dir: string) => Promise; /** A note's text, null when absent, "unreadable" when it cannot be checked. */ readNote?: (path: string) => Promise; + /** SHA-256 hex of a note's text. Injected for tests. */ + hashNote?: (content: string) => Promise; sleep?: (ms: number) => Promise; now?: () => number; } @@ -126,6 +149,14 @@ export function noteSourceUrl(content: string): string | null { return value === undefined ? null : value.replaceAll('\\"', '"'); } +/** + * SHA-256 of a note's text, hex — the same digest `bridge-methods.ts` takes of + * what it handed Obsidian. Injectable only so tests need not hash real strings. + */ +export async function clipContentHash(content: string): Promise { + return new Bun.CryptoHasher("sha256").update(content).digest("hex"); +} + /** * Obsidian Web Clipper — and so `clip-format.ts` — drops a scroll-to-text * fragment from the recorded source, because it addresses a position in the @@ -200,6 +231,7 @@ export function createClipVerifier( const modifiedAt = options.modifiedAt ?? defaultModifiedAt; const readDir = options.readDir ?? defaultReadDir; const readNote = options.readNote ?? defaultReadNote; + const hashNote = options.hashNote ?? clipContentHash; const sleep = options.sleep ?? ((ms: number) => Bun.sleep(ms)); const now = options.now ?? (() => Date.now()); @@ -216,23 +248,20 @@ export function createClipVerifier( * sharing one browser — a hub and a peer — each keep their own map. Attributing * by `source` needs no shared state and holds across processes. * - * Known and accepted residual: two *processes* clipping the same URL at the - * same time, one handoff dropped, both accept the single note. Nothing on disk - * separates them — the URL matches and the maps are not shared — so closing - * that would take either a per-request marker written into the user's note or - * cross-session serialization of the whole handoff. What it costs is bounded - * and much smaller than the bug this file exists for: the note on disk holds - * that same page, clipped at that same moment, so the closed tab loses only - * whatever two tabs of one URL had extracted differently. + * With `contentHash` present it is a backstop rather than the argument: the + * hash already identifies the invocation, and two clips that hash the same are + * two clips of byte-identical text, where which note belongs to which cannot + * matter. It still carries the older extensions that send no hash. * - * So be precise about what a `landed` verdict is worth. It says a fresh note - * for this page exists — not that this exact extraction is what it holds. It - * is not a guarantee at all when the check cannot run: `unknown` is fail-open - * by design, and there the old behaviour stands, dropped handoffs included. + * Be precise about what a `landed` verdict is worth. Hashed, it says this + * exact text is on disk. Unhashed, only that a fresh note for this page is. + * Either way it is not a guarantee when the check cannot run: `unknown` is + * fail-open by design, and there the old behaviour stands, dropped handoffs + * included. */ const claimed = new Map(); - return async (vault, file, since, sourceUrl) => { + return async (vault, file, { since, sourceUrl, contentHash }) => { let paths: ReadonlyMap | null; try { paths = await vaultPaths(); @@ -249,6 +278,10 @@ export function createClipVerifier( const base = basename(target).slice(0, -3); const fresh = since - CLIP_MTIME_SLACK_MS; const deadline = now() + timeoutMs; + // A fresh note for this page that we refused on its hash. Kept so the + // timeout can say "someone wrote this, it just was not us" rather than the + // flatly wrong "nothing was written". + let sawForeign = false; for (;;) { const entries = await readDir(dir); // The folder not existing yet is a legitimate "not written", but a folder @@ -262,14 +295,33 @@ export function createClipVerifier( if (mtime === "unreadable") return "unknown"; if (mtime === null || mtime < fresh) continue; // Fresh and correctly named is not yet proof this clip wrote it. A - // concurrent clip of a same-titled page asks for the same name, so ask - // the note which page it holds before letting it close a tab. - if (sourceUrl !== undefined) { + // concurrent clip of a same-titled page asks for the same name, so read + // the note before letting it close a tab. + if (contentHash !== undefined || sourceUrl !== undefined) { const content = await readNote(path); if (content === "unreadable") return "unknown"; // Written between our listing and our read; it will be back next poll. if (content === null) continue; - if (noteOwnership(content, sourceUrl) === "other") continue; + // Obsidian creates the note and fills it a beat later — measured live, + // both of this user's real vaults. An empty file is that window, never + // a landed clip, and without this it is exactly the note that has no + // parseable `source` and so falls through as "mine". + if (content.trim() === "") continue; + if (contentHash !== undefined) { + // The invocation's own fingerprint: a note that does not hash to it + // was written by some other clip, whatever page it names. + if ((await hashNote(content)) !== contentHash) { + // Only a note naming *our* page is worth reporting as a mismatch; + // a neighbouring clip that merely shares a title is not evidence + // about ours either way. + if (sourceUrl === undefined || noteOwnership(content, sourceUrl) === "mine") { + sawForeign = true; + } + continue; + } + } else if (noteOwnership(content, sourceUrl ?? "") === "other") { + continue; + } } const spentAt = claimed.get(path); if (spentAt !== undefined && mtime <= spentAt) continue; @@ -277,7 +329,7 @@ export function createClipVerifier( prune(claimed, now() - timeoutMs - CLIP_MTIME_SLACK_MS); return "landed"; } - if (now() >= deadline) return "missing"; + if (now() >= deadline) return sawForeign ? "mismatched" : "missing"; await sleep(POLL_INTERVAL_MS); } }; diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index 4402468..536db3f 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -361,7 +361,10 @@ async function clipAndVerify( // the one thing a timestamp cannot do. Absent, verification is freshness-only, // as it was. const sourceUrl = typeof result.url === "string" ? result.url : undefined; - const verdict = await ctx.verifyClip(vault, file, startedAt, sourceUrl); + // The extension's own digest of what it handed Obsidian. Absent from older + // extensions, and then attribution falls back to the page's URL. + const contentHash = typeof result.contentHash === "string" ? result.contentHash : undefined; + const verdict = await ctx.verifyClip(vault, file, { since: startedAt, sourceUrl, contentHash }); if (verdict === "missing") { throw new BridgeRequestError( "not-enabled", @@ -373,6 +376,18 @@ async function clipAndVerify( `and to confirm Obsidian's one-time "trust this source" prompt.`, ); } + if (verdict === "mismatched") { + throw new BridgeRequestError( + "not-enabled", + `The clip may not have reached Obsidian: a note for this page is at ` + + `${JSON.stringify(file)} in vault ${JSON.stringify(vault)}, but its text is not what ` + + `was handed over, so it cannot be confirmed as this clip. The tab was left open. ` + + `Either another session filed the same page while this handoff was dropped — in which ` + + `case the page is safely filed and the tab can be closed by hand — or something in ` + + `the vault rewrites notes when they are created, which would make every clip report ` + + `this. If it is every clip, that is worth reporting as a bug.`, + ); + } if (!wantsClose) return { ...result, clipVerified: verdict === "landed" }; diff --git a/gullet/tests/clip-verify.test.ts b/gullet/tests/clip-verify.test.ts index e3e91fd..283429f 100644 --- a/gullet/tests/clip-verify.test.ts +++ b/gullet/tests/clip-verify.test.ts @@ -84,7 +84,7 @@ describe("createClipVerifier", () => { ...fakeClock(), ...vaultWith({ "Note.md": SINCE + 50 }), }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("landed"); }); // Obsidian never overwrites: a second clip of the same page lands as "Note 1.md". @@ -94,7 +94,7 @@ describe("createClipVerifier", () => { ...fakeClock(), ...vaultWith({ "Note.md": SINCE - 60_000, "Note 1.md": SINCE + 40 }), }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("landed"); }); test("landed when Obsidian writes a moment late, without waiting the full deadline", async () => { @@ -104,7 +104,7 @@ describe("createClipVerifier", () => { readDir: async () => ["Note.md"], modifiedAt: async () => (++calls >= 3 ? SINCE + 10 : null), }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("landed"); expect(calls).toBe(3); }); @@ -115,7 +115,7 @@ describe("createClipVerifier", () => { ...fakeClock(), ...vaultWith({ "Note.md": SINCE - 60_000, "Note 1.md": SINCE - 30_000 }), }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("missing"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("missing"); }); test("landed when the note is barely older than our timestamp, within slack", async () => { @@ -123,12 +123,12 @@ describe("createClipVerifier", () => { ...fakeClock(), ...vaultWith({ "Note.md": SINCE - 500 }), }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("landed"); }); test("missing once the deadline passes with nothing written", async () => { const verify = createClipVerifier(vaults, { ...fakeClock(), ...vaultWith({}) }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("missing"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("missing"); }); test("a fresh note by another name does not vouch for this clip", async () => { @@ -136,13 +136,13 @@ describe("createClipVerifier", () => { ...fakeClock(), ...vaultWith({ "Something Else.md": SINCE + 100 }), }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("missing"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("missing"); }); // The soft contract: inability to check is never a failed clip. test("unknown when the registry cannot be read", async () => { const verify = createClipVerifier(async () => null, { ...fakeClock(), ...vaultWith({}) }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("unknown"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("unknown"); }); test("unknown when the registry throws", async () => { @@ -152,12 +152,12 @@ describe("createClipVerifier", () => { }, { ...fakeClock(), ...vaultWith({}) }, ); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("unknown"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("unknown"); }); test("unknown when the registry does not name that vault", async () => { const verify = createClipVerifier(vaults, { ...fakeClock(), ...vaultWith({}) }); - expect(await verify("Some Other Vault", "Clippings/Note", SINCE)).toBe("unknown"); + expect(await verify("Some Other Vault", "Clippings/Note", { since: SINCE })).toBe("unknown"); }); // A folder that exists but cannot be listed is "cannot check", not "not written". @@ -167,7 +167,7 @@ describe("createClipVerifier", () => { readDir: async () => "unreadable", modifiedAt: async () => null, }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("unknown"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("unknown"); }); // The same failure one level down: listable folder, unstattable note. @@ -177,7 +177,7 @@ describe("createClipVerifier", () => { readDir: async () => ["Note.md"], modifiedAt: async () => "unreadable", }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("unknown"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("unknown"); }); // But a folder Obsidian never created means the clip genuinely did not land. @@ -187,7 +187,7 @@ describe("createClipVerifier", () => { readDir: async () => "missing", modifiedAt: async () => null, }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("missing"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("missing"); }); // Two pages with the same title clipped at once produce one requested name. @@ -199,8 +199,8 @@ describe("createClipVerifier", () => { ...vaultWith({ "Note.md": SINCE + 50 }), }); const [first, second] = await Promise.all([ - verify("test", "Clippings/Note", SINCE), - verify("test", "Clippings/Note", SINCE), + verify("test", "Clippings/Note", { since: SINCE }), + verify("test", "Clippings/Note", { since: SINCE }), ]); expect([first, second].sort()).toEqual(["landed", "missing"]); }); @@ -217,13 +217,98 @@ describe("createClipVerifier", () => { }); // Clip A is dropped and looks first; clip B is the one Obsidian filed. const [a, b] = await Promise.all([ - verify("test", "Clippings/Note", SINCE, "https://a.example/post"), - verify("test", "Clippings/Note", SINCE, "https://b.example/post"), + verify("test", "Clippings/Note", { since: SINCE, sourceUrl: "https://a.example/post" }), + verify("test", "Clippings/Note", { since: SINCE, sourceUrl: "https://b.example/post" }), ]); expect(a).toBe("missing"); expect(b).toBe("landed"); }); + // The residual the source-URL check could not close: two clips of the SAME + // page, from separate processes with separate claim maps, one dropped. Both + // notes would name the same source, so only the text tells them apart. + test("a content hash separates two clips of the same page across processes", async () => { + const options = { + ...vaultWith({ "Note.md": SINCE + 50 }), + readNote: async () => "the note B wrote", + hashNote: async (content: string) => `hash:${content}`, + }; + // Separate verifiers, as two Gullet processes would have. + const a = createClipVerifier(vaults, { ...fakeClock(), ...options }); + const b = createClipVerifier(vaults, { ...fakeClock(), ...options }); + const evidence = { since: SINCE, sourceUrl: "https://a.example/post" }; + expect( + await a("test", "Clippings/Note", { ...evidence, contentHash: "hash:the note A wrote" }), + ).toBe("mismatched"); + expect( + await b("test", "Clippings/Note", { ...evidence, contentHash: "hash:the note B wrote" }), + ).toBe("landed"); + }); + + // Obsidian creates the note and fills it a beat later — measured live in both + // real vaults. Without this the empty file has no parseable source, falls + // through as "mine", and vouches for a clip whose text is not there yet. + test("an empty note is the write window, not a landed clip", async () => { + let reads = 0; + const verify = createClipVerifier(vaults, { + ...fakeClock(), + ...vaultWith({ "Note.md": SINCE + 50 }), + readNote: async () => (++reads >= 3 ? clipNote("https://a.example/post") : ""), + }); + expect( + await verify("test", "Clippings/Note", { since: SINCE, sourceUrl: "https://a.example/post" }), + ).toBe("landed"); + expect(reads).toBe(3); // it kept polling rather than accepting the empty file + }); + + // Distinguishing the two failures matters: "nothing was written" and "someone + // wrote this and it is not ours" call for different actions from the user. + test("reports mismatched, not missing, when a note for this page is foreign", async () => { + const verify = createClipVerifier(vaults, { + ...fakeClock(), + ...vaultWith({ "Note.md": SINCE + 50 }), + readNote: async () => clipNote("https://a.example/post"), + hashNote: async () => "some other hash", + }); + expect( + await verify("test", "Clippings/Note", { + since: SINCE, + sourceUrl: "https://a.example/post", + contentHash: "ours", + }), + ).toBe("mismatched"); + }); + + // But a note for a *different* page says nothing about ours either way, so it + // must not colour the verdict — that is still a plain missing. + test("a same-named note for another page leaves the verdict missing", async () => { + const verify = createClipVerifier(vaults, { + ...fakeClock(), + ...vaultWith({ "Note.md": SINCE + 50 }), + readNote: async () => clipNote("https://elsewhere.example/post"), + hashNote: async () => "some other hash", + }); + expect( + await verify("test", "Clippings/Note", { + since: SINCE, + sourceUrl: "https://a.example/post", + contentHash: "ours", + }), + ).toBe("missing"); + }); + + // An extension that predates the hash still gets the source-URL attribution. + test("falls back to source attribution when no hash is supplied", async () => { + const verify = createClipVerifier(vaults, { + ...fakeClock(), + ...vaultWith({ "Note.md": SINCE + 50 }), + readNote: async () => clipNote("https://a.example/post"), + }); + expect( + await verify("test", "Clippings/Note", { since: SINCE, sourceUrl: "https://a.example/post" }), + ).toBe("landed"); + }); + // Two Gullets sharing a browser keep separate claim maps, so attribution has // to hold with no shared state: a verifier that has never seen the other's // request still refuses a note belonging to it. @@ -233,9 +318,12 @@ describe("createClipVerifier", () => { ...vaultWith({ "Note.md": SINCE + 50 }), readNote: async () => clipNote("https://elsewhere.example/post"), }); - expect(await verify("test", "Clippings/Note", SINCE, "https://mine.example/post")).toBe( - "missing", - ); + expect( + await verify("test", "Clippings/Note", { + since: SINCE, + sourceUrl: "https://mine.example/post", + }), + ).toBe("missing"); }); // A scroll-to-text fragment addresses a position in a page, not a page, and @@ -247,7 +335,7 @@ describe("createClipVerifier", () => { readNote: async () => clipNote("https://a.example/post"), }); const url = "https://a.example/post#:~:text=selected%20words"; - expect(await verify("test", "Clippings/Note", SINCE, url)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE, sourceUrl: url })).toBe("landed"); }); // Positive disagreement disqualifies; absence of evidence must not. A note @@ -259,7 +347,9 @@ describe("createClipVerifier", () => { ...vaultWith({ "Note.md": SINCE + 50 }), readNote: async () => "just some text", }); - expect(await verify("test", "Clippings/Note", SINCE, "https://a.example/post")).toBe("landed"); + expect( + await verify("test", "Clippings/Note", { since: SINCE, sourceUrl: "https://a.example/post" }), + ).toBe("landed"); }); test("unknown when a candidate note cannot be read", async () => { @@ -268,7 +358,9 @@ describe("createClipVerifier", () => { ...vaultWith({ "Note.md": SINCE + 50 }), readNote: async () => "unreadable", }); - expect(await verify("test", "Clippings/Note", SINCE, "https://a.example/post")).toBe("unknown"); + expect( + await verify("test", "Clippings/Note", { since: SINCE, sourceUrl: "https://a.example/post" }), + ).toBe("unknown"); }); // Both really landed: Obsidian sidesteps the collision, so there are two notes @@ -279,8 +371,8 @@ describe("createClipVerifier", () => { ...vaultWith({ "Note.md": SINCE + 50, "Note 1.md": SINCE + 60 }), }); const verdicts = await Promise.all([ - verify("test", "Clippings/Note", SINCE), - verify("test", "Clippings/Note", SINCE), + verify("test", "Clippings/Note", { since: SINCE }), + verify("test", "Clippings/Note", { since: SINCE }), ]); expect(verdicts).toEqual(["landed", "landed"]); }); @@ -295,8 +387,8 @@ describe("createClipVerifier", () => { modifiedAt: async (p): Promise => mtimes[p.slice("/vaults/test/Clippings/".length)] ?? null, }); - expect(await verify("test", "Clippings/Note", SINCE)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE })).toBe("landed"); mtimes["Note.md"] = SINCE + 500; - expect(await verify("test", "Clippings/Note", SINCE + 400)).toBe("landed"); + expect(await verify("test", "Clippings/Note", { since: SINCE + 400 })).toBe("landed"); }); }); diff --git a/gullet/tests/tools.test.ts b/gullet/tests/tools.test.ts index a6506e7..f0eefb4 100644 --- a/gullet/tests/tools.test.ts +++ b/gullet/tests/tools.test.ts @@ -396,6 +396,46 @@ describe("tab-scoped tools", () => { expect(sent).toEqual([]); }); + // A note that exists but is not ours is a different story from nothing being + // written, and the message has to tell the user which one they are in. + test("tab_clip distinguishes a foreign note from a missing one", async () => { + const { call, sent } = caller([zen], () => ({ file: "Clippings/Note", vault: "test" }), { + verifyClip: async () => "mismatched", + }); + const result = await call("tab_clip", { tabId: 7, close: true }); + expect(result.isError).toBe(true); + const text = JSON.stringify(payload(result)); + expect(text).toContain("its text is not what was handed over"); + expect(text).not.toContain("never reached Obsidian"); + expect(sent).toHaveLength(1); // nothing closed + }); + + // The evidence the extension supplies has to reach the verifier, or the whole + // attribution chain is silently freshness-only. + test("tab_clip hands the verifier the clip's own url and content hash", async () => { + let evidence: unknown; + const { call } = caller( + [zen], + () => ({ + file: "Clippings/Note", + vault: "test", + url: "https://example.com/post", + contentHash: "abc123", + }), + { + verifyClip: async (_vault, _file, seen) => { + evidence = seen; + return "landed"; + }, + }, + ); + await call("tab_clip", { tabId: 7 }); + expect(evidence).toMatchObject({ + sourceUrl: "https://example.com/post", + contentHash: "abc123", + }); + }); + test("tab_clip without close reports whether the note was verified", async () => { const { call, sent } = caller([zen], () => ({ file: "Clippings/Note", vault: "test" }), { verifyClip: async () => "landed", diff --git a/src/bridge-methods.ts b/src/bridge-methods.ts index a47a207..b48245f 100644 --- a/src/bridge-methods.ts +++ b/src/bridge-methods.ts @@ -58,6 +58,21 @@ const STALE_ID_HINT = "It may have been closed, or unloaded and given a new id (Chrome does this when it discards a tab). Re-run tabs_list for current ids."; /** The single place the hint is attached — throwing and per-tab paths alike. */ +/** + * SHA-256 of a note's text, hex, as `TabClipResult.contentHash`. + * + * The extension cannot check whether Obsidian took the handoff, but it can say + * exactly what it handed over, and that is enough for a sidecar sitting beside + * the vault to tell this clip's note from a concurrent clip of the same page. + * `crypto.subtle` is available in both the event page and the service worker. + */ +async function sha256Hex(text: string): Promise { + const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(text)); + return Array.from(new Uint8Array(digest)) + .map((byte) => byte.toString(16).padStart(2, "0")) + .join(""); +} + function missingTabReason(message: string): string { return `${message} ${STALE_ID_HINT}`; } @@ -636,7 +651,14 @@ export class BridgeMethodRunner { return request.file; }); - const filed = { tabId: params.tabId, title: payload.title, url: payload.url, file, vault }; + const filed = { + tabId: params.tabId, + title: payload.title, + url: payload.url, + file, + vault, + contentHash: await sha256Hex(content), + }; if (!params.close) return { ...filed, closed: false }; // Nothing past here fails the call: the note is already in Obsidian, so a diff --git a/src/bridge-protocol.ts b/src/bridge-protocol.ts index ca1db89..20552bd 100644 --- a/src/bridge-protocol.ts +++ b/src/bridge-protocol.ts @@ -662,6 +662,19 @@ export interface TabClipResult { closed: boolean; /** Present when `close` was honoured — pass to `undo_close` to reopen. */ batchId?: string; + /** + * SHA-256, hex, of the exact note text handed to Obsidian. + * + * The one thing that identifies *this* invocation on disk. Freshness and the + * note's own `source` cannot separate two clips of the same page racing from + * two agent sessions, so a sidecar that can hash the file can attribute the + * note to the request that wrote it — see gullet/src/clip-verify.ts. + * + * Optional because an older extension does not send it and the sidecar must + * keep working against one. Identical in both clip modes: clipboard and + * legacy-uri carry the same text by different roads. + */ + contentHash?: string; } export interface TabsCloseParams { diff --git a/tests/clip-source.test.ts b/tests/clip-source.test.ts index 384e639..e5a0dc6 100644 --- a/tests/clip-source.test.ts +++ b/tests/clip-source.test.ts @@ -10,7 +10,7 @@ import { describe, expect, test } from "bun:test"; import { markdownForClip, type ClipPayload } from "../src/clip-format.js"; -import { noteSourceUrl } from "../gullet/src/clip-verify.js"; +import { clipContentHash, noteSourceUrl } from "../gullet/src/clip-verify.js"; function payload(overrides: Partial = {}): ClipPayload { return { @@ -49,3 +49,33 @@ describe("frontmatter source, as Gullet reads it", () => { expect(noteSourceUrl(note)).toBe("https://example.com/post"); }); }); + +/** + * The other cross-boundary coupling: the extension hashes with WebCrypto in the + * browser, Gullet with Bun's hasher on the filesystem side, and a clip is + * attributed by those two agreeing. Nothing else would catch them diverging — + * over an encoding, a digest, or a hex-vs-base64 change — and the symptom would + * be every clip reported as unconfirmed. + */ +describe("content hash, across the extension/sidecar boundary", () => { + async function webCryptoSha256Hex(text: string): Promise { + const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(text)); + return Array.from(new Uint8Array(digest)) + .map((byte) => byte.toString(16).padStart(2, "0")) + .join(""); + } + + test("the browser's digest of a clip matches the one Gullet takes of the file", async () => { + const note = markdownForClip(payload()); + expect(await clipContentHash(note)).toBe(await webCryptoSha256Hex(note)); + }); + + // Non-ASCII is where an encoding mismatch would surface — a real clip is full + // of it (em dashes, smart quotes, emoji). + test("agrees on text well outside ASCII", async () => { + const note = markdownForClip( + payload({ title: "Ünïcödé — 🐊 “smart” quotes", markdown: "Ünïcödé body 🐊" }), + ); + expect(await clipContentHash(note)).toBe(await webCryptoSha256Hex(note)); + }); +}); From 7320b8c6827d4fde5457b0d00321cbb500521e25 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Tue, 4 Aug 2026 12:27:30 -0700 Subject: [PATCH 2/3] Ignore every dev browser profile, not three by name --- .gitignore | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index c1a1136..3387533 100644 --- a/.gitignore +++ b/.gitignore @@ -5,9 +5,10 @@ dist-firefox/ dist-chrome/ .DS_Store *.log -.dev-profile/ -.dev-profile-firefox/ -.dev-profile-chrome/ +# Dev browser profiles. Wildcard rather than one line per profile: these carry +# the bridge token in extension storage, and a profile added later must not be +# one `git add -A` away from committing it. +.dev-profile*/ .claude/settings.local.json .claude/skills/ .agents/ From f2abcf7f0c07e1bcfe86f1e33f40c8af4612fea0 Mon Sep 17 00:00:00 2001 From: Michael Simon Date: Tue, 4 Aug 2026 17:53:29 -0700 Subject: [PATCH 3/3] Hash a clip's text the way the clipboard delivers it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The content fingerprint was taken over the LF text the extension composed, but in the default clipboard mode that text reaches Obsidian through the OS clipboard, and Windows carries plain text as CF_UNICODETEXT with CRLF. The note on disk therefore has endings the extension never produced, so every landed clip on Windows would hash differently, report `mismatched`, leave the requested close undone, and send the agent back to re-clip a page Obsidian had already filed. Both hashers now fold CRLF to LF first. Line endings are transport, not content; the only thing given up is telling apart two clips that differ in nothing else, and there is no such pair. The cross-boundary test was proving less than it claimed: it hashed with its own WebCrypto copy, so it only ever showed that Bun agrees with the test. It imports the real extension helper now — which is why that helper moves out of bridge-methods.ts, whose import graph reaches `browser`, into its own pure module — and covers both the CRLF route and a pair that must still hash apart. `tab_clip`'s MCP description still told agents a verified clip is no proof of the extraction and that concurrent same-URL sessions can share evidence. With a contentHash present that has not been true since c6a6a70; it now splits the guarantee by whether the result carries one. --- AGENTS.md | 2 +- gullet/src/clip-verify.ts | 14 ++++++++++--- gullet/src/tools.ts | 2 +- src/bridge-methods.ts | 18 ++--------------- src/clip-hash.ts | 41 +++++++++++++++++++++++++++++++++++++++ tests/clip-source.test.ts | 33 ++++++++++++++++++++++--------- 6 files changed, 80 insertions(+), 30 deletions(-) create mode 100644 src/clip-hash.ts diff --git a/AGENTS.md b/AGENTS.md index 3be82b7..d1f060a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -47,7 +47,7 @@ This is a Bun-powered TypeScript WebExtension for Zen Browser, Firefox, and Chro Global Gullet settings live at `${XDG_CONFIG_HOME:-$HOME/.config}/tabglutton/config.json`, with the token in a separate `0600` file by default. The config is deliberately safe to commit: an inline `"token"` key is rejected even when a CLI or environment token would otherwise win. Keep the additive token precedence (`--token` → env → `./.env` → `tokenCommand` → `tokenFile` → default file). `tokenCommand` is bounded and lazy; its timeout/nonzero error, including stderr, goes through `Supervisor.fault()`, and the supervisor retries with backoff so unlocking a secret manager heals the existing MCP session. Do not move command execution ahead of that recoverable startup path. -**A clip is confirmed on disk before anything is closed, because the browser cannot confirm it.** A refused `obsidian://` launch is indistinguishable from a successful one from inside the extension: the ephemeral launch tab sits at `about:blank` with `status: "complete"` in both cases, raises no dialog and fires no error (measured on Firefox 134 with the scheme's `network.protocol-handler.*` prefs both set and unset — the tab samples are byte-identical). So `openObsidianUrl` reports the path it _intended_ to write, a dropped handoff read as success, and `tab_clip({ close: true })` would close the tab over a note that was never saved — a close `undo_close` can reverse, but the clip itself is simply gone. Gullet can answer what the browser cannot, because it runs beside the vault and already reads Obsidian's registry: `clip-verify.ts` resolves the vault directory and looks for the note. Six things are load-bearing. **The close is taken away from the extension**: `clipAndVerify` requests the clip with `close: false`, verifies, and only then calls `tabs_close`, so the undo batch is still written by the one method that owns it and `batchId` keeps its meaning — and that close is allowed to fail. The note is already on disk by then, so a tab that went away or was renumbered during verification comes back as `closed: false` with a reason, never as a failed `tab_clip`: an agent told the clip failed re-clips it, and Obsidian writes the duplicate. **Existence is not proof** — re-clipping a page that was already filed would find the _old_ note and vouch for a handoff that never happened, so the note must have been written since a timestamp sampled before the request, with `CLIP_MTIME_SLACK_MS` absorbing the gap between our clock and the filesystem's. **`obsidian://new` never overwrites**: handed a name that is taken it writes `Note 1.md`, then `Note 2.md` (verified live), and since the extension only ever reports the name it _asked_ for, checking that one path called a landed clip missing the second time a page was filed — `isClipNoteName` matches the numbered variants, by exact string comparison because a note title may contain regex metacharacters. **A note is attributed, not merely counted**: two same-titled pages clipped concurrently ask for the same name and both timestamps precede both writes, so freshness alone let the dropped clip claim the note the other one wrote — and close its tab over nothing. The note says which page it holds, in the `source` property `markdownForClip` writes, so `verifyClip` takes the clipped tab's URL and a note recording a different page never vouches. That needs no shared state, so it holds across the hub/peer processes a claim map cannot reach. `noteSourceUrl` reads the value to the _last_ quote on the line, because the writer escapes `"` and nothing else; the format is duplicated rather than imported (clip-format's import graph reaches browser-typed code, and Gullet has no DOM) and `tests/clip-source.test.ts` pins the two together. Only positive disagreement disqualifies — an unparseable note falls back to freshness, so drift costs a defence rather than failing real clips. Behind that, **one note still vouches for one clip**: the verifier spends each note path+mtime once, which is all that is left when two clips of the _same_ page race. **The clip's own text is what closes the last gap.** Two _processes_ clipping one URL simultaneously defeat both of the above — same source, and the claim maps are not shared — so `tab_clip` returns `contentHash`, the SHA-256 of exactly what it handed Obsidian, and a note that does not hash to it was written by some other clip. That identifies the invocation rather than the destination, needs no marker in the user's notes, and holds across processes. It rests on a measurement, not an assumption: `obsidian://new` was handed known bytes and the file compared, in both clip modes, at 238 B and 50 kB, in the `test` vault and in both of the user's real (plugin-bearing) vaults — byte-identical every time, no trailing newline added, no line endings rewritten, no truncation. **If that ever stops being true the symptom is every clip reporting `mismatched`**, which is why that verdict exists as its own answer and says so in its message. The hash is never required: an older extension sends none and attribution falls back to `source`. The extension hashes with WebCrypto and Gullet with Bun's hasher, so `tests/clip-source.test.ts` pins the two implementations together — nothing else would catch an encoding or digest change, and the symptom would again be every clip unconfirmed. **An empty note is never a landed clip**: Obsidian creates the file and fills it a beat later (measured live in both real vaults), and without that guard the empty file is exactly the one with no parseable `source`, which the fallback would have read as ours. **State the guarantee at its real strength**: hashed, a `landed` verdict means this exact text is on disk; unhashed, only that a fresh note for this page is; and it means nothing at all when the check cannot run, because `unknown` is fail-open and the pre-verification behaviour stands there, dropped handoffs included. "A failed handoff can never cost the user the tab" is false in both directions and must not come back. **Inability to check is never a failure**: an unreadable registry, an unknown vault name, an unlistable folder, or an unstattable note returns `unknown` and behaviour falls back to what it was, the same soft contract as the vault-override check — only `ENOENT` means "not written", which is why `readDir`/`modifiedAt` distinguish absent from unreadable rather than collapsing both to `null`. Only a folder that exists and provably lacks the note is `missing`. For the same reason `parseObsidianVaultEntries` stores the registry's **exact** path and trims only the copy the vault name is derived from: a trailing space is a legal directory name on macOS and Linux, and a trimmed path checks the wrong directory. +**A clip is confirmed on disk before anything is closed, because the browser cannot confirm it.** A refused `obsidian://` launch is indistinguishable from a successful one from inside the extension: the ephemeral launch tab sits at `about:blank` with `status: "complete"` in both cases, raises no dialog and fires no error (measured on Firefox 134 with the scheme's `network.protocol-handler.*` prefs both set and unset — the tab samples are byte-identical). So `openObsidianUrl` reports the path it _intended_ to write, a dropped handoff read as success, and `tab_clip({ close: true })` would close the tab over a note that was never saved — a close `undo_close` can reverse, but the clip itself is simply gone. Gullet can answer what the browser cannot, because it runs beside the vault and already reads Obsidian's registry: `clip-verify.ts` resolves the vault directory and looks for the note. Six things are load-bearing. **The close is taken away from the extension**: `clipAndVerify` requests the clip with `close: false`, verifies, and only then calls `tabs_close`, so the undo batch is still written by the one method that owns it and `batchId` keeps its meaning — and that close is allowed to fail. The note is already on disk by then, so a tab that went away or was renumbered during verification comes back as `closed: false` with a reason, never as a failed `tab_clip`: an agent told the clip failed re-clips it, and Obsidian writes the duplicate. **Existence is not proof** — re-clipping a page that was already filed would find the _old_ note and vouch for a handoff that never happened, so the note must have been written since a timestamp sampled before the request, with `CLIP_MTIME_SLACK_MS` absorbing the gap between our clock and the filesystem's. **`obsidian://new` never overwrites**: handed a name that is taken it writes `Note 1.md`, then `Note 2.md` (verified live), and since the extension only ever reports the name it _asked_ for, checking that one path called a landed clip missing the second time a page was filed — `isClipNoteName` matches the numbered variants, by exact string comparison because a note title may contain regex metacharacters. **A note is attributed, not merely counted**: two same-titled pages clipped concurrently ask for the same name and both timestamps precede both writes, so freshness alone let the dropped clip claim the note the other one wrote — and close its tab over nothing. The note says which page it holds, in the `source` property `markdownForClip` writes, so `verifyClip` takes the clipped tab's URL and a note recording a different page never vouches. That needs no shared state, so it holds across the hub/peer processes a claim map cannot reach. `noteSourceUrl` reads the value to the _last_ quote on the line, because the writer escapes `"` and nothing else; the format is duplicated rather than imported (clip-format's import graph reaches browser-typed code, and Gullet has no DOM) and `tests/clip-source.test.ts` pins the two together. Only positive disagreement disqualifies — an unparseable note falls back to freshness, so drift costs a defence rather than failing real clips. Behind that, **one note still vouches for one clip**: the verifier spends each note path+mtime once, which is all that is left when two clips of the _same_ page race. **The clip's own text is what closes the last gap.** Two _processes_ clipping one URL simultaneously defeat both of the above — same source, and the claim maps are not shared — so `tab_clip` returns `contentHash`, the SHA-256 of exactly what it handed Obsidian, and a note that does not hash to it was written by some other clip. That identifies the invocation rather than the destination, needs no marker in the user's notes, and holds across processes. It rests on a measurement, not an assumption: `obsidian://new` was handed known bytes and the file compared, in both clip modes, at 238 B and 50 kB, in the `test` vault and in both of the user's real (plugin-bearing) vaults — byte-identical every time, no trailing newline added, no line endings rewritten, no truncation. **That measurement is macOS-only, and one platform is known to break it**: in the default clipboard mode the text crosses the _OS clipboard_, and Windows carries plain text as `CF_UNICODETEXT` with CRLF, so the note on disk has endings the extension never composed. Both hashers therefore fold `\r\n` to `\n` first (`src/clip-hash.ts`, `clipContentHash` in `clip-verify.ts`) — line endings are transport, not content, and the cost is only the ability to tell apart two clips that differ in nothing else. **If byte-identity fails for any other reason the symptom is every clip reporting `mismatched`**, which is why that verdict exists as its own answer and says so in its message. The hash is never required: an older extension sends none and attribution falls back to `source`. The extension hashes with WebCrypto and Gullet with Bun's hasher, so `tests/clip-source.test.ts` pins the two implementations together — importing the extension's real helper rather than restating it, since a restated copy would prove only that Bun agrees with the test — and nothing else would catch an encoding, digest, or normalization change, whose symptom would again be every clip unconfirmed. That test is also why the hash lives in its own pure module instead of in `bridge-methods.ts`, whose import graph reaches `browser`. **An empty note is never a landed clip**: Obsidian creates the file and fills it a beat later (measured live in both real vaults), and without that guard the empty file is exactly the one with no parseable `source`, which the fallback would have read as ours. **State the guarantee at its real strength**: hashed, a `landed` verdict means this exact text is on disk; unhashed, only that a fresh note for this page is; and it means nothing at all when the check cannot run, because `unknown` is fail-open and the pre-verification behaviour stands there, dropped handoffs included. "A failed handoff can never cost the user the tab" is false in both directions and must not come back. **Inability to check is never a failure**: an unreadable registry, an unknown vault name, an unlistable folder, or an unstattable note returns `unknown` and behaviour falls back to what it was, the same soft contract as the vault-override check — only `ENOENT` means "not written", which is why `readDir`/`modifiedAt` distinguish absent from unreadable rather than collapsing both to `null`. Only a folder that exists and provably lacks the note is `missing`. For the same reason `parseObsidianVaultEntries` stores the registry's **exact** path and trims only the copy the vault name is derived from: a trailing space is a legal directory name on macOS and Linux, and a trimmed path checks the wrong directory. The underlying launch bug is [#18](https://github.com/mlsimon734/tabglutton/issues/18) and is **not fixed** by any of this — on a profile without `network.protocol-handler.external.obsidian=true` and `network.protocol-handler.warn-external.obsidian=false`, Firefox still drops the handoff, and the popup's Devour has no sidecar to check it. Note that `bun run start` and `start:firefox` both pass those prefs, so **dev profiles cannot reproduce it**; that is why it went unnoticed. Verification turns the silent failure into a reported one for the bridge path only. diff --git a/gullet/src/clip-verify.ts b/gullet/src/clip-verify.ts index 6114c65..28cb435 100644 --- a/gullet/src/clip-verify.ts +++ b/gullet/src/clip-verify.ts @@ -150,11 +150,19 @@ export function noteSourceUrl(content: string): string | null { } /** - * SHA-256 of a note's text, hex — the same digest `bridge-methods.ts` takes of - * what it handed Obsidian. Injectable only so tests need not hash real strings. + * SHA-256 of a note's text, hex — the same digest `src/clip-hash.ts` takes of + * what the extension handed Obsidian. Injectable only so tests need not hash + * real strings. + * + * CRLF is folded to LF for the same reason it is on the extension side, and it + * has to be folded on *both*: in clipboard clip mode the note's text reaches + * Obsidian through the OS clipboard, and Windows carries plain text as + * CF_UNICODETEXT with CRLF endings, so the bytes on disk are not the bytes that + * were hashed. Normalizing here is what keeps that path from reporting every + * landed clip as `mismatched`. `tests/clip-source.test.ts` pins the two halves. */ export async function clipContentHash(content: string): Promise { - return new Bun.CryptoHasher("sha256").update(content).digest("hex"); + return new Bun.CryptoHasher("sha256").update(content.replaceAll("\r\n", "\n")).digest("hex"); } /** diff --git a/gullet/src/tools.ts b/gullet/src/tools.ts index 536db3f..9235fbd 100644 --- a/gullet/src/tools.ts +++ b/gullet/src/tools.ts @@ -202,7 +202,7 @@ export const GULLET_TOOLS: readonly McpTool[] = [ name: "tab_clip", title: "File a tab into Obsidian", description: - "Save a tab into the user's Obsidian vault as a markdown note with frontmatter — exactly what the Tabglutton popup's Devour does, including per-site subfolders. Requires a vault configured in Tabglutton's settings. Set close: true to close the tab afterwards; that close is undoable via the returned batchId. Filing alone changes nothing in the browser — the tool is annotated destructive because close: true removes the tab. The result reports the vault it filed into.\n\nWhen the vault can be checked, a fresh note for the clipped page is confirmed on disk before anything is closed: `clipVerified: true` means one was found, and a clip that provably never reached Obsidian is reported as an error with the tab left open. `clipVerified: false` means the vault could not be checked at all — the clip was still handed over, and the close, if asked for, still happened. Treat a verified clip as \"a note for this page landed just now\", not as proof this exact extraction is what it holds: concurrent clips of the same URL from separate agent sessions can share one note as evidence.", + "Save a tab into the user's Obsidian vault as a markdown note with frontmatter — exactly what the Tabglutton popup's Devour does, including per-site subfolders. Requires a vault configured in Tabglutton's settings. Set close: true to close the tab afterwards; that close is undoable via the returned batchId. Filing alone changes nothing in the browser — the tool is annotated destructive because close: true removes the tab. The result reports the vault it filed into.\n\nWhen the vault can be checked, a fresh note for the clipped page is confirmed on disk before anything is closed: `clipVerified: true` means one was found, and a clip that provably never reached Obsidian is reported as an error with the tab left open. `clipVerified: false` means the vault could not be checked at all — the clip was still handed over, and the close, if asked for, still happened. How much a verified clip proves depends on whether the result carries a `contentHash`: with one, the note on disk is this exact clip's text, so it holds even against another agent session clipping the same URL at the same moment; without one (an older extension), it means only \"a fresh note for this page landed just now\", and concurrent clips of the same URL from separate sessions can share one note as evidence.", inputSchema: { type: "object", properties: { diff --git a/src/bridge-methods.ts b/src/bridge-methods.ts index b48245f..d5a22df 100644 --- a/src/bridge-methods.ts +++ b/src/bridge-methods.ts @@ -7,6 +7,7 @@ // scripting beyond the existing Defuddle clipper, and every close is logged // before it happens. +import { clipContentHash } from "./clip-hash.js"; import { markdownForClip, OBSIDIAN_HANDOFF_GAP_MS, resolveClipRequest } from "./clip-format.js"; import type { ClipPayload } from "./clip-format.js"; import { @@ -58,21 +59,6 @@ const STALE_ID_HINT = "It may have been closed, or unloaded and given a new id (Chrome does this when it discards a tab). Re-run tabs_list for current ids."; /** The single place the hint is attached — throwing and per-tab paths alike. */ -/** - * SHA-256 of a note's text, hex, as `TabClipResult.contentHash`. - * - * The extension cannot check whether Obsidian took the handoff, but it can say - * exactly what it handed over, and that is enough for a sidecar sitting beside - * the vault to tell this clip's note from a concurrent clip of the same page. - * `crypto.subtle` is available in both the event page and the service worker. - */ -async function sha256Hex(text: string): Promise { - const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(text)); - return Array.from(new Uint8Array(digest)) - .map((byte) => byte.toString(16).padStart(2, "0")) - .join(""); -} - function missingTabReason(message: string): string { return `${message} ${STALE_ID_HINT}`; } @@ -657,7 +643,7 @@ export class BridgeMethodRunner { url: payload.url, file, vault, - contentHash: await sha256Hex(content), + contentHash: await clipContentHash(content), }; if (!params.close) return { ...filed, closed: false }; diff --git a/src/clip-hash.ts b/src/clip-hash.ts new file mode 100644 index 0000000..1ec061c --- /dev/null +++ b/src/clip-hash.ts @@ -0,0 +1,41 @@ +// The extension half of the clip's content fingerprint. Gullet's half is +// `gullet/src/clip-verify.ts`'s `clipContentHash`, and the two are pinned +// together by `tests/clip-source.test.ts` — which imports *this* module, not a +// copy of it, because a divergence here reports every clip as unconfirmed and +// nothing else in the suite would notice. +// +// Kept out of `bridge-methods.ts` purely so it is reachable from `bun test`: +// that module's import graph reaches `browser`, this one is pure. + +/** + * Line endings are transport, not content. + * + * In the default clipboard clip mode the note's text goes to Obsidian through + * the OS clipboard, and Windows carries plain text as CF_UNICODETEXT with CRLF + * endings — so the LF the extension composed and hashed is not what Obsidian + * writes to disk. Hashing the normalized form on both sides costs the ability + * to tell two clips apart by their line endings alone (there is no such pair) + * and buys back the whole Windows clipboard path, which would otherwise report + * every successful clip as `mismatched` and leave every requested close undone. + */ +function normalizeLineEndings(text: string): string { + return text.replaceAll("\r\n", "\n"); +} + +/** + * SHA-256 of a clip's text, hex, as `TabClipResult.contentHash`. + * + * The extension cannot check whether Obsidian took the handoff, but it can say + * exactly what it handed over, and that is enough for a sidecar sitting beside + * the vault to tell this clip's note from a concurrent clip of the same page. + * `crypto.subtle` is available in both the event page and the service worker. + */ +export async function clipContentHash(text: string): Promise { + const digest = await crypto.subtle.digest( + "SHA-256", + new TextEncoder().encode(normalizeLineEndings(text)), + ); + return Array.from(new Uint8Array(digest)) + .map((byte) => byte.toString(16).padStart(2, "0")) + .join(""); +} diff --git a/tests/clip-source.test.ts b/tests/clip-source.test.ts index e5a0dc6..421dc08 100644 --- a/tests/clip-source.test.ts +++ b/tests/clip-source.test.ts @@ -10,6 +10,10 @@ import { describe, expect, test } from "bun:test"; import { markdownForClip, type ClipPayload } from "../src/clip-format.js"; +// The production extension-side helper, imported rather than re-implemented: a +// local copy would only prove Bun agrees with the test, leaving the suite green +// through exactly the drift it exists to catch. +import { clipContentHash as extensionClipHash } from "../src/clip-hash.js"; import { clipContentHash, noteSourceUrl } from "../gullet/src/clip-verify.js"; function payload(overrides: Partial = {}): ClipPayload { @@ -58,16 +62,9 @@ describe("frontmatter source, as Gullet reads it", () => { * be every clip reported as unconfirmed. */ describe("content hash, across the extension/sidecar boundary", () => { - async function webCryptoSha256Hex(text: string): Promise { - const digest = await crypto.subtle.digest("SHA-256", new TextEncoder().encode(text)); - return Array.from(new Uint8Array(digest)) - .map((byte) => byte.toString(16).padStart(2, "0")) - .join(""); - } - test("the browser's digest of a clip matches the one Gullet takes of the file", async () => { const note = markdownForClip(payload()); - expect(await clipContentHash(note)).toBe(await webCryptoSha256Hex(note)); + expect(await clipContentHash(note)).toBe(await extensionClipHash(note)); }); // Non-ASCII is where an encoding mismatch would surface — a real clip is full @@ -76,6 +73,24 @@ describe("content hash, across the extension/sidecar boundary", () => { const note = markdownForClip( payload({ title: "Ünïcödé — 🐊 “smart” quotes", markdown: "Ünïcödé body 🐊" }), ); - expect(await clipContentHash(note)).toBe(await webCryptoSha256Hex(note)); + expect(await clipContentHash(note)).toBe(await extensionClipHash(note)); + }); + + // The Windows clipboard route: the extension hashes the LF text it composed, + // but plain text crosses the OS clipboard as CF_UNICODETEXT with CRLF, so the + // note Obsidian writes has endings the extension never produced. Unnormalized, + // that is every clip on Windows reported as `mismatched` with its close undone. + test("a CRLF note on disk still hashes to the LF clip that produced it", async () => { + const note = markdownForClip(payload({ markdown: "First line.\n\nSecond line." })); + const asWindowsWroteIt = note.replaceAll("\n", "\r\n"); + expect(asWindowsWroteIt).not.toBe(note); + expect(await clipContentHash(asWindowsWroteIt)).toBe(await extensionClipHash(note)); + }); + + // The weakening stops there: line endings are transport, anything else is not. + test("still tells two clips apart by their text", async () => { + const a = markdownForClip(payload({ markdown: "Body." })); + const b = markdownForClip(payload({ markdown: "Body!" })); + expect(await clipContentHash(a)).not.toBe(await extensionClipHash(b)); }); });