oficina / memoria Public

🤖 fix: the third review — a tone bug fixed twice wrongly, and a spec

amendment made without reading the spec

Nineteen mutations, thirteen survived, three of them live defects. The
two worst are both this project's named failure, committed while
repairing an instance of it.

**The callout tone, third attempt.** tokens.css pairs every kind: the
class spelling takes `data-tone`, the attribute spelling takes
`rst-tone`, and the file header says so in a sentence. Attempt one wrote
`rst-callout="danger"` — a value on the kind attribute, which nothing
selects. Attempt two, the commit that described fixing it, wrote
`[rst-callout] data-tone=` — half of one pair and half of the other,
matching neither — and its three new assertions passed because the
string `data-tone="negative"` does occur in the stylesheet, inside the
class selector standing next to the one needed. Every flash in the
product still rendered untoned.

The lesson is not about tones: **grepping for a string is not reading a
selector.** The gate now derives its vocabulary from `[rst-…` selectors
only, checks each *value* against `name~="value"` rather than against
"something somewhere takes a value", and rejects `data-tone` on an
attribute-spelled element by name.

That found two more, both pre-existing and app-wide: `rst-cell-mut` is a
class-only utility, so every `<p rst-cell-mut>` selects nothing; and
`rst-btn` takes primary, ghost and danger, so every button spelled
"quiet compact" renders as the default. **Neither is fixed here** —
which variant a quiet button should be is a visual decision, not a
defect with one repair — both are listed by name in the gate, dated, and
flagged for Paul.

**The ASKED counter is reverted, and §8.1's clause moved instead.** The
last commit added a third integer, arguing that §8.1's "the window
contains at least one NOT_VISIBLE" could not fail. The observation was
right; the remedy was wrong twice. §13 already carried the correct fix —
*require each counted asker to have `VISIBLE = 0` and `NOT_VISIBLE ≥ 1`*
— and §8.1 was amended without opening it. Per asker, all three clauses
collapse into one condition, "asked in the window" needs no storage
because a refusal is itself the evidence, and the only per-person record
memoria keeps stays the size §3.2 says it is instead of quietly gaining
a "how often did they query" count.

The window-global version had a live consequence the fixtures missed: a
24-hour outage plus one routine refusal of one Docs-only member fires
the refusal breaker — the exact case the prose says is excluded.

**`PruneTallies` was never called.** It had a comment saying a retention
rule nothing enforces is a sentence in a document. That is what shipped.
It runs on every wake now.

**The breaker could be unwired and the suite stayed green** — deleting
the `recordForBreaker` call, and the `StartCounting` loop, both passed.
The predicate and the wiring were tested separately and the join not at
all, which is how a breaker ships reading "cannot determine" forever on
a working instance.

The rest: the counter write moves to `context.WithoutCancel` (AGENTS.md's
rule for additions to derived state) and the comment claiming a dropped
counter yields "cannot determine" is replaced with what actually happens,
because it does not; gate 8's placement half asserted against a slice the
test wrote itself and is replaced by one reading a real search's notices;
two predicate clauses gained the fixtures that pin them; the retention
test's stated reason was false (day 7 is the oldest baseline day, not day
8); and an unreachable "cannot determine" case was removed rather than
described.

Twelve mutations re-run against the fixes, all red, none surviving —
including the two that survived the first pass. `NotVisible >= 1` was one
of them: with no zero-rows written it is unreachable, the same shape as
the counter it replaced, so rather than delete a correct rule the row is
now inserted directly by a test. `make ci` green, `-race -count=2` clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01H3VKtXnq6fSmM2F11Bjcsy
Paul Campbell pushed by paul@keymail.dev 297514bf98d8370a4492352046df1ae05ccbe506 parent b790e67
15 files changed, +782 −118
  • docs/superpowers/specs/2026-09-03-memoria-design.md +26 −14
  • internal/memoria/breaker.go +57 −42
  • internal/memoria/breaker_test.go +134 −15
  • internal/memoria/breaker_wiring_test.go +369 −0
  • internal/memoria/custody.go +20 −0
  • internal/memoria/render.go +19 −7
  • internal/memoria/render_test.go +23 −7
  • internal/memoria/store.go +21 −7
  • internal/memoria/templates/connectors.html +2 −2
  • internal/memoria/templates/index.html +1 −1
  • internal/memoria/templates/notion.html +1 −1
  • internal/memoria/templates/results.html +2 −2
  • internal/memoria/templates/signin.html +1 −1
  • internal/memoriatest/gates_test.go +91 −11
  • internal/memoriatest/search_test.go +15 −8
