amadan
Third-hand review: GPT-6 requests changes, sixteen findings
Paul asked for a second reader on the review pass, via codex. GPT-6 read the doctrine at 0c4f098, the revision-1 diff, and the sibling specs at their real commits — it noticed the home checkout was on a stale branch missing rulings 17 to 21 and went to main instead. Verdict: request changes. CODEX-FINDINGS.md is its report, unedited. It confirms all three review-authored rule changes as answering real defects, and passes the typed asker subject outright. Two of its findings land on the revision-2 corrections' own reasoning, and both are fixed here: - §4.3 claimed the custody intersection "costs a correct consumer nothing", reasoning from visibility at ingestion. That assumes away the case the defect turns on: an item ingested while team-visible that goes private while a grantee can still read it in the app of record. The intersection does cost that, it cannot not cost it while §4.2's promise stands, and the section now says so. - §7.1 claimed "no new machinery" and no extra round trip. Measured against the pinned dependencies, not the specs: memoria's rastrillo default CSP sets form-action 'self' and stops the form leaving; Docs mounts csrf.Protect above every route and its SameOrigin check refuses the same-site cross-origin POST, 204 same-origin against 403 same-site; and receipt-then-commit is a second POST. §7.1 now states the real bill and records that the commit step is still under-specified. The other fourteen are not folded in. They are a third party's report and §14 holds them for Paul, with the six trust-boundary ones summarised there: the instance-token write path into the rows §5.1 makes authoritative, §5.5's too-kind compromised-home bound, provenance missing from both write paths, a failed roster refetch with no stated behaviour, custody as a rule about rendering rather than erasure, and the hand-off's commit protocol. The status block and README now say reviewed but not settled: §§0-2 and the rulings drew no finding, §§3-8 are not final, and §12 step 1 waits.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016XuAriz559moY27sTegqpt
4 files changed,
+433
−27
CODEX-FINDINGS.md+259 −0CODEX-REVIEW-BRIEF.md+76 −0README.md+11 −6docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md+87 −21
Intent
Human
Can you have GPT sol review too via codex
Human
<task-notification> <task-id>buyd5va2s</task-id> <tool-use-id>toolu_01W1nySvyGT9ZMZRPME59n2T</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-suite/df6721cd-97ce-45b2-b664-3fea70beada0/tasks/buyd5va2s.output</output-file> <status>completed</status> <summary>Background command "Run codex review of the doctrine" completed (exit code 0)</summary> </task-notification>
Human
<task-notification> <task-id>bz3q7xlfg</task-id> <tool-use-id>toolu_01QGcLFE5SyBTBg28JXhJGxV</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-suite/df6721cd-97ce-45b2-b664-3fea70beada0/tasks/bz3q7xlfg.output</output-file> <status>completed</status> <summary>Background command "Run codex review outside sandbox" completed (exit code 0)</summary> </task-notification>
Human
<task-notification> <task-id>bsc5q8di2</task-id> <tool-use-id>toolu_01Q6Bw1hAPoQvTY2vwXwsjc6</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-suite/df6721cd-97ce-45b2-b664-3fea70beada0/tasks/bsc5q8di2.output</output-file> <status>completed</status> <summary>Background command "Run codex review at high reasoning effort" completed (exit code 0)</summary> </task-notification>
diff --git a/CODEX-FINDINGS.md b/CODEX-FINDINGS.md| new file mode 100644 |
| index 0000000..d8faba5 |
| --- /dev/null |
| +++ b/CODEX-FINDINGS.md |
| @@ -0,0 +1,259 @@ |
| +# Findings on the suite sharing doctrine, revision 2 |
| + |
| +**Verdict: request changes.** Reviewed by GPT-6 (Codex), 2026-09-05, independently of the draft and revision-2 authors. These are findings and proposed corrections, not new rulings. The doctrine has not been edited. |
| + |
| +The review covers the whole doctrine at `0c4f098`, including `git diff 609394e..HEAD`, and the sibling sources identified below. P1 means a trust boundary or promised security property is broken or lacks an enforceable design. P2 means a contract, adoption, or verification defect that needs correction before implementation. |
| + |
| +## Verdicts on the three priority changes |
| + |
| +| Change | Is the defect real? | Is the correction right? | Consequences | |
| +|---|---|---|---| |
| +| S2 intersects authorization with custody, §4.3 | Yes. A previously ingested id lets a consumer distinguish a private item still granted to the asker from a deleted item. The unintersected join in memoria §4.1 permits this. | Yes, for the stated current-visibility privacy guarantee. A determinate non-custodial item must not produce `VISIBLE` or an existence-revealing alternative. | The claim that correct consumers lose nothing is false. They can hold legitimately ingested items when visibility changes. See finding 8. The fix also does not erase those copies: finding 6. | |
| +| Typed asker on S2 and S6, §3 | Yes. Provisioning intent alone does not ensure that the two endpoints interpret the subject identically. | Yes. `{kind, value}` distinguishes a configuration mismatch from an authorization denial; `UNKNOWN` is the correct failure class. | Clean verdict on this change. Both request types and their fixtures must adopt it. Unknown or unsupported kinds must fail before item-dependent results; an unknown value of a supported kind can still be an ordinary denial. No extra network round trip is inherent in the tag. It does not prove shared authority or shared membership, and should not be claimed to do so. | |
| +| Hand-off checks `Origin` and asks for confirmation, §7.1 | Yes. Same-site cookies permit a sibling page to submit a credentialed form without a deliberate create action. | The two protections are appropriate, but the commit protocol is incomplete. The home citation supplies a return-host check and login-state binding, not an already specified document-confirmation protocol. | The receiving and committing requests need different rules. Existing middleware and content policy also block the proposed flow, and confirmation adds a round trip. See findings 4 and 9. | |
| + |
| +## Findings, ordered by severity |
| + |
| +### 1. P1 — Making directory rows authoritative leaves an instance-token route around roster step-up |
| + |
| +**Where:** doctrine §5.1, lines 221–225; §5.4, lines 279–283; §6, line 327; A.1. Home §6 and §9; Docs §9.1. |
| + |
| +**What is wrong:** §5.1 says the roster is the existing directory rows with `kind = member`, made authoritative. The doctrine also says home calls 1–6 are unchanged. Home call 2 lets an instance upsert and revoke its directory rows under a consumer token. Docs spells out the submitted `kind` at lines 607–608. That was safe only while directory rows were hints. |
| + |
| +**Why:** an instance can announce an attacker-controlled address as a member, or replay a queued pre-removal announcement. If call 8 derives the roster from those rows, the attacker becomes a team member without the owner action or fresh assertion required by §5.4. Conversely, instance-scoped deletion can affect membership if directory-row lifetime also defines roster lifetime. A table that serves both purposes needs a write-authority distinction; changing the reader does not create one. |
| + |
| +**Confidence:** high; this follows from the specified writers and the proposed source of truth, without assuming a broken token check. |
| + |
| +**Smallest correction:** give authoritative `(team, person, role)` membership its own owner-controlled record or equivalent protected provenance. Make call-2 rows projections/hints that cannot grant or revoke team standing. Amend the home-owned contract by citation after recording that boundary there. Test a member upsert, revoke, and delayed replay under an instance token against the authoritative roster. |
| + |
| +### 2. P1 — The compromised-home bound is false, including its claim of unavoidable evidence |
| + |
| +**Where:** doctrine §5.5, lines 292–301; §6.1, lines 359–361; §9 item 4, lines 502–503. Home §5.3–§5.5; activity design §2.2(b) and §5.1. |
| + |
| +**What is wrong:** the doctrine concedes that a compromised home can add members but says it still cannot read documents or act silently. It names owner step-up and the instance's cached roster as bounds on that compromise. |
| + |
| +**Why:** an added member can read team-visible content by definition. The attacker can use an account it controls after inserting that account into the roster. If compromise includes the credential origin and signing key, the attacker can also mint an assertion for an existing owner. The receiving grants join then authorizes that owner's actual rights; it cannot distinguish impersonation by the identity authority. The home spec explicitly identifies hostile `sub` as account takeover. This latter risk predates ruling 4. |
| + |
| +Step-up performed by the compromised authority does not constrain it. Nor is a directory row or notification unavoidable when the attacker controls the process that writes or sends it. Existing-identity impersonation need not create a membership row at all. The activity design already acknowledges that call 7 removes an observable distinction between normal home traffic and exfiltration (§2.2(b)). |
| + |
| +**Confidence:** high. A directory-origin XSS alone has a narrower scope; the doctrine must distinguish it from live directory-server or credential-origin compromise. |
| + |
| +**Smallest correction:** separate those threat models and disclose the resulting membership and impersonation capabilities in §5.5 and §9. Describe notifications as evidence produced by honest execution, unless an independent, enforceable audit mechanism is designed. Do not claim that an instance's grants join defeats compromise of the authority that names the grantee. |
| + |
| +### 3. P1 — The new writes have no contract for preserving or checking their source authority |
| + |
| +**Where:** doctrine §4.3, lines 185–188; §7.1–§7.2; §11; A.4. Memoria §5.2, §9.1, §10 gates 9, 17 and 19, and §12. |
| + |
| +**What is wrong:** transitive provenance is required, but the hand-off describes a document body and S8 describes actor and destination constraints. Neither defines how source identities and revisions survive a write into another app, or how that app enforces them on ordinary reads, exports, activity, and onward copies. |
| + |
| +**Why:** save an answer derived from a Sheet into Docs, then revoke or edit the Sheet. Docs' own grant to Ana is insufficient to authorize the saved answer under §4.3. Merely assigning the newly created Doc a Docs id must not reset authority. The generic declaration does not tell Docs how to recognize or check this case. |
| + |
| +There is also a concrete missing endpoint: an answer can include memoria-owned uploads or Notion material. Docs would need to check that source authority. Memoria decision 7, §9.1, and gate 17 explicitly prohibit sibling-facing endpoints; its §12 requires a new decision and leak entry before adding one. A.4 does not amend those clauses. A browser confirmation establishes the writer's intent, not an exemption from provenance. |
| + |
| +**Confidence:** high that the required contract is missing. A particular implementation might enforce the general rule correctly, but the proposed fixtures and amendments do not establish that it can. |
| + |
| +**Smallest correction:** define a bounded provenance envelope for both write paths, retain the original source authority, and require source checks on every disclosure path. Add the required memoria authorization surface through an explicit amendment, or defer hand-offs containing sources that the receiver cannot check. Add a two-hop write/read fixture with mixed authorities and a source revocation. Do not implicitly turn “save” into permanent republication. |
| + |
| +### 4. P1 — A confirmation page does not specify a protected commit, and a receiver check is too late to prevent cross-team transmission |
| + |
| +**Where:** doctrine §4.1, §7.1 lines 395–428, and §11 gate 7. Home §5.5–§5.6. |
| + |
| +**What is wrong:** the new rule describes the initial POST's origin check and a confirmation screen, but does not bind the final mutation to the reviewed pending body, the receiver's browser/session, or a single redemption. Applying the sibling-origin rule to both POSTs would also reject the final same-origin Docs form unless Docs unnecessarily had a consumer row for itself. |
| + |
| +**Why:** a bare second POST with a `confirm` flag can be submitted by the same allowed sibling that submitted the first. A screen appearing somewhere in the flow is not evidence that this browser reviewed this body. A nonce carried only in a URL is not itself browser binding; home §5.6 separately requires that binding to prevent login CSRF. |
| + |
| +The sending side needs a boundary too. If memoria's form action is misconfigured to another team's Docs host, the body has already reached that instance before its `Origin` check rejects it. §4.1's sibling HTTP-client guard does not automatically govern an HTML form action. This contradicts §0's literal “a byte … reaches an instance” test even when the receiver refuses to save it. |
| + |
| +**Confidence:** high that both obligations are unstated; this is a design gap, not a claim that a deployed hand-off is exploitable. |
| + |
| +**Smallest correction:** distinguish the draft-receipt and commit phases. Validate the exact same-team destination before rendering the sending form. Accept allowed sibling origins only at draft receipt; commit through a receiver-origin form against bounded, expiring, single-use pending state bound to the browser and authenticated person, with the reviewed body fixed server-side. Preserve safe rendering and framing restrictions. Gate forged commits, replay, changed bodies, session changes, absent/null origins, and a foreign-team destination before transmission. OWASP likewise treats same-site subdomains and actual request validation as distinct concerns ([CSRF prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html)). |
| + |
| +### 5. P1 — Roster expiry does not yet bound revocation, and rollback can restore admission |
| + |
| +**Where:** doctrine §5.3 lines 249–275; §5.4 lines 285–286; §5.5 lines 298–299; §11 gate 5. |
| + |
| +**What is wrong:** a TTL triggers a refetch, but the doctrine never says what happens when that refetch fails. It still says requests enforce from the local copy and characterizes an outage or rollback as lag rather than admission. |
| + |
| +**Why:** cache epoch 10, remove Ana at epoch 11, then make the home unreachable. Retrying at expiry while continuing to enforce epoch 10 admits Ana indefinitely. Refusing stale membership decisions bounds that exposure, but costs availability and produces `UNKNOWN` for S2, not the gate's universal `NOT_VISIBLE`. |
| + |
| +A home restored to epoch 10 can also return Ana as a member to a receiver that already applied 11 unless lower epochs are rejected. An instance restored from backup must refresh before using its restored roster. A TTL alone does not settle either direction of rewind. “Within one epoch” is not an elapsed-time bound for an instance that has not learned the new epoch. |
| + |
| +**Confidence:** high; these are direct failure sequences for the specified cache. |
| + |
| +**Smallest correction:** state the usable-age limit and behavior after an unsuccessful refresh, including existing sessions and sibling reads. Publish a new roster only after a complete fetch and validation. Specify rewind detection/recovery and refresh-before-admission after restore. Express the revocation bound in time, including request deadline or grace if any; use a fake clock in gate 5 and separate successful revocation from unavailable authority. The TTL is an availability/security choice, not merely a sentence tightening or a number derivable from cold-wake latency. |
| + |
| +### 6. P1 — The “inside” invariant and leak bound ignore copies retained after visibility changes |
| + |
| +**Where:** doctrine §0 lines 38–41; §4.2; §9 item 1. Memoria §5.2, §6.1, §7.4 and §8.1. |
| + |
| +**What is wrong:** §0 describes current custody as an invariant, and §9 says a compromised sibling cannot read private items. The pull design necessarily retains formerly team-visible content after a source makes it private or deletes it. |
| + |
| +**Why:** memoria ingests revision 7 while team-visible; Docs then makes it private; memoria has not yet pulled `gone`. Publishing memoria's holdings now reveals content the team cannot currently read in Docs. S2 can prevent honest rendering, but cannot remove stored bytes. A compromised consumer can retain previously received content permanently. Memoria's snapshots, delayed pulls, and explicit circuit breaker against wholesale purges make it especially important not to equate denial at render with erasure of custody. |
| + |
| +This also limits the existence-hiding claim: `gone` can conceal the distinction between deletion and privatization; it cannot erase knowledge that an item existed before. |
| + |
| +**Confidence:** high; no implementation bug or malicious source is required for the initial retention window. |
| + |
| +**Smallest correction:** distinguish authorized acquisition, current authorization to render, and eventual retirement of honest cached copies. State the synchronization/retirement bound and restore obligation by citation. In §9 include historical content acquired while eligible and the inability to recall it from a compromised consumer. Rewrite §0's test to match those guarantees rather than promise instantaneous distributed erasure. Also scope the “across” test to suite-mediated flows: existing export/upload features cannot prevent a person deliberately copying material between teams. |
| + |
| +### 7. P2 — Gate 5 silently changes what removal means for guests and individual grants |
| + |
| +**Where:** doctrine §5.2 lines 227–237; §5.4; §11 gate 5, lines 569–572. Home §5.5; activity design §4–§5.2; Sheets §8.2; Docs §8.2. |
| + |
| +**What is wrong:** the gate requires every removed member to receive an empty call-7 response from every instance. The doctrine preserves instance-owned individual/guest grants and says nothing about guests changes. |
| + |
| +**Why:** remove Ana's team standing while leaving a valid grant to one document. The instance still owns that grant, and call 7 is specified to evaluate it. Returning its activity is correct under the cited contract. Returning empty solely because Ana is off the team roster revokes a right §5.2 leaves instance-side. S2's custody filter does not change the separate call-7 contract. |
| + |
| +**Confidence:** high. If complete offboarding is intended, that is an additional rule, not the current meaning of roster removal. |
| + |
| +**Smallest correction:** make gate 5's removed-person fixture explicitly have no remaining independent grants, and add a guest fixture that retains only its granted activity. Alternatively, specify a distinct complete-offboarding operation and its effect on instance grants. Do not implement that policy by asserting the desired empty list in a test. |
| + |
| +### 8. P2 — The S2 correction is right, but “costs a correct consumer nothing” is false |
| + |
| +**Where:** doctrine §4.3 lines 194–208; A.4. Memoria §4.1 and §6.1. |
| + |
| +**What is wrong:** the correction justifies its zero-cost claim using visibility at ingestion, even though its own attack depends on a later visibility change. |
| + |
| +**Why:** memoria correctly ingested a document when team-visible. It is now private but still granted to Ana; its `gone` row has not reached memoria, or an answer already being processed still contains it. Before this change, S2 could return `VISIBLE` for Ana. After it, S2 returns `NOT_VISIBLE`. Ana loses the passage and any derived answer requiring it in memoria, while direct access in the source app remains valid. Asking S2 about previously ingested content is exactly how a correct consumer checks current authorization. |
| + |
| +**Confidence:** high. This is a necessary product restriction under the chosen indistinguishability guarantee, not an argument to undo the intersection. |
| + |
| +**Smallest correction:** replace the zero-cost paragraph with that explicit consequence and a source-app fallback. Add a transition fixture: ingest, downgrade while retaining Ana's grant, query before feed catch-up, receive `NOT_VISIBLE`. Keep the private/deleted response equality. Require custody denial to dominate revision-specific diagnostics in S2 and S6; otherwise `STALE(cell)` or a numeric diagnostic can reopen an existence distinction. |
| + |
| +### 9. P2 — The hand-off needs middleware changes and an additional round trip |
| + |
| +**Where:** doctrine §7.1 lines 401–428; A.2 lines 710–713. Docs `internal/docs/app.go:406–445`; memoria `cmd/memoria/main.go:24,75–77`; their rastrillo pins. |
| + |
| +**What is wrong:** “no new machinery” and “neither costs a round trip” obscure actual consumer work. The proposed form cannot traverse the current sender and receiver unchanged. |
| + |
| +**Why:** memoria uses rastrillo v0.23.0's default `Content-Security-Policy`, whose `form-action 'self'` blocks a form targeting Docs. Docs places rastrillo v0.22.0's `csrf.Protect` above every route; its `SameOrigin` rejects `Sec-Fetch-Site: same-site` even with the legitimate memoria `Origin`. Its session middleware also refuses an unauthenticated POST rather than preserving its body through sign-in. Finally, for an already signed-in person, receipt → confirmation → commit requires a second POST where revision 1 committed on the first. |
| + |
| +**Confidence:** high. A temporary probe against Docs' actual pinned `csrf.Protect` produced `204` for a same-origin request and `403` for the proposed same-site request. Sender CSP and session behavior were checked in source, not exercised in a browser. |
| + |
| +**Smallest correction:** name the narrowly scoped sender `form-action` allowance, dedicated receiving route and its replacement origin guard, pending-body/sign-in handling, and additional confirmation round trip. Preserve the normal same-origin write protections elsewhere. Extend gate 7 through the real middleware and browser policy, including a successful same-team flow. A handler-only fixture would miss both blockers. |
| + |
| +### 10. P2 — Call 8 has no coherent paginated snapshot contract |
| + |
| +**Where:** doctrine §5.3 lines 244–247 and 269–271; §6 call 8. |
| + |
| +**What is wrong:** the review adds pagination but leaves the wire example unpaged, specifies no cap or cursor behavior, and says S2 and S6 already have cursors. They do not: their caps bound caller-supplied batches. |
| + |
| +**Why:** fetch page one at epoch 10, change the roster, then fetch page two at epoch 11. Combining the pages can produce a membership set that never existed, skip a removal, or omit an owner. Publishing a partial response has a similar effect. Neither the example nor the citation tells the client which snapshot it received or how to restart. |
| + |
| +**Confidence:** high, verified against memoria §6.1's signatures and §4.1's multiple-batch rule. |
| + |
| +**Smallest correction:** specify a bounded page shape with epoch-bound cursor, completion indication, and invalidated-cursor response. Require staging and atomic replacement only after all pages of one epoch arrive. Name restart behavior on epoch change and refusal to treat partial/failing results as an empty roster. Add concurrent-change and failed-middle-page fixtures. Replace the inaccurate comparison to S2/S6 pagination. |
| + |
| +### 11. P2 — The adopted S2 result vocabulary still lacks the source-behind case |
| + |
| +**Where:** doctrine §4.3 and §6's adoption of memoria §4.1; §11 gates 1–2. Memoria §4.1 lines 484–514 and its acknowledged §13 defect. |
| + |
| +**What is wrong:** the doctrine adopts four values without correcting the cited definition of `STALE`, which only covers a source that has moved past the requested revision. |
| + |
| +**Why:** memoria has revision 7 and a restored source has revision 5. Returning `VISIBLE` would serve an unverified revision; returning `NOT_VISIBLE` silently represents synchronization failure as a denial. The total decoder only checks response completeness and shape. It cannot repair a validly encoded wrong classification. Memoria already records this exact unresolved defect in §13. |
| + |
| +**Confidence:** high. |
| + |
| +**Smallest correction:** define the nonmatching-revision outcome for both directions, with a recovery distinction if needed. Preserve a loud failure whenever current revision authority cannot be established, while keeping determinate private/deleted denial indistinguishable. Add forward and backward revision fixtures and include the correction in A.4. |
| + |
| +### 12. P2 — The roster epoch does not pass the home claim test as cited |
| + |
| +**Where:** doctrine §5.3 lines 251–256; A.1 lines 687–691. Home §5.4. |
| + |
| +**What is wrong:** the doctrine says the epoch passes the existing test because a failed match is treated as absent. The home test requires non-identity claims to validate against a closed enumerated set and be cosmetically failable. A changing roster epoch is neither a closed cosmetic choice nor merely presentation data. |
| + |
| +**Why:** if “failed match” means a different valid epoch, discarding it suppresses the very refresh this feature needs. If it means a malformed claim, the text needs to say so. More fundamentally, suppressing a refresh can leave an old membership decision in force until TTL expiry. Refetching rather than trusting a claimed role is a useful safeguard, but it does not make this the same kind of claim as theme. |
| + |
| +**Confidence:** high about the citation mismatch. The bounded-staleness design can still be sound after finding 5 is resolved. |
| + |
| +**Smallest correction:** record a narrow invalidation-hint exception in the home-owned test, distinguishing invalid encoding from a valid differing epoch. Make call 8 authoritative and its freshness bound independent of the claim. Cite that exception here instead of saying the unchanged test already passes. |
| + |
| +### 13. P2 — The suite leak list omits new roster information and already-disclosed activity costs |
| + |
| +**Where:** doctrine §5.1, §5.3, §9, and §11 gate 8. Home §6/§10 and activity design §2.2 and §7. |
| + |
| +**What is wrong:** the list omits consequences of the features it adopts, and gate 8 claims the roster adds no column class. |
| + |
| +**Why:** home's existing directory `kind` distinguishes member from guest; it does not distinguish owner from member. The new `role` does. If stored openly, publishing the database now identifies team owners. If sealed, the disclosure and test need to say that. Call 8 also distributes the complete team roster and those roles to each instance, rather than just the people known to that instance under the old local models. |
| + |
| +The activity design explicitly discloses two suite effects in §2.2: a fine-grained signal of a person's front-door visits sent to their instances, and the loss of outbound-home traffic as a compromise signal. Neither appears in §9. Its §7 also retains last activity in RAM briefly after removal. Those are relevant alongside the existing title-in-RAM entry. Findings 2 and 6 identify two further bounds that need correction, rather than additional reassuring prose. |
| + |
| +**Confidence:** high; the old directory schema and the activity disclosures are explicit. |
| + |
| +**Smallest correction:** add citations for these costs in §9, choose the roster-role storage/disclosure treatment, and amend gate 8's expected disclosure accordingly. Exercise the actual persisted schema and failure paths; one roster fixture cannot prove that an entire database has no additional information class. An audit of column types alone also misses new information encoded in existing columns. |
| + |
| +### 14. P2 — The consuming amendments preserve contradictions and introduce restatements |
| + |
| +**Where:** doctrine §10; §12 steps 1–3; Appendix A; repository `AGENTS.md`. |
| + |
| +**What is wrong:** the prescribed amendments do not produce the cited, consistent specs that §12 says unblock implementation. |
| + |
| +**Why:** several concrete leftovers survive them: |
| + |
| +- A.2's proposed Docs sentence names sibling service “in suite shape”; §3 and §12 explicitly require Docs and Sheets to serve lone peers before the home exists. The amendment must name the independent sibling-token switch or cite it without narrowing the scope. |
| +- A.4 leaves memoria §11's explicit person-ref/home dependency on slice 1, its no-inbound-surface decision and gate, and its stale S7 admission gate. Replacing §4.3 alone does not remove those contradictions. Finding 3 makes the inbound-surface conflict operational rather than editorial. |
| +- A.1–A.3 prescribe paragraphs paraphrasing boundaries, membership, and egress. That is the practice `AGENTS.md` and §10 say to replace with ruling-and-commit citations. Adding a citation after another copy of a rule still leaves another copy to drift. |
| + |
| +**Confidence:** high; these are literal mismatches between the proposed replacements and the surviving source text. |
| + |
| +**Smallest correction:** revise Appendix A as a complete list of consuming sections and gates to replace with citations, including the independent shape/token switch, memoria sequencing, admission fixtures, and required new authority surface. Keep locally owned behavior local. Do not declare slice 1 unblocked until the resulting specs agree; the doctrine itself can remain findings-only in this review. |
| + |
| +### 15. P2 — Two stated sibling positions are not what their cited text says |
| + |
| +**Where:** doctrine §2, §8 lines 474–479, §10 lines 523–527, A.2 §11.2, and A.3's AGENTS amendment. |
| + |
| +**What is wrong:** the doctrine supplies a new reason for Docs' conversion-API rejection and treats Sheets' local spec-synchronization rule as a cross-repository rule. |
| + |
| +**Why:** Docs §11.2, lines 929–931, rejects conversion APIs because they send customer documents to a third party. It does not say the provider was unnamed or the sending unavoidable. A per-team optional converter with a named provider could satisfy suite §8, so the narrower reason is a policy qualification, not verification that the old rejection already meant it. The ruling may authorize that qualification; the cited passage does not establish its historical premise. |
| + |
| +Sheets `AGENTS.md`, lines 5–7, says that where **that file** disagrees with **its design spec**, the spec wins and the file must be fixed in the same commit. Both files are in Sheets. It does not demand one atomic commit spanning five repositories. Replacing that rule would remove a useful local consistency obligation to solve a problem it did not create. |
| + |
| +**Confidence:** high for both readings. |
| + |
| +**Smallest correction:** identify the Docs change as an intentional qualification under suite ruling 6, with its prior reason accurately cited. Retain Sheets' local same-commit rule and add a separate suite-citation requirement. Do not attribute the new positions to the earlier text. |
| + |
| +### 16. P2 — Citation currency can stay green across binding changes |
| + |
| +**Where:** doctrine §11 gate 9, lines 584–590; §10; Appendix A's introduction. |
| + |
| +**What is wrong:** the new measurable revision only has to increase for changes to §1 or §6. Binding behavior also lives in §3–§5 and §7–§8. |
| + |
| +**Why:** change the hand-off commit rule or roster stale-cache policy in its owning section, leave §6's citation untouched, and the prescribed revision can remain unchanged. A consumer of the old behavior passes currency indefinitely. The gate also does not specify checking that a claimed revision actually belongs to the cited commit. The appendix still asks only for a commit, although the new gate requires both. |
| + |
| +**Confidence:** high. |
| + |
| +**Smallest correction:** bump the revision for every normative contract change, wherever it lives; derive or verify revision from the cited commit; and update the consuming-section format consistently. A one-revision grace period is measurable, but remains a deliberate compatibility allowance, not proof that a consumer follows current security requirements. |
| + |
| +## Gate-by-gate assessment |
| + |
| +These are assessments of the proposed tests. No contract module exists here, so none of its gates was executed. |
| + |
| +| Gate | Can it be written as stated? | |
| +|---|---| |
| +| 1. Fixture conformance | The harness can be written. Complete golden requests/responses still need the missing call-8/write shapes and error precedence resolved. Generic conformance must distinguish the calls each app actually serves; S6, for example, belongs to Sheets. | |
| +| 2. S2 total decoder | Yes as a bounded decoder property, including an unsupported subject kind. It does not prove correct authorization or revision semantics: findings 8 and 11 need semantic fixtures. | |
| +| 3. Custody | Yes with clarified fixtures. “Grant on a deleted item” should mean a former grantee or retained grant row, not require a dangling live grant contrary to an app's schema. Compare responses for the same requested id in private/deleted alternative states. Add the transition in finding 8. | |
| +| 4. No cross-team dial | Literal zero connections for a server-issued redirect is impossible: the client must connect to the allowed first host to receive the redirect. Require zero connections for an initially forbidden host and zero **follow-up** connections after an allowed host redirects. Use two fixture servers or an instrumented transport. | |
| +| 5. Roster | Not yet. “After one epoch” lacks an observation/time condition; failed refresh and retained guest grants have conflicting expected results. See findings 5, 7 and 10. The lone-instance no-home-network assertion is independently testable. | |
| +| 6. Write-as-self | Yes for the three stated access checks, but derive actor identity from the authenticated consumer and test attempted actor spoofing. Actor/destination checks alone do not exercise provenance; finding 3 needs its own gate. | |
| +| 7. Hand-off | Not as a complete proof yet. Define receipt/commit states and test through actual middleware plus browser policy. A screen-render fixture cannot establish user-confirmed mutation. See findings 4 and 9. | |
| +| 8. Publish database | A seeded database disclosure test is feasible. The unchanged-information premise is false or at least undecided for owner roles, and a roster response fixture does not establish at-rest behavior. See finding 13. | |
| +| 9. Citation currency | Arithmetic is feasible; normative coverage and commit/revision agreement are missing. See finding 16. | |
| + |
| +## Sources and verification limits |
| + |
| +Paths below identify the exact local documents used. Section and line references in the findings refer to those files as read on 2026-09-05. |
| + |
| +| Source | Checkout and commit | |
| +|---|---| |
| +| [Doctrine](docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md) | This worktree, `0c4f098`; all sections read, revision-1 diff inspected. | |
| +| [Home design](/home/paulca/amadan.net/oficina/home/.claude/worktrees/trunk/docs/superpowers/specs/2026-08-30-home-design.md) | `home/.claude/worktrees/trunk`, `5a42f2d`. The supplied primary checkout is on older `domain/oficina-works` at `462df84`, which lacks rulings 17–21; those citations were checked against `main` instead. | |
| +| [Activity design](/home/paulca/amadan.net/oficina/home/.claude/worktrees/design-activity-pull/docs/superpowers/specs/2026-09-05-activity-pull-design.md) | `design-activity-pull`, `c9274bf`; still explicitly awaiting independent review. Adoption in principle is not approval of all its protocol details. | |
| +| [Memoria design](/home/paulca/amadan.net/oficina/memoria/docs/superpowers/specs/2026-09-03-memoria-design.md) | `617eb2a`; relevant contracts, grants, ingestion, snapshots, gates and acknowledged open defects inspected. | |
| +| [Docs design](/home/paulca/amadan.net/oficina/docs/docs/superpowers/specs/2026-08-30-docs-design.md) | `f6cf5c2`; also checked router/session integration and the v0.22.0 rastrillo CSRF implementation. | |
| +| [Sheets design](/home/paulca/amadan.net/oficina/sheets-core/docs/superpowers/specs/2026-08-30-sheets-design.md) | `bad02cb`; grants join, roles, home contract, PRODUCT.md and AGENTS.md checked. | |
| +| [Calendar design](/home/paulca/amadan.net/oficina/calendar/.claude/worktrees/calendar-first-screens/docs/superpowers/specs/2026-09-02-calendar-design.md) | `calendar-first-screens`, `f18898b`; no spec exists at the brief's primary-checkout path. The worktree spec and `internal/calendar/models.go` confirm the team-calendar grant model and absence of a commenter role. | |
| + |
| +The module-path claim in §10 is correct for the checked `go.mod` files: Docs alone still uses `github.com/carlosframework/docs`. The S4/S5 resolution is supported by memoria §6.1. The home really does specify return-host allowlisting and browser-bound login state; finding 4 concerns the additional document-write protocol, not the existence of those protections. |
| + |
| +Browser-request reasoning was checked against the [Fetch Standard's Origin handling](https://fetch.spec.whatwg.org/#origin-header) and [OWASP's CSRF guidance](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html). The middleware probe ran from a temporary file, using the installed dependency without network access; it made no live HTTP request. No end-to-end browser test or deployment was performed. Cross-repository discussions were not sent, and claims about Paul's private brainstorm or discussion 33's full contents were not treated as independently verified evidence. |
diff --git a/CODEX-REVIEW-BRIEF.md b/CODEX-REVIEW-BRIEF.md| new file mode 100644 |
| index 0000000..26fcb8b |
| --- /dev/null |
| +++ b/CODEX-REVIEW-BRIEF.md |
| @@ -0,0 +1,76 @@ |
| +# Review brief — the Oficina suite sharing doctrine, revision 2 |
| + |
| +You are the **third hand**. Be adversarial. Your job is to find what is |
| +wrong, not to agree. |
| + |
| +## What to review |
| + |
| +`docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md` in this |
| +worktree. It is a design doctrine binding five apps. Read all of it. |
| + |
| +## History you need |
| + |
| +- **Revision 1** was drafted by one model (Fable 5.1) from a brainstorm |
| + with Paul, the repository owner. |
| +- **Revision 2** is revision 1 plus a review pass by a second model |
| + (Claude Opus 5), which found six defects and wrote corrections into the |
| + text. Every correction is marked `*(Review pass, 2026-09-05.)*`. |
| +- `git diff 609394e..HEAD` shows exactly what the review changed. |
| + |
| +## What matters most |
| + |
| +The status block and §14 flag that **three of those corrections change a |
| +rule and have been read by nobody but their author**. They are the |
| +priority of this review: |
| + |
| +1. **S2's answer is intersected with custody** (§4.3). Claim: without it, |
| + §4.2's "a sibling cannot learn that a private item exists" is false, |
| + because an item that leaves team-visibility shows in the S1 feed as |
| + `gone` — which §4.2 requires to be indistinguishable from deletion — |
| + while an unintersected S2 would answer `VISIBLE` for it to a grantee's |
| + asker and `NOT_VISIBLE` for a deleted one. |
| +2. **The S2/S6 asker subject is typed on the wire** (§3). Claim: its |
| + meaning is shape-dependent (person ref in suite shape, lowercased |
| + address in lone shape), nothing else on the wire says which, and a |
| + mismatched pair fails to `NOT_VISIBLE` — the one value §4.3 permits to |
| + be silent — for what is really a mis-provisioning. |
| +3. **The hand-off checks `Origin` and asks the person to commit** (§7.1). |
| + Claim: same registrable domain is what carries the session cookie and |
| + also what lets any same-site page drive a create; home call 6, whose |
| + shape §7.1 borrows, validates hosts against the consumers allowlist and |
| + binds to the browser (home spec §5.6), and §7.1 borrowed the shape |
| + without either. |
| + |
| +For each: **is the defect real, is the correction right, and does the |
| +correction break something else?** A correction that is right for the |
| +wrong reason is a finding. So is one that costs a consumer something the |
| +review claimed it costs nothing. |
| + |
| +## Then the rest of the document |
| + |
| +The review may have missed things or introduced new ones. Look hardest at: |
| + |
| +- §0's two falsifiable halves. Does the rest of the document actually |
| + satisfy them, or is there a path that breaks one? |
| +- The leak list, §9. Is it complete for the design as it now stands? Every |
| + new rule the review added should either be absent from it or accounted |
| + for. |
| +- The gates, §11. §11's own first line says a gate that cannot be written |
| + as stated is a spec bug. Can each one actually be written and run? |
| +- Anything the doctrine asserts about a sibling spec. **Verify the |
| + citations rather than trusting them** — the sibling repositories are on |
| + disk, read-only, at `/home/paulca/amadan.net/oficina/{home,docs,memoria, |
| + sheets-core,calendar}`, each with its spec under |
| + `docs/superpowers/specs/`. The review found one defect (finding 1) by |
| + reading memoria's §4.1 and noticing the doctrine's adoption of S2 did |
| + not match what memoria actually specifies. There may be more of those. |
| + |
| +## What to produce |
| + |
| +Write your findings to `CODEX-FINDINGS.md` in this worktree. For each |
| +finding: what is wrong, why it is wrong, how confident you are, and the |
| +smallest correction. Order by severity. If one of the three rule changes |
| +above is fine, say so explicitly and say why — a clean verdict on those is |
| +as useful as a defect. |
| + |
| +Do not edit the doctrine itself. Findings only. |
diff --git a/README.md b/README.md| index efa9330..e66aa2c 100644 |
| --- a/README.md |
| +++ b/README.md |
| @@ -25,9 +25,14 @@ the conformance suite in CI against the version it pins. |
| ## Status |
| -The doctrine is a design, revision 2, reviewed 2026-09-05 by Claude Opus |
| -5 in a session that did not write it. The review found six defects and its |
| -corrections are in the text. Three of them change a rule rather than |
| -tighten a sentence, and those are review-authored and open for Paul — the |
| -doctrine's §14 says which. Read the status block before adopting anything |
| -from it. |
| +The doctrine is a design, revision 2, and two models have now read it. |
| +Claude Opus 5 reviewed it on 2026-09-05 and found six defects, whose |
| +corrections are in the text. GPT-6 read it after that as a third hand and |
| +returned **request changes**: sixteen findings, six at the trust boundary, |
| +two of them against the first review's own reasoning. Those two are |
| +corrected; the rest are `CODEX-FINDINGS.md`, held open in the doctrine's |
| +§14 for Paul. |
| + |
| +So the doctrine is reviewed but not settled. §§0–2 and the rulings drew no |
| +finding. Do not adopt §§3–8 as final, and do not run §12 step 1, until |
| +§14's first item is answered. Read the status block first. |
diff --git a/docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md b/docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md| index 860fbb8..73bcd3f 100644 |
| --- a/docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md |
| +++ b/docs/superpowers/specs/2026-09-05-suite-sharing-doctrine.md |
| @@ -16,9 +16,18 @@ gate 9 gains a measurable unit (§11). Three change a rule: S2's answer is |
| intersected with custody (§4.3), the S2 asker subject is typed on the wire |
| (§3), and the hand-off gains an origin check and a confirmation step |
| (§7.1). No ruling in §1 changed. |
| -**The three rule changes are review-authored and have not themselves been |
| -read by a third hand.** They are the review's own text, not Paul's, and |
| -§14 holds them open for his word. Everything else stands as drafted. |
| +**Read by a third hand, 2026-09-05: GPT-6 via Codex, verdict *request |
| +changes*.** It confirmed all three rule changes as answering real defects, |
| +and confirmed the fix for the typed asker subject outright. It found |
| +sixteen defects in the document as a whole, six of them at the trust |
| +boundary, and two of them in the revision-2 corrections' own reasoning — |
| +both since corrected here (§4.3, §7.1). Its findings are |
| +`CODEX-FINDINGS.md` beside this file, and they are **not** folded into the |
| +text: they are a third party's report, and §14 holds them for Paul. |
| +**So this is reviewed but not settled.** Two models have now read it and |
| +the second asks for changes. Do not adopt §§3–8 as final, and do not let |
| +§12 step 1 carry them into four app specs, before §14's first item is |
| +answered. §§0–2 and the §1 rulings drew no finding and stand as drafted. |
| **Scope:** the rules that bind every app in the suite when it talks to |
| another app or to the home. Consumed by `oficina/home`, `oficina/docs`, |
| `oficina/sheets`, `oficina/memoria`, `oficina/calendar`, and any wrapper |
| @@ -200,13 +209,27 @@ named asker, and `NOT_VISIBLE` for the one that was deleted. The leak the |
| S1 visibility flag was removed to prevent, and the leak S3's "404, never |
| 403" was designed to prevent, would arrive on the third call. |
| -The intersection costs a correct consumer nothing. It speaks only items it |
| -ingested, and S3 gave it none that were not team-visible. It costs an |
| -incorrect one exactly what decision 3 says it should: custody bounds what a |
| -sibling may hold, and a sibling has no business asking about what it may |
| -not hold. *(Review pass, 2026-09-05. memoria §4.1 states the join |
| -unintersected — "plus rows granted to me individually" — so A.4 carries |
| -the correction.)* |
| +**The intersection has a cost, and it is a product cost.** A consumer that |
| +ingested an item while it was team-visible, and asks about it after it went |
| +private but before `gone` reaches it, now gets `NOT_VISIBLE` where it would |
| +have got `VISIBLE` for a person who still holds an individual grant. Ana |
| +loses that passage in memoria, and any answer built on it, while the |
| +document itself stays open to her in the app of record. That is the price |
| +of the indistinguishability §4.2 promises; it cannot be avoided while the |
| +promise stands, and the consumer's honest move is to say which app the |
| +answer went to rather than to return a shorter one silently. |
| + |
| +Revision 2 first wrote that the intersection cost a correct consumer |
| +nothing, reasoning from visibility at ingestion. That reasoning is wrong: |
| +a later visibility change is exactly what the defect turns on, so the case |
| +where the intersection bites is the case the argument assumed away. It |
| +costs an *incorrect* consumer what decision 3 says it should — custody |
| +bounds what a sibling may hold, and a sibling has no business asking about |
| +what it may not hold. *(Review pass, 2026-09-05. memoria §4.1 states the |
| +join unintersected — "plus rows granted to me individually" — so A.4 |
| +carries the correction. The cost paragraph is corrected from the GPT-6 |
| +review, finding 8, which also wants a transition fixture: ingest, downgrade |
| +while keeping Ana's grant, query before the feed catches up.)* |
| --- |
| @@ -416,16 +439,33 @@ it, and a create endpoint that commits on receipt is a drive-by. Home call |
| binds the flow to the browser that began it. So: |
| - **The `Origin` header must match a row in the receiver's `consumers` |
| - table**, which §4.1 already makes an origin allowlist. No new machinery, |
| - and it holds the §4.1 barrier on a path that otherwise bypasses it. |
| + table**, which §4.1 already makes an origin allowlist. It holds the §4.1 |
| + barrier on a path that otherwise bypasses it. |
| - **The receiver renders what it received and the person commits it.** One |
| screen, one button, the person's own session. This is what makes the |
| document Ana's act rather than memoria's, which is the entire claim of |
| §7.1; without it §7.2's three constraints are reachable by any sibling |
| through §7.1, and the distinction between the two sections collapses. |
| -Neither is a script and neither costs a round trip the flow did not |
| -already have. *(Review pass, 2026-09-05.)* |
| +**Both cost more than revision 2 said they did.** It claimed no new |
| +machinery and no extra round trip. Neither is true, and the third hand |
| +established it against the pinned dependencies rather than by reading the |
| +specs: memoria's rastrillo default CSP sets `form-action 'self'`, which |
| +stops the form on the way out; Docs mounts `csrf.Protect` above every |
| +route and its `SameOrigin` check refuses a same-site cross-origin POST, |
| +measured at 204 same-origin against 403 same-site; and for a person |
| +already signed in, receipt-then-commit is a second POST where revision 1 |
| +wrote on the first. So the real bill is a narrowly scoped sender |
| +`form-action` allowance, a dedicated receiving route carrying its own |
| +origin guard, and one more round trip — and gate 7 has to run through that |
| +middleware, since a handler-level fixture passes while a browser does not. |
| +Still scriptless, which was the property worth protecting. |
| + |
| +**The commit step is also under-specified**, and this section does not yet |
| +fix it: nothing here binds the final mutation to the reviewed body, to the |
| +browser that reviewed it, or to a single redemption. *(Review pass, |
| +2026-09-05; corrected and extended from the GPT-6 review, findings 4 and |
| +9.)* |
| ### 7.2 No person is present: write-as-self |
| @@ -637,13 +677,39 @@ change to where the list comes from. |
| ## 14. Open |
| -- **The review's three rule changes**, for Paul. The review pass is done |
| - and recorded, and it authored three rules nobody else has read: S2's |
| - custody intersection (§4.3), the typed asker subject (§3), and the |
| - hand-off's origin check and confirmation step (§7.1). Each closes a |
| - defect stated beside it, and each is the reviewer's own text, so each |
| - wants his word before step 1 carries it into four app specs. The other |
| - three corrections tighten sentences and need nobody. |
| +- **The GPT-6 review's sixteen findings**, for Paul. `CODEX-FINDINGS.md`, |
| + 2026-09-05, verdict *request changes*. It reads the three review-authored |
| + rule changes as answering real defects and passes the typed asker subject |
| + outright; its other findings are open. Six sit at the trust boundary and |
| + none is editorial: |
| + - **Directory rows carry an instance-token write path** (finding 1). |
| + Home call 2 lets an instance upsert its own directory rows. §5.1 makes |
| + those rows the authoritative roster without giving membership its own |
| + write authority, so an instance can announce a member around §5.4's |
| + owner step-up. This is the one that most looks like it wants a ruling. |
| + - **§5.5's compromised-home bound is too kind to itself** (finding 2). A |
| + home that mints assertions can impersonate an existing owner, which |
| + needs no membership row and predates ruling 4; and a compromised |
| + process is not made to leave the directory row or notification the |
| + section counts as evidence. |
| + - **Neither write path carries provenance** (finding 3), so §4.3's |
| + transitive-provenance rule has no mechanism at the moment content |
| + crosses an app. |
| + - **A roster refetch that fails has no stated behaviour** (finding 5), |
| + and a home rolled back to a lower epoch can readmit. |
| + - **Custody is a rule about rendering, not about erasure** (finding 6). |
| + A consumer keeps what it lawfully ingested after the item goes |
| + private, so §0's "inside" test and §9 item 1 promise more than the |
| + design delivers. |
| + - **The hand-off's commit step** (finding 4), above. |
| + |
| + Ten more at P2, including that Appendix A leaves contradictions standing |
| + in memoria and paraphrases where §10 asks for citations (finding 14), |
| + that §2 and §8 put words in Docs' and Sheets' mouths their own text does |
| + not say (finding 15), and that gate 9 still goes green across a change to |
| + §3, §5 or §7 (finding 16). Its gate-by-gate table marks 5 and 7 as not |
| + yet writable, and 4 as impossible literally — a client must connect to |
| + the first host to be told to redirect. |
| - **Pegamento filing.** §5.5's divergence from Pegamento §10.3, and |
| ruling 1's application of §1 to a suite, both to `carlos/pegamento`. |
| - **The `/auth/begin` dial.** rastrillo's sign-in classifier fetches |