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 <team>.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 <n> 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 <n> 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 <name>~<operator keymail>; 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/<branch> 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 <ns>/docs -branch <n> 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 <n> 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.<hash>.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** (<div rst-inspector>), 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 <select> changes its own value on Left/Right, so a toolbar containing one must consume those keys and leave Up/Down alone. Neither is in the ARIA practices; both are unmissable in a browser and invisible in a unit test, which is what scripts/browser-drive.sh is for.

Work in a worktree, and catch main up first

Take a worktree for the work. Do not work in the shared checkout.

git -C <repo> fetch origin git -C <repo> worktree add ../<repo>-<task> -b <branch> origin/main

Both halves matter, for different reasons.

The worktree, because several sessions share this machine and some share this repository, and a checkout has one HEAD, one index and one working tree between all of them. A branch switch under somebody else's uncommitted work destroys it with nothing on screen to say so, and two sessions editing the same paths disagree quietly instead of conflicting loudly. A worktree gives the work its own HEAD and its own files, and costs a directory.

From origin/main, after a fetch — not from whatever the checkout happens to be sitting on. A branch cut from a stale main is a merge conflict scheduled for later, against files somebody else has renamed in the meantime. A branch cut from another task's branch is worse: it carries that task's commits into yours, where they are reviewed as though you wrote them, and the two can only be separated by hand.

Remove it when the work has landed — git worktree remove — so that git worktree list keeps saying something true.

Written down because of 2026-09-17, in meet. The shared checkout was sitting on one task's branch while holding a different task's feature uncommitted on disk — some four hundred lines, a new migration among them — put there by a session nobody else knew was working in that tree. Two other sessions were meanwhile coordinating over the same checkout by asking each other not to touch it, which is the arrangement a worktree makes unnecessary. Nothing was lost. Nothing about the arrangement would have said so if it had been, and by then the same feature existed twice, once on each side of a rename, because the two sessions had branched from different starting points.