oficina / estilo Public

Shared Home account connection presentation in Estilo open

On branch feat/home-connection

User asked to extend working Home linking across apps and keep reusable UI in Estilo. Candidate dec3908 extracts the common connection and OAuth templates/data into Estilo; security protocol stays home/connect. Based on already reviewed/deployed Estilo672 theme/module transport. Caller-supplied labels support twelve tested locales, no new theme/colors, scriptless post/link/continuation. Back becomes a real button-sized target; addresses use bdi. Full make test passed; actual Fable review running frozen dec3908. Consumer alias/pin and browser evidence follow before activation. No app deployment from this source alone.

Verification complete. Writing the verdict now.

Verdict: APPROVE for merge to main after CI is green and the branch discussion records this review. Activation still requires the consumer integration browser check and each consumer's gates, as stated in the request.

Reviewing model: Claude Fable 5.1 (claude-fable-5-1), read-only, 2026-09-10. Scope: dec3908b vs 672a447, source/AGENTS.md, branch.diff, all four new files under source/homeconnect/, the gate log, and rastrillo v0.23.0's ui/tokens.css in the module cache.

Evidence checked

  • The diff is purely additive: four new files, no changes to theme, Makefile, CI, or go.mod. No auth, key, session, or destination-validation code is present. Package comment and README both push validation to the caller.
  • Every rst-* attribute the templates use has a styled attribute twin in the pinned rastrillo v0.23.0, including the ghost variant. Nothing renders unstyled.
  • Back link target: [rst-btn] computes to roughly 34px tall at the day theme's small font size, plus padding and border. Clears the 24px floor without relying on the spacing exception.
  • Escaping: href and action are URL contexts, so javascript: becomes #ZgotmplZ. The test proves this and proves & becomes & in the OAuth anchor.
  • Gate log shows vet, build, and all four packages passing. test-fmt is silent on success, so the log is consistent with a green make test.
  • Inline style="overflow-wrap: anywhere" works under rastrillo's default CSP, which allows 'unsafe-inline' for styles (serve.go:398).

Non-blocking findings

  1. Low, design. The Back link destination is hardcoded to / in home_link.html:7. An app mounted under a prefix or with a non-root landing page cannot correct it. Acceptable for a first extraction that mirrors current app behaviour. Per AGENTS' "second use corrects it", add a CancelHref field when the second consumer needs one rather than now.
  2. Low, test gap. The escaping test covers home-connect only. home-oauth-head puts the URL in a content attribute, which html/template treats as plain attribute text, not a URL context, so an unsafe scheme there would not be neutralised. Design intent is that the shared security package validates first, and browsers refuse javascript: in meta refresh, so this is informational. A one-line test asserting & in the head output would document the contract.
  3. Low, doc staleness outside this branch. AGENTS §Vocabulary still says the attribute grammar "exists in no rastrillo tree". v0.23.0 ships attribute twins for every selector. Worth a follow-up line so the next reader does not "correct" these templates back to classes.
  4. Informational. The templates use rastrillo's vocabulary in attribute form and ship no CSS, so the internal/lint prefix tripwire is not engaged. That is the correct row of the three-way split for markup that consumes rastrillo primitives rather than defining estilo ones.

Conditions already stated by the requester and confirmed as necessary

  • Push is blocked by Amadan 503, so the branch discussion cannot yet record this review. AGENTS makes that record a merge condition, not optional.
  • Consumer adoption must be reviewed separately. Parsing these definitions into an app template set that still defines the same names will conflict, so the old app definitions must go in the same consumer change.

No commands were run and no files were written, per the request.

Verdict: APPROVE for 9fbeff92 vs dec3908b. Reviewing model: Claude Fable 5.1 (claude-fable-5-1), read-only, 2026-09-10. Write is disabled this session, so this message is the review record.

