oficina / slides Public

Independently review Slides SECOND adoption of Estilo shared save status, exact94ac454 in /home/paulca/amadan.net/oficina/slides/.claude/worktrees/overnight-save-status against deployed/editor-owner baseline a863fb6. Read-only; no mutations, deployments or additional agents. Read bounded diff /tmp/oficina-slides-status-review/branch.diff, relevant implementation/tests and actual committed bundle delivery. Do not audit unrelated history or whole app. Shared approved design: /home/paulca/amadan.net/oficina/estilo/.claude/worktrees/overnight-save-status/implementation/save-status.md. Library pinned real private module amadan.net/oficina/estilo.git@v0.0.0-20260907201639-532746acdbc1 (read savestatus package in Go module cache). Root owns library/Docs first adoption review independently. This branch includes CI support commit db7be72 cherry-picked from previously green998534c (separate review pending), adding npm tests, twice-regenerated committed bundle check and browser Go tests to existing full gate. No original editor branch or main changes authorized. Review both support and adoption changes but not baseline editor history. Adoption: shared Go template and CSS/JS bytes through rastrillo.Assets hashed same-origin URLs, import module URL from root.dataset before editor setup. Module load failure shows existing localized load error and throws, so editor never silently mounts without persistence feedback. Shared status uses existing Saved/Saving/Offline strings; offline maps retrying. App-owned adapter keeps dropped-batch sticky across resync/save success; load error has independent live region. Duplicate Slides status CSS/DOM-writing removed. No new product copy, CDN imports, local replace or public repo visibility. Evidence: rendered asset bytes/mime/hash/cache tests pass;103+ JS fixtures pass; real browser editing and permanent422 dropped-batch/resync/next-success test pass, sticky Offline, retrying remains at320px. Deliberately clearing dropped flag makes lifecycle test fail saved!=retrying. Full gate log /tmp/oficina-slides-status-review/gate.log currently running; do not claim complete until final gate:ok. Private pin fetched with GOPRIVATE=amadan.net and no committed replace. Original Slides owner retains main integration authority. Return concise APPROVE or CHANGES REQUIRED, concrete blocking findings, and any remaining canary/review preconditions. Verify module failure flow in bundle, sticky-save/load-error independence, shared asset routing/security, semantic accessibility, and CI hidden-success paths. Avoid suggestions that change existing user strings.

