carlos / platform Public
| Round 1 | Disposition | |---|---| | 1 — Missing status import | Resolved. Explicit import added. | | 2 — Storage key not bound to record | Resolved. Load checks the binding; the substitution test exercises it. | | 3 — Wrong signed region accepted | Resolved. Eligibility requires the expected region to match both identity and edges file. | | 4 — Unsafe readStable traversal/open | Partly resolved. External symlink escape and substituted-file FIFO blocking are addressed. In-root ancestor links remain followed; see finding 4. | | 5 — Restore escapes destination | Resolved. Restore operations use os.Root. | | 6 — Queue accepts other senders | Resolved. The explicit Deny covers other or missing source ARNs, exempts the named rule, and does not affect receive/delete. This matches EventBridge’s source-ARN delivery policy and IAM’s negated-condition semantics. | | 7 — File/directory transitions | Resolved. Tombstones precede writes, obstructions are removed, and both directions are tested. | | 8 — Overflow leaves watches missing | Partly resolved. Overflow now requests repair, but concurrent degradation can be erased and the new test is nondeterministic; findings 5 and 7. | | 9 — Dead watcher disables polling | Resolved. Closed channels switch to polling; a dead reader cannot subsequently pass Repair’s entry guard. | | 10 — Headroom never refreshes | Resolved. The timer supplies refresh; the seeded gate also excludes headroom. | | 11 — Default replication not imported | Resolved. SetReplication is unconditional. | | 12 — Incorrect spread rule | Resolved. App presence, total hosts, then box ID. | | 13 — Excluded directories tombstoned | Resolved. Both passes check excluded ancestor prefixes. | | 14 — Startup scan masks tests | Resolved. Tests wait for the initial scan before introducing changes. | | 15 — Special-file and size tests incomplete | Partly resolved. FIFO, substitution and zero-read size tests are added. Socket/device cases remain absent from Task 6, Step 1; add socket coverage and privilege-gated device coverage. | | 16 — Transaction test fails before transaction | Resolved. Duplicate names fail during insertion and check rollback. | | 17 — Identity test passes on absence | Resolved. The observation is now found, terminated and coordinate-matched. | | 18 — Conflict diagnostics missing | Partly resolved. Diagnostics exist but miss conflicts involving self and some three-box cases; finding 6. Dropping unstamped alerts is justified by everywhereClass (internal/provision/reconcile.go:202) and unconditional home publication (internal/host/edgesfile.go:326). | | 19 — Storage enforcement misattributed | Resolved. Enforcement is explicitly deferred to 1b and required before enabling publication. |
Reviewed the plan at d4e7d2df against the working tree, spec and installed Go 1.26.5 source. plan:L below refers to that revision. This was a static review; no files were changed or proposed tests executed.
- P1 — Task 6, Step 3: directory traversal can still block on a substituted FIFO. At plan:3271,
fs.WalkDir(root.FS(), …)descends using a previously obtainedDirEntry. If a directory becomes a FIFO after the callback, traversal opens that FIFO beforereadStableis involved. Go’sio/fs/walk.go:81callsReadDir;os/root.go:389implements it throughr.Open(name), which uses plainO_RDONLY(os/root.go:103). The scan can therefore hang indefinitely, including after context cancellation. Fix: traverse using directory handles opened with directory-only, nonblocking semantics. Add a deterministic directory-to-FIFO substitution test between enumeration and descent; the existing file-to-FIFO test does not cover this path.
- P1 — Task 6, Step 3 / Task 7, Step 6: read failures become destructive tombstones. plan:3184–3200 converts every non-absence
Lstator open error intofileNotRegular. A previously published regular file that becomes unreadable therefore receives a tombstone:syncRootomits it fromseenand deletes it atplan:3324;publishOnedeletes it directly atplan:3932–3936. A regular-file replacement between inspection and opening also takes this path throughSameFile. Neither condition proves deletion. Fix: propagate permission/I/O errors and treat identity changes as retryable instability. Preserve the committed entry until absence or a nonregular replacement is established. Test an unreadable existing file and a regular-to-regular substitution.
- P1 — Task 5, Step 5: successful restore does not establish crash durability. plan:2730–2740 syncs the temporary file, renames it and returns without syncing its directory. Directory creation and tombstone removal likewise lack directory syncs. Syncing file contents does not guarantee persistence of their directory entries. Linux
fsyncdocumentation explicitly distinguishes these operations. Consequently, the planned restore-then-install-marker sequence can acknowledge installation without a durability guarantee for restored names or deletions. Task 10 syncs.installed, not the restored directory tree. Fix: make successful restore include durability barriers for affected directories, including newly created ancestors and tombstone removals. Propagate barrier failures and test their ordering before successful completion.
- P2 — Task 6, Step 3 / Task 7, Step 6: internal ancestor symlinks bypass exclusions. At plan:3180 and 3195, both operations follow ancestor symlinks that remain inside the root; this is permitted by
os.Root. For example, queuesub/x, replacesubwith a relative symlink to excluded directorycache, then process the event.skippedcheckssub/x, while both identity checks observecache/x. Its contents are published despite the exclusion and the promise never to follow links. Fix: reject symlink components through descriptor-based traversal, preserving that decision against substitution. Test an in-root ancestor link pointing into an excluded directory, including the queued-event path.
- P2 — Task 7, Step 9: repair can erase a newer coverage failure. plan:4197–4205 unconditionally clears
degradedafteraddTree. Meanwhile, the reader can process another overflow or failed watch addition and set it true. If that happens after repair enumerated the affected parent but before its final store, repair clears the newer failure. The requested scan publishes existing content, but the missing watch remains and polling stops. Fix: track a degradation generation, or serialize coverage changes so repair clears only the failure state it actually repaired. Test an overflow after the repair walk has passed a parent, with a newly missed child directory beneath it.
- P2 — Task 4, Step 4: tie reporting depends on self exclusion and the current winner. plan:1697 compares only the incoming entry with the folded winner. With sorted stamps
9, 7, 7, the duplicate7is never reported. Also, existinginternal/edge/homemap.go:547excludes self before folding: in a two-box deployment where both publish stamp7, neither box reports the conflict. The new test uses a third box as self and misses this. Fix: collect conflicting positive stamps independently of dial selection, including self in diagnostics while retaining self exclusion from routing. Test both examples.
- P2 — Task 7, Steps 4 and 9: the new repair tests can pass with their mechanisms disabled.
TestRunScansOnceAfterARepairThenStopsPollinguseseventsOnlyat plan:3697: polling is an hour apart, but the no-more-scans assertion waits only 200 ms. Continuing to poll would pass.
The overflow test at plan:4080–4106 creates lost while the real reader runs, then deletes only its descriptor-map entry. Draining w.ch does not synchronize the kernel reader. A delayed real IN_CREATE can subsequently install the watch, allowing the test to pass even if Repair stops rebuilding watches. Fix: use a short polling interval and observe several polling periods after repair. For overflow, deterministically suppress and acknowledge the creation event, establish missing coverage, and prove only repair restores it. Both tests should fail when their respective mechanism is disabled.
Verdict: not ready.