Rebase onto main, so the correction lands on the text that is actually there
The sixth review's one block. This branch was cut from 462df84 and the
AGENTS.md correction was written against that text, while main's
5a42f2d had since rewritten the same Data bullet with its own
auth_sessions fix. git merge-tree showed a content conflict, so the
branch could not be squash-merged at all without a resolution commit —
and that commit would have been unreviewed text in a binding file.
Rebased. The bullet now carries both halves: main's auth_sessions
correction and this branch's signin-not-auth one, with auth.New's
hardwired Links: &linkStore{db: cfg.DB} named as the reason "use auth
only for the parts that hold nothing" was never possible. merge-tree
against main is clean.
The plan still said in three places that the correction was pending —
including its first architectural sentence, which an executor reads
before anything else and which was false at source. All three now say
it has landed.
Also fixed a markdown break I introduced on main: reordering the
rulings table left a blank line between rows 20 and 21, which ends the
table, so ruling 21 rendered as a paragraph rather than a row. It is a
one-line fix and it matters, because §1 is the file people read to find
out what has been decided.
Follow-ups: Task 2's git add lacked app.go and testapp_test.go and Task
4's lacked go.mod, so both intermediate commits would have failed to
build; render.go is created in Task 5 and modified in Task 7, rather
than created twice; CanaryAddress must be IN Canaries, or gate 2 signs
in with a value AssertNoPlaintext never scans for and passes while
proving nothing; scriptSrcs has one return, not two; the App must
retain the passkey.Config it passed to passkey.New, since Handlers
keeps it unexported and passkeyConfig has to hand back what the app
built; and the File Structure, a stranded table row and an empty code
fence.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs/superpowers/plans/2026-09-03-home-signin.md+23 −19docs/superpowers/specs/2026-08-30-home-design.md+0 −1
CI passed — run details
Intent
<task-notification> <task-id>acb7193a90855ef50</task-id> <tool-use-id>toolu_01WSRH9sw3WT6BzGZUjTn6RR</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-home/d63f5ac7-6b42-48c5-a13d-34e6faf8e44c/tasks/acb7193a90855ef50.output</output-file> <status>completed</status> <summary>Agent "Fable review: activity-pull design" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>I have what I need. Everything below was checked against the branch tip, main at `5a42f2d`, the Sheets spec, rastrillo v0.22.0 in the module cache, and the platform source. **Reviewer:** Fable 5.1 (`claude-fable-5-1`) — record this in the status line if the spec is revised. **Preliminary:** the task named one commit `6833731`; `origin/design-activity-pull` has three (`6833731`, `92ddcc1`, `c9274bf`), adds two review documents under `docs/superpowers/reviews/`, and is cut from `462df84`, three commits behind main. I reviewed the tip. # VERDICT: CHANGES NEEDED Paul's ruling settles the shape (pull, not push; titles; nothing at rest; step 10). The findings below are what the shape does not settle. 1–6 are blocking; 7–14 are required corrections that are cheap. ## Blocking **1. The spec is written against a §11 that no longer exists, and it does not say whether the feed is cross-team.** §8: *"§11 stands: your teams grouped by app, and a New button."* On main, ruling 17 replaced that page: *"one page: a grid of apps, with a team switcher as a dropdown in the header. You pick a team above; the grid below is that team's apps"*, and *"The selection lives in the URL (`?team=…`)"*. The whole justification in §2.1 — *"only the home can answer 'my things' rather than 'this team's things'"* — presumes a cross-team page. Either the feed follows `?team=` (then it is a per-team merged list, finding 1 of the verdict returns, and the fan-out is 2 hosts not 6) or it spans every team on a page whose grid is one team's. The spec must choose, and rebase on main. **2. §7 contradicts itself, and the losing half is the safety claim.** §7: *"The TTL is short enough that a stale row is a curiosity, not a lie… bounded by a number we choose, not by a hibernating fleet's wake schedule."* Then: *"A failed pull never evicts a good cached answer. It renders the cached rows plus the failure marker"*, and §8's marker *"showing what we had at 09:14"*. If cached rows survive a failed pull, a person removed from a team whose instance is asleep keeps seeing its titles until the instance next *answers* — which is the hibernation tail the sentence before says it avoids. Specify a hard maximum age after which rows are discarded regardless of pull outcome, distinct from the refresh TTL, and make gate 13 drive it. **3. §10's 404 rule is ruling 5 in reverse.** §10: *"a `404` from an instance is 'this app does not participate', rendered as nothing rather than as a failure."* Ruling 5: *"An instance that did not answer must be visually distinguishable from an instance with nothing to say."* A participating instance that loses its route in a deploy, or a hostile one, returns 404 and vanishes silently from every front door — exactly *"a plausible empty list, not an error"*. And 404 is already overloaded: §5.1 uses it for "no such thing", §8.1 for a dead host rendered *"unavailable"*. Participation must be declared, not inferred — a compiled-in registry flag beside `integrated` (main §11) — so a 404 from a participating app is a gap. Gate 11 should include this case. **4. The platform edge is a shared cache upstream of both ends, and the spec's cache-header sentence does not reach it.** Verified at `carlos/platform/internal/edge/edge.go`: the edge caches any 2xx GET whose response carries a positive `max-age`, keyed by `{Host, Method, Path, RawQuery}`, shared across every visitor of a host. Its refusals are `private`, `Set-Cookie`, `Vary: Cookie`, `Vary: *`, and *"an Authorization header on the REQUEST… unless Cache-Control explicitly says public"*. It does **not** refuse on a request `Cookie`. Two consequences the spec must handle: - The front door (`oficina.works/?team=acme`, session cookie) would be served to the next visitor if it ever carried `max-age` without `private`. Rastrillo sets no default `Cache-Control` on pages, so today the door is safe by omission, which is not a mechanism. Require `Cache-Control: no-store` on the front door and assert it in a gate. - `/activity` is answered by a consumer-supplied instance behind the same edge. An instance emitting `Cache-Control: public, max-age=60` has the edge serve person A's rows to the home's next pull for person B with the same query string. The home's own cache key (*"person ref and instance host"*) cannot see this. Call 7's contract must require `no-store` from instances, the home must treat a cacheable response as a failed pull, and gate 15 should include it. **5. §2.1 understates what process memory holds, and §3.1's second threat model is not re-priced.** §2.1: *"the home's process memory holds a cross-app list of titles for one person."* The cache holds it for every person who rendered within the TTL — a cross-person, cross-app set of recent titles, resident in the one process that also mints sessions. §3.1's live-host model already concedes the pepper; this adds every active person's recent titles, which §11 calls out as never licensed (*"no cross-person aggregation"*) as a feature while building it as a data structure. Write it into §2.1 and §3, bound the cache (max entries, max bytes per person), and give the TTL a ceiling in the spec — see 6. Also name the platform facts the "in RAM" claim leans on: hibernation kills the process (`activator.go`: SIGTERM then SIGKILL, so instances hold nothing), the home runs under `MemoryMax=1G`/`MemoryHigh=920M` (`caps/render.go`) with reclaim before kill, I found no swap configuration either way in `platform/infra`, and the unit template sets no `LimitCORE`. Either require core dumps off and no swap, or record them as accepted. **6. The open numbers: two can wait for measurement, two cannot.** §12: *"The per-instance deadline, the total fan-out budget, the RAM cache TTL and the body cap are all named as decisions and none is chosen."* The deadline and body cap are tunables with specified behaviour at the limit. The TTL is load-bearing for §7's "curiosity, not a lie" and needs at least a ceiling. The fan-out budget has **no behaviour specified** when exceeded: a person with forty guest rows either gets forty wakes or some instances go un-asked, and an un-asked instance must render as a gap of its own kind or ruling 5 is breached silently. Specify the behaviour; the number can follow. ## Required corrections **7. Fan-out is a cost lever on other teams' bills, and refresh makes it deliberate.** §6: *"A person who opens the front door twenty times a day keeps six instances awake."* Instance-hours are billed to the instance's account (hibernation spec §1). A guest with one row in `acme.docs` wakes Acme's instance on every front-door load, and the refresh link presumably bypasses the cache. Add a per-(person, instance) minimum pull interval enforced even on refresh, and disclose the residency cost in §6. **8. Who mints the call-7 assertion, and where the key lives.** §4 puts *"assertion minting"* on the credential origin and has the directory *obtain* assertions like any consumer. The front door is on the directory origin and now mints N assertions per render. Say whether this is one process serving two hostnames (then the key exposure is unchanged and should be stated) or a mint call across origins. The directory origin is also the surface that now parses N consumer-supplied JSON bodies, so the answer matters. **9. Client hardening is unspecified and Go's defaults are wrong for it.** `http.DefaultClient` has no timeout and follows redirects. Require: the deadline is a context covering connect, TLS, headers and full body read; **redirects are never followed** (a 3xx is a failed pull — an instance redirecting to a document URL otherwise puts that URL into a `net/http` error string, which §3.4 rules is storage); TLS verification on; bounded concurrency across all in-flight renders so one instance that trickles bytes cannot exhaust the process that also serves sign-in (§2.1 SPOF). Gate 15 lists no redirect case. **10. Gate 10 inherits gate 9's blind spot and should say so.** Main's `internal/home/logs_test.go`: *"Rastrillo resolves slog.Default() at construction, which is why its own machinery is outside this gate."* Rastrillo's `recoverPanics` (`serve.go:438`) logs the panic value verbatim. A panic that carries a response body is outside gate 10 as extended. Either route the fan-out through a logger the test owns or rule that call-7 code never panics with a value derived from a body, and add address-shaped and title-shaped `actor` strings to the canary set. Also state that a failed-pull log line may carry the instance host (permitted by home §9's *"team sqid and consumer origin, never either name"*) and nothing else. **11. §0's prose form is amended whether the spec says so or not.** §3: *"§0 is unamended."* The falsifiable form survives — verified, nothing new at rest. But §0's sentence *"it never learns what is in anything"* is now read as "never retains", and Sheets §2.1 seals titles as content. Write that reading into §0 (or its gloss) rather than leaving the prose and the design to disagree. **12. The replacement clause should bind the audience, not just the origin.** Proposed §2.2: *"It may render, transiently and per request, what an instance hands it for the person it hands it for."* Add: *only to that person*. That one word is what keeps the operator-view and support-console prohibitions inside the clause rather than in §11's table. **13. `since` must never be persisted.** It can only come from the RAM cache. A future "remember last pull per (person, instance) across restarts" is the contentless activity table §11 forbids, arriving as an optimisation. One sentence in §4 or §7. **14. Stale claims to fix on rebase.** §12 bullet 2 (*"AGENTS.md is stale against the home spec on three counts"*) is false on main — `5a42f2d` carries all three retractions (AGENTS.md lines 28–31 and 143). The peer-pass paragraph and §2's "Both sources are in this repo" are true only on this branch. Discussions #1 and #6 could not be checked (the `amadan` CLI has no discussions verb). ## Verified and sound - Escritorio brief and verdict are on the branch; every quotation in §2 matches the verdict verbatim, including the confirmed/suspected markings and the ellipsis in the recommendation quote. - Sheets citations: §2.1 seals titles (line 65), §8.3 *"A view link mints nothing"* (476), §9.1.1 *"wait weeks for an organic wake"* (542), §10.2 recency list (781). - `docs/step-2-signin-plan` exists and its plan does block on `HOME_DEK` provisioned by Paul. - §5's audience argument is correct: `aud` is app plus host, per-verifier `jti` stores suffice, and a hostile instance can only lie about its own content to the person whose assertion fetched it — which it could do on its own pages. Gate 18 correctly makes the one-assertion optimisation a tested never. Worth one explicit sentence: the home does not and cannot validate that rows belong to the person; it does not need to, because it renders them only to that person under the instance's own name with a same-host `url`. - §5.1's oracle answer (200-empty for no-grant and no-such-person, gate 17) is sound. §2.2's two disclosed costs are real and correctly not routed around; note the platform also logs every wake with its reason (`carlos logs`), so the §2.2(a) timestamp signal lands in platform logs as well as at the instance.</result> <usage><subagent_tokens>140247</subagent_tokens><tool_uses>26</tool_uses><duration_ms>411927</duration_ms></usage> </task-notification>
<task-notification> <task-id>a53017f6690effb98</task-id> <tool-use-id>toolu_017eofnHgk1ZpuYcUbKm2bud</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-home/d63f5ac7-6b42-48c5-a13d-34e6faf8e44c/tasks/a53017f6690effb98.output</output-file> <status>completed</status> <summary>Agent "Fable review 6: signin plan" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>**VERDICT: CHANGES NEEDED** — one block, mechanical but real; everything else is follow-ups. The previously reported items are all verified present except one half of one (Task 2's `git add`), and the RP-ID guard is genuinely fixed. ## The block (B6): the correction landed on a base `main` has already moved past, and the branch does not merge `git merge-tree --write-tree main HEAD` → **CONFLICT (content) in AGENTS.md**. The spec auto-merges cleanly (0 markers; merged §5.1 line 319 carries the branch's `keymaildev/signin` sentence, rulings 20/21 present). AGENTS.md does not: `main`'s `5a42f2d` rewrote the *same* Data bullet (its own `auth_sessions` correction, different wording) and still ends "…and use `auth` only for address classification and the Keymail upgrade, which hold nothing." So the correction is still needed on `main`, but the branch delivers it against `462df84`'s text. The cadence rule is squash-merge only with CI green; this branch cannot be squash-merged at all without a resolution commit, and that commit would be unreviewed binding-file text. The same stale base is why, *read from the branch*, AGENTS.md still says `class="rst-*"` and "Zero JS. No surface…", and the spec's rulings table stops at 16 — while the plan cites rulings 20 and 21. That is not the plan's fault, but it means "verify the spec and AGENTS.md from the branch" currently reads two binding files that contradict the plan on things `main` has since settled. And the plan itself still says what the commit retracts: - line 7, Architecture: "`rastrillo/auth` is used only for address classification and the Keymail upgrade" — the first architectural sentence an executor reads, false at source (verified: `auth.Config` has no `Links` hook; `auth.New` hardwires `Links: &linkStore{db: cfg.DB}` at `auth/auth.go:203`, and that store writes `auth_links (hash, address, …)` at `auth/store.go:82`). - line 96: "the spec's sentence should be corrected to match — filed, not silently diverged from." - line 1376: "Two spec corrections needed…" Fix, one commit: rebase onto `main`, resolve the single hunk by keeping the branch's bullet (it is a superset of `main`'s content), rewrite lines 7/96/1376 to say the correction has landed, and re-run `git merge-tree --write-tree main HEAD` to show zero markers. While in the spec: `main`'s rulings table has a blank line between rows 20 and 21, which breaks the table so ruling 21 renders as a paragraph — it will auto-merge in; fix it here. ## Coherence of editing two binding files in a plan branch Coherent. AGENTS.md's rule ("spec wins, this file is wrong — fix it in the same commit") is about a *disagreement* between the two; here both were wrong identically, so the plan's own standard ("neither made silently") is the governing one, and the branch satisfies it: one commit, both files, both marked "Corrected 2026-09-05", both now agree with each other and with `signin.Flow`'s actual `LinkStore` seam (`magiclink.go:22-30`). The `auth_sessions` detail is right: `auth/store.go:26` says it is no longer written, `sessions/migrations/0001_init.sql:3` has `subject TEXT NOT NULL`, and `auth/handlers.go:136` sets `Subject: id.Address`. The only incoherence is the one above — the correction has to land on top of `main`'s version of the bullet, not beside it. ## RP-ID guard — fixed this time, quoted from the branch `docs/superpowers/plans/2026-09-03-home-signin.md`: - line 1210: `Create: internal/home/rpid_test.go — package home, not guardstest` - line 1256: `cfg := passkeyConfig(t) // what the app configures, never a literal` - line 1258: `u, err := url.Parse(cfg.Origin)` - line 1262: `if u.Hostname() != "id.oficina.works" {` - line 1270: `if cfg.LegacyRPID != "" && !strings.HasSuffix(cfg.LegacyRPID, ".oficina.works") {` - line 1349-1351: `passkeyConfig must return the config the **app** builds, not one the test declares` `grep "until Task 6"` → nothing; `configuredOrigins` and `Count(host, ".")` appear only in the description of the old draft (line 1236). Source checks hold: `passkey.Config` has `Origin` and `LegacyRPID` and no `RPID` field (`passkey/passkey.go:108-132`); RP ID is `u.Hostname()` (`:161`); `LegacyRPID` is assertion-only (`webauthn/webauthn.go:122,148`). `HasSuffix(".oficina.works")` correctly rejects exactly `oficina.works` and `x.oficina.works.attacker.example`. ## Other reported follow-ups, verified Present: `app.go` in Task 1's `git add` (line 410); `newTestApp` starts small and grows (395-399); stub `LookupTXT` must return a real error (401-405) — confirmed at source: `delegate` returns the domain on an empty successful TXT and `domainInfo` then GETs `/.well-known/keymail` (`classify.go:244-281,158-197`), while a non-`DNSError` error returns `""` and touches nothing; the two carve-out assets on stable paths (1152-1159) and named in the allowlist (1304-1307); `sealtest` in the testing-import guard (340-341); `sessions.Sweep` and `passkey.Sweep` (795-797) — both exist (`sessions.go:446` as a method on `*Sessions`; `passkey.go:616`, sweeping `passkey_challenges` and `passkey_pending`); `loadSecrets` out of the table (116-117); `scriptSrcs` in step 2a (1347-1348); `noscript_test.go` in Files and `git add` (1211, 1356); Task 4 has one Consumes line (657); File Structure's logtest location (206). **Not present:** Task 2's `git add` (line 547) still lacks `app.go` — `git add internal/home/models.go internal/home/migrations cmd/home/main.go internal/guardstest`. It also lacks `testapp_test.go`, which Task 2 extends (BootSchema, connection hook). ## Follow-ups (none stop the executor) 1. File Structure line 210 still lists `internal/guardstest/rpid_test.go`; Task 7 puts it in `internal/home/`. 2. Task 4's `git add` (807) lacks `go.mod go.sum` though Task 4 says "Modify: go.mod" — `links.go` imports `signin`, so that commit does not build; also lacks `testapp_test.go` (line 396 says the `signin` dependency enters `newTestApp` in Task 4). 3. `render.go`: Task 5 says "build `(*App).render` HERE" (823-829) but omits it from Files and `git add`; Task 7 says "Create". Make Task 5 create it and Task 7 modify it. 4. `sealtest.CanaryAddress` (346) should be stated as a `string` *included in `Canaries`* — otherwise gate 2 signs in with a value `AssertNoPlaintext` never scans for. Task 5 step 6 ("break the seal, watch it fail") would expose this, but say it. 5. `passkeyConfig(t)`: `passkey.Handlers.cfg` is unexported, so the app must keep the `passkey.Config` it passed to `passkey.New` (a field on `App`) for the helper to return what the app built. One sentence in Task 6 saves the executor a detour. 6. Test at line 1313 destructures `srcs, allowed := scriptSrcs(…)` but step 2a (1347) specifies a single return; `allowed` is never defined. Drop it: `if len(srcs) == 0 { continue }`. 7. Markdown leftovers: line 139 is a table row stranded after prose; lines 1161-1162 are an empty code fence. 8. Line 341 "one string in a slice": on `main`, `logtest_containment_test.go:12` is a single `const logtestImport`; the executor turns it into a slice. Wording. 9. Standing precondition, unchanged: `HOME_DEK`, `HOME_PEPPER`, `HOME_SIGNIN_KEY`, `CARLOS_SMTP_PASS` are Paul's to provision before deploy (line 65, 137-138); tests use test keys so this does not block Tasks 1-7. Everything else I re-read at the pin holds: `Flow.Begin(ctx, address, clientIP, force Method)` (`flow.go:119`), budget skipped only on `""` (`:122`), `clientIP` returns `RemoteAddr` unchanged on `SplitHostPort` failure (`auth/handlers.go:259-265`), `Next{Kind, Redirect, Pending}`, `Identity{Address, Method, At, AuthTime}`, `Fresh` falling back to `At` (`sessions.go:366-371`), `Keymail.RedirectPath` defaulting to `/auth/callback` (`keymail.go:28-36`), `db.Open`'s DSN and single-connection writer (`db/db.go:53-60`), `db.DB.G` exported (`:43`), `passkey.Schema`, `webauthn/authtest`, `rastrillo doctor` in `cmd/rastrillo/main.go:31`, `RegisterConnectionHook` at `modernc.org/sqlite@v1.55.0/sqlite.go:785`, and Go maps `.mjs` to `text/javascript; charset=utf-8`.</result> <usage><subagent_tokens>148531</subagent_tokens><tool_uses>33</tool_uses><duration_ms>357906</duration_ms></usage> </task-notification>
diff --git a/docs/superpowers/plans/2026-09-03-home-signin.md b/docs/superpowers/plans/2026-09-03-home-signin.md| index acd926b..aca9eb0 100644 |
| --- a/docs/superpowers/plans/2026-09-03-home-signin.md |
| +++ b/docs/superpowers/plans/2026-09-03-home-signin.md |
| @@ -4,7 +4,7 @@ |
| **Goal:** A person can sign in to the Oficina home with a magic link, optionally add a passkey as a second factor, and recover with a one-time code — with no readable email address anywhere at rest. |
| -**Architecture:** Hand-rolled sign-in on the credential origin. `rastrillo/auth` is used only for address classification and the Keymail upgrade, never for its link or session stores, because those hold readable addresses (ruling 14). The session Subject is an opaque person ref minted at first sign-in; link state is sealed before it is written. This is the first step that opens a database, so it is also the step that makes gates 1 and 2 buildable — and therefore mandatory. |
| +**Architecture:** Hand-rolled sign-in on the credential origin. `keymaildev/signin` supplies classification and the Keymail upgrade; `rastrillo/auth` is not used at all (see below), never for its link or session stores, because those hold readable addresses (ruling 14). The session Subject is an opaque person ref minted at first sign-in; link state is sealed before it is written. This is the first step that opens a database, so it is also the step that makes gates 1 and 2 buildable — and therefore mandatory. |
| **Tech Stack:** Go 1.25.0, rastrillo v0.22.0, chi v5, modernc.org/sqlite (cgo-free), zero JavaScript. |
| @@ -93,7 +93,7 @@ classification goes through `rastrillo/auth`; both are wrong for the reason |
| below, and both are on `main`. Fix them in **this branch**, so the plan and |
| the binding files stop disagreeing the moment it lands. |
| -The usable seam is one level down: `github.com/keymaildev/signin` exports `Classifier` as a plain struct with exported fields (`HTTP`, `LookupTXT`, `Scheme`, `Now`, `TTL`, `NegTTL`) and a `Classify(ctx, address) Class` method. No constructor, no database, no stores. **Depend on `signin` directly** (rastrillo already requires v0.1.1). This plan does that; the spec's sentence should be corrected to match — filed, not silently diverged from. |
| +The usable seam is one level down: `github.com/keymaildev/signin` exports `Classifier` as a plain struct with exported fields (`HTTP`, `LookupTXT`, `Scheme`, `Now`, `TTL`, `NegTTL`) and a `Classify(ctx, address) Class` method. No constructor, no database, no stores. **Depend on `signin` directly** (rastrillo already requires v0.1.1). This plan does that, and **the correction has landed**: spec §5.1 and AGENTS.md both say `keymaildev/signin` as of this branch. Not filed — fixed. |
| --- |
| @@ -109,8 +109,9 @@ mid-task. |
| |---|---| |
| | `newTestApp(t, dbPath)` | Opens a temp DB, applies migrations, wires a capturing `mail.Sender`, a `Sealer` from a test DEK, a test pepper, and a `signin.Flow` with a stub `LookupTXT`. Returns `*App`. | |
| | `capturedLink(t, app)` | Pulls the magic link out of the captured mail. | |
| -| `signIn(t, app, addr)` | The whole ceremony: POST `/signin`, take the link, GET the confirm page, POST the form. Task 4's tests all go through it. | |
| +| `signIn(t, app, addr)` | The whole ceremony: POST `/signin`, take the link, GET the confirm page, POST the form. Sends an `Origin` header, or `csrf.Protect` refuses. Task 5's tests all go through it. | |
| | `passkeyConfig(t)` | Returns the `passkey.Config` the app builds, for Task 7's guard. | |
| +| `templateNames(t)`, `renderTo(t, w, name, data)`, `sampleData(name)`, `scriptSrcs(t, b)` | Task 7's scriptless sweep. `scriptSrcs` parses the HTML and returns each `<script>`'s `src`, with the empty string for an inline one. | |
| `loadSecrets` is **not** in this table: it is the production loader |
| @@ -136,7 +137,6 @@ POSTs and every test fails for the wrong reason. |
| an instance data key" applies — sign-in runs before any session exists. So |
| the list Paul provisions is `HOME_DEK`, `HOME_PEPPER`, `HOME_SIGNIN_KEY` |
| and `CARLOS_SMTP_PASS`. |
| -| `templateNames(t)`, `renderTo(t, w, name, data)`, `sampleData(name)` | Task 7's scriptless sweep over every template. | |
| **`sessions` wiring**, which no step created: |
| @@ -338,13 +338,17 @@ Test that unset, malformed and 31-byte values each error, and a valid one return |
| The plan commits `internal/home/sealtest` and no step ever created it. |
| Port `sealtest.go` from sheets-core **with its own `TestLeakedDetectsCanary` |
| and WAL tests**, and **extend `TestLogtestOnlyImportedByTests` to cover |
| -`sealtest` too** — one string in a slice. Like `logtest`, it imports |
| +`sealtest` too** — on `main` it checks a single `const logtestImport`, so this |
| +turns that constant into a slice of two. Like `logtest`, it imports |
| `testing` from a non-test file, so importing it from production code would |
| drag the testing package into the binary. — a detector nobody has watched detect anything is the |
| thing gate 9's review found twice. |
| -Export **`CanaryAddress`** (a lowercase, address-shaped |
| -`canary-8f3a@example.invalid`) alongside `Canaries`. Sheets' `Canaries` is |
| +Export **`CanaryAddress`** — a lowercase, address-shaped |
| +`canary-8f3a@example.invalid` — and **include it in `Canaries`**, or gate 2 |
| +signs in with a value `AssertNoPlaintext` never scans for and passes while |
| +proving nothing. (Task 5 step 6's deliberate break would expose it, which is |
| +what that step is for; say it anyway.) Sheets' `Canaries` is |
| `[][]byte` and shaped like `CANARY-CELL-VALUE-8f3a`; both matter here. |
| Case, because the blind index and `signin`'s limiter key on |
| `strings.ToLower(address)`, so the lowercased copy is the one likeliest to |
| @@ -544,7 +548,7 @@ accessor, so reach one through `d.G.Clauses(dbresolver.Read)`. `migrate.Apply` t |
| - [ ] **Step 6: Commit** |
| ```bash |
| -git add internal/home/models.go internal/home/migrations cmd/home/main.go internal/guardstest |
| +git add internal/home/models.go internal/home/migrations internal/home/app.go internal/home/testapp_test.go cmd/home/main.go internal/guardstest |
| git commit -m "Open the database, and the schema that has no address column" |
| ``` |
| @@ -804,7 +808,7 @@ and is simply not worth a goroutine for this.) |
| - [ ] **Step 6: Commit** |
| ```bash |
| -git add internal/home/links.go internal/home/links_test.go internal/home/migrations/0002_signin_links.sql |
| +git add internal/home/links.go internal/home/links_test.go internal/home/migrations/0002_signin_links.sql internal/home/testapp_test.go go.mod go.sum |
| git commit -m "A sign-in link store that seals before it writes and takes exactly once" |
| ``` |
| @@ -813,7 +817,7 @@ git commit -m "A sign-in link store that seals before it writes and takes exactl |
| ### Task 5: `signin.Flow`, the sign-in handlers, and the Keymail upgrade |
| **Files:** |
| -- Create: `internal/home/signin.go`, `internal/home/signin_test.go` |
| +- Create: `internal/home/signin.go`, `internal/home/signin_test.go`, `internal/home/render.go` |
| - Create: `internal/home/templates/signin.html`, `check_your_mail.html`, `signin_confirm.html` |
| - Modify: `internal/home/app.go` |
| @@ -1063,7 +1067,7 @@ step 4 did for the store. |
| - [ ] **Step 7: Commit** |
| ```bash |
| -git add internal/home/signin.go internal/home/signin_test.go internal/home/templates internal/home/app.go go.mod go.sum |
| +git add internal/home/signin.go internal/home/signin_test.go internal/home/render.go internal/home/templates internal/home/app.go go.mod go.sum |
| git commit -m "Sign-in through signin.Flow, with the link redeemed on POST" |
| ``` |
| @@ -1158,8 +1162,6 @@ rastrillo's fingerprinting asset helper; these two may not, because Task 7's |
| allowlist has to name them, and a rule that cannot name its subject is not a |
| rule. |
| -``` |
| -``` |
| `ConfirmPath` is `/passkey/confirm` (its default, named rather than |
| defaulted). `RequireFresh`'s `maxAge` for the codes page is **5 minutes** — |
| @@ -1206,7 +1208,7 @@ git commit -m "Passkeys as an optional second factor, gated on both sign-in path |
| ### Task 7: The scriptless screens, and the RP-ID guard |
| **Files:** |
| -- Create: `internal/home/render.go`, `internal/home/templates/*.html` |
| +- Modify: `internal/home/render.go` (Task 5 created it); create the remaining `internal/home/templates/*.html` |
| - Create: `internal/home/rpid_test.go` — **package `home`, not `guardstest`**: it reads the app's own config through a helper over the unexported `App`, which a `guards` test cannot reach. |
| - Create: `internal/home/noscript_test.go` |
| - Create: `internal/home/static/tokens.css`, `internal/home/static/theme.css` — vendored from rastrillo. **Under `internal/home/`, not the repo root:** `go:embed` cannot reach a parent directory, sheets-core embeds `all:static` from inside its package, and `rastrillo new` scaffolds `internal/<pkg>/static/` with a `TestVendoredAssetsMatchTheLibrary` that reads it there. |
| @@ -1310,9 +1312,8 @@ func TestNoScriptTagOutsideTheAllowlist(t *testing.T) { |
| for _, name := range templateNames(t) { |
| var buf bytes.Buffer |
| renderTo(t, &buf, name, sampleData(name)) |
| - srcs, allowed := scriptSrcs(t, buf.Bytes()) // parses, so an inline body has no src |
| - |
| - if !allowed && len(srcs) == 0 { |
| + srcs := scriptSrcs(t, buf.Bytes()) // parses, so an inline body has no src |
| + if len(srcs) == 0 { |
| continue // no script at all: the common case |
| } |
| permitted, ok := scriptAllowed[name] |
| @@ -1348,7 +1349,10 @@ func TestConfirmPageRecoveryFormSurvivesScriptStripping(t *testing.T) |
| string for an inline one) and `passkeyConfig(t)` — all five are used by the two guards above and created |
| by no step in an earlier draft. `passkeyConfig` must return the config the |
| **app** builds, not one the test declares, or the RP-ID guard asserts a |
| -constant against itself. |
| +constant against itself. `passkey.Handlers` keeps its config unexported, so |
| +**the `App` must retain the `passkey.Config` it passed to `passkey.New`** |
| +for the helper to hand back — one field, and the executor otherwise loses an |
| +hour to it. |
| - [ ] **Step 3: Commit** |
| @@ -1373,6 +1377,6 @@ git commit -m "Scriptless sign-in screens, and the guard that the RP ID is a ful |
| - **Spec coverage:** §5.1 is Tasks 1–5; §5.2 is Task 6, unblocked by ruling 21; §10's `people`, `signin_links` and `sessions` are Tasks 2–4; §12 gates 1 and 2 are Tasks 1 and 5; §11 is Task 7. §5.3–§5.6 are step 3, out of scope and listed above. |
| - **Deliberately not covered:** `prefs`, `directory`, `teams`, `provisioning_queue`, `rekey_journal`, `consumers`, `assertion_jti` — later steps. |
| -- **Two spec corrections needed, neither made silently:** §5.1's "use `rastrillo/auth` for classification" is not achievable; §3.1 contradicts ruling 3 on HMAC-vs-KDF. |
| +- ~~Two spec corrections needed~~ — **landed in this branch**, neither made silently:** §5.1's "use `rastrillo/auth` for classification" is not achievable; §3.1 contradicts ruling 3 on HMAC-vs-KDF. |
| - ~~The one unverified identifier: `signin.ClassKeymail` in Task 5.~~ **It did not exist** — `Class` is a struct. Flagging it was the right instinct; writing it anyway was the wrong response. Verify, or do not write. Every other signature in this plan was read at source, and the 2026-09-05 review pass re-read them at the pin. |
| - ~~The riskiest assumption: that `rastrillo/db.Open` either sets `PRAGMA secure_delete` or lets the app set it.~~ **Resolved 2026-09-05: it does neither.** The DSN carries `busy_timeout`, `journal_mode` and `foreign_keys` and passes nothing else through. Task 2 step 5 now uses a connection hook registered before `db.Open`, because the writer pool's one long-lived connection is opened inside it. |
diff --git a/docs/superpowers/specs/2026-08-30-home-design.md b/docs/superpowers/specs/2026-08-30-home-design.md| index c03991c..b3ee301 100644 |
| --- a/docs/superpowers/specs/2026-08-30-home-design.md |
| +++ b/docs/superpowers/specs/2026-08-30-home-design.md |
| @@ -62,7 +62,6 @@ blind, replaceable*. |
| | 18 | **Activity is approved in principle, pulled at render, and sequenced last** — the front door shows a real feed with titles, the home holds none of it at rest, and its slot is a step 10 after step 4 — `design-activity-pull` | Push into the home; a contentless heat map; refusing the feature | Paul, 2026-09-05 | |
| | 19 | **A team name is reserved suite-wide. Both paths end in the drain — an existing instance settles the label question for the operator, it does not bypass them. The home answers only facts about its own records, never "may this person"** — §7 | Per-app first-come, which lets a stranger squat `acme.sheets` behind your `acme.docs` on a domain that reads as one suite | Paul, 2026-09-05 | |
| | 20 | **Write the attribute spelling — `rst-btn`, not `class="rst-btn"`. The seven unpaired utilities keep `class`** — §11 | The class vocabulary, which this section ruled on 2026-08-30 and whose every stated reason has since died; "follow whatever the pin's partials use", which changes this app's markup on a dependency bump | Paul, 2026-09-05 | |
| - |
| | 21 | **§11's zero-JS rule gains one named carve-out: passkey enrolment and ceremony screens may load script. Everything else stays scriptless** — §11 | Passkeys leaving slice one; a blanket exemption; a progressive-enhancement version, since the ceremony itself cannot happen without script either way | Paul, 2026-09-05 | |
| --- |