carlos / platform Public

Not ready for Phase 1 planning. The spec addresses both confirmed outage causes, but its rollback and crash-recovery protocol still has paths to two running instances, discarded target writes, and unsafe deletion.

I reviewed e41c4fef against this worktree and the sibling spec at ef0b7e45. This was a read-only review with failure traces against the code; I did not run production experiments or change files. M1–M3 remain fixed.

1. P1 — A landing deadline does not prove the target stopped. §7.6; INV-1; I-3. The target starts its probe process, then its worker stalls. The source reaches the deadline, flips the record back, and starts another process. Losing the target’s eventual commit CAS prevents opening its edge; it does not stop its already-running child. The leases are bucket/prefix scoped, so the source and target can hold different leases simultaneously: backend_exec.go:228. Fix: require target stop evidence before restarting the source, or introduce a cross-region execution fence enforced independently of the stalled worker. When termination cannot be proved, keep the source fenced and report a recoverable failure. A timeout alone cannot establish INV-1.

1. P1 — “No public traffic” does not establish “no target writes.” §0.2, §4 INV-3/4, §7.5 L5, §7.6. L5 runs the real application with writable DB, files, credentials and outbound access: backend_exec.go:912. A boot worker can consume a queue message, acknowledge it externally, and record completion locally before health fails. Restoring the source generation discards that completion. CARLOS_MOVE_PROBE=1 is optional cooperation, not enforcement. Fix: define and enforce a probe contract that prevents externally committed work, or preserve and transfer target changes before rollback. The claim that rollback never loses target writes is false as written. The simulation’s edge-client-only write model would miss this.

1. P1 — Rollback opens the source before checking its files or health. §7.6 steps 1–5; §20.1. Step 1 sets ready and lifts the fence; step 2 may discover damaged files and declare stuck. Requests and ticks can already have started the application. Reconcile’s proposed “landing for landing, ready otherwise” also makes a re-created stuck row serve. Current serving gates confirm that ready is the admission decision: edge.go:595, activator.go:2785. Fix: keep rolling_back and stuck durably closed. Check files, render config, restore and probe under that hold; commit recovery before opening.

1. P1 — A failure after stopping the source but before flipping can strand it indefinitely. §7.3–7.4, §7.7–7.8; INV-7. A final archive can exceed the cap, contain a newly created special file, or fail to upload. There is no legal recovery transition after cut_at, and the landing deadline does not exist yet. On restart, step 1 can also reject an expired readiness report after the route is already moving. Today’s worker simply retries failures: movespass.go:415, movespass.go:468. Fix: add bounded pre-flip recovery to the source, including after local DB purge. Persist the cut’s phase before its side effects; make the cut_at admission decision atomic with abort eligibility.

1. P1 — Replaying landing can combine an old key directory with a newer database. §7.5 L2/L5, §7.8; I-1/I-6. After L5 changes both files and DB, a crash causes L2 to extract the original files again. Under #199 §2.5, the DB may retain a matching .lineage, or restore target-side changes from its generation. A key rotation can therefore leave old keys beside newly encrypted rows. The current warm-file shortcut is at backend_exec.go:264. Re-running the proposed staging-to-instances mirror is also destructive: it deletes destination-only objects, mirror.go:222. Fix: journal landing stages durably. Install DB/files/timers as one consistent recovery set; never re-extract or re-copy over a generation after its probe has started writing. Use unique retirement destinations for repeated attempts.

1. P1 — Clearing move before finalization loses recovery information and releases concurrency protection too early. §7.5 L6–L7, §10, §12; I-3/I-7. After L6, another move or push can be accepted while the old worker still owes L7. Its delayed ready write can reopen a newer operation’s hold. A crash immediately after L6 also loses the record’s move ID, source pointer and timings before the terminal event and cleanup task are durable. Current exclusion depends on rec.Move != nil: provision.go:966. Fix: retain a durable committed outcome/finalization phase. Make opening conditional on the exact incarnation, generation and operation; preserve enough information to reconstruct the ledger, log and cleanup task after a crash.

1. P1 — The #199 rollback floor does not protect the new landing fence. §20.3, §21; I-12. Release N understands DB, but need not understand landing. Rolling a target back to N after readiness permits its reconciler to mint a ready route before extraction or probing. Current minting unconditionally sets ready: reconcile.go:364. Fix: establish a minimum release for the move protocol itself before enabling it, covering source, target and console. Test downgrade and restart during every new state, not just an old target at request time.

1. P1 — P5 can delete a live generation, including without a race. §10; I-11. A legacy source’s instances/<host>/ is an ancestor of the proposed v2 instances/<host>/<incarnation>/<gen>/. Move away, return before the hold expires, then run legacy cleanup: the condition “record has a DB pointer” passes, but a recursive delete removes the returned instance’s live generation. The deletion helper recursively deletes every listed object: mirror.go:354. Separately, a v2 “not current” read can race #199’s in-region db rollback to that generation. Fix: enumerate legacy-owned objects without descending into v2 namespaces. Introduce generation retirement/tombstones that atomically exclude future pointer selection before deletion; a fire-time read alone is insufficient.