Verified

  • Diff is exactly five files: AGENTS.md (row 3, §Dependency, §Vocabulary note, split-table row 3), README rewrite, new savestatus/doc.go, theme/estilo.css line 2 only, theme_test.go line 133 message only. Theme body untouched, so byte identity with v0.23.0 day holds and the test is not loosened.
  • Gate log shows vet, build, all four packages green. test-fmt is silent on success, consistent with a green make test.
  • No stale v0.19, v0.22 or "no code yet" text remains in prose. Only the harmless stale go.sum lines noted in the theme review.
  • Vocabulary rewrite is accurate. rastrillo v0.23.0 tokens.css pairs every component class with an attribute twin and partials emit the attribute spelling. Utility classes (rst-sr-only, rst-mono) remain classes, which "attributes in v0.23.0" glosses over. Fair for component vocabulary.
  • README facts check against the tree: module path and GOPRIVATE match go.mod and row 7; theme claim matches the test; save-status states match status.go; homeconnect description matches view.go; Building section matches the Makefile; Node claim matches status_test.go:66.
  • doc.go carries the script-required line AGENTS.md demands.

Approval chain, whole tree

ad0edf4 → c5f695a (full-tree APPROVE, read every baseline file) → 672a447 (diff APPROVE) → dec3908 (diff APPROVE) → 9fbeff9 (this). Diff reviews compose, so the full tree at 9fbeff9 is covered end to end, all by claude-fable-5-1. Pushing ad0edf4 as unborn main is the supported route and was assessed in the c5 review; its conditions stand. Squash with --expect 9fbeff9 is correct after the conditions below.

Not verified by me: git ancestry. No repository is present here, so the runner's diffs are trusted. Confirm 9fbeff9 descends from dec3908, 672a447, c5f695a, ad0edf4 before merging.

Merge conditions

  1. Hub CI green at exactly 9fbeff9, checked not assumed.
  2. Branch discussion records all four verdicts with model before squash. Discussion 2's snapshot has no theme verdict post; discussion 1 post 3 only says the c5 review is queued. Squash makes the discussion the only record.
  3. Record the baseline push in discussion 1: main initialized from ad0edf4 with the c5 review as its record, plus a green CI run at ad0edf4 or the statement that the same code passed at 5f712c7 and c5f695a.
  4. Keep the feature branches until Docs and Slides repin. Their pseudo-versions point at 532746a and 672a447.

Explicit override of one earlier condition. The c5 review made dropping the /tmp citation at implementation/save-status.md:96 a merge precondition. It is still there. I downgrade it to a tag follow-up: lines 3–5 of the same file already point to discussion 1, which contains both verdicts, so the path is redundant rather than misleading. If root prefers the earlier bar, it is a one-line deletion and a re-review of the new SHA.

Non-blocking, before tag

  • save-status.md:96 drop the /tmp path; line 112 missing space ("of5f712c7").
  • AGENTS.md row 3 "Who, when" still says 2026-08-30. The resolution date (2026-09-08) is recorded nowhere, which the c5 review asked for.
  • AGENTS.md line 116 example <div rst-inspector> contradicts row 4 and the prefix lint. Pre-existing.
  • doc.go meets the AGENTS bar but not the fuller ask (document Data, template name, consumer-owned semantics).
  • README "already adopted by Docs and Slides" is true at branch level only, until repins land. User-edited copy, noted only.
  • Theme header still lacks upstream day's --rst-header-rule paragraph.
  • GOPRIVATE differs between AGENTS.md (amadan.net/oficina/estilo.git) and save-status.md (amadan.net). Both work.

Consumer gates unchanged: Home must refresh its frozen tokens asset when it takes this revision. No tag, release or deploy is implied.

Final source9fbeff92d33fc245ce0da592095829bce931bda1 merged through Amadan as9873c648660c00bd12e2ddfa8c5eb3b924b49be7; root verified identical git trees. No tag cut. Shared homeconnect templates at reviewed reachabledec3908 are now adopted by all five native app previews through connect5b8d841, with mandatory gates, exact CI, Fable adoption approval, eight synthetic rendered browser checks per app and four live redirect checks per app. No local duplicate component definitions remain.