amadan
spec: relay revision 5, codex round 4
Three findings taken: a relay-writer lease so one console process at a time writes relay state, config acks naming the full username suffix so a re-enable between syncs cannot look adopted, and retirement no earlier than max(rotation + 28h, latest cutover + 4h) with manual restarts counted as cutovers.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
2 files changed,
+115
−30
docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md+54 −30docs/superpowers/specs/CODEX-FINDINGS-RELAY-R4.md+61 −0
CI passed — run details
diff --git a/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md b/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md| index c0fe3f470..693305018 100644 |
| --- a/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md |
| +++ b/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md |
| @@ -1,14 +1,15 @@ |
| # Stage 4a: a platform STUN and TURN relay |
| -**Date:** 2026-09-07, revision 4 |
| +**Date:** 2026-09-07, revision 5 |
| **Status:** rulings taken. Paul ruled on the six decisions in §9 on |
| 2026-09-07, each marked **(ruling)**; the roadmap's stage 4a text and |
| -gate are amended so the two documents agree. Reviewed three times by |
| +gate are amended so the two documents agree. Reviewed four times by |
| GPT-6 via Codex: revision 1's 20 findings are in |
| `CODEX-FINDINGS-RELAY.md`, revision 2's 12 in |
| `CODEX-FINDINGS-RELAY-R2.md`, revision 3's 8 in |
| -`CODEX-FINDINGS-RELAY-R3.md`, unedited; every one is taken, marked |
| -**(codex R1-N)**, **(codex R2-N)** or **(codex R3-N)**. Stage 4a of the media roadmap |
| +`CODEX-FINDINGS-RELAY-R3.md`, revision 4's 3 in |
| +`CODEX-FINDINGS-RELAY-R4.md`, unedited; every one is taken, marked |
| +**(codex R1-N)** through **(codex R4-N)**. Stage 4a of the media roadmap |
| (`2026-09-07-media-platform-roadmap-design.md`), built under rulings 8, |
| 10 and 11 there. |
| @@ -409,14 +410,23 @@ ours. **(codex R1-20)** |
| reconcile converges everything.** An operation writes its intent to |
| `relay.json` first (`op` bumped, `phase` set), then does the config |
| write, then the aggregate publish, advancing `phase` after each step |
| -with a CAS that carries the `op`. Two fences against a superseded |
| -writer: first, the console serialises every relay operation for one |
| -app, handler or reconcile, behind a per-app lock in its process, so |
| -two operations on one app never interleave their steps in the common |
| -case; second, because the config write is a separate object the lock |
| -cannot cover across processes, the **reconcile** does not only finish |
| -unfinished records, it **converges both derived artefacts from every |
| -record, every five minutes and at console start**: for each enabled |
| +with a CAS that carries the `op`. **One writer, enforced.** Every relay |
| +operation, handler or reconcile, runs only in a console process that |
| +holds the relay-writer lease, `control/relay-writer.json`: a CAS-written |
| +record naming the holder with a sixty-second expiry, renewed every |
| +twenty seconds, taken over by another process only after it lapses. A |
| +process that fails a renewal stops every relay write in flight before |
| +its next bucket write and abandons the operation, which the next |
| +holder's reconcile finishes. Inside the holder, operations on one app |
| +are serialised behind a per-app lock. So two console processes, |
| +during a deploy overlap or otherwise, never both write relay config, |
| +and a superseded key selection cannot reach config after a migration: |
| +the writer that selected it either holds the lease, in which case it |
| +is the only writer and no migration happened under it, or lost it and |
| +stopped. **(codex R3-2, R4-1)** On top of that fence, the **reconcile** |
| +does not only finish unfinished records, it **converges both derived |
| +artefacts from every record, every five minutes and at console |
| +start**: for each enabled |
| record it derives the expected `CARLOS_RELAY` value from the record and |
| the active masters and re-applies it if the config bundle holds |
| anything else; for each disabled record it removes the entry if |
| @@ -485,10 +495,14 @@ calls for, writes an additive acknowledgement |
| `control/config-ack/<box>/<route host>.json`: the route's shape (unit, |
| socket exec, TCP exec with file delivery), the time, whether a restart |
| happened and succeeded, and **the relay key identities it rendered**: |
| -the `kid` per region parsed out of the `CARLOS_RELAY` value it just |
| -wrote, or none. The ack is keyed on the route, not the app, because an |
| +the `username_suffix` per region parsed out of the `CARLOS_RELAY` value |
| +it just wrote (account, app, incarnation and kid together), or none. A |
| +kid alone would not do: a disable and re-enable between two host syncs |
| +changes the incarnation and the key while keeping the kid, and an ack |
| +naming only kids would report the old credentials as adopted. |
| +**(codex R4-2)** The ack is keyed on the route, not the app, because an |
| app can have several routes on one box and one can adopt while another |
| -does not; and it names the kids rather than a generation string, |
| +does not; and it names identities rather than a generation string, |
| because the app-wide generation object is a change notification that |
| can coalesce or fail after the manifest write |
| (`internal/appconfig/generation.go:28`) and is not an identity of what |
| @@ -498,9 +512,10 @@ the platform has, and the plan adds it to `internal/appconfig/sync.go` |
| as its own small step. **(codex R1-14, R2-6, R3-4)** |
| **`carlos relay status`** reports, separately: desired state from |
| -`relay.json` with its `op` and `phase`; the kids the console last |
| -issued per region; delivery per route, "adopted at <time>, restarted" |
| -when a config-ack names the issued kids and a successful restart, |
| +`relay.json` with its `op` and `phase`; the identities (username |
| +suffixes) the console last issued per region; delivery per route, |
| +"adopted at <time>, restarted" when a config-ack names exactly the |
| +issued identities and a successful restart, |
| "written, awaiting delivery" when no ack names them yet, "file |
| delivered, not restarted by the platform" for a TCP exec route with |
| file delivery (the app must re-read), "restart failed" when the ack |
| @@ -540,15 +555,20 @@ R1-10, R2-4)** |
| the earliest permitted retirement time for the old `kid`: 28 hours |
| from now. |
| 5. `carlos relay status --region r` shows, per route of every enabled |
| - app, which kid its config-ack names, and lists the routes that |
| - cannot be observed (TCP-address exec routes with file delivery, |
| - which must be restarted by hand, and routes with no delivery). The |
| - old master's `retire_at` may be set to the later of: 28 hours after |
| - rotation, and four hours after the last observed route acked the |
| - new kid with every unobservable route restarted by hand. Setting it |
| - earlier is refused by nothing but the docs, and ends any allocation |
| - still on the old `kid`, which is the force-retirement case §2 |
| - excludes from its guarantee. **(codex R3-4)** |
| + app, which identities its config-ack names and when, and for the |
| + routes the sync cannot observe adopting (TCP-address exec routes |
| + with file delivery, and routes with no delivery) the time of their |
| + last platform-driven restart through `carlos restart`, which the |
| + activator records, since a process on such a route holds the old |
| + key until it is restarted. **Each route's cutover** is its ack time |
| + or, for an unobservable route, its restart time after the file was |
| + delivered. The old master's `retire_at` may be set no earlier than |
| + `max(rotation + 28h, latest cutover + 4h)` across every route of |
| + every enabled app in the region, and `status` prints that time once |
| + every route has one. Setting it earlier is refused by nothing but |
| + the docs, and ends any allocation still on the old `kid`, which is |
| + the force-retirement case §2 excludes from its guarantee. **(codex |
| + R3-4, R4-3)** |
| 6. After `retire_at` the operator removes the line from the boxes and |
| the console secret at leisure. |
| @@ -616,9 +636,13 @@ unfinished ones, publishes without the app within five minutes. |
| stale config write landing after a disable and undone by the next |
| reconcile, a deletion that removed the record without publishing |
| repaired by the next rebuild, an enable during a staged-but-unmoved |
| - rotation issuing from the old kid, and `rotate` refusing until the |
| - fixture heartbeats list the kid; the config sync writing a per-route |
| - ack naming the kids it rendered. |
| + rotation issuing from the old kid, a process that lost the writer |
| + lease making no further relay write, a delayed old-key publisher |
| + refused after a migration, a disable and re-enable between two host |
| + syncs not reported as adopted by the old ack, and `rotate` refusing |
| + until the fixture heartbeats list the kid; the config sync writing a |
| + per-route ack naming the identities it rendered; the retirement time |
| + computed with a manual cutover after hour 28. |
| - `docs/site/cli.md` gains `### carlos relay` with a fenced example, or |
| the docsite gate fails. |
| - Flagship gate, as amended in the roadmap: a Meet call between two |
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-RELAY-R4.md b/docs/superpowers/specs/CODEX-FINDINGS-RELAY-R4.md| new file mode 100644 |
| index 000000000..f548a83bd |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-RELAY-R4.md |
| @@ -0,0 +1,61 @@ |
| +# Findings on the relay spec, revision 4 |
| + |
| +Reviewed by **GPT-6 (Codex), 2026-09-07**, fourth pass, against revision 4 at `ee4b78a`, with Paul's rulings treated as fixed. Unedited apart from this note. Revision 5 records how each finding was taken. |
| + |
| +| Revision-3 finding | Disposition | Reason | |
| +|---|---|---| |
| +| R3-1 — Interrupted deletion | Closed | Unconditional aggregate rebuilds recover deletion even after its source record disappears. | |
| +| R3-2 — Stale config publication | Partial | The lock prevents in-process interleaving; cross-process repair still permits old-key delivery after the rotation cutoff. | |
| +| R3-3 — Allocation cleanup | Closed | Wrapper-owned cleanup has the identity needed to preserve a replacement binding. | |
| +| R3-4 — Config adoption and retirement | Partial | Per-route acknowledgements improve granularity, but omit incarnation; the retirement formula also omits the manual cutover time. | |
| +| R3-5 — Premature key issuance | Closed | Every derivation path uses the gated issuer pointer; staging a master alone issues nothing. | |
| +| R3-6 — Rolling-window recovery | Closed | Recovery explicitly reads every hourly object intersecting the window. | |
| +| R3-7 — Memory-pressure behavior | Closed | The OOM claim is narrowed and host-pressure shedding has an explicit trigger. | |
| +| R3-8 — Roadmap contradictions | Closed | The amended passages agree with the fixed rulings and stage contract. | |
| + |
| +Reviewed commit `ee4b78ab3956800448d4df47d87749340133b6b4`, treating Paul’s six rulings as fixed. The cryptographic vector reproduces. Pion v4.1.4’s [documented API](https://pkg.go.dev/github.com/pion/turn/v4) and local source were checked. No files were changed; no live TURN or EC2 tests were run. |
| + |
| +## R4-1 — P2 — Eventual config repair cannot establish the rotation cutoff |
| + |
| +**Sections:** §2, credential lifetime; §5, reconciliation and rotation. |
| + |
| +The revised mechanism explicitly permits stale config writes across console processes and repairs them within five minutes. That bounds config inconsistency, but does not preserve the promised lifetime of credentials minted during it. |
| + |
| +A permitted sequence is: |
| + |
| +1. An operation reads the old issuer and derives its config, then pauses. |
| +2. Another console process rotates, delivers the new key, and obtains every required adoption acknowledgement. |
| +3. The old operation resumes shortly before the scheduled retirement and publishes its old-key config. A route adopts it and mints a four-hour credential. |
| +4. Retirement ends that credential’s allocation early. Subsequent reconciliation cannot restore the interrupted call. |
| + |
| +This requires a delayed writer, not force-retirement by the operator: the operator can follow the stated retirement procedure throughout. |
| + |
| +**Evidence:** [The reconciliation mechanism](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:408) expressly allows cross-process stale publication. [Rotation](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:536) treats migration acknowledgements as establishing when old issuance has stopped. |
| + |
| +**Correction:** Require a publication fence that prevents superseded key selections from reaching config after migration, or establish an enforceable single-writer model covering takeover between console processes. Keep reconciliation for recovery, but do not use eventual repair as the fence. Test a delayed old-key publisher resuming after the migration acknowledgements. |
| + |
| +## R4-2 — P2 — Acknowledgements cannot distinguish credentials before and after re-enable |
| + |
| +**Sections:** §2, incarnation and key changes; §5, config adoption and status. |
| + |
| +The acknowledgement records only the `kid` per region. A disable followed by re-enable changes the incarnation and derived key while retaining the same regional master and `kid`. |
| + |
| +If both operations finish before the host’s next config sync, its previous successful acknowledgement still names all the newly issued kids. The prescribed status comparison therefore reports “adopted” while the process holds the previous incarnation’s credentials, which the relay rejects. |
| + |
| +**Evidence:** [The re-enable transition](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:450) creates a new incarnation. [The acknowledgement and status rules](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:482) identify and compare credentials using kids alone. |
| + |
| +**Correction:** Record and compare the complete derived-key identity, including account, app, incarnation, region and kid. The delivered `username_suffix` already supplies most of it. Test disable/re-enable between host syncs: the previous acknowledgement must not establish adoption. |
| + |
| +## R4-3 — P2 — Retirement does not explicitly allow four hours after manual cutover |
| + |
| +**Sections:** §2, credential lifetime; §5, rotation step 5. |
| + |
| +The retirement formula uses the later of 28 hours after rotation and four hours after the last **observed route acknowledgement**, with unobservable routes restarted by hand. It does not include those manual restart times in the four-hour calculation. |
| + |
| +Suppose observed routes acknowledge at hour 1, while a TCP exec route continues minting from its old in-memory key until its manual restart at hour 28. The written formula permits retirement at hour 28. A credential minted just before that restart can consequently lose almost its entire four-hour lifetime. |
| + |
| +**Evidence:** [Step 5’s formula](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:542) names only the observed acknowledgement as the grace-period clock, whereas [§2’s guarantee](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/media-roadmap-spec/docs/superpowers/specs/2026-09-07-relay-stun-turn-design.md:121) requires four hours after the last instance stops minting from the old key. |
| + |
| +**Correction:** Define retirement as no earlier than `max(rotation time + 28h, latest issuer cutover + 4h)`, where the latest cutover includes manually verified adoption. Restarting an unobservable route must establish that it actually loaded the new key. Include a manual cutover after hour 28 in the rotation test. |
| + |
| +**Verdict: request changes.** No P1 remains. Three P2 defects remain in publication fencing, adoption identity and the retirement rule. |
| \ No newline at end of file |