1. P1 — P5 does not deliver the stated residency cleanup. §10; INV-8; I-11. Data buckets are versioned: deployment-buckets/main.tf:28. The existing delete sends no version ID: mirror.go:493. It leaves previous bytes behind, and the box policy lacks version-list/delete permissions. P5 also names only from_gen, leaving earlier retained generations and potentially retired directories in the source region. Fix: define the complete incarnation-scoped residue inventory, including object versions, delete markers, earlier generations and retired directories. Add scoped cleanup authority and verify actual removal before reporting residency complete.

1. P1 — The deletion warrant does not bind what will be deleted. §10, §14. Signing (move id, host, from, not before) omits account, app, incarnation, storage root, generation and exact resource coordinates. The referenced completion logs are writable by every region-host role: region-host/main.tf:166. A legitimate ledger ID plus forged completion/task data can therefore authorize a different deletion than the console intended. Fix: retain the immutable move scope in the console ledger and sign the exact deletion scope. Do not derive privileged coordinates or completion solely from box-writable records/logs. Apply the same binding to store-copy credential issuance.

1. P1 — A member can bypass the proposed owner gate by minting an instance token. §14–15; Q10. The existing mint checks that the caller is a person and an app member, then creates the token; it does not require ownership: apiinstances.go:1554. Those tokens retain move request/cut/abort authority under authcli.go:625. Fix: make move-capable service authority explicitly owner-delegated, including the legacy mint path. Specify treatment of existing member-minted tokens while preserving approved router credentials.

1. P1 — Private-store cutover and automatic instance rollback have incompatible commit points. §11, §7.4, §7.6. The private store is rebound before the instance flips, but store recovery becomes forward-only after rebind. If target startup fails because it cannot use that twin, the source comes back with the same changed bundle and can fail too, despite its healthy original store remaining available. Store coordinates are indeed delivered through the shared environment bundle: walk.go:825. Fix: specify a joint instance/store transaction with source and target binding versions, health checks and compensation. Either delay authoritative store cutover until landing commits, or define a write-safe reverse transfer. A region-and-DB flip alone cannot implement this rollback.

1. P2 — Existing config, adoption and retirement helpers cannot implement landing unchanged. §7.3 T7, §7.5 L3/L5. SyncExec excludes landing rows: syncexec.go:643. An unchanged file produces no fresh ack: syncexec.go:1159. Release adoption also excludes them: adopt.go:816. Finally, an edge reload retires ordinary activator entries absent from its ready-route set: edge.go:924. Fix: specify explicit held-route render/materialization APIs and move-bound acknowledgements, plus probe-process ownership that survives ordinary edge reloads without admitting traffic.

1. P2 — Readiness checks do not check the environment or process that will actually run. §7.3 T3/T4, §7.5 L3–L5. <host>.env overrides rendered config at spawn: backend_exec.go:1135. Thus T4 can validate new store credentials while the application uses carried old ones. HeadBucket does not prove object reads/writes. L5 accepts any response below 500, including an unrelated 404 or another process answering the socket: backend_exec.go:1426, backend_exec.go:1010. Fix: validate the final merged launch environment, report conflicting overrides, check layout-specific startup prerequisites, and bind health to the spawned process and an explicit application readiness contract.

1. P2 — Timer carriage is neither frozen nor restored on rollback. §5 S11/S12, §7.4 step 7, §7.6, §8.3. Blocking delivery does not freeze timer bookkeeping: failed deliveries still advance attempts/state in runner.go:345. Once the source route retires, the converger deletes its timer rows: converge.go:114. Rollback never imports the saved rows, and the source cannot read the target archive. Ordinary UpsertTimer also resets registration/attempt state rather than restoring an exact snapshot: timers.go:45. Fix: quiesce scheduler bookkeeping, retain a source-readable snapshot, and implement transactional snapshot replacement for landing and rollback. Remove target-only stale timers and preserve registration identity where required.

1. P2 — The state inventory misses non-reconstructible registry state. §5 S13. routes.replication is a box-local setting explicitly absent from instance records, and routes.next_due is separate from the timers table: registry.go:138. Re-minting does not reproduce either. Caps is also box-local authority and requires an explicit refusal or independently authorized target grant, not implicit carriage. Fix: classify every route column as carried, reproduced, reset or refused. Carry replication policy; carry or explicitly recompute next_due before the instance can sleep. The S1–S25 list is therefore not exhaustive as a state contract.

1. P2 — Store rebind omits the authoritative store record and existing lifecycle operations. §11. The specified step changes config only. Rotation later obtains bucket, region and IAM user from Record.Provisioned, then rewrites config using those coordinates: walk.go:427, walk.go:448. An unchanged record can send the application back to the old, fenced bucket. Scanning/enforcement and credential cleanup also need an explicit transfer contract. Fix: journal the store-record transition with config delivery; serialize grant/rotate/revoke/purge against the move. Preserve security policy and define which principal deletes the fenced old bucket.

