amadan
app: fix round 1 on Task 6 — permanent save failures, visible status, cap headroom
F1 (HIGH): the save queue's catch-all requeued any non-ok response
unconditionally. Right for a network blip or a 5xx (worth retrying),
wrong for a 4xx: the core has already looked at those exact bytes and
refused them, so retrying is futile, and the next flush would
Y.mergeUpdates the requeued batch with whatever got typed since,
dragging every later edit down with it. Reproduced for real: a
~1.2MB paste got 413, every later flush also 413'd, and after reload
the paste and both subsequent edits were gone with nothing shown.
Fixed by factoring the queue into fixtures/save-queue.mjs (post/
mergeUpdates/timing injected) so the retry-vs-drop decision runs
under plain Node with no browser — fixtures/save-queue.test.mjs
reproduces the loss against the pre-fix logic (confirmed failing for
the right reason) and passes against the fix: a 4xx drops its batch
and reports failure without touching what's typed next; a network
failure or 5xx requeues and retries. Wired into .amadan/ci.d/10-gate.
F2 (MEDIUM): data-save-failed had no CSS and no copy, so F1's failure
was invisible — which is also why it took a real repro to find rather
than a glance at the screen. Added docs.editor.status.{retrying,
failed} to render.go's catalog, rendered hidden as [docs-save-status]
next to (never inside) [docs-editor] in show.html, styled with the
app's own --rst-tone-* tokens: quiet/neutral while retrying, the
negative tone and sticky once a batch is actually dropped, so a later
successful save can't quietly hide that something was lost.
F3 (MEDIUM): one doc_updates row per 400ms flush made core.go's
4096-update cap reachable after ~27 minutes of continuous typing, and
hitting it (ErrUpdateLogFull, a 400) used to poison the queue via F1.
Widened the debounce to 2000ms (4096 * 2s ≈ 136min ≈ 2h16m of
continuous typing before the cap, arithmetic in editor-entry.js's own
comment) as cheap headroom, left a pointer at maxRetainedUpdates
naming snapshots as the real fix, and F1's fix already makes hitting
the cap a reported failure rather than a silent one.
LOW: beforeunload only beaconed queued-but-unsent bytes, not bytes
already in flight when the tab closed — save-queue.mjs's pendingBytes
now covers both. sendBeacon's return value is checked; a refusal
(its own ~64KB quota) falls back to a keepalive fetch instead of
silently doing nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011F5CivVm4CmL9ByfSSAaXA
8 files changed,
+408
−33
.amadan/ci.d/10-gate+10 −0fixtures/editor-entry.js+71 −33fixtures/save-queue.mjs+118 −0fixtures/save-queue.test.mjs+96 −0internal/core/core.go+15 −0internal/docs/handlers.go+16 −0internal/docs/render.go+15 −0internal/docs/views/show.html+67 −0
Intent
Human
<task-notification> <task-id>a7260d609c3b53ad8</task-id> <tool-use-id>toolu_01E7RrFaRk34N4g965ryfHJL</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-carlos-docs/70f302d1-9d5d-4eb2-8b38-e09a15fdd1c3/tasks/a7260d609c3b53ad8.output</output-file> <status>completed</status> <summary>Agent "Editing Task 6 the editor" 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>## Summary **Status:** Task 6 complete. `.amadan/ci.d/10-gate` exits 0. **Commit:** `c52259f` on branch `editing`. **Bundle:** `internal/docs/static/editor.283b4d376ff04289.js`, 304998 bytes (reproducible — gate rebuilds and diffs clean). Implemented the ProseMirror editor per the brief (`fixtures/editor-entry.js`), added the `docs-editor`/`data-sqid` mount point and script tag to `internal/docs/views/show.html`, wired `Sqid`/`EditorScript` through `handlers.go`'s `showPage`, and added `internal/docs/editor_view_test.go` (failing first, then passing). While actually exercising the running app (per Step 6), found and fixed two real bugs that the green test suite never caught: 1. `new EditorView(el, {state})` **appends** a sibling DOM node instead of replacing `el`'s contents — fixed with `{mount: el}`. 2. `GET /d/{sqid}/updates` returns a raw, unframed concatenation of per-POST Yjs updates; a single `Y.applyUpdate` call silently decodes only the first message, so a second edit vanished on reload. Proved this in isolation with real `yjs` first, then fixed by looping `Y.readUpdate` over one `lib0/decoding` decoder until exhausted (added `lib0` as an explicit fixtures dependency). Verified in a real Playwright-driven browser session (real magic-link sign-in): typed a sentinel sentence into a seeded document, waited past the debounce, reloaded, and confirmed the text persisted. Then confirmed with plain `curl` (no JS at all) that the same edit renders server-side, with exactly one `docs-editor` mount point referencing the committed bundle. Full details, including the real command output, are in `/home/paulca/amadan.net/carlos/docs/.superpowers/sdd/2026-08-31-editing/task-6-report.md`.</result> <usage><subagent_tokens>198859</subagent_tokens><tool_uses>142</tool_uses><duration_ms>1224190</duration_ms></usage> </task-notification>
Human
<task-notification> <task-id>ae8b2ec985c624cae</task-id> <tool-use-id>toolu_01VEB4YidyhFPWfKJtVkZm6n</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-carlos-docs/70f302d1-9d5d-4eb2-8b38-e09a15fdd1c3/tasks/ae8b2ec985c624cae.output</output-file> <status>completed</status> <summary>Agent "Whole-branch review editing" 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>**MERGE** — with one HIGH follow-up to land immediately. Full review: `/home/paulca/amadan.net/carlos/docs/.superpowers/sdd/2026-08-31-editing/final-review.md`. **What I did in the browser** (real Chromium, real magic-link sign-in, dev server on 8177): two separate edits with reload — both persisted (the fixed decode bug stays fixed); edit → reload → edit with a fresh Y.Doc client → all persisted; rapid typing across five debounce windows — all persisted; navigate-away inside the 400ms window — the beacon really carried the bytes; one aborted POST — requeue/retry recovered; a persisted-but-client-saw-failure resend — idempotent, content once. Then plain `curl`: the server-rendered page contains the browser-made edits verbatim (§6.2 holds for edited documents), and CSRF probes got 403. `go test ./...`, `cargo test` (DOCS_REQUIRE_WASM=1), and `10-gate` all green; tree left clean, server stopped. **Findings:** 1. **HIGH, reproduced — F1**: `fixtures/editor-entry.js:88-97` requeues any `!r.ok` flush and merges later edits into the poisoned batch. A ~1.2MB paste → 413 → observed `413,413,413` for every later edit → after reload the paste **and both later small edits were gone**, silently. Same sibling class as the decode bug; violates the file's own "never lose an edit silently" invariant. Not BLOCKING only because it needs a >1MB single flush and loses nothing until tab close; fix is ~5 lines (don't retry 4xx / don't merge a rejected batch). 2. **MEDIUM — F2**: `data-save-failed` has no CSS and no catalog copy — save failure is invisible (plan-authored). 3. **MEDIUM — F3**: one row per 400ms flush makes the core's 4096-update cap (`internal/core/core.go:103`) reachable in ordinary sustained use; hitting it is a permanent per-document F1 wedge. §4.1 snapshots just became time-sensitive. 4. **LOW**: in-flight flush bytes aren't beaconed at unload, and `sendBeacon`'s return is ignored (~64KB quota); `Store.ApplyUpdate`'s owner re-check (`store.go:494`) doesn't filter `deleted_at` (unreachable today). No cross-task drift (root `"prosemirror"` consistent; the ABI text repoint chased through every caller; D1–D5 all honoured; no vacuous tests found in the branch's additions). **Security composes**: every path to the CRDT passes session → origin-check (verified 403) → owner-scoped 404 → size cap → DB-re-derived ownership → wasm-sandboxed yrs, with escaping done once in Rust against a fixed allowlist emitting zero attributes — nothing opens what another layer closed.</result> <usage><subagent_tokens>172221</subagent_tokens><tool_uses>56</tool_uses><duration_ms>576454</duration_ms></usage> </task-notification>
diff --git a/.amadan/ci.d/10-gate b/.amadan/ci.d/10-gate| index 4fca6e4..d58fa1f 100755 |
| --- a/.amadan/ci.d/10-gate |
| +++ b/.amadan/ci.d/10-gate |
| @@ -27,6 +27,16 @@ export CGO_ENABLED=0 |
| echo "=== fixtures: node dependencies" |
| (cd fixtures && npm ci) |
| +# save-queue.mjs is the editor's retry/drop logic, factored out so it |
| +# runs under plain Node with no DOM/browser needed (fixtures/ |
| +# save-queue.mjs's own doc comment). This is fix round 1's regression |
| +# test for F1 — a permanently-rejected batch used to retry forever and |
| +# absorb every later edit, losing all of it silently — and it belongs |
| +# in the gate, not just something run by hand once, or a future |
| +# regression here goes uncaught the same way F1 itself did. |
| +echo "=== fixtures: the save queue's retry/drop logic (F1 regression)" |
| +(cd fixtures && node --test save-queue.test.mjs) |
| + |
| # fixtures/build-editor.mjs is byte-reproducible: no sourcemaps, no |
| # banners, no timestamps, no absolute paths baked into the output, and |
| # esbuild's actual work happens in a pinned native binary (0.28.2), |
diff --git a/fixtures/editor-entry.js b/fixtures/editor-entry.js| index c871e2c..3186ca5 100644 |
| --- a/fixtures/editor-entry.js |
| +++ b/fixtures/editor-entry.js |
| @@ -20,17 +20,44 @@ import { addListNodes } from 'prosemirror-schema-list' |
| import { keymap } from 'prosemirror-keymap' |
| import { baseKeymap, toggleMark } from 'prosemirror-commands' |
| import { ySyncPlugin, yUndoPlugin, undo, redo } from 'y-prosemirror' |
| +import { createSaveQueue } from './save-queue.mjs' |
| const schema = new Schema({ |
| nodes: addListNodes(basic.spec.nodes, 'paragraph block*', 'block'), |
| marks: basic.spec.marks, |
| }) |
| +// Coalesce a burst of typing into one request. Widened from Task 6's |
| +// original 400ms (fix round 1, F3): at one accepted row per flush, |
| +// core.Handle's maxRetainedUpdates cap (internal/core/core.go, 4096) |
| +// was reachable after only ~27 minutes of continuous typing |
| +// (4096 * 400ms ≈ 27min). 2000ms stretches the same arithmetic to |
| +// 4096 * 2s ≈ 8192s ≈ 136 minutes — about 2h16m of typing with no |
| +// pause longer than 2 seconds anywhere in it, which real writing |
| +// does not do; ordinary pauses (thinking, punctuation, rereading) |
| +// push the practical number well past that worst case. Nothing here |
| +// shows a "saving" indicator during an ordinary 2s wait — only a |
| +// failure does, via the save queue's own status (see save-queue.mjs |
| +// and views/show.html's [docs-save-status]) — so this delay is |
| +// invisible exactly when saving is going fine, which is the case |
| +// that matters for feel. This only buys headroom against the cap; |
| +// the real fix is snapshot/compaction (core.go's own comment on |
| +// maxRetainedUpdates). |
| +const DEBOUNCE_MS = 2000 |
| + |
| export async function mountEditor(el) { |
| const sqid = el.getAttribute('data-sqid') |
| const base = `/d/${sqid}/updates` |
| const ydoc = new Y.Doc() |
| + // The save-status indicator (fix round 1, F2) is a sibling of el, |
| + // never a descendant: ProseMirror's {mount: el} below takes over |
| + // everything inside el, so anything meant to survive that has to |
| + // live outside it. views/show.html renders it, hidden, with both |
| + // states' text already server-rendered (catalog copy, never a |
| + // literal in this file) — this only ever toggles which one shows. |
| + const statusEl = document.querySelector('[docs-save-status]') |
| + |
| // Load: replay the server's log. handlers.go's updatesGet writes it |
| // as a raw concatenation of one encoded Yjs update per accepted |
| // POST, back to back with no framing between them (its own doc |
| @@ -56,50 +83,61 @@ export async function mountEditor(el) { |
| const fragment = ydoc.getXmlFragment('prosemirror') |
| + // The save queue (fixtures/save-queue.mjs) owns the retry/drop |
| + // decision — fix round 1's F1: a permanently-rejected batch (a 4xx) |
| + // must be dropped, not retried forever and not merged into by |
| + // whatever gets typed next, while a network failure or a 5xx is |
| + // worth retrying. See that module's own doc comment for the bug |
| + // this replaced and how fixtures/save-queue.test.mjs reproduces it. |
| + const saveQueue = createSaveQueue({ |
| + post: (bytes) => fetch(base, { |
| + method: 'POST', |
| + credentials: 'same-origin', |
| + headers: { 'Content-Type': 'application/octet-stream' }, |
| + body: bytes, |
| + }), |
| + mergeUpdates: Y.mergeUpdates, |
| + debounceMs: DEBOUNCE_MS, |
| + onStatus: (state) => { |
| + if (statusEl) statusEl.setAttribute('data-state', state) |
| + }, |
| + }) |
| + |
| // Send only what originated here. The 'server' origin above marks |
| // replayed bytes, and echoing those back would grow the log without |
| // adding information. |
| - let queue = [] |
| - let timer = null |
| ydoc.on('update', (update, origin) => { |
| if (origin === 'server') return |
| - queue.push(update) |
| - if (timer) return |
| - // Coalesce a burst of keystrokes into one request. 400ms is below |
| - // the threshold where a person notices a save is pending and well |
| - // above per-character chatter. |
| - timer = setTimeout(flush, 400) |
| + saveQueue.enqueue(update) |
| }) |
| - async function flush() { |
| - timer = null |
| - if (!queue.length) return |
| - const merged = Y.mergeUpdates(queue) |
| - queue = [] |
| - try { |
| - const r = await fetch(base, { |
| + // A close or reload with unsent bytes must not lose the edit. |
| + // pendingBytes() covers both what is still queued AND what is |
| + // already in flight (fix round 1's LOW finding: a flush's own |
| + // fetch, awaited but not yet resolved, held bytes that a same-tick |
| + // unload would otherwise abandon with nothing beaconed at all, |
| + // since they had already been cleared out of the queue proper). |
| + addEventListener('beforeunload', () => { |
| + const bytes = saveQueue.pendingBytes() |
| + if (!bytes) return |
| + const blob = new Blob([bytes], { type: 'application/octet-stream' }) |
| + const sent = navigator.sendBeacon(base, blob) |
| + if (!sent) { |
| + // sendBeacon can refuse outright — its total in-flight quota is |
| + // small (browsers commonly cap it around 64KB) and a paste-sized |
| + // edit can exceed it. A keepalive fetch survives unload the same |
| + // way and is worth trying even though its own outcome can never |
| + // be observed from here; doing nothing on refusal would be |
| + // exactly the silent loss this whole path exists to prevent. |
| + console.error('beforeunload: sendBeacon refused the pending edit, retrying via keepalive fetch') |
| + fetch(base, { |
| method: 'POST', |
| credentials: 'same-origin', |
| + keepalive: true, |
| headers: { 'Content-Type': 'application/octet-stream' }, |
| - body: merged, |
| - }) |
| - if (!r.ok) throw new Error(`save: ${r.status}`) |
| - el.removeAttribute('data-save-failed') |
| - } catch (err) { |
| - // Put the bytes back so the next flush retries them. Dropping |
| - // them would lose the edit silently, which is the one outcome a |
| - // text editor may never produce. |
| - queue.unshift(merged) |
| - el.setAttribute('data-save-failed', '1') |
| - console.error(err) |
| + body: bytes, |
| + }).catch(() => {}) |
| } |
| - } |
| - |
| - // A close or reload with a pending timer must not lose the edit. |
| - addEventListener('beforeunload', () => { |
| - if (!queue.length) return |
| - navigator.sendBeacon(base, new Blob([Y.mergeUpdates(queue)], |
| - { type: 'application/octet-stream' })) |
| }) |
| // {mount: el}, not plain el: passed as a bare DOM node, EditorView |
diff --git a/fixtures/save-queue.mjs b/fixtures/save-queue.mjs| new file mode 100644 |
| index 0000000..53064cc |
| --- /dev/null |
| +++ b/fixtures/save-queue.mjs |
| @@ -0,0 +1,118 @@ |
| +// fixtures/save-queue.mjs — the editor's save queue, factored out of |
| +// fixtures/editor-entry.js so its retry/drop logic can be exercised |
| +// under plain Node with no DOM, no ProseMirror, and no real Y.Doc — |
| +// see fixtures/save-queue.test.mjs. |
| +// |
| +// This is fix round 1's F1 fix. The original inline version in |
| +// editor-entry.js requeued a failed batch unconditionally: right for |
| +// a network blip or a 500 (worth retrying), catastrophic for a 4xx |
| +// (the core has already looked at these exact bytes and refused |
| +// them — retrying is futile, and worse, the NEXT flush would run |
| +// Y.mergeUpdates(queue) over the requeued batch plus whatever the |
| +// person typed since, merging every later edit into a batch that can |
| +// never be accepted. A ~1.2MB paste that gets 413 would then also |
| +// carry away everything typed afterwards, forever, with nothing |
| +// shown on screen — reproduced for real against a running server |
| +// before this fix went in. |
| +// |
| +// The fix: classify the response. A network failure (post() rejects) |
| +// or a 5xx is retryable — our own fault or a transient one, so the |
| +// bytes go back on the queue and a retry is scheduled. A 4xx is |
| +// permanent — the bytes themselves are the problem, so the batch is |
| +// dropped (never requeued, never merged into) and the failure is |
| +// reported instead of hidden. Ordinary edits keep flowing through the |
| +// same queue immediately afterwards, unaffected. |
| +// |
| +// post/mergeUpdates/setTimeoutFn are injected rather than imported so |
| +// the test can drive this with tiny byte arrays, a stub network call, |
| +// and a fake clock instead of real Yjs updates, a real fetch, and |
| +// real timers — the control flow being tested does not depend on any |
| +// of those being real. |
| +export function createSaveQueue({ post, mergeUpdates, onStatus, debounceMs = 2000, setTimeoutFn = setTimeout }) { |
| + let queue = [] |
| + // Bytes currently in flight (already merged and sent, response not |
| + // back yet) live here, separately from queue — not just so a |
| + // concurrent enqueue() during the await doesn't get folded into a |
| + // request already on the wire, but so pendingBytes() below can |
| + // include them: bytes an in-flight fetch is carrying are exactly |
| + // as at-risk from a same-tick tab close as bytes still sitting in |
| + // queue (fix round 1's LOW finding), so beforeunload must be able |
| + // to beacon both. |
| + let inFlight = null |
| + let timer = null |
| + let status = 'ok' |
| + |
| + function report(next) { |
| + // 'failed' is sticky: once bytes are known dropped, a later |
| + // successful save must not quietly erase that history from |
| + // view — the person still needs to know something did not make |
| + // it, even after the next paragraph saves fine. |
| + if (status === 'failed' && next !== 'failed') return |
| + status = next |
| + if (onStatus) onStatus(status) |
| + } |
| + |
| + function scheduleRetry() { |
| + if (!timer) timer = setTimeoutFn(flush, debounceMs) |
| + } |
| + |
| + async function flush() { |
| + timer = null |
| + if (!queue.length) return |
| + const merged = mergeUpdates(queue) |
| + queue = [] |
| + inFlight = merged |
| + |
| + let r |
| + try { |
| + r = await post(merged) |
| + } catch (err) { |
| + inFlight = null |
| + queue.unshift(merged) |
| + report('retrying') |
| + scheduleRetry() |
| + return |
| + } |
| + inFlight = null |
| + |
| + if (r.ok) { |
| + report('ok') |
| + return |
| + } |
| + if (r.status >= 400 && r.status < 500) { |
| + // Permanent: drop it. Do not requeue, do not merge later edits |
| + // into it — see this module's own header comment. |
| + report('failed') |
| + return |
| + } |
| + // 5xx: not the client's fault, worth retrying. |
| + queue.unshift(merged) |
| + report('retrying') |
| + scheduleRetry() |
| + } |
| + |
| + function enqueue(update) { |
| + queue.push(update) |
| + if (timer) return |
| + timer = setTimeoutFn(flush, debounceMs) |
| + } |
| + |
| + // pendingBytes merges whatever is still owed to the server — |
| + // queued-but-unsent and in-flight-but-unconfirmed alike — into one |
| + // update, for beforeunload to beacon. null when there is nothing |
| + // to send. |
| + function pendingBytes() { |
| + const parts = [] |
| + if (inFlight) parts.push(inFlight) |
| + if (queue.length) parts.push(mergeUpdates(queue)) |
| + if (!parts.length) return null |
| + return parts.length === 1 ? parts[0] : mergeUpdates(parts) |
| + } |
| + |
| + return { |
| + enqueue, |
| + flush, |
| + pendingBytes, |
| + get status() { return status }, |
| + } |
| +} |
diff --git a/fixtures/save-queue.test.mjs b/fixtures/save-queue.test.mjs| new file mode 100644 |
| index 0000000..c8968ea |
| --- /dev/null |
| +++ b/fixtures/save-queue.test.mjs |
| @@ -0,0 +1,96 @@ |
| +// fixtures/save-queue.test.mjs — regression test for fix round 1's |
| +// F1: a permanently-rejected batch used to retry forever AND absorb |
| +// every later edit via Y.mergeUpdates, losing all of it silently. |
| +// Reproduced against a real running server (a ~1.2MB paste got a |
| +// real 413, every later flush also 413'd, and the paste plus both |
| +// subsequent small edits were gone after reload) before the fix in |
| +// save-queue.mjs went in. |
| +// |
| +// Runs under Node's own test runner (node --test), no framework |
| +// added: `post`/`mergeUpdates`/timing are all injected by |
| +// createSaveQueue, so this drives the real retry/drop control flow |
| +// with tiny stand-in byte arrays and a short debounce — no DOM, no |
| +// ProseMirror, no real Y.Doc, no browser needed. |
| +import { test } from 'node:test' |
| +import assert from 'node:assert/strict' |
| +import { createSaveQueue } from './save-queue.mjs' |
| + |
| +// A fake mergeUpdates that behaves like Y.mergeUpdates for this |
| +// test's purposes: concatenate, in order, so a merged batch's |
| +// identity is checkable byte-for-byte. |
| +function concatUpdates(updates) { |
| + const out = [] |
| + for (const u of updates) out.push(...u) |
| + return new Uint8Array(out) |
| +} |
| + |
| +function makePostStub(responses) { |
| + const calls = [] |
| + let i = 0 |
| + return { |
| + calls, |
| + post: async (bytes) => { |
| + calls.push(bytes) |
| + const next = responses[Math.min(i, responses.length - 1)] |
| + i++ |
| + if (next.throws) throw new Error('network failure') |
| + return { ok: next.status < 300, status: next.status } |
| + }, |
| + } |
| +} |
| + |
| +test('a permanent 4xx drops its batch instead of retrying it, and does not poison the next edit', async () => { |
| + const { calls, post } = makePostStub([{ status: 413 }, { status: 204 }]) |
| + const statuses = [] |
| + const q = createSaveQueue({ |
| + post, |
| + mergeUpdates: concatUpdates, |
| + onStatus: (s) => statuses.push(s), |
| + debounceMs: 5, |
| + }) |
| + |
| + const bigPaste = new Uint8Array([9, 9, 9]) |
| + q.enqueue(bigPaste) |
| + await new Promise((r) => setTimeout(r, 40)) |
| + |
| + assert.equal(calls.length, 1, 'the oversized batch should be posted exactly once') |
| + assert.deepEqual(calls[0], bigPaste) |
| + assert.equal(q.status, 'failed', 'a permanent rejection must be reported, not swallowed') |
| + assert.equal(q.pendingBytes(), null, 'a dropped batch must not sit in the queue forever') |
| + |
| + // The crux of F1: an edit typed after the permanent rejection must |
| + // reach the server on its own, never merged with the dead batch. |
| + const secondEdit = new Uint8Array([1, 2, 3]) |
| + q.enqueue(secondEdit) |
| + await new Promise((r) => setTimeout(r, 40)) |
| + |
| + assert.equal(calls.length, 2, 'the later edit must be posted, not swallowed') |
| + assert.deepEqual(calls[1], secondEdit, 'the later edit must be posted on its own — merging it with the dead 413 batch would 413 it too') |
| + assert.equal(statuses.filter((s) => s === 'failed').length >= 1, true) |
| + assert.equal(q.status, 'failed', "the indicator must stay sticky — a later success must not hide that something was lost") |
| +}) |
| + |
| +test('a network failure or a 5xx is retried, not dropped', async () => { |
| + const { calls, post } = makePostStub([{ throws: true }, { status: 204 }]) |
| + const q = createSaveQueue({ post, mergeUpdates: concatUpdates, debounceMs: 5 }) |
| + |
| + const edit = new Uint8Array([7, 7]) |
| + q.enqueue(edit) |
| + await new Promise((r) => setTimeout(r, 60)) |
| + |
| + assert.equal(calls.length, 2, 'a network failure must be retried automatically, not just left queued') |
| + assert.deepEqual(calls[0], edit) |
| + assert.deepEqual(calls[1], edit, 'the retried batch must be the exact same bytes, not lost or duplicated') |
| + assert.equal(q.status, 'ok', 'once the retry lands, nothing was actually lost') |
| +}) |
| + |
| +test('a 500 is retried the same way a network failure is, never dropped', async () => { |
| + const { calls, post } = makePostStub([{ status: 500 }, { status: 204 }]) |
| + const q = createSaveQueue({ post, mergeUpdates: concatUpdates, debounceMs: 5 }) |
| + |
| + q.enqueue(new Uint8Array([4, 4])) |
| + await new Promise((r) => setTimeout(r, 60)) |
| + |
| + assert.equal(calls.length, 2, 'a 5xx is our own fault, not the client bytes — it must retry, not drop') |
| + assert.equal(q.status, 'ok') |
| +}) |
diff --git a/internal/core/core.go b/internal/core/core.go| index 54bad65..da6b0ac 100644 |
| --- a/internal/core/core.go |
| +++ b/internal/core/core.go |
| @@ -100,6 +100,21 @@ const defaultGuestCallTimeout = 1 * time.Second |
| // bound or silently dropping old entries — dropping old entries would make |
| // a later rebuild replay an incomplete history and produce silently wrong |
| // content, which is worse than refusing. |
| +// |
| +// How reachable this cap actually is, for whoever writes the |
| +// snapshot/compaction plan: the docs app persists one doc_updates row |
| +// per accepted POST (internal/docs/store.go's Store.ApplyUpdate), and |
| +// fixtures/editor-entry.js posts at most once per its own debounce |
| +// window — 400ms at Task 6's original writing, which put this 4096 |
| +// cap within reach after roughly 27 minutes of one person typing |
| +// continuously (4096 * 400ms). Task 6 fix round 1 (F3) widened that |
| +// debounce to 2000ms as cheap headroom (4096 * 2s ≈ 136 minutes) and |
| +// made hitting the cap a reported, non-silent condition on the client |
| +// (editor-entry.js's save queue treats ErrUpdateLogFull's 400 as |
| +// permanent, same as any other 4xx) rather than a client-side fix |
| +// here — but headroom is all a wider debounce ever buys. The actual |
| +// fix for a document that keeps being edited past that point is this |
| +// comment's own snapshot support, not a bigger number here. |
| const maxRetainedUpdates = 4096 |
| // ErrUpdateLogFull is returned by Handle.Apply once a document's retained |
diff --git a/internal/docs/handlers.go b/internal/docs/handlers.go| index 6871231..778790d 100644 |
| --- a/internal/docs/handlers.go |
| +++ b/internal/docs/handlers.go |
| @@ -190,6 +190,20 @@ type showPage struct { |
| SignOutLabel string // see listPage.SignOutLabel's own doc comment |
| Sqid string // views/show.html's [docs-editor] mount point's data-sqid, the editor bundle's own base for /d/{sqid}/updates |
| EditorScript string // views/show.html's <script src>, from editorScriptPath() — never a literal, so it can never drift from what staticHandler actually serves |
| + |
| + // SaveRetrying/SaveFailed are the editor's own save-status copy |
| + // (fix round 1, F2 on Task 6): views/show.html renders both, |
| + // hidden, next to [docs-editor] — never inside it, since |
| + // ProseMirror's {mount: el} takes over everything inside that |
| + // element — and fixtures/editor-entry.js toggles which one shows |
| + // via a sibling [docs-save-status]'s data-state, driven by the |
| + // save queue's own retryable-vs-permanent classification (F1). |
| + // Server-rendered text, not a literal in the JS: every |
| + // user-visible string is a catalog key (render.go), including |
| + // this one, so the bundle never carries its own copy to keep in |
| + // sync with the catalog by hand. |
| + SaveRetrying string |
| + SaveFailed string |
| } |
| // show serves GET /d/{sqid}: the document read-only, its title and |
| @@ -235,6 +249,8 @@ func (h *handlers) show(w http.ResponseWriter, r *http.Request) { |
| SignOutLabel: rastrillo.T(r, "docs.signout.label"), |
| Sqid: doc.Sqid, |
| EditorScript: editorScriptPath(), |
| + SaveRetrying: rastrillo.T(r, "docs.editor.status.retrying"), |
| + SaveFailed: rastrillo.T(r, "docs.editor.status.failed"), |
| } |
| if err := h.render.page(w, r, "show", data); err != nil { |
| h.logger.Error("docs: render show", "err", err) |
diff --git a/internal/docs/render.go b/internal/docs/render.go| index 9f5bdbc..474a214 100644 |
| --- a/internal/docs/render.go |
| +++ b/internal/docs/render.go |
| @@ -109,6 +109,21 @@ var Catalog = rastrillo.Catalog{ |
| // and show.html's page headers) — both its button label and its |
| // accessible name. |
| "docs.signout.label": "Sign out", |
| + |
| + // docs.editor.status.* is the save-status indicator fix round 1's |
| + // F2 added next to [docs-editor] (show.html), driven by |
| + // fixtures/editor-entry.js's save queue (F1's own fix). retrying |
| + // is the quiet state: a network hiccup or a 500 is being retried |
| + // automatically, nothing is lost yet, so this stays understated. |
| + // failed is the loud one: the core has permanently rejected a |
| + // batch (a 4xx — e.g. a paste over maxUpdateBytes, or |
| + // core.ErrUpdateLogFull once a document's log is full) and that |
| + // batch has been dropped rather than retried forever or left to |
| + // poison later edits (see save-queue.mjs's own doc comment) — the |
| + // one case where a person genuinely needs to know something did |
| + // not save, in time to do something about it. |
| + "docs.editor.status.retrying": "Trouble saving. Retrying…", |
| + "docs.editor.status.failed": "Some changes couldn't be saved. Copy anything important before leaving this page.", |
| } |
| // Locales and DefaultLocale declare this app's locale set for |
diff --git a/internal/docs/views/show.html b/internal/docs/views/show.html| index 54df8f8..49a63eb 100644 |
| --- a/internal/docs/views/show.html |
| +++ b/internal/docs/views/show.html |
| @@ -158,6 +158,58 @@ |
| padding: 0.15em 0.35em; |
| border-radius: 3px; |
| } |
| + |
| + /* [docs-save-status] (fix round 1, F2) is chrome, not paper — it |
| + reports on the save itself, not the document's own content — so |
| + unlike [docs-page]/[docs-text] above it uses the app's own theme |
| + tokens (var(--rst-tone-*)) rather than literal colours, and is |
| + free to repaint with the reader's light/dark preference the way |
| + the header above already does. Hidden by default: fixtures/ |
| + editor-entry.js only ever sets data-state on it, and an ordinary |
| + save that goes through on the first try — the overwhelming |
| + common case — should be invisible, not merely quiet. retrying |
| + (a network blip or a 500, being retried automatically) stays |
| + understated, matching --rst-text-muted's own existing use for |
| + de-emphasised text; failed (a permanent rejection — save-queue. |
| + mjs dropped a batch rather than lose everything typed after it) |
| + uses the negative tone precisely because it is the one state a |
| + person must not be able to miss. Fixed position, not inline next |
| + to [docs-editor]: the document reflows under editing, and a |
| + status message pinned to a moving line would be easy to lose |
| + track of, unlike one anchored to the viewport. */ |
| + [docs-save-status] { |
| + display: none; |
| + position: fixed; |
| + right: var(--rst-sp-4); |
| + bottom: var(--rst-sp-4); |
| + max-inline-size: 22em; |
| + padding: var(--rst-sp-2) var(--rst-sp-3); |
| + border-radius: var(--rst-radius-sm); |
| + font-size: var(--rst-fs-sm); |
| + line-height: 1.4; |
| + box-shadow: 0 1px 4px rgba(0, 0, 0, 0.2); |
| + pointer-events: none; |
| + } |
| + [docs-save-status] span { |
| + display: none; |
| + } |
| + [docs-save-status][data-state="retrying"] { |
| + display: block; |
| + background: var(--rst-tone-neutral-bg); |
| + color: var(--rst-tone-neutral-fg); |
| + } |
| + [docs-save-status][data-state="retrying"] [docs-save-status-retrying] { |
| + display: inline; |
| + } |
| + [docs-save-status][data-state="failed"] { |
| + display: block; |
| + background: var(--rst-tone-negative-bg); |
| + color: var(--rst-tone-negative-fg); |
| + font-weight: 600; |
| + } |
| + [docs-save-status][data-state="failed"] [docs-save-status-failed] { |
| + display: inline; |
| + } |
| </style> |
| <header rst-page-header> |
| <div rst-page-header-titles> |
| @@ -188,6 +240,21 @@ |
| <div docs-text docs-editor data-sqid="{{.Sqid}}">{{.HTML}}</div> |
| </div> |
| </div> |
| +{{/* [docs-save-status] (fix round 1, F2): a sibling of [docs-editor], |
| + never inside it — ProseMirror's {mount: el} above takes over |
| + everything inside that element, so anything meant to survive a |
| + mount has to live outside it. Both states' text is rendered here, |
| + server-side, from the catalog (render.go's docs.editor.status.*) |
| + and hidden by CSS; editor-entry.js's save queue only ever flips |
| + data-state between "retrying" and "failed" (or leaves it unset, |
| + the default, quiet state) — the bundle carries no copy of its |
| + own to keep in sync with the catalog by hand. role="status" + |
| + aria-live="polite" so a failure is announced without stealing |
| + focus from whatever the person is doing when it appears. */}} |
| +<div docs-save-status role="status" aria-live="polite"> |
| + <span docs-save-status-retrying>{{.SaveRetrying}}</span> |
| + <span docs-save-status-failed>{{.SaveFailed}}</span> |
| +</div> |
| {{/* type="module" defer: modules are deferred by default, but stated |
| explicitly here so load order (after the DOM, before DOMContentLoaded) |
| doesn't depend on that default going unnoticed by a future edit. Served |