amadan
Plan 7, revision 2: the node lands when the bytes do
Fable's review changed two decisions and corrected several facts. The largest correction: revision 1 proposed writing an escaper that already exists at core/src/fragment.rs:25, escaping the same five characters in the same order. D1 is therefore not escaper-plus-allowlist; it is the allowlist, the reject-to-text path, and the tests, and it is small enough to be the first commit of the links branch rather than a branch landing with no caller — which AGENTS.md's own record is against, having deleted devauth.go for having zero readers. Three real gaps in D1. The allowlist has to run in the SHARED path, not in node_html: a rejected href means the run is emitted with no link mark at all, so fragment_json and both exporters cannot see a rejected URL, rather than each re-implementing the check and making a second copy of a security table. The scheme rule has to be a positive grammar, because a denylist of control characters misses a value with no colon and misses the code points browsers do not strip. And src has to match its literal shape, because "app-relative" admits //evil.example/x.png. SVG in the accepted types would run script on the app's origin, so the type list and its response headers are a D1 rule, not a Task 6 detail. D4 reverses. Inserting the node with its final src before the upload — hashing in the browser so the URL was known up front — looked like it removed a placeholder, but crypto.subtle.digest is async and transformPasted is synchronous, and the Google Docs path has no bytes at paste time at all, so a placeholder was always required. What it actually bought was a dangling src in a CRDT every tab already holds: other tabs and no-JavaScript readers get a 404 broken image, and a permanently failed upload becomes cross-tab node surgery with no owner. So the decoration renders immediately and the node is inserted on 2xx at the decoration's mapped position — §6.1's own rule that placeholders are decorations, never nodes. Paul's brief is met unchanged, the failure class disappears, and the server computes the hash. document_blobs grows content_type (§11.5 forces the stored type to application/octet-stream, so the true one cannot be recovered at serve time), plain_size rather than the sealed Ref.Size, and a unique key so a re-paste does not mint a fresh ciphertext object. No sealing key exists anywhere in the app yet — nothing calls SealSym — so that is Paul's decision and it blocks Task 6. D6 flips. A googleusercontent fetch with a cross-origin Origin returns access-control-allow-origin: *, from the same infrastructure that serves the clipboard's docsz URLs, which are signed-and-expiring rather than cookie-gated. The test stands but the expected answer is now "it works", and the real risk is expiry mid-retry, not CORS. Also: "degrade loudly" had nothing to degrade into — there is no paste report anywhere — so that is now its own task. Also corrected: image is inline in schema-basic; internal/pdf's addStream Flate-compresses unconditionally and its /Resources carries only /Font, so JPEG needs an uncompressed variant and images need /XObject; PNG alpha needs an /SMask; GFM's delimiter row is mandatory and its first row is the header, so a Docs table with no table_header needs one promoted; a Markdown image link is dead once the file leaves the app; noopener does nothing without target, and noreferrer is the one that earns its place. The link fixture must be generated through y-prosemirror rather than hand-built, because a mark's wire value is an attrs object and no existing fixture has that shape.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AmyRH8GkGkt5ay8gDAUKNm
docs/superpowers/plans/2026-09-02-images-links-tables.md+322 −235
CI passed — run details
Intent
<task-notification> <task-id>a87b79c69c9aa6a7c</task-id> <tool-use-id>toolu_01Pv8BKx9KDN4i4wXwMJdc15</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-docs/b67dca22-59bf-4290-8b46-545183f24ea9/tasks/a87b79c69c9aa6a7c.output</output-file> <status>completed</status> <summary>Agent "Fable review of plan 7" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>Verdict first: the plan is sound in its big calls — `prosemirror-tables`, tables before links before images, one scheme allowlist in Rust, `blobs.Sealed` with a mapping row. Two decisions need changing: D4's insert-before-upload ordering (it creates the dangling-src failure class it then has to mitigate, and it does not survive the async paths anyway), and D6's expectation (the browser fetch almost certainly works — I checked). Several factual claims need correcting before anyone builds from them, the biggest being that the escaper D1 proposes already exists. ## 1. D1 — right in shape, wrong about what is missing, and incomplete in three places **The escaper already exists.** `core/src/fragment.rs:25-36` `escape()` escapes exactly `& < > " '`, ampersand first — the function D1 step 1 proposes to write. Task 4 is therefore not "escaper + allowlist + tests"; it is the allowlist, the reject-to-text path, and the tests. It is small. Landing it as its own *branch* with no caller contradicts the repo's own record (AGENTS.md deleted `devauth.go` for having zero readers). Make it the first commit of the links branch, reviewed as a commit, and wire it in the second. Same review value, no dead code on `main`. **Where the allowlist should live: the JSON path, not just the HTML path.** `fragment_json` (`fragment.rs:311-500`) feeds `internal/export/document.go`, whose `Mark` is `{Type string}` and whose `Attrs` is closed to `Level`/`Align`. Task 5 has to grow both — the plan does not say so. More importantly, if the allowlist runs only in `node_html`, then `markdown.go` emits `[text](javascript:…)` and the PDF `/Link` annotation carries `/URI (javascript:…)` unless each exporter re-implements the check — a second copy of a security table, the exact coupling the plan says it avoids. Apply the allowlist once in Rust, in the shared path: a rejected `href` means `text_json` emits the run with no `link` mark at all, so no consumer can ever see a rejected URL. That is one table with one set of adversarial tests, and it is what makes "text, not anchor" hold in all three renderers. **Use a positive grammar, not a denylist of bad characters.** "Rejects control characters or whitespace before the colon" is the right instinct but the wrong shape. Browsers strip ASCII tab/newline anywhere and leading/trailing C0 controls; they do not strip U+00A0 or U+2028, and a value with no colon at all (`foo`, `#anchor`) is a third case. One rule covers every case: the value must match `^[A-Za-z][A-Za-z0-9+.-]*:` and the scheme, case-folded, must be in `{http, https, mailto}`; anything else — no colon, `//evil`, `data:`, `blob:`, `java\tscript:`, `\u0000javascript:` — renders as text. Add `data:` and `blob:` to the adversarial list; `blob:` is the one this very feature will be handling on the client side and must never reach the CRDT. **`src` "app-relative" is not enough as stated.** `//evil.example/x.png` starts with `/`. The `src` grammar must be the full literal shape `^/d/[A-Za-z0-9]+/blobs/[0-9a-f]{64}$` and nothing else. Do not try to check that the sqid is *this* document's — `fragment_html(doc)` has no sqid — the GET route does that by construction (see 2). Two things the plan misses that are not about escaping: - **SVG.** If Task 6's accepted-type list ever includes `image/svg+xml`, a navigated-to `/d/{sqid}/blobs/{hash}` runs script on the app origin. Accepted types: JPEG, PNG, GIF, WebP, sniffed from the bytes with `http.DetectContentType`, never from the client's header; serve with `X-Content-Type-Options: nosniff` and `Content-Security-Policy: sandbox`. This belongs in D1, not as an implementation detail of Task 6. - **The `validate: "string"` trap.** `prosemirror-schema-basic`'s `image.src`/`link.href` carry `validate: "string"`. A raw update via `POST /updates` with a non-string `href` makes `y-prosemirror`'s `createNodeFromYElement` throw, and its documented behaviour is to *delete the yjs type* (`sync-plugin.js:723`). That is pre-existing, but links and images widen it from "attrs the editor never writes" to "attrs every paste writes". Worth one line in the plan's risks; it is not the plan's job to fix. ## 2. D4 — the mapping table earns its place; the ordering trick is the wrong half **Checked claims.** `blobs.Sealed.Put` runs `crypto.SealSym`, which draws a random IV (`rastrillo@v0.22.0/crypto/crypto.go:305`), so the stored hash is non-deterministic — confirmed. `Store` has no put-under-key, so a row mapping plaintext hash → ciphertext hash is unavoidable. `document_blobs` earns its place. Authorisation via the owner-scoped chi group (`internal/docs/app.go:442-474`) is right today: there is no sharing implemented (grep finds no grants), so "as reachable as the document" holds, provided the handler resolves the document through `Store.Get` *before* looking up the row. **What the row is missing.** No `content_type` — Sealed passes the *stored* type through as visible metadata and §11.5 says to set that to `application/octet-stream`, so the true type has to come from somewhere at serve time: a column, or re-sniff the plaintext on every GET. Say which. Say whether `size` is plaintext or sealed. Make `(document_id, plain_hash)` unique and short-circuit a repeat upload before `Put`, or every re-paste of the same image mints a fresh ciphertext object. Have the server compute the hash itself and return it; never trust the client's. And add two entries to the spec's leak list §2.1: the plaintext SHA-256 sits in a plaintext column and in every access-log line for the URL (confirm-a-guess only, but it is a new item), and the CRDT (`doc_updates.update_bin`, itself still plaintext per `models.go`) carries `alt` text. **No sealing key exists.** `grep SealSym` across `internal/` and `cmd/` is empty. Task 6 says "`blobs.Sealed` over `S3FromEnv()`" without saying under what key. This is the first sealing the app does; it needs a `$DOCS_BLOB_KEY` (or whatever `carlos secrets` delivers), the same fail-closed-outside-development shape as `InstanceKey`, and a stated relationship to the eventual document DEK. That is a decision, and it is bigger than the sentence it gets. **The ordering trick.** Insert the node with its final `src`, then upload. I checked the three windows the brief asked about: - *Other tabs* get the node within ~250ms and render `<img src=/d/…/blobs/h>`; the GET is 404 until the upload lands; the browser shows a broken image with the alt text. Nothing in the NodeView on those tabs knows the upload is pending, and nothing triggers a re-fetch when it lands. - *A no-JavaScript reader* sees the same 404 broken image. D5's "a blob the store does not have renders as the alt text everywhere, never as a broken image icon" is not implementable in `fragment.rs` — it is a pure function of the CRDT with no store access. It is implementable only in the Go exporters. - *Permanent failure* leaves a dangling `src` in a CRDT every tab already has. Retry-then-drop "replaces the node by its alt text" — but which tab does that, and what if another tab has since moved, copied or edited the node? The trick also does not deliver what it promises even locally. `crypto.subtle.digest` is async and `transformPasted` is synchronous, and the Google Docs path (D6) has no bytes at paste time at all — so a placeholder mechanism is required anyway to hold the insertion point while a hash or fetch is in flight. "No placeholder" is untrue; what the trick actually avoids is a placeholder *node* and a second edit to `src`. **What it should be instead.** Keep §6.1's own rule: placeholders are decorations, never nodes. Render immediately by decoration in the local tab (the `blob:` preview, exactly ProseMirror's upload example), insert the node into the CRDT only when the upload returns 2xx, at the decoration's mapped position (the decoration set maps through every intervening transaction, and if undo removed it the result is discarded). This satisfies Paul's brief — the image is on screen instantly and the upload is async — and it removes the entire failure class: other tabs and readers never see a `src` that cannot resolve, and "retry-then-drop" becomes "remove the decoration and report", with no cross-tab node surgery. Given the save queue's own 2s debounce, collaborators would not see the image sooner under insert-first anyway. Once insertion follows upload, the browser-side hash is unnecessary: the server computes it, returns the URL, and the secure-context requirement of `crypto.subtle` (real, though `localhost` qualifies) and the client/server hash-mismatch case both go away. Hash-first is not wrong; it is a solution to a problem the better ordering does not have. Two gaps regardless of ordering: copy-paste of an image *between* documents gives doc B an `<img src="/d/A/blobs/h">`, which B's readers may not be able to fetch. The paste handler should recognise a same-origin `/d/…/blobs/…` `src` from another document, fetch it (same origin, no CORS) and re-upload under B. And the NodeView should retry on `img.onerror` with backoff for a bounded period, which covers the residual race where a tab receives the node slightly before the object is servable. ## 3. D6 — not honest, in the optimistic direction I fetched `https://lh3.googleusercontent.com/a/default-user=s96-c` with `Origin: https://example.com` from this sandbox today and got `access-control-allow-origin: *`, `cross-origin-resource-policy: cross-origin`, `timing-allow-origin: *` from `server: fife`. The `lh7-rt.googleusercontent.com/docsz/…` URLs in Google Docs clipboard HTML are served by the same fife infrastructure and are signed-and-expiring, not cookie-gated — the plan's "credentialed" is very likely wrong. One observation on one URL, not proof for `docsz`, so the plan's "test with a real Doc before building Task 7" is exactly right; but the expected outcome should be flipped, the test should be the first thing done rather than a risk row, and the risk row's real risk is expiry: the URL dies after some window, so a paste that sits in a retry loop can go from fetchable to 403. "Degrade loudly" is the right fallback slice and server-side fetching is correctly kept out. But the plan leans on "the paste report names it" — there is no paste report. `grep report fixtures/paste.mjs` is empty; §11.2's import report is unbuilt. That is a missing task with UI, catalog keys and a copy review, and both D4's failure path and D6 depend on it. ## 4. D2 — right, with wire and bundle details checked **Wire.** `y-prosemirror@1.3.7`'s `createTypeFromElementNode` (`sync-plugin.js:874-880`) skips `null` attrs — confirmed, so `colwidth: null` never hits the wire. But `colspan`/`rowspan` default to `1`, not `null`, so *every cell* carries both as numeric attrs. `yrs::XmlElementRef::get_attribute` returns `Option<String>` via `to_string` (`yrs-0.23.5/src/types/xml.rs:1026`), and `heading.level` already proves a JS number round-trips through that to `parse::<u8>`. Clamp `colspan` and `rowspan` to `1..=1000`. `table_header` → `<th scope="col">` (literal). No new closed *value* set — the plan's "names not values" distinction is correct. **Fixtures.** `fixtures/generate-fragment.mjs` hand-builds every vector from y-prosemirror's *described* behaviour. For tables that is fine (element names + numeric attrs, same shape as `aligned.bin`). For the `link` mark it is not: the value on the wire is `mark.attrs` — an object `{href, title: null}` under the key `link` (`marksToAttributes`, `sync-plugin.js:1121-1129`; `link` is not "overlapping" because a mark excludes itself by default) — a shape no existing fixture has. Build at least `links.bin` through `y-prosemirror`'s exported `prosemirrorToYXmlFragment` so the Rust side is tested against the real encoding, not a transcription of it. Add `tables.bin` and `images.bin` alongside. The plan's Task 8 says the fixtures "will not change"; correct for existing ones, but new ones must be *added*, and that is the gate for the new node types. **`node_text`.** "A row as a line, a cell as a tab-separated field" collides with the existing block-flush logic: cells contain paragraphs, which are blocks that push their own entries into `blocks` (`fragment.rs:203-260`). Either add a mode, or accept one block per cell in the plain-text preview — which is fine for a list preview. Say which; it is fiddlier than a line. **Package facts.** `prosemirror-tables@1.8.5`; its dependencies (`model ^1.25.4`, `view ^1.41.4`, `state ^1.4.4`, `keymap ^1.2.3`, `transform ^1.10.5`) all dedupe onto the exact versions `fixtures/package.json` pins. Verify with `npm ls prosemirror-model` after install — two copies of `prosemirror-model` breaks ProseMirror silently. The package exports *commands* (`goToNextCell`, `addRowAfter`, …) and the `tableEditing()` plugin; it does not export a keymap, so "its keymap" means binding Tab/Shift-Tab yourself. It also needs `.selectedCell` CSS for cell selection (`style/tables.css` is the reference), which Task 3 should list. Bundle: ESM `dist/index.js` minifies to roughly 40KB against the current 320KB minified `editor.7b30ca1e00c791c0.js`, ~12%. Visible in the diff, acceptable; the plan's "measure" is right. **Google Docs specifics.** Docs tables have no `<th>`, so a paste yields zero `table_header` rows. `<tbody>` and `<colgroup>` are transparent to ProseMirror's DOMParser. Cell widths arrive as `style="width:…"`, not `data-colwidth`, so `colwidth` stays null on paste too — good. ## 5. Ordering and task breakdown Tables → links → images is right; it matches Paul's actual pain and the dependency order. Corrections: - **Task 2's PDF tables is the largest single item in the plan** and is the least specified. Bound the first slice explicitly: equal column widths from the text column, cells wrap, a row that does not fit starts a new page, no cell merging across pages, `colspan` rendered, `rowspan` degraded to repeated cells. Otherwise this task swallows the branch. - **Task 4** → first commit of the links branch (see 1). - **Task 5** must also grow `internal/export/document.go`'s `Mark` and `Attrs`, and `text_json` must emit mark attrs. - **Task 6** needs the sealing-key decision and the content-type/SVG rules (see 1, 2). - **Task 7** is three things: file paste/drop with decoration + upload; the Google Docs remote fetch; PDF images. Split it. - **Missing task: the paste report** (see 3). - **Missing task: cross-document image copy** (see 2), or an explicit "not in this slice" line. - `image` in `prosemirror-schema-basic` is **inline** (`group: "inline"`). D5's "scaling to the text column width" is block behaviour. Decide: keep inline and have the PDF break the line around it, or override the spec to a block node. Google Docs pastes images inside `<p>`, so inline is the path of least resistance; say so. ## 6. Factual claims - **Escaper**: already exists (`fragment.rs:25`). See 1. - **DCTDecode**: correct, with one catch — `internal/pdf/pdf.go:195` `addStream` Flate-compresses every stream unconditionally, so it needs an uncompressed variant for JPEG (or a `/Filter [/FlateDecode /DCTDecode]` array, which is legal but pointless). Width/height/colourspace come from `image/jpeg.DecodeConfig`; CMYK JPEGs need `/Decode [1 0 1 0 1 0 1 0]` or should take the decode-and-Flate path. `Bytes()` builds a single `/Resources` dict with only `/Font`; it needs `/XObject`. - **PNG via `image/png` + FlateDecode**: correct for opaque images. Alpha needs a separate `/SMask` XObject or the image renders on black; 16-bit and palette PNGs need decoding to 8-bit RGB. "No rasterizer" is true — decoding is not rasterizing. - **GFM tables**: the delimiter row is mandatory, and the first row *is* the header — a Google Docs table with no `table_header` still needs one (promote the first row, or emit an empty header). Pipes in cells escape as `\|`; `colspan`/`rowspan`, multi-paragraph cells and lists in cells are outside GFM entirely. §11.3's "lossless within its subset" is fine only if the export says merged cells are outside the subset. Likewise `` is a dead link the moment the `.md` leaves the app; say so rather than calling it lossless. - **`crypto.subtle.digest`**: correct API, returns an `ArrayBuffer` you hex-encode yourself; requires a secure context (HTTPS or `localhost`, so `run-dev.sh` is fine). Unneeded under the ordering recommended in 2. - **`rel="noopener noreferrer"` with no `target`**: `noopener` does nothing without `target=_blank`; `noreferrer` is the one earning its place — it stops the document's URL (with its sqid) reaching the link target. Keep it, fix the reasoning. - **"y-prosemirror skips null attributes"**: confirmed. **"the wire stays as simple as it is today"**: nearly — every cell gains two numeric attrs. ## Files read `docs/superpowers/plans/2026-09-02-images-links-tables.md`, `AGENTS.md`, the spec §§2.1, 6, 11–14, `core/src/fragment.rs`, `fixtures/align.mjs`, `fixtures/editor-entry.js`, `fixtures/paste.mjs`, `fixtures/save-queue.mjs`, `fixtures/generate-fragment.mjs`, `fixtures/package.json`, `fixtures/build-bundles.mjs`, `.amadan/ci.d/10-gate`, `scripts/wire-gate.sh`, `internal/docs/app.go`, `internal/docs/models.go`, `internal/docs/store.go`, `internal/docs/views/show.html`, `internal/export/{document,markdown,pdf}.go`, `internal/pdf/pdf.go`, `fixtures/node_modules/y-prosemirror/src/plugins/sync-plugin.js`, `fixtures/node_modules/prosemirror-schema-basic/src/schema-basic.ts`, `~/.cargo/registry/src/*/yrs-0.23.5/src/types/xml.rs`, and `/home/paulca/go/pkg/mod/github.com/carlosframework/rastrillo@v0.22.0/{blobs/blobs.go,blobs/s3.go,crypto/crypto.go}`. Nothing was changed.</result> <usage><subagent_tokens>127637</subagent_tokens><tool_uses>18</tool_uses><duration_ms>533915</duration_ms></usage> </task-notification>
Attributed to this commit: 65.5 g CO₂e (5351701 tokens). This is an attribution, not a measurement — usage counted from this commit's own marker forward to the next one on worktree-paste-fidelity, never backward from it. A turn that produced more than one commit books all of its tokens to the last one, and a concurrent session on this branch can add to the same figure. Some of it was priced using the shared default rate, because no model-specific factor row exists yet for at least one model involved. The Carbon view has the repo total, which is never derived from this attribution.
diff --git a/docs/superpowers/plans/2026-09-02-images-links-tables.md b/docs/superpowers/plans/2026-09-02-images-links-tables.md| index a8c69a6..b4a6b14 100644 |
| --- a/docs/superpowers/plans/2026-09-02-images-links-tables.md |
| +++ b/docs/superpowers/plans/2026-09-02-images-links-tables.md |
| @@ -8,8 +8,12 @@ and the formatting gone. |
| Spec §11.2 already commits to all three: *"The schema covers headings and |
| named styles, lists, tables, images, links, headers and footers, page |
| setup and comments."* So none of this is a new decision about scope. What |
| -it is, is one real decision about safety that two of the three depend on, |
| -and a storage seam the app does not yet have. |
| +it is, is one decision about a safety property that two of the three |
| +depend on, and a storage seam the app does not yet have. |
| + |
| +*Revision 2, after Fable's review. It changed two decisions — D4's |
| +ordering and D6's expectation — and corrected several facts, the largest |
| +being that revision 1 proposed writing an escaper that already exists.* |
| ## What is actually broken today |
| @@ -33,274 +37,357 @@ own "silence here is data loss", happening now. |
| ## D1 — the safety property, and what it costs to spend it |
| -This is the decision everything else waits on. |
| - |
| `fragment_html`'s output is returned as `template.HTML` and written |
| unescaped by `views/show.html`. It is safe today because of a property |
| the module states and maintains: **no byte from a document reaches |
| attribute position**. Marks are attribute-free and render as fixed |
| -literals; the one node attribute that exists, `paragraph.align`, is only |
| -ever a key into a closed table whose right-hand side is a literal |
| -(`fragment.rs`'s `ALIGNMENTS`); `heading.level` is parsed as an integer |
| -and clamped. The `MARKS` comment says plainly what adding a valued mark |
| -would mean: |
| - |
| -> `POST /d/{sqid}/updates` accepts arbitrary client-supplied Yjs bytes |
| -> that never went through ProseMirror's schema, so the value would be |
| -> genuinely arbitrary. Adding one is a real decision about attribute |
| -> escaping and (for `href`) a scheme allowlist, not a row in this table. |
| - |
| -**Tables do not spend this property.** `colspan` and `rowspan` are |
| -integers, clamped exactly as `heading.level` already is. A table can be |
| -added without touching the escaping question at all. |
| - |
| -**Links and images do spend it.** `href`, `src`, `alt` and `title` are |
| -strings chosen by whoever posted the update. So this plan buys the |
| -property back deliberately, once, before either lands: |
| - |
| -1. An `attr_escaped(&str) -> String` in `fragment.rs` escaping `&`, `<`, |
| - `>`, `"` and `'` — in that order, ampersand first. |
| -2. A **scheme allowlist** for `href` and `src`: `http`, `https`, `mailto` |
| - for links; app-relative paths only for images (D4 makes every image |
| - src app-relative, so `src` never carries a scheme). Parsing is |
| - deliberately strict and case-folded, and rejects anything with a |
| - control character or whitespace before the colon — `java\tscript:` is |
| - the classic bypass. |
| -3. **A URL that fails the allowlist renders as text, not as a dead |
| - anchor.** Degrading loudly (§11.2) beats an anchor that looks live and |
| - is not. |
| -4. Tests in `core/tests/fragment.rs` that are adversarial rather than |
| - illustrative: `"><script>`, `javascript:`, `JaVaScRiPt:`, |
| - `java\u{0000}script:`, a scheme-relative `//evil`, and a `title` |
| - containing a quote. |
| - |
| -Doing this once, with its own tests, is the whole reason links come |
| -before images: by the time images need `alt` escaped, the machinery and |
| -the tests exist. |
| +literals; `paragraph.align` is only ever a key into a closed table whose |
| +right-hand side is a literal; `heading.level` is parsed as an integer and |
| +clamped. |
| + |
| +**The escaper already exists** — `fragment.rs:25` escapes `&`, `<`, `>`, |
| +`"` and `'`, ampersand first. What is missing is everything about |
| +*which* values are allowed to reach an attribute at all. |
| + |
| +**Tables do not spend the property.** `colspan` and `rowspan` are |
| +integers, clamped exactly as `heading.level` already is. |
| + |
| +**Links and images do.** So this plan buys it back deliberately: |
| + |
| +**1. A positive scheme grammar, not a denylist.** A rejected-characters |
| +rule is the right instinct and the wrong shape: browsers strip ASCII tab |
| +and newline anywhere in a URL and strip leading C0 controls, but not |
| +U+00A0 or U+2028, and a value with no colon at all is a third case. One |
| +rule covers all of them — the value must match |
| +`^[A-Za-z][A-Za-z0-9+.-]*:`, and that scheme, case-folded, must be one of |
| +`http`, `https`, `mailto`. Everything else renders as text: no colon, |
| +`//evil`, `data:`, `blob:`, `java\tscript:`, `\u{0}javascript:`. |
| +`blob:` matters more than it looks — it is what this very feature |
| +manipulates in the browser, and it must never reach the CRDT. |
| + |
| +**2. `src` is matched against its literal shape, not merely "app-relative".** |
| +`//evil.example/x.png` begins with `/`. The rule is |
| +`^/d/[A-Za-z0-9]+/blobs/[0-9a-f]{64}$` and nothing else. It does not |
| +check that the sqid is *this* document — `fragment_html(doc)` has no sqid |
| +— because the GET route does that by construction (D4). |
| + |
| +**3. The allowlist runs once, in the shared path, not per renderer.** |
| +This is the correction that matters most. If the check lived only in |
| +`node_html`, then `markdown.go` would emit `[text](javascript:…)` and the |
| +PDF's `/Link` annotation would carry the same, unless each exporter |
| +re-implemented the check — a second copy of a security table, which is |
| +the exact coupling this plan claims to avoid. Instead a rejected `href` |
| +means the run is emitted **with no `link` mark at all**, in |
| +`fragment_json`/`text_json` as much as in `fragment_html`. No consumer |
| +can then see a rejected URL, because none is ever produced. |
| + |
| +**4. Accepted image types are sniffed, and SVG is not one of them.** If |
| +`/d/{sqid}/blobs/{hash}` can serve `image/svg+xml`, navigating to it runs |
| +script on the app's own origin. JPEG, PNG, GIF and WebP only, sniffed |
| +from the bytes with `http.DetectContentType` and never taken from the |
| +client's header, served with `X-Content-Type-Options: nosniff` and |
| +`Content-Security-Policy: sandbox`. This is a safety rule, not an |
| +implementation detail of the upload route. |
| + |
| +**5. Adversarial tests in `core/tests/fragment.rs`**, covering every item |
| +in 1 and 2 plus `"><script>` in `title` and `alt`. |
| ## D2 — tables are `prosemirror-tables`, not a hand-rolled node set |
| -`prosemirror-tables` is the same team's package, is what TipTap's table |
| -extension wraps, and brings the parts that are genuinely hard: cell |
| -selection, the column/row commands, and the fixups that keep a table |
| -rectangular after every edit. Writing those by hand is how you get a |
| -document model that can represent a broken grid. |
| +`prosemirror-tables` (1.8.5) is the same team's package, is what TipTap's |
| +table extension wraps, and brings the parts that are genuinely hard: cell |
| +selection, the column and row commands, and the fixups that keep a table |
| +rectangular after every edit. Hand-rolling those is how you get a model |
| +that can represent a broken grid. |
| It contributes `table`, `table_row`, `table_cell`, `table_header`, with |
| -`colspan`, `rowspan` and `colwidth` on cells. |
| +`colspan`, `rowspan` and `colwidth` on cells. Its dependency ranges all |
| +dedupe onto the versions `fixtures/package.json` already pins — check |
| +with `npm ls prosemirror-model` after installing, because two copies of |
| +`prosemirror-model` break ProseMirror silently. It exports commands and |
| +the `tableEditing()` plugin but **no keymap**, so Tab and Shift-Tab are |
| +ours to bind, and it needs `.selectedCell` CSS for cell selection to be |
| +visible at all. |
| **Column resizing is off in this slice.** `columnResizing` is what |
| -populates `colwidth`, and `colwidth` is an *array* of numbers or null — |
| -a different shape from every attribute the wire currently carries, and |
| -one `fragment.rs` would have to validate element by element. Leaving |
| -resizing off keeps `colwidth` null, y-prosemirror skips null attributes |
| -entirely, and the wire stays as simple as it is today. Resizing can come |
| -later without a migration, exactly as §11.2 requires of footnotes. |
| - |
| -The vocabulary then has to be added in the four places that cannot import |
| -each other — the same shape of coupling `fixtures/align.mjs` documents, |
| -though a weaker one: these are node **names**, which every consumer |
| -already handles by name, not a closed set of **values** that four tables |
| -must keep in agreement. |
| +populates `colwidth`, and `colwidth` is an *array* — a different shape |
| +from anything on the wire today. Off, it stays null, and y-prosemirror |
| +skips null attributes entirely. Google Docs' own cell widths arrive as |
| +`style="width:…"` rather than `data-colwidth`, so a paste leaves it null |
| +too. Resizing can come later with no migration. |
| + |
| +`colspan` and `rowspan` are **not** null: they default to `1`, so every |
| +cell carries both as numeric attributes. That is the one way the wire |
| +gets busier. Clamp both to `1..=1000` on the Rust side, the way |
| +`heading.level` is clamped. `table_header` renders `<th scope="col">`, a |
| +literal. No new closed set of *values* — these are node names, which |
| +every consumer already dispatches on by name. |
| + |
| +Google Docs tables contain no `<th>`, so a paste produces zero |
| +`table_header` rows. `<tbody>` and `<colgroup>` are transparent to |
| +ProseMirror's DOMParser. |
| ## D3 — a link is a mark with an href, and the read view renders it |
| -`link` is already in the browser schema and already round-trips through |
| -the CRDT. What is missing is the reader. Once D1 is done this is a row in |
| -`MARKS` — except that `MARKS` is a table of literal open/close strings |
| -and `link` needs to compute its open tag, so the table grows a variant |
| -for a mark that takes an attribute rather than getting a second table. |
| - |
| -Rendered as `<a href="…" rel="noopener noreferrer">`. No `target`: a |
| -document is a page, and opening a new tab is the reader's choice, not the |
| -author's. |
| - |
| -`title` is escaped and passed through. Everything else on the mark is |
| -dropped. |
| - |
| -## D4 — an image renders immediately and uploads in the background |
| - |
| -Paul's shape, and the ordering constraint it creates is the interesting |
| -part. |
| - |
| -**Where the bytes come from is not one case but two.** |
| - |
| -- *A file dropped or pasted from the filesystem* arrives as a `File` on |
| - the clipboard. We have the bytes. |
| -- *An image pasted out of Google Docs* does **not**. Google's clipboard |
| - HTML carries `<img src="https://lh7-…googleusercontent.com/…">` — a |
| - remote, expiring, credentialed URL. We have an address, not an image. |
| - D6 is about that. |
| - |
| -**The src that goes into the CRDT must be app-relative and stable.** Not |
| -a `blob:` URL, which is per-tab and dies with the tab; not a |
| -googleusercontent URL, which expires and leaks the reader's request to |
| -Google. So every image node's `src` is `/d/{sqid}/blobs/{hash}`, where |
| -`hash` is the **SHA-256 of the plaintext bytes**, computed in the browser |
| -with `crypto.subtle.digest` before the upload starts. |
| - |
| -That last part is what makes "render immediately" work without a rewrite |
| -pass. The browser knows the final URL up front, so it inserts the node |
| -with its real `src` straight away and shows the local `blob:` URL through |
| -a NodeView until the upload lands. No placeholder attribute, no second |
| -edit to rewrite, and other tabs converge on the same node. |
| - |
| -**The plaintext hash cannot be the storage key.** Spec §11.5 requires all |
| -blob storage to go through `blobs.Sealed`, and says: *"The address |
| -becomes the ciphertext's hash, so dedup is per key and `Ref.Size` is the |
| -sealed size."* Sealing is not deterministic, so the stored key is not |
| -predictable from the plaintext. A small owned table closes the gap: |
| - |
| - document_blobs(document_id, plain_hash, blob_hash, size, created_at) |
| - |
| -`plain_hash` is what the URL carries and what the CRDT holds; `blob_hash` |
| -is the `blobs.Ref.Hash` that `Store.Get` wants. The row is also what |
| -makes authorisation work: `GET /d/{sqid}/blobs/{hash}` is inside the |
| -existing owner-scoped group in `app.go`, so an image is exactly as |
| -reachable as the document that references it — by construction, not by a |
| -second check somebody has to remember. |
| - |
| -**Failure is not allowed to be silent.** The document holds a `src` the |
| -moment the node is inserted, so a permanently failed upload leaves a |
| -reference to bytes that will never exist. `fixtures/save-queue.mjs` was |
| -written for exactly this class of bug (its F1 comment: a rejected batch |
| -that retried forever and absorbed every later edit), and the image |
| -uploader reuses its retry-then-drop shape. On permanent failure the node |
| -is replaced by its alt text and the person is told which image did not |
| -make it. |
| +`link` is already in the browser schema and already round-trips. What is |
| +missing is the reader. Once D1 exists this is a row in `MARKS` — except |
| +`MARKS` is a table of literal open/close strings and `link` must compute |
| +its open tag, so the table grows a variant for a valued mark rather than |
| +gaining a second table beside it. |
| + |
| +Rendered as `<a href="…" rel="noreferrer">`. Not `noopener`, which does |
| +nothing without `target`; `noreferrer` is the one that earns its place, |
| +because it stops the document's own URL — sqid and all — reaching the |
| +link target. No `target`: a document is a page, and opening a new tab is |
| +the reader's choice, not the author's. |
| + |
| +`internal/export/document.go` has to grow with this: its `Mark` is |
| +`{Type string}` and its `Attrs` is closed to `Level` and `Align`, so both |
| +need widening and `text_json` needs to emit mark attributes. |
| + |
| +## D4 — the image is on screen at once; the node lands when the bytes do |
| + |
| +Paul's brief is that an image renders immediately and uploads in the |
| +background. Revision 1 satisfied that by inserting the node with its |
| +final `src` up front — hashing the bytes in the browser so the URL was |
| +known before the upload. **That was the wrong half of the trick**, and |
| +this revision reverses it. |
| + |
| +The reason is what the other windows see. A node inserted before its |
| +bytes exist reaches every other tab within ~250ms, where the GET 404s and |
| +the reader gets a broken image; a no-JavaScript reader gets the same; and |
| +a permanently failed upload leaves a dangling `src` in a CRDT every tab |
| +already holds, so "replace the node with its alt text" becomes cross-tab |
| +node surgery with no obvious owner. It did not even buy what it claimed: |
| +`crypto.subtle.digest` is async and `transformPasted` is synchronous, and |
| +the Google Docs path has no bytes at paste time at all, so a placeholder |
| +was always required. |
| + |
| +**So: render by decoration, insert on success.** §6.1's own rule is that |
| +placeholders are decorations, never nodes. The local tab shows the |
| +`blob:` preview as a decoration — ProseMirror's own upload example — and |
| +the node is inserted into the CRDT only when the upload returns 2xx, at |
| +the decoration's mapped position. The decoration set maps through every |
| +intervening transaction; if an undo removed it, the result is discarded. |
| +Paul's brief is met exactly — instant on screen, upload async — and the |
| +entire dangling-`src` failure class disappears: no other tab and no |
| +reader ever sees a `src` that cannot resolve, and retry-then-drop becomes |
| +"remove the decoration and say so". Given the save queue's debounce, |
| +collaborators would not have seen the image sooner the other way round. |
| + |
| +Once insertion follows the upload, **the server computes the hash and |
| +returns the URL**. The browser never hashes, which also disposes of |
| +`crypto.subtle`'s secure-context requirement and of the client/server |
| +mismatch case. |
| + |
| +**The mapping table is still needed.** `blobs.Sealed.Put` runs |
| +`crypto.SealSym`, which draws a random IV, so the stored address is the |
| +ciphertext's hash and is not derivable from the plaintext (§11.5 says as |
| +much). `Store` has no put-under-key. So: |
| + |
| + document_blobs(document_id, plain_hash, blob_hash, content_type, |
| + plain_size, created_at) |
| + unique (document_id, plain_hash) |
| + |
| +`plain_hash` is what the URL carries and the CRDT holds; `blob_hash` is |
| +the `blobs.Ref.Hash` that `Store.Get` wants. `content_type` is a column |
| +because §11.5 requires the *stored* type to be |
| +`application/octet-stream`, so the true type cannot be recovered from the |
| +store at serve time. `plain_size` is the plaintext size, not `Ref.Size`, |
| +which is the sealed one. The unique constraint short-circuits a repeated |
| +paste of the same image before `Put`, so re-pasting does not mint a fresh |
| +ciphertext object every time. |
| + |
| +Authorisation falls out of placement: `GET /d/{sqid}/blobs/{hash}` sits |
| +inside the existing owner-scoped group in `app.go`, and the handler |
| +resolves the document through `Store.Get` **before** looking up the row, |
| +so an image is exactly as reachable as the document referencing it — by |
| +construction, not by a second check somebody has to remember. |
| + |
| +**A sealing key does not exist yet, and that is a decision, not a line.** |
| +Nothing in `internal/` or `cmd/` calls `SealSym` today; this is the first |
| +sealing the app does. It needs a key with the same fail-closed-outside- |
| +development shape as `InstanceKey`, and a stated relationship to the |
| +eventual per-document DEK. **This needs Paul's call before Task 6.** |
| + |
| +Two things to add to the spec's §2.1 leak list while doing it: the |
| +plaintext SHA-256 sits in a plaintext column and in every access-log line |
| +for the URL (confirm-a-guess only, but new), and `alt` text rides in |
| +`doc_updates.update_bin`, which is itself still plaintext. |
| + |
| +**Copying an image between documents** gives document B an |
| +`<img src="/d/A/blobs/…">` that B's readers may not be able to fetch. The |
| +paste handler should recognise a same-origin `/d/…/blobs/…` src from |
| +another document, fetch it (same origin, no CORS) and re-upload under B. |
| ## D5 — what each renderer does with an image |
| +`image` in `prosemirror-schema-basic` is **inline**, and Google Docs |
| +pastes images inside `<p>`, so it stays inline. That is the path of least |
| +resistance and it means the PDF breaks the line around an image rather |
| +than laying it out as a block. |
| + |
| - **Read view** (`fragment.rs`): `<img src="…" alt="…">`, both escaped, |
| - `src` required to be app-relative. No dimensions in this slice, so the |
| - page reflows as images load; `loading="lazy"` and an aspect-ratio box |
| - are a later refinement, not a correctness question. |
| -- **Markdown** (`markdown.go`): ``, which is lossless within |
| - the subset §11.3 promises. |
| -- **PDF** (`pdf.go`): the real work. JPEG bytes embed directly as |
| - `DCTDecode` and PNG needs decode-and-reflate, which is `image/png` from |
| - the standard library plus a `FlateDecode` stream — no rasterizer, so |
| - §11.4's `internal/raster` is **not** pulled in by this. Scaling is |
| - to the text column width, preserving aspect ratio. |
| -- **A blob the store does not have** renders as the alt text everywhere, |
| - never as a broken image icon. |
| - |
| -## D6 — the Google Docs image, and the risk this plan cannot design away |
| - |
| -Getting bytes for a `googleusercontent.com` src is the part most likely |
| -to fail, and it should be stated rather than discovered. |
| - |
| -- **Fetching it from the browser** is subject to CORS. If Google does not |
| - send permissive headers the fetch fails, and the canvas fallback fails |
| - too — an image drawn from a cross-origin source without CORS taints the |
| - canvas and `toBlob` throws. |
| -- **Fetching it from the server** works and is an SSRF surface: a |
| - document could name `http://169.254.169.254/…` and have the app fetch |
| - it. Doing this safely means a host allowlist, no redirect following to |
| - a private range, a size cap and a timeout. |
| - |
| -**This slice tries the browser fetch and degrades loudly if it fails.** |
| -The image does not come in, and the paste report names it — which is |
| -§11.2's rule, and is honest. Server-side fetching is a separate decision |
| -with its own threat model, and it should not be smuggled in as an |
| -implementation detail of a paste. |
| - |
| -This is the risk to check first with a real Google Doc, because if the |
| -browser fetch does work the whole question goes away, and if it does not |
| -then Paul should decide between "images pasted from Docs do not come in" |
| -and "the app gets a fetcher". |
| + `src` matched against D1's literal shape. No dimensions in this slice, |
| + so the page reflows as images load. |
| +- **Markdown** (`markdown.go`): `` — and this is a |
| + **dead link the moment the `.md` leaves the app**. Say so in the export |
| + rather than calling it lossless. |
| +- **PDF** (`pdf.go` and `internal/pdf`): JPEG embeds as `DCTDecode` and |
| + PNG decodes with `image/png` and re-deflates — no rasterizer, so |
| + §11.4's `internal/raster` is **not** pulled in. Three details revision 1 |
| + missed: `internal/pdf`'s `addStream` Flate-compresses every stream |
| + unconditionally and needs an uncompressed variant for JPEG; the |
| + `/Resources` dict it builds carries only `/Font` and needs `/XObject`; |
| + and PNG alpha needs a separate `/SMask` XObject or the image renders on |
| + black, with 16-bit and palette PNGs decoded to 8-bit RGB first. CMYK |
| + JPEGs need `/Decode [1 0 1 0 1 0 1 0]` or should take the decode path. |
| +- **A blob the store does not have** renders as the alt text — in the Go |
| + exporters, which can ask the store. `fragment.rs` cannot: it is a pure |
| + function of the CRDT with no store access. The browser covers the |
| + residual race by retrying on `img.onerror` with bounded backoff. |
| + |
| +## D6 — the Google Docs image, and what to test first |
| + |
| +A Google Docs paste gives a `googleusercontent.com` URL, not bytes. |
| +Revision 1 assumed fetching it from the browser would be CORS-blocked and |
| +planned around that. **The evidence points the other way**: a |
| +`googleusercontent.com` fetch with a cross-origin `Origin` header returns |
| +`access-control-allow-origin: *` and `cross-origin-resource-policy: |
| +cross-origin`, from the same `fife` infrastructure that serves the |
| +`lh7-rt.googleusercontent.com/docsz/…` URLs in the clipboard HTML — which |
| +are signed-and-expiring rather than cookie-gated, so "credentialed" was |
| +probably wrong too. |
| + |
| +That is one observation on one URL, not proof for `docsz`. So the test |
| +stands, but it moves: **fetch a real Docs image URL cross-origin before |
| +building any of Task 7**, because the expected answer is now "it works", |
| +and that changes the deliverable rather than confirming a fallback. |
| + |
| +The real risk is **expiry**, not CORS: the URL dies after some window, so |
| +a paste sitting in a retry loop can go from fetchable to 403. Fetch |
| +early, once, and fail loudly. |
| + |
| +Server-side fetching stays out. It is an SSRF surface with its own threat |
| +model and should not arrive as an implementation detail of a paste. |
| + |
| +**"Degrade loudly" needs something to degrade into, and it does not |
| +exist.** There is no paste report anywhere in the codebase, and §11.2's |
| +import report is unbuilt. Both D4's failure path and D6's depend on it, |
| +so it is a task with UI, catalog keys and a copy review — not a sentence. |
| ## D7 — order, and why |
| **Tables, then links, then images.** Tables need no new safety property |
| -and no new storage, so they ship first and independently. Links buy the |
| -escaping machinery of D1 and pay for it with adversarial tests. Images |
| -need D1 done, plus a new table, a new route, a new upload path and PDF |
| -work — and they carry D6, the only genuinely open risk here. |
| +and no new storage. Links buy D1 and pay for it with adversarial tests. |
| +Images need D1, a new table, a new route, a sealing key, an upload path |
| +and PDF work — and carry D6. |
| -Each is a branch that can merge on its own. |
| +D1 is **not** its own branch. Revision 1 proposed landing it with no |
| +caller "so it can be reviewed on its own"; AGENTS.md's own record is |
| +against that — `devauth.go` was deleted for having zero readers. It is |
| +the first commit of the links branch instead, reviewed as a commit. |
| ## The shape of the work |
| ### Task 1 — tables in the schema and on the wire |
| - |
| -`fixtures/editor-entry.js` (add the four node types from |
| -`prosemirror-tables`, its plugin and its keymap; column resizing off), |
| -`fixtures/package.json`, `fixtures/tables.mjs` if the node specs need |
| -more than a line each, `fixtures/tables.test.mjs`, the gate. |
| +`fixtures/editor-entry.js` (the four node types, `tableEditing()`, Tab |
| +and Shift-Tab bound by hand), `fixtures/package.json`, |
| +`fixtures/tables.test.mjs`, the gate. Measure the bundle: roughly 40KB |
| +minified against the current 320KB, and visible in the diff because §6.5 |
| +forbids a build step. |
| ### Task 2 — tables in the three renderers |
| - |
| -`core/src/fragment.rs`: `node_html` arms for the four names, with |
| -`colspan`/`rowspan` parsed and clamped the way `heading.level` is; |
| -`node_text` treating a row as a line and a cell as a tab-separated field; |
| -`node_json` emitting the numeric attrs. `core/tests/fragment.rs`. |
| -`internal/export/markdown.go`: GFM tables, with the alignment row. |
| -`internal/export/pdf.go`: column widths from the text column, cell |
| -wrapping, and a rule for a row that will not fit on the page. |
| +`core/src/fragment.rs`: the four arms, `colspan`/`rowspan` parsed and |
| +clamped, `table_header` → `<th scope="col">`. `node_text` needs a ruling: |
| +cells contain paragraphs, which already push their own entries into |
| +`blocks`, so either add a mode or accept one block per cell in the |
| +plain-text preview — the latter is fine for a list preview and should be |
| +stated rather than discovered. `internal/export/markdown.go`: GFM tables, |
| +whose delimiter row is mandatory and whose first row **is** the header — |
| +so a Google Docs table with no `table_header` needs its first row |
| +promoted. Pipes escape as `\|`. `colspan`, `rowspan`, multi-paragraph |
| +cells and lists in cells are outside GFM entirely and the export must say |
| +so rather than claiming losslessness. `internal/export/pdf.go`: **bound |
| +this explicitly or it swallows the branch** — equal column widths from |
| +the text column, cells wrap, a row that will not fit starts a new page, |
| +no cell merging across pages, `colspan` rendered, `rowspan` degraded to |
| +repeated cells. |
| ### Task 3 — tables on screen |
| - |
| -`internal/docs/views/show.html`: table CSS in the `[docs-text]` |
| -vocabulary, borders that read as a document rather than a spreadsheet, |
| -and `overflow-x: auto` on a wrapper so a wide table scrolls inside the |
| -page rather than making the page scroll. `fixtures/toolbar.mjs`: insert |
| -table, add/remove row and column. WCAG 2.2 AA, and every string a |
| -catalog key. |
| - |
| -### Task 4 — attribute escaping and the scheme allowlist |
| - |
| -`core/src/fragment.rs` only, plus `core/tests/fragment.rs`. Lands with no |
| -caller, deliberately: the machinery and its adversarial tests are |
| -reviewable on their own, and the next two tasks are then small. |
| - |
| -### Task 5 — links reach the reader |
| - |
| -`fragment.rs`'s `MARKS` grows a valued variant; `markdown.go` emits |
| -`[text](href)`; `pdf.go` draws the link text with the document's link |
| -styling and, if the writer supports it, a `/Link` annotation. |
| -`fixtures/toolbar.mjs` gains add/remove link. Spec §6.3's toolbar list |
| -already names link. |
| - |
| -### Task 6 — the blob store and the route |
| - |
| -`internal/docs/models.go` and a migration for `document_blobs`; |
| -`blobs.Sealed` over `blobs.S3FromEnv()` in `cmd/docs/main.go`, falling |
| -back to `blobs.Dir` for local runs; `POST /d/{sqid}/blobs` and |
| -`GET /d/{sqid}/blobs/{hash}` inside the owner-scoped group in `app.go`, |
| -with a size cap and an accepted-type list. The §12 plaintext-grep gate |
| -already covers bucket objects and will cover these. |
| - |
| -### Task 7 — the image, end to end |
| - |
| -`fixtures/image-upload.mjs` (hash, upload, retry-then-drop, reusing |
| -save-queue's shape) and its test; a NodeView in `editor-entry.js` showing |
| -the local preview until the upload lands; `fragment.rs`, `markdown.go` |
| -and `pdf.go` per D5; the paste report for what did not come in. |
| - |
| -### Task 8 — the gate |
| - |
| -Every new JS module gets a `node --test` file in `.amadan/ci.d/10-gate`, |
| -the way `paste.test.mjs` was added on this branch. The wire fixtures are |
| -the contract and must not change (they will not: no existing node's shape |
| -is altered). |
| +`views/show.html`: table CSS in the `[docs-text]` vocabulary, borders |
| +that read as a document rather than a spreadsheet, `overflow-x: auto` on |
| +a wrapper so a wide table scrolls inside the page, and `.selectedCell`. |
| +`fixtures/toolbar.mjs`: insert table, add/remove row and column. WCAG 2.2 |
| +AA, every string a catalog key. |
| + |
| +### Task 4 — new wire fixtures |
| +`fixtures/generate-fragment.mjs` hand-builds its vectors from |
| +y-prosemirror's described behaviour. That is fine for tables (element |
| +names and numeric attrs, the same shape as `aligned.bin`) and **not fine |
| +for `link`**, whose wire value is a `mark.attrs` object under the mark |
| +name — a shape no existing fixture has. Generate `links.bin` through |
| +y-prosemirror's own `prosemirrorToYXmlFragment` so Rust is tested against |
| +the real encoding rather than a transcription of it. `tables.bin` and |
| +`images.bin` alongside. Existing fixtures do not change; these are |
| +additions, and they are the gate for the new types. |
| + |
| +### Task 5 — the allowlist, then links |
| +First commit: D1's grammar, the reject-to-text path in the shared |
| +`fragment_json`/`text_json` path, and the adversarial tests. Second: |
| +`MARKS`' valued variant, `document.go`'s `Mark` and `Attrs`, |
| +`markdown.go`'s `[text](href)`, `pdf.go`'s link styling and `/Link` |
| +annotation, `toolbar.mjs`'s add/remove link. |
| + |
| +### Task 6 — the sealing key, the blob store and the route |
| +**Blocked on Paul's key decision.** Then `models.go` plus a migration for |
| +`document_blobs`; `blobs.Sealed` over `blobs.S3FromEnv()` in |
| +`cmd/docs/main.go`, falling back to `blobs.Dir` for local runs; |
| +`POST /d/{sqid}/blobs` and `GET /d/{sqid}/blobs/{hash}` in the |
| +owner-scoped group, with the size cap, D1's sniffed type list and its |
| +response headers. §12's plaintext-grep gate already covers bucket |
| +objects. |
| + |
| +### Task 7 — the paste report |
| +The thing every "degrade loudly" path needs and none of them has. UI, |
| +catalog keys, copy review. |
| + |
| +### Task 8 — images from a file |
| +`fixtures/image-upload.mjs` (decoration, upload, retry-then-drop, reusing |
| +save-queue's shape) and its test; insertion on success at the mapped |
| +position; `fragment.rs`, `markdown.go` per D5. |
| + |
| +### Task 9 — images from Google Docs |
| +Only after D6's fetch test. The cross-origin fetch, its expiry handling, |
| +and the report entry when it fails. |
| + |
| +### Task 10 — images in the PDF |
| +`internal/pdf`'s uncompressed stream variant, `/XObject` in `/Resources`, |
| +`/SMask` for alpha, the decode paths. Its own task because it is its own |
| +body of work. |
| + |
| +### Task 11 — copying an image between documents |
| +Recognise a same-origin `/d/…/blobs/…` src from another document, fetch |
| +and re-upload under this one. Or an explicit "not in this slice" line — |
| +but not silence. |
| ## Risks |
| -| Risk | Where it bites | What to do | |
| +| Risk | Where | What to do | |
| |---|---|---| |
| -| Google's images are unfetchable from the browser (CORS) | D6, Task 7 | Test it with a real Doc **before** building Task 7. It changes the deliverable. | |
| -| Escaping is got subtly wrong | D1, Task 4 | Adversarial tests, and Fable reviews Task 4 on its own before anything calls it. | |
| -| `prosemirror-tables` bloats the committed bundle | Task 1 | Measure. The bundle is committed and spec §6.5 forbids a build step, so growth is visible in the diff. | |
| -| PDF table layout is open-ended | Task 2 | Cap the first slice: no cell merging across pages, a row that will not fit starts a new page, and say so. | |
| -| A permanently failed upload leaves a dangling src | D4, Task 7 | Retry-then-drop, then replace the node with its alt text and tell the person. | |
| -| Two branches inventing the same vocabulary | All | Discussion #20 (`worktree-import-documents`) is building DOCX/Markdown/PDF **import**, which produces these same nodes. Agree the node names and attrs there before either side writes them. | |
| +| The Docs fetch behaves differently from the probe | D6 | Test a real Doc image **first**. It sets the shape of Tasks 9 and 7. | |
| +| A Docs URL expires mid-retry | D6 | Fetch early, once, fail loudly. | |
| +| No sealing key decision | D4, Task 6 | Paul's call. Blocks Task 6. | |
| +| PDF tables run away | Task 2 | The explicit bound above. | |
| +| PDF images touch `internal/pdf`'s stream and resource assumptions | Task 10 | Its own task, not a corner of another. | |
| +| `validate: "string"` on `image.src`/`link.href` | All | A raw update with a non-string attr makes y-prosemirror **delete the type**. Pre-existing, but links and images widen it from "attrs the editor never writes" to "attrs every paste writes". Not this plan's to fix; recorded so it is not rediscovered as a mystery. | |
| +| Two `prosemirror-model` copies | Task 1 | `npm ls prosemirror-model` after install. Breaks silently. | |
| +| Two branches inventing the same vocabulary | All | Discussion #20 (`worktree-import-documents`) builds DOCX import, which produces these same nodes. Agree names and attrs there first. | |
| ## What this plan does not do |
| -Headers and footers, page setup, comments, footnotes and tracked changes |
| -— all §11.2 or §14, none of them touched here. Column resizing (D2). |
| -Image dimensions and lazy loading (D5). Server-side image fetching (D6). |
| -Underline, strikethrough, colour and highlight, which are a separate |
| -question about the attribute-free `MARKS` discipline and not about these |
| -three nodes. |
| +Headers and footers, page setup, comments, footnotes and tracked changes. |
| +Column resizing (D2). Image dimensions and lazy loading (D5). |
| +Server-side image fetching (D6). Underline, strikethrough, colour and |
| +highlight, which are a separate question about the attribute-free `MARKS` |
| +discipline rather than about these three nodes. |