From dd5d3af4c7b67d8dc1b84d75a73083773a807c0c Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 10:48:02 -0700 Subject: [PATCH 01/26] feat(share): parseShareFragment helper extracts k and t from URL hash --- web/src/lib/paste/identity.test.ts | 30 +++++++++++++++++++++++++++++- web/src/lib/paste/identity.ts | 11 +++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/web/src/lib/paste/identity.test.ts b/web/src/lib/paste/identity.test.ts index 7e5f83b..de1cbc1 100644 --- a/web/src/lib/paste/identity.test.ts +++ b/web/src/lib/paste/identity.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom import { describe, it, expect, beforeEach } from 'vitest'; -import { getReviewerName, setReviewerName, clearReviewerName } from './identity'; +import { getReviewerName, setReviewerName, clearReviewerName, parseShareFragment } from './identity'; describe('identity', () => { beforeEach(() => localStorage.clear()); @@ -21,3 +21,31 @@ describe('identity', () => { expect(getReviewerName()).toBe('Carol'); }); }); + +describe('parseShareFragment', () => { + it('parses k only', () => { + expect(parseShareFragment('#k=abc')).toEqual({ k: 'abc', t: null }); + }); + + it('parses k and t', () => { + expect(parseShareFragment('#k=abc&t=xyz')).toEqual({ k: 'abc', t: 'xyz' }); + }); + + it('order-independent', () => { + expect(parseShareFragment('#t=xyz&k=abc')).toEqual({ k: 'abc', t: 'xyz' }); + }); + + it('handles missing leading #', () => { + expect(parseShareFragment('k=abc&t=xyz')).toEqual({ k: 'abc', t: 'xyz' }); + }); + + it('returns null for missing keys', () => { + expect(parseShareFragment('')).toEqual({ k: null, t: null }); + expect(parseShareFragment('#')).toEqual({ k: null, t: null }); + expect(parseShareFragment('#other=foo')).toEqual({ k: null, t: null }); + }); + + it('treats empty values as null', () => { + expect(parseShareFragment('#k=&t=')).toEqual({ k: null, t: null }); + }); +}); diff --git a/web/src/lib/paste/identity.ts b/web/src/lib/paste/identity.ts index 420b672..abe35a5 100644 --- a/web/src/lib/paste/identity.ts +++ b/web/src/lib/paste/identity.ts @@ -14,3 +14,14 @@ export function clearReviewerName(): void { if (typeof localStorage === 'undefined') return; localStorage.removeItem(KEY); } + +export function parseShareFragment(hash: string): { k: string | null; t: string | null } { + const raw = hash.startsWith('#') ? hash.slice(1) : hash; + const params = new URLSearchParams(raw); + const k = params.get('k'); + const t = params.get('t'); + return { + k: k && k.length > 0 ? k : null, + t: t && t.length > 0 ? t : null + }; +} From 24ad0c3304d9f227768e8619f88fe279bab106e9 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 10:51:09 -0700 Subject: [PATCH 02/26] refactor(share): simplify parseShareFragment with || null --- web/src/lib/paste/identity.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/web/src/lib/paste/identity.ts b/web/src/lib/paste/identity.ts index abe35a5..1796b59 100644 --- a/web/src/lib/paste/identity.ts +++ b/web/src/lib/paste/identity.ts @@ -18,10 +18,8 @@ export function clearReviewerName(): void { export function parseShareFragment(hash: string): { k: string | null; t: string | null } { const raw = hash.startsWith('#') ? hash.slice(1) : hash; const params = new URLSearchParams(raw); - const k = params.get('k'); - const t = params.get('t'); return { - k: k && k.length > 0 ? k : null, - t: t && t.length > 0 ? t : null + k: params.get('k') || null, + t: params.get('t') || null }; } From 19ff61947779754da7bfe74cbcf5eb02cc583f03 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 10:52:20 -0700 Subject: [PATCH 03/26] refactor(share): use parseShareFragment in page onMount --- web/src/routes/share/[id]/+page.svelte | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index fefc35e..ae2c367 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -3,7 +3,7 @@ import { PasteClient, b64ToBytes, eventBytes } from '$lib/paste/client'; import { importKey, encryptJSON, decryptJSON } from '$lib/paste/crypto'; import { replayEvents, type CommentState } from '$lib/paste/events'; - import { getReviewerName } from '$lib/paste/identity'; + import { getReviewerName, parseShareFragment } from '$lib/paste/identity'; import type { PlanPlaintext, EventPlaintext, @@ -85,14 +85,14 @@ client = new PasteClient(window.location.origin); try { - const fragment = window.location.hash.replace(/^#/, ''); - const params = new URLSearchParams(fragment); - const k = params.get('k'); + const { k, t } = parseShareFragment(window.location.hash); if (!k) { loadError = 'Missing #k= in URL — share link is incomplete.'; return; } key = await importKey(k); + // `t` is unused in this commit; consumed in the next task. + void t; const resp = await client.get(data.id); plan = await decryptJSON( From 33e15a45c3372b8004ce1b5b7cfaf0e581530a88 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 10:54:49 -0700 Subject: [PATCH 04/26] feat(share): token-based isAuthor; auto-populate name from author URL --- web/src/routes/share/[id]/+page.svelte | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index ae2c367..4627c26 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -3,7 +3,7 @@ import { PasteClient, b64ToBytes, eventBytes } from '$lib/paste/client'; import { importKey, encryptJSON, decryptJSON } from '$lib/paste/crypto'; import { replayEvents, type CommentState } from '$lib/paste/events'; - import { getReviewerName, parseShareFragment } from '$lib/paste/identity'; + import { getReviewerName, parseShareFragment, setReviewerName } from '$lib/paste/identity'; import type { PlanPlaintext, EventPlaintext, @@ -32,6 +32,7 @@ // --- Reviewer identity --- let reviewerName = $state(null); + let authorToken = $state(null); let showNamePrompt = $state(false); let pendingAfterName: (() => void) | null = null; @@ -52,12 +53,7 @@ let client: PasteClient | undefined; - const isAuthor = $derived( - reviewerName !== null && - plan !== null && - !!plan.author_name && - plan.author_name === reviewerName - ); + const isAuthor = $derived(authorToken !== null); const orderedStates = $derived.by(() => { return [...comments.values()].sort((a, b) => @@ -91,8 +87,7 @@ return; } key = await importKey(k); - // `t` is unused in this commit; consumed in the next task. - void t; + authorToken = t; const resp = await client.get(data.id); plan = await decryptJSON( @@ -101,6 +96,14 @@ key ); + // Author URL flow: token + plan author name → auto-populate reviewer identity. + // If the share was created without --author, fall through to the standard + // reviewer flow; isAuthor still stays true via authorToken. + if (authorToken && plan?.author_name) { + reviewerName = plan.author_name; + setReviewerName(plan.author_name); + } + const events: EventPlaintext[] = []; for (const ev of resp.events) { const { blob, iv } = eventBytes(ev); From 5c3b012b7e687f26a6534b9c2bffc2415fa7535e Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 10:58:42 -0700 Subject: [PATCH 05/26] fix(share): clarify empty-name fallback; use isAuthor for resolve local-state --- web/src/routes/share/[id]/+page.svelte | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index 4627c26..9004182 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -97,8 +97,10 @@ ); // Author URL flow: token + plan author name → auto-populate reviewer identity. - // If the share was created without --author, fall through to the standard - // reviewer flow; isAuthor still stays true via authorToken. + // If the share was created without --author, fall through to the + // reviewerName already loaded from localStorage at the top of onMount — + // the lazy NamePromptModal will fire on first action. isAuthor still + // stays true via authorToken in that case. if (authorToken && plan?.author_name) { reviewerName = plan.author_name; setReviewerName(plan.author_name); @@ -315,7 +317,7 @@ await postEvent(event); const next = new Map(comments); const target = next.get(commentId); - if (target && plan?.author_name === reviewerName) { + if (target && isAuthor) { next.set(commentId, { ...target, status, reply, replyAt: event.created_at }); } comments = next; From 9a4bf3e1e362448e7dfbb708fafbdc5d4175920b Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:00:34 -0700 Subject: [PATCH 06/26] refactor(share): funnel author-only events through postAuthorEvent --- web/src/routes/share/[id]/+page.svelte | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index 9004182..3e89e7c 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -152,6 +152,12 @@ await client.appendEvent(data.id, blob, iv); } + async function postAuthorEvent(event: EventPlaintext) { + // Phase B: identical to postEvent. Phase C will add an auth header here + // and route the call through a server endpoint that verifies authorToken. + return postEvent(event); + } + function buildAnchor(sel: SelectionInfo): Anchor { return { line_start: sel.lineStart, @@ -287,7 +293,11 @@ suggested_text: suggestedText, created_at: new Date().toISOString() }; - await postEvent(event); + if (isAuthor && !isMyComment) { + await postAuthorEvent(event); + } else { + await postEvent(event); + } // Apply locally so the card updates without a round-trip refetch. const next = new Map(comments); @@ -314,7 +324,7 @@ author_name: reviewerName, created_at: new Date().toISOString() }; - await postEvent(event); + await postAuthorEvent(event); const next = new Map(comments); const target = next.get(commentId); if (target && isAuthor) { From 88e9877552ba625c549242f8569bef1a077b6b57 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:03:20 -0700 Subject: [PATCH 07/26] feat(share): drop Sign in button; chip becomes rename affordance --- web/src/routes/share/[id]/+page.svelte | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index 3e89e7c..690349d 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -379,16 +379,13 @@ {#if reviewerName} - - {reviewerName}{isAuthor ? ' · author' : ''} - - {:else} {/if} From 75fdab9e6b270ffdce0a33090c0d3012f24bdc9e Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:06:28 -0700 Subject: [PATCH 08/26] feat(share): NamePromptModal accepts initialName for rename flow --- web/src/routes/share/[id]/+page.svelte | 2 +- .../routes/share/[id]/components/NamePromptModal.svelte | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index 690349d..3ada009 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -460,6 +460,6 @@ {/if} {#if showNamePrompt} - + {/if} diff --git a/web/src/routes/share/[id]/components/NamePromptModal.svelte b/web/src/routes/share/[id]/components/NamePromptModal.svelte index e922a18..0a5166b 100644 --- a/web/src/routes/share/[id]/components/NamePromptModal.svelte +++ b/web/src/routes/share/[id]/components/NamePromptModal.svelte @@ -2,8 +2,11 @@ import { onMount, tick } from 'svelte'; import { setReviewerName } from '$lib/paste/identity'; - const { onSave }: { onSave: (name: string) => void } = $props(); - let name = $state(''); + const { + onSave, + initialName = '' + }: { onSave: (name: string) => void; initialName?: string } = $props(); + let name = $state(initialName); let input: HTMLInputElement | undefined = $state(); function save() { @@ -23,6 +26,7 @@ onMount(async () => { await tick(); input?.focus(); + input?.select(); }); From d2d4e8d92b81e8c82f88524a4d6ab4f2074884cc Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:09:14 -0700 Subject: [PATCH 09/26] feat(share): print share + author URLs from arc share create --- cmd/arc/share.go | 8 +++++-- cmd/arc/share_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/cmd/arc/share.go b/cmd/arc/share.go index 9432c4c..99fc535 100644 --- a/cmd/arc/share.go +++ b/cmd/arc/share.go @@ -254,8 +254,12 @@ func runShareCreate(cmd *cobra.Command, args []string) error { }); err != nil { return err } - fmt.Printf("Share URL: %s/share/%s#k=%s\n", strings.TrimRight(server, "/"), resp.ID, keyB64) - fmt.Printf("Edit token: %s (saved in ~/.arc/shares.json — keep safe)\n", resp.EditToken) + trimmedServer := strings.TrimRight(server, "/") + fmt.Printf("Share URL (send to reviewers):\n %s/share/%s#k=%s\n\n", + trimmedServer, resp.ID, keyB64) + fmt.Printf("Author URL (keep private — gives you Accept/Resolve):\n %s/share/%s#k=%s&t=%s\n\n", + trimmedServer, resp.ID, keyB64, resp.EditToken) + fmt.Println("Edit token saved to ~/.arc/shares.json") return nil } diff --git a/cmd/arc/share_test.go b/cmd/arc/share_test.go index 83baedf..c1c7094 100644 --- a/cmd/arc/share_test.go +++ b/cmd/arc/share_test.go @@ -516,6 +516,57 @@ func captureStdout(t *testing.T, fn func()) string { // global `configPath` at a temp file, and from the user's git identity by // running each subtest with $PATH cleared so `git` is unavailable (the // helper falls back silently to "" on git failure). +func TestShareCreatePrintsBothURLs(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + srv := startTestPasteServer(t) + defer srv.Close() + + plan := filepath.Join(t.TempDir(), "plan.md") + _ = os.WriteFile(plan, []byte("# Hello\n\nBody."), 0o600) + + // Capture stdout. + r, w, _ := os.Pipe() + stdout := os.Stdout + os.Stdout = w + defer func() { os.Stdout = stdout }() + + shareCreateServer = srv.URL + if err := runShareCreate(shareCreateCmd, []string{plan}); err != nil { + t.Fatalf("runShareCreate: %v", err) + } + _ = w.Close() + out, _ := io.ReadAll(r) + output := string(out) + + if !strings.Contains(output, "Share URL") { + t.Errorf("expected 'Share URL' label, got: %s", output) + } + if !strings.Contains(output, "Author URL") { + t.Errorf("expected 'Author URL' label, got: %s", output) + } + if !strings.Contains(output, "send to reviewers") { + t.Errorf("expected reviewer guidance, got: %s", output) + } + if !strings.Contains(output, "keep private") { + t.Errorf("expected privacy guidance for author URL, got: %s", output) + } + if strings.Contains(output, "Edit token: ") { + t.Errorf("raw 'Edit token: ' line should not appear in output: %s", output) + } + if !strings.Contains(output, "Edit token saved to") { + t.Errorf("expected pointer to shares.json, got: %s", output) + } + + // Author URL must contain &t=. + for _, line := range strings.Split(output, "\n") { + if strings.Contains(line, "/share/") && strings.Contains(line, "&t=") { + return + } + } + t.Errorf("expected an author URL line with &t=, got: %s", output) +} + func TestResolveAuthor(t *testing.T) { // Save & restore globals touched by the helper. Env vars use t.Setenv // which auto-restores; configPath is package-global so we restore manually. From 1e607b2fb0ac114c1abcae808645d6fe6075c782 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:10:10 -0700 Subject: [PATCH 10/26] test(share): prefer strings.SplitSeq over Split for ranging --- cmd/arc/share_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/arc/share_test.go b/cmd/arc/share_test.go index c1c7094..f72fa43 100644 --- a/cmd/arc/share_test.go +++ b/cmd/arc/share_test.go @@ -559,7 +559,7 @@ func TestShareCreatePrintsBothURLs(t *testing.T) { } // Author URL must contain &t=. - for _, line := range strings.Split(output, "\n") { + for line := range strings.SplitSeq(output, "\n") { if strings.Contains(line, "/share/") && strings.Contains(line, "&t=") { return } From 4de273b0912769e6e4b6b94a976d8153e7509836 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:12:45 -0700 Subject: [PATCH 11/26] feat(share): arc share show --author-url reprints the author URL Adds --author-url flag to `arc share show` that reads the registered share from ~/.arc/shares.json and prints the full author URL (with edit_token fragment) to stdout, so it can be piped into pbcopy/xclip by authors who lost their original CLI scrollback. --- cmd/arc/share.go | 22 ++++++++++++++++++ cmd/arc/share_test.go | 54 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/cmd/arc/share.go b/cmd/arc/share.go index 99fc535..b8ab98e 100644 --- a/cmd/arc/share.go +++ b/cmd/arc/share.go @@ -180,6 +180,7 @@ var ( shareCreateTitle string shareCommentsAccepted bool shareCommentsJSON bool + shareShowAuthorURL bool ) func init() { @@ -196,6 +197,8 @@ func init() { "Optional plan title shown in the share UI header (defaults to the filename)") shareCommentsCmd.Flags().BoolVar(&shareCommentsAccepted, "accepted-only", false, "Only print accepted comments") shareCommentsCmd.Flags().BoolVar(&shareCommentsJSON, "json", false, "Output as JSON") + shareShowCmd.Flags().BoolVar(&shareShowAuthorURL, "author-url", false, + "Print the author URL (includes edit_token) instead of the plan content") shareCmd.AddCommand(shareCreateCmd, shareListCmd, shareShowCmd, shareCommentsCmd, sharePullCmd, shareApproveCmd, shareUpdateCmd, shareDeleteCmd) @@ -279,6 +282,9 @@ func runShareList(cmd *cobra.Command, args []string) error { } func runShareShow(cmd *cobra.Command, args []string) error { + if shareShowAuthorURL { + return printAuthorURL(args[0]) + } id, server, key, err := resolveShareRef(args[0]) if err != nil { return err @@ -291,6 +297,22 @@ func runShareShow(cmd *cobra.Command, args []string) error { return nil } +func printAuthorURL(ref string) error { + // Resolve to id; accept either bare id or full share URL. + id, _, _, err := resolveShareRef(ref) + if err != nil { + return err + } + s, _ := sharesconfig.Find(id) + if s == nil || s.EditToken == "" || s.KeyB64Url == "" { + return fmt.Errorf("no edit_token for share %s in ~/.arc/shares.json "+ + "(--author-url requires a share registered on this machine)", id) + } + fmt.Printf("%s/share/%s#k=%s&t=%s\n", + strings.TrimRight(s.URL, "/"), id, s.KeyB64Url, s.EditToken) + return nil +} + func runShareComments(cmd *cobra.Command, args []string) error { id, server, key, err := resolveShareRef(args[0]) if err != nil { diff --git a/cmd/arc/share_test.go b/cmd/arc/share_test.go index f72fa43..1b704d1 100644 --- a/cmd/arc/share_test.go +++ b/cmd/arc/share_test.go @@ -711,6 +711,60 @@ func TestResolveServer(t *testing.T) { } } +func TestShareShowAuthorURL(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + srv := startTestPasteServer(t) + defer srv.Close() + + // Create a share so shares.json has an entry. + plan := filepath.Join(t.TempDir(), "plan.md") + _ = os.WriteFile(plan, []byte("# Hi"), 0o600) + shareCreateServer = srv.URL + if err := runShareCreate(shareCreateCmd, []string{plan}); err != nil { + t.Fatal(err) + } + f, _ := sharesconfig.Load() + id := f.Shares[0].ID + editToken := f.Shares[0].EditToken + keyB64 := f.Shares[0].KeyB64Url + + // Capture stdout for `arc share show --author-url`. + r, w, _ := os.Pipe() + stdout := os.Stdout + os.Stdout = w + defer func() { os.Stdout = stdout }() + + shareShowAuthorURL = true + defer func() { shareShowAuthorURL = false }() + if err := runShareShow(shareShowCmd, []string{id}); err != nil { + t.Fatalf("runShareShow: %v", err) + } + _ = w.Close() + out, _ := io.ReadAll(r) + got := strings.TrimSpace(string(out)) + + want := srv.URL + "/share/" + id + "#k=" + keyB64 + "&t=" + editToken + if got != want { + t.Errorf("author URL mismatch:\n got: %q\nwant: %q", got, want) + } +} + +func TestShareShowAuthorURLMissingShare(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + shareShowAuthorURL = true + defer func() { shareShowAuthorURL = false }() + err := runShareShow(shareShowCmd, []string{"abc12345"}) + if err == nil { + t.Fatal("expected error for missing share, got nil") + } + if !strings.Contains(err.Error(), "abc12345") { + t.Errorf("error should reference share id, got: %v", err) + } +} + // writeShareServerConfig points the global configPath at a temp cli-config.json // containing the given share_server (omitted entirely when empty). func writeShareServerConfig(t *testing.T, server string) { From d7697daf46301589314a5a655791e88ef2dd9341 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:15:13 -0700 Subject: [PATCH 12/26] docs(review): document share + author URLs, drop sign-in language --- docs/review_howto.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docs/review_howto.md b/docs/review_howto.md index 9c7f847..d6fec26 100644 --- a/docs/review_howto.md +++ b/docs/review_howto.md @@ -19,6 +19,21 @@ Mental shortcut: The discriminator is whether the comment should *cause an edit downstream*. Accept is the only path that does. Resolve and Reject both close the thread; the difference is whether the disagreement is worth recording — Reject preserves a reply, Resolve doesn't. +## URLs you'll receive + +`arc share create` prints two URLs: + +| URL | What it is | Who gets it | +|---|---|---| +| **Share URL** (`#k=…`) | Read + comment | Reviewers | +| **Author URL** (`#k=…&t=…`) | Adds Accept / Resolve / Reject | Plan author only | + +The author URL is a strict superset of the share URL — anyone given the author URL can also read and comment. Treat it like a write password: don't paste it into tickets, screenshots, or shared chat threads. The first time you open the author URL, the page detects your role from the URL itself; you don't sign in or pick a name. + +Lost the URL? Run `arc share show --author-url` to reprint it (uses the `edit_token` saved to `~/.arc/shares.json`). + +Reviewers see no sign-in screen. The first time a reviewer leaves a comment, a small modal asks for their name. The name is stored in this browser only. + ## Concrete examples ### Accept From 717cf4ba91cd43ae3eaf9663db4e7e61f4b0d3fc Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:18:25 -0700 Subject: [PATCH 13/26] docs(runbook): cover share+author URL manual scenarios Update paste-server runbook to reflect the frictionless auth UX: two-URL output from arc share create, token-based author detection, lazy reviewer identity, and the new Frictionless auth scenarios walkthrough section. --- docs/runbooks/paste-server.md | 62 +++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/docs/runbooks/paste-server.md b/docs/runbooks/paste-server.md index 5530c81..2c86abb 100644 --- a/docs/runbooks/paste-server.md +++ b/docs/runbooks/paste-server.md @@ -59,11 +59,18 @@ Validate the shared review feature. EOF ./bin/arc share create /tmp/test-plan.md --local -# → Share URL: http://localhost:7432/share/#k= -# → Edit token: (saved in ~/.arc/shares.json) - -./bin/arc share list # see all known shares -ls -la ~/.arc/shares.json # verify file mode 0600 +# Output: +# Share URL (send to reviewers): +# http://localhost:7432/share/#k= +# +# Author URL (keep private — gives you Accept/Resolve): +# http://localhost:7432/share/#k=&t= +# +# Edit token saved to ~/.arc/shares.json + +./bin/arc share list # see all known shares +ls -la ~/.arc/shares.json # verify file mode 0600 +./bin/arc share show --author-url # reprint the author URL on demand ``` The author identity embedded in the plan is resolved in this order (highest to lowest priority): @@ -73,7 +80,7 @@ The author identity embedded in the plan is resolved in this order (highest to l 3. `$ARC_SHARE_AUTHOR` env var 4. `git config user.name` -**Note the name that gets used** — you'll need to type it back in the UI to claim the author role. If none of these produce a value, `arc share create` prints a warning and Accept/Resolve/Reject controls won't appear for anyone. +**Note the name that gets used** — it is embedded in the share bundle and displayed automatically when the Author URL is opened. If none of these produce a value, `arc share create` prints a warning and Accept/Resolve/Reject controls won't appear for anyone. To set a persistent default once: @@ -87,16 +94,17 @@ echo '{"share_author":"Ben Firestone"}' | jq -s '.[0] * .[1]' ~/.arc/cli-config. ### Exercise the UI -Open the printed URL in Chrome/Firefox. +Open the **Author URL** (the one with `&t=`) in Chrome/Firefox. -1. **Name prompt** appears on first comment — type the **same name** that was embedded as the author at create time (i.e. your `git config user.name` output, or whatever you passed to `--author`). The reviewer-name chip in the header will read ` · author` once the names match. -2. **Highlight a paragraph** in the rendered plan → floating annotation toolbar appears -3. **Pick a label** (`praise` / `issue` / `suggestion` / `question` / `nit`) -4. **Type a comment**, optionally toggle "Suggest replacement text" -5. **Post** — the comment appears in the sidebar -6. As the author (your localStorage name matches `plan.author_name`), you'll see **Accept / Resolve / Reject** controls. Try Accept on one comment, Reject (with reply) on another, and Resolve on a third. If the controls don't appear, double-check that the name in the header chip matches the value in `git config user.name` (or whatever you passed to `--author`) — the comparison is case- and whitespace-sensitive. -7. As a reviewer (your localStorage name does NOT match the plan's author), find your own annotation in the sidebar. You should see an **✎ Edit** button on it. Click it; the body becomes a textarea pre-filled with your existing comment. Refine the wording — e.g. expand "expand this more" into a fully-formed suggestion — and **⌘/Ctrl-⏎** (or click Save). The card re-renders with `· edited Nm` next to the timestamp. Confirm `arc share comments ` prints the new body, not the original. -8. Switch back to the author window. The **✎ Edit** button should also appear on every reviewer's annotation (not just your own). Use it to sharpen a thin reviewer comment — the displayed `author_name` stays as the reviewer, only the body changes. Run `arc share comments ` again and confirm the refined body shows up; this is the path that lets the author shape feedback for downstream LLM consumption without round-trips with the reviewer. +1. **Open the Author URL.** The header chip immediately reads ` · author` (no modal). Author detection is now driven by the `&t=` fragment param, not a name-string match — so even if your localStorage holds a different name, the page knows you're the author and renders Accept/Resolve/Reject controls. (Remember the URL printed by `arc share create`; if you've lost it, run `arc share show --author-url`.) +2. **Highlight a paragraph** in the rendered plan → floating annotation toolbar appears. +3. **Pick a label** (`praise` / `issue` / `suggestion` / `question` / `nit`). +4. **Type a comment**, optionally toggle "Suggest replacement text". +5. **Post** — the first time you do this, a small modal asks for your name. (As the author you should already have a name from step 1; the modal won't fire.) The comment appears in the sidebar. +6. As the author, you'll see **Accept / Resolve / Reject** controls on every comment. Try Accept on one, Reject (with reply) on another, and Resolve on a third. If the controls don't appear, you opened the Share URL (no `&t=`) instead of the Author URL — close the tab and reopen via the Author URL. +7. As a reviewer (a fresh browser profile or incognito window with NO `&t=` in the URL), find your own annotation in the sidebar. You should see an **✎ Edit** button on it. Click it; the body becomes a textarea pre-filled with your existing comment. Refine the wording — e.g. expand "expand this more" into a fully-formed suggestion — and **⌘/Ctrl-⏎** (or click Save). The card re-renders with `· edited Nm` next to the timestamp. Confirm `arc share comments ` prints the new body, not the original. +8. Switch back to the author window. The **✎ Edit** button should also appear on every reviewer's annotation (not just your own). Use it to sharpen a thin reviewer comment — the displayed `author_name` stays as the reviewer, only the body changes. Run `arc share comments ` again and confirm the refined body shows up. +9. **Click the chip** in the header — the name prompt opens prefilled with your current name (text selected). Edit and save → chip updates. This is the rename affordance. ### Pull comments back to the CLI @@ -108,13 +116,25 @@ Open the printed URL in Chrome/Firefox. ### Simulate a second reviewer -Without spinning up another machine, open the same URL in an **incognito window** (or a different browser entirely). Incognito gets a fresh `localStorage`, so: +Without spinning up another machine, open the **Share URL (no `&t=`)** in an **incognito window** (or a different browser entirely). Incognito gets a fresh `localStorage`, so: + +1. **No chip** appears in the header. There's no "Sign in" button anywhere — identity is captured lazily. +2. Highlight a paragraph and click Comment in the toolbar. A name prompt fires; type any name (e.g. "Reviewer-2"). The comment posts; the chip now reads `Reviewer-2` (no `· author`, because the URL has no `&t=`). +3. Post a few more comments (the prompt won't fire again — name is sticky in localStorage). +4. Refresh the author's window → new comments replay in via `replayEvents()`. + +The author's resolution events still apply: their UI is gated on `authorToken` (in-memory, parsed from `&t=` in the fragment), not a name match. Reviewer-2 cannot resolve comments because Reviewer-2 doesn't have the token — the Accept/Resolve/Reject buttons are never rendered for that profile. + +### Frictionless auth scenarios -1. The name prompt fires again — type any name **other than** the embedded author name (e.g. "Reviewer-2"). The chip in the header should NOT show `· author`. -2. Post a few comments -3. Refresh the author's window → new comments replay in via `replayEvents()` +Walk these in order to confirm the new auth UX end-to-end: -The author's resolution events still apply (their `author_name` matches the plan's). Reviewer-2's comments cannot be marked as `accepted` by Reviewer-2 itself, even if they tried — the client filters out resolution events whose `author_name` doesn't match `plan.author_name`. +1. **Create a share.** `arc share create --local` — confirm output prints both Share URL and Author URL, no raw `Edit token: ` line, and a "saved to ~/.arc/shares.json" pointer. +2. **Reprint the author URL.** `arc share show --author-url` — single line, contains `&t=`. +3. **Author URL flow.** Open the Author URL in a fresh browser profile. Header chip shows ` · author` immediately, no modal. Accept / Resolve buttons visible on existing comments. +4. **Reviewer URL flow.** Open the Share URL (without `&t=`) in a different fresh profile. No chip in the header, no "Sign in" button. Select text → click Comment in the toolbar → name modal opens → save a name → comment posts → chip now shows the name. +5. **Rename via chip.** Click the chip on the reviewer side. Modal opens prefilled with the saved name (text selected). Edit, save → chip updates. +6. **Author opens the bare share URL.** Open the share URL (without `&t=`) on the author's browser. They are now in reviewer mode (no Accept buttons). Reopen via the Author URL → author mode restored. ## 3. Shared / remote review @@ -198,7 +218,7 @@ Step 7 (review loop) uses `arc share approve` and `arc share pull` instead of th | Symptom | Cause | Fix | |---|---|---| -| Accept / Resolve / Reject controls never appear, even when typing the "right" name | Plan was created without an author name (`arc share create` was run pre-fix, or `git config user.name` was empty and no `--author` flag passed). `plan.author_name` is empty, so `isAuthor` is `false` for every reviewer | Recreate the share with `--author "Your Name"` (or set `git config user.name` first), then enter that exact name in the SPA prompt | +| Accept / Resolve / Reject controls never appear | You opened the Share URL (no `&t=`) instead of the Author URL — author detection is token-based, not name-based. Or the share was created without an author name (`git config user.name` was empty and no `--author` flag passed), so `plan.author_name` is empty and the chip never shows `· author` | Close the tab and reopen via the Author URL (run `arc share show --author-url` to retrieve it). If the share has no author name, recreate it with `--author "Your Name"` | | `/share/` returns `{"message":"Not Found"}` (Echo's default 404 JSON) | Binary built without the `webui` build tag — `web.RegisterSPA` is the no-op stub | Rebuild with `make build` (not `make build-quick`); for arc-paste use `go build -tags webui -o ./bin/arc-paste ./arc-paste` | | `/share/` returns blank HTML / cannot find static assets | `web/build/` not present at compile time, even with the `webui` tag | Re-run `bun run build` in `web/`, then rebuild the binary | | SPA console says `missing #k= in URL` | URL was pasted without its fragment | Use the full URL printed by `arc share create` — fragments are dropped by some chat apps; copy carefully | From f20a2ae47c66c09056d5c603c5fc342a2d13b40f Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:23:46 -0700 Subject: [PATCH 14/26] docs(share): correct --author flag help and resolveAuthor doc to token gating --- cmd/arc/share.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/cmd/arc/share.go b/cmd/arc/share.go index b8ab98e..7582992 100644 --- a/cmd/arc/share.go +++ b/cmd/arc/share.go @@ -192,7 +192,9 @@ func init() { shareCreateCmd.Flags().StringVar(&shareCreateAuthor, "author", "", "Author name embedded in the plan (precedence: flag > share_author in "+ "cli-config.json > $ARC_SHARE_AUTHOR > `git config user.name`). "+ - "Reviewers entering this exact name gain Accept/Resolve/Reject controls.") + "Auto-populates the name chip when the Author URL is opened and is "+ + "used for display attribution. Author privileges are gated by the "+ + "&t= in the Author URL, not by this name.") shareCreateCmd.Flags().StringVar(&shareCreateTitle, "title", "", "Optional plan title shown in the share UI header (defaults to the filename)") shareCommentsCmd.Flags().BoolVar(&shareCommentsAccepted, "accepted-only", false, "Only print accepted comments") @@ -403,10 +405,11 @@ func runShareDelete(cmd *cobra.Command, args []string) error { // // Returns "" if none of these produce a value. // -// The author name is the only thing that lets the share UI distinguish the -// plan owner from reviewers — when a visitor enters this exact name in the -// SPA's name prompt, they get Accept/Resolve/Reject controls. Without it, -// nobody is recognized as the author and the controls stay hidden for all. +// The author name is used for: (a) auto-populating the name chip when the +// Author URL is opened in the SPA, (b) display attribution on resolution and +// edit events, and (c) the replay-time guard in events.ts that drops forged +// edits/resolutions. Author UI privileges (Accept/Resolve/Reject) are gated +// by the &t= in the Author URL fragment, not by this name. func resolveAuthor(flag string) string { if s := strings.TrimSpace(flag); s != "" { return s From 4270d130e8af50200e17664acebb61ed92fe3c36 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:43:52 -0700 Subject: [PATCH 15/26] fix(share): seed NamePromptModal name inside onMount to silence state-ref warning --- .../routes/share/[id]/components/NamePromptModal.svelte | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/web/src/routes/share/[id]/components/NamePromptModal.svelte b/web/src/routes/share/[id]/components/NamePromptModal.svelte index 0a5166b..8e4e1f5 100644 --- a/web/src/routes/share/[id]/components/NamePromptModal.svelte +++ b/web/src/routes/share/[id]/components/NamePromptModal.svelte @@ -6,7 +6,11 @@ onSave, initialName = '' }: { onSave: (name: string) => void; initialName?: string } = $props(); - let name = $state(initialName); + // Seed inside onMount so we don't snapshot the prop at the + // reactive-graph top level (svelte/state_referenced_locally). The + // modal is destroyed/recreated by `{#if showNamePrompt}`, so reading + // `initialName` once on mount captures the correct value each open. + let name = $state(''); let input: HTMLInputElement | undefined = $state(); function save() { @@ -24,6 +28,7 @@ } onMount(async () => { + name = initialName; await tick(); input?.focus(); input?.select(); From 83a441f355d232b61480b61f73248b8f4dd7b409 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 11:48:41 -0700 Subject: [PATCH 16/26] feat(share): add author-only Share-link stamp in the header Adds a top-right "Share link" button that copies the bare share URL (no &t=) to the reviewer's clipboard. Visible only when isAuthor is true (token present in fragment). Visual treatment: monospace uppercase stamp tied to the existing ink-on-paper aesthetic. On click, briefly switches to the warm-amber --ink-comment palette already established as the "noted" signal. A thin vertical hairline separates the stamp from the identity chip to read as one "header instruments" cluster. --- web/src/app.css | 73 ++++++++++++++++++++++ web/src/routes/share/[id]/+page.svelte | 86 +++++++++++++++++++++++--- 2 files changed, 149 insertions(+), 10 deletions(-) diff --git a/web/src/app.css b/web/src/app.css index 316be49..98fd128 100644 --- a/web/src/app.css +++ b/web/src/app.css @@ -1088,3 +1088,76 @@ padding: 0.25rem 0.625rem; border-radius: 9999px; } + +/* Author-only "Share link" stamp. + * + * Visual sibling of the mono "Plan · " label on the left of the header: + * monospace, uppercase, tight tracking. Distinguishes ACTION (verb, mono) from + * IDENTITY (noun, sans pill). Confirmation reuses the established amber + * --ink-comment palette — already the language of "noted / acknowledged" on + * this page — instead of inventing a new accent. */ +.share-page .share-stamp { + display: inline-flex; + align-items: center; + gap: 0.4rem; + min-width: 7.25rem; /* keeps width stable across "Share link" / "Copied" */ + justify-content: center; + font-family: "JetBrains Mono", ui-monospace, monospace; + font-size: 0.6875rem; + font-weight: 500; + letter-spacing: 0.14em; + text-transform: uppercase; + color: var(--ink-text-muted); + background: var(--ink-paper-raised); + border: 1px solid var(--ink-rule); + padding: 0.3rem 0.7rem 0.28rem; + border-radius: 4px; /* squarer than the chip — reads as a stamp, not a tag */ + cursor: pointer; + transition: + color 140ms ease, + border-color 140ms ease, + background-color 140ms ease, + transform 80ms ease, + opacity 80ms ease; +} +.share-page .share-stamp:hover { + color: var(--ink-text); + border-color: oklch(from var(--ink-rule) calc(l - 0.05) c h); +} +.share-page .share-stamp:active { + /* The "stamp press" — a 1px tap on click before the copied state takes + * over. Subliminal; you feel it more than see it. */ + transform: translateY(1px); + opacity: 0.85; +} +.share-page .share-stamp.is-copied { + color: var(--ink-comment); + background: var(--ink-comment-bg); + border-color: var(--ink-comment-edge); +} +.share-page .share-stamp-icon { + display: inline-flex; + width: 12px; + height: 12px; + flex: none; +} +.share-page .share-stamp-label { + display: inline-block; +} + +/* Header instruments group: a thin vertical hairline ties the stamp and the + * identity chip into one cluster on the right side of the header. The rule + * matches --ink-rule from the bottom border of the header itself. */ +.share-page .header-instruments > * + * { + position: relative; +} +.share-page .header-instruments > * + *::before { + content: ""; + position: absolute; + left: -0.625rem; /* halfway into the parent's gap-3 */ + top: 50%; + width: 1px; + height: 0.875rem; + background: var(--ink-rule); + transform: translateY(-50%); +} diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index 3ada009..0a695be 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -36,6 +36,10 @@ let showNamePrompt = $state(false); let pendingAfterName: (() => void) | null = null; + // --- Share-link copy (author-only) --- + let copiedShareLink = $state(false); + let copyResetTimer: ReturnType | null = null; + // --- UI state for selection-driven actions --- type SelectionInfo = { lineStart: number; @@ -146,6 +150,25 @@ sel?.removeAllRanges(); } + // Build the bare share URL (no &t=) from the current location and copy it. + // Author URL fragment carries both k and t; reviewers must only ever see k. + async function copyShareLink() { + const { k } = parseShareFragment(window.location.hash); + if (!k) return; + const url = `${window.location.origin}${window.location.pathname}#k=${k}`; + try { + await navigator.clipboard.writeText(url); + } catch { + return; + } + copiedShareLink = true; + if (copyResetTimer) clearTimeout(copyResetTimer); + copyResetTimer = setTimeout(() => { + copiedShareLink = false; + copyResetTimer = null; + }, 1500); + } + async function postEvent(event: EventPlaintext) { if (!key || !client) return; const { blob, iv } = await encryptJSON(event, key); @@ -378,16 +401,59 @@ {plan?.title ?? 'Untitled plan'} - {#if reviewerName} - - {/if} +
+ {#if isAuthor} + + {/if} + {#if reviewerName} + + {/if} +
{#if loadError} From 83f49611a9abb14bc11324b7d3d738de16455c3c Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 12:12:33 -0700 Subject: [PATCH 17/26] feat(share): inline name capture in FloatingToolbar (replaces lazy modal) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewers without a name no longer get a separate "What's your name?" modal mid-flow — a real footgun where users typed their comment text INTO the name field and got permanently identified by it. Now: when the floating toolbar opens for a no-name reviewer, it grows two extra rows. A monospace name input on top with an editorial proof-sheet baseline rule. The action icons in the middle stay visible but render at 0.35 opacity (cursor: not-allowed). Below the icons, a mono micro-caption: "ENTER YOUR NAME TO BEGIN". Click an icon while the field is empty: the input snaps to a 2px red outline (--ink-delete-edge), the row shakes laterally for ~250ms, focus moves to the input, and the caption text-swaps to "NAME REQUIRED". After ~700ms the row resets. Type a name + Enter (or click the commit arrow): name persists to localStorage, the two extra rows collapse, the toolbar reflows to its normal single-row icon strip and actions become live. ensureName() and pendingAfterName are now dead code — toolbar actions are guaranteed to have a name by the time they reach the page handler. NamePromptModal is still used for the chip-click rename flow (which is intentional and prefilled, no risk of confusion). --- web/src/app.css | 121 +++++++++++++++++ web/src/routes/share/[id]/+page.svelte | 64 ++++----- .../[id]/components/FloatingToolbar.svelte | 126 +++++++++++++++++- 3 files changed, 266 insertions(+), 45 deletions(-) diff --git a/web/src/app.css b/web/src/app.css index 98fd128..6925306 100644 --- a/web/src/app.css +++ b/web/src/app.css @@ -993,6 +993,127 @@ color 120ms ease-out; } +/* ── Inline name capture (when reviewer has no name yet) ───────────────── + * The toolbar grows two extra rows: a name input on top, a mono caption + * on the bottom. The icons in the middle are visually inert until the + * input has content. Editorial proof-sheet treatment — a baseline rule + * rather than a form border — keeps the aesthetic of writing on paper. */ + +.share-page .floating-toolbar.needs-name { + min-width: 220px; + padding: 0.5rem 0.625rem 0.4rem; +} + +.share-page .floating-toolbar .name-row { + display: flex; + align-items: center; + gap: 0.4rem; + padding-bottom: 0.45rem; + border-bottom: 1px dashed var(--ink-rule); + transition: + border-color 160ms ease, + transform 80ms ease; +} +/* Required-field treatment: thin red baseline (proof mark) + 2px outline + * pulse on the input itself. The shake fires once via the .is-error class + * being applied for ~700ms by the component. */ +.share-page .floating-toolbar .name-row.is-error { + border-bottom-color: var(--ink-delete-edge); + animation: name-row-shake 240ms cubic-bezier(0.36, 0.07, 0.19, 0.97) both; +} +@keyframes name-row-shake { + 0%, 100% { transform: translateX(0); } + 25% { transform: translateX(-5px); } + 75% { transform: translateX(5px); } +} + +.share-page .floating-toolbar .name-field { + flex: 1; + min-width: 0; + font-family: "JetBrains Mono", ui-monospace, monospace; + font-size: 0.75rem; + letter-spacing: 0.01em; + color: var(--ink-text); + background: transparent; + border: 0; + padding: 0.2rem 0.1rem; + outline: 0; + caret-color: var(--ink-comment); +} +.share-page .floating-toolbar .name-field::placeholder { + color: var(--ink-text-faint); + font-style: italic; +} +.share-page .floating-toolbar .name-row.is-error .name-field { + outline: 2px solid var(--ink-delete-edge); + outline-offset: 2px; + border-radius: 2px; + background: var(--ink-delete-bg); +} + +.share-page .floating-toolbar .name-commit { + display: inline-flex; + align-items: center; + justify-content: center; + width: 22px; + height: 22px; + padding: 0; + border: 1px solid var(--ink-rule); + border-radius: 4px; + background: var(--ink-paper); + color: var(--ink-text-faint); + cursor: pointer; + flex: none; + transition: + color 140ms ease, + border-color 140ms ease, + background-color 140ms ease; +} +.share-page .floating-toolbar .name-commit:hover { + color: var(--ink-text-muted); + border-color: oklch(from var(--ink-rule) calc(l - 0.05) c h); +} +/* When the field has substantive content, the commit arrow "wakes up" to + * the warm amber comment color — the same idiom we use elsewhere for + * "noted / acknowledged" — signaling the action is now available. */ +.share-page .floating-toolbar .name-commit.is-ready { + color: var(--ink-comment); + border-color: var(--ink-comment-edge); + background: var(--ink-comment-bg); +} + +/* Action row: when no name is set, dim the icons so they read as + * unavailable. Click handlers still fire (they trigger the name-row flash); + * the visual signal is intent only. */ +.share-page .floating-toolbar .action-row.is-locked { + opacity: 0.35; + pointer-events: auto; /* keep clickable so we can flash the name field */ +} +.share-page .floating-toolbar .action-row.is-locked button { + cursor: not-allowed; +} + +/* Mono micro-caption below the icon row. Default state is instructional; + * error state swaps the same line of text to a red directive. The text + * change is intentional rather than a separate slide-in toast — fewer + * moving parts, same information. */ +.share-page .floating-toolbar .name-caption { + margin-top: 0.4rem; + padding-top: 0.4rem; + border-top: 1px dashed var(--ink-rule); + font-family: "JetBrains Mono", ui-monospace, monospace; + font-size: 0.625rem; + font-weight: 500; + letter-spacing: 0.16em; + text-transform: uppercase; + color: var(--ink-text-faint); + text-align: center; + transition: color 180ms ease; +} +.share-page .floating-toolbar .name-caption.is-error { + color: var(--ink-delete); +} + /* Annotation card type chips */ .share-page .chip-comment { color: var(--ink-comment); diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index 0a695be..e75203b 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -34,7 +34,6 @@ let reviewerName = $state(null); let authorToken = $state(null); let showNamePrompt = $state(false); - let pendingAfterName: (() => void) | null = null; // --- Share-link copy (author-only) --- let copiedShareLink = $state(false); @@ -125,21 +124,9 @@ } }); - function ensureName(after: () => void) { - if (reviewerName) { - after(); - return; - } - pendingAfterName = after; - showNamePrompt = true; - } - function handleNameSaved(name: string) { reviewerName = name; showNamePrompt = false; - const cb = pendingAfterName; - pendingAfterName = null; - cb?.(); } function clearSelection() { @@ -217,51 +204,50 @@ comments = next; } + // Name capture moved into FloatingToolbar.svelte: when reviewerName is + // null, the toolbar renders an inline name field and gates its action + // icons on it. By the time we receive an action here, we know the name + // is set — so these handlers no longer need ensureName(). async function handleToolbarAction(action: ToolbarAction) { if (!activeSelection) return; const sel = activeSelection; switch (action) { case 'praise': - ensureName(async () => { - await createComment({ - body: 'Looks good', - comment_type: 'praise', - action: 'comment', - anchor: buildAnchor(sel) - }); - clearSelection(); + await createComment({ + body: 'Looks good', + comment_type: 'praise', + action: 'comment', + anchor: buildAnchor(sel) }); + clearSelection(); return; case 'comment': - ensureName(() => { - popoverMode = 'comment'; - }); + popoverMode = 'comment'; return; case 'delete': - ensureName(async () => { - await createComment({ - body: '', - comment_type: 'comment', - action: 'delete', - anchor: buildAnchor(sel) - }); - clearSelection(); + await createComment({ + body: '', + comment_type: 'comment', + action: 'delete', + anchor: buildAnchor(sel) }); + clearSelection(); return; case 'suggest': - ensureName(() => { - popoverMode = 'suggest'; - }); + popoverMode = 'suggest'; return; case 'quick-label': - ensureName(() => { - showQuickLabel = true; - }); + showQuickLabel = true; return; } } + function handleSetName(name: string) { + reviewerName = name; + setReviewerName(name); + } + async function handlePopoverSave(body: string, suggestedText?: string) { if (!activeSelection) return; await createComment({ @@ -504,6 +490,8 @@ anchorRect={activeSelection.rect} onAction={handleToolbarAction} onDismiss={clearSelection} + {reviewerName} + onSetName={handleSetName} /> {/if} diff --git a/web/src/routes/share/[id]/components/FloatingToolbar.svelte b/web/src/routes/share/[id]/components/FloatingToolbar.svelte index f136502..6a449fc 100644 --- a/web/src/routes/share/[id]/components/FloatingToolbar.svelte +++ b/web/src/routes/share/[id]/components/FloatingToolbar.svelte @@ -7,13 +7,69 @@ const { anchorRect, onAction, - onDismiss + onDismiss, + reviewerName, + onSetName }: { anchorRect: DOMRect; onAction: (a: ToolbarAction) => void; onDismiss: () => void; + reviewerName: string | null; + onSetName: (name: string) => void; } = $props(); + // Inline name capture — only rendered when the reviewer has no name yet. + // Replaces the legacy modal flow where typing a comment first then being + // asked for a name led to people answering with their comment text. + // Here the name input lives in the toolbar itself, sibling to the action + // icons; actions are visibly inert until the field has content. + let nameDraft = $state(''); + let nameError = $state(false); + let nameInput: HTMLInputElement | undefined = $state(); + const needsName = $derived(!reviewerName); + const nameReady = $derived(nameDraft.trim().length > 0); + + function commitName() { + const trimmed = nameDraft.trim(); + if (!trimmed) { + flashRequired(); + return; + } + onSetName(trimmed); + } + + function flashRequired() { + nameError = true; + nameInput?.focus(); + setTimeout(() => { + nameError = false; + }, 700); + } + + function handleNameKey(e: KeyboardEvent) { + if (e.key === 'Enter') { + e.preventDefault(); + commitName(); + } + } + + function tryAction(a: ToolbarAction) { + if (needsName) { + // Persist what they've typed if it's substantive — they may have + // just typed a name and reached for an action. Otherwise flash. + if (nameReady) { + commitName(); + // Don't auto-perform: let them deliberately click again now + // that the toolbar has woken up. Avoids accidentally posting + // "Praise" the moment the name input clears. + return; + } + flashRequired(); + return; + } + onAction(a); + } + let toolbar: HTMLDivElement | undefined = $state(); // Measured after the toolbar mounts (its width is content-driven, so we // can't know it ahead of time). We seed with a conservative upper bound @@ -73,16 +129,62 @@ From 6dff7350a69f1e4ea54bf62ac3bd459b8f66a0d5 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 12:27:06 -0700 Subject: [PATCH 18/26] feat(share): annotation retraction (delete) for original commenters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewers can now delete their own annotations, with the asymmetry discussed in design: only the original commenter retracts; plan author keeps Reject (with reply) for unwanted feedback so rationale survives in the audit trail. Mechanism: new RetractionEvent kind. Replay marks the target comment status='retracted' iff the event's author_name matches the comment's. Forged retractions (third-party or plan-author-targeting-someone-else) are silently dropped at replay, mirroring the EditEvent authorization. UI: trash icon next to ✎ Edit on the action bar, gated on canRetract (commenter matches reviewerName + status open/reopened). Click expands an inline confirm row mirroring the reject-reply pattern's footprint — prose explanation + Cancel / Yes-delete buttons in --ink-delete palette. No textarea (retraction has no rationale field). Retracted comments are filtered from: - Right-rail annotations panel - Inline document marks - `arc share comments` default output - JSON bundle for LLM consumers The encrypted retraction event remains in the log so the action is auditable; downstream consumers don't see (and don't act on) revoked material. Tests: - 5 new replay cases in events.test.ts (happy path + 3 forge cases + one chronological override edge case) - TestRunShareCommentsHidesRetracted in cmd/arc covering CLI default output filtering plus a forged retraction that must NOT take effect --- cmd/arc/share.go | 53 +++++++++++- cmd/arc/share_test.go | 71 ++++++++++++++++ web/src/lib/paste/events.test.ts | 39 ++++++++- web/src/lib/paste/events.ts | 18 +++- web/src/lib/paste/types.ts | 27 +++++- web/src/routes/share/[id]/+page.svelte | 39 ++++++++- .../[id]/components/AnnotationCard.svelte | 82 ++++++++++++++++++- .../[id]/components/AnnotationsPanel.svelte | 5 +- 8 files changed, 319 insertions(+), 15 deletions(-) diff --git a/cmd/arc/share.go b/cmd/arc/share.go index 7582992..d4007fe 100644 --- a/cmd/arc/share.go +++ b/cmd/arc/share.go @@ -98,6 +98,21 @@ type editEvent struct { CreatedAt string `json:"created_at"` } +// retractionEvent is the original commenter taking their annotation back. +// Replay marks the target comment with status='retracted'; the printers +// and JSON bundle filter retracted entries out of the default output so +// downstream LLM consumers don't act on revoked material. The encrypted +// event remains in the log as audit. Only an event whose author_name +// matches the target's author_name takes effect — plan author canNOT +// retract someone else's comment (they have Reject for that). +type retractionEvent struct { + Kind string `json:"kind"` // always "retraction" + ID string `json:"id"` + CommentID string `json:"comment_id"` + AuthorName string `json:"author_name"` + CreatedAt string `json:"created_at"` +} + // --- commands --- var shareCmd = &cobra.Command{ @@ -592,8 +607,8 @@ func printComments(server, id string, key []byte, acceptedOnly, asJSON bool) err return err } decoded := decodeAndSortEvents(events, key) - comments, resolutions := replayEvents(decoded, plan.AuthorName) - entries := buildCommentEntries(comments, resolutions, acceptedOnly) + comments, resolutions, retracted := replayEvents(decoded, plan.AuthorName) + entries := buildCommentEntries(comments, resolutions, retracted, acceptedOnly) if asJSON { return emitBundle(id, plan, entries) } @@ -638,9 +653,10 @@ func decodeAndSortEvents(events []paste.Event, key []byte) []decodedEvent { return out } -func replayEvents(events []decodedEvent, planAuthor string) (map[string]commentEvent, map[string]resolutionEvent) { +func replayEvents(events []decodedEvent, planAuthor string) (map[string]commentEvent, map[string]resolutionEvent, map[string]bool) { comments := map[string]commentEvent{} resolutions := map[string]resolutionEvent{} + retracted := map[string]bool{} for _, d := range events { switch d.kind { case "comment": @@ -649,9 +665,31 @@ func replayEvents(events []decodedEvent, planAuthor string) (map[string]commentE applyResolutionEvent(d.raw, planAuthor, resolutions) case "edit": applyEditEvent(d.raw, planAuthor, comments) + case "retraction": + applyRetractionEvent(d.raw, comments, retracted) } } - return comments, resolutions + return comments, resolutions, retracted +} + +// applyRetractionEvent marks the target comment as retracted iff the event's +// author_name matches the comment's original author_name. Plan author canNOT +// retract someone else's comment — they have Reject (with reply) for that +// purpose. Forged retractions are silently dropped, matching the edit-event +// authorization model. +func applyRetractionEvent(raw json.RawMessage, comments map[string]commentEvent, retracted map[string]bool) { + var r retractionEvent + if err := json.Unmarshal(raw, &r); err != nil { + return + } + target, ok := comments[r.CommentID] + if !ok { + return + } + if r.AuthorName != target.AuthorName { + return + } + retracted[r.CommentID] = true } func applyCommentEvent(raw json.RawMessage, comments map[string]commentEvent) { @@ -709,12 +747,19 @@ func applyEditEvent(raw json.RawMessage, planAuthor string, comments map[string] func buildCommentEntries( comments map[string]commentEvent, resolutions map[string]resolutionEvent, + retracted map[string]bool, acceptedOnly bool, ) []commentEntry { // Build a deterministic-order list of comment entries so multiple runs // produce identical output (Go map iteration is randomized). entries := make([]commentEntry, 0, len(comments)) for cid, c := range comments { + // Retracted comments are filtered from default output; the encrypted + // event still lives in the log for audit but downstream LLM consumers + // shouldn't see (and act on) revoked material. + if retracted[cid] { + continue + } status := "open" reply := "" if res, ok := resolutions[cid]; ok { diff --git a/cmd/arc/share_test.go b/cmd/arc/share_test.go index 1b704d1..ffc3a8f 100644 --- a/cmd/arc/share_test.go +++ b/cmd/arc/share_test.go @@ -108,6 +108,77 @@ func TestRunShareCommentsRoundTrip(t *testing.T) { } } +// TestRunShareCommentsHidesRetracted verifies that a retraction event from the +// comment's original author removes the comment from `arc share comments` +// default output, while a forged retraction (mismatched author_name) is +// silently dropped at replay time. The encrypted retraction event remains in +// the log for audit. +func TestRunShareCommentsHidesRetracted(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + srv := startTestPasteServer(t) + defer srv.Close() + + plan := filepath.Join(t.TempDir(), "p.md") + _ = os.WriteFile(plan, []byte("# P"), 0o600) + shareCreateServer = srv.URL + if err := runShareCreate(shareCreateCmd, []string{plan}); err != nil { + t.Fatal(err) + } + f, _ := sharesconfig.Load() + s := f.Shares[0] + keyBytes := mustDecodeKey(t, s.KeyB64Url) + + postEv := func(t *testing.T, payload map[string]any) { + t.Helper() + blob, iv, err := paste.EncryptJSON(payload, keyBytes) + if err != nil { + t.Fatalf("encrypt: %v", err) + } + body, _ := json.Marshal(paste.AppendEventRequest{Blob: blob, IV: iv}) + if err := postRaw(t, srv.URL+"/api/paste/"+s.ID+"/blobs", body); err != nil { + t.Fatal(err) + } + } + + // Two comments: Alice's (which she'll retract) and Bob's (which she can't). + postEv(t, map[string]any{ + "kind": "comment", "id": "c1", "author_name": "Alice", "comment_type": "comment", + "body": "regret posting this", + "anchor": map[string]any{"line_start": 1, "line_end": 1, "quoted_text": "P"}, + "created_at": "2026-04-29T00:00:00Z", + }) + postEv(t, map[string]any{ + "kind": "comment", "id": "c2", "author_name": "Bob", "comment_type": "comment", + "body": "stays visible", + "anchor": map[string]any{"line_start": 1, "line_end": 1, "quoted_text": "P"}, + "created_at": "2026-04-29T00:01:00Z", + }) + + // Alice retracts her own — must take effect. + postEv(t, map[string]any{ + "kind": "retraction", "id": "x1", "comment_id": "c1", "author_name": "Alice", + "created_at": "2026-04-29T00:02:00Z", + }) + // Mallory tries to retract Bob's — must be silently dropped. + postEv(t, map[string]any{ + "kind": "retraction", "id": "x2", "comment_id": "c2", "author_name": "Mallory", + "created_at": "2026-04-29T00:03:00Z", + }) + + out := captureStdout(t, func() { _ = runShareComments(shareCommentsCmd, []string{s.ID}) }) + + if strings.Contains(out, "regret posting this") { + t.Errorf("retracted comment must be hidden; got:\n%s", out) + } + if !strings.Contains(out, "stays visible") { + t.Errorf("forged retraction should not have removed Bob's comment; got:\n%s", out) + } + if !strings.Contains(out, "Bob") { + t.Errorf("expected Bob's comment to still be present; got:\n%s", out) + } +} + // TestRunShareCommentsAppliesEdits verifies that an `edit` event from the // comment's original author rewrites the body shown by `arc share comments`. // This locks in CLI parity with the SPA's replay logic. diff --git a/web/src/lib/paste/events.test.ts b/web/src/lib/paste/events.test.ts index 614409d..7bf5217 100644 --- a/web/src/lib/paste/events.test.ts +++ b/web/src/lib/paste/events.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; import { replayEvents, acceptedOnly } from './events'; -import type { CommentEvent, EditEvent, ResolutionEvent } from './types'; +import type { CommentEvent, EditEvent, ResolutionEvent, RetractionEvent } from './types'; const c: CommentEvent = { kind: 'comment', @@ -192,4 +192,41 @@ describe('replayEvents', () => { }); }); }); + + describe('retraction events', () => { + const retract: RetractionEvent = { + kind: 'retraction', + id: 'x1', + comment_id: 'c1', + author_name: 'Alice', + created_at: '2026-04-29T00:08:00Z' + }; + + it('marks comment retracted when original author retracts', () => { + const states = replayEvents('Ben', [c, retract]); + expect(states.get('c1')?.status).toBe('retracted'); + }); + + it('ignores retraction by someone other than the original commenter', () => { + // Plan author Ben tries to retract Alice's comment — must be silently dropped. + const states = replayEvents('Ben', [c, { ...retract, author_name: 'Ben' }]); + expect(states.get('c1')?.status).toBe('open'); + }); + + it('ignores forged retraction by a third party', () => { + const states = replayEvents('Ben', [c, { ...retract, author_name: 'Mallory' }]); + expect(states.get('c1')?.status).toBe('open'); + }); + + it('ignores retraction targeting a non-existent comment', () => { + const states = replayEvents('Ben', [c, { ...retract, comment_id: 'nope' }]); + expect(states.get('c1')?.status).toBe('open'); + }); + + it('retraction overrides a prior accept (chronological replay)', () => { + // Edge case: author accepts, reviewer then retracts. Last write wins. + const states = replayEvents('Ben', [c, accept, retract]); + expect(states.get('c1')?.status).toBe('retracted'); + }); + }); }); diff --git a/web/src/lib/paste/events.ts b/web/src/lib/paste/events.ts index 205e12f..4a0df3f 100644 --- a/web/src/lib/paste/events.ts +++ b/web/src/lib/paste/events.ts @@ -1,8 +1,16 @@ import type { CommentEvent, EventPlaintext, ResolutionStatus } from './types'; +/** + * 'retracted' is added to the status union for original-commenter retractions. + * It is intentionally distinct from author-driven 'rejected': retraction means + * "I take this back, never mind"; reject means "the author considered this + * and explicitly disagreed." Different semantics, different consumers. + */ +export type CommentStatus = 'open' | ResolutionStatus | 'retracted'; + export type CommentState = { event: CommentEvent; - status: 'open' | ResolutionStatus; + status: CommentStatus; reply?: string; replyAt?: string; /** @@ -58,6 +66,14 @@ export function replayEvents( comment_type: e.comment_type !== undefined ? e.comment_type : target.event.comment_type }; target.editedAt = e.created_at; + } else if (e.kind === 'retraction') { + const target = states.get(e.comment_id); + if (!target) continue; + // Only the original commenter can retract. Plan author canNOT — + // they have Reject (with reply) for that purpose, which preserves + // rationale in the audit trail. + if (e.author_name !== target.event.author_name) continue; + target.status = 'retracted'; } } return states; diff --git a/web/src/lib/paste/types.ts b/web/src/lib/paste/types.ts index 2ebc9a6..88ccea1 100644 --- a/web/src/lib/paste/types.ts +++ b/web/src/lib/paste/types.ts @@ -121,4 +121,29 @@ export type PlanEditEvent = { created_at: string; }; -export type EventPlaintext = CommentEvent | ResolutionEvent | EditEvent | PlanEditEvent; +/** + * The original commenter retracting their own annotation. Replay marks the + * target comment with status='retracted'; UI hides retracted comments and + * their inline marks. The encrypted event stays in the log so the action is + * auditable, but `arc share comments` filters retracted entries out of the + * default output (LLM consumers shouldn't act on retracted material). + * + * Authorization is replay-time: only an event whose `author_name` matches + * the target comment's `author_name` takes effect. The plan author canNOT + * retract someone else's comment — they must use Reject (with a reply that + * preserves rationale in the audit trail). + */ +export type RetractionEvent = { + kind: 'retraction'; + id: string; + comment_id: string; + author_name: string; + created_at: string; +}; + +export type EventPlaintext = + | CommentEvent + | ResolutionEvent + | EditEvent + | RetractionEvent + | PlanEditEvent; diff --git a/web/src/routes/share/[id]/+page.svelte b/web/src/routes/share/[id]/+page.svelte index e75203b..d8f1c76 100644 --- a/web/src/routes/share/[id]/+page.svelte +++ b/web/src/routes/share/[id]/+page.svelte @@ -12,6 +12,7 @@ EditEvent, ResolutionEvent, ResolutionStatus, + RetractionEvent, Anchor } from '$lib/paste/types'; import PlanRenderer from './components/PlanRenderer.svelte'; @@ -59,15 +60,20 @@ const isAuthor = $derived(authorToken !== null); const orderedStates = $derived.by(() => { - return [...comments.values()].sort((a, b) => - b.event.created_at.localeCompare(a.event.created_at) - ); + return [...comments.values()] + .filter((s) => s.status !== 'retracted') + .sort((a, b) => b.event.created_at.localeCompare(a.event.created_at)); }); const marks = $derived.by((): InlineMark[] => { const out: InlineMark[] = []; for (const state of comments.values()) { - if (state.status === 'resolved' || state.status === 'rejected') continue; + if ( + state.status === 'resolved' || + state.status === 'rejected' || + state.status === 'retracted' + ) + continue; out.push({ id: state.event.id, kind: state.event.action === 'delete' ? 'delete' : 'comment', @@ -342,6 +348,30 @@ comments = next; } + // The original commenter retracting their own annotation. Goes through the + // regular postEvent (not postAuthorEvent) — retraction is a reviewer-side + // authority, not an author privilege, so a future Phase C server-enforced + // auth would NOT gate retraction behind the author token. + async function handleRetract(commentId: string) { + if (!reviewerName) return; + const target = comments.get(commentId); + if (!target) return; + // Replay also enforces this; failing fast here avoids posting events + // the replay would silently drop. + if (target.event.author_name !== reviewerName) return; + const event: RetractionEvent = { + kind: 'retraction', + id: `x-${crypto.randomUUID()}`, + comment_id: commentId, + author_name: reviewerName, + created_at: new Date().toISOString() + }; + await postEvent(event); + const next = new Map(comments); + next.set(commentId, { ...target, status: 'retracted' }); + comments = next; + } + function handleSelection(sel: SelectionInfo | null) { if (!sel) { if (!popoverMode && !showQuickLabel) activeSelection = null; @@ -476,6 +506,7 @@ onCardClick={handleCardClick} onResolve={handleResolve} onEdit={handleEdit} + onRetract={handleRetract} /> diff --git a/web/src/routes/share/[id]/components/AnnotationCard.svelte b/web/src/routes/share/[id]/components/AnnotationCard.svelte index d96ad1d..f4d9369 100644 --- a/web/src/routes/share/[id]/components/AnnotationCard.svelte +++ b/web/src/routes/share/[id]/components/AnnotationCard.svelte @@ -11,7 +11,8 @@ isActive = false, onClick, onResolve, - onEdit + onEdit, + onRetract }: { entry: CommentState; isAuthor: boolean; @@ -20,6 +21,7 @@ onClick: () => void; onResolve: (status: ResolutionStatus, reply?: string) => Promise; onEdit: (body: string, suggestedText: string | undefined) => Promise; + onRetract: () => Promise; } = $props(); const modKey = modifierGlyph(); @@ -101,6 +103,30 @@ (entry.status === 'open' || entry.status === 'reopened') ); + // Retraction is strictly the original commenter's authority — author + // keeps Reject (with reply) for unwanted feedback so rationale survives + // in the audit trail. Same status window as canEdit: only meaningful + // while the comment is still under discussion. + const canRetract = $derived( + reviewerName !== null && + e.author_name === reviewerName && + (entry.status === 'open' || entry.status === 'reopened') + ); + + let showRetractConfirm = $state(false); + let retracting = $state(false); + + async function confirmRetract() { + if (retracting) return; + retracting = true; + try { + await onRetract(); + } finally { + retracting = false; + showRetractConfirm = false; + } + } + async function startEdit() { editBody = e.body ?? ''; editSuggested = e.suggested_text ?? ''; @@ -302,7 +328,7 @@ - Reject: `--ink-delete` (editorial red — load-bearing no) - Resolve / Reopen: muted (closing actions, no decision weight) --> - {#if (canEdit || isAuthor) && !isEditing && !showRejectReply} + {#if (canEdit || canRetract || isAuthor) && !isEditing && !showRejectReply && !showRetractConfirm}
{#if canEdit} + {/if} + + {#if (canEdit || canRetract) && isAuthor}
{/if} + + {#if showRetractConfirm} + +
+

+ Take this annotation back? It disappears from the rail and is excluded from + arc share comments output. +

+
+ + +
+
+ {/if} diff --git a/web/src/routes/share/[id]/components/AnnotationsPanel.svelte b/web/src/routes/share/[id]/components/AnnotationsPanel.svelte index f31d282..9c170ea 100644 --- a/web/src/routes/share/[id]/components/AnnotationsPanel.svelte +++ b/web/src/routes/share/[id]/components/AnnotationsPanel.svelte @@ -9,7 +9,8 @@ activeId, onCardClick, onResolve, - onEdit + onEdit, + onRetract }: { states: CommentState[]; isAuthor: boolean; @@ -22,6 +23,7 @@ reply?: string ) => Promise; onEdit: (commentId: string, body: string, suggestedText: string | undefined) => Promise; + onRetract: (commentId: string) => Promise; } = $props(); const visibleStates = $derived(states); // could filter by status later @@ -72,6 +74,7 @@ onClick={() => onCardClick(entry.event.id)} onResolve={(status, reply) => onResolve(entry.event.id, status, reply)} onEdit={(body, suggestedText) => onEdit(entry.event.id, body, suggestedText)} + onRetract={() => onRetract(entry.event.id)} /> {/each} From 1862c7e12e94d392a23edd6d1c720e3465304b2c Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 12:41:40 -0700 Subject: [PATCH 19/26] fix(share): inline marks for multi-block annotations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit selection.toString() inserts newlines between block-level elements, so a multi-block annotation's quotedText never matched any single block's textContent — the lineStart-only querySelector returned a block, the needle wasn't found in it, and the wrap silently failed. The card still showed in the rail; only the inline yellow mark was missing. Fix: when lineStart !== lineEnd, split quotedText on \n+ and wrap each segment in its corresponding [data-source-line] block, walked in source order. Single-block annotations keep the existing fast path. If segment count and block count diverge (rare — would mean the markdown renderer collapsed/dropped a block since the comment was posted), wrap as many as align rather than nothing — partial highlight beats a missing one. --- .../[id]/components/inline-annotations.ts | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/web/src/routes/share/[id]/components/inline-annotations.ts b/web/src/routes/share/[id]/components/inline-annotations.ts index 6a91ea5..94ced07 100644 --- a/web/src/routes/share/[id]/components/inline-annotations.ts +++ b/web/src/routes/share/[id]/components/inline-annotations.ts @@ -32,9 +32,35 @@ export function applyInlineAnnotations( clearMarks(container); for (const mark of marks) { - const block = container.querySelector(`[data-source-line="${mark.lineStart}"]`); - if (!block) continue; - wrapFirstOccurrence(block, mark, mark.id === activeId); + const isActive = mark.id === activeId; + // Single-block selection: existing fast path. Most annotations land here. + if (mark.lineStart === mark.lineEnd) { + const block = container.querySelector(`[data-source-line="${mark.lineStart}"]`); + if (block) wrapNeedleInBlock(block, mark.quotedText, mark, isActive); + continue; + } + // Multi-block selection. `selection.toString()` inserts \n (often \n\n) + // between block-level elements, so the needle won't be found inside any + // single block's textContent. Split on runs of newlines and wrap each + // segment in its corresponding block, walked in source-line order. + const segments = mark.quotedText + .split(/\n+/) + .map((s) => s.trim()) + .filter((s) => s.length > 0); + if (segments.length === 0) continue; + const blocks: HTMLElement[] = []; + for (let line = mark.lineStart; line <= mark.lineEnd; line++) { + const b = container.querySelector(`[data-source-line="${line}"]`); + if (b) blocks.push(b); + } + // Pair segments with blocks one-for-one. If counts diverge (rare — + // would mean the markdown renderer collapsed/dropped a block since + // the comment was created), wrap as many as we can rather than + // nothing — partial highlight beats a missing one. + const n = Math.min(segments.length, blocks.length); + for (let i = 0; i < n; i++) { + wrapNeedleInBlock(blocks[i], segments[i], mark, isActive); + } } } @@ -50,8 +76,12 @@ function clearMarks(container: HTMLElement): void { } } -function wrapFirstOccurrence(block: HTMLElement, mark: InlineMark, isActive: boolean): void { - const needle = mark.quotedText; +function wrapNeedleInBlock( + block: HTMLElement, + needle: string, + mark: InlineMark, + isActive: boolean +): void { if (!needle) return; // Walk text nodes in document order; track running offset so we can find the From aeaf15f0ccce9cba51cae3f63702aa784f92a4f7 Mon Sep 17 00:00:00 2001 From: Ben Firestone Date: Thu, 30 Apr 2026 12:48:17 -0700 Subject: [PATCH 20/26] fix(share): inline marks for blocks with internal multi-line structure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous fix split needle by \n+ and paired one segment per [data-source-line] block. That works for paragraph + paragraph but breaks for any block that has multi-line internal structure: -
    with several
  • s (one source line, many \n-separated items in selection.toString()) - 
     with many lines (one source line, many newlines in
    selection)

When segment count and block count diverge, fall back to wrapping every
non-whitespace text node across the blocks in range. Per-text-node
wrap (rather than a Range spanning sibling block elements) keeps the
list/code structure intact — ranges that cross 
  •  boundaries either
fail surroundContents or, on extractContents fallback, flatten the
list into one .

Trade-off: in the rare partial-mid-block multi-block selection that
hits the fallback, this over-highlights the partial blocks. Strictly
better than the no-highlight failure mode it replaces.
---
 .../[id]/components/inline-annotations.ts     | 74 ++++++++++++++++---
 1 file changed, 63 insertions(+), 11 deletions(-)

diff --git a/web/src/routes/share/[id]/components/inline-annotations.ts b/web/src/routes/share/[id]/components/inline-annotations.ts
index 94ced07..5128273 100644
--- a/web/src/routes/share/[id]/components/inline-annotations.ts
+++ b/web/src/routes/share/[id]/components/inline-annotations.ts
@@ -39,10 +39,9 @@ export function applyInlineAnnotations(
 			if (block) wrapNeedleInBlock(block, mark.quotedText, mark, isActive);
 			continue;
 		}
-		// Multi-block selection. `selection.toString()` inserts \n (often \n\n)
-		// between block-level elements, so the needle won't be found inside any
-		// single block's textContent. Split on runs of newlines and wrap each
-		// segment in its corresponding block, walked in source-line order.
+		// Multi-block selection. `selection.toString()` inserts \n between
+		// block-level elements, so the needle won't be found inside any
+		// single block's textContent.
 		const segments = mark.quotedText
 			.split(/\n+/)
 			.map((s) => s.trim())
@@ -53,13 +52,66 @@ export function applyInlineAnnotations(
 			const b = container.querySelector(`[data-source-line="${line}"]`);
 			if (b) blocks.push(b);
 		}
-		// Pair segments with blocks one-for-one. If counts diverge (rare —
-		// would mean the markdown renderer collapsed/dropped a block since
-		// the comment was created), wrap as many as we can rather than
-		// nothing — partial highlight beats a missing one.
-		const n = Math.min(segments.length, blocks.length);
-		for (let i = 0; i < n; i++) {
-			wrapNeedleInBlock(blocks[i], segments[i], mark, isActive);
+		if (blocks.length === 0) continue;
+
+		// Two strategies:
+		//   1. Strict pairing: segment count == block count, e.g. paragraph +
+		//      paragraph. Each segment goes in its corresponding block.
+		//   2. Fallback: counts diverge, which happens when a block has
+		//      internal multi-line structure — a 
       with several 
    • s, or
+		//      a 
       with many lines. The block has ONE source line
+		//      but the needle has many \n separators within. We wrap every
+		//      text node across the blocks in range.
+		// The fallback can over-highlight when the user partially selected
+		// the first or last block in range, but that's strictly better than
+		// the no-highlight failure mode it replaces.
+		if (segments.length === blocks.length) {
+			for (let i = 0; i < blocks.length; i++) {
+				wrapNeedleInBlock(blocks[i], segments[i], mark, isActive);
+			}
+		} else {
+			wrapAllTextNodesInBlocks(blocks, mark, isActive);
+		}
+	}
+}
+
+/**
+ * Wrap every non-empty text node inside the given blocks in a fresh .
+ *
+ * Used as the multi-block fallback when segment count and block count diverge
+ * — typically when a block has internal multi-line structure (lists, code
+ * blocks). Wrapping per text node (rather than spanning a Range across
+ * sibling block elements) avoids breaking the structure: ranges that cross
+ * 
    •  boundaries would either fail surroundContents or, on extractContents
+ * fallback, rip the list apart. Each text node is its own atom — wrapping it
+ * inline is always safe.
+ *
+ * Trade-off: if the original selection was partial inside the first or last
+ * block in range, this over-highlights those blocks. That's an acceptable
+ * regression of "exact selection visible" in exchange for restoring "any
+ * highlight at all" for the multi-line section / code-block cases.
+ */
+function wrapAllTextNodesInBlocks(
+	blocks: HTMLElement[],
+	mark: InlineMark,
+	isActive: boolean
+): void {
+	const className = CLASS_BY_KIND[mark.kind] + (isActive ? ' is-active' : '');
+	for (const block of blocks) {
+		// Snapshot text nodes before mutating; otherwise the wrapping inserts
+		// new  nodes and the live walker would visit them too.
+		const walker = document.createTreeWalker(block, NodeFilter.SHOW_TEXT);
+		const nodes: Text[] = [];
+		while (walker.nextNode()) nodes.push(walker.currentNode as Text);
+
+		for (const node of nodes) {
+			if (!node.data.trim()) continue; // skip whitespace-only nodes
+			if (!node.parentNode) continue;
+			const wrapper = document.createElement('mark');
+			wrapper.className = className;
+			wrapper.dataset.annoId = mark.id;
+			node.parentNode.insertBefore(wrapper, node);
+			wrapper.appendChild(node);
 		}
 	}
 }

From f79a578ed385546538e3e68a5146333b0d526369 Mon Sep 17 00:00:00 2001
From: Ben Firestone 
Date: Thu, 30 Apr 2026 12:55:30 -0700
Subject: [PATCH 21/26] fix(share): inline marks for partial mid-block
 selections across structure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two-tier match + two-tier wrap, replacing the special-cased single vs
multi-block paths.

Search (in this order):
  1. Exact substring of the flat concatenated text-node string.
  2. Whitespace-normalized match. Collapses runs of whitespace
     (including the \n that selection.toString() inserts between
     block-level elements), with a position map back to raw offsets so
     the wrap step can act on real text-node positions.

Wrap (in this order):
  1. Range surroundContents from start text node to end text node.
     Works when the range doesn't cross sibling block elements.
  2. Per-text-node wrap. Each text node's slice within [start, end) is
     wrapped in its own . Text-node ranges are always safe to
     surroundContents regardless of ancestor structure, so this
     preserves 
    • /
      / boundaries that the Range path can't
     surround.

Replaces the prior wrapAllTextNodesInBlocks fallback (which over-
highlighted entire blocks). The new path computes the actual matched
region and only wraps that — partial selections inside lists and code
blocks now show the correct extent of yellow markup.

The previous extractContents fallback (in the catch of surroundContents)
is removed: extracting across 
    •  boundaries flattened the list into
one , breaking structure. Per-text-node wrap is always safe.
---
 .../[id]/components/inline-annotations.ts     | 273 +++++++++++-------
 1 file changed, 163 insertions(+), 110 deletions(-)

diff --git a/web/src/routes/share/[id]/components/inline-annotations.ts b/web/src/routes/share/[id]/components/inline-annotations.ts
index 5128273..9b60e97 100644
--- a/web/src/routes/share/[id]/components/inline-annotations.ts
+++ b/web/src/routes/share/[id]/components/inline-annotations.ts
@@ -1,13 +1,27 @@
 /**
  * Apply inline annotation marks to already-rendered markdown.
  *
- * Strategy: walk text nodes inside each block (data-source-line) referenced by
- * the anchor, find the first occurrence of the anchor's quoted_text, and wrap
- * it in a  with the right CSS class. This is similar to plannotator's
- * web-highlighter but tailored to our line-anchored model.
+ * Strategy: for each annotation, gather the [data-source-line] blocks in
+ * [lineStart, lineEnd], walk all their text nodes in document order, and find
+ * the anchor's quoted_text. The needle from selection.toString() may contain
+ * \n separators between block-level elements (paragraphs, 
    • s, etc.) while
+ * the flat text walked via TreeWalker has none — so the search is two-tier:
  *
- * The implementation rebuilds the marks every render rather than diffing; this
- * is fine for the volume we expect (tens of annotations per plan).
+ *   1. Exact substring match (most annotations land here).
+ *   2. Whitespace-normalized match (collapses runs of whitespace, including
+ *      block-boundary newlines) with a position map back to raw offsets.
+ *
+ * Wrapping is also two-tier:
+ *
+ *   1. Range surroundContents on a single Range from start text node to end
+ *      text node — clean for ranges within one inline-friendly element.
+ *   2. Per-text-node wrap when (1) fails because the range crosses sibling
+ *      block elements like 
    • s. surroundContents throws in that case;
+ *      falling back to extractContents would rip the list structure apart,
+ *      so we instead wrap the matched slice of each individual text node
+ *      (which is always inline-safe).
+ *
+ * The implementation rebuilds the marks every render rather than diffing.
  */
 
 export type InlineMark = {
@@ -33,86 +47,13 @@ export function applyInlineAnnotations(
 
 	for (const mark of marks) {
 		const isActive = mark.id === activeId;
-		// Single-block selection: existing fast path. Most annotations land here.
-		if (mark.lineStart === mark.lineEnd) {
-			const block = container.querySelector(`[data-source-line="${mark.lineStart}"]`);
-			if (block) wrapNeedleInBlock(block, mark.quotedText, mark, isActive);
-			continue;
-		}
-		// Multi-block selection. `selection.toString()` inserts \n between
-		// block-level elements, so the needle won't be found inside any
-		// single block's textContent.
-		const segments = mark.quotedText
-			.split(/\n+/)
-			.map((s) => s.trim())
-			.filter((s) => s.length > 0);
-		if (segments.length === 0) continue;
 		const blocks: HTMLElement[] = [];
 		for (let line = mark.lineStart; line <= mark.lineEnd; line++) {
 			const b = container.querySelector(`[data-source-line="${line}"]`);
 			if (b) blocks.push(b);
 		}
 		if (blocks.length === 0) continue;
-
-		// Two strategies:
-		//   1. Strict pairing: segment count == block count, e.g. paragraph +
-		//      paragraph. Each segment goes in its corresponding block.
-		//   2. Fallback: counts diverge, which happens when a block has
-		//      internal multi-line structure — a 
         with several 
      • s, or
-		//      a 
         with many lines. The block has ONE source line
-		//      but the needle has many \n separators within. We wrap every
-		//      text node across the blocks in range.
-		// The fallback can over-highlight when the user partially selected
-		// the first or last block in range, but that's strictly better than
-		// the no-highlight failure mode it replaces.
-		if (segments.length === blocks.length) {
-			for (let i = 0; i < blocks.length; i++) {
-				wrapNeedleInBlock(blocks[i], segments[i], mark, isActive);
-			}
-		} else {
-			wrapAllTextNodesInBlocks(blocks, mark, isActive);
-		}
-	}
-}
-
-/**
- * Wrap every non-empty text node inside the given blocks in a fresh .
- *
- * Used as the multi-block fallback when segment count and block count diverge
- * — typically when a block has internal multi-line structure (lists, code
- * blocks). Wrapping per text node (rather than spanning a Range across
- * sibling block elements) avoids breaking the structure: ranges that cross
- * 
      •  boundaries would either fail surroundContents or, on extractContents
- * fallback, rip the list apart. Each text node is its own atom — wrapping it
- * inline is always safe.
- *
- * Trade-off: if the original selection was partial inside the first or last
- * block in range, this over-highlights those blocks. That's an acceptable
- * regression of "exact selection visible" in exchange for restoring "any
- * highlight at all" for the multi-line section / code-block cases.
- */
-function wrapAllTextNodesInBlocks(
-	blocks: HTMLElement[],
-	mark: InlineMark,
-	isActive: boolean
-): void {
-	const className = CLASS_BY_KIND[mark.kind] + (isActive ? ' is-active' : '');
-	for (const block of blocks) {
-		// Snapshot text nodes before mutating; otherwise the wrapping inserts
-		// new  nodes and the live walker would visit them too.
-		const walker = document.createTreeWalker(block, NodeFilter.SHOW_TEXT);
-		const nodes: Text[] = [];
-		while (walker.nextNode()) nodes.push(walker.currentNode as Text);
-
-		for (const node of nodes) {
-			if (!node.data.trim()) continue; // skip whitespace-only nodes
-			if (!node.parentNode) continue;
-			const wrapper = document.createElement('mark');
-			wrapper.className = className;
-			wrapper.dataset.annoId = mark.id;
-			node.parentNode.insertBefore(wrapper, node);
-			wrapper.appendChild(node);
-		}
+		wrapNeedleAcrossBlocks(blocks, mark.quotedText, mark, isActive);
 	}
 }
 
@@ -128,32 +69,65 @@ function clearMarks(container: HTMLElement): void {
 	}
 }
 
-function wrapNeedleInBlock(
-	block: HTMLElement,
+function wrapNeedleAcrossBlocks(
+	blocks: HTMLElement[],
 	needle: string,
 	mark: InlineMark,
 	isActive: boolean
 ): void {
 	if (!needle) return;
 
-	// Walk text nodes in document order; track running offset so we can find the
-	// first occurrence even when the needle spans multiple text nodes.
-	const walker = document.createTreeWalker(block, NodeFilter.SHOW_TEXT);
+	// Walk text nodes across all blocks in DOM order.
 	const textNodes: Text[] = [];
 	let acc = '';
-	while (walker.nextNode()) {
-		const t = walker.currentNode as Text;
-		textNodes.push(t);
-		acc += t.data;
+	for (const block of blocks) {
+		const walker = document.createTreeWalker(block, NodeFilter.SHOW_TEXT);
+		while (walker.nextNode()) {
+			const t = walker.currentNode as Text;
+			textNodes.push(t);
+			acc += t.data;
+		}
+	}
+	if (textNodes.length === 0) return;
+
+	// Two-tier search.
+	let start = -1;
+	let end = -1;
+	const exactOffset = acc.indexOf(needle);
+	if (exactOffset >= 0) {
+		start = exactOffset;
+		end = exactOffset + needle.length;
+	} else {
+		const norm = normalizeWithMap(acc);
+		const needleNorm = normalizeWS(needle);
+		if (!needleNorm) return;
+		const normOffset = norm.normalized.indexOf(needleNorm);
+		if (normOffset < 0) return; // anchor lost
+		start = norm.rawPositions[normOffset];
+		const lastNormIdx = normOffset + needleNorm.length - 1;
+		if (lastNormIdx >= norm.rawPositions.length) return;
+		end = norm.rawPositions[lastNormIdx] + 1;
 	}
 
-	const offset = acc.indexOf(needle);
-	if (offset < 0) return; // anchor lost; caller already showed a drift badge
-
-	const start = offset;
-	const end = offset + needle.length;
+	// Wrap. Try the contiguous Range first; on failure (range crosses sibling
+	// block elements like 
      • s), fall back to per-text-node wrap.
+	if (!tryRangeWrap(textNodes, start, end, mark, isActive)) {
+		wrapPerTextNode(textNodes, start, end, mark, isActive);
+	}
+}
 
-	// Find the start text node + offset within it.
+/**
+ * Attempts to wrap [start, end) of the concatenated text-node string in a
+ * single Range surroundContents. Works for ranges that don't cross sibling
+ * block elements. Returns false on any failure so the caller can fall back.
+ */
+function tryRangeWrap(
+	textNodes: Text[],
+	start: number,
+	end: number,
+	mark: InlineMark,
+	isActive: boolean
+): boolean {
 	let cum = 0;
 	let startNode: Text | null = null;
 	let startInner = 0;
@@ -172,30 +146,109 @@ function wrapNeedleInBlock(
 		}
 		cum = next;
 	}
-	if (!startNode || !endNode) return;
+	if (!startNode || !endNode) return false;
 
 	try {
 		const range = document.createRange();
 		range.setStart(startNode, startInner);
 		range.setEnd(endNode, endInner);
+		const wrapper = createMarkWrapper(mark, isActive);
+		range.surroundContents(wrapper);
+		return true;
+	} catch {
+		// surroundContents throws when the range crosses element boundaries
+		// it can't surround (e.g., from inside one 
      •  to inside another).
+		// We deliberately do NOT call extractContents here — that would rip
+		// the structure apart and put the list contents into one flat .
+		return false;
+	}
+}
 
-		const wrapper = document.createElement('mark');
-		wrapper.className = CLASS_BY_KIND[mark.kind] + (isActive ? ' is-active' : '');
-		wrapper.dataset.annoId = mark.id;
-
-		// `surroundContents` works for ranges that don't cross element boundaries.
-		// For multi-element ranges we extract+wrap+reinsert which works for inline
-		// content (text + simple inline elements like , ).
+/**
+ * Wraps the [start, end) slice of each text node that overlaps that range.
+ * Each text-node range is its own atom: surroundContents on a single text
+ * node is always safe regardless of the surrounding element structure, so
+ * this preserves 
      • /
        / boundaries when the contiguous Range path
+ * couldn't wrap across them.
+ */
+function wrapPerTextNode(
+	textNodes: Text[],
+	start: number,
+	end: number,
+	mark: InlineMark,
+	isActive: boolean
+): void {
+	let cum = 0;
+	for (const t of textNodes) {
+		const tStart = cum;
+		const tEnd = cum + t.data.length;
+		cum = tEnd;
+		const overlapStart = Math.max(start, tStart);
+		const overlapEnd = Math.min(end, tEnd);
+		if (overlapStart >= overlapEnd) continue;
+		const localStart = overlapStart - tStart;
+		const localEnd = overlapEnd - tStart;
+		// Skip slices that are pure whitespace (e.g., a "\n" between code-block
+		// lines that contributed only to the separator). They shouldn't get
+		// their own visible .
+		if (!t.data.slice(localStart, localEnd).trim()) continue;
+		if (!t.parentNode) continue;
 		try {
+			const range = document.createRange();
+			range.setStart(t, localStart);
+			range.setEnd(t, localEnd);
+			const wrapper = createMarkWrapper(mark, isActive);
 			range.surroundContents(wrapper);
 		} catch {
-			const frag = range.extractContents();
-			wrapper.appendChild(frag);
-			range.insertNode(wrapper);
+			// A text-node range shouldn't fail surroundContents under normal DOM,
+			// but if it does, skip rather than throw.
 		}
-	} catch {
-		// Range manipulation can fail on edge cases (e.g., needle spans across
-		// block elements). Skip silently — the right-rail card still shows the
-		// annotation so the reviewer's intent isn't lost.
 	}
 }
+
+function createMarkWrapper(mark: InlineMark, isActive: boolean): HTMLElement {
+	const wrapper = document.createElement('mark');
+	wrapper.className = CLASS_BY_KIND[mark.kind] + (isActive ? ' is-active' : '');
+	wrapper.dataset.annoId = mark.id;
+	return wrapper;
+}
+
+/**
+ * Collapses runs of whitespace in `s` to single spaces (trimming ends),
+ * returning the normalized string and a map from each normalized index to
+ * its corresponding index in the original string. Used to bridge the gap
+ * between needle (with \n separators from selection.toString()) and the
+ * flat text walked via TreeWalker (no separators).
+ */
+function normalizeWithMap(s: string): { normalized: string; rawPositions: number[] } {
+	let normalized = '';
+	const rawPositions: number[] = [];
+	let prevWS = true; // skip leading whitespace
+	for (let i = 0; i < s.length; i++) {
+		const ch = s[i];
+		if (isWS(ch)) {
+			if (!prevWS) {
+				normalized += ' ';
+				rawPositions.push(i);
+				prevWS = true;
+			}
+		} else {
+			normalized += ch;
+			rawPositions.push(i);
+			prevWS = false;
+		}
+	}
+	if (normalized.endsWith(' ')) {
+		normalized = normalized.slice(0, -1);
+		rawPositions.pop();
+	}
+	return { normalized, rawPositions };
+}
+
+function normalizeWS(s: string): string {
+	return s.replace(/\s+/g, ' ').trim();
+}
+
+function isWS(ch: string): boolean {
+	return ch === ' ' || ch === '\t' || ch === '\n' || ch === '\r' || ch === '\f' || ch === '\v';
+}

From 242bcaeb635ec3270bf5b19a8587f0343d0abd45 Mon Sep 17 00:00:00 2001
From: Ben Firestone 
Date: Thu, 30 Apr 2026 14:53:16 -0700
Subject: [PATCH 22/26] fix(share): inline marks for 
         and block-boundary
 selections
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

selection.toString() inserts \n at 
         elements and at block boundaries
(between 
      • s, between 
        s, after a heading), but a SHOW_TEXT
TreeWalker over the same DOM yields zero whitespace between adjacent
text nodes. The whitespace-normalization fallback could only collapse
existing runs, so multi-block selections with no inter-block whitespace
(e.g. heading + bulleted list) and selections spanning a markdown hard
line break (`
        `) silently failed to highlight.

Walk SHOW_TEXT | SHOW_ELEMENT and build a parallel `searchSpace` string
that mirrors selection.toString()'s output by inserting synthetic '\n'
at every block-level boundary (closest-block ancestor differs) and at
every 
        /
         element. Matches map back to acc positions through
searchToAcc[], skipping the synthetic chars before the wrap step.

Adds inline-annotations.test.ts with regression coverage for the
heading + bulleted list, two-paragraph, hard-break, and code-block
cases — all 9 pass under jsdom.
---
 .../components/inline-annotations.test.ts     | 222 ++++++++++++++++++
 .../[id]/components/inline-annotations.ts     | 171 ++++++++++++--
 2 files changed, 373 insertions(+), 20 deletions(-)
 create mode 100644 web/src/routes/share/[id]/components/inline-annotations.test.ts

diff --git a/web/src/routes/share/[id]/components/inline-annotations.test.ts b/web/src/routes/share/[id]/components/inline-annotations.test.ts
new file mode 100644
index 0000000..bd4700c
--- /dev/null
+++ b/web/src/routes/share/[id]/components/inline-annotations.test.ts
@@ -0,0 +1,222 @@
+// @vitest-environment jsdom
+import { describe, it, expect, beforeEach } from 'vitest';
+import { applyInlineAnnotations, type InlineMark } from './inline-annotations';
+
+function el(tag: string, opts: { dataSourceLine?: number; text?: string } = {}): HTMLElement {
+	const e = document.createElement(tag);
+	if (opts.dataSourceLine !== undefined) {
+		e.setAttribute('data-source-line', String(opts.dataSourceLine));
+	}
+	if (opts.text !== undefined) e.appendChild(document.createTextNode(opts.text));
+	return e;
+}
+
+function makeContainer(): HTMLElement {
+	return document.createElement('article');
+}
+
+function mark(opts: Partial & Pick): InlineMark {
+	return {
+		id: opts.id ?? 'm1',
+		kind: opts.kind ?? 'comment',
+		lineStart: opts.lineStart,
+		lineEnd: opts.lineEnd,
+		quotedText: opts.quotedText
+	};
+}
+
+describe('applyInlineAnnotations', () => {
+	let container: HTMLElement;
+	beforeEach(() => {
+		container = makeContainer();
+	});
+
+	it('wraps a single contiguous selection inside one paragraph', () => {
+		const p = el('p', { dataSourceLine: 1, text: 'The quick brown fox' });
+		container.appendChild(p);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'quick brown', lineStart: 1, lineEnd: 1 })
+		]);
+
+		const marks = container.querySelectorAll('mark.anno-comment');
+		expect(marks.length).toBe(1);
+		expect(marks[0].textContent).toBe('quick brown');
+		expect(marks[0].parentElement?.tagName).toBe('P');
+	});
+
+	it('wraps text inside an inline element (e.g. ) without splitting structure', () => {
+		const p = el('p', { dataSourceLine: 1 });
+		p.appendChild(document.createTextNode('use '));
+		const code = el('code', { text: 'foo()' });
+		p.appendChild(code);
+		p.appendChild(document.createTextNode(' here'));
+		container.appendChild(p);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'use foo() here', lineStart: 1, lineEnd: 1 })
+		]);
+
+		// The whole phrase should be wrapped — possibly as a single Range surround
+		// (one ) or as per-text-node wraps (three s). Either is OK
+		// as long as the visible covered text is exactly the needle.
+		const marks = Array.from(container.querySelectorAll('mark.anno-comment'));
+		expect(marks.length).toBeGreaterThanOrEqual(1);
+		const covered = marks.map((m) => m.textContent ?? '').join('');
+		expect(covered).toBe('use foo() here');
+		// Crucially,  is still present and still contains its text.
+		expect(container.querySelector('code')?.textContent).toBe('foo()');
+	});
+
+	it('wraps a multi-paragraph selection where the needle has \\n separators', () => {
+		const p1 = el('p', { dataSourceLine: 1, text: 'First paragraph.' });
+		const p2 = el('p', { dataSourceLine: 3, text: 'Second paragraph.' });
+		container.appendChild(p1);
+		container.appendChild(p2);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'First paragraph.\nSecond paragraph.', lineStart: 1, lineEnd: 3 })
+		]);
+
+		const marks = Array.from(container.querySelectorAll('mark.anno-comment'));
+		expect(marks.length).toBeGreaterThanOrEqual(2);
+		expect(marks.some((m) => m.textContent === 'First paragraph.')).toBe(true);
+		expect(marks.some((m) => m.textContent === 'Second paragraph.')).toBe(true);
+		// Paragraph elements remain intact.
+		expect(container.querySelectorAll('p').length).toBe(2);
+	});
+
+	it('wraps a heading + bulleted list selection (the regression case)', () => {
+		// Reproduces the "Non-goals" failure from the handoff: 
         followed by
+		// a 
           with several 
        • s. The TreeWalker yields text nodes with no
+		// whitespace between them, but selection.toString() inserts \n between
+		// the heading and each 
        •  — so the search needs synthetic block
+		// separators to match.
+		const h = el('h2', { dataSourceLine: 1, text: 'Non-goals' });
+		const ul = el('ul', { dataSourceLine: 3 });
+		const li1 = el('li', { text: 'Real authentication, OAuth, or accounts' });
+		const li2 = el('li', { text: 'Server-side comment aggregation' });
+		const li3 = el('li', { text: 'Live multi-user co-editing' });
+		ul.appendChild(li1);
+		ul.appendChild(li2);
+		ul.appendChild(li3);
+		container.appendChild(h);
+		container.appendChild(ul);
+
+		const needle = [
+			'Non-goals',
+			'Real authentication, OAuth, or accounts',
+			'Server-side comment aggregation',
+			'Live multi-user co-editing'
+		].join('\n');
+
+		applyInlineAnnotations(container, [mark({ quotedText: needle, lineStart: 1, lineEnd: 3 })]);
+
+		const marks = Array.from(container.querySelectorAll('mark.anno-comment'));
+		// One mark per text node (heading + 3 list items).
+		expect(marks.length).toBe(4);
+		expect(marks.map((m) => m.textContent ?? '')).toEqual([
+			'Non-goals',
+			'Real authentication, OAuth, or accounts',
+			'Server-side comment aggregation',
+			'Live multi-user co-editing'
+		]);
+		// List structure is preserved — three 
        •  children of one 
            .
+		expect(container.querySelectorAll('ul > li').length).toBe(3);
+	});
+
+	it('wraps a code-block selection containing literal newlines', () => {
+		// 
            ...
             — text contains real \n chars, not block
+		// boundaries. The needle from selection.toString() also has real \n.
+		const pre = el('pre', { dataSourceLine: 1 });
+		const code = el('code');
+		code.appendChild(document.createTextNode('line one\nline two\nline three'));
+		pre.appendChild(code);
+		container.appendChild(pre);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'line one\nline two\nline three', lineStart: 1, lineEnd: 1 })
+		]);
+
+		const marks = Array.from(container.querySelectorAll('mark.anno-comment'));
+		expect(marks.length).toBeGreaterThanOrEqual(1);
+		const covered = marks.map((m) => m.textContent ?? '').join('');
+		expect(covered).toBe('line one\nline two\nline three');
+		// 
             structure preserved.
