oficina / sheets Public

CI passed at 6fc76ab: vet (with -tags browser), fmt, build, migrations (go tool rastrillo), go-test and race. Not merged; waiting for review.

Reviewer: Astra (GPT-6-Astra via Codex).

VERDICT: APPROVE-WITH-NITS

Reviewed the whole branch at 6fc76ab02da1c45d827543250d4f219317b5e746 against freshly fetched origin/main / merge base 2335fe1d8f4fde09c0bcfa9d19c209c322d042b7, following AGENTS.md and REVIEW-BRIEF.md. No blocking correctness, security, migration or data-loss finding.

1. P3 — internal/sheets/native.go:27: put the two-package native-session bridge in the ordinary regression gate. The bridge works at these pins, including native mint → canonical reader and expiry rejection, which I exercised. However, the existing approval/exchange helper is reached only by TestNativeReview, which skips unless a native binary/simulator is configured (native_review_test.go:27). Ordinary native API tests do not cover that handshake. A future canonical sessions upgrade could therefore break interoperability while make test stays green. Suggested fix: add a plain Go test using the existing approval/exchange helper, then verify canonical authentication, expiration and revocation of the resulting token. No native client is needed.

1. P3 — internal/sheets/static/sheets.css:1290: add an overflow assertion for the Format fix. Removing the new coarse-pointer label-width block in the scratch tree still leaves make test-browser green. The current test measures text size, not menu containment. The fix itself works: I measured both Set buttons inside the panel at 320px and 390px with touch enabled. Suggested fix: open Format at those widths with a coarse pointer and assert both buttons' bounds and hit targets remain within the panel; removing this block should fail it.

Verification and behavior review:

• make test passed, including browser-tag vet, the pinned CLI migration check, all Go tests and race tests. make test-browser passed using Playwright Chromium 1148 through RASTRILLO_CHROME. Remote CI also reports passed at this exact SHA.
• The unchanged TestNoZoomOnTap was copied into a separate main worktree, with only its build dependencies supplied. It failed on main's sign-in, title, size controls, formula bar, sharing, comments, table controls and real cell editor (12.5–14px). This is a meaningful computed-style regression test with an asserted coarse pointer; it is not a direct Mobile Safari viewport-zoom measurement.
• Shared chrome command with the supplied r2-sheets-check.config.mjs: 989 ok, 0 FAIL.
• make release succeeded for its default Linux/arm64 target. A host-runnable build using the same target with RELEASE_GOOS=darwin RELEASE_GOARCH=arm64 returned v0.1.0-36-g6fc76ab from /api/version.
• Built clean main using its original go.mod (v0.23 application imports), booted it, and seeded a sheet, encrypted cells, a session, grant and public link. Stopped it and booted the release build over the same DB/key. Migration succeeded; schema and all 16 ledger entries were unchanged; the old cookie authenticated and old cell content decrypted. This upgrade needs no new schema migration.
• Compared complete module trees, normalizing only the old module path. Sessions, CSRF, db, migrate, jobs and blobs have no other production-code differences. Auth's substantive change is the optional SubjectFor hook after address admission and before session creation/second factor; nil retains the verified-address subject, as Sheets requires. Middleware, authorization and existing migration SQL do not change.
• Mail is more than a Date-header change: SMTP now owns a context-aware connection, cancels blocking work when the context ends, and adds a 30-second fallback only when the caller has no deadline (not an unconditional 30-second ceiling). STARTTLS remains opportunistic, and PlainAuth retains its cleartext restrictions. One edge case becomes stricter: when credentials are configured but a server falls back to HELO without AUTH, the new sender refuses, whereas the former net/smtp.SendMail path could skip AUTH. That is fail-closed, not a security weakening. List-mail/Deliverer APIs are additive and unused by Sheets. Upstream mail, auth, sessions and CSRF tests all passed.
• Both sessions packages use __Host-rastrillo_session on HTTPS, rastrillo_session on HTTP, Path=/, HttpOnly, SameSite=Lax and HTTPS Secure. Tokens are 32 random bytes in unpadded base64url (43 characters); the DB stores their hex SHA-256. Both default to 30 days and use identical UTC RFC3339 storage, expiry checks and row revocation. Native exchange performs fresh cookie/DB lookup and delegates admission to the app; it does not rely on the other package's distinct context key. I exercised approval using the pre-upgrade browser cookie, old-package Mint, new-package authentication, and expiry rejection.
• Doctor re-added all four scripts in the scratch tree. TestBusyRuleIsLinkedAndServed then failed for each one, as intended. Independently, sharedStaticFiles overwrites the copied shim with busy.ShimJS(), and the linked/fingerprinted response is checked against Estilo's bytes. Running doctor alone cannot substitute the combined shim at that URL. No live template links the unused picker scripts.
• Reviewed the CSS/module partial changes and the supplied screenshots, including pixel comparison of 52 mouse before/after pairs. Grid, table, Format and most menu screenshots are identical; differences are consistent with dynamic sky/spinner/timestamp content and timestamp-driven table sizing. Touch screenshots and live measurements show the larger controls and contained Set buttons. Narrow mouse windows still have the pre-existing Format overflow; the coarse-only fix does not change mouse CSS. Other upstream changes include lg form/confirmation buttons, optional primary fields/detail-list time markup, native meters/progress, date/calendar assets, and localized money errors. Sheets does not invoke the affected form/meter/date partials or money parser; its job-status map supplies no Percent. New assertion/PoW functionality does not add a Sheets route. This pin does not bring the later CSP/time-storage migration.
• The drive-script waits now wait for the actual sign-in result URL before reading the log, covering the busy delay. I did not rerun the unrelated full drive suite.

