amadan
Plan 7, revision 3: what 250db1c already did for it
Nothing is re-decided. The word-processor basics landed while this plan
sat on main, and they moved four things it describes.
MARKS now holds eleven point-size marks plus the four named sizes they
replaced. All of them are still attribute-free spans with literal
classes, so `link` remains the FIRST valued mark and D1 is still unspent
— which was the live risk when that work started, since a size mark
carrying "14pt" would have spent the property fragment_html's unescaped
output rests on, for a feature nobody thought of as a security change.
The closed enumeration is what makes it additive instead.
node_json was restructured to collect attributes into one object rather
than write a complete `,"attrs":{…}` from independent per-attribute
arms. That is now the shape a valued mark has to copy, and the reason is
worth reading before touching the JSON path: the old shape would have
emitted "attrs" twice in one object the moment a node carried two, which
every JSON parser accepts, none agrees on, and nothing would report.
D3 gains the address revision 2 could not give. mark_names
(fragment.rs:520) returns names and text_json emits each as a bare
{"type":…}; that function is the single point both the JSON consumers
and the HTML renderer agree on what marks a run carries, so it is where
the allowlist belongs — which is what makes "the check runs once, in the
shared path" true rather than aspirational. Also noted: mark_names
carries UNKNOWN marks through by name, so the allowlist must reject an
unknown mark's value rather than assume only link has one.
Tasks 4 and 5 shrink. document.go's Attrs is already widened, so only
Mark is left. The fixture generator now runs the real encoding path and
already carries cases for values no editor can produce but a raw POST
can — the exact shape the adversarial URLs need. And the Insert menu
ships Link, Image and Table present-but-disabled, reserving
insert-link, insert-image and insert-table, so implementing against
those names needs no menu change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 file changed,
+57
−16
docs/superpowers/plans/2026-09-02-images-links-tables.md+57 −16
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 b4a6b14..a342cb8 100644 |
| --- a/docs/superpowers/plans/2026-09-02-images-links-tables.md |
| +++ b/docs/superpowers/plans/2026-09-02-images-links-tables.md |
| @@ -15,6 +15,16 @@ depend on, and a storage seam the app does not yet have. |
| ordering and D6's expectation — and corrected several facts, the largest |
| being that revision 1 proposed writing an escaper that already exists.* |
| +*Revision 3, after `250db1c` landed the word-processor basics. Nothing |
| +here is re-decided; three tasks got smaller and one got a precise |
| +address. `MARKS` now holds eleven point-size marks plus the four named |
| +sizes it replaced, all still attribute-free — so `link` remains the |
| +first valued mark and D1 remains unspent. `node_json` was restructured |
| +to collect attributes into one object, which is the shape a valued mark |
| +must copy. `document.go`'s `Attrs` is already widened. The fixture |
| +generator now runs the real encoding path with raw-`POST`-only cases in |
| +it. And the Insert menu already reserves the three command names.* |
| + |
| ## What is actually broken today |
| Verified against `origin/main` by running Google-Docs clipboard HTML |
| @@ -141,9 +151,32 @@ 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. |
| +`internal/export/document.go` has to grow with this. Revision 2 said its |
| +`Mark` and its `Attrs` both needed widening; `250db1c` has since widened |
| +`Attrs` (adding `Indent` beside `Level` and `Align`), so the pattern |
| +exists and only `Mark` is left — it is still `{Type string}`. |
| + |
| +The Rust side is more specific than revision 2 could be. `mark_names` |
| +(`fragment.rs:520`) returns a `Vec<String>` of names, and `text_json` |
| +emits each as a bare `{"type":"…"}`. **That function is where the |
| +allowlist belongs**: it is the single place both the JSON consumers and |
| +the HTML renderer agree on what marks a run carries, so a rejected |
| +`href` dropping the mark there is what makes point 3 above true rather |
| +than aspirational. It needs to return names *with their values* — and |
| +`node_json`'s attribute handling, restructured in `250db1c` to collect |
| +into one object rather than write a complete `,"attrs":{…}` from |
| +independent arms, is the shape to copy. That restructure is worth |
| +reading before touching this: the old shape would have emitted `"attrs"` |
| +twice in one object the moment a node carried two attributes, which |
| +every JSON parser accepts, none agrees on, and nothing would report. |
| + |
| +One thing `mark_names` already does that matters here: it carries |
| +*unknown* marks through by name rather than dropping them, on the same |
| +reasoning as unknown nodes rendering their children. So a mark a raw |
| +`POST` invents reaches the JSON output as a name. It is escaped by |
| +`escape_json` and contributes no HTML wrapper, so this is not a hole — |
| +but a valued mark makes the question live, and the allowlist has to |
| +reject an unknown mark's *value* rather than assume only `link` has one. |
| ## D4 — the image is on screen at once; the node lands when the bytes do |
| @@ -322,22 +355,30 @@ a wrapper so a wide table scrolls inside the page, and `.selectedCell`. |
| 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. |
| +Revision 2 warned that `fixtures/generate-fragment.mjs` hand-built its |
| +vectors from y-prosemirror's *described* behaviour, which is fine for |
| +element names and numeric attrs and not fine for a mark, whose wire value |
| +is a `mark.attrs` object under the mark name — a shape no existing |
| +fixture had. `250db1c` moved the generator onto the real path and added |
| +cases for values no editor can produce but a raw `POST` can, which is |
| +exactly the shape the allowlist needs. So this task is now smaller than |
| +it was: add `links.bin`, `tables.bin` and `images.bin` through the |
| +existing generated path, with the adversarial URLs among them. 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`, |
| +First commit: D1's grammar, the reject-to-text path in `mark_names` (see |
| +D3 — that is the shared point, not `node_html`), and the adversarial |
| +tests. Second: `MARKS`' valued variant, `document.go`'s `Mark`, |
| `markdown.go`'s `[text](href)`, `pdf.go`'s link styling and `/Link` |
| -annotation, `toolbar.mjs`'s add/remove link. |
| +annotation, and the `insert-link` / `remove-link` commands. |
| + |
| +The menu is already shaped for all three: `250db1c` ships File/Edit/View/ |
| +Insert/Format/Tools with **Insert → Link, Image and Table present but |
| +disabled**, reserving `insert-link`, `insert-image` and `insert-table` in |
| +`toolbar.mjs`'s command table. Implement against those names and no menu |
| +changes are needed — the items simply become enabled. |
| ### Task 6 — the sealing key, the blob store and the route |
| **Blocked on Paul's key decision.** Then `models.go` plus a migration for |