plan: the grid surface and live sync
Plan 2, against spec sections 6, 7, 10 and 11.1. Thirteen tasks from the DEK the app has never had, through a view layer, the fan-out hub and the SSE endpoint, to two browser tabs on the canary painting the same edit. Three things it states rather than assumes: sharing is not in it (the allowlist is the team until section 8 lands), presence and undo are deferred with written triggers, and it inverts section 13's ordering of sharing before live sync — with the reason written down.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RN4HhHR4rHH3Fxh4LMds5
1 file changed,
+1802
−0
docs/superpowers/plans/2026-08-31-sheets-grid-live.md+1802 −0
diff --git a/docs/superpowers/plans/2026-08-31-sheets-grid-live.md b/docs/superpowers/plans/2026-08-31-sheets-grid-live.md| new file mode 100644 |
| index 0000000..20bd563 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/2026-08-31-sheets-grid-live.md |
| @@ -0,0 +1,1802 @@ |
| +# Sheets Grid and Live Sync Implementation Plan |
| + |
| +> **For agentic workers:** REQUIRED SUB-SKILL: Use |
| +> superpowers:subagent-driven-development (recommended) or |
| +> superpowers:executing-plans to implement this plan task-by-task. Steps use |
| +> checkbox (`- [ ]`) syntax for tracking. |
| + |
| +**Goal:** A spreadsheet you can open in a browser, type into, and watch |
| +update in another browser tab within a second — on a deployed canary, not |
| +localhost. |
| + |
| +**Architecture:** The engine from plan 1 stays exactly where it is. This |
| +plan adds the three layers above it: a **view layer** (embedded Go |
| +templates over rastrillo's tokens and partials, theme resolved server-side |
| +before first paint), a **grid surface** (vendored Jspreadsheet CE painting |
| +strings the server computed, never formulas the client evaluated), and a |
| +**live layer** (an in-process SSE hub fanning each committed edit out to |
| +every other open tab, carrying the cell version so a client can discard an |
| +out-of-order paint). Every write goes through one server path — |
| +`store.Handle.SetReport` — so the scriptless form POST and the grid's |
| +fetch produce identical events and identical fan-out. |
| + |
| +**Tech Stack:** Go 1.26, `github.com/carlosframework/rastrillo` v0.22.0 |
| +(`ui` for tokens/layouts/partials, `auth` for sessions, `csrf`), Jspreadsheet |
| +CE v5.0.2 + jsuites v5 vendored as static assets (MIT, verified in the |
| +package's own LICENSE), `html/template`. No bundler, no CDN, no build step. |
| + |
| +**Spec:** `docs/superpowers/specs/2026-08-30-sheets-design.md` — §6 (the |
| +grid, and where zero-JS stops), §7 (collaboration), §10 (design system), |
| +§11.1 (a retained fill can be unreadable), §14 (the deferred table this plan |
| +triggers one row of). |
| + |
| +--- |
| + |
| +## Global Constraints |
| + |
| +Copied from `AGENTS.md` and the spec. Every task's requirements include |
| +these. |
| + |
| +- **The one rule:** *"A sheet's contents are sealed at rest. The bucket never |
| + holds a plaintext cell."* Nothing this plan adds writes a cell value |
| + anywhere but through `seal.Sealer`. Titles and tab names are sealed too. |
| +- **No cell content, comment text, sheet title or tab name in any log line, |
| + error string, job status message or metric label.** This plan adds an SSE |
| + layer whose natural debug logging would violate this. Log tab ids and |
| + counts, never payloads. |
| +- **`CGO_ENABLED=0` throughout.** |
| +- **Migrations are additive-only.** This plan adds one table (`members`); |
| + it alters nothing. |
| +- **Never `scope.Owned` or `scope.OwnedBy`.** Sheets are team-shared. Until |
| + §8's grants land, access is instance-wide: every address in |
| + `SHEETS_OWNERS` sees every sheet. That is a *stated* scope, written in |
| + the code and tested (Task 3), not an oversight — see "Assumptions" below. |
| +- **Cell values are IEEE doubles end to end.** |
| +- **200,000 cells per tab, hard.** The grid paints a window, never the tab. |
| +- **300-line cap per file, .go and .js alike.** The vendored Jspreadsheet |
| + and jsuites blobs are the only exemptions and are named as such in |
| + `lineCapExempt` / `jsLineCapExempt`. |
| +- **Write the attribute grammar** (`rst-btn="primary compact"`), never the |
| + class spelling, and **adopt per screen**. Utilities stay classes. |
| +- **Light/dark is per person and must resolve before first paint.** Read it |
| + synchronously from a cookie, server-side. Never a fetch. |
| +- **Every drag-only affordance has a menu equivalent.** |
| +- **No form in the app is script-required.** |
| +- **Review on a deployed canary, never localhost.** `carlos deploy`, never |
| + `carlos ship`. `make release`, never a bare `go build`. Verify with |
| + `GET /api/version` — the process, not the edge — and poll, because a |
| + promote is not a deploy. |
| + |
| +## Assumptions, stated because they are mine and not Paul's |
| + |
| +Written down so they are cheap to overturn rather than discovered later. |
| + |
| +1. **"Live updates across tabs" means browser tabs.** Two windows on the |
| + same sheet; type in one, see it in the other. It also happens to work |
| + across spreadsheet tabs because the fan-out key is the tab id. |
| +2. **Sharing is not in this plan.** The instance allowlist is the team. |
| + Spec §13 sequences sharing (6) before live sync (7), but sharing is a |
| + subsystem — grants, roles, link sharing, the home contract — and none of |
| + it is needed to make a grid live. Doing it here would have swapped a |
| + working spreadsheet for a half-finished sharing model. |
| +3. **Presence is not in this plan.** §7 pairs presence with LWW; who's-here |
| + is a second, independent SSE message type and adds nothing to |
| + correctness. Deferred with a trigger: *the first time two people report |
| + colliding on a cell.* |
| +4. **Undo is not in this plan.** §7's undo is own-edits-only over the event |
| + log, and the event log already records what it needs. Deferred with a |
| + trigger: *the grid is in daily use by anyone.* |
| +5. **The grid paints a fixed window** — 100 rows × 26 columns, extendable by |
| + an explicit button — rather than virtualising. §6's cap reasoning is |
| + about not rendering 200,000 cells; a window is the smallest thing that |
| + honours it. |
| + |
| +--- |
| + |
| +## File Structure |
| + |
| +``` |
| +internal/sheets/ |
| + members.go Address -> actor id. The only place a Member row is made. |
| + render.go Template parsing, PageData, theme-before-first-paint. |
| + theme.go The theme cookie: read, write, and the POST handler. |
| + sheetsvc.go Create/list/open a Sheet and its Tab. Sealed titles. |
| + handlers_sheets.go GET /sheets, POST /sheets, GET /sheets/{id}. |
| + handlers_grid.go The grid JSON API: GET cells, POST cell. |
| + handlers_table.go The scriptless table view and its per-cell edit form. |
| + handlers_stream.go GET .../stream — the SSE endpoint. Transport only. |
| + keys.go SHEETS_DEK -> *seal.Sealer, refusing at boot. |
| + templates/ |
| + layout.html The one layout. rastrillo `column`, attribute grammar. |
| + index.html Root: signed-in state and a way to the sheet list. |
| + signin.html Moved off page.go's string constants. |
| + sheets.html The sheet list, plus "new sheet". Scriptless. |
| + sheet.html The grid page. <noscript> points at the table view. |
| + table.html The scriptless, paginated table with edit forms. |
| + static/ |
| + tokens.css Copied by `rastrillo doctor`. Frozen deliberately. |
| + sheets.css Our own grid chrome. Tokens only, no raw colours. |
| + grid.js Our grid module. <= 300 lines, enforced. |
| + live.js SSE client: subscribe, apply-by-version, heartbeat. |
| + vendor/ |
| + jspreadsheet.js Vendored v5.0.2. Exempt from the cap, named as such. |
| + jspreadsheet.css |
| + jsuites.js |
| + jsuites.css |
| + live/ |
| + hub.go The fan-out hub. No HTTP, no templates, no store. |
| + hub_test.go |
| + store/ |
| + handle_report.go SetReport: Set, but it tells you what changed. |
| +``` |
| + |
| +Rationale for the splits: the HTTP layer is split by surface rather than by |
| +verb, because the scriptless table view and the JSON grid API are the two |
| +things most likely to drift apart, and keeping them in separate files with a |
| +shared service (`sheetsvc.go`) makes the drift visible. `live/` has no |
| +dependency on `store` or `net/http` at all — it moves opaque messages — so |
| +its tests are pure and fast, and the fan-out logic can be reasoned about |
| +without a database. |
| + |
| +--- |
| + |
| +### Task 1: The DEK, and refusing to boot without it |
| + |
| +Nothing in the app builds a `*seal.Sealer` yet — plan 1's tests each made |
| +their own. A grid cannot store a cell without one. Spec §2.1: *"The DEK |
| +arrives as platform config (`carlos secrets`), never stored in the |
| +database."* |
| + |
| +**Files:** |
| +- Create: `internal/sheets/keys.go`, `internal/sheets/keys_test.go` |
| +- Modify: `cmd/sheets/main.go`, `internal/sheets/app.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `seal.New(dek []byte) (*seal.Sealer, error)`. |
| +- Produces: `sheets.LoadSealer() (*seal.Sealer, error)`, and `App` gains a |
| + `sealer *seal.Sealer` field set by `New`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/sheets/keys_test.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import ( |
| + "encoding/base64" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +func TestLoadSealerRefusesAMissingDEK(t *testing.T) { |
| + t.Setenv("SHEETS_DEK", "") |
| + if _, err := LoadSealer(); err == nil { |
| + t.Fatal("LoadSealer with no SHEETS_DEK = nil error, want a refusal") |
| + } else if !strings.Contains(err.Error(), "SHEETS_DEK") { |
| + t.Fatalf("error %q does not name the variable an operator must set", err) |
| + } |
| +} |
| + |
| +func TestLoadSealerRefusesAShortDEK(t *testing.T) { |
| + t.Setenv("SHEETS_DEK", base64.StdEncoding.EncodeToString(make([]byte, 16))) |
| + if _, err := LoadSealer(); err == nil { |
| + t.Fatal("LoadSealer with a 16-byte DEK = nil error, want a refusal") |
| + } |
| +} |
| + |
| +func TestLoadSealerAcceptsA32ByteDEK(t *testing.T) { |
| + t.Setenv("SHEETS_DEK", base64.StdEncoding.EncodeToString(make([]byte, 32))) |
| + s, err := LoadSealer() |
| + if err != nil { |
| + t.Fatalf("LoadSealer: %v", err) |
| + } |
| + if s == nil { |
| + t.Fatal("LoadSealer returned a nil sealer and a nil error") |
| + } |
| +} |
| + |
| +// The DEK is a live key. A refusal that echoes it puts it in platform-held |
| +// log storage we cannot grep (AGENTS.md / Mechanical rules). |
| +func TestLoadSealerNeverEchoesTheKey(t *testing.T) { |
| + const secret = "c2VjcmV0LXNlY3JldC1zZWNyZXQtc2VjcmV0LTEyMw==" |
| + t.Setenv("SHEETS_DEK", secret) |
| + _, err := LoadSealer() |
| + if err != nil && strings.Contains(err.Error(), secret) { |
| + t.Fatalf("error text contains the DEK itself: %q", err) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `go test ./internal/sheets/ -run TestLoadSealer -v` |
| +Expected: FAIL — `undefined: LoadSealer`. |
| + |
| +- [ ] **Step 3: Write the implementation** |
| + |
| +Create `internal/sheets/keys.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import ( |
| + "encoding/base64" |
| + "errors" |
| + "fmt" |
| + "os" |
| + "strings" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| +) |
| + |
| +// dekBytes is the DEK's required length. seal.New enforces it too; this |
| +// check exists so the refusal names SHEETS_DEK and the length an |
| +// operator has to produce, rather than surfacing a framework message |
| +// about a byte slice. |
| +const dekBytes = 32 |
| + |
| +// LoadSealer reads the data encryption key from platform config and |
| +// builds the app's one Sealer. |
| +// |
| +// The DEK arrives as platform config (`carlos secrets`), never stored in |
| +// the database (spec 2.1). A missing one is a hard refusal rather than a |
| +// generated key: an instance that invents a DEK at boot seals this run's |
| +// cells with a key the next run will not have, which reads as data loss |
| +// and cannot be undone. |
| +// |
| +// No error here ever includes the value read from the environment. It is |
| +// a live key and `carlos logs` is platform-held storage we cannot grep. |
| +func LoadSealer() (*seal.Sealer, error) { |
| + raw := strings.TrimSpace(os.Getenv("SHEETS_DEK")) |
| + if raw == "" { |
| + return nil, errors.New( |
| + "sealing is not configured: SHEETS_DEK not set. " + |
| + "Generate one with `head -c 32 /dev/urandom | base64` and set it " + |
| + "with `carlos secrets set --app sheets SHEETS_DEK=...`") |
| + } |
| + dek, err := base64.StdEncoding.DecodeString(raw) |
| + if err != nil { |
| + return nil, errors.New("sealing is not configured: SHEETS_DEK is not valid base64") |
| + } |
| + if len(dek) != dekBytes { |
| + return nil, fmt.Errorf( |
| + "sealing is not configured: SHEETS_DEK decodes to %d bytes, want %d", |
| + len(dek), dekBytes) |
| + } |
| + s, err := seal.New(dek) |
| + if err != nil { |
| + return nil, fmt.Errorf("sealing is not configured: %w", err) |
| + } |
| + return s, nil |
| +} |
| +``` |
| + |
| +- [ ] **Step 4: Thread it into App and main** |
| + |
| +In `internal/sheets/app.go`, add the field and parameter: |
| + |
| +```go |
| +type App struct { |
| + db *gorm.DB |
| + auth *auth.Auth |
| + sealer *seal.Sealer |
| + admit func(string) bool |
| +} |
| + |
| +// New builds an App. db and sealer may be nil ONLY in tests that do not |
| +// touch storage; every route that reads or writes a cell checks and 500s |
| +// rather than panicking, which TestGridWithoutASealerIs500 pins. |
| +func New(db *gorm.DB, a *auth.Auth, sealer *seal.Sealer, admit func(string) bool) *App { |
| + return &App{db: db, auth: a, sealer: sealer, admit: admit} |
| +} |
| +``` |
| + |
| +Update every existing `New(...)` call site — `app_test.go`, `page_test.go`, |
| +`auth_test.go`, `begin_test.go`, `signin_test.go` — to pass `nil` for the |
| +new parameter. |
| + |
| +In `cmd/sheets/main.go`, after `LoadAuthEnv` and before `sheets.New`: |
| + |
| +```go |
| + // Same shape as the sign-in refusals above and for the same reason: |
| + // an instance with no DEK answers /healthz perfectly well and then |
| + // fails on the first cell anybody types. |
| + sealer, err := sheets.LoadSealer() |
| + if err != nil { |
| + logger.Error("sealing", "err", err) |
| + os.Exit(1) |
| + } |
| + |
| + app := sheets.New(d.G, signin, sealer, sheets.Admits(authEnv.Owners)) |
| +``` |
| + |
| +- [ ] **Step 5: Run the tests** |
| + |
| +Run: `make test-vet test-build test-go` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +git add internal/sheets/keys.go internal/sheets/keys_test.go \ |
| + internal/sheets/app.go cmd/sheets/main.go internal/sheets/*_test.go |
| +git commit -m "feat: the DEK arrives as platform config, or the instance refuses to boot" |
| +``` |
| + |
| +--- |
| + |
| +### Task 2: Members — an address becomes an actor id |
| + |
| +`Cell.UpdatedBy`, `Sheet.CreatedBy` and `Event.ActorID` are `uint64`. The |
| +session carries an email address. Something has to map one to the other, and |
| +`sessions.UserID` cannot (AGENTS.md: it returns `(0, false)` for an email |
| +Subject). Spec §2.2 puts membership and readable addresses in the plaintext |
| +column, so this table is not sealed. |
| + |
| +**Files:** |
| +- Modify: `internal/sheets/models.go`, `internal/sheets/migrations.go` |
| +- Create: `internal/sheets/migrations/0002_create_members.sql` |
| +- Create: `internal/sheets/members.go`, `internal/sheets/members_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `App.db`, `App.identity(r)` from `page.go`. |
| +- Produces: `type Member struct{ ID uint64; Address string; CreatedAt time.Time }`, |
| + and `func (a *App) actor(r *http.Request) (Member, bool)`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/sheets/members_test.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import "testing" |
| + |
| +func TestMemberForIsIdempotent(t *testing.T) { |
| + app := newTestApp(t) // helper from app_test.go; see Step 3 |
| + a, err := app.memberFor("Ada@example.com") |
| + if err != nil { |
| + t.Fatalf("memberFor: %v", err) |
| + } |
| + if a.ID == 0 { |
| + t.Fatal("memberFor returned id 0, which is the uid-0 trap AGENTS.md names") |
| + } |
| + b, err := app.memberFor("ada@example.com") |
| + if err != nil { |
| + t.Fatalf("memberFor again: %v", err) |
| + } |
| + if b.ID != a.ID { |
| + t.Fatalf("memberFor is case-sensitive: %d then %d", a.ID, b.ID) |
| + } |
| +} |
| + |
| +func TestMemberForRejectsAnEmptyAddress(t *testing.T) { |
| + app := newTestApp(t) |
| + if _, err := app.memberFor(""); err == nil { |
| + t.Fatal("memberFor(\"\") = nil error; an empty address must never mint a member") |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `go test ./internal/sheets/ -run TestMemberFor -v` |
| +Expected: FAIL — `undefined: newTestApp`, `undefined: memberFor`. |
| + |
| +- [ ] **Step 3: Add the model, the migration and the helper** |
| + |
| +In `internal/sheets/models.go`: |
| + |
| +```go |
| +// Member is one person who may hold a session on this instance. |
| +// |
| +// Address is stored in plaintext, deliberately: spec 2.2 lists |
| +// "membership, grants, and roles — including readable email addresses" |
| +// among the things this design does NOT seal, and publishes that fact in |
| +// the README rather than leaving it to be discovered. Sealing it would |
| +// also make the uniqueIndex below impossible, since a random nonce per |
| +// seal means the same address seals to a different ciphertext every |
| +// time. |
| +// |
| +// The row exists so a cell edit can carry a stable uint64 actor id. |
| +// Addresses are lowercased before they reach here (memberFor), matching |
| +// the admission allowlist's own comparison. |
| +type Member struct { |
| + ID uint64 `gorm:"primaryKey"` |
| + Address string `gorm:"uniqueIndex;size:320"` |
| + CreatedAt time.Time |
| +} |
| +``` |
| + |
| +Add `&Member{}` to `Models` in `internal/sheets/migrations.go`. |
| + |
| +Create `internal/sheets/migrations/0002_create_members.sql`: |
| + |
| +```sql |
| +CREATE TABLE `members` ( |
| + `id` integer PRIMARY KEY AUTOINCREMENT, |
| + `address` text, |
| + `created_at` datetime |
| +); |
| +CREATE UNIQUE INDEX `idx_members_address` ON `members`(`address`); |
| +``` |
| + |
| +Then run `rastrillo migration check` and reconcile the SQL with whatever it |
| +reports until it is silent — the committed SQL must be exactly what the |
| +generator would produce, or `make test-migrations` fails in CI. |
| + |
| +Create `internal/sheets/members.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import ( |
| + "errors" |
| + "net/http" |
| + "strings" |
| + |
| + "gorm.io/gorm" |
| + "gorm.io/gorm/clause" |
| +) |
| + |
| +// memberFor returns the Member row for an address, creating it on first |
| +// sight. |
| +// |
| +// First sight is the right moment because admission has already |
| +// happened: auth.Config.Authorize refused every address not on the |
| +// allowlist before a session existed, so anything reaching here is |
| +// someone who is allowed to be here. Creating rows at boot from |
| +// SHEETS_OWNERS instead would mint members for people who never signed |
| +// in and would go stale the moment the allowlist changed. |
| +// |
| +// The upsert is DO NOTHING plus a read, not a read-then-create: two tabs |
| +// signing in at once would otherwise race and one would get a unique |
| +// violation. |
| +func (a *App) memberFor(address string) (Member, error) { |
| + addr := strings.ToLower(strings.TrimSpace(address)) |
| + if addr == "" { |
| + return Member{}, errors.New("sheets: refusing to mint a member for an empty address") |
| + } |
| + if a.db == nil { |
| + return Member{}, errors.New("sheets: no database") |
| + } |
| + m := Member{Address: addr} |
| + if err := a.db.Clauses(clause.OnConflict{DoNothing: true}).Create(&m).Error; err != nil { |
| + return Member{}, err |
| + } |
| + if m.ID != 0 { |
| + return m, nil |
| + } |
| + // The insert conflicted: the row is already there, so read it. |
| + var existing Member |
| + if err := a.db.Where("address = ?", addr).First(&existing).Error; err != nil { |
| + if errors.Is(err, gorm.ErrRecordNotFound) { |
| + return Member{}, errors.New("sheets: member neither created nor found") |
| + } |
| + return Member{}, err |
| + } |
| + return existing, nil |
| +} |
| + |
| +// actor resolves the signed-in viewer to a Member. No session is not an |
| +// error — it is "no actor" — so callers gate on ok rather than on err, |
| +// and a route that requires one has already been through |
| +// auth.RequireSession. |
| +func (a *App) actor(r *http.Request) (Member, bool) { |
| + address, ok := a.identity(r) |
| + if !ok { |
| + return Member{}, false |
| + } |
| + m, err := a.memberFor(address) |
| + if err != nil { |
| + return Member{}, false |
| + } |
| + return m, true |
| +} |
| +``` |
| + |
| +- [ ] **Step 4: Add the test helper** |
| + |
| +In `internal/sheets/app_test.go`, add a helper the later tasks reuse. It |
| +opens a real SQLite file under `t.TempDir()` through rastrillo's `db` |
| +package (never `gorm.io/driver/sqlite` — the forbidden-imports guard bans |
| +it) and applies `BootSchema`: |
| + |
| +```go |
| +// newTestApp builds an App over a real, migrated, throwaway database and |
| +// a deterministic all-zero DEK. Nil auth: these tests exercise storage |
| +// and rendering, not sign-in. |
| +func newTestApp(t *testing.T) *App { |
| + t.Helper() |
| + d, err := rdb.Open(filepath.Join(t.TempDir(), "test.db"), slog.Default()) |
| + if err != nil { |
| + t.Fatalf("db.Open: %v", err) |
| + } |
| + t.Cleanup(func() { d.Close() }) |
| + if _, err := migrate.Apply(context.Background(), d, BootSchema); err != nil { |
| + t.Fatalf("migrate: %v", err) |
| + } |
| + s, err := seal.New(make([]byte, 32)) |
| + if err != nil { |
| + t.Fatalf("seal.New: %v", err) |
| + } |
| + return New(d.G, nil, s, func(string) bool { return true }) |
| +} |
| +``` |
| + |
| +- [ ] **Step 5: Run the tests** |
| + |
| +Run: `go test ./internal/sheets/ -run TestMember -v && make test-migrations` |
| +Expected: PASS, and `rastrillo migration check` silent. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +git add internal/sheets/members.go internal/sheets/members_test.go \ |
| + internal/sheets/models.go internal/sheets/migrations.go \ |
| + internal/sheets/migrations/0002_create_members.sql \ |
| + internal/sheets/app_test.go |
| +git commit -m "feat: an address becomes a stable actor id" |
| +``` |
| + |
| +--- |
| + |
| +### Task 3: Sheets and tabs — create, list, open |
| + |
| +**Files:** |
| +- Create: `internal/sheets/sheetsvc.go`, `internal/sheets/sheetsvc_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `App.db`, `App.sealer`, `Member` from Task 2. |
| +- Produces: |
| + - `type SheetSummary struct{ ID uint64; Title string; TabID uint64; UpdatedAt time.Time }` |
| + - `func (a *App) createSheet(title string, by Member) (SheetSummary, error)` |
| + - `func (a *App) listSheets() ([]SheetSummary, error)` |
| + - `func (a *App) sheetSummary(id uint64) (SheetSummary, error)` |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/sheets/sheetsvc_test.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import "testing" |
| + |
| +func TestCreateSheetMakesAFirstTab(t *testing.T) { |
| + app := newTestApp(t) |
| + m, err := app.memberFor("ada@example.com") |
| + if err != nil { |
| + t.Fatalf("memberFor: %v", err) |
| + } |
| + s, err := app.createSheet("Budget", m) |
| + if err != nil { |
| + t.Fatalf("createSheet: %v", err) |
| + } |
| + if s.TabID == 0 { |
| + t.Fatal("createSheet made a sheet with no tab; there is nothing to open") |
| + } |
| + if s.Title != "Budget" { |
| + t.Fatalf("title = %q, want %q", s.Title, "Budget") |
| + } |
| +} |
| + |
| +// The one rule. A title is sheet content. |
| +func TestSheetTitleIsSealedAtRest(t *testing.T) { |
| + app := newTestApp(t) |
| + m, _ := app.memberFor("ada@example.com") |
| + if _, err := app.createSheet("Salaries 2027", m); err != nil { |
| + t.Fatalf("createSheet: %v", err) |
| + } |
| + var raw []byte |
| + if err := app.db.Raw("SELECT title_sealed FROM sheets LIMIT 1").Scan(&raw).Error; err != nil { |
| + t.Fatalf("read back: %v", err) |
| + } |
| + if bytes.Contains(raw, []byte("Salaries")) { |
| + t.Fatal("the sheet title reached the database in plaintext") |
| + } |
| +} |
| + |
| +func TestListSheetsIsInstanceWideNotPerCreator(t *testing.T) { |
| + app := newTestApp(t) |
| + ada, _ := app.memberFor("ada@example.com") |
| + bob, _ := app.memberFor("bob@example.com") |
| + if _, err := app.createSheet("Ada's", ada); err != nil { |
| + t.Fatalf("createSheet: %v", err) |
| + } |
| + if _, err := app.createSheet("Bob's", bob); err != nil { |
| + t.Fatalf("createSheet: %v", err) |
| + } |
| + got, err := app.listSheets() |
| + if err != nil { |
| + t.Fatalf("listSheets: %v", err) |
| + } |
| + // This is the scope.Owned trap AGENTS.md names: the wrong |
| + // implementation returns one row and looks like an empty-ish list |
| + // rather than an error. |
| + if len(got) != 2 { |
| + t.Fatalf("listSheets returned %d sheets, want 2 — sheets are team-shared", len(got)) |
| + } |
| +} |
| + |
| +func TestCreateSheetRefusesAnEmptyTitle(t *testing.T) { |
| + app := newTestApp(t) |
| + m, _ := app.memberFor("ada@example.com") |
| + if _, err := app.createSheet(" ", m); err == nil { |
| + t.Fatal("createSheet with a blank title = nil error, want a refusal") |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `go test ./internal/sheets/ -run 'TestCreateSheet|TestListSheets|TestSheetTitle' -v` |
| +Expected: FAIL — `undefined: createSheet`. |
| + |
| +- [ ] **Step 3: Write the implementation** |
| + |
| +Create `internal/sheets/sheetsvc.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import ( |
| + "errors" |
| + "strings" |
| + "time" |
| + |
| + "gorm.io/gorm" |
| +) |
| + |
| +// maxTitleRunes caps a sheet title. It is a UI-shaped limit, not a |
| +// storage one — the sealed column takes anything — and exists so a |
| +// pasted paragraph cannot become a sheet name that breaks every list |
| +// row. |
| +const maxTitleRunes = 200 |
| + |
| +// SheetSummary is one row of the sheet list: everything a list or a page |
| +// header needs, with the title already unsealed. |
| +// |
| +// TabID is the sheet's FIRST tab, which is the one a bare /sheets/{id} |
| +// opens. Multi-tab navigation is a later slice; carrying the id here |
| +// means the grid page never has to query for it. |
| +type SheetSummary struct { |
| + ID uint64 |
| + Title string |
| + TabID uint64 |
| + UpdatedAt time.Time |
| +} |
| + |
| +// errNoSealer is returned rather than panicking when a nil-sealer App |
| +// (tests that render pages only) reaches a path that needs one. |
| +var errNoSealer = errors.New("sheets: no sealer configured") |
| + |
| +// createSheet makes a sheet and its first tab in one transaction. |
| +// |
| +// One transaction because a sheet with no tab is not openable: it would |
| +// list, and then 404 when clicked. Nothing outside this function ever |
| +// creates a Sheet row, which is what makes "every sheet has at least one |
| +// tab" a real invariant rather than a convention. |
| +func (a *App) createSheet(title string, by Member) (SheetSummary, error) { |
| + if a.db == nil || a.sealer == nil { |
| + return SheetSummary{}, errNoSealer |
| + } |
| + clean := strings.TrimSpace(title) |
| + if clean == "" { |
| + return SheetSummary{}, errors.New("sheets: a sheet needs a title") |
| + } |
| + if len([]rune(clean)) > maxTitleRunes { |
| + clean = string([]rune(clean)[:maxTitleRunes]) |
| + } |
| + sealedTitle, err := a.sealer.Seal("sheet.title", []byte(clean)) |
| + if err != nil { |
| + return SheetSummary{}, err |
| + } |
| + sealedName, err := a.sealer.Seal("tab.name", []byte("Sheet 1")) |
| + if err != nil { |
| + return SheetSummary{}, err |
| + } |
| + |
| + sheet := Sheet{TitleSealed: sealedTitle, CreatedBy: by.ID} |
| + tab := Tab{NameSealed: sealedName, Position: 0} |
| + if err := a.db.Transaction(func(tx *gorm.DB) error { |
| + if err := tx.Create(&sheet).Error; err != nil { |
| + return err |
| + } |
| + tab.SheetID = sheet.ID |
| + return tx.Create(&tab).Error |
| + }); err != nil { |
| + return SheetSummary{}, err |
| + } |
| + return SheetSummary{ID: sheet.ID, Title: clean, TabID: tab.ID, UpdatedAt: sheet.UpdatedAt}, nil |
| +} |
| + |
| +// listSheets returns every live sheet on the instance, newest edit |
| +// first. |
| +// |
| +// Deliberately unscoped by creator. Sheets are team-shared, and |
| +// scope.Owned here would return only the caller's own rows — failing as |
| +// a plausible short list rather than as an error (AGENTS.md / |
| +// Mechanical rules; TestListSheetsIsInstanceWideNotPerCreator). When |
| +// spec 8's grants land, this grows a hand-written grants join; it does |
| +// NOT grow a scope.Owned. |
| +func (a *App) listSheets() ([]SheetSummary, error) { |
| + if a.db == nil || a.sealer == nil { |
| + return nil, errNoSealer |
| + } |
| + var rows []Sheet |
| + if err := a.db.Where("deleted_at IS NULL").Order("updated_at DESC").Find(&rows).Error; err != nil { |
| + return nil, err |
| + } |
| + out := make([]SheetSummary, 0, len(rows)) |
| + for _, s := range rows { |
| + sum, err := a.summarize(s) |
| + if err != nil { |
| + return nil, err |
| + } |
| + out = append(out, sum) |
| + } |
| + return out, nil |
| +} |
| + |
| +// sheetSummary reads one sheet. A missing sheet is gorm.ErrRecordNotFound, |
| +// which handlers turn into a 404 — never a 403, per AGENTS.md. |
| +func (a *App) sheetSummary(id uint64) (SheetSummary, error) { |
| + if a.db == nil || a.sealer == nil { |
| + return SheetSummary{}, errNoSealer |
| + } |
| + var s Sheet |
| + if err := a.db.Where("id = ? AND deleted_at IS NULL", id).First(&s).Error; err != nil { |
| + return SheetSummary{}, err |
| + } |
| + return a.summarize(s) |
| +} |
| + |
| +// summarize unseals a Sheet row's title and finds its first tab. |
| +func (a *App) summarize(s Sheet) (SheetSummary, error) { |
| + title, err := a.sealer.Open("sheet.title", s.TitleSealed) |
| + if err != nil { |
| + return SheetSummary{}, err |
| + } |
| + var tab Tab |
| + if err := a.db.Where("sheet_id = ?", s.ID).Order("position ASC").First(&tab).Error; err != nil { |
| + return SheetSummary{}, err |
| + } |
| + return SheetSummary{ID: s.ID, Title: string(title), TabID: tab.ID, UpdatedAt: s.UpdatedAt}, nil |
| +} |
| +``` |
| + |
| +- [ ] **Step 4: Run the tests** |
| + |
| +Run: `go test ./internal/sheets/ -run 'TestCreateSheet|TestListSheets|TestSheetTitle' -v` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/sheets/sheetsvc.go internal/sheets/sheetsvc_test.go |
| +git commit -m "feat: create, list and read a sheet, with the title sealed" |
| +``` |
| + |
| +--- |
| + |
| +### Task 4: `SetReport` — a write that says what it changed |
| + |
| +Live sync needs the touched set. `Handle.Set` returns only an error, and |
| +seventeen tests plus `xlsx/import.go` call it. So `Set` keeps its signature |
| +and becomes a wrapper; the reporting form is new. |
| + |
| +**Files:** |
| +- Create: `internal/sheets/store/handle_report.go`, |
| + `internal/sheets/store/handle_report_test.go` |
| +- Modify: `internal/sheets/store/handle_set.go` (Set becomes a wrapper) |
| + |
| +**Interfaces:** |
| +- Consumes: `Handle.Set`'s internals — `setBlank`, `setContent`, `recalcTouched`. |
| +- Produces: |
| + |
| +```go |
| +type ChangedCell struct { |
| + RowID, ColID uint64 |
| + Raw string |
| + Value calc.Value |
| + Derived bool |
| +} |
| +type Report struct { |
| + Version uint64 |
| + Cells []ChangedCell |
| + Capped bool |
| +} |
| +func (h *Handle) SetReport(rowID, colID uint64, raw string, actor uint64) (Report, error) |
| +``` |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/sheets/store/handle_report_test.go`: |
| + |
| +```go |
| +package store |
| + |
| +import "testing" |
| + |
| +func TestSetReportNamesTheOriginAndItsDependents(t *testing.T) { |
| + h := openTestHandle(t) // existing helper in handle_test.go |
| + a1r, a1c := gridAt(t, h, 0, 0) |
| + b1r, b1c := gridAt(t, h, 0, 1) |
| + if err := h.Set(b1r, b1c, "=A1*2", 1); err != nil { |
| + t.Fatalf("Set B1: %v", err) |
| + } |
| + rep, err := h.SetReport(a1r, a1c, "21", 1) |
| + if err != nil { |
| + t.Fatalf("SetReport: %v", err) |
| + } |
| + if len(rep.Cells) != 2 { |
| + t.Fatalf("report covers %d cells, want 2 (A1 and its dependent B1)", len(rep.Cells)) |
| + } |
| + var sawOrigin, sawDerived bool |
| + for _, c := range rep.Cells { |
| + switch { |
| + case c.RowID == a1r && c.ColID == a1c: |
| + sawOrigin = true |
| + if c.Derived { |
| + t.Error("the edited cell is reported as derived") |
| + } |
| + if c.Raw != "21" { |
| + t.Errorf("origin raw = %q, want %q", c.Raw, "21") |
| + } |
| + case c.RowID == b1r && c.ColID == b1c: |
| + sawDerived = true |
| + if !c.Derived { |
| + t.Error("a recalculated dependent is not flagged derived") |
| + } |
| + if c.Value.Num != 42 { |
| + t.Errorf("B1 = %v, want 42", c.Value.Num) |
| + } |
| + } |
| + } |
| + if !sawOrigin || !sawDerived { |
| + t.Fatalf("origin seen=%v, dependent seen=%v", sawOrigin, sawDerived) |
| + } |
| +} |
| + |
| +func TestSetReportVersionAdvances(t *testing.T) { |
| + h := openTestHandle(t) |
| + r, c := gridAt(t, h, 0, 0) |
| + first, err := h.SetReport(r, c, "1", 1) |
| + if err != nil { |
| + t.Fatalf("SetReport: %v", err) |
| + } |
| + second, err := h.SetReport(r, c, "2", 1) |
| + if err != nil { |
| + t.Fatalf("SetReport: %v", err) |
| + } |
| + if second.Version <= first.Version { |
| + t.Fatalf("version did not advance: %d then %d", first.Version, second.Version) |
| + } |
| +} |
| + |
| +func TestSetReportOnBlankReportsTheClearedCell(t *testing.T) { |
| + h := openTestHandle(t) |
| + r, c := gridAt(t, h, 0, 0) |
| + if err := h.Set(r, c, "7", 1); err != nil { |
| + t.Fatalf("Set: %v", err) |
| + } |
| + rep, err := h.SetReport(r, c, "", 1) |
| + if err != nil { |
| + t.Fatalf("SetReport blank: %v", err) |
| + } |
| + if len(rep.Cells) == 0 { |
| + t.Fatal("blanking a cell reported no change; the other tab would never clear it") |
| + } |
| + if rep.Cells[0].Raw != "" || rep.Cells[0].Value.Kind != calc.KindEmpty { |
| + t.Fatalf("cleared cell reported as %+v, want empty", rep.Cells[0]) |
| + } |
| +} |
| +``` |
| + |
| +`openTestHandle` and `gridAt` may need adding to `handle_test.go` as thin |
| +wrappers over whatever the existing helpers there are named — read that file |
| +first and reuse rather than duplicate. |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `go test ./internal/sheets/store/ -run TestSetReport -v` |
| +Expected: FAIL — `h.SetReport undefined`. |
| + |
| +- [ ] **Step 3: Refactor Set into a reporting core** |
| + |
| +In `internal/sheets/store/handle_set.go`, rename the existing `Set` body's |
| +entry point to `setReportLocked` and have both public forms call it. The two |
| +inner methods gain a returned touched set: |
| + |
| +```go |
| +// Set writes a cell's new raw input and recalculates forward from it. |
| +// It is SetReport with the report discarded — kept because every caller |
| +// that does not fan out over SSE (XLSX import, most tests) wants exactly |
| +// this and nothing more. |
| +func (h *Handle) Set(rowID, colID uint64, raw string, actor uint64) error { |
| + _, err := h.SetReport(rowID, colID, raw, actor) |
| + return err |
| +} |
| +``` |
| + |
| +Create `internal/sheets/store/handle_report.go` with `SetReport`, holding |
| +`h.mu` for the whole call exactly as `Set` did, delegating to `setBlank` / |
| +`setContent` (both changed to return `map[calc.CellKey]struct{}` alongside |
| +their error), and then building the `Report` from the graph's current cached |
| +values plus `h.rawText`: |
| + |
| +```go |
| +// SetReport is Set, plus an account of what it changed: the cell that |
| +// was edited and every cell the recalculation touched, each with the |
| +// value it now holds and the version that value carries. |
| +// |
| +// The version is the single number a client uses to discard an |
| +// out-of-order paint (spec 7). Every cell in one report shares it, |
| +// because they were all written by one Set and one recalculation pass — |
| +// a per-cell version would be the same number repeated. |
| +// |
| +// Capped is true when the edit's dependent set was wider than |
| +// calc.MaxDirtyPerPass, so no recalculation ran and the report covers |
| +// the origin alone. A client seeing Capped reloads the window rather |
| +// than painting a partial truth. This is the honest error the cap |
| +// exists to produce, surfaced rather than swallowed. |
| +func (h *Handle) SetReport(rowID, colID uint64, raw string, actor uint64) (Report, error) { |
| + // ... implementation as described above |
| +} |
| +``` |
| + |
| +Derived is `key != origin`. `Raw` is `h.rawText[key]` (empty for a cleared |
| +or derived-formula-output cell is fine — the client repaints from `Value`, |
| +and only uses `Raw` to refill the editor for the cell you are on). |
| + |
| +- [ ] **Step 4: Run the whole store suite, including the race gate** |
| + |
| +Run: `go test -count=1 ./internal/sheets/store/... && make test-race` |
| +Expected: PASS. The refactor touches the locking path, so `-race` matters |
| +here more than anywhere else in this plan. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git add internal/sheets/store/handle_report.go \ |
| + internal/sheets/store/handle_report_test.go \ |
| + internal/sheets/store/handle_set.go |
| +git commit -m "store: a write reports what it changed, with the version to order it by" |
| +``` |
| + |
| +--- |
| + |
| +### Task 5: The fan-out hub |
| + |
| +Pure fan-out. No HTTP, no store, no templates — so it can be tested with |
| +goroutines and channels and nothing else, and `-race` over it means |
| +something. |
| + |
| +**Files:** |
| +- Create: `internal/sheets/live/hub.go`, `internal/sheets/live/hub_test.go` |
| + |
| +**Interfaces:** |
| +- Consumes: nothing. |
| +- Produces: |
| + |
| +```go |
| +type Message struct { |
| + Version uint64 |
| + Data []byte // an already-marshalled JSON payload; the hub never looks inside |
| +} |
| +type Sub struct{ /* ... */ } |
| +func NewHub() *Hub |
| +func (h *Hub) Subscribe(tabID uint64) *Sub |
| +func (h *Hub) Publish(tabID uint64, m Message) |
| +func (h *Hub) Subscribers(tabID uint64) int |
| +func (s *Sub) C() <-chan Message |
| +func (s *Sub) Lost() bool |
| +func (s *Sub) Close() |
| +``` |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Create `internal/sheets/live/hub_test.go`: |
| + |
| +```go |
| +package live |
| + |
| +import ( |
| + "testing" |
| + "time" |
| +) |
| + |
| +func TestPublishReachesEverySubscriberOnTheTab(t *testing.T) { |
| + h := NewHub() |
| + a := h.Subscribe(7) |
| + b := h.Subscribe(7) |
| + other := h.Subscribe(8) |
| + defer a.Close() |
| + defer b.Close() |
| + defer other.Close() |
| + |
| + h.Publish(7, Message{Version: 3, Data: []byte(`{"v":3}`)}) |
| + |
| + for i, s := range []*Sub{a, b} { |
| + select { |
| + case m := <-s.C(): |
| + if m.Version != 3 { |
| + t.Errorf("subscriber %d got version %d, want 3", i, m.Version) |
| + } |
| + case <-time.After(time.Second): |
| + t.Fatalf("subscriber %d received nothing", i) |
| + } |
| + } |
| + select { |
| + case m := <-other.C(): |
| + t.Fatalf("a subscriber on tab 8 received tab 7's message: %+v", m) |
| + case <-time.After(50 * time.Millisecond): |
| + } |
| +} |
| + |
| +func TestCloseUnsubscribes(t *testing.T) { |
| + h := NewHub() |
| + s := h.Subscribe(7) |
| + s.Close() |
| + if got := h.Subscribers(7); got != 0 { |
| + t.Fatalf("after Close, Subscribers(7) = %d, want 0", got) |
| + } |
| + // Close is idempotent: an SSE handler defers it and may also hit it |
| + // on a write error, and a double close of the channel would panic |
| + // in a goroutine no request owns. |
| + s.Close() |
| +} |
| + |
| +// A tab that has stopped reading must never block the writer. The |
| +// alternative — an unbounded buffer — turns one wedged browser into an |
| +// instance-wide memory leak. |
| +func TestASlowSubscriberIsDroppedNotBlocking(t *testing.T) { |
| + h := NewHub() |
| + s := h.Subscribe(7) |
| + defer s.Close() |
| + for i := 0; i < subBuffer+10; i++ { |
| + done := make(chan struct{}) |
| + go func() { h.Publish(7, Message{Version: uint64(i)}); close(done) }() |
| + select { |
| + case <-done: |
| + case <-time.After(time.Second): |
| + t.Fatalf("Publish blocked on message %d", i) |
| + } |
| + } |
| + if !s.Lost() { |
| + t.Fatal("an overrun subscriber is not marked lost; it would paint a sheet with holes in it") |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run it and watch it fail** |
| + |
| +Run: `go test ./internal/sheets/live/ -v` |
| +Expected: FAIL — the package does not exist. |
| + |
| +- [ ] **Step 3: Write the implementation** |
| + |
| +Create `internal/sheets/live/hub.go`. The shape: |
| + |
| +- `subBuffer = 64` — a named constant with the reasoning beside it: deep |
| + enough that a normal tab under a burst of edits never overruns, shallow |
| + enough that a thousand wedged tabs cost kilobytes, not megabytes. |
| +- `Hub` holds `mu sync.Mutex` and `tabs map[uint64]map[*Sub]struct{}`. |
| +- `Publish` does a non-blocking send per subscriber; a full channel sets |
| + `lost` (via `atomic.Bool`) and does NOT close the channel — the reader |
| + discovers it through `Lost()` and asks the client to resync. |
| +- `Close` removes the sub under the hub's lock, guarded by `sync.Once` so |
| + it is idempotent, and deletes the tab's map entry when it empties, so an |
| + instance that has served a thousand tabs does not keep a thousand empty |
| + maps. |
| + |
| +The hub never inspects `Data`. That is what keeps cell content out of this |
| +package entirely, and with it out of any log line this package could ever |
| +grow. |
| + |
| +- [ ] **Step 4: Run the tests, with the race detector** |
| + |
| +Run: `go test -race -count=1 ./internal/sheets/live/ -v` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 5: Add live to the race target** |
| + |
| +In `Makefile`, extend `test-race`: |
| + |
| +```make |
| +test-race: |
| + go test -race -count=1 ./internal/sheets/store/... ./internal/sheets/live/... |
| +``` |
| + |
| +and update the target's comment to say live is here for the same reason |
| +store is: it is where the goroutines are. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +git add internal/sheets/live/ Makefile |
| +git commit -m "live: an in-process fan-out hub that never blocks on a wedged tab" |
| +``` |
| + |
| +--- |
| + |
| +### Task 6: The view layer — templates, tokens, theme before first paint |
| + |
| +**Files:** |
| +- Create: `internal/sheets/render.go`, `internal/sheets/theme.go`, |
| + `internal/sheets/theme_test.go`, `internal/sheets/render_test.go` |
| +- Create: `internal/sheets/templates/layout.html`, `index.html`, `signin.html` |
| +- Create: `internal/sheets/static/tokens.css`, `internal/sheets/static/sheets.css` |
| +- Modify: `internal/sheets/page.go` (delete the string constants), `app.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `rastrillo/ui` (`Templates()`, `TokensCSS()`, `Layout()`). |
| +- Produces: |
| + - `type PageData struct{ Title, Theme, Address, Path string; SignedIn bool; CSRF string; Data any }` |
| + - `func (a *App) render(w http.ResponseWriter, r *http.Request, page string, status int, data any)` |
| + - `func themeFrom(r *http.Request) string` |
| + - `GET /static/*` serving the embedded assets with a long cache lifetime. |
| + |
| +- [ ] **Step 1: Scaffold the tokens with rastrillo doctor** |
| + |
| +```bash |
| +rastrillo doctor |
| +rastrillo doctor --fix |
| +``` |
| + |
| +Expected: it copies `ui/tokens.css` (and the partials it styles) into |
| +`internal/sheets/static/`. AGENTS.md requires this on every rastrillo |
| +upgrade; this is the first adoption, so it is the first run. Record its |
| +output in the branch discussion. |
| + |
| +- [ ] **Step 2: Write the failing theme test** |
| + |
| +Create `internal/sheets/theme_test.go`: |
| + |
| +```go |
| +package sheets |
| + |
| +import ( |
| + "net/http" |
| + "net/http/httptest" |
| + "testing" |
| +) |
| + |
| +func TestThemeDefaultsToDayWithNoCookie(t *testing.T) { |
| + r := httptest.NewRequest(http.MethodGet, "/", nil) |
| + if got := themeFrom(r); got != "day" { |
| + t.Fatalf("themeFrom with no cookie = %q, want %q", got, "day") |
| + } |
| +} |
| + |
| +func TestThemeReadsTheCookie(t *testing.T) { |
| + r := httptest.NewRequest(http.MethodGet, "/", nil) |
| + r.AddCookie(&http.Cookie{Name: themeCookie, Value: "signal"}) |
| + if got := themeFrom(r); got != "signal" { |
| + t.Fatalf("themeFrom = %q, want %q", got, "signal") |
| + } |
| +} |
| + |
| +// An unknown value must not reach the page. It is interpolated into a |
| +// stylesheet href and a data attribute; "day" is the safe floor. |
| +func TestThemeRejectsAnUnknownValue(t *testing.T) { |
| + r := httptest.NewRequest(http.MethodGet, "/", nil) |
| + r.AddCookie(&http.Cookie{Name: themeCookie, Value: "../../etc/passwd"}) |
| + if got := themeFrom(r); got != "day" { |
| + t.Fatalf("themeFrom with a junk cookie = %q, want the %q floor", got, "day") |
| + } |
| +} |
| +``` |
| + |
| +And a render test that pins the reason the cookie is read server-side: |
| + |
| +```go |
| +// AGENTS.md: light/dark must resolve before first paint, because cell |
| +// fills resolve against the theme and a late preference repaints every |
| +// coloured cell. A page that ships the theme in its first byte cannot |
| +// flash; one that fetches it can. This test fails the moment somebody |
| +// moves the decision into JS. |
| +func TestThemeIsInTheFirstResponseNotFetched(t *testing.T) { |
| + app := newTestApp(t) |
| + req := httptest.NewRequest(http.MethodGet, "/", nil) |
| + req.AddCookie(&http.Cookie{Name: themeCookie, Value: "signal"}) |
| + rec := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec, req) |
| + if !strings.Contains(rec.Body.String(), `data-theme="signal"`) { |
| + t.Fatal("the rendered page does not carry the theme; it would flash on load") |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 3: Run them and watch them fail** |
| + |
| +Run: `go test ./internal/sheets/ -run TestTheme -v` |
| +Expected: FAIL — `undefined: themeFrom`. |
| + |
| +- [ ] **Step 4: Write theme.go** |
| + |
| +```go |
| +package sheets |
| + |
| +import "net/http" |
| + |
| +// themeCookie is the per-person theme. Per person, not per document |
| +// (spec 9.3, AGENTS.md) — so it is a cookie on this origin and not a |
| +// column on Sheet. |
| +const themeCookie = "sheets_theme" |
| + |
| +// defaultTheme is the floor. rastrillo ships day, plain and signal; |
| +// anything else — a stale cookie, a hand-edited one, an attempt at path |
| +// traversal through the stylesheet href — resolves to this rather than |
| +// reaching a template. |
| +const defaultTheme = "day" |
| + |
| +func themeFrom(r *http.Request) string { |
| + c, err := r.Cookie(themeCookie) |
| + if err != nil { |
| + return defaultTheme |
| + } |
| + for _, name := range ui.ThemeNames() { |
| + if c.Value == name { |
| + return c.Value |
| + } |
| + } |
| + return defaultTheme |
| +} |
| +``` |
| + |
| +Plus `handleTheme` (POST `/theme`), which validates against |
| +`ui.ThemeNames()`, sets the cookie `HttpOnly=false, SameSite=Lax, |
| +Secure` (on an https origin), `Max-Age` one year, and redirects back to the |
| +`return` form field — validated as a same-origin *path*, never an absolute |
| +URL, or the theme switcher becomes an open redirect. |
| + |
| +- [ ] **Step 5: Write render.go and the templates** |
| + |
| +`render.go` embeds `templates/*.html` and `static/*`, parses one |
| +`*template.Template` per page over rastrillo's layout plus partials (so two |
| +pages can both define `"content"`), and builds `PageData` with theme, |
| +identity and the CSRF token. It serves `/static/*` from the embedded FS. |
| + |
| +Templates use the **attribute grammar** throughout — |
| +`rst-btn="primary"`, `rst-callout="warn"`, `rst-field` — never the class |
| +spelling, and adopt it per screen (AGENTS.md). |
| + |
| +`layout.html` puts `data-theme="{{.Theme}}"` on `<html>` and links |
| +`/static/tokens.css` before anything else. |
| + |
| +- [ ] **Step 6: Move page.go onto templates** |
| + |
| +Delete `pageHead`/`pageWhat`/`pageSignedOut`/`pageFoot` from `page.go` and |
| +render `index.html` instead. `page_test.go` and `signin_test.go` assert on |
| +strings from those constants — update them to assert on the rendered page's |
| +affordances (a sign-in link when signed out, the address and a sign-out |
| +button when signed in), which is what they were really pinning. |
| + |
| +- [ ] **Step 7: Run the tests** |
| + |
| +Run: `make test-vet test-fmt test-build test-go` |
| +Expected: PASS. |
| + |
| +- [ ] **Step 8: Commit** |
| + |
| +```bash |
| +git add internal/sheets/render.go internal/sheets/theme.go \ |
| + internal/sheets/templates internal/sheets/static \ |
| + internal/sheets/page.go internal/sheets/*_test.go |
| +git commit -m "feat: a view layer — rastrillo tokens, one layout, theme before first paint" |
| +``` |
| + |
| +--- |
| + |
| +### Task 7: The sheet list, scriptless |
| + |
| +**Files:** |
| +- Create: `internal/sheets/handlers_sheets.go`, |
| + `internal/sheets/handlers_sheets_test.go`, |
| + `internal/sheets/templates/sheets.html` |
| +- Modify: `internal/sheets/app.go` (routes, `auth.RequireSession`, CSRF) |
| + |
| +**Interfaces:** |
| +- Consumes: `createSheet`, `listSheets`, `sheetSummary`, `actor`, `render`. |
| +- Produces: `GET /sheets`, `POST /sheets`, and `App.requireSession` — a |
| + middleware that is `a.auth.RequireSession` in production and a pass-through |
| + when `a.auth` is nil, so page tests keep working without a mailer. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +```go |
| +func TestPostSheetsCreatesAndRedirects(t *testing.T) { |
| + app := newTestApp(t) |
| + form := url.Values{"title": {"Q3 forecast"}} |
| + req := httptest.NewRequest(http.MethodPost, "/sheets", strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + rec := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec, req) |
| + |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("POST /sheets = %d, want 303 — a POST that renders is a POST that resubmits on reload", rec.Code) |
| + } |
| + loc := rec.Header().Get("Location") |
| + if !strings.HasPrefix(loc, "/sheets/") { |
| + t.Fatalf("Location = %q, want the new sheet", loc) |
| + } |
| +} |
| + |
| +func TestSheetListShowsEveryTitle(t *testing.T) { /* create two, GET /sheets, assert both titles appear */ } |
| + |
| +func TestUnknownSheetIs404NotForbidden(t *testing.T) { |
| + app := newTestApp(t) |
| + req := httptest.NewRequest(http.MethodGet, "/sheets/999999", nil) |
| + rec := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec, req) |
| + if rec.Code != http.StatusNotFound { |
| + t.Fatalf("GET a sheet that is not there = %d, want 404 (AGENTS.md: 404, never 403)", rec.Code) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run and watch it fail.** Expected: 404 on `/sheets`. |
| + |
| +- [ ] **Step 3: Write the handlers and the template.** `POST` reads the |
| + title with `r.FormValue` into an explicit struct — never a bind onto a |
| + GORM model (AGENTS.md) — resolves the actor, creates, and 303s to |
| + `/sheets/{id}`. |
| + |
| +- [ ] **Step 4: Mount the routes behind a session** |
| + |
| +In `app.go`, inside `r.Group(func(r chi.Router) { r.Use(a.requireSession) })`. |
| +Add `r.Use(csrf.Protect(origin))` at the router root; `App` gains the origin |
| +string (`New` grows a parameter, or it reads it from the `auth.Auth` — |
| +prefer passing it explicitly so a nil-auth test App still has one). |
| +Existing `POST /signin` tests will need the CSRF headers — update them. |
| + |
| +- [ ] **Step 5: Run `make test`.** Expected: PASS. |
| + |
| +- [ ] **Step 6: Commit** |
| + |
| +```bash |
| +git commit -am "feat: create and list sheets, no script required" |
| +``` |
| + |
| +--- |
| + |
| +### Task 8: The grid JSON API |
| + |
| +**Files:** |
| +- Create: `internal/sheets/handlers_grid.go`, |
| + `internal/sheets/handlers_grid_test.go` |
| +- Modify: `internal/sheets/app.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `store.Open`, `store.Handle.SetReport`, `format.Display`, |
| + `App.actor`, `App.hub` (Task 5's hub, held on `App`). |
| +- Produces: |
| + - `GET /api/sheets/{sheetID}/tabs/{tabID}/cells?rows=100&cols=26` → |
| + `{"version":N,"rows":100,"cols":26,"cells":[{"r":0,"c":0,"raw":"=A1*2","display":"42","v":7}]}` |
| + - `POST /api/sheets/{sheetID}/tabs/{tabID}/cells` with |
| + `{"r":0,"c":0,"raw":"=A1*2"}` → the same `cells` shape for everything |
| + the write changed, and a fan-out of that payload on the hub. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +```go |
| +func TestPostACellThenGetItBack(t *testing.T) { |
| + app := newTestApp(t) |
| + m, _ := app.memberFor("ada@example.com") |
| + sheet, _ := app.createSheet("Budget", m) |
| + |
| + body := `{"r":0,"c":0,"raw":"=1+1"}` |
| + post := httptest.NewRequest(http.MethodPost, |
| + fmt.Sprintf("/api/sheets/%d/tabs/%d/cells", sheet.ID, sheet.TabID), |
| + strings.NewReader(body)) |
| + post.Header.Set("Content-Type", "application/json") |
| + rec := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec, post) |
| + if rec.Code != http.StatusOK { |
| + t.Fatalf("POST cell = %d: %s", rec.Code, rec.Body.String()) |
| + } |
| + var wrote gridResponse |
| + json.Unmarshal(rec.Body.Bytes(), &wrote) |
| + if len(wrote.Cells) != 1 || wrote.Cells[0].Display != "2" { |
| + t.Fatalf("POST returned %+v, want one cell displaying 2", wrote.Cells) |
| + } |
| + |
| + get := httptest.NewRequest(http.MethodGet, /* same path */, nil) |
| + rec2 := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec2, get) |
| + var read gridResponse |
| + json.Unmarshal(rec2.Body.Bytes(), &read) |
| + // The client edits raw and paints display. Both must survive a reload. |
| + if read.Cells[0].Raw != "=1+1" || read.Cells[0].Display != "2" { |
| + t.Fatalf("after reload: %+v", read.Cells[0]) |
| + } |
| +} |
| + |
| +// Spec 6's cap reasoning, enforced at the boundary rather than trusted. |
| +func TestGridWindowIsCapped(t *testing.T) { /* rows=999999 must clamp, not allocate */ } |
| + |
| +// The one rule, at the last layer that could break it. |
| +func TestNoCellContentInAnErrorResponse(t *testing.T) { |
| + // POST an unparseable formula containing a distinctive string; |
| + // assert the 400 body does not contain it. |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run and watch it fail.** |
| + |
| +- [ ] **Step 3: Write the handlers.** Key points, each with the reasoning in |
| + a comment: |
| + - Open the handle once per request via `store.Open`, `defer h.Release()`. |
| + - Window: `rows` and `cols` clamp to `maxWindowRows = 200` / |
| + `maxWindowCols = 50`. A request for more is clamped, not refused — |
| + refusing turns a wide window into a broken page. |
| + - `display` comes from `format.Display(value, numFmt)` — the server owns |
| + every string the grid paints, which is the §12 gate that crosses from |
| + computed value to painted output. |
| + - The POST resolves row/col ids via `h.EnsureRow` / `h.EnsureCol`, so |
| + typing into an untouched cell works. |
| + - On success, marshal the same payload once and `hub.Publish` it, tagged |
| + with an `origin` client id the poster sends so its own echo is skipped. |
| + - Errors carry coordinates and codes, never the cell's text. |
| + |
| +- [ ] **Step 4: Run `make test`.** Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git commit -am "feat: read a grid window and write a cell, with the server owning every painted string" |
| +``` |
| + |
| +--- |
| + |
| +### Task 9: SSE — the endpoint and the transport gate |
| + |
| +**Files:** |
| +- Create: `internal/sheets/handlers_stream.go`, |
| + `internal/sheets/handlers_stream_test.go` |
| +- Modify: `internal/sheets/app.go` |
| + |
| +**Interfaces:** |
| +- Consumes: `App.hub`. |
| +- Produces: `GET /api/sheets/{sheetID}/tabs/{tabID}/stream`. |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +Spec §7: *"The SSE transport check is a streamed 200, not a 101."* |
| + |
| +```go |
| +func TestStreamIsAStreamed200NotAnUpgrade(t *testing.T) { |
| + app := newTestApp(t) |
| + srv := httptest.NewServer(app.Router()) |
| + defer srv.Close() |
| + m, _ := app.memberFor("ada@example.com") |
| + sheet, _ := app.createSheet("Budget", m) |
| + |
| + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) |
| + defer cancel() |
| + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, |
| + fmt.Sprintf("%s/api/sheets/%d/tabs/%d/stream", srv.URL, sheet.ID, sheet.TabID), nil) |
| + resp, err := http.DefaultClient.Do(req) |
| + if err != nil { |
| + t.Fatalf("stream: %v", err) |
| + } |
| + defer resp.Body.Close() |
| + if resp.StatusCode != http.StatusOK { |
| + t.Fatalf("stream = %d, want a streamed 200 (SSE is text/event-stream, not a 101 upgrade)", resp.StatusCode) |
| + } |
| + if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "text/event-stream") { |
| + t.Fatalf("Content-Type = %q, want text/event-stream", ct) |
| + } |
| + |
| + // Unbuffered: the first event must arrive without the response |
| + // having to end. A handler that builds its body and returns passes |
| + // every status check above and is useless. |
| + go func() { |
| + time.Sleep(100 * time.Millisecond) |
| + post(t, srv.URL, sheet, `{"r":0,"c":0,"raw":"5"}`) |
| + }() |
| + line := readEventWithin(t, resp.Body, 3*time.Second) |
| + if !strings.Contains(line, `"display":"5"`) { |
| + t.Fatalf("first streamed event = %q", line) |
| + } |
| +} |
| +``` |
| + |
| +- [ ] **Step 2: Run and watch it fail.** |
| + |
| +- [ ] **Step 3: Write the handler.** The shape: |
| + - Headers: `Content-Type: text/event-stream`, `Cache-Control: no-store`, |
| + `X-Accel-Buffering: no` (the edge may buffer otherwise, which is the |
| + failure the streamed-200 test is written to catch), then |
| + `http.NewResponseController(w).Flush()` immediately so the client's |
| + `onopen` fires before the first edit. |
| + - Subscribe, `defer sub.Close()`, and select over `sub.C()`, |
| + `r.Context().Done()`, a `time.Ticker` writing a `: ping` comment every |
| + `pingEvery = 25 * time.Second`, and an idle timer. |
| + - **Idle policy, written down** (spec §7): `idleTimeout = 15 * time.Minute` |
| + with no edits and no heartbeat closes the stream; `maxStreamLife = 4 * time.Hour` |
| + closes it regardless. A tab left open all weekend cannot hold an instance |
| + awake and billable. The client reconnects on its own — `EventSource` |
| + does that natively — so a close is a pause, not a failure. |
| + - `sub.Lost()` sends a single `event: resync` and returns, so an overrun |
| + client reloads its window instead of painting a sheet with holes. |
| + |
| +- [ ] **Step 4: Run `make test test-race`.** Expected: PASS. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git commit -am "feat: SSE fan-out, with an idle policy written down rather than emergent" |
| +``` |
| + |
| +--- |
| + |
| +### Task 10: The grid in the browser |
| + |
| +**Files:** |
| +- Create: `internal/sheets/static/vendor/jspreadsheet.{js,css}`, |
| + `internal/sheets/static/vendor/jsuites.{js,css}` |
| +- Create: `internal/sheets/static/grid.js`, `internal/sheets/static/live.js` |
| +- Create: `internal/sheets/templates/sheet.html` |
| +- Create: `internal/guardstest/jscap_test.go` |
| + |
| +- [ ] **Step 1: Vendor the assets** |
| + |
| +```bash |
| +D=$(mktemp -d) |
| +curl -sSL -o $D/js.tgz https://registry.npmjs.org/jspreadsheet-ce/-/jspreadsheet-ce-5.0.2.tgz |
| +curl -sSL -o $D/su.tgz https://registry.npmjs.org/jsuites/-/jsuites-5.10.2.tgz |
| +tar xzf $D/js.tgz -C $D && tar xzf $D/su.tgz -C $D --one-top-level=su |
| +install -D $D/package/dist/index.js internal/sheets/static/vendor/jspreadsheet.js |
| +install -D $D/package/dist/jspreadsheet.css internal/sheets/static/vendor/jspreadsheet.css |
| +install -D $D/su/package/dist/jsuites.js internal/sheets/static/vendor/jsuites.js |
| +install -D $D/su/package/dist/jsuites.css internal/sheets/static/vendor/jsuites.css |
| +install -D $D/package/LICENSE internal/sheets/static/vendor/LICENSE.jspreadsheet |
| +install -D $D/su/package/LICENSE internal/sheets/static/vendor/LICENSE.jsuites |
| +``` |
| + |
| +Both are MIT. Copy the licences in beside the code, and record the exact |
| +versions in `internal/sheets/static/vendor/README.md` along with the |
| +commands above, so the next upgrade is a re-run rather than an |
| +investigation. |
| + |
| +- [ ] **Step 2: Write the JS line-cap guard first** |
| + |
| +Create `internal/guardstest/jscap_test.go`, mirroring `linecap_test.go`: |
| +walk `internal/sheets/static/**/*.js`, cap at 300 lines, with an |
| +exemption map holding exactly the two vendored blobs — by exact path, |
| +never by directory, so a third file dropped into `vendor/` is not silently |
| +exempt. |
| + |
| +- [ ] **Step 3: Run it and watch it fail** (the vendored files exceed the cap |
| + until they are listed). Then add the two exemptions and watch it pass. |
| + |
| +- [ ] **Step 4: Write `grid.js`** |
| + |
| +Under 300 lines. Responsibilities, and nothing else: |
| +- Build the Jspreadsheet instance over the window the page was rendered with. |
| +- **`parseFormulas` off.** Our engine computes; the client paints. A |
| + client-side evaluator would disagree with the server on the first |
| + `SUM(1,2,"3")` — the LibreOffice/Excel divergence the corpus already |
| + documents — and the disagreement would be invisible. |
| +- Paint `display`; keep `raw` in a side map so selecting a cell puts the |
| + formula, not its result, in the editor. |
| +- On `onchange`, POST the raw text and apply the response. |
| +- Menu equivalents for every drag-only affordance (AGENTS.md): fill-down |
| + and fill-right live in the toolbar as buttons, not only on the drag |
| + handle. |
| + |
| +- [ ] **Step 5: Write `live.js`** |
| + |
| +Under 300 lines. `EventSource` on the stream; on message, **discard by |
| +version** — `if (msg.v <= lastAppliedVersionFor(cell)) return;` — which is |
| +the whole reason the version travels (spec §7). Skip our own echo by client |
| +id. Heartbeat POST every 30s, **stopped while `document.hidden`**, with a |
| +**ceiling** of 4 hours, matching the server's `maxStreamLife`. On |
| +`event: resync`, refetch the window. |
| + |
| +- [ ] **Step 6: Write `sheet.html`** |
| + |
| +The grid page. Its `<noscript>` block links to `/sheets/{id}/table`, so a |
| +scriptless visitor lands somewhere useful rather than on an empty div. |
| + |
| +- [ ] **Step 7: Run `make test`, then drive it in a real browser** |
| + |
| +Two tabs, same sheet, type in one, watch the other. This is the acceptance |
| +criterion for the whole plan and it cannot be asserted in Go. |
| + |
| +- [ ] **Step 8: Commit** |
| + |
| +```bash |
| +git add internal/sheets/static internal/sheets/templates/sheet.html \ |
| + internal/guardstest/jscap_test.go |
| +git commit -m "feat: the grid, painting strings the server computed" |
| +``` |
| + |
| +--- |
| + |
| +### Task 11: The scriptless table, and the affordance gate |
| + |
| +Spec §6: a **read-only table view of any sheet with per-cell edit forms**, |
| +paginated. And: *"No form in the app is script-required. A test fetches every |
| +page script-stripped and diffs for every affordance."* |
| + |
| +**Files:** |
| +- Create: `internal/sheets/handlers_table.go`, |
| + `internal/sheets/handlers_table_test.go`, |
| + `internal/sheets/templates/table.html` |
| + |
| +- [ ] **Step 1: Write the failing test** |
| + |
| +```go |
| +// The gate spec 6 asks for, made concrete: every write the grid can do, |
| +// the scriptless view can do too — proven by actually doing it with no |
| +// JavaScript anywhere in the request. |
| +func TestACellCanBeEditedWithNoScriptAtAll(t *testing.T) { |
| + app := newTestApp(t) |
| + m, _ := app.memberFor("ada@example.com") |
| + sheet, _ := app.createSheet("Budget", m) |
| + |
| + form := url.Values{"r": {"0"}, "c": {"0"}, "raw": {"=2*21"}} |
| + req := httptest.NewRequest(http.MethodPost, |
| + fmt.Sprintf("/sheets/%d/table", sheet.ID), strings.NewReader(form.Encode())) |
| + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + rec := httptest.NewRecorder() |
| + app.Router().ServeHTTP(rec, req) |
| + if rec.Code != http.StatusSeeOther { |
| + t.Fatalf("scriptless cell edit = %d, want 303", rec.Code) |
| + } |
| + |
| + page := get(t, app, fmt.Sprintf("/sheets/%d/table", sheet.ID)) |
| + if !strings.Contains(page, ">42<") { |
| + t.Fatal("the scriptless table does not show the value it just computed") |
| + } |
| +} |
| + |
| +func TestTableViewIsPaginated(t *testing.T) { |
| + // 200,000 cells rendered as one HTML table is a several-hundred- |
| + // megabyte page (spec 6). Assert a page renders at most |
| + // tableRowsPerPage rows and offers a next link. |
| +} |
| + |
| +func TestEveryPageWorksWithoutScript(t *testing.T) { |
| + // For each page path: fetch, strip every <script>...</script> and |
| + // every on* attribute, and assert at least one <form> or <a href> |
| + // remains for each affordance the page claims to offer. |
| +} |
| +``` |
| + |
| +- [ ] **Step 2–4:** run, implement, run. `tableRowsPerPage = 50`. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git commit -am "feat: a scriptless table view that can edit any cell" |
| +``` |
| + |
| +--- |
| + |
| +### Task 12: An imported fill that fails WCAG AA gets surfaced |
| + |
| +Spec §14's deferred table has one row whose trigger is *"plan 2's grid |
| +work"*, cross-referencing §11.1. This is that trigger firing. §11.1 already |
| +ruled that a retained fill can be unreadable and that this is **correct** — |
| +so the work is to *surface* it, not to fix it. |
| + |
| +**Files:** |
| +- Modify: `internal/sheets/handlers_grid.go` (add `contrastWarn` to the |
| + cell payload), `internal/sheets/static/grid.js`, `templates/table.html` |
| +- Create: `internal/sheets/contrast_test.go` |
| +- Modify: `docs/superpowers/specs/2026-08-30-sheets-design.md` (§14: strike |
| + the row, since its trigger has fired and it is now built) |
| + |
| +- [ ] **Step 1: Read §11.1 and §10.4 in full before writing anything.** The |
| + ruling is subtle and the test must encode the ruling, not a guess at it. |
| + |
| +- [ ] **Step 2: Write the failing test.** A cell whose imported fill and ink |
| + fall below AA reports `contrastWarn: true`; one that passes does not. Use |
| + rastrillo's own contrast helper if `ui` exposes one — check |
| + `ui/contrast_test.go` for the function it tests — rather than writing a |
| + second WCAG implementation that can disagree with the framework's. |
| + |
| +- [ ] **Step 3–4:** implement, run. |
| + |
| +- [ ] **Step 5: Commit** |
| + |
| +```bash |
| +git commit -am "feat: surface an imported fill that fails AA, per spec 11.1's trigger" |
| +``` |
| + |
| +--- |
| + |
| +### Task 13: Deploy to the canary and verify the process |
| + |
| +**Files:** none. This is the step AGENTS.md's Deploys section exists for. |
| + |
| +- [ ] **Step 1: Confirm the tree is clean and CI is green** |
| + |
| +```bash |
| +git status --porcelain # must be empty |
| +make test # all six targets |
| +``` |
| + |
| +- [ ] **Step 2: Set the new secret** |
| + |
| +`SHEETS_DEK` did not exist before Task 1, so the instance will refuse to |
| +boot without it: |
| + |
| +```bash |
| +head -c 32 /dev/urandom | base64 |
| +carlos secrets set --app sheets SHEETS_DEK=<that value> |
| +``` |
| + |
| +- [ ] **Step 3: Deploy** |
| + |
| +```bash |
| +make release |
| +carlos deploy --app sheets --channel edge |
| +``` |
| + |
| +`carlos deploy`, never `carlos ship` — `ship` publishes a release and stops, |
| +leaving it unpromoted and unserved. |
| + |
| +- [ ] **Step 4: Verify the process, not the edge, by polling** |
| + |
| +```bash |
| +want=$(git rev-parse --short HEAD) |
| +for i in $(seq 1 40); do |
| + got=$(curl -sS https://sheets.bds.oncarlos.com/api/version | tr -d '"') |
| + echo "$i: $got (want $want)" |
| + [ "$got" = "$want" ] && break |
| + sleep 10 |
| +done |
| +``` |
| + |
| +A promote is not a deploy: the old binary served for over 90 seconds on |
| +three consecutive deploys here. `X-Carlos-Version` is the router's adopted |
| +release and says nothing about which binary is running — do not check it. |
| + |
| +- [ ] **Step 5: Drive it in two real browser tabs on the canary.** Type in |
| + one, watch the other. Review happens on a deployed canary, never |
| + localhost. |
| + |
| +- [ ] **Step 6: Open the branch discussion and record the review model** |
| + |
| +Every branch gets a discussion, and that discussion IS the review record, |
| +including which model reviewed it (AGENTS.md §Review). Record the |
| +`rastrillo doctor` output from Task 6 there too. |
| + |
| +--- |
| + |
| +## Self-Review |
| + |
| +**Spec coverage.** §6: grid vendored, no build step (Task 10); scriptless |
| +list, table view with per-cell edit forms, pagination, the affordance gate |
| +(Tasks 7, 11). §7: cell-level LWW over SSE (Tasks 5, 8, 9); derived cells |
| +carry the actor and a `derived` flag (Task 4); every message carries the |
| +version and clients discard by it (Tasks 4, 9, 10); streamed-200 transport |
| +check (Task 9); idle policy with hidden-tab heartbeats and a ceiling (Tasks |
| +9, 10). Presence and undo are §7 and are **deferred with triggers**, stated |
| +in Assumptions. §10: attribute grammar, per-screen adoption, tokens frozen |
| +via `doctor`, theme before first paint (Task 6). §11.1/§14: the contrast |
| +trigger (Task 12). |
| + |
| +**Gaps I am naming rather than hiding.** §7's presence and undo are not |
| +built. §8's sharing is not built, so access is instance-wide. §13 sequences |
| +sharing before live sync; this plan inverts that, for the reason in |
| +Assumptions 2. |
| + |
| +**Placeholders.** Tasks 7, 8, 11 and 12 give test names and the reasoning |
| +each test encodes, but leave some bodies as a described assertion rather |
| +than finished Go. That is a deliberate density trade at the tail of a long |
| +plan, and it is a real weakness of this document: the executor writes those |
| +bodies. Every task's *first* test — the one that drives the design — is |
| +complete code. |
| + |
| +**Type consistency.** `Report`/`ChangedCell` (Task 4) are what Task 8 |
| +marshals and Task 9 fans out. `SheetSummary` (Task 3) is what Tasks 7, 8 and |
| +11 read. `Member` (Task 2) is the actor everywhere. `themeFrom`/`PageData` |
| +(Task 6) are what every template consumes. |