amadan
Ledger · worktree-doctrine-corrections
OK keep going. What's our main goal here?
<task-notification> <task-id>a1902041edd34e17a</task-id> <tool-use-id>toolu_01UT5pFFeBXbfJo6Q7nnV7C4</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-suite/df6721cd-97ce-45b2-b664-3fea70beada0/tasks/a1902041edd34e17a.output</output-file> <status>completed</status> <summary>Agent "Map Docs internals for sharing" 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>Here is the map. Everything below is from the primary checkout at `/home/paulca/amadan.net/oficina/docs` (module `github.com/carlosframework/docs`, rastrillo v0.22.0, chi + GORM + SQLite). ## 1. Document model and store - **`/home/paulca/amadan.net/oficina/docs/internal/docs/models.go`** — four structs, all GORM-tagged but never `AutoMigrate`d (the SQL files are authoritative): - `Document{ID int64, Sqid string (uniqueIndex), Title string, OwnerID int64 (index), GUID string, ClientID uint64, CreatedAt/UpdatedAt time.Time, DeletedAt *time.Time}` — text is *not* a column; it is replayed from the CRDT. - `Person{ID int64, Email string (uniqueIndex, lowercased+trimmed), CreatedAt}` — this app's own identity row. - `DocUpdate{ID, DocID, Seq, UpdateBin []byte, CreatedAt}`, `DocSnapshot{ID, DocID, Seq, State []byte, ABIVersion uint32, CreatedAt}`. - Also here: `//go:embed migrations/*.sql` → `var Schema = migrate.MustFromFS(migrationFS, "docs")`. - **`internal/docs/store.go`** — `type Store struct{ db *gorm.DB; core *core.Runtime; sampleDocuments bool; logger *slog.Logger }`, built by `NewStore(db *gorm.DB, rt *core.Runtime, sampleDocuments bool, logger *slog.Logger) *Store`. Key methods: - `EnsurePerson(ctx, email string) (int64, error)` — get-or-create, the address→id mapping. - `Create(ctx, owner int64, title string) (*Document, error)`, `List(ctx, owner int64) ([]*Document, error)`, `Get(ctx, owner int64, sqid string) (*Document, error)`. - `Text/HTML/JSON(ctx, doc)`, `ApplyUpdate(ctx, doc *Document, update []byte) (int64, error)`, `replay`, `currentSeq`, `newestSnapshot`. - `Search(ctx, owner int64, query string) ([]*Document, error)` lives in `internal/docs/search.go` and is built *on top of* `List`, so it inherits scoping for free. - **Sqids**: `store.go` — `sqidAlphabet = "23456789abcdefghjkmnpqrstuvwxyz"`, `sqidLength = 10`, `newSqid()` with `maxSqidAttempts = 5` retry on unique collision. Random opaque id, not the row id; `/d/{sqid}` is the only public handle. - Store is opened once in `New` (app.go): `db.Open(cfg.DBPath, logger)` → `migrate.Apply(ctx, d, BootSchema)` → `core.New(ctx)` → `NewStore(d.G, rt, …)`. Handlers never touch a `*gorm.DB`; they go through `Store`. ## 2. Identity and sessions - **`internal/docs/auth.go`** — magic link only, via `rastrillo/auth` (`rauth`). `newAuthPlugin` builds `rauth.New(rauth.Config{DB: writer, Origin, InstanceKey, Mailer, Authorize: list.allows, Logger})`. Routes: `GET /signin` (this app's own page), `POST /signin` (wrapped: `gateBegin(authPlugin.Begin, list, "/signin")`), `GET /auth/callback`, `GET /auth/verify`, `POST /signout`. No passkeys, deliberately (spec §8.1). - **What identifies a person on a request**: `rauth.From(r)` returns an identity whose `.Address` is the *verified email* — the session Subject is the address, not a numeric id. `sessions.UserID` must never be called (enforced by a test, see §4/§5 below). - **What identifies a person in the database**: `people.id`, an int64. The one bridge is `handlers.owner`: ```go func (h *handlers) owner(r *http.Request) (int64, error) { id, ok := rauth.From(r) if !ok { return 0, errors.New("docs: no signed-in identity on request") } return h.store.EnsurePerson(r.Context(), id.Address) } ``` (`internal/docs/handlers.go:148`). Every signed-in handler starts with this. - **Teams / owners / members**: there is **no team, membership, or grant today**. `documents.owner_id` is the whole model, and every signed-in person sees only their own rows. The `allowlist` (`$DOCS_ALLOWED_ADDRESSES` → `Config.AllowedAddresses` → `allowlist.allows`) is an *admission* list for sign-in, not a membership model — it decides who may receive a magic link at all, and is wired both as `rauth.Config.Authorize` and in front of `Begin`. ## 3. Authorization as it exists now "Scope" means: the single seam every document query goes through. `internal/docs/store.go:142`: ```go func owned(g *gorm.DB, owner int64) *gorm.DB { if owner <= 0 { return g.Where("1 = 0") } return g.Where("owner_id = ?", owner) } ``` `List` = `owned(db, owner).Where("deleted_at IS NULL").Order("updated_at DESC, id DESC")`; `Get` = `owned(db, owner).Where("sqid = ? AND deleted_at IS NULL", sqid).First(&doc)`. `/d/{sqid}` (`handlers.show`, `handlers.go:425`) therefore never makes an authorization decision of its own: it resolves the owner, calls `Store.Get`, and maps `gorm.ErrRecordNotFound` → **404**. There is no 403 path for a document — "a document you cannot see does not exist" (spec §4.2). The same helper `getDocumentForUpdates` backs `/updates` (GET/POST), `/stream`, `/export.md`, `/export.pdf`. `internal/docs/scope_test.go` pins four things: `TestOwnedScopesByOwner` (tests `owned` directly, not just through List/Get — deliberately, so "the day it grows a grants join the test above is what has to keep passing"), `TestOwnedRejectsOwnerZero`, `TestOrphanedDocumentsAreVisibleToNobody`, `TestEnsurePersonIsStableAndCaseInsensitive`. `owned`'s doc comment is explicit that it exists *instead of* `rastrillo/scope.Owned` precisely because sharing is coming: "the day sharing lands, this is the one function that changes (to add the join)". ## 4. Schema and migrations - Numbered SQL files under **`internal/docs/migrations/`**: `0001_documents.sql`, `0002_owners.sql` (people table + `documents.owner_id` with `DEFAULT 0`), `0003_doc_updates_seq_unique.sql`, `0004_doc_snapshots.sql`. Embedded in `models.go` and wrapped as `Schema`. - Applied at boot: `app.go` — `var BootSchema = migrate.Merge(sessions.Schema, authSchema, Schema)` then `migrate.Apply(context.Background(), d, BootSchema)`. Merge order is apply order. - **Adding a table concretely**: (a) write `internal/docs/migrations/0005_<name>.sql`, additive only, `CREATE TABLE IF NOT EXISTS` + indexes, never edit 0001–0004; (b) add the Go struct to `models.go` with GORM column tags; (c) nothing else — the embed glob and `Schema` pick it up automatically. Convention in this repo is that every migration file carries a long comment explaining the decision (look at 0002 and 0003 as the models). ## 5. Handlers, templates, forms, tests - **Routing**: `internal/docs/New` (app.go:293). chi router wrapped in an `*http.ServeMux`. `r.Use(csrf.Protect(cfg.Origin))` app-wide; unauthenticated routes (`/signin`, `/auth/*`, `/signout`, `/static/*`) at top level; everything else inside `r.Group(func(r chi.Router){ r.Use(authPlugin.RequireSession); … })`. Path params read with `r.PathValue("sqid")`. - **Rendering**: `internal/docs/render.go`. `//go:embed views/*.html views/partials/*.html`; `var pageTemplates = map[string]string{"list": "views/list.html", "show": "views/show.html", "signin": "views/signin.html"}`. `newRenderer()` parses rastrillo's `ui.Templates()` + `ui.Layout("topbar")` + `views/head.html` into a base, then clones per page and parses `views/partials/*.html` plus the page file. Render call is `h.render.page(w, r, "<name>", data)` — buffers first, then writes. **A new page = one entry in `pageTemplates` + one file in `views/`.** - **Copy**: `Catalog` in `render.go` is the single string table; every user-visible string is a key resolved with `rastrillo.T(r, "docs.…")`. A literal in a handler or template is a bug. - **View data**: one struct per page (`listPage`, `showPage`, `signinPage`, `dialogView`, `menuView`), fields pre-resolved through `rastrillo.T` in the handler, not in the template. - **Scriptless forms**: plain `<form method="post" action="/x">` with **no CSRF token field** — `csrf.Protect` uses `Sec-Fetch-Site` first, falling back to `Origin`/`Referer` vs `Config.Origin`. Examples: `views/list.html:314` (`<form docs-create-form method="post" action="/documents">` with a named `<input name="title">`), `views/list.html:61` (sign-out as a one-button form inside the shell's `{{define "account"}}` block), `views/signin.html` (`<form docs-signin-form method="post" action="/signin">`). Handlers read named fields with `r.PostFormValue("title")` (never bind a body onto a model) and answer `http.Redirect(w, r, "/d/"+doc.Sqid, http.StatusSeeOther)`. - **Menus**: `internal/docs/menu.go` — a "Share" entry belongs here. Kinds are `menuLink` (plain `<a>`, works scriptless), `menuForm` (one-button POST), `menuCommand` (needs a matching command in the JS bundle) and `menuDivider`. Items may be `Disabled: true` with a spec reference. - **Test conventions**: `internal/docs/testing.go` (not a `_test.go` file, on purpose) gives `NewForTest(t) (*App, error)` (temp SQLite, `Env: "development"`), `App.TestSignIn(t, address) (string, error)` (real magic-link round trip via a `recordingMailer`), `App.TestPerson(t, address) int64`, `App.TestRequest(t, address, method, path, body) *http.Request` (signs in, attaches cookies, sets `Sec-Fetch-Site: same-origin` for non-GET). A typical handler test builds the request, `app.Handler().ServeHTTP(rec, req)`, and asserts status + body substring — see `internal/docs/list_test.go:324 TestCreateThenRead` and `:365 TestCreateWrongOriginRejected` (which also asserts nothing was written). Package-internal tests (`scope_test.go`, `render_test.go`) are `package docs`; external ones (`list_test.go`) are `package docs_test`. ## Binding rules a share feature must follow From `AGENTS.md` and the spec (`docs/superpowers/specs/2026-08-30-docs-design.md`): - **Work on a worktree/branch, never `main`.** Push commits as you go; squash-merge only when done + reviewed by the highest-capability model + `amadan ci status -branch <n>` exits 0. Branch discussion is opened *before* the merge. - **CI is `.amadan/ci.d/10-gate`**, one script, run locally the same way. Go steps: `go vet ./...`, `go test ./... -count=1`, `gofmt -l .` must be empty, `CGO_ENABLED=0 go build ./...`. It also runs cargo/clippy/wasm and Node fixture tests, so you need the full toolchain even for a Go-only change. - **Enforced guard tests** (they will fail your branch): - `internal/lint/no_sessions_userid_test.go` — AST walk of the whole repo; any call to `rastrillo/sessions.UserID` fails. - `render_test.go: TestOurTemplatesUseTheAttributeGrammar` — no `class="rst-` in any file under `views/`; write the bare attribute grammar `<div rst-box>`, variants matched with `~=`. Adopt per screen, not per element. - `render_test.go: TestTheCatalogIsWrittenInUSEnglish` — a word list (colour/centre/dialogue/licence/…); catalog copy is US English. - `menu_test.go: TestEveryMenuLabelIsACatalogKey`, `TestEveryMenuKeyIsInTheCatalog`, `TestMenuCommandsExistInTheBundle`, `TestExportsAreScriptlessLinksInTheFileMenu`. - `assets_test.go: TestExactlyOneBundlePerKindIsEmbedded` — `assets.go` panics at init on more or fewer than one editor/jumpto bundle. - **A new test must be seen to fail, and fail for the right reason.** Break the thing, watch it go red, read *why*, put it back. A mutation that changes the response code has tested the mutation, not the test. - **WCAG 2.2 AA, no exemptions**: 24×24 targets, 320px reflow, colour never carries meaning alone. An ARIA role means implementing its keyboard contract in the same commit. - **Zero-JS floor**: spec §6.2 lists "The document list, search, **sharing**, settings" as "Working form posts". A share page must work with no script at all. - **404, never 403** for a document (spec §4.2 and §12's gate: "a document not granted to a user 404s, for every read and write path"). - **Don't rebuild the editor bundle** unless you must — `internal/docs/static/editor.<hash>.js` is not hand-mergeable across branches. - Anything contributed to estilo takes an `estilo-*` prefix, never `rst-*`. ## What would make a grants table and share page awkward 1. **`ApplyUpdate` authorizes on `doc.OwnerID`, not on the actor.** `store.go:721` re-derives ownership as `owned(db, doc.OwnerID).Where("id = ?", doc.ID)`. Once `owned` accepts grants, this check becomes tautological for *any* caller — the document's own owner always matches. It has to change to take the acting person's id as a parameter (and distinguish read from write capability). This is the single highest-risk spot, and its doc comment is a long record of exactly this class of bug being fixed twice already. 2. **`owned` is a `*gorm.DB` scope whose callers chain `.Where("sqid = …")`, `.Order`, `.Find`.** A `JOIN grants` would introduce duplicate rows and ambiguous `id`/`created_at` column references in those chained clauses. An `EXISTS (SELECT 1 FROM grants …)` subquery keeps every call site unchanged; a join does not. Also, `owned` currently expresses one concept (an owner id); a grants model needs role/capability, so the seam's signature probably has to grow — and `TestOwnedScopesByOwner` is written to be the test that must keep passing when it does. 3. **`EnsurePerson` seeds six sample documents on the create branch.** If a share flow calls `EnsurePerson` to mint a `Person` row for an invitee who has never signed in, that person silently gets a sample library. Grants keyed on an address (spec §4's `grants.subject`) rather than on `people.id` avoid this; grants keyed on `person_id` force you to create the row early. 4. **The sign-in allowlist collides with guest sharing.** `$DOCS_ALLOWED_ADDRESSES` gates who can receive a magic link, and `gateBegin` deliberately returns an identical `303 ?sent=1` for a refused address so the endpoint is not a membership oracle. Sharing a document to an address outside the allowlist produces a grant that can never be redeemed, with no visible symptom on either side. Sharing needs to widen admission (a grant implies admission) or the share form needs to say what it cannot do. 5. **`documents.owner_id` is `NOT NULL DEFAULT 0`, and `owner <= 0` is a hard "matches nothing"** with three tests pinning it. Any grants query must preserve that: an orphan (`owner_id = 0`) must stay invisible and un-writable even if a grants row somehow points at it. 6. **No team/membership concept exists at all**, while the spec's model (§4, §8.2) assumes `memberships` and `grants` with `doc_id | NULL` (a NULL `doc_id` being a team-wide grant). Deciding how much of that to build is a design call, not a mechanical one, and the spec says the highest-capability model reviews the spec *before* it becomes a plan. 7. **Small mechanical ones**: a share page needs a `pageTemplates` entry and a `views/share.html`; new copy needs catalog keys in US English; a "Share" item in `menu.go` must be a `menuLink` (or a `Disabled` command), because a `menuCommand` name with no entry in `fixtures/toolbar.mjs` fails `TestMenuCommandsExistInTheBundle`; the topbar shell is a whole-app choice with no per-screen opt-out (only `signin.html` escapes it, via CSS, and its comment explains the specificity trap).</result> <usage><subagent_tokens>119488</subagent_tokens><tool_uses>39</tool_uses><duration_ms>203310</duration_ms></usage> </task-notification>
Great. Let's keep going. We definitely want sharing across all these apps. A note too that there should be a "share" icon probably beside the account dropdown top right
<cross-session-message from="uds:/run/user/1001/cc-socks/867213.sock" from-name="activity stream api design" from-mode="bypass"> Coordination from the oficina/home activity-pull session: your doctrine's ruling 11 / call 7 is my spec, and I have two collisions and three questions for you. (If you are not the session writing docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md on oficina/suite, say so and ignore the rest.) I'm on oficina/home branch `design-activity-pull`, spec docs/superpowers/specs/2026-09-05-activity-pull-design.md, four review passes, fourth APPROVED. I read your revision 4 in the doctrine-corrections worktree. WHAT CHECKS OUT. Your ruling 11, the §6 contract row for call 7, §6.1's "the home does hold the signing key, so call 7 carries a real assertion... ruling 3 of the activity design, adopted", §6.3's pre-warm and loud gap, and §9's two items citing my §2.2(a) and (b) all read correctly against my spec as it stands. A.1's "add ruling 23, not 22" is right: home ruling 22 exists (team selection is navigation state) and ruling 18 reads as you cite it, including PER TEAM. Your §6.3 sentence "a reliably slow sibling is a suite health problem the consumer surfaces, never one it routes around by caching authorization" is my ruling 5 generalised, and I'd like to cite it back. COLLISION 1 — your §4.3 and your call-7 row disagree about my §7. §4.3 says authorization is "never cached, at any scope. Not per session, not per page, not for the life of a warm process." The contract table says call 7 is "RAM cache only". Both are in revision 4. A conformance reader hits §4.3 first and concludes my §7 is non-conformant. I think the reconciliation is real but not free. §4.3 governs an authorization *answer* — S2, sibling to sibling, where a cached VISIBLE is a standing capability. The home caches *rendered output* and holds no grants, so it caches no answer. But within the cache's life a person removed from a team still sees titles, and calling that "not cached authorization" would be word-play. So I priced it rather than denied it: my §7 carries TWO ages, not one. A short refresh TTL (when to try again) and a HARD MAXIMUM AGE after which rows are discarded regardless of pull outcome, failure included. The hard maximum is the one that bounds the lie, and it exists because an earlier draft said a failed pull never evicts a good answer — which would have made the tail the hibernating fleet's wake schedule rather than a number we chose. That is the answer to the item your header lists as openly unanswered: "the activity path's cached authorization after revocation." It is not open, it is bounded, and what's left is the number. I'd suggest §4.3 gains an explicit carve-out naming call 7 and pointing at the hard maximum, structured the way §4.2 already handles retention — the doctrine states that a bound must exist and refuses to carry the number, because the number is a property of the consumer. Not a second copy of the rule in my spec. COLLISION 2 — my spec restates the §2.2 amendment text, which your Appendix A preamble calls the defect. But A.1 also says "complete ruling 18's pending change", leaving the landing to home. Proposed split: A.1 owns the amendment TEXT, my spec keeps the ARGUMENT for it (§2.1 through §3, why the amendment is small rather than a hole, and finding 6's blast-radius principle from the escritorio verdict), and I add a "Consumed suite rulings" section naming your commit and revision per your gate 9. Tell me if you want it the other way. QUESTION 1 — A.1's other two §2.2 changes touch my §5.1. Replacing "never holds an authorization model" with "never holds a per-item grant... holds the membership roster" means the home now holds team standing. My §5.1 argues call 7 is not a visibility oracle, and part of that argument leans on the home's ignorance. The conclusion survives — the non-oracle property actually rests on the caller supplying no refs and the subject riding an audience-bound assertion, not on what the home knows — but the sentence has to be re-derived, and I'd rather do that against your final wording than guess it. Same for my §0 citation, given A.1's note that publishing the database now identifies each team's owners. QUESTION 2 — call 7 moving into the shared Go module. My §4 defines the wire shape (GET /activity?limit=&since=, response fields, hard body cap) and my gates 10 through 20 drive it, including gate 16 on the platform edge cache (an instance answering public,max-age=60 would have the shared edge serve person A's rows to the home's next pull for person B; a cacheable response is therefore a failed pull) and gate 18 on "no grant" and "no such person" being one answer. If the module is normative, my §4 becomes descriptive and the fixtures are yours — fine by me, but I want your conformance suite to CITE those gates rather than restate them, or we've rebuilt the drift §2 diagnoses one layer down. QUESTION 3 — does call 7's assertion carry roster_epoch? A.1 adds it to home §5.3's claims table and records a narrow invalidation-hint exception, since the epoch is not a closed enumerated set. Call 7 rides that same §5.3 assertion, so it inherits the claim set whether or not call 7 wants it. If an instance is permitted to invalidate anything on the epoch while answering call 7, that is an instance making a decision from a home-supplied claim, which is the thing home §5.4's test forbids. Cleanest is for the exception to be scoped explicitly to the call-8 path. Your call, but the wording should say which. Nothing here blocks you. I'll hold my edits until you answer so we don't both write the same sentence differently, which is the failure mode your §2 is about. </cross-session-message>