oficina / memoria Public

🤖 fix: four green mutations in the sibling client, and a silent value that spoke

The review found four mutations my five had missed. §1.9's rule again: the
mutations an author picks are the ones already defended against.

M9 IS THE ONE THAT MATTERED. Withheld rendered "Sheets: 7 results
withheld." for NOT_VISIBLE — a count of exactly what a reader is not
allowed to see, from the one value §4.1 and §8.1 say may be silent. It fell
through to a default branch. It returns nothing now, and the test asserts
that rather than asserting a string.

M6 is the instructive one. The slow-sibling case bounded elapsed time
against Deadline itself, so it passed for ANY value: setting Deadline to
30 seconds left a page hanging for half a minute with the suite green. A
test that measures a constant against itself measures nothing. §6.5 states
800 ms and the number is asserted now, so moving it changes the spec too.

M7: the caller's context was not asserted to reach the request, so
replacing it with context.Background() stayed green — a reader who closed
the tab would still cost a hibernating sibling the full deadline.

M8: Content-Type was never checked.

All four verified red and reverted.

And two things this package does NOT do are now recorded in it rather than
left to be discovered: §8.1's durable per-(sibling, asker, day) counters,
which the spec says are written on the path that already made the call —
this path — and §6.5's fan-out, which makes a page one round with the floor
at the slowest sibling rather than the sum. Gate 23 holds both and is not
written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell pushed by paul@keymail.dev db7665ad343aa36bb2f74f4da63595b2a2f2ce53 parent 8593a1b
2 files changed, +81 −6
  • internal/memoria/sibling/sibling.go +16 −0
  • internal/memoria/sibling/sibling_test.go +65 −6