For the other apps: the fetched #44 currently contains the report and CI reply, but no explicit recipe at its end. The general import/tool/stamp/doctor/test sequence is sound; copying Sheets' exact cleanup or pin everywhere is not. In the inspected checkouts:

• Docs: already requires d9c28d708556, and team_session_reader.go uses its FromContext API. Do not downgrade it to 29e4491. Audit existing canonical/legacy adapters, preserve the native-server bridge where needed, and change the stamp in scripts/build-release.sh.
• Calendar: its layout actually links select.js/datetime.js and its event form uses the enhanced date controls. Preserve/update the used assets and include calendar.js before datetime.js if adopting the new calendar overlay. Do not copy Sheets' blanket picker deletion.
• Memoria: also uses the old-path native server, so it needs the same deliberate session bridge. It already links a separate busy.js: ensure the served shim is the split one when doctor updates assets.
• Home: has its own sign-in/session wiring rather than Sheets' auth setup. Preserve its subject and freshness semantics while migrating all readers/writers consistently. Verify existing sessions and its actual release stamp.

For each app, resolve the effective module version, pin the CLI to that version, audit served assets and specific CSS overrides, and repeat the old-DB, release-stamp, touch and main-negative checks. Keep the later CSP/time-format upgrade separate.

Runtime checks here were isolated local fixtures as requested; no deployed canary was supplied or deployed, so this is not a canary deployment sign-off. No existing checkout was moved or edited; no commit, push or merge was made. Scratch worktrees and fixture processes were removed after testing. Disk was measured with df -h /System/Volumes/Data (49 GiB available before, approximately 48 GiB after).

Posting note: the initial agent post was refused because this Codex environment did not supply Amadan session metadata. I followed amadan agent skill, supplying AMADAN_AGENT_SESSION from this session's CODEX_THREAD_ID and AMADAN_AGENT_HARNESS=codex; the account remains the configured agent account, with no -as-human use.

Both of Astra's nits are fixed. Head is 477ea96, and CI passed on all six steps. Not merged.

1. native.go (095b010). nativeexchangetest.go runs the native sign-in through the router, across both sessions packages, in three tests:

• Exchange: - Before approval, the exchange answers 202, and a wrong verifier gets 404. - The minted cookie has the name the app reads, and it is a new token, not the browser's. - The app accepts it as the same person: the browser's sheet appears in its list. - The id can't be exchanged a second time.
• Expiry: - An expired browser session can't approve, and an approval that expires before the exchange gets 401. - A minted session expires like any other.
• Revocation: - If the browser signs out between approval and exchange, the exchange gets 401. - The app's /signout ends a minted session and leaves the browser's session alone.

Each mutation of native.go fails the tests; the output is in polish-audit/r3/scripts/migrate-sheets-native-red.txt:

• an http origin, so the cookie gets a different name
• sessions that are already expired when minted
• a minted row that resolves to someone else

2. sheets.css (477ea96). TestFormatMenuFitsOnTap opens Format on a touch screen at 320 and 390 wide. It checks that each Set button sits inside its panel and that a tap at the button's centre lands on it.

• Without 48c1052's label rule it fails at both widths: Set spans 225–263 in a panel that ends at 245.
• With the rule, it passes.
• The output is in polish-audit/r3/scripts/migrate-sheets-format-red.txt.

The title field belongs upstream in estilo's chrome.css. I opened that as oficina/estilo#22, branch fix/title-touch-size at bb2db36. Its test failed before the fix, and its CI passed. I proved it in a browser too: with Sheets' own title rule removed, Sheets' TestNoZoomOnTap fails on estilo's current pin and passes with the estilo fix. Once that merges and Sheets bumps estilo, the [estilo-bar-title-input] line in sheets.css can go.

Merging. Astra approved with two nits, both fixed at 477ea96 and each shown red by mutation. CI passed. The estilo title-field fix landed separately (estilo #22).