amadan
plan: region HA 1a, Astra plan round 3 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-R3.md+41 −0
diff --git a/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R3.md b/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R3.md| new file mode 100644 |
| index 000000000..643f59775 |
| --- /dev/null |
| +++ b/docs/superpowers/plans/CODEX-PLAN-REGION-HA-1A-R3.md |
| @@ -0,0 +1,41 @@ |
| +| Finding | Disposition | |
| +|---|---| |
| +| R2-1 — Directory traversal can block | **Resolved.** Directory opens now use `O_DIRECTORY`, `O_NOFOLLOW` and `O_NONBLOCK`. | |
| +| R2-2 — Read failures become tombstones | **Resolved.** Unreadable files retain their entries; regular-file substitutions request retry. | |
| +| R2-3 — Restore lacks crash durability | **Partly resolved.** Directory barriers are present, but retrying a failed restore can omit a required barrier; details below. | |
| +| R2-4 — Ancestor symlinks bypass exclusions | **Resolved.** Publishing traverses components through directory descriptors without following links. | |
| +| R2-5 — Repair erases newer failures | **Resolved.** Capturing `failures` before the walk preserves subsequent failures, including those arriving before traversal starts. | |
| +| R2-6 — Tie reporting misses self and losing entries | **Resolved.** `tiedStamps` checks independently of routing and includes self. | |
| +| R2-7 — Repair tests pass with mechanisms disabled | **Resolved.** Short polling periods expose continued polling; the unwatched-directory fixture removes the competing creation event. | |
| +| R1-4 residual — Internal ancestor links | **Resolved**, as R2-4. | |
| +| R1-8 residual — Overflow repair | **Resolved**, as R2-5 and R2-7. | |
| +| R1-15 residual — Socket/device coverage | **Resolved.** Both cases are present with explicit environment skips. | |
| +| R1-18 residual — Conflict diagnostics | **Resolved**, as R2-6. | |
| + |
| +**Remaining R2-3 issue — P1, Task 5, Step 5:** retrying a failed restore can acknowledge an unsynced deletion. `touched` includes a tombstone’s parent only when this invocation removes the file ([plan:2703](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:2703)); an already-absent file returns `false` ([plan:2813](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:2813)). |
| + |
| +For a manifest containing only the tombstone `gone/old.txt`, let the first restore remove the file and then fail its directory sync. A second restore sees absence, records no touched directory and returns success without any directory barrier. The deletion’s durability remains unestablished. |
| + |
| +**Fix:** sync the relevant surviving directories for tombstones even when their files are already absent. Add a test that fails the first barrier and requires the successful retry to perform it. |
| + |
| +New findings, ranked: |
| + |
| +1. **P1 — Task 6, Step 1: the new tests depend on Task 7.** |
| + `sync_unix_test.go` calls `publishOne` at [plan:3400](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:3400) and `manifestHas` at [plan:3437](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:3437). Their definitions first appear in Task 7, at lines 4442 and 4125 respectively. Task 6’s completed test gate therefore fails with undefined identifiers when tasks are executed in order. |
| + **Fix:** retain the `SyncDir` assertions in Task 6 and move tests exercising `publishOne`, with their required helpers, into Task 7. |
| + |
| +2. **P2 — Task 5, Step 5: directory syncing breaks deeper directory-to-file transitions.** |
| + Start with a restored `a/b/c`, then tombstone that path and publish a regular file at `a`. Removing `c` records `a/b` for syncing; writing `a` removes that directory tree. At [plan:2750](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:2750), `Lstat("a/b")` returns `ENOTDIR`, which does not match `fs.ErrNotExist`. The subsequent `syncDir("a/b")` fails, aborting an otherwise valid restore before the root barrier. The existing transition test uses only `a/b`, so it misses this case. |
| + **Fix:** distinguish surviving directories from paths removed beneath a replacement file, while still syncing the surviving parent that records the replacement. Extend the transition test to `a/b/c → a`. |
| + |
| +3. **P2 — Task 7, Step 9: the held-open-file test does not require `IN_MODIFY`.** |
| + [plan:4554](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:4554) creates `open.log` after starting the watcher, writes once and checks publication. Removing `IN_MODIFY` from `watchMask` still allows `IN_CREATE` to queue that publication. The test therefore passes while later writes through the same open descriptor would remain unpublished until a scan or close. |
| + **Fix:** after observing the first publication, write again through the still-open descriptor and require the updated contents to publish. Verify that removing `IN_MODIFY` fails this assertion. |
| + |
| +4. **P2 — Task 7, Step 10: the mandatory kernel-test command omits the new repair-race test.** |
| + The command at [plan:4965](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/region-ha/docs/superpowers/plans/2026-10-05-region-ha-1a-foundations.md:4965) uses `-run Watcher`. Only six of the seven real-inotify tests match: `TestARepairDoesNotEraseAFailureThatArrivedDuringItsWalk` does not. The prescribed verification cannot produce the stated seven passes. |
| + **Fix:** rename that test to include `Watcher`, or widen both selectors to include it explicitly. |
| + |
| +Reviewed `805894fc` against the working tree, spec and installed Go source. This was a static review; proposed Go tests were not executed and no files were changed. |
| + |
| +**Verdict: not ready.** |
| \ No newline at end of file |