amadan
Astra round 2 findings on the move-any-app spec (45b77691), unedited
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+150
−0
docs/superpowers/specs/CODEX-FINDINGS-MOVES-R2.md+150 −0
diff --git a/docs/superpowers/specs/CODEX-FINDINGS-MOVES-R2.md b/docs/superpowers/specs/CODEX-FINDINGS-MOVES-R2.md| new file mode 100644 |
| index 000000000..4183a4b4f |
| --- /dev/null |
| +++ b/docs/superpowers/specs/CODEX-FINDINGS-MOVES-R2.md |
| @@ -0,0 +1,150 @@ |
| +Reviewed `45b77691` against this worktree and #199 rev 3 at `66f1dad3`. Read-only review; no files changed or production tests run. M1–M3 stand. Follow-on phases are assessed only for acceptable deferral. |
| + |
| +| R1 finding | Disposition | Why | |
| +|---|---|---| |
| +| 1. Deadline is not stop evidence | **Partly** | Correct protocol rule; the existing stop implementation does not prove all writers stopped or reconstruct evidence after restart. | |
| +| 2. Probe writes lost on rollback | **Partly** | Reverse carriage is specified, but recovery itself lacks a durable install/probe/finalization journal. | |
| +| 3. Source opens before recovery checks | **Resolved** | Source recovery now probes under `restoring`; `stuck` stays closed. | |
| +| 4. Failure between stop and flip strands source | **Partly** | `reverting` exists, but it requires `source.files.plain`, which is not recorded until `archived`; stopping/staging failures can precede that baseline. | |
| +| 5. Landing replay mixes generations | **Partly** | Probe replay is constrained, but installation completion markers precede some installation work. | |
| +| 6. Clearing `move` loses finalization | **Partly** | Successful landing retains it; rollback clears it without equivalent recovery and target-cleanup completion stamps. | |
| +| 7. Protocol downgrade bypasses landing fence | **Resolved** | The move-specific floor covers boxes and console, with production activation ordered after floor publication. | |
| +| 8. P5 deletes live generations | **Deferred-by-scope; acceptable** | F3 states the required exclusion and ancestor rules. Phase 1 parking still needs the fixes below. | |
| +| 9. P5 leaves versions/residency residue | **Deferred-by-scope; acceptable** | F3 explicitly includes versions, delete markers and complete residue verification. | |
| +| 10. Deletion warrant lacks scope binding | **Deferred-by-scope; acceptable** | F3/F4 require exact coordinates and an immutable console ledger. | |
| +| 11. Member-minted token bypasses ownership | **Resolved for Phase 1** | All move mutations require an operator; service credentials retain only status. Owner delegation is acceptably deferred to F1. | |
| +| 12. Store cutover conflicts with rollback | **Deferred-by-scope; acceptable** | Private stores are refused; F4 requires a joint transaction. | |
| +| 13. Existing helpers exclude held rows | **Resolved** | Explicit held render/materialization APIs and activator retention are specified. | |
| +| 14. Readiness checks the wrong environment/process | **Partly** | Process-bound health improves this; T3 still has launch-environment and freshness gaps described below. | |
| +| 15. Timers are neither frozen nor restored | **Partly** | Exact replacement is specified, but skipping held deliveries does not drain already-running deliveries or freeze declaration convergence; rollback target retirement also races capture. | |
| +| 16. Registry inventory incomplete | **Resolved** | Columns are classified; `Replication` and `NextDue` are carried and authority-bearing shapes refused. | |
| +| 17. Store record/lifecycle omitted | **Deferred-by-scope; acceptable** | F4 includes record/config coordination and lifecycle serialization. | |
| +| 18. App-move partial failure underspecified | **Deferred-by-scope; acceptable** | F5 requires stable child IDs, durable outcomes and a barrier covering all consumers and concurrent creates. | |
| +| 19. Archive identity contradicts randomized encryption | **Resolved** | Plaintext and ciphertext identities are separated; recorded sealed artifacts are reused. | |
| +| 20. Incident cause called unconfirmed | **Resolved** | Missing regional secrets keys are recorded as confirmed, with remediation and fingerprint verification. | |
| + |
| +The §17 interfaces do **not yet constitute an agreement implemented by both documents**: |
| + |
| +| Interface | Assessment against #199 rev 3 | |
| +|---|---| |
| +| I-1 Incarnation | Agrees, including zero-`CreatedAt` identity. | |
| +| I-2 Tombstone region | Agrees on current record region; the other-region recovery obligation needs durable tracking. | |
| +| I-3 Staging ownership | **Requires #199 amendment:** its §3.3 still parks every key, without `move.json` filtering. | |
| +| I-4 Park helpers | Namespaces agree; reuse during installation needs additional journaling, below. | |
| +| I-5 Recreate deferral | Landing can reuse the predicate, but that predicate does not cover an abandoned replica outside the tombstone’s region. | |
| +| I-6 Stale-row retirement | Compatible for a live move record; rollback additionally needs target-row retention. | |
| +| I-7 Flat files | **Requires #199 amendment:** its §5 expressly excludes flat app files. | |
| +| I-8 Generation/lock markers | Compatible with copying generation metadata and deleting the lock last. | |
| +| I-9 Fresh destructive checks | Compatible as a requirement; sharing a trigger or rereading does not serialize competing park/install operations. | |
| +| I-10 Push | Agrees: deferred onto the move handoff. | |
| + |
| +The following are **new failure traces in rev 2**, rather than repetitions of the original findings. |
| + |
| +1. **P1 — Flat-exec stop evidence can leave a descendant writing.** |
| + **Spec:** §5.3, §7.4, §7.8, §14.2. |
| + **Code:** [backend_exec.go:1525](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/backend_exec.go:1525), [maintenance.go:40](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/maintenance.go:40). |
| + |
| + A flat application forks a background worker that closes inherited output descriptors. The main process exits on SIGTERM; `childProc.Stop` and `Exited` succeed while the descendant continues writing. `StopEvidence` consequently passes. The target starts another writer. The edge service’s `KillMode` only helps when that service stops, not during an ordinary instance cut. |
| + |
| + **Fix:** require an empty per-instance process group/cgroup, with enforced termination of descendants, before publishing evidence. Refuse unsupported flat layouts. Add independently surviving descendants to the simulation; its current single application-process model misses this case. |
| + |
| +2. **P1 — Stop evidence cannot currently be recovered after losing activator memory.** |
| + **Spec:** §5.3, §7.8 target release, §7.13. |
| + **Code:** [maintenance.go:90](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/maintenance.go:90). |
| + |
| + `FenceAndStop` requires either an entry or a complete in-memory `stopHistory`. After an agent restart, both can be absent. The same problem occurs when landing fails before any target probe starts. Repeated calls then return `ErrStopUnproven`, even when no process exists. The source remains fenced indefinitely. |
| + |
| + **Fix:** specify a restart-safe evidence-reconstruction operation using actual unit/cgroup state, replica closure and lease state. Also define evidence for an operation that durably never spawned. Do not implement either case as “missing entry means stopped.” |
| + |
| +3. **P1 — Dirty rollback has no durable source-recovery journal.** |
| + **Spec:** §7.8 source recovery, §7.13. |
| + **Code:** [backend_exec.go:264](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/backend_exec.go:264), [backend_exec.go:912](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/backend_exec.go:912). |
| + |
| + The source installs the returned generation, starts its recovery probe, and acknowledges background work. It then crashes before logging and clearing `move`. The durable rollback phase remains `returned`. Re-entering source recovery starts again at installation, which can park the changed database and restore the original returned snapshot, losing those acknowledgements. |
| + |
| + “Exactly as §7.6” does not identify a separate source phase journal: `landing_phase` already describes the target’s earlier attempt. |
| + |
| + **Fix:** give the return installation its own operation-bound phases through `probing`, recovery commit, open and finalization. Once its probe starts, recovery must never reinstall. Retain `move` until both recovery and target retirement are durable. |
| + |
| +4. **P1 — Reconcile can remove the rollback target before its state is captured.** |
| + **Spec:** §7.8 target release, §9.6. |
| + **Code:** [agent.go:1793](/home/paulca/amadan.net/carlos/platform-moves-design/internal/host/agent.go:1793), [reconcile.go:1155](/home/paulca/amadan.net/carlos/platform-moves-design/internal/provision/reconcile.go:1155), [timers.go:183](/home/paulca/amadan.net/carlos/platform-moves-design/internal/registry/timers.go:183). |
| + |
| + The rollback CAS immediately changes `region` to the source. The next target converge runs reconcile before the move worker. §9.6 protects only the `from` row, so the target row can disappear before release, timer capture and return publication. Schedule pruning can then delete timers registered by the target probe. |
| + |
| + **Fix:** retain both participating rows under their operation IDs until the corresponding release/return obligations finish. Target retirement must require a durable return-complete or clean-release state, followed by cleanup completion—not merely a region mismatch. |
| + |
| +5. **P1 — A rolled-back target leaves a replica that #199 can later expose to a new incarnation.** |
| + **Spec:** §7.8 step 6, §7.12, §17 I-2/I-5. |
| + **Code:** [backend_exec.go:300](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/backend_exec.go:300), [activator.go:138](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/activator.go:138). |
| + |
| + Rollback parks the target’s **local** database and files, but never parks its installed `instances/<host>/` replica. Later, deletion in the source region produces a source-region tombstone. #199 parks remote replicas only in the tombstone’s region; the former target now has no local leftovers to trigger deferral. Recreating that hostname there can restore the previous incarnation’s replica. |
| + |
| + **Fix:** rollback finalization must park every installed target replica out of the live host namespace before clearing the operation. Persist the obligation across crashes and deletion. This is incarnation isolation required in Phase 1, not deferred residency deletion. |
| + |
| +6. **P1 — Installation markers are published before installation is complete.** |
| + **Spec:** §6.3, §7.6, §7.8. |
| + **Code:** [paths.go:50](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/paths.go:50), [admintoken.go:115](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/admintoken.go:115), [registry.go:150](/home/paulca/amadan.net/carlos/platform-moves-design/internal/registry/registry.go:150). |
| + |
| + In `installed`, `.files-gen` is written before `<host>.env`, timers, `Replication` and `NextDue`. A crash after that marker satisfies the stated replay stopping condition while those operations remain incomplete. The target may mint a replacement admin token or commit without the carried timers. |
| + |
| + The bucket phase also combines parking and replacement installation. After parking finishes and replacement copying starts, replaying #199’s park helper encounters new source keys against its completed manifest; it cannot safely treat the whole phase as replayable. |
| + |
| + **Fix:** journal park completion separately from installation. Publish an operation-specific completion marker only after every required component is durable. Check the expected generation and incarnation, not marker existence alone. Exercise crashes between individual side effects. |
| + |
| +7. **P2 — Replica mirroring deletes the metadata sharing its staging prefix.** |
| + **Spec:** §7.3, §7.4, §7.12, §8.3. |
| + **Code:** [mirror.go:222](/home/paulca/amadan.net/carlos/platform-moves-design/internal/lease/mirror.go:222). |
| + |
| + `MirrorPrefix` deletes destination keys absent from the source replica. The spec puts replica objects, files archives, timer snapshots and `move.json` beneath the same staging prefix. A later replica round deletes the archive and ownership metadata. Snapshot rounds only upload when plaintext changes, so an unchanged archive can remain missing and T5 never recover. |
| + |
| + **Fix:** give each artifact class a separate subprefix, such as `replica/`, `files/` and `timers/`, with immutable operation metadata outside the mirrored subtree. Specify which objects landing installs into `instances/`. |
| + |
| +8. **P2 — T3 neither checks the complete launch environment nor permits all supported layouts.** |
| + **Spec:** §7.3 T3/T7, §7.6 configured, §9.1. |
| + **Code:** [backend_exec.go:1173](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/backend_exec.go:1173), [backend_exec.go:980](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/backend_exec.go:980), [paths.go:37](/home/paulca/amadan.net/carlos/platform-moves-design/internal/activator/paths.go:37). |
| + |
| + The specified report covers `instanceEnv` plus agent-owned variables, but `childEnv` also inherits non-`CARLOS_*` agent variables. Those can change launch behavior without appearing in T3. Conversely, strict equality except `CARLOS_REGION` rejects flat-to-nested moves because `CARLOS_CONTROL_SOCKET` legitimately changes. |
| + |
| + A fresh report also does not pin the final archive, bundle versions or release: landing renders again, and its acknowledgement is not required to match the approved report. |
| + |
| + **Fix:** compare the actual composed launch environment, classify layout-derived variables explicitly, and bind readiness to immutable input versions. Revalidate changed inputs before stopping the source. Include nested control-channel and mount readiness. |
| + |
| +9. **P2 — The expanded inventory still omits generation-marker treatment and restore-verification state.** |
| + **Spec:** §5.1, §6.1, §8.1, §17 I-7. |
| + **Code:** [dbverify.go:29](/home/paulca/amadan.net/carlos/platform-moves-design/internal/dbverify/dbverify.go:29), [dbverify.go:71](/home/paulca/amadan.net/carlos/platform-moves-design/internal/dbverify/dbverify.go:71). |
| + |
| + `.files-gen` has no inventory disposition and is not excluded from F4’s archive. Carrying it as application content and then replacing it changes the content digest even without application writes. The host-keyed restore-verification verdict is also absent; an earlier generation’s successful verdict can survive a replacement. |
| + |
| + **Fix:** classify all new markers explicitly, exclude protocol markers from application-content digests, and reset or generation-bind restore verdicts on installation and rollback. Include flat `<dbPath>.gen` in local parking. I found no additional unclassified `routes` column. |
| + |
| +10. **P2 — T11 does not bound the cut window it claims to admit.** |
| + **Spec:** §7.3 T11, §7.4, §7.7. |
| + **Code:** [movespass.go:468](/home/paulca/amadan.net/carlos/platform-moves-design/internal/host/movespass.go:468), [movespass.go:480](/home/paulca/amadan.net/carlos/platform-moves-design/internal/host/movespass.go:480). |
| + |
| + T11 adds restore time and stop time only. It omits final checkpoint/mirror, archive creation/upload, installation, configuration and the probe’s 20-second settle window and retries. A move can pass the default 120-second admission check while its specified successful sequence exceeds that window. |
| + |
| + **Fix:** estimate the entire stop-to-open sequence, reserve its budget across phases, and define when budget exhaustion begins recovery. Keep recovery deadlines distinct from the promised cut window. |
| + |
| +For the simulation, the critical additions are descendants that outlive the main process, empty activator memory, rollback-source writes, reconcile before target release, deletion/recreation after rollback, and crashes between marker publication and subsequent writes. Atomic phase-level mocks would conceal these failures. |
| + |
| +There is **no §23 in rev 2**. Its former questions are now rulings M4–M15; these recommendations retain those decisions: |
| + |
| +| Former question | Recommendation | |
| +|---|---| |
| +| Q1 Files | Fixed paths; explicitly classify protocol metadata and layout mapping. | |
| +| Q2 Cap | Retain 256 MiB, enforced while streaming and at cut. | |
| +| Q3 Automatic cut | Retain validated auto-cut and manual API default. | |
| +| Q4 Hold | Retain seven days and owner early deletion through F3. | |
| +| Q5 App partial failure | Retain pause and target app region; F5 owns durable child recovery. | |
| +| Q6 Store barrier | Retain a durable barrier covering every consumer and concurrent creates. | |
| +| Q7 Sidecars | Retain refusal until sidecar moves exist. | |
| +| Q8 Prewarming | Defer it; correct the complete cut-duration admission estimate. | |
| +| Q9 Rehearsals | Retain moveprobe → disposable ips → kpmg, each within scoped approval. | |
| +| Q10 Authority | Retain owners-only delegation in F1 and operator-only Phase 1. | |
| +| Q11 Landing prerequisite | Retain one landing protocol; #199 rev 3 has no epoch to require. | |
| +| Q12 Live replica location | Retain installation into `instances/`, with durable completion and replay protection. | |
| + |
| +For rev 2’s actual open question, **§18: choose (a)**, but restrict `--declare-lost` to an exact pre-commit handoff, require independently established execution fencing, and recover any durable target/return data before accepting disclosed loss. |
| + |
| +**Verdict: not ready for planning (Phase 1).** Stop evidence, rollback recovery/finalization, installation journaling and #199 incarnation isolation still require protocol decisions. The follow-on scope reductions are acceptable. |
| \ No newline at end of file |