| 1 | # Review brief: `carlos db` reads for instances in another region |
| 2 | |
| 3 | You are an independent second reader of one design spec for the CARLOS |
| 4 | platform (this repository). Read it cold and adversarially. Nothing is built |
| 5 | yet; you are reviewing the design against the code on `main`. |
| 6 | |
| 7 | **Spec:** `docs/superpowers/specs/2026-10-22-db-region-forward-design.md`, |
| 8 | at the commit named in the prompt. Read `AGENTS.md` first. |
| 9 | |
| 10 | ## Round 5 |
| 11 | |
| 12 | This is round 5. Rev 5 of the spec answers your round 4 findings, |
| 13 | `docs/superpowers/specs/CODEX-FINDINGS-DB-REGION-FORWARD-R4.md` (reviewed |
| 14 | at `010d29f6`). Earlier rounds are in `CODEX-FINDINGS-DB-REGION-FORWARD.md`, |
| 15 | `-R2.md` and `-R3.md`. Round 4 found the five invariants J1 to J5 (§5.3) |
| 16 | fixed and the design converged. Rev 5 adds no mechanism; it makes four |
| 17 | direct fixes, each marked **(codex N)**: |
| 18 | |
| 19 | | # | Finding | Round 4 said | Rev 5's answer | |
| 20 | |---|---|---|---| |
| 21 | | 7 | stored-ciphertext wording | partly, P3 | the 64 MiB plaintext limit and the stored ciphertext are now separate: raw sealed envelopes, under 100 bytes of overhead per 256 KiB chunk, base64 only on the wire, §5.7 | |
| 22 | | 8 | restart recovery for disk reservations | partly, P2 | reconciliation by refusal against the units actually running, never a guessed size; and a worker that loses its connection exits by itself within 5 s, because litestream's file client and the LTX compactor ignore cancellation, §5.7, §8.3 | |
| 23 | | 19 | progress writes not guaranteed to leave the console | new, P2 | every line flushed with `ResponseController.Flush` under its write deadline; `admitted` flushed with the headers; `application/x-ndjson` is not gzipped by the edge; work before the `200` bounded at 20 s for backhaul's 30 s header timeout, §5.5 step 6 | |
| 24 | | 20 | the result can overtake the final chunk | new, P2 | result read before the listing, as the run's follow door does; the result line written only after every chunk through `last_seq`, §5.5 step 6 | |
| 25 | |
| 26 | Findings 1 to 6 and 9 to 18 were fixed or declined-agreed in round 4 and |
| 27 | are unchanged. |
| 28 | |
| 29 | **Your answer has two parts, in this order.** |
| 30 | |
| 31 | 1. **A disposition table** for findings 1 to 20: fixed, declined with an |
| 32 | acceptable reason, or not fixed. Check rev 5's four answers against the |
| 33 | code, not against the spec's summary of it. |
| 34 | 2. **Only new findings of severity P0, P1 or P2.** Leave out P3s this |
| 35 | round. Do not repeat an earlier finding unless its fix is wrong or |
| 36 | incomplete; then say so in the table and give the new evidence. |
| 37 | |
| 38 | ## Fixed rulings: do not re-argue them |
| 39 | |
| 40 | The product owner approved this direction: |
| 41 | |
| 42 | - the console forwards a db read to a box in the region where the data |
| 43 | lives; |
| 44 | - that box reads its own region's replica, runs the read-only operation |
| 45 | there, and returns only the result; |
| 46 | - the console never reads another region's bucket; |
| 47 | - there is no console per region. |
| 48 | |
| 49 | The `carlos run` spec's ruling also holds: the console never connects to a |
| 50 | box, and output travels as objects in the deployment bucket. |
| 51 | |
| 52 | Do not propose alternatives to these (a console per region, an IAM grant |
| 53 | that lets the console read other regions, a direct console-to-box |
| 54 | connection). Report only: |
| 55 | |
| 56 | - a mechanism that **fails to honour** a ruling; |
| 57 | - a mechanism that breaks something else in the platform; |
| 58 | - a mechanism that cannot be built as described against `main`. |
| 59 | |
| 60 | The spec's own decisions (§4, D1 to D8) are open to challenge. The |
| 61 | provisional rulings in §16 are for the product owner; comment on them only |
| 62 | where the spec's mechanism fails to honour one or is unsafe. |
| 63 | |
| 64 | ## What to check |
| 65 | |
| 66 | Check against the code, not the spec's description of it. Cite `file:line` |
| 67 | for every claim about existing code. These are the places that matter. |
| 68 | |
| 69 | **The db door and the CLI:** |
| 70 | |
| 71 | - `internal/console/apidb.go`: `resolveReplica`, `dbGate`, |
| 72 | `resolveDBSource`, `dbClassRefusal`, `listReplicaLevel`, the stream bounds; |
| 73 | - `internal/console/apidbclaim.go`: why the claim door stays refused; |
| 74 | - `db.go`: `runQueryContext`, `checkQuerySQL`, `dbRestoreError`, |
| 75 | `dbQueryStatus`; |
| 76 | - `internal/dbdoor/`: `plan.go` (`PlanRestore`), `restore.go` |
| 77 | (`RestoreAsOf`, the `--at` path), `client.go`; |
| 78 | - `internal/lease/endpoint.go`: `ReadReplicaClient`, the root-credential |
| 79 | branch of `newReplicaClientForUID`. |
| 80 | |
| 81 | **The transport the spec copies:** |
| 82 | |
| 83 | - `internal/runs/runs.go`, `store.go`; `internal/console/apiruns.go`, |
| 84 | `apirunsfollow.go`, `runsweep.go`; `internal/host/runs.go`; |
| 85 | - `internal/release/rootenvelope.go` and `hostretry.go`: the signed-request |
| 86 | precedent; |
| 87 | - `internal/appconfig/crypto.go`: the sealing construction the spec reuses. |
| 88 | |
| 89 | **The long request (round 4):** |
| 90 | |
| 91 | - `internal/edge/edge.go`: the public listeners (around lines 3188 and |
| 92 | 3354) and `unixProxy` and `tcpProxy` (around line 2377); |
| 93 | - `cmd/console/main.go`: the console's `http.Server` (around line 586); |
| 94 | - `internal/console/apiinstanceimport.go`: the per-request read deadline |
| 95 | (around line 479); |
| 96 | - `auth.go` (the CLI's 30 s console client) and `apimode.go` (`shipTimeout`); |
| 97 | - `internal/console/apidb.go`: `handleAPIDBGet` and `dbStream`, the |
| 98 | streaming precedent. |
| 99 | |
| 100 | **Boxes, capability and sandbox:** |
| 101 | |
| 102 | - `internal/host/edgesfile.go` (capabilities), `movesoffline.go` and |
| 103 | `movesoffline_stage.go` (how a capability is tied to an armed worker); |
| 104 | - `internal/edgesfile/snapshot.go`: `ReadLiveSnapshot`, `Draining`, `Fresh`; |
| 105 | - `internal/host/claimunit_template.go`, `bootstrap.go` (`tmpfilesConf`, |
| 106 | socket ownership), and wherever `ensureSystemUnits` installs units; |
| 107 | - `internal/activator/backend_unit.go`: how `carlos-app@` units and their |
| 108 | sockets are started without root. |
| 109 | |
| 110 | **IAM and infrastructure:** |
| 111 | |
| 112 | - `infra/modules/region-host/main.tf`: `host_buckets`; |
| 113 | - carlosframework/carloku-infrastructure `region-uk.tf` and `region-ie.tf` |
| 114 | (on GitHub): which bucket each box role reaches, and the module refs. |
| 115 | |
| 116 | **Census:** |
| 117 | |
| 118 | - `internal/provision/plan14_root_reads_test.go`, |
| 119 | `plan9_b8_census_test.go`, `moves_offline_census_test.go`; |
| 120 | - `docs/superpowers/notes/census-proposal-moves-offline-precopy.md` for how |
| 121 | an overlay is proposed and ruled. |
| 122 | |
| 123 | ## Questions worth your time |
| 124 | |
| 125 | 1. **Isolation.** Can the member's SQL, or a crafted replica, reach anything |
| 126 | but its own copy? Check the worker unit in §8.3 option by option against |
| 127 | what `DynamicUser`, `PrivateNetwork`, `TemporaryFileSystem`, |
| 128 | `PrivateTmp` and `Accept=yes` actually do. Check every SQLite path that |
| 129 | opens a file (ATTACH, `VACUUM INTO`, extensions, temp files, `immutable`) |
| 130 | with modernc.org/sqlite v1.55.0. Can the worker reach the metadata |
| 131 | service, the box role, `/data`, `/etc/carlos` or another tenant's files? |
| 132 | 2. **The prefix.** Is there any path where a request field, rather than the |
| 133 | fresh record and the box's own config, picks the bucket, prefix, host or |
| 134 | level? Is the incarnation check placed so a host recreated between |
| 135 | request and read cannot be read under the old request? |
| 136 | 3. **Forgery and replay.** Is the root envelope bound tightly enough |
| 137 | (purpose, id, region, issued_at)? Can any other root-signed payload be |
| 138 | read as a db-read request, or the reverse? Does the seen set plus the |
| 139 | 60 s window close a second run of one request? Does sealing really make |
| 140 | a replay harmless? |
| 141 | 4. **Election and expiry.** Can two boxes both run one read? Can the |
| 142 | console's expiry and a late box both believe they own `picked`? Can a |
| 143 | result land before its last chunk, or a chunk after the result? |
| 144 | 5. **Cleanup and disk.** On every exit path (done, failed, cancel, CLI gone, |
| 145 | agent restart, `kill -9`, box reboot, console restart), what is left in |
| 146 | the deployment bucket and on the box? Is the disk floor enough on a 60 GiB |
| 147 | box that also stages versions? Can the unacked window deadlock? |
| 148 | 6. **Parity.** Will the forwarded `--at` stop at the same transaction as the |
| 149 | in-region door for the same replica and time, given the second |
| 150 | truncation of `LastModified`? Will the forwarded output be byte-identical |
| 151 | to the in-region output across CLI and box versions? |
| 152 | 7. **Mixed fleet.** Walk §9.2 and §11.2. Can a box publish `db-read-1` |
| 153 | without an armed worker? What does a host-claim console, a console with |
| 154 | a scoped key, or a console with `CARLOS_DB_FORWARD` unset answer? |
| 155 | 8. **Census and build.** Are §13's classifications right for the root-read |
| 156 | scan as it is actually written (it matches `Open`, `Query`, |
| 157 | `QueryContext` and more across all production packages)? Does the worker |
| 158 | code cross-compile for darwin and windows as described? |
| 159 | 9. **Provider boundary.** Does anything in the design need AWS to work on a |
| 160 | MinIO deployment with one store per region? |
| 161 | 10. **Missing pieces.** Anything the spec relies on that does not exist, or |
| 162 | exists in a different shape. |
| 163 | |
| 164 | ## Output |
| 165 | |
| 166 | A numbered list of findings. Each one gives: |
| 167 | |
| 168 | - **severity:** P1 breaks a ruling, crosses a tenant, or exposes box |
| 169 | credentials; P2 is wrong behaviour; P3 is clarity or a gap; |
| 170 | - the spec section; |
| 171 | - the code evidence, with `file:line`; |
| 172 | - a concrete fix. |
| 173 | |
| 174 | Then a one-line verdict: accept, or revise. Do not rewrite the spec. No |
| 175 | praise. |
| 176 | |
| 177 | On every round after the first, follow the answer format in the round |
| 178 | section at the top. |
| 179 | |
amadan