amadan
plan: region HA 1a, Astra plan round 1 findings (unedited)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+41
−0
docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R1.md+41 −0
diff --git a/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R1.md b/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R1.md| new file mode 100644 |
| index 000000000..3b43fd608 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R1.md |
| @@ -0,0 +1,41 @@ |
| +Reviewed the [plan at `7a8cf520`](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md) against the working tree and spec. This was a static review; the proposed code and tests were not executed. `plan:L` below refers to that plan’s line numbers. |
| + |
| +1. **P1 — Task 2, Step 3: missing import prevents compilation.** `plan:876` introduces `status.InstanceID()`, but `internal/host/edgesfile.go:3–20` does not import `internal/status`. The import in `edgesfile_test.go` does not apply to another file. **Fix:** explicitly add `github.com/carlosframework/platform/internal/status` to `edgesfile.go`. |
| + |
| +2. **P1 — Task 1, Steps 1 and 3: the storage key is not bound to the verified box.** `Load` reads `Key(box)` but never checks `rec.Box == box` (`plan:378–398`). An intact, valid record for A stored under `control/boxes/B.json` therefore passes verification as A (`plan:470–471`). Review Focus 5’s test changes the record’s `Box` field instead of exercising this substitution (`plan:197–203`). **Fix:** reject a storage-key/record mismatch and test loading A’s unchanged record from B’s key. |
| + |
| +3. **P1 — Task 1, Step 4 / Task 3, Step 3: a supported but wrong region is accepted.** Verification selects a certificate using the document’s own region, with no expected-region comparison (`plan:453–461`). Eligibility then selects by unsigned placement metadata without comparing the verified region (`plan:1254–1274`). A signed US identity can qualify through an edges file advertising the Irish placement, contrary to spec lines 193–196. **Fix:** require the signed region to match the expected AWS region; test a mismatch where both regions have certificates. |
| + |
| +4. **P1 — Task 6, Step 3: `readStable` can follow ancestor symlinks or block opening a substituted FIFO.** `Lstat` followed by ordinary `os.Open` (`plan:2551–2579`) does not secure the path. Replacing a parent directory with an external symlink before the read lets both inode checks succeed on the external file. Replacing the final file with a FIFO between inspection and opening can block before `f.Stat`. Task 7 reaches this through queued paths at `plan:3144`. **Fix:** use root-relative traversal that refuses symlink components and a nonblocking, no-follow final open. Add deterministic ancestor and FIFO substitution tests. |
| + |
| +5. **P1 — Task 5, Step 5: restore can write or delete outside its destination.** `CleanPath` checks spelling, but `filepath.Join` and `os.Remove` follow ancestor symlinks (`plan:2219–2226`), as do the parent-directory operations in `writeAtomic` (`plan:2271–2274`). With `files/sub -> outside`, entry `sub/x` modifies `outside/x`. **Fix:** constrain all restore operations, including tombstones and temporary files, to an opened destination root. Test both writes and deletes through an external symlink. |
| + |
| +6. **P1 — Task 11, Step 2: the queue policy does not enforce exclusive termination-event provenance.** The statement at `plan:4655–4662` only grants the EventBridge rule access. A same-account principal with an identity-based `sqs:SendMessage` grant can still submit a forged termination event, whose JSON fields Task 9 accepts. AWS documents that these grants combine unless explicitly denied. [SQS policy evaluation](https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-using-identity-based-policies.html). **Fix:** explicitly deny `SendMessage` when the source ARN is missing or differs from the rule. Test both the legitimate rule and an otherwise IAM-authorized sender. |
| + |
| +7. **P2 — Task 5, Step 5: directory-to-file transitions cannot restore.** Publish `a/b`, then replace directory `a` with file `a`. The resulting manifest has live `a` and tombstoned `a/b`. Restore sorts paths (`plan:2216`), writes `a`, then fails removing `a/b` with `ENOTDIR` (`plan:2222–2226`). Restoring over an earlier copy also fails when renaming a file over the existing directory. **Fix:** handle deletions and file/directory transitions explicitly. Test both transition directions against empty and previously restored destinations. |
| + |
| +8. **P2 — Task 7, Step 9: overflow does not repair missing directory watches.** `IN_Q_OVERFLOW` requests a scan without marking coverage degraded (`plan:3434–3436`), while `run` repairs watches only when degraded (`plan:3093–3098`). If overflow loses a directory-creation event, the scan publishes its current files but leaves the directory unwatched; later changes wait ten minutes. **Fix:** mark coverage uncertain, rebuild recursive watches, and retain polling until repair succeeds. Test a lost directory-creation event followed by another write inside that directory. |
| + |
| +9. **P2 — Task 7, Steps 6 and 9: watcher failure can disable fallback polling.** A read error exits the reader and closes its channel (`plan:3403–3411`). `run` ignores the receive’s `ok` value (`plan:3076`), producing a busy loop. `Repair` can then clear degradation after adding watches even though no reader remains (`plan:3332–3336`). **Fix:** detect channel closure and recreate the watcher or stay in polling mode. Test continued publication after the event channel closes. |
| + |
| +10. **P2 — Task 2, Step 3: headroom never refreshes solely on the five-minute tick.** The new comparison deliberately removes headroom (`plan:853–871`), but the existing timed path still calls the same gated function without forcing a write (`internal/host/edgesfile.go:692–695`). If nothing else changes, headroom remains stale indefinitely, contrary to `plan:642`. **Fix:** distinguish a periodic refresh from a change-triggered pass. Test that headroom-only changes skip ordinary passes but publish on a timed refresh. |
| + |
| +11. **P2 — Task 8, Step 6: importing default replication leaves the destination’s previous class.** `ImportWake` skips `SetReplication` when the source value is empty (`plan:3895–3898`). Importing default state onto a destination set to `daily` therefore retains `daily`. Empty is explicitly valid and supported by the setter (`internal/registry/registry.go:415–420, 1259–1278`). **Fix:** apply the empty value too; test an empty source class against a nonempty destination. |
| + |
| +12. **P2 — Task 3, Steps 1 and 3: `chooseBox` implements a different spread rule.** Spec lines 266–269 prefer a box with no instance of the app, then use total claimed-host count. The implementation instead minimizes app-instance count before total load (`plan:1299–1306`). Its test deliberately picks a box with five total hosts over one with two when both already run the app (`plan:1204–1206`). **Fix:** compare whether the app count is zero, then total hosts, then box ID; update that test. |
| + |
| +13. **P2 — Task 6, Step 3: directory exclusions can tombstone previously published children.** The walk respects `skip("cache", true)`, but the deletion pass checks only `skip("cache/x", false)` (`plan:2627–2630, 2666–2670`). With the plan’s callback, an existing `cache/x` entry becomes a tombstone. Individual-file exclusions are preserved, so the two exclusion forms behave differently. **Fix:** retain excluded directory prefixes during reconciliation and test exclusion after the child has already been published. |
| + |
| +14. **P2 — Task 7, Step 4: startup scanning masks event and polling test failures.** `startRun` returns before its mandatory initial scan finishes (`plan:2878–2883, 3070`). The deadline, overflow and polling tests immediately create their files (`plan:2908–2915, 2936–2965`), so the initial scan can satisfy their assertions with the tested mechanism disabled. **Fix:** synchronize on completion of the initial scan before making changes; assert the publication timing and disable unrelated scan paths. |
| + |
| +15. **P2 — Task 6, Step 1: Review Focus 2 and 3 are only partly tested.** The nonregular-file test covers static symlinks, but no FIFO, socket, device or substitution race (`plan:2430–2453`). The oversized-file test checks the final skipped result, which would also pass if the implementation read the entire file before rejecting it (`plan:2457–2480`). **Fix:** add applicable special-file cases and deterministic races, plus an instrumented read boundary that proves oversized files are rejected without reading their contents. |
| + |
| +16. **P2 — Task 8, Step 1: the all-or-nothing test never reaches the transaction.** Its wrong-host row (`plan:3575–3579`) is rejected before `Begin` (`plan:3620–3625`). The test would pass with nontransactional deletion and insertion. **Fix:** use duplicate names belonging to the correct host so insertion fails after deletion and one successful insert; assert that the original rows survive. |
| + |
| +17. **P2 — Task 9, Step 1: the unverified-identity test passes for the wrong reason.** `FromDescribe(..., "", "")` returns `Found:false` immediately (`plan:4065–4068, 4193–4196`). Removing `Verify`’s empty-identity guard would still leave the test passing on absence. **Fix:** construct a `Found:true`, terminated, describe observation with matching empty identity coordinates and verify that the identity guard rejects it. |
| + |
| +18. **P2 — Task 4, Step 4: required conflict diagnostics are absent.** The plan promises a once-per-refresh diagnostic for equal positive stamps (`plan:1348`), and spec lines 322–326 require an alert for conflicting unstamped entries. Neither the fold replacement nor `dialBeats` implements reporting (`plan:1497–1529`). **Fix:** add refresh-scoped diagnostics and tests that preserve the deterministic winner while checking the warning. |
| + |
| +19. **P3 — Task 5, Step 6: the stated storage protection is deferred elsewhere.** `plan:2303` says Task 11 enforces create-only content objects, but Task 11 protects only `control/boxes/`; the files-prefix policy is explicitly deferred to 1b at `plan:4877`. **Fix:** correct the statement to describe the deferred enforcement and its prerequisite before enabling the publisher. |
| + |
| +**Verdict: not ready.** |
| \ No newline at end of file |