docs(corpus): add push gateway architecture container node (#658) - #1570
Conversation
Signed-off-by: Serina Mcfall <serina.mcfall@gmail.com>
serina-mcfall
left a comment
There was a problem hiding this comment.
4 findings survived adjudication (2 major, 2 minor). Most of the cross-model pass's other findings were checked against the code and repo and did not hold up; see notes below.
Findings
1. (Major) PostgreSQL role description contradicts its own cited evidence
docs/push-gateway-deployment.md:41 is explicit that the migration role runs migrations and then the migration Job revokes CREATE from the runtime role (a separate role, default buzz_push_gateway_runtime) — the migration role itself is never demoted, and it "is never provided to runtime replicas." The corpus node's own evidence-ledger entry (front matter, the docs/push-gateway-deployment.md-cited FACT about roles) states this correctly. But the "Security" section of the body says: "a PostgreSQL migration-role/runtime-role split (DDL-capable role runs migrations and then revokes its own future CREATE grants...)". That swaps the actor — the DDL-capable role does not revoke its own grants; it revokes the runtime role's grants. The document contradicts its own cited source.
launchpad/docs/corpus/architecture/containers/push-gateway.md(Security section) vsdocs/push-gateway-deployment.md:41
2. (Major) "First six routes are App Attest-authenticated" overstates the challenge-issuance route
crates/buzz-push-gateway/src/http.rs's challenge handler (POST /v1/installations/challenges) performs no App Attest verification at all — it only generates and stores a random challenge value for the client to sign next. It cannot itself be "App Attest-authenticated"; it's the unauthenticated bootstrap step that precedes attestation. The doc's connected-containers table nonetheless labels all six client-facing routes, including this one, as "App Attest-authenticated routes," and the interfaces section describes "the first six" as a block "authenticated with Apple App Attest." This is a real overstatement of the route's own trust boundary, not just an imprecise gloss.
crates/buzz-push-gateway/src/http.rs#L110-L142(challengehandler, noapp_attestcall) vs the doc's inbound-interfaces table and prose
3. (Minor) Relationships-omission justification mischaracterizes corpus-agents as a governance node
The "No relationships are declared" section states the four other merged corpus nodes (corpus-readme, corpus-agents, corpus-standard-confidence, corpus-standard-decision-references) "are all governance nodes about the corpus itself." Verified against each node's front matter at the recorded revision (a44cf52f...): corpus-agents's type is agent, not governance — the other three are indeed governance. The underlying conclusion (none of the four are architecture nodes worth linking) still holds, so this doesn't change the decision to omit relationships, but the stated reasoning contains a verifiably wrong claim about the corpus's node-type composition.
launchpad/docs/corpus/architecture/containers/push-gateway.md("No relationships are declared") vslaunchpad/docs/corpus/AGENTS.mdfront matter (type: agent)
4. (Minor) Evidence ledger cites no tests and no Git/PR history, though both exist and the issue's DoD asks for both
Issue #658's "Required evidence before drafting" line names "tests... and relevant Git history, PRs and issues" as evidence categories to record. crates/buzz-push-gateway/src/ has #[cfg(test)] unit-test modules in 7 files (config.rs, authority.rs, strict_json.rs, grant.rs, postgres.rs, metrics.rs, apns.rs), and the whole crate was added in a single traceable PR (feat(push): add public APNs gateway (#1770)). Neither is cited anywhere in the evidence ledger or the "expected but not verified" section, and the doc doesn't state that it checked and found nothing relevant. This is a real gap against the DoD's evidence checklist, though it's a completeness gap rather than a factual error, and the doc's FACT/INFERENCE claims that are made are correctly sourced to the code/deployment doc they cite.
launchpad/docs/corpus/architecture/containers/push-gateway.md(evidence ledger) vscrates/buzz-push-gateway/src/*.rstest modules and PR #1770
Findings that did not survive adjudication
- Codex's "CLAUDE.md" provenance-mismatch claim (
provided_bynamingCLAUDE.mdwhen the notice is "actually" inAGENTS.md): refuted.CLAUDE.mdis a symlink toAGENTS.mdin this repo (ls -laconfirms); they are the same file with two names, so the citation is accurate. - Codex's claim that "PostgreSQL (gateway's own database)" overstates isolation: refuted.
docs/push-gateway-deployment.md:41explicitly mandates a dedicated gateway database (not just isolated roles/tables), so the doc's wording is accurate, not an overstatement. - Codex's claim that "sole holder of APNs provider credentials" and "only outbound network dependencies" are unsupported exhaustive claims: checked independently (grepped the whole crate tree and
Cargo.toml) and found no other component touching APNs credentials or making outbound HTTP calls outsideapns.rs. The claims hold as written; downgraded out of the survivor list. - Codex's claim that raw APNs response bodies could still leak into logs despite the
DeliveryOutcomeclassification:apns.rscontains no logging/tracing calls at all and is the only file usingreqwestto talk to APNs, so nothing outside the classified enum ever leaves that module. The "never leak" claim is well-supported by the code structure, not just asserted. - Codex's "architectural conclusions without classified evidence" (replica/database sharing, abuse-ceiling, backup exposure bounding): the core sentences are near-verbatim facts from
docs/push-gateway-deployment.md(already an evidence-ledger source), not unclassified inferences invented by this document. - Codex's "no Markdown links, only code-formatted paths" complaint: doesn't match this corpus's own convention — evidence-ledger citations throughout the corpus (including this node's own front matter) are plain path strings, not
[text](url)links. - Codex's "seven POST routes" misreading concern: the doc's own section headers already separate "Inbound, public" (the seven POST routes) from "Inbound, private" (the GET health/metrics routes), so this isn't ambiguous in context.
- Codex's implementation-reference-duplication concern (route list, table names, CI job names, etc. is "broader than a container-level node"): an editorial/scope opinion, not a verifiable defect — not carried forward as a finding.
Automated review: Claude (review-code pass) + Codex (cross-model pass), adjudicated together. Not an independent human review -- final merge decision is the repo owner's.
serina-mcfall
left a comment
There was a problem hiding this comment.
9 findings survived adjudication (5 major, 3 minor, 1 nit); several Codex-flagged items were re-verified and refuted.
Major
-
NIP-11 push advertisement claim omits its second gating condition (tenant/community binding).
push-gateway.mdstates (front-matter evidence + body, e.g. "the relay advertises ... only when a gateway delivery URL is configured, so a relay with no gateway configured advertises no push capability at all"). I re-readcrates/buzz-relay/src/nip11.rsat the cited revision (a44cf52):nip11_document()only computes a non-Nonetenant_hostwhenpush_gateway_delivery_url.is_some()andtenant::bind_community(&state.db, raw_host)succeeds;push_descriptor()itself doeslet host = tenant_host?;before even checkingpush_configured. The crate's own unit test names this:push_descriptor_is_gated_by_gateway_configuration_and_tenant_bindingassertspush_descriptor(true, ..., None).is_none()— fully configured push still advertises nothing without a bound tenant. The doc states a necessary condition as if it were the operative gate, and would mislead an operator debugging "why doesn't my relay advertise nip-pl even though I set the gateway URL." (review-code, confirmed as written) -
App Attest claim is false for one of the six routes it covers. The doc says (body, "Inbound and outbound interfaces"): "The first six are the client enrollment/delegation/rotation/revocation flow, authenticated with Apple App Attest." I read
crates/buzz-push-gateway/src/http.rsat the cited revision:challenge()(/v1/installations/challenges) performs no App Attest verification at all — it just mints and stores a random challenge value; App Attest attestation is only checked inenroll(), and per-request assertions indelegate/rotate_endpoint/revoke_delegation/revoke_installation. The challenge endpoint is necessarily pre-auth (nothing to attest against yet), so calling all six "authenticated with Apple App Attest" is materially wrong for one of them. (Codex, confirmed) -
PostgreSQL role-model claim contradicts the doc's own evidence ledger and the deployment doc. Body text ("Security" section) says: "a PostgreSQL migration-role/runtime-role split (DDL-capable role runs migrations and then revokes its own future
CREATEgrants...)". But the doc's own evidence entry (front matter, sourced fromdocs/push-gateway-deployment.md) correctly says CREATE is revoked from the runtime role. I confirmed againstdocs/push-gateway-deployment.mdat the cited revision: "the Job revokes databaseCREATEfrom that role [the runtime role] ... The migration role must ... be allowed to issue those grants; it is never provided to runtime replicas" — the migration role keeps its own DDL capability, it does not revoke its own grants. The body describes a different (and wrong) security model than the one actually implemented and than the doc's own citation. (Codex, confirmed) -
TEAM_KNOWLEDGEentry conflates a supported claim with an unsupported one. The entry attributes both "this fork operates rather than develops Buzz" and "does not currently deploy the push gateway to any launchpad-operated host" to the same source (CLAUDE.md's fork-scope notice). I confirmed the fork-scope notice only supports the first claim ("we operate Buzz; we do not develop it") — it says nothing about push-gateway deployment specifically. The negative deployment claim needs its own recorded search (which the doc's "Expected but not verified" section does more honestly describe as "no manifest/Terraform/ArgoCD app was found"), not a blanket attribution to a document that doesn't address it. (Codex, confirmed) -
Required pre-drafting evidence (tests, Git history, PRs/issues) was not recorded, and its absence is not cosmetic. Issue #658 explicitly requires "tests, specifications, migrations and configuration; relevant Git history, PRs and issues" before drafting, and
launchpad/docs/corpus/AGENTS.md's "Creating a node" step 3 repeats this. The plan file's evidence-gathering step (launchpad/plans/2026-08-27-issue-658-corpus-doc.mdstep 1) lists only source.rsfiles and the deployment doc — no test file, and nothing in the front-matter evidence ledger (23 entries) cites a test. This is not just a paperwork gap: the unit test named in finding 1 above (push_descriptor_is_gated_by_gateway_configuration_and_tenant_binding) would have directly caught that factual error had it been read. (Codex, confirmed; downgraded from "blocker" to "major" — the document is still substantially accurate, but the process gap has a demonstrated causal link to a real defect)
Minor
-
Required
checkCI job is currently failing.gh pr checks 1570showscheckfailing withPR body check failed: - Missing '### Issue type' section.(launchpad/scripts/pr_body_check.py). Mechanical, not a content defect —validate,audit, andscriptschecks all pass, and the PR body's own claimed local verification (validate.pyPASS with 6 pre-existing UNVERIFIED notices;unittest discoverOK, 79 tests) checks out against CI. Not mergeable as-is until the PR body gets an "### Issue type" section. (review-code, confirmed) -
A few absolute claims outrun what a single module can establish. "Only outbound network dependencies," "sole inbound client," "sole holder" of APNs credentials, and APNs response bodies "never" reaching logs/metrics are all sourced to single modules (
apns.rs,push_runtime.rs). These are reasonable architectural summaries (the deployment doc does independently instruct that relays must never hold APNs credentials), but the absolute wording slightly exceeds what any one cited file can prove system-wide. (Codex, confirmed but downgraded from major — the doc's own deployment-doc citations partially back the framing) -
Two discovered follow-up nodes are described but not filed as linked tasks. AGENTS.md's node-creation guidance says a second concept that turns up while writing "does not get folded in. File it as its own task and link to it." The doc names "a future interfaces-events corpus node" (NIP-PL protocol) and "a future architecture-containers node for the relay" in its Scope/Omissions table but links neither to an actual issue. Mitigated by the fact that this is disclosed prominently as a known gap rather than silently omitted. (Codex, confirmed but downgraded from major for that reason)
Nit
origin: launchpadis a debatable choice for a node whose content is almost entirely about upstream (block/buzz) behavior. Per ADR-0003's vocabulary (reused byorigininnode.schema.json),[upstream]marks how the actual system behaves and[launchpad]marks this fork's own work/process. Nearly every evidence entry here is a FACT aboutblock/buzz's own crates; only the finalTEAM_KNOWLEDGEentry is fork-specific. Neithernode.schema.json,README.md, norAGENTS.mdresolves whether the single node-leveloriginfield means authoring/hosting context (trivially "launchpad" for every node in this corpus) or subject-matter provenance (mostly "upstream" here). No merged precedent exists yet to check against. Not a clear defect given the corpus's own documentation is ambiguous on this point, but worth the batch owner's attention since it will recur across every architecture/container node in this 47-PR batch. (review-code, confirmed as an open ambiguity, not a defect)
Findings that did not survive adjudication (Codex-only, refuted on re-check)
- "Implementation and further reading" links aren't Markdown hyperlinks / verification sources absent — refuted:
node.schema.json'sevidenceEntry.evidencedescription explicitly permits "paths, commit-pinned links, or similar" as citations, and noverification-type corpus node exists yet in the merged corpus to link to (onlygovernancenodes are merged). - "gateway's own database" overstates the evidence — refuted:
docs/push-gateway-deployment.mdexplicitly requires "a dedicated gateway database, not the relay database," so "own database" is directly supported. - Capability-stateless wording is misleading — refuted: the body already distinguishes "delegation capabilities... opaque, AEAD-sealed... gateway does not track them server-side" from "installation/delegation/admission bookkeeping... is genuinely stateful, persisted" in the same paragraph.
- Abuse-ceiling statement is an unclassified inference — refuted: "replica count does not multiply the abuse ceiling" is a near-verbatim quote of
docs/push-gateway-deployment.md, a source already cited multiple times in the evidence ledger, not an unsupported inference. - No recorded validation/unit-test result in the diff — refuted: the PR body records both
validate.py(PASS) and the unittest run (79 tests, OK), and CI'svalidatecheck independently passes.
Automated review: Claude (review-code pass) + Codex (cross-model pass), adjudicated together. Not an independent human review -- final merge decision is the repo owner's.
Summary
Adds
launchpad/docs/corpus/architecture/containers/push-gateway.md, a single canonical corpus node documentingbuzz-push-gatewayas an architecture container: its responsibility/technology/ownership boundary against the relay, its inbound/outbound interfaces and directly connected containers (mobile client, relay, PostgreSQL, Apple APNs), and the deployment/data/security implications of that boundary. Links implementation paths and the existingdocs/push-gateway-deployment.mdrunbook without duplicating their detail.Closes #658
Verification
python3 launchpad/project-intelligence/corpus/validate.pyexits 0 (PASS; 6 non-fatal UNVERIFIED commit/graph-edge provenance notices across the corpus, none from this node's substantive claims).python3 -m unittest discover -s launchpad/project-intelligence/corpus/tests -p "test_*.py"reports OK (79 tests).Self-review
Self-review was performed: the diff was re-read against the issue's full Definition of Done checklist and the category-containers DoD tail (states responsibility/technology/ownership boundary; names inbound/outbound interfaces and directly connected containers/systems; links deployment/data/security implications; links implementation without duplicating it), every evidence entry was checked against the source it cites, and the diff was confirmed to touch exactly the plan file and the one target document -- no second canonical document was created. No automated review-code pass ran on this PR -- that is a real gap, not implied coverage.
Draft -- adjudicate/cross-model pass deferred to the batch owner's review before merge.
Issue type
Task