carlos / platform Public

Astra brief: hostfiles path cap, round 4 (revised design); round 3 review committed unedited

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Paul Campbell pushed by claude 1 week ago c6dc57d0eb673aa7da30d03d837616fd0ce3d654 parent 3fb0e57

2 files changed, +58 −0

docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-3.md +33 −0
docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-BRIEF-4.md +25 −0

CI cancelled — superseded by a newer push — run details

CI log

=== gate ===
cd launchctx && go vet ./... && go test ./... -count=1
ok  	amadan.net/carlos/platform.git/launchctx	0.011s
?   	amadan.net/carlos/platform.git/launchctx/controlname	[no test files]
?   	amadan.net/carlos/platform.git/launchctx/wireid	[no test files]
go vet ./...
gofmt -l .: clean
go test ./... -count=1 -timeout 30m

amadan: job cancelled — a newer push superseded it

diff --git a/docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-3.md b/docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-3.md

new file mode 100644

index 000000000..795bb3e8c

--- /dev/null

+++ b/docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-3.md

@@ -0,0 +1,33 @@

+**Request changes: two P2 findings. Round 2’s P2 is only partially closed; this HEAD is not ready to merge.**

+1. **P2 — Over-cap parent directories still strand the flush.** At [restore.go:153](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/hostfiles-cap-live-entries/internal/hostfiles/restore.go:153), removal deletes only the tombstoned file and leaves every directory standing. With **2049** one-character directories, the parent path is 4097 bytes. Restore succeeds, but [walk_unix.go:63](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/hostfiles-cap-live-entries/internal/hostfiles/walk_unix.go:63) rejects that now-empty directory with `ErrTooDeep`, so relocation’s flush still cannot commit. Both new regression fixtures use 2048 directories, keeping the parent exactly within the cap. Add the one-level-deeper case and resolve its restore → scan → flush behavior.

+2. **P2 — An ancestor rename can redirect deletion outside the tombstone’s path.** At [restore_unix.go:72](/home/paulca/amadan.net/carlos/platform/.claude/worktrees/hostfiles-cap-live-entries/internal/hostfiles/restore_unix.go:72), `unlinkat` uses the parent descriptor obtained during descent without checking its continuing relationship to the restore root. After the first directory opens, a concurrent actor can rename that subtree elsewhere; subsequent opens and deletion continue inside the moved subtree. This can delete at an unlisted path, including outside the restore root when the actor can move the subtree there. `O_NOFOLLOW` prevents following a replacement symlink but does not prevent this case. Add a deterministic ancestor-rename regression and establish the required confinement or enforced exclusion of concurrent renames.

+The other requested checks:

+- **Durability:** syncing the immediate parent is sufficient for this unlink when ancestor entries remain unchanged. Syncing even after `ENOENT` correctly covers retry after an earlier unlink whose sync failed.

+- **Five ruling conditions:** the implementation has descriptor-relative `O_NOFOLLOW|O_DIRECTORY` descent, plain-component validation, final-name-only `unlinkat`, cancellation checks between components, and no path-based fallback. Expected absence/type errors become no-ops; other errors propagate. Those mechanical conditions do **not** establish confinement against ancestor renames.

+- **Symlinks and directories:** symlinks are not followed, including at the final name. `unlinkat` with flags `0` cannot remove a directory.

+Static review only; no tests ran, including the full `internal/host` suite. HEAD remains `5065b52ae663207dd32306fbe090ad23046653d5`; the working tree is unchanged.

+---

+**FIRST PROMPT:**

+Review the round-3 brief at HEAD 5065b52, read-only; avoid the full internal/host suite.

+**WHAT:**

+Assess the deep-tombstone fix, durability, ruling conditions, races, and merge readiness.

+**STATUS:**

+Review complete: two P2 findings; request changes.

+**NEEDED:**

+Nothing from you for this review.

+**RECOMMENDED:**

+Address both findings and cover the deeper-parent and ancestor-rename cases.

+**SESSION:**

+Safe to close. No files or background processes were created.

\ No newline at end of file

diff --git a/docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-BRIEF-4.md b/docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-BRIEF-4.md

new file mode 100644

index 000000000..e2121c42e

--- /dev/null

+++ b/docs/superpowers/plans/CODEX-REVIEW-HOSTFILES-CAP-BRIEF-4.md

@@ -0,0 +1,25 @@

+# Astra: hostfiles path cap, round 4 (revised design)

+Round 3 (`CODEX-REVIEW-HOSTFILES-CAP-3.md`) found two P2s in the deep-tombstone removal walk: over-cap parent directories left behind, and an ancestor-rename redirect. Paul ruled to withdraw the walk and refuse over-cap tombstones up front. Commit 54196e88, then merged with main in 3fb0e576.

+The rule is `hostfiles.RestorableWithinCap`, which is `manifestWithinCap` under the name Open, OpenAt and Commit use:

+- **Refused:** live entries and tombstones past 4095 bytes are ErrTooDeep. The readers (checkManifest: Restore, CopyManifest, ValidateManifest), Open, OpenAt and Commit all apply this.

+- **Exempt:** skipped entries, because Restore never touches their path.

+- **StageDelete:** for a path past the cap it is a no-op, so an inherited over-cap skipped entry whose file is gone stays skipped.

+- **The walk is removed.** removePastCap, its stub, descendCtx and their tests are gone, and Restore's tombstone loop is main's again.

+Upstream guards, per #236: moves' cut preflight will call RestorableWithinCap, relocation's pin already calls ValidateManifest, and #250's backfill reports failing manifests per host before activation.

+**Judge:**

+1. Are round 3's P2s moot now that the walk is gone?

+2. Does exempting skipped entries alone leave any path that strands a recovery or a flush? Consider:

+ - superseded();

+ - Present;

+ - the scan, with an over-cap skipped entry whose file still exists on disk (the walk refuses it: is that only an existing #279-class issue, or new?);

+ - CopyManifest and NewFromCopy;

+ - moves' Establish.

+3. Is a StageDelete no-op past the cap safe? Could it hide a real deletion that a reader then misrestores?

+4. Census: is the overlay envelope-only, with Plan 9 and Plan 14 legacy units unchanged?

+5. **Verdict:** is it ready to merge?

+Give findings P1/P2/P3 with file:line. Read-only; run nothing heavy.