+		expect(container.querySelector('pre > code')).toBeTruthy();
+	});
+
+	it('treats 
             as a line break (the markdown hard-break case)', () => {
+		// Markdown's two-trailing-spaces hard break renders as 
             inside a
+		// single 
          •  or 
            . Selection.toString() emits '\n' at the 
            ; the
+		// TreeWalker SHOW_TEXT path doesn't see it. Without explicit handling
+		// the search whiffs because needle has '\n' but searchSpace doesn't.
+		const ul = el('ul', { dataSourceLine: 1 });
+		const li = el('li');
+		li.appendChild(document.createTextNode('they remain as'));
+		li.appendChild(document.createElement('br'));
+		li.appendChild(document.createTextNode('legacy storage'));
+		ul.appendChild(li);
+		container.appendChild(ul);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'they remain as\nlegacy storage', lineStart: 1, lineEnd: 1 })
+		]);
+
+		const marks = Array.from(container.querySelectorAll('mark.anno-comment'));
+		// Either a single Range wrap (1 mark covering text+
            +text) or per-
+		// text-node fallback (2 marks). Both are correct outcomes; what matters
+		// is that *something* wrapped, the structural 
             survived, and both
+		// halves of the text are inside a mark.
+		expect(marks.length).toBeGreaterThanOrEqual(1);
+		const covered = marks.map((m) => m.textContent ?? '').join('|');
+		expect(covered).toContain('they remain as');
+		expect(covered).toContain('legacy storage');
+		expect(container.querySelectorAll('br').length).toBe(1);
+	});
+
+	it('handles a partial selection inside a single 
          • ', () => {
+		const ul = el('ul', { dataSourceLine: 1 });
+		ul.appendChild(el('li', { text: 'one apple' }));
+		ul.appendChild(el('li', { text: 'two oranges' }));
+		container.appendChild(ul);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'two oranges', lineStart: 1, lineEnd: 1 })
+		]);
+
+		const marks = Array.from(container.querySelectorAll('mark.anno-comment'));
+		expect(marks.length).toBe(1);
+		expect(marks[0].textContent).toBe('two oranges');
+		expect(container.querySelectorAll('ul > li').length).toBe(2);
+	});
+
+	it('clears prior marks on re-application', () => {
+		const p = el('p', { dataSourceLine: 1, text: 'Hello world' });
+		container.appendChild(p);
+
+		const m1: InlineMark = mark({ quotedText: 'Hello', lineStart: 1, lineEnd: 1, id: 'a' });
+		const m2: InlineMark = mark({ quotedText: 'world', lineStart: 1, lineEnd: 1, id: 'b' });
+
+		applyInlineAnnotations(container, [m1]);
+		expect(container.querySelectorAll('mark[data-anno-id="a"]').length).toBe(1);
+
+		applyInlineAnnotations(container, [m2]);
+		// First mark torn down, second applied.
+		expect(container.querySelectorAll('mark[data-anno-id="a"]').length).toBe(0);
+		expect(container.querySelectorAll('mark[data-anno-id="b"]').length).toBe(1);
+	});
+
+	it('returns silently when the anchor is missing', () => {
+		const p = el('p', { dataSourceLine: 1, text: 'Hello' });
+		container.appendChild(p);
+
+		applyInlineAnnotations(container, [
+			mark({ quotedText: 'goodbye', lineStart: 1, lineEnd: 1 })
+		]);
+
+		expect(container.querySelectorAll('mark.anno-comment').length).toBe(0);
+		// Original text untouched.
+		expect(p.textContent).toBe('Hello');
+	});
+});
diff --git a/web/src/routes/share/[id]/components/inline-annotations.ts b/web/src/routes/share/[id]/components/inline-annotations.ts
index 9b60e97..8ca0302 100644
--- a/web/src/routes/share/[id]/components/inline-annotations.ts
+++ b/web/src/routes/share/[id]/components/inline-annotations.ts
@@ -1,15 +1,31 @@
 /**
  * Apply inline annotation marks to already-rendered markdown.
  *
+ * The hard problem: selection.toString() inserts \n separators at block
+ * boundaries (between 
          • s, between 
            s, after a heading) and at 
            
+ * elements, but a TreeWalker over the same DOM yields just the text-node
+ * data with no separators. Naively searching the concatenated text-node
+ * string for the needle, or even a whitespace-normalized version of it,
+ * fails when the gap between blocks is zero whitespace — normalization can
+ * only collapse existing runs.
+ *
  * Strategy: for each annotation, gather the [data-source-line] blocks in
- * [lineStart, lineEnd], walk all their text nodes in document order, and find
- * the anchor's quoted_text. The needle from selection.toString() may contain
- * \n separators between block-level elements (paragraphs, 
          • s, etc.) while
- * the flat text walked via TreeWalker has none — so the search is two-tier:
+ * [lineStart, lineEnd], walk all their text nodes plus 
            /
             elements
+ * in document order, and build TWO parallel strings:
+ *
+ *   - `acc`: raw concatenation of text-node data. Cumulative offsets into
+ *     this string map directly into textNodes via length math — used by the
+ *     wrap step.
+ *   - `searchSpace`: same content but with a synthetic '\n' inserted at
+ *     every block-level boundary AND every 
            /
             element. This mirrors
+ *     what selection.toString() emits, so the needle can match.
+ *     `searchToAcc[i]` maps every searchSpace index to its acc index, with
+ *     -1 marking a synthetic boundary char.
  *
- *   1. Exact substring match (most annotations land here).
- *   2. Whitespace-normalized match (collapses runs of whitespace, including
- *      block-boundary newlines) with a position map back to raw offsets.
+ * Search is two-tier on `searchSpace`: exact indexOf, then whitespace-
+ * normalized fallback for cases like extra trailing whitespace. The hit's
+ * range gets mapped back through `searchToAcc` (skipping synthetic chars)
+ * before reaching the wrap step.
  *
  * Wrapping is also two-tier:
  *
@@ -77,38 +93,103 @@ function wrapNeedleAcrossBlocks(
 ): void {
 	if (!needle) return;
 
-	// Walk text nodes across all blocks in DOM order.
+	// Walk text nodes AND inline structural elements (
            /
            ) across all
+	// blocks in document order. We build:
+	//
+	//   - `acc`: raw text-node concatenation (textNode position math basis).
+	//   - `searchSpace`: acc with synthetic '\n' at:
+	//       * block-level boundaries (different closest-block ancestor), and
+	//       * 
            /
             elements (invisible to SHOW_TEXT but they produce a
+	//         '\n' in Selection.toString()).
+	//     Mirrors what selection.toString() emits so the needle can match.
 	const textNodes: Text[] = [];
 	let acc = '';
+	let searchSpace = '';
+	const searchToAcc: number[] = [];
+	let prevBlock: Element | null = null;
+	const filter: NodeFilter = {
+		acceptNode(node) {
+			if (node.nodeType === Node.TEXT_NODE) return NodeFilter.FILTER_ACCEPT;
+			if (
+				node.nodeType === Node.ELEMENT_NODE &&
+				LINE_BREAK_TAGS.has((node as Element).tagName)
+			) {
+				return NodeFilter.FILTER_ACCEPT;
+			}
+			return NodeFilter.FILTER_SKIP;
+		}
+	};
 	for (const block of blocks) {
-		const walker = document.createTreeWalker(block, NodeFilter.SHOW_TEXT);
+		const walker = document.createTreeWalker(
+			block,
+			NodeFilter.SHOW_TEXT | NodeFilter.SHOW_ELEMENT,
+			filter
+		);
 		while (walker.nextNode()) {
-			const t = walker.currentNode as Text;
+			const node = walker.currentNode;
+			if (node.nodeType === Node.ELEMENT_NODE) {
+				if (acc.length > 0) {
+					searchSpace += '\n';
+					searchToAcc.push(-1);
+				}
+				continue;
+			}
+			const t = node as Text;
+			const curBlock = closestBlockAncestor(t);
+			if (prevBlock && curBlock !== prevBlock) {
+				searchSpace += '\n';
+				searchToAcc.push(-1);
+			}
+			prevBlock = curBlock;
 			textNodes.push(t);
+			const baseAcc = acc.length;
+			for (let i = 0; i < t.data.length; i++) {
+				searchSpace += t.data[i];
+				searchToAcc.push(baseAcc + i);
+			}
 			acc += t.data;
 		}
 	}
 	if (textNodes.length === 0) return;
 
-	// Two-tier search.
-	let start = -1;
-	let end = -1;
-	const exactOffset = acc.indexOf(needle);
+	// Two-tier search against searchSpace (which mirrors selection.toString()).
+	let searchStart = -1;
+	let searchEnd = -1;
+	const exactOffset = searchSpace.indexOf(needle);
 	if (exactOffset >= 0) {
-		start = exactOffset;
-		end = exactOffset + needle.length;
+		searchStart = exactOffset;
+		searchEnd = exactOffset + needle.length;
 	} else {
-		const norm = normalizeWithMap(acc);
+		const norm = normalizeWithMap(searchSpace);
 		const needleNorm = normalizeWS(needle);
 		if (!needleNorm) return;
 		const normOffset = norm.normalized.indexOf(needleNorm);
-		if (normOffset < 0) return; // anchor lost
-		start = norm.rawPositions[normOffset];
+		if (normOffset < 0) return;
+		searchStart = norm.rawPositions[normOffset];
 		const lastNormIdx = normOffset + needleNorm.length - 1;
 		if (lastNormIdx >= norm.rawPositions.length) return;
-		end = norm.rawPositions[lastNormIdx] + 1;
+		searchEnd = norm.rawPositions[lastNormIdx] + 1;
 	}
 
+	// Map searchSpace [start, end) back to acc, skipping synthetic boundary
+	// chars. The first real char at-or-after searchStart anchors `start`;
+	// the last real char before searchEnd anchors `end`.
+	let start = -1;
+	for (let i = searchStart; i < searchToAcc.length; i++) {
+		if (searchToAcc[i] >= 0) {
+			start = searchToAcc[i];
+			break;
+		}
+	}
+	let end = -1;
+	for (let i = searchEnd - 1; i >= 0; i--) {
+		if (searchToAcc[i] >= 0) {
+			end = searchToAcc[i] + 1;
+			break;
+		}
+	}
+	if (start < 0 || end <= start) return;
+
 	// Wrap. Try the contiguous Range first; on failure (range crosses sibling
 	// block elements like 
          • s), fall back to per-text-node wrap.
 	if (!tryRangeWrap(textNodes, start, end, mark, isActive)) {
@@ -116,6 +197,56 @@ function wrapNeedleAcrossBlocks(
 	}
 }
 
+// Tags that produce a literal '\n' in Selection.toString() despite having no
+// text-node children. We have to walk SHOW_ELEMENT to see them and translate
+// each into a synthetic separator.
+const LINE_BREAK_TAGS = new Set(['BR', 'HR']);
+
+// Tags treated as inline for the purpose of "did selection.toString() insert
+// a \n between these two text nodes?". Anything not in this set is treated
+// as a block boundary (paragraphs, list items, headings, code blocks, table
+// cells, etc.). Matches the HTML spec's "phrasing content" tags that the
+// markdown renderer can plausibly emit.
+const INLINE_TAGS = new Set([
+	'A',
+	'ABBR',
+	'B',
+	'BDI',
+	'BDO',
+	'BR',
+	'CITE',
+	'CODE',
+	'DATA',
+	'DEL',
+	'DFN',
+	'EM',
+	'I',
+	'INS',
+	'KBD',
+	'MARK',
+	'Q',
+	'S',
+	'SAMP',
+	'SMALL',
+	'SPAN',
+	'STRONG',
+	'SUB',
+	'SUP',
+	'TIME',
+	'U',
+	'VAR',
+	'WBR'
+]);
+
+function closestBlockAncestor(node: Node): Element | null {
+	let cur: Node | null = node.parentNode;
+	while (cur && cur.nodeType === 1) {
+		if (!INLINE_TAGS.has((cur as Element).tagName)) return cur as Element;
+		cur = cur.parentNode;
+	}
+	return null;
+}
+
 /**
  * Attempts to wrap [start, end) of the concatenated text-node string in a
  * single Range surroundContents. Works for ranges that don't cross sibling

From 0c3a596d311a745caf728126933f030fe046791d Mon Sep 17 00:00:00 2001
From: Ben Firestone 
Date: Thu, 30 Apr 2026 14:53:19 -0700
Subject: [PATCH 23/26] chore(web): allow Vite proxy backend override via
 ARC_PASTE_BACKEND

Dev workflow against a non-default arc-paste port (e.g. :7436) needs
the Vite proxy to talk to that port. Falls back to :7432 when the env
var is unset so the existing default still works.
---
 web/vite.config.ts | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/web/vite.config.ts b/web/vite.config.ts
index 0abd395..3ddaa7e 100644
--- a/web/vite.config.ts
+++ b/web/vite.config.ts
@@ -6,8 +6,10 @@ export default defineConfig({
 	plugins: [tailwindcss(), sveltekit()],
 	server: {
 		proxy: {
+			// Backend target overridable via ARC_PASTE_BACKEND so the dev server
+			// can drive a non-default port (e.g. arc-paste running on :7436).
 			'/api': {
-				target: 'http://localhost:7432',
+				target: process.env.ARC_PASTE_BACKEND ?? 'http://localhost:7432',
 				changeOrigin: true
 			}
 		}

From 29092feac0b66e5c96733d7a7ee50349354674ec Mon Sep 17 00:00:00 2001
From: Ben Firestone 
Date: Thu, 30 Apr 2026 15:12:19 -0700
Subject: [PATCH 24/26] feat(share): tighten name-capture toolbar UX
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Remove the always-on caption block at the bottom of the FloatingToolbar
when no reviewer name is set. Three feedback channels now form an
escalation gradient: dimmed icons (passive), hover tooltip "Enter your
name first" (intentional), shake + red outline + auto-refocus on click
(committed). The caption was redundant with the placeholder in the
default state and unnecessary in the error state since the input itself
already shouts.

- Auto-focus the name field on toolbar mount so the user can type
  immediately without an extra click.
- Quieter italic placeholder ("your name…") replacing the slightly
  verbose "your name to begin".
- Locked action buttons swap their hover title to "Enter your name
  first" via a small actionTitle() helper.
- Drop the now-dead .name-caption CSS rules.
---
 web/src/app.css                               | 21 ------------
 .../[id]/components/FloatingToolbar.svelte    | 34 +++++++++++--------
 2 files changed, 19 insertions(+), 36 deletions(-)

diff --git a/web/src/app.css b/web/src/app.css
index 6925306..1c5e33c 100644
--- a/web/src/app.css
+++ b/web/src/app.css
@@ -1093,27 +1093,6 @@
 	cursor: not-allowed;
 }
 
-/* Mono micro-caption below the icon row. Default state is instructional;
- * error state swaps the same line of text to a red directive. The text
- * change is intentional rather than a separate slide-in toast — fewer
- * moving parts, same information. */
-.share-page .floating-toolbar .name-caption {
-	margin-top: 0.4rem;
-	padding-top: 0.4rem;
-	border-top: 1px dashed var(--ink-rule);
-	font-family: "JetBrains Mono", ui-monospace, monospace;
-	font-size: 0.625rem;
-	font-weight: 500;
-	letter-spacing: 0.16em;
-	text-transform: uppercase;
-	color: var(--ink-text-faint);
-	text-align: center;
-	transition: color 180ms ease;
-}
-.share-page .floating-toolbar .name-caption.is-error {
-	color: var(--ink-delete);
-}
-
 /* Annotation card type chips */
 .share-page .chip-comment {
 	color: var(--ink-comment);
diff --git a/web/src/routes/share/[id]/components/FloatingToolbar.svelte b/web/src/routes/share/[id]/components/FloatingToolbar.svelte
index 6a449fc..696d3ad 100644
--- a/web/src/routes/share/[id]/components/FloatingToolbar.svelte
+++ b/web/src/routes/share/[id]/components/FloatingToolbar.svelte
@@ -111,6 +111,9 @@
 		// width — typically ~200px depending on the icon set.
 		await tick();
 		if (toolbar) measuredWidth = toolbar.offsetWidth;
+		// Focus the name field on first render so the user can start typing
+		// without an extra click. Skipped once a name is already on file.
+		if (needsName) nameInput?.focus();
 	});
 
 	onDestroy(() => {
@@ -125,6 +128,13 @@
 		delete: 'text-[var(--ink-delete)] hover:bg-[var(--ink-delete-bg)]',
 		muted: 'text-[var(--ink-text-muted)] hover:bg-[var(--ink-paper)]'
 	};
+
+	// While the field is empty, action buttons are functionally locked. Their
+	// hover tooltip swaps from the action's normal label to a single directive
+	// — the gentle middle tier between "passively dimmed" and "shake on click".
+	function actionTitle(active: string): string {
+		return needsName ? 'Enter your name first' : active;
+	}
 
 
 
            
+			 dimmed action icons below carry the "locked" signal; their
+			 hover tooltip swaps to "Enter your name first" on hover, and
+			 the shake fires when a locked action is clicked. -->
 		
            
 			 tryAction('praise')}
 	>
 		 tryAction('comment')}
 	>
 		 tryAction('delete')}
 	>
 		 tryAction('suggest')}
 	>
 		 tryAction('quick-label')}
 	>
 		
 	
            
 