Independent Claude Fable final delta review. You approved the whole Slides Estilo adoption at94ac454; canonical model evidence saved. New exact tip 6d4c90a5edbee3268c923263ba830a071ed29320; only following delta added. No tools needed: complete delta and previous verdict supplied. Prior approval remains subject to realCI and approved library pin/canary. New browser failure proof after patch: disabling dropped flag produces TestBrowserDroppedBatchStatusStaysOfflineAfterRecovery failure state=saved text=Saved after successful subsequent save. Restored code passes earlier fullgate; finalgate running. CI export appends private module path preserving existing patterns; release recipe scopes same env; error live region atomic; browser success/reject flags defer to next macrotask, after queue continuation. Review whether these exact changes resolve the timing finding and preserve correctness. Return concise APPROVE or CHANGES REQUIRED, no broad new audit. Previous verdict: **Verdict: APPROVE.** No blocking findings. Full gate finished `gate: ok` (`/tmp/oficina-slides-status-review/gate.log:355`). **Verified** - **Module failure flow in the committed bundle.** `editor.2bdf5acdba405fc5.js` contains the runtime `import(t)` against `dataset.saveStatusModule`, writes `textError` into `[slides-load-error]` and rethrows; `mountEditor` awaits it before any other setup, and the entry's catch marks `data-editor-failed`. The editor cannot mount without persistence feedback. - **Sticky drop / load-error independence.** `createEditorStatus` forces `retrying` whenever `dropped` or queue is offline; `loadFailed` only touches the separate span. `onRecovered` now says `saved` and the adapter overrides it. Unit tests cover both directions; the 422 browser test drives the real queue and resync. - **Asset routing.** `editorAssetFS` intercepts exactly two names, everything else is the embed. rastrillo hashes via `Stat`+`ReadFile`, so `fstest.MapFS` works; served bytes equal `savestatus.CSS()/JS()`, immutable cache asserted. Same-origin only, no CDN, no `replace`, no CSP in the app to block the dynamic import. - **Copy.** No new strings; `status.offline` reused as `Retrying`; `Failed` intentionally empty (silent). - **CI hidden-success paths.** Both scripts `set -euo pipefail`; `RASTRILLO_BROWSER_OPTIONAL` is unset so no Chromium fails, not skips; double build plus `--untracked-files=all` catches a renamed hash; `go test` output lists `slidestest ok` twice. - **Accessibility.** Shared span keeps `role=status`, polite, atomic, `aria-label`; weight-plus-words emphasis retained in shared inline CSS; header is `flex-wrap`, text has `overflow-wrap:anywhere` for 320px. **Non-blocking notes** - `TestBrowserDroppedBatchStatusStaysOfflineAfterRecovery` reads state right after the wrapper flags a 2xx, possibly before the queue's `onStatus('ok')` runs, so it could pass with sticky broken. The Node lifecycle test is the real proof (mutation confirmed). Consider waiting on a post-save signal before asserting. - On module failure the server-rendered span still shows "Saved" beside the load error. Acceptable under the no-new-copy rule; worth a follow-up in Estilo (e.g. an `idle` initial state for adopters). - `[slides-load-error]` lost the old muted/emphasis styling and has no `aria-atomic`. Cosmetic. - `.amadan/README.md` still says "There is no Node in this gate" and the browser drive is "deliberately outside". Now false after db7be72; belongs to that review. **Preconditions before merge/canary** 1. `amadan ci status -branch feat/estilo-save-status` exit 0. Local log does not prove the runner has node, npm, Chromium, `GOPRIVATE=amadan.net` and git credentials for the estilo fetch. 2. Discussion opened for the branch before the squash merge (AGENTS.md rule). 3. Root's Estilo/Docs review confirms the pinned commit `532746acdbc1` is the approved library revision. 4. Post-deploy: editor page fetches `/static/estilo-save-status.<hash>.mjs` with a JavaScript MIME type and 200 on the real host, and `[slides-editor]` carries no `data-editor-failed`. Complete delta: diff --git a/.amadan/ci.d/10-gate b/.amadan/ci.d/10-gate index 9dc4388..ad3a390 100755 --- a/.amadan/ci.d/10-gate +++ b/.amadan/ci.d/10-gate @@ -3,6 +3,7 @@ # squash-merge, in one script, so what the runner executes and what you # run before pushing are one definition rather than two that drift. set -euo pipefail +export GOPRIVATE="${GOPRIVATE:+$GOPRIVATE,}amadan.net/oficina/estilo.git" # Run from the repo root whatever the caller's working directory is: # .amadan/ci execs this file, an amadan runner with step support runs it diff --git a/Makefile b/Makefile index fb19311..e6e8101 100644 --- a/Makefile +++ b/Makefile @@ -42,7 +42,7 @@ build: # that. rastrillo dev never strips, for the same reason. release: version-check @mkdir -p releases - CGO_ENABLED=0 GOOS=$(RELEASE_GOOS) GOARCH=$(RELEASE_GOARCH) \ + GOPRIVATE="$${GOPRIVATE:+$$GOPRIVATE,}amadan.net/oficina/estilo.git" CGO_ENABLED=0 GOOS=$(RELEASE_GOOS) GOARCH=$(RELEASE_GOARCH) \ go build -ldflags="-s -w -X 'amadan.net/rastrillo/rastrillo.BuildVersion=$(VERSION)'" \ -o $(RELEASE_BIN) ./cmd/$(APP) @echo diff --git a/internal/slides/templates/editor.html b/internal/slides/templates/editor.html index 4ce7f63..68be6c8 100644 --- a/internal/slides/templates/editor.html +++ b/internal/slides/templates/editor.html @@ -79,7 +79,7 @@ <button rst-btn="primary" type="submit">{{.PresentLabel}}</button> </form> {{template "estilo-save-status" .SaveStatus}} - <span slides-load-error role="status" aria-live="polite"></span> + <span slides-load-error role="status" aria-live="polite" aria-atomic="true"></span> </header> {{/* One strip item, the shape the bundle clones for a slide it adds: diff --git a/internal/slidestest/save_status_browser_test.go b/internal/slidestest/save_status_browser_test.go index 4c99183..750f6e5 100644 --- a/internal/slidestest/save_status_browser_test.go +++ b/internal/slidestest/save_status_browser_test.go @@ -23,9 +23,9 @@ func TestBrowserDroppedBatchStatusStaysOfflineAfterRecovery(t *testing.T) { let rejectNext = true; window.fetch = async (...args) => { if (String(args[0]).endsWith('/updates') && args[1]?.method === 'POST') { - if (rejectNext) { rejectNext = false; return new Response('', {status:422}); } + if (rejectNext) { rejectNext = false; setTimeout(() => window.statusProbeRejected = true, 0); return new Response('', {status:422}); } const response = await fetchBefore(...args); - if (response.ok) window.statusProbeSaved = true; + if (response.ok) setTimeout(() => window.statusProbeSaved = true, 0); return response; } return fetchBefore(...args); @@ -33,7 +33,7 @@ func TestBrowserDroppedBatchStatusStaysOfflineAfterRecovery(t *testing.T) { })()`, nil), chromedp.Click(heading, chromedp.ByQuery), chromedp.SendKeys(heading, string(kb.End)+" discarded-fixture", chromedp.ByQuery), - waitFor(`document.querySelector('[estilo-save-status]').dataset.state === 'retrying'`), + waitFor(`window.statusProbeRejected === true`), waitFor(`!document.querySelector('`+heading+`').textContent.includes('discarded-fixture')`), chromedp.Click(heading, chromedp.ByQuery), chromedp.SendKeys(heading, string(kb.End)+" retained-fixture", chromedp.ByQuery),