carlos / platform Public

Reviewed HEAD 3b99911e087cbe6f417aa27a3dc9d6d6e32a5979. Five correctness findings and one rollout correction:

1. P1 — O2 accepts a key the app may never have used. Plan:139 The platform mints its key even when the app uses its own instance.key, which takes precedence. If that app-owned key never reaches the backup, the proposed instance.key OR CARLOS_INSTANCE_KEY check still accepts the unrelated platform key. Relocation can then restore sealed data without its actual key, bypassing the existing refusal. Require evidence of which key source protects the database; mere presence of a platform key is insufficient. Test a backed-up platform key alongside a missing app-owned key.

1. P1 — A failed durability step becomes success on retry. Plan:585 After the append succeeds, Chmod, file Sync, or directory Sync can fail. The next ensure sees the nonempty key and returns immediately, allowing a launch without completing the failed durability step. The claim early return at line 1005 bypasses it too. Preserve the bytes, but complete durability before reporting success. Add injected file-sync and directory-sync failures followed by retries, asserting that no child starts before successful synchronization.

1. P2 — Every nested backfill that needs to append fails with EINVAL. Plan:605 Backfill uses create=false, so the flags exclude O_CREATE, but OpenBeneath still receives mode 0600. That helper passes the mode directly to openat2, which rejects a nonzero mode without creation flags. I confirmed this kernel behavior with a read-only syscall check. Pass mode zero for an existing-file open, retaining the later fchmod. Add a successful nested backfill test; the current successful backfill test exercises only flat layout.

1. P2 — An empty instance override defeats the configured-key exception. Plan:591 With CARLOS_INSTANCE_KEY= in <host>.env and a nonempty key in <host>.config.env, the mint returns InstanceKeyFromConfig. However, mergeEnv preserves the empty instance override, so the child receives an empty key and an adopting app refuses to start. Resolve this using the effective merged environment and define how the empty override is handled. Test both files together through launch; the two separate empty-value tests miss this combination.

1. P2 — O2 rejects the configuration-backed keys that ruling 5 explicitly supports. Plan:139 An app using CARLOS_INSTANCE_KEY from configuration has neither instance.key nor a key in the backed-up <host>.env, because this plan deliberately mints nothing. With a nonempty database, O2 therefore refuses relocation despite the key being durable in configuration. Define verifiable recovery evidence for this supported source, including delivery of the same effective key on the target, and add a relocation test.

1. P3 — The rollout uses an unsupported merge option. Plan:1421 amadan branch merge -squash contradicts the repository’s explicit workflow: squash locally, push, describe the branch, check CI on the final tip, then merge with -expect <tip>.

The remaining review points:

• Concurrency: sharing adminTokenMu serializes the proposed token/key appends within the agent. I found no new lock recursion in the listed callers.
• Durability: legacy durability remains disk plus move archives; this does not provide off-box recovery after box loss. O1 is necessary for claim launches, but the capturing fake proves only ordering—not committed publication or launch refusal on publication failure. Those need provider integration tests.
• Tests: the snippets appear compatible with the inspected interfaces, but I did not independently compile or run them in this read-only workspace. Coverage also lacks a first flat seed, nested RoleSeed, actual hibernation restore, and the failure/retry cases above. A second Start does not exercise restore.
• Secrecy: I found no proposed production log/error that prints the key, or new console/CLI exposure. Move reports retain HMAC-only values.

For the five open questions:

• Replicas: may remain deferred only with the stated exclusion from platform-key adoption.
• Configuration: blocks the current design until findings 4 and 5 are resolved; reserving the name is not required.
• report.token: does not block this key-only scope.
• Workers/sidecars: does not block the explicit exec-instance scope.
• Claim-session restarts: the alleged bypass is not established at this HEAD. Claim retries use captured launch inputs and call Materialize before each attempt; production claim swaps refuse. Correct the premise and pin that behavior with a test. Any future path that mints before an asynchronous publish would require a synchronous publication gate.

---

FIRST PROMPT: Review the instance-key plan at HEAD using the supplied brief.

WHAT: Check whether the plan is ready to implement.

STATUS: Review complete; six findings. No files changed.

NEEDED: Nothing for this review.

RECOMMENDED: Resolve key-source evidence, durability retries, and the two backfill/configuration defects before implementation.

SESSION: Safe to close; no processes or scratch files were created.

Verdict: not ready to build.