-	{#if needsName}
-		
-		
            
-			{nameError ? 'Name required' : 'Enter your name to begin'}
-		
            
-	{/if}
 
            

From 2b752f4b4c66de47384f4484798a807959f3cee3 Mon Sep 17 00:00:00 2001
From: Ben Firestone 
Date: Thu, 30 Apr 2026 15:29:28 -0700
Subject: [PATCH 25/26] fix(share): triple-click selection no longer dismisses
 toolbar
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Chromium's triple-click frequently lands Range endpoints at element
boundaries (e.g. startContainer = 
            , startOffset = 0) rather than
inside a text node. The old `range.startContainer.parentElement?.closest
('[data-source-line]')` walked from the PARENT of 
             upward — past
the actual block — and returned null when the parent was 
            
(which has no data-source-line). That tripped the empty-selection
dismiss branch in handleMouseUp, hiding the toolbar 50% of the time
on triple-click.

Extract a resolveBlock() helper that uses the node itself as the
closest() search root when it's already an element, and falls back to
parentElement for text nodes. Both endpoint shapes now resolve to the
correct containing block.
---
 .../share/[id]/components/PlanRenderer.svelte | 22 ++++++++++++++-----
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/web/src/routes/share/[id]/components/PlanRenderer.svelte b/web/src/routes/share/[id]/components/PlanRenderer.svelte
index 97e5ca1..5c7206a 100644
--- a/web/src/routes/share/[id]/components/PlanRenderer.svelte
+++ b/web/src/routes/share/[id]/components/PlanRenderer.svelte
@@ -132,6 +132,20 @@
 		});
 	});
 