diff --git a/internal/memoria/sibling/sibling.go b/internal/memoria/sibling/sibling.go
index 1055f75..652cf5a 100644
--- a/internal/memoria/sibling/sibling.go
+++ b/internal/memoria/sibling/sibling.go
@@ -170,7 +170,23 @@ func (w Withheld) String() string {
return fmt.Sprintf("%s did not answer; %d results withheld.", w.Sibling, w.Count)
case suite.STALE:
return fmt.Sprintf("%s has newer versions of %d results; refetching.", w.Sibling, w.Count)
+ case suite.NOT_VISIBLE:
+ // The ONE value that may be silent (§4.1, §8.1). A count here would
+ // tell a reader how many things they are not allowed to see, which
+ // is the number the whole four-value design exists to not say. The
+ // default branch used to render it as "Sheets: 7 results withheld."
+ return ""
default:
return fmt.Sprintf("%s: %d results withheld.", w.Sibling, w.Count)
}
}
+
+// Not built, and recorded here rather than discovered later:
+//
+// - §8.1's durable per-(sibling, asker, day) VISIBLE/NOT_VISIBLE counters.
+// The spec says they are written "on the path that already made the
+// call" — this is that path, and Authorize is where they belong.
+// - §6.5's fan-out. A page is ONE round of concurrent calls with the
+// floor at the slowest sibling; this client asks one sibling at a time
+// and nothing here yet makes a page's cost the max rather than the sum.
+// Gate 23 holds both halves together and is not written.
diff --git a/internal/memoria/sibling/sibling_test.go b/internal/memoria/sibling/sibling_test.go
index 5307044..a2a63aa 100644
--- a/internal/memoria/sibling/sibling_test.go
+++ b/internal/memoria/sibling/sibling_test.go
@@ -83,10 +83,23 @@ func newTestClient() *Client {
// M4 always UNKNOWN, never asks the sibling -> happy path red
// M5 the consumer token is not sent -> happy path red
//
-// Author-chosen, which §1.7 rates as the weak kind. An adversarial pass by
-// someone who did not write this is owed, and the last one on this branch
-// found two live bypasses in a guard whose author had already mutated it
-// five ways — so the pass is not a formality.
+// Those five were author-chosen, and a review then found FOUR more that
+// stayed green — which is §1.9's rule earning itself again:
+//
+// M6 Deadline widened to 30s -> the deadline case, now pinned
+// M7 caller's context dropped -> the cancellation case
+// M8 Content-Type not sent -> the happy path
+// M9 NOT_VISIBLE renders a count -> the withholding case
+//
+// M6 is the instructive one. The slow-sibling case bounded elapsed time
+// against `Deadline` itself, so it passed for ANY value — a page could
+// hang for thirty seconds with the suite green. A test that measures a
+// constant against itself measures nothing; §6.5 states 800 ms and the
+// number is now asserted.
+//
+// M9 is the one that mattered most: `Withheld` rendered "Sheets: 7 results
+// withheld." for `NOT_VISIBLE`, a count of what a reader is not allowed to
+// see — from the one value §4.1 and §8.1 say must be silent.
func TestEveryPairGetsExactlyOneAnswer(t *testing.T) {
ctx := context.Background()
@@ -118,6 +131,29 @@ func TestEveryPairGetsExactlyOneAnswer(t *testing.T) {
}
})
+ t.Run("a cancelled request does not wait out the deadline", func(t *testing.T) {
+ // Authorize wraps the caller's context; replacing it with
+ // context.Background() would leave a reader who closed the tab
+ // costing a sibling 800ms of a hibernating instance's time, and
+ // every other case here would stay green.
+ s := serve(t, func(w http.ResponseWriter, r *http.Request) {
+ time.Sleep(Deadline * 2)
+ })
+ cancelled, cancel := context.WithCancel(context.Background())
+ cancel()
+ start := time.Now()
+ got := newTestClient().Authorize(cancelled, s, asker(t), pairs(2))
+ if elapsed := time.Since(start); elapsed > Deadline/2 {
+ t.Errorf("waited %v on an already-cancelled context: the caller's "+
+ "cancellation is not reaching the request", elapsed)
+ }
+ for i, a := range got {
+ if a != suite.UNKNOWN {
+ t.Errorf("pair %d = %q, want UNKNOWN", i, a)
+ }
+ }
+ })
+
t.Run("a batch over the cap is refused, never truncated", func(t *testing.T) {
var asked int
s := serve(t, func(w http.ResponseWriter, r *http.Request) {
@@ -146,10 +182,11 @@ func TestEveryPairGetsExactlyOneAnswer(t *testing.T) {
want := []suite.Authorization{
suite.VISIBLE, suite.NOT_VISIBLE, suite.STALE, suite.VISIBLE,
}
- var gotToken string
+ var gotToken, gotType string
var gotBody suite.S2Request
s := serve(t, func(w http.ResponseWriter, r *http.Request) {
gotToken = r.Header.Get("Authorization")
+ gotType = r.Header.Get("Content-Type")
raw, _ := io.ReadAll(r.Body)
_ = json.Unmarshal(raw, &gotBody)
type row struct {
@@ -171,6 +208,10 @@ func TestEveryPairGetsExactlyOneAnswer(t *testing.T) {
"this file is measuring nothing", i, got[i], want[i])
}
}
+ if gotType != "application/json" {
+ t.Errorf("Content-Type = %q, want application/json: a sibling that "+
+ "guesses at the body is a sibling that can guess wrong", gotType)
+ }
// §6.2: the consumer token rides the transport, never the body.
if gotToken != "Bearer test-consumer-token" {
t.Errorf("Authorization header = %q, want the consumer token", gotToken)
@@ -185,6 +226,18 @@ func TestEveryPairGetsExactlyOneAnswer(t *testing.T) {
}
})
+ t.Run("the deadline is §6.5's number, not whatever the constant says", func(t *testing.T) {
+ // The slow case below bounds elapsed time against Deadline itself,
+ // so it passes for ANY value — a review set it to 30s and the test
+ // stayed green while a page hung for half a minute. §6.5 states
+ // 800 ms; assert the number, not the shape.
+ if Deadline != 800*time.Millisecond {
+ t.Errorf("Deadline = %v, want 800ms: §6.5 sets it and §13 owes the "+
+ "measurement that would change it — moving it here changes the "+
+ "product without changing the spec", Deadline)
+ }
+ })
+
t.Run("a slow sibling is UNKNOWN at the deadline, not a hung page", func(t *testing.T) {
s := serve(t, func(w http.ResponseWriter, r *http.Request) {
time.Sleep(Deadline + 500*time.Millisecond)
@@ -219,12 +272,18 @@ func TestWithheldNamesTheSiblingAndTheReason(t *testing.T) {
}{
{suite.UNKNOWN, "Sheets did not answer; 7 results withheld."},
{suite.STALE, "Sheets has newer versions of 7 results; refetching."},
+ // NOT_VISIBLE is the one value that may be silent, so it renders
+ // nothing at all. It used to fall through to the default and say
+ // "Sheets: 7 results withheld." — a count of what a reader is not
+ // allowed to see, which is the number this vocabulary exists to
+ // avoid saying.
+ {suite.NOT_VISIBLE, ""},
} {
got := Withheld{Sibling: "Sheets", Count: 7, Reason: c.reason}.String()
if got != c.want {
t.Errorf("Withheld(%s) = %q, want %q", c.reason, got, c.want)
}
- if !strings.Contains(got, "Sheets") {
+ if c.want != "" && !strings.Contains(got, "Sheets") {
t.Errorf("withholding notice does not name the app: %q", got)
}
}