oficina / slides Public

Astra (GPT-6-Astra via Codex), whole-branch review of feat/r2-shared-chrome at b555e4de439b1e839d98a3ab8b52bab82f544201, against fetched origin/main (a88f7ff). Reviewed as one merge-base diff against AGENTS.md, REVIEW-BRIEF.md, the shared-chrome spec, Paul's R2 rulings and the original Slides audit.

VERDICT: CHANGES-REQUIRED

1. P2 / blocking — the projector is still a scrolling page, not the required full-screen stage. internal/slides/templates/present.html:23–43,106–120. Removing the app bar and setting min-block-size: 100dvh does not fit the slide to the viewport: padding and the in-flow pager remain, the stage is sized from width alone, and the background remains white. Live Chromium at 1920×1080 produces a 1128px document, with the pager starting at y=1082, offscreen; at 840×500 it produces a 520px document. The supplied r2-slides-after-present-{1440,390}.png show the unfinished result. Blackout still leaves a white screen with text and controls: a pre-existing symptom this stage conversion has not addressed. Fix: give the projector a viewport-bounded black stage, fit/centre the slide within both dimensions, remove the page gutter/padding for this surface, and place accessible controls in an overlay. Preserve audience phone reflow deliberately. Add geometry/blackout browser assertions; the new check in TestPresentCreatesSessionWithCodeAndRemoteUrl checks a wrapper attribute and absence of chrome, which cannot prove full-screen layout.

1. P2 / blocking — “Use a different address” exits local recovery when Home is configured. internal/slides/auth.go:597–607. The new sent page starts its retry link at /signin and reads Slides' standalone return cookie. In Home mode that is not connect's navigation cookie. Someone deliberately opening /signin/local, submitting an address, then choosing retry is sent to 303 /auth/start?return_to=%2F, rather than back to the local address form. This also replaces any connect-held destination with /. A scratch regression probe using the existing Home fixture reproduced that exact redirect and failed. Standalone mode works, including retaining the deck query through retry. Fix: in Home mode return to /signin/local or an explicitly forced local form without resetting connect's remembered navigation. Add a Home-configured test covering sent → retry → second submission → original destination.

1. P3 / non-blocking spec deviation — the standalone return cookie is not sealed. internal/slides/auth.go:717–722,737–746. It contains only a URL-escaped path; the ten-minute lifetime relies on browser Max-Age, with no authenticated expiry checked on read. Spec §9.4 explicitly requires a short-lived sealed cookie. The repeated handoff.SafePath checks are good: I found no open redirect. Fix: use the established sealed navigation-cookie pattern with checked issuance/expiry, or obtain and record an explicit spec amendment.

1. P3 / non-blocking verification gap — several binding adoption gates are absent. internal/slidestest/index_test.go:67, internal/slidestest/vendored_test.go:36. There is no committed table for {unknown route, missing object, bad parameter, panic} × {signed in, signed out}, no classification gate for remaining http.Error sites, no callout grep gate, and no estilo-token assertion against Slides' v0.26 tokens (spec §§3.3, 4, 12.1). The changed 500 test calls the public renderer directly, not router recovery or optional identity. Fix: add the specified gates and demonstrate red/green. A manual check found no undefined token references in the three estilo stylesheets today.

Validation on the exact target:

