| 1 | # Findings on the relay spec, revision 3 |
| 2 | |
| 3 | Reviewed by **GPT-6 (Codex), 2026-09-07**, third pass, against revision 3 at `6b64d75`, with Paul's rulings treated as fixed. Unedited apart from this note. Revision 4 records how each finding was taken. |
| 4 | |
| 5 | | Revision-2 finding | Disposition | Reason | |
| 6 | |---|---|---| |
| 7 | | R2-1 — Aggregate publication | Partial | ETag-first ordering fixes stale publication, but interrupted deletion escapes the specified reconciler. | |
| 8 | | R2-2 — Interrupted operations | Partial | Phases recover interrupted enable; checking `op` before a separate config write does not fence superseded writers. | |
| 9 | | R2-3 — Owner binding | Partial | Typed addresses distinguish transports, but the deletion callback lacks the relay address required by the text. | |
| 10 | | R2-4 — Rotation | Partial | Retirement and loaded-key heartbeats are specified; adoption evidence remains insufficient and other issuance paths bypass the gate. | |
| 11 | | R2-5 — Metering | Partial | Forwarding directions and rejected ingress are corrected; restart recovery loses the preceding-hour portion of the budget. | |
| 12 | | R2-6 — Config adoption | Partial | An acknowledgement is specified, but its identity and granularity cannot establish per-instance adoption. | |
| 13 | | R2-7 — Operational limits | Closed | Account and box admission limits and token buckets supply the bounds chosen by ruling 6. | |
| 14 | | R2-8 — Memory priority | Partial | The new arithmetic omits swap and page tables; local shedding does not establish the stated host-pressure behavior. | |
| 15 | | R2-9 — EIP isolation | Closed | Forwarding is restricted to live relay ports, with a real-EIP acceptance test. | |
| 16 | | R2-10 — Authoritative stage boundary | Partial | Paul’s decisions are recorded, but several contradictory roadmap passages remain. | |
| 17 | | R2-11 — Region ordering | Closed | Console order removes the placement-conversion ambiguity; the stale parent promise is covered below. | |
| 18 | | R2-12 — Incorrect references | Closed | The nonexistent section reference is removed and `writeEnv` now points to its definition. | |
| 19 | |
| 20 | Reviewed commit `6b64d753c9e62d5a4b8550fa5d2cd317ec98c0f4`. Paul’s six rulings are treated as fixed. All three cryptographic vector values reproduce. No files were changed and no live TURN or EC2 tests were run. |
| 21 | |
| 22 | ## R3-1 — P1 — Interrupted deletion can leave authority enabled indefinitely |
| 23 | |
| 24 | **Section:** §5, reconciliation and app deletion. |
| 25 | |
| 26 | The reconciler processes existing records whose phase is not `done`. App deletion removes the record itself before publishing the aggregate. If deletion stops between those steps, no unfinished record remains for reconciliation to find. |
| 27 | |
| 28 | The old aggregate can therefore continue authorizing the deleted app. Successful sixty-second reads renew the relay’s lease against that stale aggregate, so the fifteen-minute lease does not bound the failure. A holder of the derived key can keep minting credentials while that authority remains. |
| 29 | |
| 30 | **Evidence:** The [reconciliation algorithm][reconcile] only resumes unfinished records, whereas [app deletion][deletion] claims that reconciliation catches a missing record within five minutes. |
| 31 | |
| 32 | **Correction:** Require an unconditional aggregate rebuild at console start and every five minutes, including when no unfinished records exist, or preserve a deletion tombstone until publication completes. Specify recovery after deletion removes the record but before aggregate publication. |
| 33 | |
| 34 | ## R3-2 — P2 — The operation check still permits stale config publication |
| 35 | |
| 36 | **Section:** §5, operation walk. |
| 37 | |
| 38 | The following sequence satisfies the text: |
| 39 | |
| 40 | 1. Enable operation A re-reads `relay.json` and confirms its `op`. |
| 41 | 2. A pauses before calling `appconfig.Apply`. |
| 42 | 3. Disable operation B removes the credentials, publishes the aggregate and reaches `done`. |
| 43 | 4. A resumes. `Apply` reads the current config bundle and successfully writes A’s credentials back. |
| 44 | 5. A’s subsequent phase CAS fails, but its config write has already landed. |
| 45 | |
| 46 | Reconciliation skips B because it is `done`. The wrong config can persist indefinitely. Re-enable or rotation can similarly leave an enabled app holding superseded credentials. |
| 47 | |
| 48 | **Evidence:** The [operation check][reconcile] precedes the config write. [`applyAt`][apply] obtains its own config ETag and conditionally writes that object; it does not condition publication on the relay operation. |
| 49 | |
| 50 | **Correction:** Define a fence enforced at config publication, covering API handlers and reconciliation. The config write must reject a superseded operation, rather than discovering supersession afterward. Merely adding another record read leaves the same race. |
| 51 | |
| 52 | ## R3-3 — P2 — Allocation deletion requires an argument Pion does not provide |
| 53 | |
| 54 | **Section:** §3, owner binding. |
| 55 | |
| 56 | The spec requires `OnAllocationDeleted` to remove only the binding whose relay address matches the event’s. Pion v4.1.4 supplies no relay address in that callback. It supplies source and destination addresses, protocol, username and realm. The creation callback does supply the relay address. |
| 57 | |
| 58 | This matters because Pion removes the allocation from its manager before closing it and issuing the deletion event. Cleanup must not erase a replacement binding created while the old allocation is closing. |
| 59 | |
| 60 | **Evidence:** [Owner-binding requirement][binding]; Pion’s [callback signatures][events] and [deletion sequence][pion-delete]. |
| 61 | |
| 62 | **Correction:** Associate cleanup with the registered relay-socket wrapper, which knows its allocation identity, or specify another available discriminator that cannot remove a replacement binding. Correct the callback claim. |
| 63 | |
| 64 | ## R3-4 — P2 — The config acknowledgement cannot prove the rotation cutoff |
| 65 | |
| 66 | **Section:** §2, config delivery and credential lifetime; §5, config adoption and rotation. |
| 67 | |
| 68 | The proposed object is keyed by box, account and app, but adoption happens per route. One app can have multiple routes and environments on one box. If one route restarts successfully while another remains on old config, the successful acknowledgement can make the box appear adopted without identifying the stale instance. |
| 69 | |
| 70 | The generation string also lacks the required relationship to rendered content. The existing `config-gen.json` is a separate change notification across all bundles. Its update can fail after the manifest write, and concurrent touches can coalesce. It is not an immutable identifier embedded in the config being rendered. |
| 71 | |
| 72 | There is also a direct delivery contradiction: §2 says exec routes opting into file delivery restart, while §5 says they are not restarted. Existing socket-exec routes receive files and are bounced; opted-in TCP-address routes receive files but are never bounced. |
| 73 | |
| 74 | These distinctions determine whether the last issuer actually stopped using the old key. They cannot be collapsed into “a config-ack for that generation exists.” |
| 75 | |
| 76 | **Evidence:** [Acknowledgement schema and status rules][ack]; [delivery and lifetime promises][delivery]; existing [per-route/environment synchronization][sync], [generation semantics][generation], and [exec delivery contract][syncexec]. |
| 77 | |
| 78 | **Correction:** Define acknowledgements per route, including the effective relay key identity actually adopted and the result of application/restart. Distinguish running adoption, hibernating delivery, failed restart and externally managed processes. Bind the acknowledgement to the rendered config identity rather than relying on the app-wide notification alone. Retirement must wait for evidence covering every issuer; file delivery alone must not satisfy that condition. |
| 79 | |
| 80 | ## R3-5 — P2 — Enable and reconciliation can issue the new key before the rotation gate passes |
| 81 | |
| 82 | **Section:** §5, enable, reconciliation and rotation. |
| 83 | |
| 84 | Rotation updates the console’s master secret before invoking the command that checks relay heartbeats. During that interval, a first enable, region addition or resumed config operation derives credentials from the region’s current console master. |
| 85 | |
| 86 | Those paths have no heartbeat gate. They can distribute the new `kid` while a relay has failed to load it—even while `rotate` correctly refuses to proceed. A member then receives credentials that an advertised relay rejects. |
| 87 | |
| 88 | **Evidence:** [Enable selects the current master][enable]; [reconciliation derives from the record and masters][reconcile]; [rotation changes the console secret before checking acknowledgements][rotation]. |
| 89 | |
| 90 | **Correction:** Separate staged masters from the active issuance master, and switch issuance only after the heartbeat gate succeeds. Every credential-writing path must use that gated selection, including reconciliation and region additions. |
| 91 | |
| 92 | ## R3-6 — P2 — Restart recovery drops part of the rolling-hour budget |
| 93 | |
| 94 | **Section:** §3, limits and metering. |
| 95 | |
| 96 | The spec explicitly recovers only the current hour’s metrics object. At 12:01, the rolling-hour budget still includes traffic from 11:01 onward, most of which resides in the preceding object. |
| 97 | |
| 98 | A restart therefore forgets almost the entire window and can immediately readmit an app or account whose budget remains exhausted. This contradicts the stated recovery behavior. |
| 99 | |
| 100 | **Evidence:** [Rolling-hour limits and current-hour recovery][meters]. |
| 101 | |
| 102 | **Correction:** Load every hourly object intersecting the rolling window—normally the previous and current hour—and rebuild budgets from the relevant samples. Include a restart across an hour boundary in the acceptance tests. |
| 103 | |
| 104 | ## R3-7 — P2 — The OOM arithmetic does not establish the claimed ordering |
| 105 | |
| 106 | **Section:** §3, hardening and memory; §7, flagship gate. |
| 107 | |
| 108 | The stated tenant maximum of 1000 follows from RSS alone. Linux’s score also includes swap entries and page tables, and global scoring uses RAM plus swap. This repository permits a tenant to use swap equal to its memory cap, so a 1 GiB memory ceiling does not establish the claimed scoring bound. [Linux scoring implementation](https://raw.githubusercontent.com/torvalds/linux/v6.12/mm/oom_kill.c); [tenant swap allowance][swap]. |
| 109 | |
| 110 | There is a separate eligibility problem: exhausting the edge’s 80% cgroup ceiling invokes a kill within that tree. The relay, in its own unit, is not a candidate. Shedding only when the relay exceeds its own 160 MiB threshold does not respond to pressure created elsewhere while the relay remains small. |
| 111 | |
| 112 | **Evidence:** [The arithmetic and shedding trigger][memory]; [the edge’s cgroup boundary][edge-memory]; [the relay-first gate][relay-gate]. |
| 113 | |
| 114 | **Correction:** State which pressure conditions the guarantee covers and correct the scoring envelope to include swap and page tables. If relay-first shedding is required under host-wide pressure, specify a host-pressure trigger. Align the gate with that scope; the fixed defaults alone do not establish it. |
| 115 | |
| 116 | ## R3-8 — P2 — The roadmap still contradicts the approved relay contract |
| 117 | |
| 118 | **Sections:** Roadmap stage 4a and §11; relay §§2–5 and §9. |
| 119 | |
| 120 | The amendments resolve TLS, credential duration and the main metering gate, but leave these conflicting instructions: |
| 121 | |
| 122 | | Topic | Remaining roadmap text | Revision-3 contract | |
| 123 | |---|---|---| |
| 124 | | Master exposure | The custody paragraph describes exposure of enabled apps’ derived keys and calls the boundary no worse than existing secrets. | Ruling 3 explicitly exposes every region’s master through console sealed config, including authority to derive future app keys. | |
| 125 | | Region order | Placement puts the instance’s own region first. | Every instance receives console order. | |
| 126 | | Peer policy | Permissions refuse the relay’s own addresses. | The EIP is permitted, with forwarding restricted to live relay ports. | |
| 127 | | Authority lifecycle | Authority is synced from the route table; disable bumps incarnation. | Authority derives from per-app relay records; disable removes authority and re-enable creates the new incarnation. | |
| 128 | | Accounting amendment | §11 assigns three relay meters to stage 4a. | Ruling 5 assigns two meters to 4a and moves the inter-region meter to stage 5. | |
| 129 | |
| 130 | **Evidence:** Roadmap [authority and custody][roadmap-authority], [peer policy and placement][roadmap-policy], and [unit-economics amendment][roadmap-accounting]. |
| 131 | |
| 132 | **Correction:** Update these passages to reflect the fixed rulings and revision-3 mechanisms. No further policy decision is needed; the parent must stop giving the implementation plan conflicting instructions. |
| 133 | |
| 134 | **Verdict: request changes.** One P1 and seven P2 findings remain. |
| 135 | |
| 136 | [reconcile]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:389 |
| 137 | [deletion]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:505 |
| 138 | [apply]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/internal/appconfig/store.go:146 |
| 139 | [binding]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:199 |
| 140 | [events]: /home/paulca/.claude/jobs/f1f6099c/tmp/gomodcache/github.com/pion/turn/v4@v4.1.4/internal/allocation/event_handler.go:20 |
| 141 | [pion-delete]: /home/paulca/.claude/jobs/f1f6099c/tmp/gomodcache/github.com/pion/turn/v4@v4.1.4/internal/allocation/allocation_manager.go:150 |
| 142 | [ack]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:455 |
| 143 | [delivery]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:108 |
| 144 | [sync]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/internal/appconfig/sync.go:30 |
| 145 | [generation]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/internal/appconfig/generation.go:28 |
| 146 | [syncexec]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/internal/appconfig/syncexec.go:1 |
| 147 | [enable]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:442 |
| 148 | [rotation]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:481 |
| 149 | [meters]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:242 |
| 150 | [swap]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/internal/cgroup/cgroup.go:313 |
| 151 | [memory]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:157 |
| 152 | [edge-memory]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/internal/host/units/carlos-edge.service:57 |
| 153 | [relay-gate]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:560 |
| 154 | [roadmap-authority]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-media-platform-roadmap-design.md:620 |
| 155 | [roadmap-policy]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-media-platform-roadmap-design.md:656 |
| 156 | [roadmap-accounting]: /home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-media-platform-roadmap-design.md:979 |
amadan