oficina / sheets Public

Correction to commit 8fa462a's message

8fa462a bumps rastrillo v0.20.0 → v0.21.0 and its message claims v0.21.0 carried "the migrate/dump plumbing this task depends on". That is wrong, and the branch is pushed, so the message cannot be amended without a force-push — recording the correction here instead, where a reader reaches it.

A reviewer diffed both module versions directly. migrate/apply.go, diff.go, set.go, schema.go, adopt.go, migrate/dump/* and cmd/rastrillo/migration.go are byte-identical between v0.20.0 and v0.21.0. The migration tooling already existed at the version we were on; the bump was not needed for it.

What v0.21.0 actually adds: the rastrillo doctor subcommand, asset-vendoring refactors in cmd/rastrillo/new.go, and the internal/designsystem/ui/iconsets packages. None are reachable from this app's runtime path — serve.go, run.go, manifest.go and assets.go are unchanged between the two versions.

The bump is still justified, for a different reason than the one given: it keeps the linked library in sync with the v0.21.0 CLI that generates and checks the migrations. Generating migrations with a CLI that differs from the linked library is a mismatch waiting to happen.

The conclusion in the commit message was right. The premise was asserted rather than verified, and turned out false — which is the failure mode this branch has spent two days cataloguing, arriving this time in a commit message rather than a comment or a test.

Overnight follow-up complete — both merge blockers closed

44 commits now. Everything below was added after the whole-branch review, each piece implemented by a fresh agent and separately reviewed.

CI (blocker 1). Six .amadan/ci.d/ steps, each execing a Makefile target so debugging CI is running it. Five prose rules became guards in the test binary; each was deliberately broken and watched fail. The review then found the session's own recurring pattern inside the guard mechanism: calc isolation used exact-directory equality, so a future calc/functions subpackage would have been silently unchecked rather than exempted. Fixed with prefix matching and proved live — a throwaway calc/scratch package importing store is caught now and was invisible before.

Migrations (blocker 2). One schema definition shared by tests and production; AutoMigrate is gone. rastrillo migration check gates CI. Correction already posted above regarding 8fa462a's commit message.

The verification ceiling. LibreOffice cannot be installed here, so the corpus stays hand-authored. Instead every expression is cross-checked against excelize's independent engine: 59 agree, 1 divergence, 0 unavailable — and the divergence is excelize's TRIM bug, confirmed by reading its source. Disagreements live in a list someone must edit; a new one fails the build.

Performance. Open recovery at 199,999 cells: 32m2s → 8.818s. Lock hold on a blank: 6.728s → 2.779s. Real measurements. The store test suite also dropped from ~35s to ~6.8s as a side effect.

What remains genuinely unverified

Stated plainly, because it is the honest limit of what was achieved:

• tools/gencorpus has never run against a real LibreOffice.
• Nothing has opened our XLSX output in real Excel or LibreOffice.
• The corpus verifies self-consistency plus agreement with one independent engine — not conformance to a reference implementation.

These are named follow-ups, not silent gaps, and each needs a machine with soffice on it.

The performance claim is now reproducible

The batching review flagged that the headline timings — Open recovery 32m02s → 8.818s at N=199,999 — were measured with a throwaway benchmark, deleted after use. Its words: "trust-based, not independently reproducible from what's in the repo."

That is a published number resting on one instrument, run once, by its author. Exactly the class of thing this branch spent two days finding in everything else.

internal/sheets/store/handle_blank_recovery_bench_test.go now commits it. Gated behind RUN_BLANK_RECOVERY_BENCH=1 plus a testing.Short() skip so it stays out of CI, with N configurable via BLANK_RECOVERY_BENCH_N — cheap to run small, honest to run large.

Re-run on the committed code, it corroborates the recorded figures rather than replacing them:

| N=199,999 | recorded | re-measured | |---|---:|---:| | setBlank lock hold | 2.779s | 2.874s (+3.4%) | | Open recovery | 8.818s | 8.881s (+0.7%) |

The instruction was explicit that a disagreement should be reported, not silently used to update the published number — a figure quietly overwritten by its own re-run is unfalsifiable. There was no disagreement worth flagging, but the rule mattered more than the outcome.

Credit where it is due: this exists because the rastrillo design-system session retracted a "zero differences" result of its own after finding its measuring drive had no control — a reviewer had already reproduced the number exactly, using the same instrument. Their corollary is the one worth keeping: when a review's job is to confirm a measurement, build a second instrument rather than running the first one again. Running it again feels like verification and is closer to a retweet.

Only the pre-fix 32m02s figure remains unreproducible, and deliberately so — reproducing it would mean reverting the fix.

Plan 2 — the grid surface and live sync

Thirteen tasks, one session, b54989d. The branch now serves a spreadsheet in a browser that live-updates across tabs, deployed and verified on the canary.

Plan: docs/superpowers/plans/2026-08-31-sheets-grid-live.md.

What landed

The DEK (nothing built a seal.Sealer before, so no cell could be stored); members, mapping an address to the uint64 every authored column wants; sheet and tab creation with sealed titles; store.SetReport, which is Set plus an account of what changed; internal/sheets/live, an in-process fan-out hub; a view layer over rastrillo's tokens; the grid JSON API; SSE; the vendored grid; the scriptless table view; and spec §11.1's contrast trigger.

Three bugs worth naming, because a green suite found none of them

1. Jspreadsheet v5 takes its event handlers on the root config, not on a worksheet. Nested one level too deep they are accepted in silence and never called. The grid rendered, the console was clean, every Go test passed, and not one edit was ever sent to the server. Two real browser tabs found it in about a minute. tools/drive/drive.mjs is committed as a manual gate for exactly this class — no static check knows which options a third-party library reads.

2. The JS line-cap guard walked past everything its exemption list named. It reused repo_test.go's skipDir, which skips any directory called vendor — correct for a Go dependency tree, exactly wrong for our vendored JavaScript. It passed while proving nothing, and the exemptions read as load-bearing while being dead. TestEveryJSExemptionIsLive now asserts the walk reaches each one. This is the "suite that agrees with itself" failure the spec's §12 warns about, arriving in the guard rather than in the code.

3. rst-sr-only written as an attribute. One of AGENTS.md's seven class-only utilities, so it matched nothing and a screen-reader label rendered in the middle of the toolbar. Caught by a screenshot, now caught by TestUtilitiesAreNeverWrittenAsAttributes, which I verified fails on the exact markup that caused it.

A structural change that needs a second opinion

The models moved from package sheets to package store. store imported sheets for those structs, so sheets could not import store back — and every route that reads a cell is in package sheets. Putting them where the persistence is removes the cycle rather than routing around it, and it is where models.go's own comment on Event already said they belonged. Member stayed behind: who may hold a session is an app concern.

Three store tests moved to the external test package as a consequence (they migrate with sheets.BootSchema); export_test.go carries the seams they need, deliberately as seams, because what those tests prove is that the real crash-safety path works.

rastrillo migration check still agrees, which was the risk.

Two deferred minors from plan 1, both handled

• Sheet.DeletedAt is *time.Time, so soft-delete scoping is not automatic — listSheets excludes deleted rows explicitly, as this thread said the first task listing sheets would have to.
• calc.CellKey's position+1 convention is valid until rows move. Plan 2 adds no insert, delete or reorder, so it is untouched and still owed by whoever adds one.

Deliberately not built, with triggers

Stated in the plan rather than discovered later. Sharing is not in this (the instance allowlist is the team, and §13's ordering of sharing before live sync is inverted — reason written down). Presence: trigger is the first time two people report colliding on a cell. Undo: trigger is the grid being in daily use. The event log already records what undo needs.

Spec §7's idle policy is implemented differently from its wording, and the code says so: there is no presence heartbeat, because a heartbeat POST cannot reach the goroutine holding the stream. The client closes its stream when the tab is hidden instead, which is stronger — a hidden tab holds no socket rather than an idle one. The server cannot enforce that (EventSource retries after any close), so its timeouts are a backstop, and the comment says exactly that.

Verification

make test green — all six targets, including -race over store and live. rastrillo migration check agrees. rastrillo doctor reports tokens.css, theme.css and rastrillo.js all matching v0.22.0.

Browser-driven on a real instance: two tabs on one sheet, type 6 and =A1*7 in the first, and the second shows 6 and 42 without a reload. Its formula bar shows =A1*7 for the cell displaying 42. No console errors.

Canary sheets.bds.oncarlos.com runs b54989d, confirmed by polling GET /api/version — the process, not the edge. It took ~2 minutes to cycle, consistent with the >90s this repo has recorded before.

SHEETS_DEK was added to the app's secrets. The new binary refuses to boot without it. No sheet data existed before tonight, so nothing could be orphaned by minting a fresh key.

Review status — NOT satisfied

Per §Review, a review by the model that wrote the code is not a review. I am the author. I have not dispatched a reviewer, and I am not going to pass a self-read off as a review pass.

So this branch is not mergeable yet. What it needs is a review pass by a model at least as capable as the author, recorded here. The structural change (models moving packages) and handle_set.go's refactor into SetReport are where I would point a reviewer first: the second one touches the crash-safety path that most of plan 1's review effort went into.

The last mile I could not walk: signing in on the canary needs a magic link emailed to paul@keymail.dev, which I cannot read.

Follow-up: dark mode was broken, and it hid the row and column names

Paul screenshotted the canary in dark mode: dark chrome, a white grid, and column letters and row numbers that were all but invisible. Three bugs, 1dd05db.

1. data-theme carried the wrong axis. theme.css declares every colour as light-dark(light, dark) under color-scheme: light dark, and its entire toggle is :root[data-theme="light"] and :root[data-theme="dark"] — setting color-scheme re-resolves every one of those calls. We were writing the theme name (day, plain, signal), which are the palette files an app picks once at scaffold and rastrillo doctor copies. Neither rule matched, so an explicit choice never beat the OS and the Day/Plain/Signal buttons in the account menu did nothing whatsoever. Theme and scheme are orthogonal: this app ships day (ruling 7) and each person picks light or dark within it.

The cookie now carries light, dark or auto, and auto writes no attribute at all — that absence is how "follow the OS" is spelled.

page_test.go asserted data-theme="day" and passed. A test can only pin a mistake it does not share. The scheme is now pinned against the toggle selectors read out of theme.css itself, so the day rastrillo moves them, the test fails instead of the page.

2. The grid CSS targeted .jexcel. Jspreadsheet v5 renamed everything to .jss_*. The entire override block matched nothing, so the grid kept its hard-coded #fff inside dark chrome. Same shape as the event-handler bug earlier on this branch: a v4 assumption applied to v5, accepted in silence.

3. Five invented token names. --rst-border, --rst-text-mut, --rst-warning, --rst-font-mono and --rst-shell-bar-h do not exist. The real ones are --rst-line, --rst-text-muted, --rst-tone-warning-fg and --rst-font; the fifth has no equivalent at all, and the layout was subtracting a height the framework never published. CSS resolves an unknown custom property to its fallback in silence — no console warning, no parse error — so the page rendered plausibly and simply was not themed.

internal/guardstest/csstokens_test.go now checks every var(--rst-*) we write against the two files doctor copies. Verified to fail on the exact name that shipped.

Why the names disappeared

Falls out of 2 and 3. The library sets background-color: #f3f3f3 on both headers and no color, so the text came from the page — light grey on light grey. Every rule in our stylesheet that sets a background now sets a foreground beside it, which is the actual invariant.

Measured in a real browser across all four cases — OS dark, OS light, dark picked on a light OS, light picked on a dark OS. Headers 8.05:1 and 7.05:1, cells 15.3:1 and 16.9:1. All four now do what they say.

One thing the fix broke, caught before shipping

Dropping the invented --rst-shell-bar-h left height: 100dvh on the surface inside a shell that also holds the topbar: a 771px document in a 720px viewport, the whole page scrolling by exactly the bar. Fixed with layout rather than another guessed number — the shell becomes a viewport-height flex column, scoped with :has() so only a sheet page gets it, and the surface takes the remainder. Measured back to 720 in 720.

Standing back

Three of the five defects on this branch are one failure: a plausible name, written without checking, that fails silently. rst-sr-only as an attribute, .jexcel for .jss_worksheet, five --rst-* tokens. Each renders something that looks approximately right, none produce an error anywhere, and every one needed a human looking at a screen.

Three are now mechanical guards. The fourth kind — which options a third-party library actually reads — cannot be, which is what tools/drive/drive.mjs is for.

Canary runs 1dd05db, confirmed on the process. Review status is unchanged: still needs a pass by a model that did not write this.

§Review pass: dispatched to Fable, twice. Both returned REQUEST CHANGES.

This closes the "still needs a pass by a model that did not write this" note at the end of the previous entry. Two separate dispatches, because they are two different questions:

1. b54989d..core — grants, the share screen, link sharing, version history/undo/restore. The security-bearing work nobody outside the session that wrote it had read.
2. core..design-fixes — the design fixes on top, reviewed on the same terms.

Model recorded per §Review: Fable, on both. Author was Opus 5; the reviewer did not write either diff.

---

BLOCKER, and it was already on the canary

A sheet could be left with no owner in one form submission.

wouldOrphan guarded revoke and nothing else. But grant upserts on (sheet_id, member_id) — re-sharing with an address that already holds access changes their role — and counted nobody. So the sole owner typing their own address into the share form and picking viewer demoted themselves. Zero owner grants remain, mayShare is false for everybody including them, and, in the words of revoke's own comment, there is no admin path to repair it.

TestTheLastOwnerCannotBeRemoved asserted this state was impossible while testing only one of the two doors.

Reproduced with a test before fixing (grants_orphan_test.go). Both doors refuse it now.

Second finding, same class

The orphan check was check-then-act with no transaction. Two owners revoking each other concurrently both counted two owners, both passed, and both deleted. SQLite's single-writer pool serialises the two DELETEs; it does not serialise a read against someone else's later write. The check now runs inside the transaction that does the write. lastOwnerGoing went to its own file — grants.go crossed the 300-line cap, which the guard caught.

Also fixed on the design-fixes branch

• The share <select> defaulted to owner. shareableRoles was ordered most-able-first and rendered with nothing marked selected, so the browser picked the first. The fastest path through the screen — type an address, press Share, never open the dropdown — handed the sheet away. Now viewer → commenter → editor → owner, and the ordering is held by two tests, because a reordering that reads like tidying would otherwise restore it with everything still green.

• rst-help came back. Fixed on sign-in, it reappeared within days on the share and history screens — a different session reaching for the obvious name for help text. It is a 28px square badge; given a sentence it keeps the box and spills the text out of it. Both fixed, and now guarded. This is the same failure the previous entry described: a plausible name, written without checking, that fails silently. Fourth instance.

• The table view told viewers a lie. handleTable admits anyone who may read; handleTableWrite 404s a viewer or commenter. So the subtitle said "every cell here is editable" to people the server would refuse, above a per-cell link that opened a form that 404d on submit. Gated on MayWrite, expressed as a coordinate no cell can equal — the same shape the link view already used.

• No sign-out below 800px. rastrillo hides [rst-shell-tail] and reveals it only from an [rst-shell-menu][open] that immediately precedes it. We shipped no menu, so navigation and the account dropdown were absent on every phone — not collapsed, absent. Added, with a guard on its position, since the framework's rule is a + combinator.

• The scriptless table was ~650 blue underlined links a page, data and emptiness identical in colour and weight. Cells read as cells now, numbers right-align off the computed kind, and the anonymous link view gets the same treatment through the shared tableRows.

Still open — on core, not fixed here

Recorded rather than actioned, so they are not lost:

1. The plaintext link token transits a GET URL. handleLinkCreate redirects to …/share?notice=link&token=<plaintext>. Verified clean otherwise: never persisted (only the SHA-256), never in an error string, never in an app log line. But a query string in a request line also reaches the platform edge's access logs, and this file's own rule is that platform-held logs are storage we cannot grep. Fix is to render the share page from the POST, or stash the token server-side for one fetch.
2. safeReturn misses /\evil.com. Browsers normalise \ to / and read it as scheme-relative; Go's url.Parse leaves Scheme and Host empty. Very low exploitability — POST-only, behind the origin-checking middleware, and return is server-populated — but the fix is one character class.
3. Every createLink failure reads as "bad-link-role", including a DB failure.
4. Undo says "nothing to undo" if the newest own event fails to unseal — Limit: 1 plus a skip returns empty while older undoable events exist.
5. Revoking a team member's explicit grant says "Access removed" while they keep editor through team standing. The fallback is deliberate; the notice is misleading.

What Fable checked and found clean

Worth recording, because coverage is the part a verdict hides: the sealing discipline on the history path (inputs sealed, derived values never logged, coordinate narrowing done post-unseal so no plaintext shadow column exists), the link token handling (32 bytes from crypto/rand, stored as a hash with a unique index, constant-time compare, mint refuses editor and owner), the 404-not-403 contract across every new route including history, no scope.Owned anywhere, no model binding, tx correctly scoped in every existing transaction, migrations 0004 and 0005 additive-only, no address or cell content in any new log line or error string, and the own-edits-only undo filter being authoritative server-side so a forged form cannot undo as somebody else.

The two verdicts were REQUEST CHANGES on the blocker and on the branch findings above. Every one of them is now closed except the five listed as still open, which are on core and belong to whoever picks them up.

Plan 3 — sharing, and the rest of the maximally viable slice

01e8adf, deployed and verified on the process. Seven pieces, each committed, gated and deployed on its own.

Merged design-catchup first. Two branches were both editing the design system and divergence on tokens.css compounds hourly. rastrillo v0.23.0, PRODUCT.md and the first design review are now on core; doctor (upgraded to v0.23.0, as that branch's own AGENTS.md change now requires) reports all three vendored files matching, and the new CSS token guard passes against the new tokens — which is exactly the check that bump needed.

Sharing (spec §8)

Grants and roles, with access through the hand-written join AGENTS.md required and not scope.Owned. A guest sees exactly what they were granted; a colleague reaches every sheet, because one instance per team is what that means. Every refusal is a 404 — a viewer asking to write and a stranger asking to read both get what a nonexistent sheet gives, because the set of ids that 403 is a map of the team's sheets.

The two-user isolation suite found two bugs that would have made sharing quietly useless:

1. A shared-with guest could not sign in. auth's Authorize was the static SHEETS_OWNERS list, so grant created a Grant row and a guest Member for somebody auth would then refuse — a share nobody could ever exercise. App.Admits now answers "on the allowlist, or holds a grant", fail-closed.
2. The two admission gates disagreed. handleBegin used the static list while redemption used the new gate, so a guest would have been admitted at redemption and never sent a link to redeem.

Share screen: invite, set a role, revoke, hand ownership on. The last owner cannot be removed — a sheet with no owner can never be shared again and there is no admin path to repair it. §9.5's ruling is honoured in words on the screen, with a test asserting the disclosure is there.

Link sharing: only the hash is stored. Plaintext, the replicated database becomes a bundle of live read capabilities to sealed sheets — rule zero through a side door for the second time. SHA-256 and not bcrypt, and the difference is the point: 256 bits of entropy means guessing is not a threat model. Never editor (ruling 12).

The rest

Version history and undo. The log holds inputs, never derived values, so an edit that recalculated four hundred cells appears once. Restore is a fresh last-write-wins write, not a rewind — it shows up as an edit and is itself undoable. Undo is own-edits-only; Ctrl-Z posts to the same endpoint the scriptless button does, so there is one undo and not two that can disagree.

Comments, on §13's pre-decided fallback: Docs' comment-thread primitive has not shipped, so this is the cell-anchored local implementation, anchored on stable ids — one of the three things §4.2 says stable identity is load-bearing for. Body sealed. This is also what makes SheetCommenter mean anything; until now it granted nothing beyond reading.

Sort and filter — and here is the finding worth reading.

A constraint that shaped a feature, not a shortcut taken

calc's dependency graph is keyed on position: store/handle.go's cellKey builds CellKey{rowPos+1, colPos+1}. So physically reordering rows changes what every existing formula points at, silently. Plan 1's review flagged this on this very thread — "valid only until rows move" — and sorting is the first feature to meet it.

So sort and filter are non-destructive: they reorder what is displayed and never what is stored. A test writes =A1*2, sorts, and asserts it still reads 60. Destructive sort now has a row in §14 with its real trigger — calc's key space rebuilt on stable ids — because that is a subsystem, and a sort that quietly corrupted formulas would have been the worst thing on this branch.

Criteria are sealed: a filter value IS a cell value (§2.1).

Two guards earned their place tonight

The 300-line cap fired twice on my own work — a test file at 381 lines and handlers_table.go at 323 — and both splits improved the code rather than annoying me. The line I keep proving to myself: I wrote a filter fixture that asserted "contains a" excluded beta, which contains an a. A test that cannot fail.

Still true, and still blocking

Review. I wrote all of this and a self-read is not a review pass. The branch is not mergeable. The places I would point a reviewer, in order: grants.go's roleFor and visibleSheets (the whole authorization model), App.Admits (two gates that must agree), and sharelink.go (a capability with no session behind it).

Not built, with triggers: presence, destructive sort, conditional formatting (ruling 17's second slip candidate), and charts (its first). The home's five-call contract is untouched — the home does not exist, and §9.1.1 says an instance without one works fine, so the directory outbox waits for a spec to build against rather than being invented.

Merged and deployed — canary runs c4a3954

design-fixes fast-forwarded onto core and shipped. Verified on the process, not the edge: GET /api/version returns c4a3954.

The promote-is-not-a-deploy gap measured 100 seconds on this one. The edge flipped to c4a3954 immediately while the process kept answering 01e8adf for ten consecutive polls. A single early read would have looked like a failed deploy; a single late one would have proved nothing about which binary answered. This file's rule about polling rather than sampling held exactly as written.

Canary checked signed-out on the deployed instance in a real browser, desktop and 390px, both schemes: the sign-in help renders as prose, not inside a 28px badge. That was the screen the whole thing started on.

The guard caught a fifth instance while this was landing

TestBadgeAttributesNeverWrapProse was written for the share and history screens. Rebasing onto 01e8adf it failed immediately on two more — the sort/filter notice on the table view, and the comments screen — both written after the sign-in fix. Five instances of one mistake in a few days, four of them after the first was fixed.

That is the previous entry's own conclusion arriving a fourth time: a plausible name, written without checking, that fails silently. The difference is that this one is now mechanical, so the sixth instance fails a test instead of reaching a screen.

Two things for a human, not for a session

1. There is no main branch on this remote. Only core, design, and the short-lived branches. Every release in carlos releases was built from a core commit. But §Shipping cadence says "squash-merge to main" and "shared hosts only ever run merged main" — so the file describes a workflow this repo has never used. One of the two is wrong and it is not a session's call which. I have not created main and have not edited the rule.

1. Five findings from the Fable pass on core are still open and are listed in the previous reply — the plaintext link token in a GET URL is the one worth doing first, because the exposure is the platform's access logs, which this file's own rule calls storage we cannot grep.

The five open Fable findings are closed, deployed and verified

9b723a5. Each was filed as its own issue (#3–#7), fixed on its own branch, deployed to edge on its own, and verified by polling GET /api/version on the process. All five branches are pushed.

| Issue | Commit | What changed | |---|---|---| | #3 token in a GET URL | 3b8870a | The token reaches the person in the response to their own POST; renderShare stopped reading the query at all | | #4 safeReturn backslash | 9920d91 | Both scheme-relative spellings checked on raw bytes, ahead of the parse | | #6 undo vs an unsealable event | 1ae7a1c | A bounded twenty-event window, taking the newest that unseals | | #5 link mint blamed the role | 1c7f43c | Sentinel error; every other failure takes the same 500 as the rest of the screen | | #7 "Access removed" untrue | 9b723a5 | Paul's sentence, naming the person, id in the query and never the address |

TDD throughout: test first, watched fail for the stated reason, then fixed. make test green including -race before each commit.

Two of my own issue reports were wrong, and the tests said so

#4 was not a live open redirect. u.RequestURI() percent-encodes the backslash, so the header was always a same-origin path. What was actually wrong is that the encoder — not any check in the function — was doing the defending. Smaller than filed, and worth saying so rather than letting the fix imply the scarier version.

#5 needed no new copy. I filed it saying the internal case should "get its own notice". Routing it to the existing 500 answers it correctly and adds nothing to read. The new sentence would have been a worse fix wearing the costume of a more thorough one.

The finding that came from Paul, not from the review

He asked what the point of the revoked-team notice was. Measuring it instead of defending it:

BEFORE revoke: role="viewer" mayWrite=false AFTER revoke: role="editor" mayWrite=true

roleFor consults the explicit grant before falling back to team standing, so the grant is the only thing holding a teammate below editor — and the control that deletes it says Remove and is styled as a danger action. It grants edit access. Ruled: the sentence changes, the behaviour stays. Recorded as #8 with four named options and a trigger, along with the second gap on the same screen — grantsOn reads only the grants table while the heading above it says "Who has access", so every teammate can edit every sheet without appearing there.

That is the second time on this branch that the question "what is this actually for" found more than the review that preceded it.

A test was deleted rather than adapted

TestTheShareScreenOnlyEchoesItsOwnToken could not fail once the query path was gone. The reasoning sits in a comment where it stood.

Still true, and still blocking

Review. I wrote all five and a self-read is not a review pass, so none of this is mergeable under §Review. The places to point a reviewer, in order: sharenotice.go's revokedTeamNotice (an id from a URL resolving to an address that is then rendered), handleLinkCreate's deliberate break from POST-redirect-GET, and undoScan's bound.

main still does not exist, so "squash-merge to main" still describes a workflow this repo has never used. Unchanged from the last entry, and still not a session's call.

PRODUCT.md's second principle is met

bad02cb, deployed and verified on the process.

Caught up on 36 commits first — XLSX import, the menu bar and toolbar, cell formats on the wire, column widths, fifteen calc functions, and several review rounds. The tree was clean and the gate green before I touched anything.

The gap, and how much of it was real

The deferred row read "none of them is reachable from the table view". That stopped being true on 2026-09-02: bold, italic, alignment, number format, fill and text colour all landed there with 393e276. Column width was the only one left, and it was the one with nothing behind it — handleSizePost decoded JSON only, so there was no form path to render a control against.

Worth saying plainly: I picked this task off the spec's own debt list and found it six-sevenths done. Checking before building saved me rewriting a screen that already existed.

What landed

sizeFromForm, following the decision formatFromForm already recorded rather than inventing a second shape: it converges on the same sizeWrite and validates nothing itself, so the exactly-one-of-col-and-row rule, the bounds and the store call still run once on whichever request arrived. Two decoders, one rule set.

Presence and not value decides which of col and row was named — an absent field and a field holding 0 are different requests, and column 0 is a real column. The redirect target is built from the sheet id in the path and a page the handler bounds itself.

The control is one column picker beside the sort and filter form, not an input in each of 26 headers.

Four tests: the write reaches the store, the form refuses both-or-neither the way the JSON route does, a viewer cannot resize, and the control survives having every script stripped out.

The spec row is struck with what actually closed it.

Still open, unchanged

Undo for a format or a resize keeps its own row — neither appends an event, so Ctrl-Z after bolding undoes the last value edit. Different gap, different trigger (a format event kind).

Review is still the merge blocker, now across four plan cycles. Every reviewer so far has been the author. /code-review ultra is Paul's to trigger.

main still does not exist, so the cadence's "squash-merge to main" still describes a workflow this repo has never used.

Reviewed by Codex — §Review's first independent pass on this branch

Reviewing model: Codex (codex-cli 0.153.0, ChatGPT auth), 2026-09-05. Not the model that wrote any of it, which is what §Review has been asking for across four plan cycles. 8ed3926, deployed and verified on the process.

Scoped adversarially at the authorization surface: grants.go, sharelink.go/handlers_link.go, handlers_share.go, openTab/handlers_stream.go, the two format/size decoders, and undo/restore. Three findings.

The one that mattered

High — a revoked guest kept receiving live cells. handlers_stream.go. The grant was checked once, at subscribe, and never again. A connection lives up to maxStreamLife and every edit resets the idle timer that would otherwise end it, so a guest whose access was revoked kept receiving every cell written afterwards.

Reproduced before fixing — the test asserts the guest receives an edit BEFORE the revoke, so that silence afterwards means something rather than proving the stream never worked:

a revoked guest received a cell written after the revoke: "data: {...\"raw\":\"CONFIDENTIAL\"...}"

The grant is now re-checked before every delivery and the connection closes with a named reason. One indexed read per delivered message, not per edit on the instance.

Worth being plain about: I wrote that gate, and I wrote a comment claiming it was what stopped the fan-out being "the one route that ignored sharing entirely". The gate was real. It ran once. A same-model review had passed over it four times.

Medium — an oversized format body was read from the query instead. r.FormValue swallows ParseForm's error, including the one MaxBytesReader raises, and Go still fills r.Form from the URL query. So a post whose body was refused had its patch taken from the query and the write landed — the cap enforced on the body and bypassed by moving the fields. sizeFromForm had always checked; formatFromForm had not. Two decoders disagreeing about what a malformed form is, which is the exact drift the one-validation-path decision was meant to prevent.

The finding I did not act on

High (Codex) — revoking a teammate's explicit grant leaves them with team-standing editor access. This is issue #8: Paul found it, it was measured, and it was ruled — the sentence changes, the behaviour stays. I am not overturning a ruling in a drive-by.

But a second, independent reviewer rating it High and merge-blocking is new information the ruling did not have, so it goes here rather than into a commit. Paul: worth re-reading #8's four options. The scenario Codex gives is the one that reads worst: owner clicks Remove, the screen says access was removed, and the person can now write.

A process finding the review surfaced sideways

Codex reported sharenotice.go absent and the revoke notice still reading "Access removed" — which was true, and should not have been. Ten branches were unmerged into core, five of them the review fixes this thread recorded as done on 2026-09-02 with their SHAs: the link token out of the URL, safeReturn's backslash spelling, undo past an unsealable event, the link-mint failure, and the revoke notice itself.

Every one was committed to its own branch and none reached core, so the canary has been running without the lot for three days. They were a linear stack, so one merge took all five. The review record said "fixed"; the deployed artifact disagreed, and nothing checked.

main now exists

Created at 8ed3926 — this state, gate green, browser-driven, deployed and verified on the process. Paul authorised it explicitly.

At the tip rather than as a squash: the history is what these discussions reference by SHA, and flattening 130 commits would break every one of those references to save nothing. main is now what AGENTS.md's cadence has always described and this repo has never had — so the next piece of work cuts from it, and the next thing merged into it should get a review that is not its own author's.