carlos / platform Public
Verdict: REJECT
Reviewed b9057caf4d1094bb676501d1f709070313de9219. Six round-1 findings are closed. Request preservation remains incomplete, and the new pre-exec check breaks planner-approved recovery.
Round-1 findings
- CLOSED — Failed refresh selects a stale build. A failed wake refresh now returns
Held, Unreadat internal/host/buildstate.go:679, which the activator refuses. The pre-exec write also revalidates against the durable state inside its CAS loop at internal/host/buildstate.go:522. The original failed-refresh scenario no longer runs A over B’s state.
- CLOSED — Recovery copies can be replaced.
BoxPinsnow includes account/app-qualified settings references at internal/release/pins.go:137.SettingsRefsincludes the pin,LastExecutedand resolved target, and rejects incomplete held copies. Restaging respects the app-scoped incompleteness at internal/release/adopt.go:1999.
- NOT CLOSED — A graceful move can reopen a completed-failed request. The ordinary sequence is fixed:
CarryRequestpreserves failed completions at internal/release/buildstate.go:243, and the move paths call it before copying. However, root can publish another consumed record after the carry’s read. New finding 2 describes the remaining sequence.
- CLOSED — A major hold is bypassed before root decides. Root’s decision is now tied to the settings column at internal/release/updatetarget.go:269. A missing or mismatched decision returns
Held, AskRootat internal/release/heldwake.go:107, rather than establishing that the route is unheld.
- CLOSED — A failed installed build runs without a build-state object. The installed-build fallback now passes through the same failure check as
LastExecuted: internal/release/heldwake.go:80 and internal/release/heldwake.go:102.
- CLOSED — The local sweep accepts an incomplete recovery set.
SettingsRefsrejects a held copy withReadErrat internal/release/pins.go:221. The sweep returns before deletion when those references are incomplete at internal/host/versionsweep.go:81.
- CLOSED — Local recovery unnecessarily requires remote release objects.
stageAuthorizednow tries verified retained evidence after any unsuccessful remote path at internal/release/adopt.go:2285. Downloading or replacing bytes still requires the promotion record’s manifest digest.
New findings
- P1 — Pre-exec revalidation rejects the planner’s legitimate fallback.
Location: internal/release/heldwake.go:135, called by internal/host/buildstate.go:522.
Scenario: A manual instance has good build A and an update request resolved to T. Its overlap wake records T before execution, making LastExecuted=T and completing the request. T then fails its health check. The activator proves T stopped and starts the planned fallback A at internal/activator/wakeplan.go:399.
That fallback must record A through internal/activator/swaphold.go:384. The new predicate rejects it: the manual target and durable LastExecuted are T, A differs from both, and root has not reported T failed. The failure-book update happens through the subsequent outcome handling, including internal/rollout/wakeplan.go:401.
This is more than a narrow race with root. It interrupts the existing overlap fallback sequence itself. Later recovery can also remain blocked while the planner knows the failure but root’s consumed file does not. It violates §6’s preservation of scheduler, swap and fallback behavior.
Smallest fix: Carry the planner’s checked recovery/fallback selection into pre-exec revalidation, allowing that specific transition while still requiring the durable write. Do not exempt every GoodVersion: that would reopen stale-build execution. Add an integration test using the real build-state hook for both immediate overlap fallback and recovery before root records the failure.
- P1 — The carry does not fence root’s consumed-file publication.
Location: internal/host/buildstate.go:542, internal/host/movescut.go:989, and internal/release/adopt.go:2222.
Scenario: Root has already selected a ready route with pending request R and failed head T. The cut marks the route moving and carries the consumed file before root publishes its decision. Root then writes R as completed-failed. The cut seals the earlier build state and transfers it. On the destination, R remains above the watermark and resolves again—potentially to a newer head H.
Root’s route eligibility was checked against its earlier snapshot at internal/release/adopt.go:997. The build-state mutex and CAS do not serialize root’s separate consumed-file write. A successful mirror therefore cannot detect this lost completion.
This is a graceful transfer, not a lost box. Calling the late record a “lost request” would reinstate the exception rejected by R-SPEC-43A-2.
Smallest fix: Serialize consumed publication with entry into the move hold. Root must either finish publication before the carry or reject publication after the hold becomes effective. Use a shared cross-process fence/lock and recheck eligibility within it; merely rereading the file before sealing leaves another race. Cover a root pass paused immediately before WriteConsumed, including failed completion.
Simulation fidelity
The simulation now calls the production decision function at internal/release/updatesim_test.go:1091, and the production pre-exec predicate at internal/release/updatesim_test.go:1153. It also permits stale known copies. Those fix the original blind-live-link model.
It does not execute the same complete enforcement path as production. Its own switch models the no-planner wake; it cannot exercise planner recovery or overlap fallback.
The move model also samples the completion watermark before its racing root callback: internal/release/updatesim_test.go:611. A failed completion published by that callback is excluded from the carried watermark. Consequently, the reported 3,000-seed pass does not establish either outstanding guarantee.
Judgment on concerns (a)–(e)
- (a) Wider holds/refusals: Acceptable as conservative admission when freshness or the applicable root decision is unavailable. “Within the wake’s normal bound” does not require running an unvalidated build. The root-decision wait shares the held-wake timeout at internal/activator/wakecheck.go:102. Manual retained recovery could avoid some unnecessary waits, but I would not block this wave solely for that cost.
- (b) Planner recovery: A defect; new finding 1.
- (c) Pre-install return: No independent defect established. Before install/probing, returning the forward seal’s exact state is appropriate; internal/host/movesrollback.go:834 selects that branch. It still depends on fixing the forward carry race.
- (d) In-flight root pass: A defect; new finding 2.
- (e)
ReadErrmeans unread: Correct. internal/release/updatetarget.go:90 prevents stale request bookkeeping, while retention preserves known references and refuses incomplete sets.
Authorization and execution recording
I found no wave-introduced route to install bytes unauthorized by §4.1. Remote staging remains digest-bound; retained recovery verifies the executable against its local sibling at internal/release/staged.go:150.
I also found no wave-introduced bypass of the mandatory pre-exec record. The recovery defect refuses a required execution; it does not execute without recording.
Rulings
I agree with the substance of R-SPEC-43A-2, R-INV1-2, R-HELD-PURE and R-READERR-UNREAD. R-ASTRA-R1-FIX is not fully satisfied. I disagree with treating a completion left behind by an in-flight root pass during a graceful cut as the spec’s lost-box exception.
Validation was code inspection. The focused Go test command could not start because the sandbox refused creation of Go’s work directory. I did not independently reproduce the reported test or seed-sweep results. No files were changed.