[Refactor] Rework the enablement / bring-up round lifecycle - #1409
Open
ZhengGong-amd wants to merge 26 commits into
Open
[Refactor] Rework the enablement / bring-up round lifecycle#1409ZhengGong-amd wants to merge 26 commits into
ZhengGong-amd wants to merge 26 commits into
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| """What a settled round leaves behind: forever, a grace, or nothing.""" | ||
| clock = virtual_clock | ||
| opened_at = clock.wall() | ||
| assert (await store.open("r", holder_task_id="t-1", lease_sec=_LEASE, now_unix=opened_at, request_id="q1")).ok |
| to prevent. Refusing the combination is what keeps it unrepresentable. | ||
| """ | ||
| clock = virtual_clock | ||
| assert (await store.open("r", holder_task_id="t-1", lease_sec=_LEASE, now_unix=clock.wall(), request_id="q1")).ok |
| """``held`` is how a later lifecycle call finds the round it must address.""" | ||
| clock = virtual_clock | ||
| assert await store.held() is None | ||
| assert (await store.open("r", holder_task_id="t-1", lease_sec=_LEASE, now_unix=clock.wall(), request_id="q1")).ok |
Comment on lines
+212
to
+221
| assert ( | ||
| await store.open( | ||
| "round-a", | ||
| holder_task_id="t-1", | ||
| lease_sec=_LEASE, | ||
| now_unix=clock.wall(), | ||
| request_id="q1", | ||
| join=_join, | ||
| ) | ||
| ).ok |
| async def test_renewing_extends_the_lease_without_invalidating_the_holders_settle(store, virtual_clock): | ||
| """A heartbeat is not a change of holder, so it leaves the fence alone.""" | ||
| clock = virtual_clock | ||
| assert (await store.open("r", holder_task_id="t-1", lease_sec=_LEASE, now_unix=clock.wall(), request_id="q1")).ok |
| async def test_a_handoff_advances_the_fence_and_the_old_holders_settle_is_rejected(store, virtual_clock): | ||
| """The fence names a holder, and only a handoff can change either.""" | ||
| clock = virtual_clock | ||
| assert (await store.open("r", holder_task_id="t-1", lease_sec=_LEASE, now_unix=clock.wall(), request_id="q1")).ok |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
CI E2E report — ❌ Failed
|
Hyperloom Forge E2E — ✅ Succeeded
|
Three primitives the bring-up work needs and the tree had no home for. `LadderStage` and `BootObservation` name how far a boot got and where it died, keyed by a stable `failure_digest` so two rounds' failures can be compared across processes and across runs. The existing `FailureSignature` classifier answers what broke; neither it nor its in-process identity tuple survives a process boundary, which is what a durable ledger needs. `Deadline` carries an absolute monotonic instant rather than a remaining duration, so a budget that crosses the subprocess bridge cannot be re-anchored by the crossing. Monotonic because a wall-clock adjustment must not expire a round. `proctree` collects a descendant set before signalling, so a child that left its process group with `setsid` is still reachable. It replaces per-call-site `/proc` walks in `_subprocess_kill` and the kernel-forge submitter.
Bring-up exclusion was a string field on in-process state. It was set before the work it guarded existed, cleared on paths that could be skipped, and read by the gate as authority; a holder that died left it set with nothing able to observe that. The round becomes a row acquired by compare-and-acquire under `BEGIN IMMEDIATE`, carrying a fencing token, an owning task id and an outcome, settled through an append-only outbox. Exclusion outlives the holder on purpose: a confirmed kill still holds the cards for a grace window, and a kill that could not be confirmed holds them permanently rather than handing them to the next round. The `leases` table cannot express that. It keys on `(lane, holder_id)` with a TTL and is swept when the TTL passes, and it has no fence, no outcome and no exclusion that can outlast its row. It is still the round's clock: an open round holds a lane row for as long as it is open. `create_in_cursor` lets the holder task row be written inside the same transaction that opens the round, so neither can exist without the other.
The gate read a mutable latch and probed the environment while deciding. A stale latch was therefore indistinguishable from a live one, and it was the sole authority denying the action that would have cleared it. It now reads an advisory projection built once per tick from the round rows and the live task set. A resource or liveness denial is still a denial, but it is labelled advisory and dated, so a stale snapshot cannot be the only thing standing between a session and its exit. The GPU pool helpers move out of the gate for the same reason: they read env and were called mid-decision. `env_grants` admits a blocked environment name for one round by name and value. A framework installed outside the default prefix needs `LD_LIBRARY_PATH`, which the untrusted-name filter blocks wholesale; the grant set is derived from the existing blocked-name sets rather than restated, so a credential can never become grantable by omission.
A boot failure was re-derived wherever it was needed, so the before and after halves of a comparison could disagree about what happened. The classifier is unchanged and is reused: `ladder` maps its failure kinds onto the boot ladder rather than restating them. What is new is that the verdict is written once as an artifact and read back by path, and that a tree identity (`trees.json`) is resolved at session start so a digest taken in one round is comparable with one taken in another.
An unrecognised server flag failed forty minutes into a benchmark, as a generic nonzero exit. A missing checkpoint or an occupied port failed the same way. Server argv is sealed and digested by one finalizer at the end of the composition pipeline, then validated against the framework's own parser in the interpreter that will serve, so an unknown flag is a named terminal. One repair is allowed: a flag the parser rejects is dropped and re-checked once. The environment checks are separate and cheapest-first: checkpoint path, port, framework import. A probe that cannot answer returns unavailable, not a fault, because inability to observe is not evidence.
A round that died left processes holding GPUs, and the next round launched into them. The kill was a process-group signal, which an unprivileged child leaves at will, and nothing recorded what the kill had established. A reap now reports its outcome and what that outcome proves: the descendant set is collected before signalling, and a success covers what was enumerable rather than claiming the tree is gone. Host devices are claimed under `flock` by a descriptor the launched child inherits, so a second session on the same host cannot launch onto cards this one holds. A launch that finds them held comes back with a named class rather than as a failed variant.
The stall terminal was armed by a classifier that compared two failure signatures and reported "advanced" whenever they differed. A specialist producing a new-looking failure each round reset the streak indefinitely, so the terminal that existed for exactly that case never fired. A round now carries a monotone allowance that only decreases. A boot that reaches a stage the session has already seen is charged; one that reaches a new stage, or produces a digest never seen before, is not. An exhausted allowance tightens the next dispatch's timeout rather than removing it.
A coordinator whose loop is wedged cannot run the repair pass that would observe it, and cannot be asked to stop through a channel that needs the loop to run a callback. A separate process reads a tick stamp and, when it stops advancing, sends the one signal that reaches a busy interpreter; a stop-signal drain on the wakeup pipe records the arrival without the loop having to run. It never opens the session database, whose journal mode is unsafe with a second writer on a network filesystem, and it never transitions round state while the coordinator is alive. Its environment is scrubbed of control-plane credentials: it calls no model, and its `/proc/<pid>/environ` is readable.
The self-heal ran inside the pump it was meant to heal, behind a cadence guard, a positive-tick guard and a modulus. The states it repaired are exactly the ones that stop the dispatcher, so it was gated on the thing it was fixing. It is now an independent pass at the top of the tick with no condition on phase, budget or state, and with per-rule isolation so one failing rule cannot skip the rest: an expired round, a holder whose process is provably gone, a review nobody answered, a settle the store rejected, a revalidation window whose task will never report. The advisory projection the gate reads is rebuilt from whatever the rules leave behind, and this pass is now the loop's only sweep of the lease table, because a round's expiry and its lane's expiry are the same fact.
Deciding whether a combination boots meant running a full baseline, because booting and measuring were one action. `boot_probe` boots, waits for health, runs one short completion and tears down, behind the same launch path a measurement uses so the two cannot disagree about how a server is started. The launch boundary becomes a protocol, which is what lets a scripted backend stand in for it without patching subprocess. Baselines persist a `BootObservation` for every attempt, so the before half of the next round's comparison is an artifact rather than a re-reading. A readiness wait also ends when the server process is already gone, instead of polling a dead pid for the full window.
Applying a patch asked whether its hunks fit. A tree that had drifted, or an artifact copied from a stale source, satisfied that question while holding different bytes than the ones the round was graded on. A pre-round baseline records what each declared target held before the round, for git and non-git trees alike, because a pip-installed framework has no git to diff. Patch post-images are hashed where the work was validated and frozen beside the patch, so the apply site can ask whether the tree holds those exact bytes. The non-git path also drops `patch`'s default fuzz, which let a tree that merely resembled the post-state read as already-applied. Backups are recorded in an fsynced ledger rather than an in-memory list, so a process that dies mid-apply leaves a revert that is still possible.
The lane's admission test was a string it had written itself, and its rearm was the only thing that cleared it. Every path that reached a terminal without passing through the rearm left the lane closed. The lane now opens a round, renews it while its holder works, hands it to the successor that owes its result, and settles it with an outcome. Admission is a query against the round rows, so a holder that died is visible to the repair pass rather than only to the lane that lost it. A refused argv and an environment fault are terminals the lane recognises rather than launch failures it retries.
The guard that protects a not-yet-enabled run from a premature close read inputs that one path sets and several clear, and it dropped the escalation that was the only exit such a run had left. Nothing bounded it and nothing reconciled its inputs, so a missed clear made it the sole authority refusing a session its terminal. It is now bounded, and the count is persisted so a resume cannot reset it and latch again. Budgets accumulate forward per leg rather than being re-anchored, deadlines cross the dispatch boundary as absolute instants, and the params build runs under one so a hang there cannot hold the tick.
The round lifecycle was reconstructed in the report from whatever the log had said, and the stop-reason vocabulary existed twice with the copies already drifted apart. The ledger is read from the round rows and reported as a section of `enablement`, which is carved out of the schema's stability guarantee because it describes a runtime being built rather than an optimization result. The vocabulary has one definition; the CLI's exit code reads the same set the report grades against, having previously kept its own. New terminals are registered in it, and the fields the ledger replaced carry a disposition.
A multi-tick test of the round lifecycle could not be written: the outcomes it needed to script are produced by subprocesses, and the timeouts it needed to cross are wall-clock. The rehearsal harness scripts the launch boundary and the specialist, and installs a clock at the `time` module so a deadline evaluates against it without any caller having to accept an injected instant. It is excluded from the wheel; nothing shipped imports it.
Prose only: the module and start-up shim explained their rationale at essay length. No behaviour changes; the probe's tiers, switches and inertness are unchanged.
Package data and wheel checks for the new asset paths, the environment variables the bring-up path reads, and the contributor docs that name the layers this branch added.
ZhengGong-amd
force-pushed
the
refactor/zgong/enablement-rework
branch
from
September 5, 2026 12:12
5e5eb1e to
496edec
Compare
The lock existed so a second session on the same host could not launch onto cards this one held. That situation does not arise: the sbatch takes a whole node per job, and `resource_lock.py` already says Ray physically prevents card sharing and its own lanes are an accounting view. Every other exclusion in the tree is per-session because the lease database is per-session, which is what this module cited as its reason to exist. Removing it also removes the `-918` sentinel it was the only producer of, and the two consumers that classified it. The launch path is otherwise unchanged: the claim only ever contributed inherited descriptors to `pass_fds`. KernelForge's own campaign device lock is a different mechanism for a different purpose and is untouched. Co-authored-by: Cursor <cursoragent@cursor.com>
The preflight runs on every enablement tick, including the ticks while a targeted build it asked for is still queued. A framework that does not import yet, an extension with no loadable build, and a port an orphan still holds were all faults, and a fault ends the session on first sight. So the lane could ask for a build and then kill the run before the build ran, on exactly the symptom the build existed to fix. Installing frameworks and building extensions is what a round does. Those three are now unavailable verdicts: named, logged, not acted on. A checkpoint path this host does not hold is the one condition outside every lever the loop owns, and it stays terminal. Also drops the two session paths for the supervisor's directive channel. The supervisor only ever sends a signal; nothing writes a directive and nothing reads a cursor, so the paths described a protocol that does not exist. Co-authored-by: Cursor <cursoragent@cursor.com>
The three resource rules read a frozen snapshot through a mutable ledger and raised a bespoke denial type carrying the instant the snapshot was taken. None of that indirection earned its place: the repair pass writes the facts at the top of a tick and rounds open in the enablement pump at the end of one, so nothing changes between the write and the reads, and a frozen box inside a mutable box protects against a race that cannot happen. Nothing branched on "advisory" either -- the word appeared in a message string and every consumer caught `PolicyDenied` alike. The facts themselves stay. The round store and the task registry answer asynchronously while the gate is synchronous, and the pool sizes reach `rocm-smi` on a host with no visible-device mask, which is not a call to make once per validated intent. So the three rules now raise `PolicyDenied` inline, the way the other twenty do, and read a plain mutable `ResourceFacts` the pass updates in place. The test file follows the concept it tests; the check that no validator can reach a database call follows with it. Co-authored-by: Cursor <cursoragent@cursor.com>
The constructor existed so three test call sites could stay one expression, which is not a reason for production API. They build and update explicitly. Co-authored-by: Cursor <cursoragent@cursor.com>
A settled round could keep the machine excluded after it ended: for a grace window when a kill was confirmed, and for good when it was not. The permanent arm also stopped the session outright. Nothing supports that. The repo already records that Ray's custom resources are the authoritative physical GPU mutex and that these SQLite lanes are "a scheduling / observability / accounting view ... not the truth source for GPU mutual exclusion" -- so a third layer guarding the same cards adds no guarantee. The grace was 90 seconds against a KFD lag the tree measures at 2 to 20, and `roofline._GPU_RECLAIM_SETTLE_S` already waits for it at the point of the next launch, which is where a wait for the allocator belongs. The permanent arm was worse than redundant. A process-group reap cannot prove a tree gone, and it is now the only reap unit, so "nothing confirmed the holder dead" is the ordinary answer rather than an emergency -- and it ended the run. No other lane does this: the framework specialist, the GPU specialist and the serving lane all expire on a TTL and are swept. Admission now reads state and lease, so a round holds the machine while it is open and its lease is live, and a round nobody settles frees itself. Drops `exclusion_permanent`, `exclusion_until`, `kill_confirmed_unix` and `reap_grace_sec`, the `bringup_round_unreaped` terminal, and the settle-time `CASE` that computed the window. The reap outcome still records whether the kill was confirmed; nothing acts on it. Co-authored-by: Cursor <cursoragent@cursor.com>
`env_grants` admitted a blocked environment name for one round. It has no bearing on the round lifecycle this branch exists to fix: the deadlock was a leaked marker, and a framework installed outside the default prefix is a packaging problem. The blocked-name sets it derived from already live in `common/env_safety`, so what it added was the grant mechanism itself, on a security boundary, unreviewed as such. `RoundStore.events` and `RoundStore.stage_high_water` had no production caller. The breakdown collector opens its own read-only connection and recomputes both from raw SQL, so the store's readers only ever served their own tests. The outbox rows they read are still written and still consumed, by `redrivable_settles` and `observations`. Co-authored-by: Cursor <cursoragent@cursor.com>
`provisional` was a column and two parameters no caller ever set. The other three -- `correctness_verified`, `probe_origin`, `reap_backend` -- were written at open and settle and read by nothing but the breakdown collector, which reaches the table with its own SQL. They were threaded through two signatures, the row decoder, the dataclass and the schema to reach a report. `stage_high_water` stays: the progress budget decides on it. Co-authored-by: Cursor <cursoragent@cursor.com>
Comment on lines
+144
to
+152
| assert ( | ||
| await store.open( | ||
| "round-next", | ||
| holder_task_id="baseline-2", | ||
| lease_sec=_LEASE, | ||
| now_unix=clock.wall(), | ||
| request_id="req-open-2", | ||
| ) | ||
| ).ok |
Comment on lines
+160
to
+168
| assert ( | ||
| await store.open( | ||
| "round-1", | ||
| holder_task_id="baseline-1", | ||
| lease_sec=_LEASE, | ||
| now_unix=clock.wall(), | ||
| request_id="req-open", | ||
| ) | ||
| ).ok |
| assert row.excludes_at(settled_at) is False | ||
| assert await store.excluding(settled_at) == [] | ||
| # And the next round is admitted at once, on the same instant. | ||
| assert (await store.open("next", holder_task_id="t-2", lease_sec=_LEASE, now_unix=settled_at, request_id="q3")).ok |
Comment on lines
+81
to
+85
| assert ( | ||
| await store.open( | ||
| "next", holder_task_id="t-2", lease_sec=_LEASE, now_unix=opened_at + _LEASE + 1.0, request_id="q2" | ||
| ) | ||
| ).ok |
The reconciler re-sent settles the store had rejected. Nothing in this process can produce that rejection: production drives `run()` alone -- `tick()` has no production caller -- and its passes are awaited in sequence, with no `gather`, `create_task` or `TaskGroup` scheduling any of them together. The only task the loop layer spawns is a kernel-step heartbeat that never touches the round. A second coordinator is excluded by the session lock, and the supervisor does not transition round state while one is alive. So between the read that supplies a fence and the write that spends it, no other writer exists, and no test in the tree constructs a stale fence. The fence stays. It costs a column and a four-line predicate, and it is what would refuse a stale write if a pump were ever moved onto its own task. The rejection is still recorded on the outbox, because a guard nothing reports is a guard nobody knows fired. What goes is the 60 lines that acted on the rejection: the reconciler rule, the store's reader, and the index that served its query. Co-authored-by: Cursor <cursoragent@cursor.com>
Enumerating this branch's changes with a two-dot `git diff origin/main..HEAD` was wrong. `origin/main` had advanced past the branch's last merge point, so every file #1370 added showed up as a deletion and every file it modified showed up as a revert. The history rebuild staged those as if the branch had meant them, and reparented onto #1370 at the same time, which turned "the branch does not have these yet" into "the branch removes these". Restores `AGENTS.md`, `CLAUDE.md` and `.github/copilot-instructions.md`, and the ten files #1370 modified, to what main holds. `.gitignore` and `pyproject.toml` are three-way merged against the branch's real merge base: main's removal of the `CLAUDE.md` ignore line and its comment fix are kept alongside this branch's own edits. CLAUDE.md is a tracked file since #1370. It is no longer ignored, and the local-only copy that predated it is gone. Co-authored-by: Cursor <cursoragent@cursor.com>
ZhengGong-amd
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
49 sessions were reported stuck in PRELUDE (68 ended
enablement_stalledacross the wider1256-session corpus). The cause is one latch.
enablement_close_guard_active()suppressedskip_to_closewhile a run was not yet enabled, and every declared exit depended on somethingthat latch blocked:
baseline_tput > 0—baselineis denied by the latch, so throughput never becomes positive(40/49 hit
baseline:enablement_round_in_flight, longest streak 380).prelude_baseline_failed— a denied action never fails, so the counter never advances.enablement_stalled— the rearm that advances the stall streak never ran; all 49 end with anon-empty
inflight_task_idand a streak of ≤ 4.block for
max_turns × per_turn_max_seconds.skip_to_close— dropped by the guard; 18/49 emitted it, one 56 times, never routed.The latch was a string field on in-process state: set before the work it guarded existed, cleared
on paths that could be skipped, read by the gate as authority, invisible outside the loop, and
reconciled against nothing.
What changed
The round is a durable, owned row. Acquired by compare-and-acquire under
BEGIN IMMEDIATEwith a fencing token and an owning task id, settled through an append-only outbox. Exclusion is
bounded by the lease, so a round nobody settles frees itself and a dead holder is visible to
someone else. Ray's custom resources remain the physical GPU mutex; these rows are the accounting
view (
state/round_store.py).Reconciliation runs unconditionally, at the top of every tick. Previously the self-heal sat
behind a cadence guard, a positive-tick guard and a modulus, inside the pump it was meant to heal.
It is now an independent pass with per-rule isolation — expired round, dead holder, unanswered
review, stranded revalidation — so one failing rule cannot skip the others.
PolicyGate is a pure function again. It reads an advisory projection built once per tick from
the round rows and the live task set, instead of a mutable latch it also probed the environment
during. The close guard is bounded and its count persisted, so a resume cannot reset it and latch
again.
Preflight refuses an unlaunchable round before it costs a launch. Server argv is sealed and
digested by one finalizer, then validated against the framework's own parser in the interpreter
that will serve; an unknown flag is a named terminal, with one repair allowed. Environment checks
run cheapest-first. A missing checkpoint is terminal; a framework that does not import yet, an
unbuildable extension and a held port are unavailable verdicts, not faults — installing and
building is what a round does.
Boot outcomes are classified once and persisted.
BootObservation/LadderStagecarry astable
failure_digest; every baseline attempt persists one, and both halves of a comparison readthe artifact by path rather than re-deriving a verdict. A session-start tree identity makes digests
comparable across rounds.
Delivery is verified. A pre-round baseline records what each declared target held, for git and
non-git trees alike (pip-installed frameworks have no git to diff). The non-git apply drops
patch's fuzz and rejects an offset-only match, so a tree that merely resembles the post-state nolonger reads as already-applied. Backups go to an fsynced ledger, so a process that dies mid-apply
still leaves a possible revert.
Reap and deadlines. A reap collects the descendant set before signalling (a child that left
its group with
setsidis still reachable) and reports what its outcome actually proves, neverthat the tree is gone.
Deadlineis an absolute monotonic instant that crosses the subprocessbridge, so a budget cannot be re-anchored by the crossing.
Out-of-band supervision. A separate process reads a tick stamp — written off the loop thread,
since it fsyncs onto a possible network mount — and signals the coordinator when the tick stops
advancing. It never opens the session database (its journal mode is unsafe with a second writer
on NFS), never transitions round state while the coordinator is alive, and runs with control-plane
credentials scrubbed.
Testability. A rehearsal harness scripts the launch boundary and the specialist and installs a
virtual clock at the
timemodule, which is what makes a multi-tick round-lifecycle testwritable. Excluded from the wheel; nothing shipped imports it.
Protocol change
Session breakdown goes to v6.0. The round ledger is additive within it: a section under
enablement, which is already carved out of the stability guarantee. The stop-reason vocabularynow has one definition instead of two drifted copies, and the CLI exit code reads the same set the
report grades against.
enablement_stalledkeeps its slot for archived sessions but no longer hasa producer — the monotone attempt cap (
enablement_attempts_exhausted) is what bounds a sessionthat cannot boot. Field dispositions are in
docs/reference/session-breakdown.md.Testing
pytest src/hyperloom/orchestrator src/hyperloom/common: 755 passed.ruff check/ruff format --check: clean.inference_optimizersuite has pre-existing sandbox failures unrelated to the branch(ambient
ANTHROPIC_API_KEY, a missing TraceLens asset, a wall-clock-sensitive budget test);they fail identically at base.
Run tests with an absolute
PYTHONPATH; a relativePYTHONPATH=srcbreaks every test thatspawns a subprocess from a different cwd and manufactures phantom failures.
Size
169 files, +17,348 / −2,979. Large for the problem.
audit.mdlists every commit with its purposeand where new code was written instead of reusing an existing path — read it before the diff.
Not done
sacct,dmesgor journal access here, so theexternal cause of the first stuck holder is inferred, not proven.