carlos / platform Public

Reviewed at a12757a1. Spec references below refer to 2026-10-22-db-region-forward-design.md. This is a static review; sandbox acceptance tests were not run. Current flagship module refs and deployment-state claims remain unverified: GitHub access failed, and the available infrastructure checkout predates the design.

1. P1 — §8.2, lines 442–458: the incarnation check does not cover the replica reads.

The box checks the record, then plans and downloads from a hostname-only prefix. A deletion and recreation between those steps can make the old request read the replacement’s replica. GetRecord validates identity only when called (provision.go:2777); HibernatePrefix includes neither account nor incarnation (activator.go:147). Computing an incarnation does not bind subsequent object reads to it (claim.go:192).

Fix: bracket the entire download with fresh identity checks and release no output until the post-download check passes. Repeat those checks on every replan. For a stronger guarantee, pin object versions to the checked incarnation. Add recreation tests during planning and download, including transfer to another account.

1. P2 — §8.3, lines 465–491: Accept=yes does not put the connection on stdin.

The worker explicitly requires a socket on stdin, but the proposed unit omits StandardInput=socket. The referenced template supplies no such setting and directs stdout to an instance-specific log socket (claimunit_template.go:19). Socket activation otherwise uses passed file descriptors; stdin delivery requires explicit configuration. See https://www.freedesktop.org/software/systemd/man/257/systemd.socket.html.

Fix: specify the complete worker service contract: StandardInput=socket, the output channel, and separate stderr handling. Alternatively, consume the activation descriptor explicitly. Test this through the actual installed units.

1. P2 — §5.4, lines 238–245: progress acknowledgements are indistinguishable from final acknowledgement.

Both carry the last sequence number. The CLI can receive the last chunk before the result exists, then send a progress ack that arrives after the result is written. Treating that as final deletes the summary before the CLI fetches it. The copied run protocol has only terminal acknowledgements: it checks both the result sequence and terminal record state (apirunsfollow.go:287, apirunsfollow.go:312).

Fix: distinguish cumulative progress acknowledgements from explicit completion acknowledgements. Bind completion to the result’s identity or digest, and accept it only after the CLI has verified that result.

1. P2 — §§5.6–5.8, lines 309–335: a lost result does not stop a late writer.

A box can miss heartbeats for 90 seconds while an output PUT remains in flight. The sweep then publishes lost; the PUT can complete afterward, violating the terminal ordering and potentially recreating objects after cleanup. Create-only result publication arbitrates only result.json, not chunk writes (runs/store.go:63). The run precedent waits until an execution deadline plus its lost interval (runsweep.go:101); it does not establish the proposed heartbeat-based guarantee.

Fix: define writer ownership expiry, bounded in-flight writes, and a terminal transition that accounts for both. Delay cleanup until the previous writer cannot publish further objects. Test heartbeat failure while output writes continue or complete late.

1. P2 — §§5.8–5.9, lines 328–356: immediate cleanup reopens the election within the acceptance window.

Final acknowledgement deletes picked while the signed request may still be acceptable. Another box, or the same box after restarting, has no durable evidence that the request already ran. A process-local seen set cannot provide deployment-wide replay protection. The existing run transport couples execution to persistent record state rather than relying solely on object existence (host/runs.go:145). Sealing protects confidentiality but does not prevent duplicate execution or resource consumption.

Fix: retain a durable completion tombstone through the entire request-acceptance window, including clock skew and pending pickup attempts. Delete bulk output promptly, but remove the election guard only after replay is impossible.

1. P2 — §§5.4–5.5 and §10, lines 240–266 and 584: the CLI cannot promise that it showed none of a rejected answer.

Rows reach stdout before the sealed summary and complete-output hash are checked. A missing final chunk or bad summary therefore leaves a partial answer already consumed by a terminal or pipeline. The existing query writer emits headers and rows immediately (db.go:427, db.go:447); those writes cannot be withdrawn.