1. P2 — App-move partial-failure recovery lacks durable child identity and a complete store-consumer hold. §12.1–12.2. A child can finish between its record commit and the sequencer checkpoint; retrying it as a new move then fails “already in target,” provision.go:971. New instances are expressly allowed during sequencing, while skipped and explicitly placed instances can still use the app store. The store cut needs to cover all of them, including restart recovery and unabortable children. Fix: persist stable child move IDs before dispatch and recover their durable outcomes. Define an app/store cut barrier covering existing consumers and concurrent creation, and specify how pause, abort and “move back” transition the existing app operation.

1. P2 — The archive’s replay contract contradicts its encryption format. §7.4, §8.1–8.2; I-8. Deterministic tar bytes do not produce deterministic sealed bytes with a fresh data key and ephemeral key. Existing sealing is randomized: crypto.go:86. Rebuilding after a crash changes files.digest. I-8 also clears the source manifest at a terminal state while §10 says it remains through the hold. Fix: distinguish plaintext-content identity from ciphertext identity; persist and reuse an immutable sealed artifact for a cut attempt. Define publication order and manifest retention explicitly. Do not make encryption deterministic by reusing key/nonce combinations.

1. P3 — The incident cause is still described as unconfirmed. §1.2b, §22 Phase 1. The supplied journal evidence settles this: UK, US and AU lack CARLOS_SECRETS_KEY; only Ireland has it. The code’s disabled-loop behavior matches that finding: agent.go:881. Fix: record the confirmed cause and replace the investigation task with deployment remediation and verification.

I-1 verdict: copy the landed generation into instances/. The moves spec is right on the IAM boundary.

| Actor | Target moves/* | Target instances/* | |---|---|---| | Target box | Read, write, delete through its own-bucket grant | Read, write, delete | | Sibling whose sibling_staging_arns includes this bucket | Prefix-scoped list; write and delete; no object read | No grant from this mechanism |

Those permissions are explicit at region-host/main.tf:166 and region-host/main.tf:217. A unique generation prevents accidental name reuse; it does not revoke sibling write/delete authority. Under #199’s Root: "moves", that authority remains over the live replica indefinitely.

The target can perform the in-bucket copy using existing permissions. However, make it a durably completed, verified landing step before any probe writes. Never re-mirror staging over that writable generation. The pointer may name instances/ during landing only if every reader fails closed until installation completes. This preserves the intended boundary without claiming to solve the broader #171 record-forgery problem.

The remaining §26 interfaces need these dispositions:

| Interface | Assessment | |---|---| | I-2, I-4 | Sound direction: shared key derivation and unique generations. Pin the generation grammar and distinguish pointerless v2 g0 from legacy storage. | | I-3, I-7 | Atomic region/DB writes are correct, but exclusion must extend through finalization; CAS does not fence a running process. | | I-5 | Unresolved: the existing RecordIdentity rejects zero timestamps at types.go:62. Refuse these moves until a defined backfill, or define a shared legacy identity algorithm. | | I-6, I-9, I-10 | Separating incarnation-owned files from generation-owned DBs is correct. Both specs must adopt the same held statuses, replay rules and scope checks. Retirement must preserve rollback files and avoid reminting outside the box’s scope. | | I-8 | Retain manifests/snapshots for their actual recovery lifetime; bind them to the move, not merely a reusable source generation. | | I-11 | Needs the deletion/rollback exclusion and complete residue inventory described above. | | I-12 | Require the epoch and a move-protocol release floor. | | I-13 | Sharing a listing is fine; fresh reads still do not make destructive operations atomic. | | I-14, I-15 | Agree with blocking transitional pulls and retiring v2 “staging wins”; also forbid empty-generation seeding before landing installation is proven complete. |

The simulation needs independent application/background work, surviving or stalled processes, scheduler bookkeeping, real config/adoption/retirement integration, versioned objects, concurrent generation selection and forged completion metadata. Treating those as successful atomic worker steps would leave several findings invisible.

Recommendations on §23:

| Question | Recommendation | |---|---| | Q1 | (a) Fixed paths initially, with the corrected state inventory and explicit eligibility checks. | | Q2 | (a) 256 MiB, enforced during streaming and again at the final cut. | | Q3 | (a) Auto-cut after validated readiness; retain manual API default and --manual-cut. | | Q4 | Seven days; permit owner-requested early deletion through the corrected scoped cleanup protocol. | | Q5 | (a) Pause and retain the target app region; make child recovery and reverse sequencing explicit. | | Q6 | (a) Hold all store consumers, with a durable barrier covering concurrent creates. | | Q7 | (a) Refuse until sidecar moves exist. | | Q8 | (a) Defer prewarming, but gate eligibility on measured restore/cut duration. | | Q9 | (a) Moveprobe, disposable ips, then kpmg with scoped approval. | | Q10 | (a) Owners only, closing the member-to-instance-token bypass. | | Q11 | (a) Require #199’s epoch; remove the pre-epoch landing path. | | Q12 | (b) Copy into instances/, with durable, non-replaying installation. |

Verdict: not ready for planning (Phase 1). Resolve execution fencing, target-write preservation, held rollback, bounded pre-flip recovery, crash-safe landing/finalization, and the landing-helper integration first. These are protocol decisions a builder cannot safely infer from the current spec.