diff --git a/docs/superpowers/specs/2026-09-03-memoria-design.md b/docs/superpowers/specs/2026-09-03-memoria-design.md
index cb6bc3c..5d686a0 100644
--- a/docs/superpowers/specs/2026-09-03-memoria-design.md
+++ b/docs/superpowers/specs/2026-09-03-memoria-design.md
@@ -2238,8 +2238,8 @@ a provider — and per §3.2, nothing leaves the instance in this mode.
per-refusal log would answer it too and is not built, because the
aggregate is sufficient and the log is not.
- **Three integers, and the window is a UTC day. Corrected 2026-09-07,
- because this section asked for two things it cannot have at once.**
+ **The window is a UTC day, and the NOT_VISIBLE clause is per counted
+ asker. Corrected 2026-09-07.**
It says the counters are keyed per `(sibling, asker, UTC day)` and that
the window is *rolling 24 hours*. Two integers a day cannot answer a
@@ -2254,18 +2254,26 @@ a provider — and per §3.2, nothing leaves the instance in this mode.
positive, which is the right direction for a notice whose purpose is not
to cry wolf.
- And the counters are **three integers, not two**: `VISIBLE`,
- `NOT_VISIBLE`, and `ASKED`. The third is not bookkeeping. Two clauses of
- the predicate — "asked at least one pair inside the window" and "the
- window contains at least one `NOT_VISIBLE`" — are both about people who
- asked and got *neither* answer, which is exactly what an outage
- produces. With two counters an all-`UNKNOWN` day writes no row at all,
- so those askers never enter the window, so **neither clause can fail**:
- the mutation this section names as (ii) leaves the code green because
- the branch it changes is unreachable. Found while building it, by
- running that mutation. A guard that cannot fail is one written to pass,
- and here it would have been the guard keeping an outage from being
- reported as a permissions anomaly.
+ **And the third clause binds each counted asker, not the window.** It
+ read "the window contains at least one `NOT_VISIBLE`", which §13 had
+ already recorded as wrong: window-global, it is satisfied by one routine
+ refusal of a Docs-only member, so a 24-hour outage plus that single
+ query fires the refusal breaker — the exact case the prose above says is
+ excluded. Per counted asker — **`VISIBLE = 0` and `NOT_VISIBLE >= 1`** —
+ all three clauses collapse into one condition, and "asked in the window"
+ stops needing its own storage: a refusal *is* the evidence that somebody
+ asked.
+
+ *An amendment made here on 2026-09-07 added a third counter, `ASKED`,
+ arguing that the clause could not otherwise fail. The observation was
+ right — an all-`UNKNOWN` day writes no row, so an outage's askers never
+ enter the window and mutation (ii) stays green — and the remedy was
+ wrong twice over. §13 already carried the correct fix, and this section
+ was amended without opening it, which is the failure AGENTS.md calls
+ characteristic, committed while repairing another one. And the third
+ integer enlarged the only per-person record memoria keeps — a "how often
+ did they query" count — without §3.2's leak list being opened either.
+ Both reverted; two integers, and the clause moved instead.*
**A window that cannot be read says so.** If the counters are missing,
unreadable or short of a full baseline — a fresh instance, a restore, a
@@ -3666,6 +3674,10 @@ on 2026-09-06 when the branches merged.
window was all `UNKNOWN` is indistinguishable from one who never
asked. Both are fixed by requiring each counted asker to have
`VISIBLE = 0` and `NOT_VISIBLE ≥ 1` in the window.
+ **Adopted 2026-09-07**, in §8.1 and in the code, after a review found
+ the branch built against the old wording. Kept in this list because
+ the route matters: the fix was written down here first, and the
+ implementation went the other way for a day because nobody opened it.
- **Gate 27's fixture arithmetic cannot go green** — 40 Docs chunks
cannot render 50 results — and its first bullet asserts what the
*fixture* returns, so it cannot go red for memoria's code.
diff --git a/internal/memoria/breaker.go b/internal/memoria/breaker.go
index 9ba151b..be543d2 100644
--- a/internal/memoria/breaker.go
+++ b/internal/memoria/breaker.go
@@ -62,31 +62,33 @@ func breakerBaseline(now time.Time) (from, to string) {
day.AddDate(0, 0, -1).Format("2006-01-02")
}
-// tally is the counters in a SiblingTally row.
+// tally is the pair of integers in a SiblingTally row. §8.1's two, and
+// two is enough.
//
-// **Three numbers, not the two §8.1 names, and the third is load-bearing.**
-// The section's predicate has a clause requiring at least one NOT_VISIBLE
-// in the window, whose whole job is to keep an outage quiet — an outage is
-// all UNKNOWN, so counting VISIBLE-absence instead of NOT_VISIBLE-presence
-// reports an availability problem as a permissions anomaly.
+// **A third — ASKED — was added here and then removed, and the round trip
+// is worth recording.** The argument for it was that §8.1's clause
+// "the window contains at least one NOT_VISIBLE" could not fail: an
+// all-UNKNOWN answer writes no row, so an outage's askers never enter the
+// window and flipping the clause changes nothing. That observation was
+// right and the remedy was wrong.
//
-// With only two counters that clause **cannot fail**, and a mutation
-// proved it: an all-UNKNOWN answer writes no row at all, so the asker
-// never enters the window, so nothing reaches the clause and flipping it
-// changes nothing. The guard was unreachable — which AGENTS.md says is the
-// same object as a guard written to pass, whatever the intent.
+// The spec's own §13 already carried the real finding, and this amended
+// §8.1 without opening it — the failure AGENTS.md calls characteristic,
+// committed in the act of fixing another one. §13: *"the breaker's
+// NOT_VISIBLE clause is window-global, not per counted asker, so one
+// routine query from a Docs-only member plus a 24-hour outage still fires
+// the refusal breaker … Both are fixed by requiring each counted asker to
+// have VISIBLE = 0 and NOT_VISIBLE >= 1 in the window."*
//
-// Asked is what puts an outage's askers in the window with zero of
-// everything else, so the clause is the only thing keeping them quiet and
-// breaking it goes red. It costs one integer and it is the difference
-// between a rule and a sentence.
+// That fix is strictly better. It closes a live hole a third counter does
+// not touch; "asked in the window" becomes computable from the two
+// integers, because a NOT_VISIBLE is itself proof the person asked; and it
+// leaves the only per-person record memoria keeps as small as §3.2 says
+// it is, instead of quietly adding a per-person "how often did they
+// query" nobody had declared.
type tally struct {
Visible int `json:"v"`
NotVisible int `json:"n"`
- // Asked counts calls this person made to this sibling, whatever came
- // back. Absence is not refusal (§8.1), and this is what tells them
- // apart.
- Asked int `json:"a"`
}
// RecordAuthorizations files one S2 answer set against the breaker's
@@ -99,11 +101,16 @@ type tally struct {
// **A counter that cannot be written is dropped, never propagated.** The
// caller is in the middle of answering somebody's search; failing that
// because a metrics row would not save would be trading a working page for
-// a broken one. What it costs is a gap in the counters, and a gap makes
-// the breaker report *cannot determine* rather than *no anomaly* — which
-// is the same rule as everywhere else in this design, and the reason
-// dropping is safe here and would not be if the failure direction were
-// reversed.
+// a broken one.
+//
+// What it costs is stated plainly rather than flattered: the period reads
+// slightly quieter than it was. Nothing records the gap, so this is a loss
+// of fidelity and not a "cannot determine" — an earlier version of this
+// comment claimed the latter, which would have been the safe direction and
+// was not true. It is tolerable because the breaker needs three separate
+// askers to fire and one dropped write moves one of them, and because the
+// loss is logged. If that stops being enough the fix is a per-(app, day)
+// gap marker that yields cannot-determine, not a stronger sentence here.
//
// UNKNOWN and STALE are counted as neither. An outage produces UNKNOWNs
// and no NOT_VISIBLE, and §8.1 is explicit that it must not trip a breaker
@@ -123,7 +130,7 @@ func (s *Store) recordAuthorizationsAt(ctx context.Context, app string, asker su
if app == "" || asker.Value == "" || len(answers) == 0 {
return nil
}
- add := tally{Asked: len(answers)}
+ var add tally
for _, a := range answers {
switch a {
case suite.VISIBLE:
@@ -131,9 +138,16 @@ func (s *Store) recordAuthorizationsAt(ctx context.Context, app string, asker su
case suite.NOT_VISIBLE:
add.NotVisible++
}
- // STALE and UNKNOWN are counted in Asked and nowhere else. They
- // withhold loudly, which is neither permission nor refusal, and
- // §8.1 requires this breaker to see an outage as neither.
+ // STALE and UNKNOWN are counted nowhere. They withhold loudly,
+ // which is neither permission nor refusal, and §8.1 requires this
+ // breaker to see an outage as neither.
+ }
+ if add.Visible == 0 && add.NotVisible == 0 {
+ // Nothing this breaker counts, so no row. An all-UNKNOWN answer
+ // set is an outage, and the predicate below reads an absent row
+ // and a zero row identically — a person is counted as refused
+ // only by a NOT_VISIBLE, never by the absence of a VISIBLE.
+ return nil
}
print, err := s.sealer.Fingerprint(seal.PurposeAsker, []byte("asker\x00"+asker.Value))
@@ -167,7 +181,6 @@ func (s *Store) recordAuthorizationsAt(ctx context.Context, app string, asker su
}
cur.Visible += add.Visible
cur.NotVisible += add.NotVisible
- cur.Asked += add.Asked
sealed, err := s.sealTally(cur)
if err != nil {
return err
@@ -314,30 +327,33 @@ func (s *Store) RefusalBreaker(ctx context.Context, app string, now time.Time) (
}
}
+ // **Per counted asker: `VISIBLE == 0` AND `NOT_VISIBLE >= 1`.**
+ //
+ // §8.1 words the second half as a property of the window — "the window
+ // contains at least one NOT_VISIBLE" — and §13 records why that is
+ // wrong: window-global, it is satisfied by one routine refusal of a
+ // Docs-only member, so a 24-hour outage plus that one query fires the
+ // breaker. The exact case the prose says is excluded.
+ //
+ // Per asker, all three of §8.1's clauses fall out of one condition.
+ // An outage gives its askers no NOT_VISIBLE, so none is counted.
+ // Somebody who did not query has no row, and no row is no
+ // NOT_VISIBLE — "absence is not refusal" needs no separate counter,
+ // because a refusal is the only thing that counts as having asked.
refused := 0
- anyNotVisible := false
for print, t := range window {
- if t.NotVisible > 0 {
- anyNotVisible = true
- }
if !allowed[print] {
// Not in the baseline. A Docs-only member refused by Sheets
// all day is the routine case, however many times it happens.
continue
}
- if t.Asked == 0 {
- // Did not actually ask. Absence is not refusal — §8.1's third
- // clause, and the reason Asked exists as its own counter
- // rather than being inferred from the other two.
- continue
- }
- if t.Visible == 0 {
+ if t.Visible == 0 && t.NotVisible >= 1 {
refused++
}
}
b.Askers = refused
- if refused >= breakerAskerFloor && anyNotVisible {
+ if refused >= breakerAskerFloor {
b.Verdict = BreakerFiring
return b, nil
}
@@ -367,7 +383,6 @@ func (s *Store) tallyRange(ctx context.Context, app, from, to string) (map[strin
cur := out[r.AskerPrint]
cur.Visible += t.Visible
cur.NotVisible += t.NotVisible
- cur.Asked += t.Asked
out[r.AskerPrint] = cur
}
return out, nil
diff --git a/internal/memoria/breaker_test.go b/internal/memoria/breaker_test.go
index cfb44b5..339bbdd 100644
--- a/internal/memoria/breaker_test.go
+++ b/internal/memoria/breaker_test.go
@@ -282,10 +282,25 @@ func TestTalliesAreRetainedEightDaysAndNoLonger(t *testing.T) {
if len(got) != 4 {
t.Fatalf("%d rows left, want 4 (days 0, 1, 7 and 8)", len(got))
}
- // The boundary itself stays: 8 days is retained, 9 is not.
+
+ // **Day 7 is the oldest day anything reads** — the baseline runs
+ // `[today-7, today-1]` — and day 8 is the spare.
+ //
+ // An earlier version of this said day 8 was "the oldest baseline day",
+ // which is simply false, and a review caught the test explaining
+ // itself with the wrong reason. §8.1 says "retained 8 days and no
+ // longer" and in the same breath calls that "the window plus the
+ // baseline plus one", which is 1 + 7 + 1 = 9 distinct days. The two
+ // numbers in that sentence disagree; the code keeps nine, because the
+ // spare day is what the "plus one" is for and losing it to a prune
+ // that runs a minute before midnight would shorten a real baseline.
+ if !got[daysBefore(7).Format("2006-01-02")] {
+ t.Error("the oldest baseline day was pruned; every window is then " +
+ "computed against less history than the predicate assumes")
+ }
if !got[daysBefore(8).Format("2006-01-02")] {
- t.Error("the day-8 row was pruned; the rule is 'retained 8 days', " +
- "and losing the oldest baseline day shortens every window")
+ t.Error("the spare day was pruned; a prune running just before " +
+ "midnight would take a day the next baseline needs")
}
if got[daysBefore(9).Format("2006-01-02")] {
t.Error("the day-9 row survived; this is the only per-person record " +
@@ -526,17 +541,15 @@ func TestTheBreakerNoticeReachesTheOwnerAndNoReader(t *testing.T) {
t.Error("a firing breaker did not mark its row for attention")
}
- // **The number is the disclosure.** Whatever the results page says
- // about withholding, it must not carry this sentence — a reader must
- // not learn how many people a sibling stopped answering for, nor how
- // many passages they themselves were refused.
- var page Page
- page.Notices = []string{"Sheets did not answer; 2 results withheld."}
- for _, n := range page.Notices {
- if strings.Contains(n, "refusing") || strings.Contains(n, "used to allow") {
- t.Errorf("a reader's notice carried the breaker's sentence: %q", n)
- }
- }
+ // **The placement half lives in TestTheFiringCountReachesOnlyTheOwner**,
+ // which runs a real search and reads the notices it actually produced.
+ //
+ // It was here, and it was worthless: it built a `Page` in the test,
+ // wrote a notice into it by hand, and then checked that the string it
+ // had just written did not contain the breaker's words. A review
+ // rendered the firing sentence straight into results.html and this
+ // stayed green, which is exactly what an assertion about your own
+ // fixture is worth.
}
// "Cannot determine" says which half it lacks, and §10 gate 8 asserts the
@@ -549,12 +562,17 @@ func TestTheBreakerNoticeReachesTheOwnerAndNoReader(t *testing.T) {
// MUTATION-TESTED 2026-09-07: return the bare phrase without the Lacking
// list, and this goes red. Reverted.
func TestCannotDetermineSaysWhichHalfIsMissing(t *testing.T) {
+ // The two combinations `RefusalBreaker` can actually produce. A
+ // window-only lack is unreachable — counting cannot begin after today
+ // without also beginning after the baseline — and a review pointed
+ // out that testing it describes a state rather than asserting one.
+ // The singular/plural split is still worth pinning, so the
+ // baseline-only case carries it.
for _, c := range []struct {
lacking []string
want []string
}{
{[]string{"the baseline"}, []string{"baseline", "is missing"}},
- {[]string{"the window"}, []string{"window", "is missing"}},
{[]string{"the baseline", "the window"}, []string{"baseline", "window", "are missing"}},
} {
got, alarming := breakerSentence(Breaker{
@@ -633,3 +651,104 @@ func TestOnlyAWrongThingIsAlarming(t *testing.T) {
"\"No refusal anomaly.\" in a warning box")
}
}
+
+// **An outage plus one ordinary refusal must not fire**, and the
+// window-global reading of §8.1's third clause let it.
+//
+// This is §13's finding, found again by a review that built the fixture:
+// Sheets is down and answers UNKNOWN to every baseline asker, while one
+// Docs-only member gets a single routine NOT_VISIBLE. Read globally, "the
+// window contains at least one NOT_VISIBLE" is satisfied by that one
+// query — so a 24-hour outage fires the refusal breaker, which is the
+// exact case §8.1's prose says is excluded.
+//
+// Per counted asker it cannot: each of the three has zero NOT_VISIBLE of
+// their own, so none is counted, and the stranger is not in the baseline.
+//
+// MUTATION-TESTED 2026-09-07, each verified red then reverted:
+//
+// restore the window-global anyNotVisible clause -> red, it fires
+// count `t.Visible == 0` without requiring a NOT_VISIBLE
+// -> red, and this is the same bug by a shorter route
+func TestAnOutageWithOneRoutineRefusalDoesNotFire(t *testing.T) {
+ ctx := context.Background()
+ s := breakerStore(t)
+
+ // Three people who could read from Sheets last week.
+ for _, p := range []string{"ana@x", "ben@x", "cleo@x"} {
+ countOn(t, s, "Sheets", p, daysBefore(3), 5, 0)
+ // Today Sheets is down: they ask, and get UNKNOWN. Nothing this
+ // breaker counts, so no row — which reads the same as not having
+ // asked, and that is correct: an outage tells you nothing about
+ // permissions either way.
+ asker := suite.Subject{Kind: suite.SubjectAddress, Value: p}
+ answers := []suite.Authorization{suite.UNKNOWN, suite.UNKNOWN, suite.UNKNOWN}
+ if err := s.recordAuthorizationsAt(ctx, "Sheets", asker, answers, breakerClock); err != nil {
+ t.Fatalf("record: %v", err)
+ }
+ }
+
+ // And one person who never had Sheets access gets one ordinary
+ // refusal — the single NOT_VISIBLE that the window-global clause
+ // accepted on everybody else's behalf.
+ countOn(t, s, "Sheets", "stranger@x", breakerClock, 0, 1)
+
+ b := verdict(t, s)
+ if b.Verdict == BreakerFiring {
+ t.Errorf("an outage plus one routine refusal fired the breaker "+
+ "(askers=%d): the NOT_VISIBLE clause has to be a property of "+
+ "each counted asker, or one stranger's ordinary Tuesday speaks "+
+ "for three people who were not refused at all", b.Askers)
+ }
+ if b.Verdict != BreakerQuiet {
+ t.Errorf("verdict = %v, want quiet", b.Verdict)
+ }
+ if b.Askers != 0 {
+ t.Errorf("askers = %d, want 0: nobody was refused", b.Askers)
+ }
+}
+
+// The baseline is people the sibling ALLOWED, not people who queried.
+//
+// `t.Visible > 0` is the whole of it. Relaxed to `>= 0` the baseline
+// becomes "anyone who asked last week", and three Docs-only members who
+// happened to query last week and are refused today trip it — draft 4's
+// failure with a week's delay. A review found the clause unpinned.
+//
+// MUTATION-TESTED 2026-09-07: `t.Visible > 0` -> `t.Visible >= 0`. Red.
+// Reverted.
+func TestTheBaselineIsWhoWasAllowedNotWhoAsked(t *testing.T) {
+ s := breakerStore(t)
+ // Three people who asked Sheets last week and were refused every
+ // time. They are not in the baseline, because Sheets never allowed
+ // them anything.
+ for _, p := range []string{"ed@x", "fay@x", "gil@x"} {
+ countOn(t, s, "Sheets", p, daysBefore(3), 0, 6)
+ countOn(t, s, "Sheets", p, breakerClock, 0, 4)
+ }
+ if b := verdict(t, s); b.Verdict != BreakerQuiet {
+ t.Errorf("verdict = %v, want quiet: being refused this week the same "+
+ "way as last week is not a transition, and a breaker that fires "+
+ "for it fires forever", b.Verdict)
+ }
+}
+
+// A mixed day is not a refusal.
+//
+// Somebody who saw four passages and was refused one had an ordinary day.
+// Counting them makes the predicate "was refused at all" rather than "was
+// refused everything", and three ordinary days then fire the breaker. A
+// review found this clause unpinned too.
+//
+// MUTATION-TESTED 2026-09-07: `t.Visible == 0` -> `true`. Red. Reverted.
+func TestAMixedDayIsNotARefusal(t *testing.T) {
+ s := breakerStore(t)
+ for _, p := range []string{"ana@x", "ben@x", "cleo@x"} {
+ countOn(t, s, "Sheets", p, daysBefore(3), 5, 0)
+ countOn(t, s, "Sheets", p, breakerClock, 4, 1)
+ }
+ if b := verdict(t, s); b.Verdict != BreakerQuiet {
+ t.Errorf("verdict = %v, want quiet: seeing four things and being "+
+ "refused one is a Tuesday, not an anomaly", b.Verdict)
+ }
+}
diff --git a/internal/memoria/breaker_wiring_test.go b/internal/memoria/breaker_wiring_test.go
new file mode 100644
index 0000000..336327d
--- /dev/null
+++ b/internal/memoria/breaker_wiring_test.go
@@ -0,0 +1,369 @@
+package memoria
+
+import (
+ "context"
+ "io"
+ "log/slog"
+ "strings"
+ "testing"
+ "time"
+
+ "amadan.net/oficina/suite"
+
+ "amadan.net/oficina/memoria/internal/memoria/seal"
+ "amadan.net/oficina/memoria/internal/memoria/sibling"
+)
+
+// countingCustody answers every candidate the same way and is a real
+// Custody, so a search goes through the whole production path: rule(),
+// recordForBreaker(), RecordAuthorizations().
+type countingCustody struct {
+ apps map[string]sibling.Sibling
+ give suite.Authorization
+}
+
+func (c *countingCustody) Sibling(source string) (sibling.Sibling, bool) {
+ s, ok := c.apps[source]
+ return s, ok
+}
+
+func (c *countingCustody) Rule(ctx context.Context, asker suite.Subject, cands []sibling.Candidate) []sibling.Disposition {
+ out := make([]sibling.Disposition, len(cands))
+ for i := range cands {
+ out[i] = sibling.Decide(c.give)
+ }
+ return out
+}
+
+// **A search writes the breaker's counters**, end to end, and nothing
+// pinned it.
+//
+// A review deleted the `recordForBreaker` call from `Store.rule` and every
+// package stayed green: the breaker's own tests write counters directly,
+// so they prove the predicate and say nothing about whether anything in
+// production ever feeds it. The wiring and the arithmetic were tested
+// separately and the join was not tested at all — which is how a breaker
+// ships permanently reading "cannot determine" on a working instance.
+//
+// MUTATION-TESTED 2026-09-07, each verified red then reverted:
+//
+// delete the recordForBreaker call in Store.rule
+// -> red, no counters at all
+// record under the request context rather than WithoutCancel
+// -> green here (the ctx is not cancelled in this test) and NOT a
+// finding this test can make; the rule it enforces is AGENTS.md's
+// about derived state, and its cost shows up under load rather
+// than in a fixture. Listed so the next reader knows it was tried.
+// map Render to NOT_VISIBLE
+// -> red on the visible count
+func TestASearchWritesTheBreakersCounters(t *testing.T) {
+ ctx := context.Background()
+
+ for _, c := range []struct {
+ name string
+ give suite.Authorization
+ visible, refused int
+ wantAnyCounterRow bool
+ }{
+ {"a rendered passage counts as visible", suite.VISIBLE, 1, 0, true},
+ {"a silently omitted one counts as refused", suite.NOT_VISIBLE, 0, 1, true},
+ // An outage counts as neither, and writes nothing at all — which
+ // is what keeps the predicate from reading it as refusal.
+ {"an outage counts as neither", suite.UNKNOWN, 0, 0, false},
+ {"a stale passage counts as neither", suite.STALE, 0, 0, false},
+ } {
+ t.Run(c.name, func(t *testing.T) {
+ s := storeForTest(t)
+ s.custody = &countingCustody{
+ apps: map[string]sibling.Sibling{
+ "Docs": {Name: "Docs", BaseURL: "https://acme.docs.oficina.works", Token: "t"},
+ },
+ give: c.give,
+ }
+ if _, err := s.Ingest(ctx, IngestSpec{
+ Source: "Docs", Key: "docs/a", Title: "A",
+ Body: "The quarterly planning process runs in March.",
+ ItemID: "docs-a",
+ SiblingRevision: 3,
+ }); err != nil {
+ t.Fatalf("ingest: %v", err)
+ }
+
+ if _, err := s.Search(ctx, testAsker, "quarterly planning", 10); err != nil {
+ t.Fatalf("search: %v", err)
+ }
+
+ var rows []SiblingTally
+ if err := s.db.G.Find(&rows).Error; err != nil {
+ t.Fatalf("read tallies: %v", err)
+ }
+ if !c.wantAnyCounterRow {
+ if len(rows) != 0 {
+ t.Errorf("%d counter rows for an answer this breaker does "+
+ "not count; a zero row puts the asker in the window as "+
+ "somebody who asked and got nothing", len(rows))
+ }
+ return
+ }
+ if len(rows) != 1 {
+ t.Fatalf("%d counter rows, want 1: a search that answers from "+
+ "a sibling must feed the breaker, or it reads 'cannot "+
+ "determine' on a working instance forever", len(rows))
+ }
+ got, err := s.openTally(rows[0].SealedCounts)
+ if err != nil {
+ t.Fatalf("open: %v", err)
+ }
+ if got.Visible != c.visible || got.NotVisible != c.refused {
+ t.Errorf("tally = %+v, want {%d %d}", got, c.visible, c.refused)
+ }
+ if rows[0].App != "Docs" {
+ t.Errorf("counted against %q, want Docs", rows[0].App)
+ }
+ })
+ }
+}
+
+// **`WireSiblings` starts the counting clock**, and nothing pinned that
+// either.
+//
+// A review deleted the `StartCounting` loop and the suite stayed green.
+// In production that leaves `CountingSince` zero for every sibling, which
+// `RefusalBreaker` reads as "I have not been looking long enough" — so
+// every source reports cannot-determine forever, on an instance that has
+// been counting perfectly well the whole time.
+//
+// MUTATION-TESTED 2026-09-07, each verified red then reverted:
+//
+// delete the StartCounting loop in WireSiblings -> red
+// StartCounting overwrites an existing timestamp -> red on the second
+// call: a wake must not reset the clock, or an instance that
+// hibernates nightly never accumulates a baseline
+func TestWiringStartsTheCountingClock(t *testing.T) {
+ ctx := context.Background()
+ env := func(pairs map[string]string) func(string) string {
+ return func(k string) string { return pairs[k] }
+ }
+ cfg := map[string]string{
+ "MEMORIA_TEAM": "acme",
+ "MEMORIA_SIBLINGS": "Docs=https://acme.docs.oficina.works",
+ "MEMORIA_SIBLING_TOKEN_DOCS": "t",
+ }
+
+ s := storeForTest(t)
+ a := &app{store: s, logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
+ if err := WireSiblings(a, env(cfg), nil); err != nil {
+ t.Fatalf("WireSiblings: %v", err)
+ }
+
+ var row SiblingFeed
+ if err := s.db.G.Where("app = ?", "Docs").First(&row).Error; err != nil {
+ t.Fatalf("no feed row after wiring: %v", err)
+ }
+ first := row.CountingSince
+ if first.IsZero() {
+ t.Fatal("CountingSince is zero after wiring: every sibling then " +
+ "reports cannot-determine forever on an instance that has been " +
+ "counting the whole time")
+ }
+
+ // A second wake must not reset it. memoria hibernates; an instance
+ // that restarts the clock nightly never accumulates seven days.
+ if err := s.StartCounting(ctx, "Docs", time.Now().UTC().Add(time.Hour)); err != nil {
+ t.Fatalf("StartCounting: %v", err)
+ }
+ if err := s.db.G.Where("app = ?", "Docs").First(&row).Error; err != nil {
+ t.Fatal(err)
+ }
+ if !row.CountingSince.Equal(first) {
+ t.Errorf("a second wake moved CountingSince from %v to %v: an "+
+ "instance that hibernates nightly would never accumulate a "+
+ "baseline, and every source would read cannot-determine",
+ first, row.CountingSince)
+ }
+}
+
+// The firing count reaches the owner's page and no reader's.
+//
+// §8.1: a message telling one reader how much they were not allowed to see
+// is a disclosure with a number in it. The first version of this assertion
+// checked a `Page.Notices` slice the test had written itself — it
+// constrained nothing, and a review rendered the breaker sentence straight
+// into results.html with it still green.
+//
+// This asserts the two real surfaces: what `siblingRows` produces for the
+// owner, and what a search's own notices carry.
+//
+// MUTATION-TESTED 2026-09-07, each verified red then reverted:
+//
+// siblingRows returns no Breaker sentence -> owner case red
+// Store.rule copies the breaker verdict into Page.Notices
+// -> reader case red
+func TestTheFiringCountReachesOnlyTheOwner(t *testing.T) {
+ ctx := context.Background()
+ s := breakerStore(t)
+ for _, p := range []string{"ana@x", "ben@x", "cleo@x"} {
+ countOn(t, s, "Sheets", p, daysBefore(3), 5, 0)
+ countOn(t, s, "Sheets", p, breakerClock, 0, 4)
+ }
+ s.custody = &countingCustody{
+ apps: map[string]sibling.Sibling{
+ "Sheets": {Name: "Sheets", BaseURL: "https://acme.sheets.oficina.works", Token: "t"},
+ },
+ give: suite.NOT_VISIBLE,
+ }
+ if _, err := s.Ingest(ctx, IngestSpec{
+ Source: "Sheets", Key: "s/a", Title: "A",
+ Body: "The quarterly planning process runs in March.",
+ ItemID: "s-a", SiblingRevision: 1,
+ }); err != nil {
+ t.Fatalf("ingest: %v", err)
+ }
+
+ // The reader's side: a real search, with a real withholding.
+ page, err := s.Search(ctx, testAsker, "quarterly planning", 10)
+ if err != nil {
+ t.Fatalf("search: %v", err)
+ }
+ joined := strings.Join(page.Notices, " ")
+ for _, forbidden := range []string{"refusing", "used to allow", "people who could read"} {
+ if strings.Contains(strings.ToLower(joined), forbidden) {
+ t.Errorf("a reader's page carried the breaker's words (%q): %q",
+ forbidden, joined)
+ }
+ }
+ // NOT_VISIBLE is the silent value, so this page says nothing at all —
+ // which is also the assertion that no count leaked.
+ if len(page.Notices) != 0 {
+ t.Errorf("a silently-omitted passage produced notices: %v", page.Notices)
+ }
+
+ // The owner's side: the sentence, with its number.
+ b, err := s.RefusalBreaker(ctx, "Sheets", breakerClock)
+ if err != nil {
+ t.Fatalf("RefusalBreaker: %v", err)
+ }
+ if b.Verdict != BreakerFiring {
+ t.Fatalf("the fixture did not fire (%v); nothing above is about "+
+ "placement if there is no notice to place", b.Verdict)
+ }
+ sentence, alarming := breakerSentence(b)
+ if !strings.Contains(sentence, "3") || !strings.Contains(sentence, "Sheets") {
+ t.Errorf("the owner's sentence lacks the app or the count: %q", sentence)
+ }
+ if !alarming {
+ t.Error("a firing breaker did not mark its row")
+ }
+}
+
+// **A zero row is not a refusal**, and this is the only thing that can
+// reach the clause saying so.
+//
+// `RecordAuthorizations` writes no row when an answer set contains neither
+// a VISIBLE nor a NOT_VISIBLE, so nothing on the production path produces
+// `{0, 0}` — which makes `NotVisible >= 1` unreachable through it, and a
+// review's mutation dropping that half stayed green. That is the same
+// shape as the `ASKED` counter it replaced: a guard nothing can reach.
+//
+// The difference is that this one is defending an invariant enforced two
+// functions away, in a different file, on a path a future change could
+// easily alter — write a row to record "this person asked", and every
+// baseline asker in an outage becomes a refusal. So rather than delete the
+// clause, the row is inserted directly here, which is the same remedy
+// `scatter` got in the sibling package: make the branch reachable from a
+// test and pin what it defends.
+//
+// MUTATION-TESTED 2026-09-07, each verified red then reverted:
+//
+// drop `&& t.NotVisible >= 1` -> red, the outage fires
+// count a zero row as an ask -> red, same
+func TestAZeroRowIsNotARefusal(t *testing.T) {
+ s := breakerStore(t)
+
+ // Three people the sibling allowed last week.
+ for _, p := range []string{"ana@x", "ben@x", "cleo@x"} {
+ countOn(t, s, "Sheets", p, daysBefore(3), 5, 0)
+ }
+
+ // Today each has a row saying they saw nothing and were refused
+ // nothing — the shape an "asked, got UNKNOWN" record would take.
+ // Inserted directly, because no production path writes one.
+ for _, p := range []string{"ana@x", "ben@x", "cleo@x"} {
+ print, err := s.sealer.Fingerprint(seal.PurposeAsker, []byte("asker\x00"+p))
+ if err != nil {
+ t.Fatalf("fingerprint: %v", err)
+ }
+ sealed, err := s.sealTally(tally{})
+ if err != nil {
+ t.Fatalf("seal: %v", err)
+ }
+ if err := s.db.G.Create(&SiblingTally{
+ App: "Sheets", AskerPrint: print,
+ Day: breakerClock.Format("2006-01-02"),
+ SealedCounts: sealed,
+ }).Error; err != nil {
+ t.Fatalf("insert: %v", err)
+ }
+ }
+
+ // The presence half: the rows really are there, so what follows is an
+ // assertion about the predicate and not about an empty window.
+ var n int64
+ if err := s.db.G.Model(&SiblingTally{}).
+ Where("day = ?", breakerClock.Format("2006-01-02")).Count(&n).Error; err != nil {
+ t.Fatal(err)
+ }
+ if n != 3 {
+ t.Fatalf("%d zero rows seeded, want 3", n)
+ }
+
+ if b := verdict(t, s); b.Verdict != BreakerQuiet {
+ t.Errorf("verdict = %v (askers=%d), want quiet: three baseline people "+
+ "with nothing recorded against them today were counted as refused, "+
+ "which is what turns a 24-hour outage into a permissions anomaly",
+ b.Verdict, b.Askers)
+ }
+}
+
+// The wake prunes.
+//
+// `PruneTallies` was written with a comment saying a retention rule
+// nothing enforces is a sentence in a document, and then nothing called
+// it — a review found the sentence describing its own state. §3.2: an
+// unbounded version of these counters is a year-long history of who was
+// locked out of what, and it is the only per-person record memoria keeps.
+//
+// MUTATION-TESTED 2026-09-07: drop the PruneTallies call from
+// WireSiblings and this goes red with the old row still there. Reverted.
+func TestWiringPrunesOldCounters(t *testing.T) {
+ s := storeForTest(t)
+
+ // A row far outside any window, and one inside it.
+ countOn(t, s, "Docs", "ana@x", time.Now().UTC().AddDate(0, 0, -40), 1, 0)
+ countOn(t, s, "Docs", "ana@x", time.Now().UTC(), 1, 0)
+ if got := tallyDays(t, s); len(got) != 2 {
+ t.Fatalf("%d rows seeded, want 2", len(got))
+ }
+
+ a := &app{store: s, logger: slog.New(slog.NewTextHandler(io.Discard, nil))}
+ if err := WireSiblings(a, func(k string) string {
+ return map[string]string{
+ "MEMORIA_TEAM": "acme",
+ "MEMORIA_SIBLINGS": "Docs=https://acme.docs.oficina.works",
+ "MEMORIA_SIBLING_TOKEN_DOCS": "t",
+ }[k]
+ }, nil); err != nil {
+ t.Fatalf("WireSiblings: %v", err)
+ }
+
+ got := tallyDays(t, s)
+ if got[time.Now().UTC().AddDate(0, 0, -40).Format("2006-01-02")] {
+ t.Error("a 40-day-old counter survived the wake: this is the only " +
+ "per-person record memoria keeps, and the retention rule is the " +
+ "whole of what bounds it")
+ }
+ if !got[time.Now().UTC().Format("2006-01-02")] {
+ t.Error("today's counter was pruned; the prune is meant to bound the " +
+ "history, not to empty it")
+ }
+}
diff --git a/internal/memoria/custody.go b/internal/memoria/custody.go
index 4031593..bebba72 100644
--- a/internal/memoria/custody.go
+++ b/internal/memoria/custody.go
@@ -178,6 +178,26 @@ func WireSiblings(a *app, getenv func(string) string, dial suite.DialFunc) error
return err
}
}
+ // **Retention, enforced on every wake.** §8.1 says the counters are
+ // kept 8 days "and no longer", and §3.2 says why the second half is
+ // the load-bearing one: an unbounded version is a year-long history
+ // of who was locked out of what — the only per-person record memoria
+ // keeps, growing without limit.
+ //
+ // `PruneTallies` existed, with a comment saying a retention rule
+ // nothing enforces is a sentence in a document, and nothing called
+ // it. A review found the sentence describing its own state.
+ //
+ // A wake is the right moment: memoria hibernates, so every instance
+ // passes through here regularly, and pruning costs nothing beside the
+ // index rebuild already happening. A failure is logged and not
+ // fatal — an over-long retention is a disclosure risk to fix, not a
+ // reason to refuse to serve.
+ if n, err := a.store.PruneTallies(context.Background(), now); err != nil {
+ a.logger.Warn("breaker counters not pruned")
+ } else if n > 0 {
+ a.logger.Info("breaker counters pruned", "rows", n)
+ }
// Names, never the roster itself: the structs carry consumer tokens,
// and `carlos logs` is storage we cannot grep.
a.logger.Info("siblings configured", "team", cfg.Team, "apps", cfg.Names())
diff --git a/internal/memoria/render.go b/internal/memoria/render.go
index a0c4479..d5f636d 100644
--- a/internal/memoria/render.go
+++ b/internal/memoria/render.go
@@ -106,13 +106,25 @@ func safeSnippet(s string) template.HTML {
// tone maps a flash kind to the design system's tone vocabulary.
//
-// **One mapping, in Go, because the templates had four spellings between
-// them and none of them worked.** `rst-callout="danger"` puts a value on
-// an attribute nothing selects a value of; tokens.css styles
-// `[rst-callout][data-tone="negative"]`. Every flash in this app rendered
-// as an untoned callout — the right box, the wrong colour, on every error
-// message the product shows — and three tests pinned it there by asserting
-// the same wrong string.
+// **The tone attribute is `rst-tone`, and getting here took three tries.**
+// tokens.css pairs every kind: a class spelling and an attribute spelling,
+// weighing the same. For a callout's tone those are
+// `.rst-callout[data-tone="negative"]` and
+// `[rst-callout][rst-tone~="negative"]` — and the header says so directly:
+// *"rst-tone carries the four tones that data-tone carries today."*
+//
+// Attempt one was `rst-callout="danger"`: a value on the kind attribute,
+// which nothing selects. Attempt two was `[rst-callout] data-tone=...`:
+// half of one pair and half of the other, matching neither, and its tests
+// passed because the string `data-tone="negative"` does occur in the
+// stylesheet — inside the *class* selector standing next to the one that
+// was needed. Both rendered every flash in the product as an untoned
+// callout: the right box, the wrong colour, on every error a reader is
+// meant to act on.
+//
+// The lesson is not about tones. **Grepping for a string is not reading a
+// selector**, and a check that greps will confirm any spelling that
+// appears anywhere in the file.
//
// The default is deliberately the loud one. A flash whose kind nobody
// recognised is more likely to be an error than a compliment, and an
diff --git a/internal/memoria/render_test.go b/internal/memoria/render_test.go
index 91ce4d3..f949af3 100644
--- a/internal/memoria/render_test.go
+++ b/internal/memoria/render_test.go
@@ -37,21 +37,37 @@ func TestEveryToneIsOneTheStylesheetsDefine(t *testing.T) {
// Every kind the handlers actually set, plus one nobody sets — the
// default arm is the one a new flash kind lands on, and it must be
// styled too.
+ //
+ // **Matched against the ATTRIBUTE selector**, not against the string
+ // anywhere in the file. The previous version looked for
+ // `data-tone="negative"`, which occurs in the class selector sitting
+ // beside the attribute one — so it confirmed a spelling the templates
+ // could not use. Grepping for a string is not reading a selector.
for _, kind := range []string{"good", "danger", "warn", "", "something-new"} {
got := tone(kind)
if got == "" {
t.Errorf("tone(%q) is empty: the callout renders untoned", kind)
continue
}
- if !strings.Contains(css, `data-tone="`+got+`"`) {
- t.Errorf("tone(%q) = %q, which no stylesheet defines: the callout "+
- "renders as the right box in the wrong colour, on a message "+
- "the reader is meant to act on", kind, got)
+ if !strings.Contains(css, `[rst-callout][rst-tone~="`+got+`"]`) {
+ t.Errorf("tone(%q) = %q, and no stylesheet selects "+
+ "[rst-callout][rst-tone~=%q]: the callout renders as the right "+
+ "box in the wrong colour, on a message the reader is meant to "+
+ "act on", kind, got, got)
}
}
- // And the mapping is not constant, or "always negative" passes.
- if tone("good") == tone("danger") {
- t.Error("a success and a failure render the same tone")
+ // Every kind maps somewhere distinct enough to be readable: a success
+ // and a failure must not look the same, and neither may collapse into
+ // the warning. Comparing only good against danger let a mutation send
+ // "warn" to positive.
+ seen := map[string]string{}
+ for _, kind := range []string{"good", "danger", "warn"} {
+ got := tone(kind)
+ if prev, dup := seen[got]; dup {
+ t.Errorf("tone(%q) and tone(%q) both render %q; the tone stops "+
+ "carrying information", kind, prev, got)
+ }
+ seen[got] = kind
}
}
diff --git a/internal/memoria/store.go b/internal/memoria/store.go
index 8107c69..56350fb 100644
--- a/internal/memoria/store.go
+++ b/internal/memoria/store.go
@@ -729,13 +729,27 @@ func (s *Store) rule(ctx context.Context, asker suite.Subject, all []Result) Pag
disp[at[j]] = d
}
// §8.1's breaker counters, written on the path that already made
- // the call. **Failing to write one must not fail the search**:
- // the caller is answering somebody's query, and trading a working
- // page for a metrics row is the wrong direction. What a gap costs
- // is that the breaker reports *cannot determine* rather than *no
- // anomaly*, which is the safe way round and the reason this may
- // be dropped at all.
- s.recordForBreaker(ctx, asker, cands, at, disp)
+ // the call.
+ //
+ // **`WithoutCancel`, because this is an addition to derived
+ // state** and AGENTS.md's rule covers it: a reader closing the
+ // tab must not decide whether the counter lands. Without it the
+ // rows most likely to be lost are the ones from the slowest
+ // pages, which is not a random sample.
+ //
+ // **Failing to write one does not fail the search.** The caller
+ // is answering somebody's query, and trading a working page for a
+ // metrics row is the wrong direction.
+ //
+ // An earlier comment here claimed a gap "makes the breaker report
+ // cannot determine". It does not: nothing records the gap, so the
+ // period simply reads quieter than it was — the false-negative
+ // direction §8.1 forbids. The honest statement is that a dropped
+ // counter is a small, logged loss of fidelity in a signal that
+ // needs three askers to fire, and that losing one is better than
+ // failing the page. If that ever stops being true the fix is a
+ // gap marker, not a louder comment.
+ s.recordForBreaker(context.WithoutCancel(ctx), asker, cands, at, disp)
}
// **Notices are aggregated once, here, in result order.** The first
diff --git a/internal/memoria/templates/connectors.html b/internal/memoria/templates/connectors.html
index 7cfdf38..83e1060 100644
--- a/internal/memoria/templates/connectors.html
+++ b/internal/memoria/templates/connectors.html
@@ -31,14 +31,14 @@
that simply had not been polled yet — found by looking at the
page, which is the only thing that would have. */}}
{{if .CurrentAlarming}}
- <div rst-callout data-tone="warning">
+ <div rst-callout rst-tone="warning">
<p rst-callout-body>{{.Current}}</p>
</div>
{{else}}
<p rst-cell-mut>{{.Current}}</p>
{{end}}
{{if .BreakerAlarming}}
- <div rst-callout data-tone="warning">
+ <div rst-callout rst-tone="warning">
<p rst-callout-body>{{.Breaker}}</p>
</div>
{{else}}
diff --git a/internal/memoria/templates/index.html b/internal/memoria/templates/index.html
index 0326eb3..6ced7c3 100644
--- a/internal/memoria/templates/index.html
+++ b/internal/memoria/templates/index.html
@@ -126,7 +126,7 @@
{{end}}
<div rst-form-bar><button rst-btn="primary">Store it</button></div>
</form>
- {{if .Flash}}<div rst-callout data-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
+ {{if .Flash}}<div rst-callout rst-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
{{/* The upload tray. It lives OUTSIDE the form on purpose: it is not
something the form submits, and a scriptless browser must find the
diff --git a/internal/memoria/templates/notion.html b/internal/memoria/templates/notion.html
index b13a92c..83e8ae7 100644
--- a/internal/memoria/templates/notion.html
+++ b/internal/memoria/templates/notion.html
@@ -12,7 +12,7 @@
<h1>Notion</h1>
-{{if .Flash}}<div rst-callout data-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
+{{if .Flash}}<div rst-callout rst-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
{{if not .Configured}}
diff --git a/internal/memoria/templates/results.html b/internal/memoria/templates/results.html
index 946b5c8..a8ce802 100644
--- a/internal/memoria/templates/results.html
+++ b/internal/memoria/templates/results.html
@@ -13,7 +13,7 @@
{{define "results"}}
{{if .SearchFailed}}
- <div rst-callout data-tone="negative">
+ <div rst-callout rst-tone="negative">
<p rst-callout-body>{{template "searchFailed"}}</p>
</div>
{{else if .Searched}}
@@ -27,7 +27,7 @@
never a document, because naming the document would report the
existence of a passage this person may not have. */}}
{{if .Notices}}
- <div rst-callout data-tone="warning" rst-withheld>
+ <div rst-callout rst-tone="warning" rst-withheld>
{{range .Notices}}<p rst-callout-body>{{.}}</p>{{end}}
</div>
{{end}}
diff --git a/internal/memoria/templates/signin.html b/internal/memoria/templates/signin.html
index 295610e..6e66141 100644
--- a/internal/memoria/templates/signin.html
+++ b/internal/memoria/templates/signin.html
@@ -3,7 +3,7 @@
<h1>Sign in</h1>
<p>memoria holds your team's work in one place. Sign in with your email and
we'll send you a link.</p>
-{{if .Flash}}<div rst-callout data-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
+{{if .Flash}}<div rst-callout rst-tone="{{.FlashKind | tone}}"><p rst-callout-body>{{.Flash}}</p></div>{{end}}
<form method="post" action="/auth/begin" rst-stack>
<label for="address">Email address</label>
<input type="email" id="address" name="address" required autocomplete="email" autofocus>
diff --git a/internal/memoriatest/gates_test.go b/internal/memoriatest/gates_test.go
index 2801e3e..fe577a4 100644
--- a/internal/memoriatest/gates_test.go
+++ b/internal/memoriatest/gates_test.go
@@ -850,12 +850,49 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) {
// Listed rather than ignored, so the list is visible and can shrink.
// A blanket "skip what is not found" would turn this gate back into
// the absence of the gate.
+ // **Pre-existing attributes that style nothing and no script reads.**
+ // They render as plain elements — a silent defect rather than an
+ // error. Listed rather than pattern-skipped so the list is visible and
+ // can shrink; a blanket "ignore what is missing" would be the absence
+ // of the gate wearing its name.
+ //
+ // `rst-cell-mut` is here for a sharper reason than the rest: it is a
+ // UTILITY, and tokens.css says utilities keep the class spelling and
+ // have no attribute twin. Written as an attribute it selects nothing.
+ // Every template in this app writes it that way, which is a change
+ // too wide to make in the commit that noticed it — recorded here
+ // rather than quietly tolerated.
unstyled := map[string]bool{
"rst-lede": true,
"rst-list-item": true,
"rst-list-title": true,
"rst-steps": true,
"rst-facts": true,
+ "rst-cell-mut": true,
+ "rst-brand": true,
+ "rst-inline": true,
+ "rst-danger": true, // a utility; class-only, like rst-cell-mut
+ "rst-bg": true,
+ }
+
+ // **Values this app writes that the design system does not define.**
+ // Same treatment and same reason as the list above: named, dated, and
+ // visible rather than pattern-skipped.
+ //
+ // `rst-btn` takes primary, ghost and danger. Every button in this app
+ // spelled "quiet" or "compact" therefore renders as the default
+ // button — the wrong weight in the header, on the sign-out control,
+ // and on the connector actions. Found 2026-09-07 by the value half of
+ // this gate, the day it could see values at all.
+ //
+ // **Not fixed here on purpose.** Which variant a "quiet compact"
+ // button should actually be is a visual decision about how the app
+ // looks, not a defect with one correct repair, and making it silently
+ // inside a commit about a circuit breaker would be the drive-by this
+ // project's AGENTS.md warns about. Recorded for Paul.
+ unstyledValues := map[string]bool{
+ "rst-btn=quiet": true,
+ "rst-btn=compact": true,
}
exempt := map[string]bool{
"rst-withheld": true, // a test hook for §6.5's notice block
@@ -876,7 +913,22 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) {
}
return out
}
- cssAttrs, jsAttrs := tokens(css), tokens(js)
+
+ // **The stylesheet vocabulary comes from ATTRIBUTE selectors only**,
+ // `[rst-…`, and not from every rst- word in the file. Two things were
+ // wrong with reading the text: it counts words in comments, and — the
+ // one that matters — it counts the CLASS spelling of utilities that
+ // have no attribute twin. tokens.css says which those are
+ // ("Utilities keep the class spelling and are not paired: rst-sr-only,
+ // rst-mono, … rst-cell-mut"), memoria.css says `rst-cell-mut` written
+ // as an attribute selects nothing, and AGENTS.md says utilities stay
+ // classes. The gate said they were fine.
+ selector := regexp.MustCompile(`\[(rst-[a-z0-9-]+)`)
+ cssAttrs := map[string]bool{}
+ for _, m := range selector.FindAllStringSubmatch(css, -1) {
+ cssAttrs[m[1]] = true
+ }
+ jsAttrs := tokens(js)
dir := filepath.Join("..", "memoria", "templates")
entries, err := os.ReadDir(dir)
if err != nil {
@@ -940,20 +992,41 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) {
if exempt[name] || unstyled[name] || jsAttrs[name] {
continue
}
- if strings.Contains(css, name+`~="`) || strings.Contains(css, name+`="`) {
+ // **Each value, not merely the attribute taking values.**
+ // Checking that something somewhere selects on a value of
+ // this name passes `rst-btn="nonsense"`, because
+ // `[rst-btn~="primary"]` exists. A value nobody styles is the
+ // same silent nothing as an attribute nobody styles.
+ //
+ // A template expression is skipped and its guarantee moves to
+ // the function producing it — see tone().
+ if strings.Contains(value, "{{") {
continue
}
- t.Errorf("%s writes %s=%q, and no stylesheet selects on a value of "+
- "%s: the attribute matches, the value is ignored, and the "+
- "element renders untoned. This is the shape of the bug the "+
- "gate above cannot see", e.Name(), name, value, name)
+ for _, word := range strings.Fields(value) {
+ if unstyledValues[name+"="+word] {
+ continue
+ }
+ if strings.Contains(css, name+`~="`+word+`"`) ||
+ strings.Contains(css, name+`="`+word+`"`) {
+ continue
+ }
+ t.Errorf("%s writes %s=%q, and no stylesheet selects "+
+ "%s~=%q: the attribute matches, the value is ignored, "+
+ "and the element renders unstyled",
+ e.Name(), name, value, name, word)
+ }
}
}
// data-tone carries the tones, so its values must be ones the
// stylesheets define — a tone nobody styles is the same silent
// nothing by another spelling.
- tone := regexp.MustCompile(`data-tone="([^"]*)"`)
+ // The attribute spelling of a tone is `rst-tone`; `data-tone` is the
+ // class-grammar half of the same pair and selects nothing beside
+ // `[rst-callout]`. Both are checked, so writing either where the
+ // other belongs is caught.
+ tone := regexp.MustCompile(`(rst-tone|data-tone)="([^"]*)"`)
for _, e := range entries {
if e.IsDir() || !strings.HasSuffix(e.Name(), ".html") {
continue
@@ -963,7 +1036,14 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) {
t.Fatal(err)
}
for _, m := range tone.FindAllStringSubmatch(string(raw), -1) {
- if strings.Contains(m[1], "{{") {
+ if m[1] == "data-tone" {
+ t.Errorf("%s writes data-tone=%q on an attribute-spelled "+
+ "element: that is the class half of the pair, and it "+
+ "selects nothing here. The attribute spelling is rst-tone",
+ e.Name(), m[2])
+ continue
+ }
+ if strings.Contains(m[2], "{{") {
// A template expression. This gate reads files, not
// renders, so it cannot know what this evaluates to —
// and guessing would be worse than declining.
@@ -975,9 +1055,9 @@ func TestEveryTemplateAttributeIsInTheDesignSystem(t *testing.T) {
// One place produces them, so one test covers them.
continue
}
- if !strings.Contains(css, `data-tone="`+m[1]+`"`) {
- t.Errorf("%s writes data-tone=%q, which no stylesheet defines",
- e.Name(), m[1])
+ if !strings.Contains(css, `[rst-callout][rst-tone~="`+m[2]+`"]`) {
+ t.Errorf("%s writes rst-tone=%q, which no stylesheet selects "+
+ "on: the callout renders untoned", e.Name(), m[2])
}
}
}
diff --git a/internal/memoriatest/search_test.go b/internal/memoriatest/search_test.go
index 033a641..2c98a01 100644
--- a/internal/memoriatest/search_test.go
+++ b/internal/memoriatest/search_test.go
@@ -181,13 +181,20 @@ func TestThePageCarriesTheFailureSentenceItMayNeed(t *testing.T) {
// well-formed request can carry makes store.Search fail.
d.Close()
broken := get(t, h, "/?q=june+revenue").Body.String()
- // `rst-callout` with `data-tone`, which is what tokens.css actually
- // styles: `[rst-callout][data-tone="negative"]`. This asserted
- // `rst-callout="danger"` — a value on the attribute itself, which
- // matches the base selector and no tone at all, so the callout had
- // been rendering untoned since it was written and this test pinned it
- // there. Found by grepping the vocabulary rather than assuming it.
- if !strings.Contains(broken, `data-tone="negative"`) {
+ // **The ATTRIBUTE tone is `rst-tone`**, and this assertion has now
+ // been wrong twice. tokens.css pairs every kind: the class spelling
+ // takes `data-tone`, the attribute spelling takes `rst-tone`, and the
+ // header says so — "rst-tone carries the four tones that data-tone
+ // carries today". `[rst-callout]` is the attribute spelling, so it
+ // wants `rst-tone`.
+ //
+ // Attempt one asserted `rst-callout="danger"`, a value on the kind
+ // attribute that nothing selects. Attempt two asserted `data-tone`,
+ // which is a real string in the stylesheet — inside the CLASS
+ // selector next to the one needed here — so it confirmed a spelling
+ // the markup could not use. Both times the callout rendered untoned
+ // and both times a test said it was fine.
+ if !strings.Contains(broken, `rst-tone="negative"`) {
t.Fatalf("a failed search did not render its callout, so there is nothing to compare:\n%s", broken)
}
// Compare the callout's OWN words to the carried ones. Searching the
@@ -210,7 +217,7 @@ func TestThePageCarriesTheFailureSentenceItMayNeed(t *testing.T) {
// calloutBody is the text of the first danger callout's body, trimmed.
func calloutBody(body string) string {
- i := strings.Index(body, `data-tone="negative"`)
+ i := strings.Index(body, `rst-tone="negative"`)
if i < 0 {
return ""
}