carlos / platform Public

Verdict: ACCEPT

Reviewed bf596e11..7630dd73, with HEAD verified as 7630dd73b643c6538dd69dda30116850b010f48d. Both outstanding P1s are closed. One new P2 should be fixed but does not block this scoped review.

Open findings

1. Round-2 finding 1 — CLOSED.

The planner’s overlap fallback and recovery can now record and run before root reports T failed. The fallback carries {Build: A, InPlaceOf: T} only after T’s termination is proven, and still passes the selection gate and mandatory record: wakeplan.go:401, swaphold.go:372.

Recovery admission is similarly narrow. recoverySelection requires a planned, non-first-wake boot of GoodVersion in place of a different installed version: plannedselection.go:58. The selection is attached to the recovery record at beforeexec.go:161; the caller checks the plan before choosing and recording that identity at activator.go:1899.

The host accepts the carrier only when sel.Build matches the build being recorded. Inside the durable-state CAS loop, the new exception additionally requires InPlaceOf to equal the freshly derived held target: buildstate.go:525, heldwake.go:155. A selection for another transition does not bypass revalidation. GoodVersion receives no general exemption, and NextBuildState plus the durable write remain mandatory.

The added integration tests cover immediate fallback, recovery before root records failure, and rejection of an ordinary stale good-build wake: buildstateplannedtest.go:224, :251, :281.

1. Round-2 finding 2 and round-1 finding 3 — CLOSED.

| Check | Verdict and evidence | |---|---| | Lock scope and root recheck | CLOSED. Root locks before rereading the route, rejects held or missing rows, and retains the lock through WriteConsumed: consumedfence.go:79. The carry locks before reading the consumed file and holds it through the durable merge: buildstate.go:567. | | Legacy cut | CLOSED. moving is set before the carry, which precedes the final mirror: movespass.go:1095, :1157. | | Phase-1 cut | CLOSED. HoldMoving precedes stopping; the final round carries before copying and sealing the build state: movescut.go:468, :989. The source remains held through landing and commitment: moveshold.go:179. | | Return | CLOSED. The target enters releasing before carrying its current state: movesrollback.go:428, :868. The pre-install branch appropriately returns the forward seal’s exact state at :834. | | Crash and restart | CLOSED. Process death releases the flock; the persistent lock file is not itself ownership. The registry hold survives, and replay restores the move’s fences. A failed carry stops before the state is copied/sealed. Repeating its CAS merge is safe. | | Absent lock file | CLOSED. Root creates the file before its locked route check. Therefore, when the carry finds no file after establishing the hold, any subsequent fenced publisher must observe that hold: consumedfence_linux.go:24, :35. This assumes both participants run the fenced implementation. | | Darwin and Windows | CLOSED for the stated development-only scope. The !linux implementation introduces no Linux dependency and preserves the route recheck: consumedfence_other.go:1. It deliberately provides no cross-process exclusion; it would not suffice for production moves on those platforms. |

The ordering closes the reported race: a publisher already inside the critical section finishes before the carry reads, while a later publisher sees the hold and writes nothing.

I found no lock-order deadlock. Root releases this lock before subsequent staging or restart work. The carry does retain it across bucket operations, including CAS retries and the replica listing—potentially more than one GET/PUT pair. Lock acquisition is bounded by cancellation or 30 seconds at consumedfence_linux.go:49; that is not a bound on critical-section duration. A root timeout skips installation, while a failed carry prevents transfer and permits retry. Apart from the permissions issue below, I found no new permanent refusal.

Simulation

The fence is modelled faithfully as a serialization abstraction. The early publisher runs before the carry; the late publisher encounters the production publishGate. The completion watermark is now sampled after either ordering: updatesim_test.go:753, :1049.

This does not test flock mechanics, process death, permissions or lock duration. The Linux tests separately exercise publication paused before lock acquisition and while holding the lock: consumedfencelinuxtest.go:128, :163.

The scripted disagreement initializes a production-reachable state at updatesim_test.go:280. A returning box can briefly resolve R from a known stale registry copy lacking the durable resolution; a subsequent refresh exposes {R,b3} while its consumed file retains {R,b1}. A pinned execution of b2 explains LastExecuted=b2 with R still pending. The test verifies recovery from this disagreement; it does not generate or prove the preceding history. That is a valid regression check for resolution precedence.

New findings

1. P2 — A restrictive root umask creates a lock file the agent cannot open.

Location: consumedfence_linux.go:24.

Scenario: The consumed directory already exists with normal permissions. An operator runs root adoption with umask 077. OpenFile(..., 0644) creates the new lock as root-owned 0600. Root successfully publishes, but the unprivileged agent later receives EACCES at lockConsumedCarry’s os.Open. Every carry retry fails, leaving the move unable to complete until permissions are repaired.

Later root passes do not repair the mode because opening an existing file ignores the creation-mode argument. Unlike this lock creation, the consumed-file writer explicitly applies its requested mode through safefile/file_linux.go:182.

Smallest fix: Explicitly apply the intended agent-readable permissions to the opened lock descriptor, closing it on failure. Preserve the inode; do not replace a lock file atomically. Add a restrictive-umask regression test. This is a nonblocking P2 because it requires a non-default execution environment.

Rulings

• R-PLANNED-RECOVERY: Agree.
• R-CARRY-FENCE: Agree with the synchronization design. The permissions issue remains, and the 30-second bound covers acquisition, not the bucket work while holding the lock.
• R-M9-SCRIPTED: Agree. The constructed state is reachable, and testing its resolution precedence is useful. It does not establish random-schedule coverage of that state’s creation.

Validation was code inspection. The focused Go tests could not start because the read-only sandbox refused Go’s work-directory creation. I did not independently reproduce the reported test, cross-build or mutation results. No files were changed.