carlos / platform Public

1. P1 — Restore can still block indefinitely on a FIFO. restore.go:22 After realParents checks a child directory, a tenant can rename it aside and replace it with a FIFO before syncDir calls root.Open. That open has neither O_DIRECTORY nor O_NONBLOCK, so it waits indefinitely without a writer. The install holds the host guard during this operation; cancellation does not interrupt the open. This is an inherited defect still reached through the new RestoreRoot call. Use a descriptor-relative directory open that cannot block, and test the replacement at the existing syncDir seam.

1. P2 — The absence check still confuses a vanished directory with a missing file. relocationflush.go:184, also present.go:63 StatDir closes its descriptor before the subsequent read reopens the directory. If the directory disappears between those operations, ReadBeneathLimit returns ENOENT from opening the directory, and readInstanceFile reports absent=true. A transient rename followed by restoration can therefore let a flush commit deletion of an existing env or adoption item. Present repeats this pattern and can incorrectly accept absence. Keep one directory descriptor and recognize absence only from the file lookup beneath it.

Verdict: request changes. The root-opening fix is sound, but the requested protection against blocking and check/use races is incomplete.

The other conclusions from the brief:

• OpenRootBeneath keeps the original descriptor open until os.OpenRoot acquires its own; renaming the pathname cannot redirect that handoff.
• I found no outside-root symlink write through RestoreRoot. Internal symlink redirection remains possible, but bounded by Go’s os.Root confinement.
• Hard-linked destinations are replaced rather than truncated. Reads still accept hardlinks; cross-tenant protection depends on the existing isolation and migration controls.
• Restore and SyncDir retain their shared implementations and existing caller behavior. No second low-level safe-open implementation was introduced, although the flawed absence-check sequence is duplicated.

Targeted tests could not start: the read-only sandbox prevented Go from creating its build directory. Findings are from source inspection.

---

FIRST PROMPT: “Follow the review brief docs/superpowers/plans/CODEX-REVIEW-REGION-HA-TENANT-DIR-BRIEF.md. Review the diff cfb4314d..3194d674 on branch region-ha-1b-next-build.”

WHAT: Review tenant-directory safety in the requested diff.

STATUS: Review complete: one P1 and one P2; tests blocked by the sandbox.

NEEDED: Nothing for this review.

RECOMMENDED: Fix both races and add deterministic regression tests before approval.

SESSION: Safe to close. No files changed or processes left running; the read-only sandbox prevented saving a handoff file.