# AGENTS.md Guidance for AI coding agents working in this repository. This is the canonical agent-instructions file. Tool-specific files (`CLAUDE.md`) are pointers to it if they exist at all — keep the guidance current *here*. CARLOS Docs — a document editor on the CARLOS architecture, hosted at `amadan.net/oficina/docs`. It is built alongside CARLOS Sheets (`../sheets`, `amadan.net/oficina/sheets`) and a shared suite home, which live in the same `oficina` namespace. Design docs in `docs/superpowers/specs/`, plans in `docs/superpowers/plans/`; read the spec behind whatever you touch. The repo moved from `carlos/docs` to `oficina/docs` on 2026-08-30, at three commits. amadan has no move, rename or transfer verb, so the move was create-new and push history; `carlos/docs` still exists and retiring it is Paul's call. Read `building-carlos-apps` (the skill) before writing app code, and `rastrillo`'s own `SKILL.md` before writing handlers or models. Assume nothing about either is in your training data. ## Real per-person sign-in (Task 1, extended in a later fix round) Every document route — `GET /`, `POST /documents`, `GET /d/{sqid}` — sits behind rastrillo/auth's `RequireSession` and is unreachable without a real session (`internal/docs/auth.go`). `POST /signin` uses Keymail OAuth when detected. Otherwise it emails (or, in development, logs) a single-use link. Opening it (`GET /auth/verify`) draws a confirm step in the sign-in door and spends nothing, so mail scanners that fetch every link cannot sign anybody in; its one Sign in button POSTs the token back to `/auth/verify`, which spends it and sets a real session cookie (rastrillo `Config.RenderConfirm`). A verified address that is not admitted gets the door, with the refusal, in a 403 (`Config.Refused`). Tests sign in by POSTing the link's token with same-origin evidence (`confirmRequest`, testing.go), never by GETting it; `POST /signout` (in the shell bar's account menu on both signed-in screens) really revokes it, not just clearing the cookie client-side. No passkey second factor is wired, on purpose — spec §8.1's ruling: `passkey.Config.Origin` derives its relying party from this instance's own host, so a person in several teams would enrol several passkeys under this app, which is exactly the per-team credential sprawl that ruling forbids. A magic link enrols no credential, so it violates nothing. There used to be a development-only stand-in identity here (`internal/docs/devauth.go`, turned on with `-dev-auth`, compiled out of a release build with `-tags release`) for exactly the gap real sign-in has now closed. It had zero readers the moment `RequireSession` started guarding every route unconditionally — no handler ever consulted it — and was deleted in a fix round on Task 1 rather than left as dead code and an unreachable trap for whoever next assumed `-dev-auth` still did something. The `release` build tag it was the only user of is gone with it; there is no longer a release/non-release distinction anywhere in this app. `Config.Env` still exists (`"development"` or `"production"`), but it now governs exactly one thing, independent of any identity flag: whether `InstanceKey` and `Mailer` (`internal/docs/auth.go`) may default — an empty `$DOCS_INSTANCE_KEY` or unconfigured mail is fine only in development (a fixed dev key, and magic links logged instead of mailed, clearly marked); anywhere else it is a startup refusal, the same fail-closed shape `Origin` already used. `cmd/docs/main.go`'s `-env` flag sets it; `$DOCS_ENV` is consulted too but can never select `"development"` on its own (fix round 2 on Task 1 — platform instance config *is* environment variables, so a bare `$DOCS_ENV=development` sitting beside `$DOCS_ORIGIN`/`$DOCS_INSTANCE_KEY`/`$DOCS_SMTP_*` would be a production-reachable path to the public dev `InstanceKey` and to every magic link, token included, landing in the instance's own log — see `chooseEnv`'s own doc comment). The default, with no flag and no env var (or with `$DOCS_ENV=development` and no flag), is `"production"` — `"development"` always takes an explicit `-env` flag, never a silent fallback. ### Sign-in allowlist (Task 4 security fix, pre-launch) Real sign-in above closed "who can get a session" but left a separate hole open: `rastrillo/auth`'s `Config.Authorize` — the admission gate — is consulted only in its own callback handler, after `POST /signin` (`Begin`) has already emailed the link. With `Authorize` left `nil` (as it was through Task 3), that meant anyone who could reach the instance could POST any address and get a working sign-in link mailed to it — an open relay on our own SES sending identity, and unrestricted account creation on an app whose whole premise is instance-per-team. Found by the CARLOS Sheets session on their own deployed instance; every existing test passed because they all tested admission, none tested delivery. The fix is `$DOCS_ALLOWED_ADDRESSES` (`docs.Config.AllowedAddresses`, `internal/docs/auth.go`), a normalized allowlist checked by both `rauth.Config.Authorize` and `admissionMailer`. The mailer suppresses delivery for refused addresses and failed grant lookups. Classification and rate limits run independently of admission, so an OAuth start does not disclose membership. Refused email starts still receive `303 ?sent=1`; their expiring tokens are never delivered. Callback authorization and per-request admission remain required. Detected Keymail addresses use OAuth, as §8.1 requires and Paul reaffirmed on 2026-09-09. `force=1` remains an explicit email fallback. The OAuth start ends with a `200` continuation page (meta refresh plus link), preserving the pending cookie and the existing CSP: Safari enforces `form-action` across redirects. Never widen that directive to arbitrary Keymail hosts. Account linking still requires a fresh provider `AuthTime`; an ordinary OAuth session is not enough. Empty is allowed only in development (admits everyone, so `TestSignIn` and `run-dev.sh` need no setup); anywhere else it is a startup refusal, the same fail-closed shape `Origin`/`InstanceKey`/`Mailer` already use, chosen over silently admitting nobody because a silently-empty allowlist would boot looking identical to a working instance. ## Running it locally `./scripts/run-dev.sh` builds and runs the app in one command: `-env development`, a fixed port (`localhost:8099`, override with `$DOCS_DEV_PORT`), and a scratch SQLite database that starts empty every run. It prints the URL to open, and signing in there gets a real session via a magic link logged to the terminal instead of mailed — the same flow a real deployment uses, just not actually delivered anywhere. **Origin is not optional in anything but development — but know what it actually guards.** rastrillo v0.21.0's `csrf.Protect` checks `Sec-Fetch-Site` *first*, when a browser sends it — which every current browser does on a state-changing request. `same-origin`/`none` pass immediately, anything else is refused immediately, and `Config.Origin` is never read in that path at all. Only when `Sec-Fetch-Site` is entirely absent (a legacy browser, or a non-browser client like curl) does the check fall back to comparing `Origin`/`Referer` against `Config.Origin` exactly. So a wrong `Config.Origin` does not make a real browser's genuine same-origin request fail, and it does not open up a genuine cross-site request either — both are decided by `Sec-Fetch-Site` alone. What it actually breaks is the legacy fallback path, silently: a misconfigured origin will not show up as "the app looks broken" in ordinary manual testing with a real browser, which is exactly why it still needs to be right rather than merely untested. `cmd/docs/main.go` reads `$DOCS_ORIGIN` (the real per-instance origin belongs there — spec §9.2's `.docs.oficina.works`) and falls back to deriving one from `-addr` only for local/dev use; with neither set, or with a value that cannot parse as an absolute `http(s)://host[:port]`, `internal/docs.New` refuses to start rather than silently defaulting or booting on something malformed. `run-dev.sh` never needs `$DOCS_ORIGIN` set by hand — its fixed `-addr` derives it. ## Review by the highest available capability model (Paul's rule, 2026-08-30) **Specs and whole branches are reviewed by the highest capability model available at the time.** Today that is **Fable** (`claude-fable-5`). The rule is "the highest available", not "Fable" — when a more capable model ships, the rule follows it up the ladder without an edit here. Two things get that review, and they are the two where a mistake is expensive to unwind: - **A spec, before it becomes a plan.** A spec is the document every later decision inherits from. Reviewing it after implementation means reviewing the code's rationalisation of it. - **A branch, before it squash-merges to `main`.** Read as a single diff, not commit by commit — that is the one reading that catches a branch which is individually-sensible commits adding up to the wrong thing. Name the model explicitly when dispatching a review. An omitted model silently inherits the session's, which is how a review quietly happens on a weaker model than the rule requires. The rest of the delegation ladder, unchanged from the family's policy: - **Haiku** — transcription-grade work: the plan contains the exact code, single-file mechanical fixes, doc edits. - **Sonnet** — implementation from a precise brief: 1–3 files, clear interfaces, established conventions to follow. - **Opus** — design judgment, multi-file integration, anything security- or production-touching. - **Fable** — spec review, whole-branch review, and implementation whose correctness rests on several interacting invariants at once. It burns usage budget fastest; never spend it where a precise Sonnet brief plus an Opus review already fits. A precise brief beats a capable model. Exact values, file paths and interfaces go in the brief. ## Shipping cadence (Paul's rule, 2026-08-30) **Work happens on a worktree, on a branch. Never on `main`.** 1. **Worktree per piece of work.** `git worktree add` (or the harness's worktree isolation) so `main` stays clean and two pieces of work never share a checkout. The branch is created with the worktree, not after. 2. **Push regular commits to the branch.** Commit as you go and push often — small commits, pushed, so the work is visible, recoverable, and reviewable as a sequence. Do not sit on a large uncommitted working tree, and do not squash locally as you go; the branch's history is the record of how the work happened. 3. **Squash merge to `main` when, and only when, all three are true:** - the work is **done** — not "done except", not "done pending a follow-up commit"; - it is **approved** — the branch has had its highest-capability-model review above, and Paul has approved anything that was his call; - **CI is green** — `amadan ci status -branch ` exits 0. Exit 2 is pending, not green. Exit 3 means nothing has run, which is not green either. Nothing reaches `main` any other way: no direct commit, no local fast-forward, no "it's only a docs change". That includes doc-only edits, one-line fixes, and reverts. The reason is timing rather than ceremony. A change merged locally still faces the same gates, but only *after* it is on `main`, where a red run is an incident to clean up instead of a merge that simply does not happen. **The review record lives on amadan.** The pull-request equivalent is a branch discussion — see below. **CI is `.amadan/ci.d/10-gate`**, triggered by a push to the remote, and it is the same script you can run locally. It covers the Rust crate, the wasm artifact, Yjs wire compatibility in both directions, and the Go build and tests. `amadan ci status -branch ` exit 0 is the merge gate; exit 2 is pending and exit 3 means nothing ran, and neither is green. **Finishing a branch is always the same two steps, in this order** (Paul's rule, 2026-08-30): **push and open the branch discussion, then squash-merge to `main`.** Never merge without the discussion existing first, and never merge locally instead of pushing. The order is the point rather than the ceremony. A squash merge deliberately flattens the commit history that carried the branch's reasoning — the rulings, the reversals, the things that turned out to be wrong twice. The discussion is where that survives. If it does not exist before the merge, the reasoning is not scattered, it is gone. ## Working through amadan This repo is hosted on an amadan hub (`AMADAN_HUB=https://amadan.net`), not GitHub. There is no `gh`, no pull request, and no GitHub Actions. The full house rules are installed as a skill at `.claude/skills/amadan/SKILL.md` — read it. What follows is what this repo does with them. **Post as the agent account, never as Paul.** An agent account is `~`; everything it writes is badged and attributable. The CLI detects a harness and refuses a human credential. `-as-human` exists for a person genuinely typing, and it warns every time — an agent reaching for it is the antipattern the badge exists to prevent. **The prompt ledger is on, at `prompts` level.** `amadan ledger init` has installed the post-commit hook, the Claude Code hooks in `.claude/settings.json`, and the ledger refspecs, so prompts record to `refs/amadan/ledger/` with nothing to do per commit. Two consequences worth knowing: - **A bare `git push` now pushes HEAD *and* the ledger refs**, because `remote.origin.push` is set and `push.default` no longer applies. - **Never edit ledger refs by hand.** `amadan ledger redact` and `amadan ledger withhold` are the only sanctioned removals, and both leave a visible mark. That is the point of them. **Every branch gets a discussion, and it is the review record.** `amadan discuss new /docs -branch ` opens it; that thread is where the branch is read as one piece, where the highest-capability-model review is posted, and where approval is recorded. One thread per topic — reply on it rather than opening a sibling. Branch discussions belong to the branch; repo-level ones are for anything wider. Close what you opened. **Tasks are a checklist, not a log.** `amadan task add -branch ` for work someone will do, `amadan task advance` when it is done. Commentary belongs in the discussion, not in task titles. A branch with real work on it should have tasks that a reader can scan to see what is left. **Rebuilding the editor bundle is not hand-mergeable** (branch convention, agreed 2026-08-31 between the three sessions then working in this repo). `internal/docs/static/editor..js` is a single committed artifact whose name is the hash of its own contents, and `assets.go` panics at init if it finds more or fewer than one. Two branches that both ran `fixtures/build-editor.mjs` therefore conflict mechanically even when no source line overlaps. Whoever squash-merges second discards their own bundle from the merge and re-runs the build on top of `main`. Never reconcile the minified file by hand. **When something is refused, say so in the discussion rather than working around it.** An `agent` grant is writer-equivalent within one namespace: push, post, manage tasks, nothing beyond. Refusals are usually a missing grant or a genuine boundary, and both are worth surfacing rather than routing around. ## Working with the sibling sessions Docs is one of three moving parts and the other two are live: - **`../sheets`** (`amadan.net/oficina/sheets`) — shares the suite home, the design-system primitives, and several rulings. Its spec is `docs/superpowers/specs/2026-08-30-sheets-design.md`. Docs is a declared consumer of the home contract in its §9. - **The design system** — `rastrillo.org/design-system`, spec at `docs/superpowers/specs/2026-08-28-design-system-design.md` in the rastrillo repo. Vendor tokens through `ui.TokensCSS()` and `ui.ThemeCSS(name)` rather than copying files by hand. When a decision affects one of them, tell that session rather than writing around it. Several of this project's better decisions came from one session's question exposing another's bug. ## Conventions that are already settled - **Markup uses the bare `rst-*` attribute grammar** (`
`), variants in the value matched with `~=`. Unparked by Paul 2026-08-31, once both of his conditions were met and verified: `rastrillo v0.21.0` styles **both** spellings (`ui/tokens.css` carries 343 bare-attribute selectors alongside 398 class ones), and the design-system guidance exists. Two constraints come with it. **Adopt per screen, not per element** — mixed authoring inside one component matches neither spelling by design. And **pin `v0.21.0` or later**, because on anything earlier attributes render unstyled. *(This reversed twice before landing here, both times by applying a conclusion drawn about one repo to a different one without re-deriving it. The reasoning is in spec §10.1.1.)* - **Anything Docs contributes to estilo takes an `estilo-*` prefix, never `rst-*`.** That namespace is the framework's and a future version may claim a name we took — rastrillo's own migration found `rst-form__foot` and `rst-form-foot` both live inside `ui`. - **A spec ruling, code on `main`, and code in the latest release are three different things.** Reading one tells you nothing about the other two. `v0.19.0` (2026-08-24) is 18 commits behind rastrillo's `main` and lacks the themes, shells and locales the design-system spec describes, so `go get` alone will not give you them. Say which of the three you mean whenever you cite the framework, and stamp any commit count with the sha and date you took it at — a bare number ages into a wrong fact. ### Watch for tools that answer wrongly instead of failing A tool that errors teaches you something. A tool that returns a confident wrong answer does not, and both of the ones below cost this project real time in a single afternoon: - **Citing your working tree instead of `origin/main`.** A checkout parked on a feature branch gives you *true statements about the wrong tree*, complete with real line numbers that verify. Cite `origin/main` after a fetch, or a tag, and say which. Never your checkout. - **`git branch --merged` against a squash-merging repo.** It returns an empty list that reads as "never merged" while `main` carries every line, because a squashed branch is not an ancestor. Compare content, not ancestry. - **`carlos channels` after a promote.** It reports which release a channel *points at*, which is not the same question as which release the box is *serving*. Adoption runs on an hourly timer, so the two can disagree for up to an hour after `carlos promote`: observed 2026-09-01, `edge` reading the new version while the live host served the previous one for five minutes straight, closed within seconds by `carlos restart`. Neither command fails and nothing says "not live yet". Trust `x-carlos-version` on the real host, or use `carlos deploy`, which ships, promotes and then watches the URL until it actually serves the new version. `carlos channels` cannot distinguish "serving it" from "pointed at it". *(Recorded from the live-sync deploy; not re-verified since, so the timer's exact behaviour is one observation rather than a measurement.)* - **A library function that ignores its own "dry run" argument.** `y-prosemirror`'s exported `undo` is `state => ...undoManager?.undo()` — it takes no `dispatch` and undoes immediately. ProseMirror's command contract says a null `dispatch` performs nothing and only reports whether it could, which is how any toolbar decides whether to grey a button out — on every state update, because that is when the answer changes. So the formatting bar asked "could I undo?" after every change and the library answered by undoing it: typing did nothing, every command did nothing, no error anywhere, Go suite green. (`undoCommand`/`redoCommand` are the dispatch-aware pair.) The general form: **a convention is not a guarantee**, and the functions that break one are the ones that look most like they honour it. Found 2026-09-01 on the formatting-and-export branch, by a browser drive, after an evening of looking at the wrong file. The shape to watch for is a verification that cannot distinguish "no" from "I can't see it". When you catch one, write it here rather than resolving to be more careful — care is not a mechanism. **A new test must be seen to fail.** Corollary of the above, and cheap: after writing one, break the thing it covers and watch it go red, then put it back. `scripts/browser-drive.sh` was kept only after it was confirmed to fail on the build whose bug it had just found — without that step a green run means "passed" and "could not see" equally. **And it must fail for the right reason.** Watching it go red is not enough — read *why* it went red. Three of these were found in one evening on the `list-thumbnails` branch, in three different shapes: - A test that could not fail at all. `TestPageOneDoesNotDisturbPDFExport` exported a document, ran the thumbnail layout, exported again and compared bytes — but the two calls share no state, so only a package-level global could ever have reddened it. It stayed green while a deliberate `l.cursor += 100` inside the code it claimed to guard reddened a different test. Two comments and a commit message cited it as proof. Deleted. - A mutation that reddened the right test for the wrong reason. Proving a canary test could fail, the injected leak used a template variable that was not in scope, so the page 500ed and the test failed on its status check without the canary ever firing — a red test proving nothing. Redone with a leak that actually renders. **If your mutation changes the response code, you have tested the mutation, not the test.** - A hit test that measured something else. Recorded in `list.html`: restated declarations "fixed" row clicking, and the probe was hitting rastrillo's own overlay both before and after. The generalisation, and the question worth asking of any gate: **what would make this go red, and have I actually done it?** A CSS rule keyed on an attribute nothing writes is the same defect in another costume — it matches no element, so it is invisible to every test and every screenshot, and any bug inside it is unreachable. The Sheets session found one of those the same night, hiding a second defect behind it. Worth a grep when touching a stylesheet; not worth a permanent gate, because it has enough false positives (comments quoting old selectors, framework-emitted hooks) that people would learn to skim it — and a gate nobody reads is another probe that answers the same in both worlds. **Do not pass on somebody else's finding as your own verification.** A report from another session is evidence about what they saw, not a fact you have checked, and relaying it flatly launders it into one. Say where it came from, or check it — and note that "two of us noticed independently" is not corroboration when the second of you is the one who mentioned it first. (Written after exactly that: a merge commit on `main` was reported here as a breach of the shipping cadence and passed on to Paul as one, when `amadan branch merge` produces a merge commit itself and the branch had landed correctly. One minute with the tool would have caught it.) - **rastrillo is `amadan.net/rastrillo/rastrillo`.** The old path, `github.com/carlosframework/rastrillo`, stops at v0.23.0: code importing it gets no fix, and its own checks cannot tell (Docs zoomed iOS on every tap for a month that way). `TestNoPreMoveRastrillo` (internal/lint) allows one import of it, native.go's bridge to rastrillo/native's server, until that server moves. The CLI is pinned as a `tool` in go.mod: run it as `go tool rastrillo …`, never a `rastrillo` from PATH. `scripts/build-release.sh` stamps `amadan.net/rastrillo/rastrillo.BuildVersion`; the old symbol leaves `/api/version` answering "dev" (`TestReleaseBinaryReportsItsVersion`). - **Docs serves rastrillo's and estilo's assets from memory** (render.go's `vendoredStaticFS`), so `go tool rastrillo doctor` finds nothing vendored to check. If a future `doctor --fix` ever writes a rastrillo.js, busy.js or tokens.css to disk, delete it: doctor's shim carries the old busy rule, which fights estilo's busy.js. `TestBusyRuleIsLinkedAndServed` fails if one is left behind. - **iOS zooms into any text control under 16px.** tokens.css lifts controls to 16px under `(pointer: coarse)`; a Docs rule that sets a control's font with a heavier selector must restate `font-size: max(1rem, 1em)` there too. `TestNoZoomOnTap` (browser tag: `go test -tags browser -run OnTap ./internal/docs/`, needs a Chromium via `RASTRILLO_CHROME` or a Playwright cache) measures every field. - **Re-copy `tokens.css` deliberately on every rastrillo upgrade.** It is frozen into `static/` at scaffold time while the partials upgrade with the module. `rastrillo doctor` will make this mechanical; until it lands it is a manual step in the upgrade runbook. - **Every user-visible string is a catalog key**, resolved at render time. Twelve base locales; a hardcoded English string is a bug. - **WCAG 2.2 AA, no exemptions**: 24×24 targets, 320px reflow, and colour never carries meaning alone. - **An ARIA role is a promise about behaviour, and an unimplemented one is worse than none.** `role="toolbar"` says "this is a single tab stop and the arrow keys move within it". Written without the roving tabindex behind it — as the formatting bar was, on this branch, until it was caught in review — it tells a screen-reader user that arrow keys are the way in, so those are what they try first, and the silence that follows reads as their own mistake rather than as a missing feature. Omitting the role would have been more honest and less harmful. Whenever you write a role, implement its keyboard contract in the same commit or do not write it. Two things that pattern hides, both found the same way: **a roving tab stop parked on a `disabled` control makes the whole group unreachable**, not merely awkward, and disabled is often the *default* state (undo and redo start that way on every page load) — so the "which control is the tab stop" decision has to re-run whenever anything is enabled or disabled, not on load. And **a closed native `