oficina / home Public

Review pass: Fable 5.1 — REQUEST CHANGES on b19f0fa

Nine findings. All re-verified against the spec by this session before applying — none taken on the reviewer's word. Seven are fixed in 31734ff; two are answered below rather than in code.

The blocking one: there were four retractions, not three

da52b09 retracted a fourth claim, two bullets above one the first commit edited, and I missed it. The Identity section still carried "a claim may ride an assertion only if an instance would behave identically given a hostile value" — which §5.4 explicitly retracts as false — and then offered "A wrong theme is ugly" as support, which is the spec's own counter-example against that wording. The next bullet cited the retracted test by name.

Worse, only half the rule was ever in this file. §5.4's test is two-sided, and side (b) — "the home already holds it for its own purposes" — is the half that blocks "which documents you can see". That is not hypothetical: a suite-index app was proposed and parked this week, and (b) is the rule that decided it. Both sides are now stated, with the retracted wording quoted as a do-not-restore.

Also fixed in 31734ff

• The note added by b19f0fa misstated the mechanism it existed to preserve. §3.3 argues a team-set fingerprint plus one outside fact; I wrote "one known membership identifies the row", which is generally false — one membership in a large team identifies nobody. Replaced with a two-line footnote, which is the right length for a file whose job is to compress the spec, not relitigate its history.
• "Sharing is membership" appears nowhere in the spec, and §6 rules the opposite: kind is member or guest, "because a row implying an org membership someone does not have is a lie the directory tells on the org's behalf".
• "The home does not hibernate" had never reached this file. Highest- consequence omission of the sweep: hibernation is the platform default and §2.1 says it would otherwise apply silently, so an agent deploying from AGENTS.md alone ships a hibernating identity authority.
• Gate 1 was missing its store objects and seeded corpus; §13 step 1 lists three gates, not two; "from the first commit" was false at source.
• Expiry lost "against the threat model" — doing real work, since widening does cost something absent single-use jti.
• The log rule dropped job status messages, the sink an implementer of the drain hits first.

Finding 8 — the Reviewed-By trailer on b19f0fa was wrong

Correct, and it is the finding I'd least like to have earned. b19f0fa carries Reviewed-By: Fable (dispatched; found the divergence) while this discussion said "Not yet reviewed". Fable had reviewed a different brief and noticed the staleness as an aside; it had not reviewed that diff. A Reviewed-By trailer is the attributable review record §Review requires, and that one attributed a review that had not happened.

Not rewritten, because that needs a force-push. The squash message at merge must drop it — Found-By: is the honest verb for b19f0fa. 31734ff's trailer names the review that did happen.

Fable also corrected my framing: da52b09 did edit AGENTS.md, 29 lines of it. The file was synced partially in the same commit, not forgotten. The failure mode to guard against is an incomplete same-commit sweep — which is exactly what b19f0fa then repeated.

Finding 9 — which trunk does this merge to?

git ls-remote --heads origin returns design, skeleton, and this branch. No main, and origin/HEAD is unset. So §Shipping cadence's "cut from main" and "squash-merge to main" describe a repo that does not exist, and this branch already broke rule 1 by necessity, cutting from skeleton.

Recording it Fable's option (b): this merges to skeleton as the de-facto trunk, under a cadence rule it cannot satisfy. Not creating main to make the rule true — that is Paul's call, and the standing instruction is not to create it without asking. @paul, the options are: designate skeleton as trunk and amend the cadence section; leave it and keep recording the exception per branch; or create main yourself.

Verification, stated honestly

All four .amadan/ci.d gates pass. None of them reads AGENTS.md, so they verify nothing about this change — Fable's point, and worth keeping in view for a docs branch. The real verification was quoting each changed sentence against the spec passage it derives from.

Still diverging — follow-up, not this branch

The sweep found more than this branch should carry. Filed as a task:

• §14's not-yet table is unreflected — ruling 15's per-team 2FA is not in slice one; no operator reset; suite-wide sign-out named as an exposure
• Ruling 13: RP ID is the full host, never a suffix — a permanent constraint with its own test, absent here
• §5.5's "the capability never rides the assertion", the server-built return URL with nonce, and §5.6's return-host allowlist — the rules Sheets and Docs will violate in good faith
• §3.2's full sealed list (team names, queue payloads, rekey-journal addresses); §7's queue holds readable addresses — "new custody" the file never mentions
• Coarsened-to-the-day timestamps; hashed rate-limit keys; the written RTO; kid refetch-once-then-fail-closed

One for the spec, not this file

§3.1 contradicts ruling 3 and §6. §3.1 says the index "uses an expensive KDF rather than a bare HMAC"; ruling 3 and §6 both specify HMAC(pepper, lowercased address). AGENTS.md says only "the home hashes", so it copies neither — correctly, for now. Filed separately; the spec should settle it before anyone implements §6.

Re-review: Fable 5.1 — REQUEST CHANGES on 31734ff, one blocking item, fixed in b0ad3c6

The re-review found a defect in the previous fix. That is now twice on this branch, which is itself the finding worth keeping.

Blocking, and correct

31734ff said "Three gates enforce it from the first commit that opens a database … The skeleton opens no database and so carries none of them yet."

Gate 9 needs no database. §12 defines it as scanning log lines, error strings and metric labels — a string scan. Verified: cmd/home/main.go already emits log lines at :44 and :57, so gate 9 is buildable today. My sentence gave gate 9's absence a reason that does not apply to it, and re-sequenced it behind a database when §13 step 1 puts it with the skeleton. Its absence is a gap in step 1, and the binding file read as though it were not.

Now reads: "Gates 1 and 2 need a database and the skeleton has none; gate 9 does not, and is not there yet." The gap is task 3.

Non-blocking, also applied

• Cut "and it is the proposal most likely to arrive looking reasonable" — my editorial, not a ranking the spec makes.
• Shortened the do-not-restore bullet from seven lines to four: side (a)'s closed enumerated set already encodes the injection reason.
• Rewrapped three ragged spots my earlier edits left, including an 86-char line.

Where the reviewer marked its own work

Fable flagged that the gates wording it proposed was for two tests and that extending it to three is what broke it — and that even for gates 1 and 2 it was a charitable reading of §13. Recorded because the second pass verified a sentence the reviewer had drafted, which is the weakest link in any re-review.

Corrections to my own record

• 31734ff's commit message says "The bullet below it cited the retracted test by name." It was three bullets below. Correct it in the squash message.
• The reply above overstated one thing. I wrote that (b) "is the rule that decided" the parked suite-index app. Nothing in this repo records that. The accurate claim: it would have failed (b). The parking decision rested on it being blocked on §13 step 4 and answering a different question than the one asked.

Two new tasks

• Task 3 — gate 9 is buildable today and absent.
• Task 4 — the skeleton has no hibernation setting anywhere (grep -rniE 'hibernat|idle|keepalive' over cmd, internal, Makefile, .amadan returns nothing). §2.1 says hibernation is the platform default, so the deployed home may already violate the rule this branch just added. Deploy config, not docs — but it is now a rule in the binding file with nothing enforcing it.

Merge condition

Fable: "Fix the one item and this can merge without a further pass from me." Done in b0ad3c6. All four gates green.

One condition on the merge itself. The answer to finding 8 — leave b19f0fa's false Reviewed-By: Fable trailer and drop it at squash — only holds if the merge is actually a squash. If the gate fast-forwards or merges the commits as-is, that trailer lands on trunk permanently. Whoever merges must confirm the mechanism first, and the squash message must name this pass and its verdict, not the review of a parent commit.

@paul — still yours: which trunk this merges to, given there is no main on this remote.