carlos / platform Public
ACCEPT WITH FIXES for a dormant merge; REJECT for flag-on. Keep CARLOS_SWAP unset. Several claimed closures remain incomplete, and some deferred “minors” are safety or recovery blockers.
Reviewed 5da1cc9b..78ef3b81 against rev17, the corrected plan, earlier findings and execution ledger. No files changed or tests run; test results below are reported evidence, not independently reproduced.
- High — T3/4/5: execution identities repeat after restart. — Flag
manager.go:881 starts wake and attempt counters afresh. Persisted markers retain identities such as w1. failures.go:654 treats the same build and identity as a duplicate, retaining its old timestamp and epoch.
Sequence: B executes and leaves a marker without a failure record; restart the agent; its first wake executes B again and leaves another marker. Both executions can be w1. An earlier signed retry still matches, and a rollback timestamp between the two executions can incorrectly authorize clearing the newer execution’s restriction.
Smallest fix: add a unique manager incarnation to execution identities, or persist their allocation. Test reopening the book with a new manager, including stale rollback and retry grants. N6 and rollback-provenance closure are incomplete.
- High — T9: the claims move launch gate excludes actual move rows. — Flag
claimmoveaccess.go:168 invokes the new gate, but rolloutdeclined.go:163 returns unrestricted when rollout.Managed(rt) is false. That predicate excludes held statuses.
Actual move probes require a held row: movesprobeowners.go:255. Their launch therefore bypasses an inherited hold-down on a declined claims box. The new test uses a ready row and misses this.
Smallest fix: evaluate restriction eligibility independently of transient move status, preserving move authorization separately. Test held landing/restoring rows against failure records and markers. Declining claims-on swaps does not justify ignoring existing restrictions.
- High — T8: signed retry epochs are not unique across books. — Flag
hostretry.go:21 signs account/app/host/build and a numeric epoch, without a book or restriction incarnation. failures.go:867 accepts numeric equality.
Independent books can assign the same epoch to different restrictions. Following relocation or recreation of the same host, an earlier valid grant can clear a later restriction within its 24-hour window. Root signing prevents forgery, but does not establish the claimed replay protection.
Smallest fix: sign and verify a persistent unpredictable restriction identity, or book incarnation plus epoch. Test two independent books with matching numeric counters. Also cover failed-save/crash counter reuse before declaring replay closed.
- Medium — T4/8: a successful marker-only retry records no retry boundary. — Flag
failures.go:871 removes a matching marker, but retryHostLocked returns without clearedLocked when no failure record exists.
A B execution begun before that successful retry can subsequently report failure and create a record/halt the cohort. It is not classified PreRetry. Retaining a safety marker for the late hold outcome is intentional; undoing retry ordering is not.
Smallest fix: advance the host clear boundary when a marker is actually removed, including without a record. Preserve no-op retry behavior. Test both scheduled and planned-wake late outcomes.
- Medium — T8: retrying a non-current build loses its canary entitlement. — Flag
manager.go:826 adds the host to B’s Retried set, but binds its queue retry only when the slot currently wants B.
If it wants A/C and later returns to B while B’s cohort halt remains, the host has a book exemption but no forced queue admission. Another host retry cannot recreate the entitlement because the record already says retried. App-wide retry or restart may recover it; the successful host retry itself does not.
Smallest fix: retain a host/build canary entitlement until that build becomes desired. Do not spend it on another build.
- Medium — T15: failed-wake cleanup can infer containment from a changed layout. — Flag
opset.go:329 skips host-empty proof when hostCgroupIsTheWakes returns false. Its classifier reads the current layout at opset.go:371.
A flat failed wake whose layout becomes fenced/conflicting before cleanup can release its lease without proving its descendants gone. Neither classification proves that the original launch had separate containment.
Smallest fix: retain the launch’s containment identity; at minimum, unknown/fenced/conflicting layouts must not count as proof. F6’s failed-wake closure remains conditional.
- Medium — T17: the new status tests change dependencies after starting readers. — Merge
status_test.go:270 and status_test.go:353 assign h.m.d.Act after h.pass() starts scheduler activity. Readers have no corresponding synchronization.
Smallest fix: install the probe adapter before starting schedulers, then attach its test state safely. Run those focused tests with -race.
Rulings. References below are line numbers in the execution ledger. “Conditional” means agree with the stated condition.
| Lines | Judgment and reason | |---|---| | 9, 10 | Agree: claims-on declines; it must not turn the flag into an outage switch. | | 11 | Conditional: root signing is right; findings 1, 3 and 4 prevent the claimed restriction binding. | | 12 | Agree: unresolved intents conservatively become failures. | | 13 | Conditional: schema-2 downgrade refusal is sound; deleting authoritative restrictions must not become routine recovery advice. | | 14 | Conditional: signed rollback provenance needs execution identities that survive restart. | | 15 | Conditional: coverage/replay work can be distinguished from code defects; real-box evidence remains a flag gate. | | 16 | Agree: one full sweep per relevant branch revision is reasonable; rerun after these gate fixes. | | 41 | Disagree: manager-local counters are not durable execution identities. | | 47 | Agree: explicit authorization covers missing-record waiting even flag off; document this additional behavior change. | | 49, 51 | Agree: rerunning the installed marked build uses the served-build exemption. | | 52 | Conditional: the accepted dispatch-time rollback tradeoff still requires unique identities and explicit clock-skew handling. | | 53 | Agree: adoption onto good must respect the standing marker. | | 58 | Agree: gate by execution cause, not queue class. | | 59 | Conditional: postponing restart/config has supported recovery, but its invisible waiting needs follow-up. | | 63 | Agree: cgroup recreation is an authorized flag-off exception. | | 65 | Agree: the end-of-Plan snapshot chooses the conservative ordering. | | 66 | Conditional: no-op retries must not advance ordering; actual marker removal is not a no-op. | | 67, 68 | Agree: late hold outcomes retain database safety; marker time must not regress. | | 70 | Conditional: existing scheduler-locked saves may remain; document their latency, including indirect activator-lock waits. | | 71 | Agree: unchanged records can reassert memory-only halts. | | 74 | Agree: intent conflict uses the existing accurate refusal. | | 75 | Agree: join after activator shutdown; the implementation shares the shutdown deadline rather than necessarily adding 60 seconds. | | 81 | Agree: abandon dispatch when ownership/status is no longer eligible. | | 90 | Conditional: sidecar gating is correct; retained-incumbent sidecar recovery remains unresolved. | | 91 | Agree: gate before stopping the serving process. | | 92 | Conditional: acceptable only for dormant merge; nested identity race remains Flag. | | 93 | Conditional: marker/path changes are sound; the move-launch fix fails finding 2. | | 97 | Agree: hashing has an identity-binding purpose on those paths. | | 98 | Conditional: seed bypass remains Flag, despite its narrow preconditions. | | 101 | Conditional: CLI recovery permits dormant merge; the explicitly owed browser surface remains Flag absent waiver. | | 105 | Agree: counting plans/wakes/coalesced execution closes shutdown gaps; caller cancellation must not poison shared policy cache. | | 110 | Conditional: clean one-shot exit is success, but good publication must still validate ownership/fencing. | | 115 | Conditional: populated-parent fix is useful; other uncapped fallbacks remain Flag. | | 116 | Conditional: manual recovery needs an accurate operator procedure; do not claim automatic lifting. | | 120 | Conditional: retaining seeding and box-condition classification are sound; preserve claim evidence and obtain kernel proof. | | 124 | Agree: retaining seeding falls within the authorized recreation exception. | | 127 | Agree for undelegated legacy operation only; leader exit is not descendant containment. | | 128 | Conditional: widening F6 is justified; finding 6 defeats its universal claim. | | 130 | Conditional: scope may defer implementation, but failed-seed and normal-stop proof remain owed Flag gates. | | 135 | Agree: a named unit test can kill M12; add restart-incarnation coverage. | | 138 | Conditional: transient reporting is right; remove finding 7 and exercise Owns in the lock probe. |
Closure assessment. The blanket statement that all flag-path code defects are closed is inaccurate. N6/provenance/retry ordering, declined move launches and F6 remain incomplete as above. Clean-one-shot publication also retains an ownership gap.
I found the intended implementations for startup intent reconciliation, uncertain nested-start treatment, bounded policy/read/ack work, shutdown joining, worker execution revalidation, GoodVersion retention, pre-stop hold publication, transient reporting and /run sweeping. The populated-parent and killed-cgroup fixes exist but lack real-kernel confirmation. The reported simulation sweep does not cover the restart/replay sequences above.
I found no new direct disk write under activator.Service.mu, demonstrated lock-order inversion, or unintended incompatible external wire shape. A declined gate can nevertheless wait under that mutex behind a book fsync. Local schema-2 downgrade refusal is intentional. New unsigned-retry rejection and missing-record waiting are explicitly ruled behavior changes beyond the four enumerated exceptions.
Deferred-item triage.
| Classification | Items | |---|---| | Merge | Finding 7; approve the three pending no-signing-key strings; correct overclaimed closures and misleading recovery instructions; protect T19’s duplicated claim-name format with a shared definition/test because recreation runs flag off; complete the normal gate on the final integrated tip. | | Flag | Findings 1–6; T8 failed-save/crash replay coverage; T9 nested identity race, ungated seeds and stranded legacy sidecar recovery; T10 grants consumed while rows are moving/landing; T12 good publication after fencing/lost lease and the duplicate-wake window; T14 all uncapped error fallbacks and positive-control kernel coverage; T15 failed-seed descendants and normal-stop proof; declined browser hold/retry surface unless waived; host-retry spec correction; mixed-version retry deployment sequencing. | | Defer | Claims-only intent/default-crash gaps until claims-on swaps; T8 overbroad epoch invalidation and API/browser no-key ordering; policy single-flight; harmless Pass-after-Close behavior; scheduling/log verbosity; extra hashing; kill-mark/lock-map growth; optional inode/reservation hardening; wording/comment/test-name cleanup; redundant kills/pins; stale documentation line numbers. | | Defer, retain explicit coverage debt | Remaining T1/2/4/5/6/7/9/10/12/13/16/17 unit/model omissions, rollback liveness/replay tooling, thin M8 detection and root-disabled I8. New regression tests for findings are not deferrable. Keep known flakes tracked; do not normalize a failing gate. |
T10’s delayed retryBuild concern is already addressed by T11’s onCleared callback. T19’s RecoverStopped concern is resolved by inspecting containment names: they include controller/boot/session identity. The pre-existing watch-finalized-as-asleep gap remains reporting debt and should not be presented as solved by transient status.
Pointer re-signing versus explicit rollback, timestamp skew, stale outcomes after pruning, effective-mode races and failed pinned-wake publication deserve explicit recovery tests before the flag gate; none establishes a new merge regression here. The unrequested git worktree prune recorded at ledger line 56 should also remain noted as a process deviation.
Remaining flag blockers. Code must satisfy the Flag list, with corrected closure claims and regression coverage, followed by the relevant simulation/mutant sweep. Claims-on swaps remain a separate future feature; claims-on boxes must continue declining.
Separately, obtain scoped approval and run the delegated arm64 kernel suite as User=carlos, including TestGenerationStartRefusedBesideADirectParentMember and TestClaimContainmentCleanupRespawn; verify killed-parent children, retained directories and systemd-stop behavior. Complete real-unit control/sidecar checks, dev rollout checks and the flagship probe gates. D1’s daemon-reload measurement is already reported complete. Every other owed gate requires evidence or Paul’s explicit waiver before flag-on.