+	// Resolve a Range endpoint (text node OR element) to its containing
+	// [data-source-line] block. Triple-click in Chromium frequently puts
+	// the range endpoints at element boundaries (e.g. startContainer = 
            ,
+	// startOffset = 0) rather than inside a text node; in that case
+	// `node.parentElement.closest(...)` walks PAST the containing block to
+	// 
            , which has no data-source-line, and returns null — which
+	// would trip the dismiss path below. Treating the node itself as the
+	// closest() search root when it's already an element fixes that.
+	function resolveBlock(node: Node): HTMLElement | null {
+		const el =
+			node.nodeType === Node.ELEMENT_NODE ? (node as Element) : node.parentElement;
+		return (el?.closest('[data-source-line]') ?? null) as HTMLElement | null;
+	}
+
 	function handleMouseUp(e: MouseEvent) {
 		const target = e.target as Element | null;
 		const existingMark = target?.closest('mark[data-anno-id]') as HTMLElement | null;
@@ -146,12 +160,8 @@
 			return;
 		}
 		const range = sel.getRangeAt(0);
-		const startEl = (range.startContainer.parentElement as Element | null)?.closest(
-			'[data-source-line]'
-		) as HTMLElement | null;
-		const endEl = (range.endContainer.parentElement as Element | null)?.closest(
-			'[data-source-line]'
-		) as HTMLElement | null;
+		const startEl = resolveBlock(range.startContainer);
+		const endEl = resolveBlock(range.endContainer);
 		if (!startEl || !endEl) {
 			onSelection?.(null);
 			return;

From 90b1ebd938fe58f66e96a362ab249a6c90ece2d2 Mon Sep 17 00:00:00 2001
From: Ben Firestone 
Date: Thu, 30 Apr 2026 15:42:56 -0700
Subject: [PATCH 26/26] fix: wrap replay events signature for lint

---
 cmd/arc/share.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/cmd/arc/share.go b/cmd/arc/share.go
index d4007fe..6ebb91f 100644
--- a/cmd/arc/share.go
+++ b/cmd/arc/share.go
@@ -653,7 +653,10 @@ func decodeAndSortEvents(events []paste.Event, key []byte) []decodedEvent {
 	return out
 }
 
-func replayEvents(events []decodedEvent, planAuthor string) (map[string]commentEvent, map[string]resolutionEvent, map[string]bool) {
+func replayEvents(
+	events []decodedEvent,
+	planAuthor string,
+) (map[string]commentEvent, map[string]resolutionEvent, map[string]bool) {
 	comments := map[string]commentEvent{}
 	resolutions := map[string]resolutionEvent{}
 	retracted := map[string]bool{}