Fix: buffer the bounded result locally, acknowledge buffered chunks to keep the window moving, verify the summary, then publish stdout. Otherwise, explicitly specify partial-output semantics and change DBF-13.

1. P2 — §5.7, lines 317–324: the unacknowledged window does not bound stored ciphertext.

The protocol advances ack but never deletes acknowledged chunks until completion. A 64 MiB answer therefore leaves roughly 64 MiB of ciphertext in the bucket, despite the claimed 4 MiB bound. The run transport likewise retains output until whole-run deletion (runs/store.go:128); copying it supplies no incremental reclamation.

Fix: specify deletion of acknowledged chunks, with monotonic acknowledgement handling and retry-safe cleanup. Alternatively, state that the window bounds outstanding delivery only and remove the storage-bound claim, including its phase-2 counterpart.

1. P2 — §§5.7 and 8.3, lines 314–318 and 484–486: independent disk checks can spend the same free space twice.

Two permitted workers can each pass the post-plan check against the same free-space reading. On a 60 GiB filesystem with 12 GiB free, two 6 GiB scratch requirements each leave the required 6 GiB floor when checked separately, yet together exhaust the filesystem. PlanRestore calculates a single restore’s input and output sizes; it provides no reservation (plan.go:60, plan.go:89). A per-file LimitFSIZE cannot enforce the aggregate.

Fix: reserve scratch capacity atomically across workers before downloading, subtract outstanding reservations, and enforce download and aggregate scratch bounds. Release reservations on every exit and reconcile them after agent restart.

1. P2 — §7.1, lines 380–388: a socket inode does not prove that a worker can run.

The capability conditions pass when the socket exists but its service is missing, masked, incompatible, or unable to start. That admits requests which the advertised worker cannot execute. The cited offline capability comes from the actual armed worker (movesoffline.go:254); installation also probes activation state rather than trusting filesystem existence (systemunits.go:79).

Fix: tie publication to the active agent loop and a successful worker protocol/readiness check. Withdraw capability when that evidence fails. Test a present socket with a missing or broken service.

1. P2 — §7.1, lines 390–397: snapshot-load failure is not represented by the API’s error return.

ReadLiveSnapshot logs a status-load failure and returns the edges snapshot with a nil error (snapshot.go:137). Missing or unreadable status evidence is represented through BoxesComplete; it is distinct from known stale status (snapshot.go:202). An error-only fallback would return DBF-2 where the design requires admission and expiry.

Fix: explicitly handle Complete, BoxesComplete, and known freshness. Specify precedence for incomplete evidence before concluding that every capable box is offline, and test failed and partial status reads.

1. P2 — §12, lines 667–680: separate regional store servers need configuration the proposed clients do not support.

A box must reach both the shared deployment store and its regional replica store. Both existing constructors use the single process-wide CARLOS_S3_ENDPOINT: the deployment client through s3compat.Options (store.go:712), and the replica client through s3Endpoint (endpoint.go:146). The endpoint source is one environment variable (s3compat.go:40). Different bucket names or signing regions do not select different MinIO servers.

Fix: add independently configured deployment and regional-data client endpoints, with credentials where needed, and include that wiring in plan 1. Test separate MinIO servers, not merely two buckets on one server.

1. P2 — §13, lines 687–693: “non-root” is not a valid census disposition, and the extracted query runner remains CLI-reachable.

The scanner intentionally examines all production packages and matches Open and QueryContext regardless of the service’s runtime user (plan14rootreads_test.go:33). Its accepted classes are authority, hint, diagnostic, and legacy (plan14rootreads_test.go:478). The existing query runner is explicitly frozen as accepted legacy, J4 (plan14-root-reads.tsv:202); moving it to a shared package does not make every caller a DynamicUser.

Fix: propose supported dispositions for each new or moved declaration, account for both CLI and worker callers, retire the old declaration correctly, and update the source envelopes and legacy seal under the required ruling.

REVISE