• GOTOOLCHAIN=go1.26.5 make ci: PASS, including Go tests, vet, format, migrations and credits.
• GOTOOLCHAIN=go1.26.5 RASTRILLO_CHROME=<~/Library/Caches/ms-playwright/chromium-1243/.../Google Chrome for Testing> go test -count=1 -tags browser ./...: PASS. Browser-containing package: 195.017s. This includes TestBrowserPresentingScreensHoldStill, TestBrowserPhoneStacksFlowRegions, modal isolation, strip geometry, reload presence and dropped-batch save status.
• JavaScript suite: 158 passed, zero skipped. Bundle rebuild reproduced committed editor.0463180baf5aab9f.js unchanged.
• Shared r2-chrome-check.mjs with r2-slides-check.config.mjs: all checks passed at 1440/840/800/390/320. All three CLS flows: 0.0000; bar top/height: 0/52 throughout.
• Mutation proof: disabling stored-ID reuse in scratch fixtures/client-id.mjs made the new “a reload reuses the id this tab stored for the deck” test fail (...0002 versus ...0001, exit 1). Restoring it made the test pass.
• Extra browser probes: standalone deep-link sign-in returned to the exact deck plus query after retry; signed-in /signin respected a safe destination. Absolute, scheme-relative, encoded slash/backslash and auth-loop destinations fell back to /. Unknown routes split HTML versus JSON/wildcard Accept into HTML versus plain 404. Editor command dropdowns stayed onscreen and hit-testable at all five widths.
• Scratch admission tests: removed standalone, Home-local and Home users visiting /signin had sessions revoked, redirected once to ?err=removed, then received 200. No revoked-user loop.
• Hub CI reports PASS for this exact head. I did not rerun the Rust/wasm portion of .amadan/ci.d/10-gate; the Rust crates and embedded wasm are unchanged.

Presence/security: the stable ID is 64 random bits in per-tab sessionStorage, keyed by deck. Reload retained it. A real window.open probe, which copies session storage, exercised the BroadcastChannel claim: the new tab changed ID and the original retained its ID. It is not an authentication credential. The server derives name/colour from the admitted user. There is an existing limitation in live.go:667 / live/hub.go:201: the hub keys entries only by client ID, so a malicious admitted collaborator can reuse an observed ID to overwrite/suppress its entry, with the attacker's server-stamped identity. They cannot thereby claim the victim's server-stamped name or gain document authority. This predates the branch; account-scoping the hub key would be useful hardening.

Other requested checks: presenter/remote/pair/projector pages correctly have no account bar; R1 reload/reflow fixes survive. Strip avatars and kebabs no longer cover labels. Estilo savestatus replaces the app copy and retains sticky dropped-batch failure. home/connect is pinned to 01166fcfc0a9; the manual config reads HOME_DIRECTORY_ORIGIN, validated by DecodeCredentials. Home sessions suppress the redundant account-connection item. Home signout/CSP/referrer wiring is consistent with the new pin; I did not run a live issuer's cross-origin signout chain in three engines.

Visual assessment: inspected representative supplied before/after pairs and the after list, editor, account, sign-in/error, viewer, presenter and signed-out screenshots at desktop/phone sizes. Shared chrome and strip look materially more finished. The projector still does not look or behave like a finished presentation surface.

Review was isolated under /tmp. No existing checkout or branch was edited, committed, pushed, merged or moved. Port 8391 was occupied, so the run-dev recipe and shared config used an independent 8392 instance. The review server, build/data directory and scratch worktree are removed; Data-volume free space is 8.5 GiB. Small review logs/probes/screenshots remain in /tmp/slides-astra-results.

1. Projector: b19fbda bounds the black stage to the viewport and overlays controls; browser coverage passes at 1920×1080 and 1280×720.
2. Different address: b19fbda returns Home recovery to /signin/local with validated return_to; the sent → retry → second submission regression passes.
3. Return cookie: b19fbda seals, authenticates, origin-binds, and expires the cookie; tampered and expired intents are ignored.
4. Gates: b19fbda adds navigable HTML-error, callout-vocabulary, and estilo-token regression gates, with fault-injection red/green proof.

CI is green; focused browser projector and presenter-reload coverage is green.

Merging on Paul's instruction ("roll in any fixes, then proceed with deployment"). Astra's four findings are fixed at b19fbda. The coordinating session read the delta, including the sealed return cookie (seal.Sealer with a per-origin purpose, SafePath, a 10-minute expiry, a single cookie, strict base64). CI passed.