fix: hold the session to its wall-clock budget

[zoroyihan7]

Refs #1146. Also fixes #1167.

Scoped to #1146's item 3 (in-flight enforcement) plus the admission gate; items 1, 2, 4 and 5 of that issue are not all closed here, so this does not auto-close it.

A run given a 3h budget spent 3h55. The budget was consulted between ticks and
nowhere else, so the only question ever asked was "should the next tick start?"
-- never "may this action start", "how long may it run", or "is it still worth
waiting for". An explore could be granted four hours inside a three-hour
session and nothing would stop it.

Four defences, each covering what the one before it cannot.

Admission. An action whose expected cost cannot fit the budget that is left
never starts. Refused before a task row exists, so the refusal stays out of the
failure ledgers -- a reaped attempt used to teach the KB that the action fails
when all that happened was the run ran out of time. Fit is judged on expected
cost (p50), not the p75 tail: judging on the tail abandons minutes that would
have been used more than half the time, and overruns are what the later layers
are for. The closing actions stay startable on an empty budget.

Timeout clamping. Action and variant grants are clamped to the budget that
is left, so no single grant can exceed the session. The stack rebench never
consulted the budget at all -- the same hole, second instance.

Session reaper. run_with_session_kill gains a session deadline on its own
channel, checked in every phase and never suspended. The separate channel is
the point: the soft deadline retires at eval start by design, because it judges
whether a variant is abnormally slow, and that boundary is meaningless for "is
the run out of time". An accuracy eval starting a minute before the session
ends used to run to completion.

In-flight cancellation. The backstop for an action already running whose
executor takes no deadline, or that is not spending its time in a subprocess.
The handles lived only in the pump's frame, so the sole caller able to cancel an
action was the frame already blocked awaiting it; they now live on the
dispatcher. This is also what makes SIGTERM work: it only set the stop event
before, and since the pump does not return until everything it dispatched has
finished, shutdown latency was the remaining runtime of the longest action --
after which teardown closed the database out from under work still using it.

Spending the budget early is the other way to lose it

Field data from two sessions on unrelated models -- different quantization,
different PRELUDE composition, one baseline-dominated and one split with a
TraceLens roofline -- showed the four layers above would not have saved either
run. Neither overran at the tail. Both spent roughly three quarters of a
three-hour budget (73.8% and 72.8%) in PRELUDE, whose declared share is 3%, and
both reached FRAMEWORK_AGENT with about 47 minutes against its 108-minute entry
threshold. They were unrecoverable the moment preparation ended.

Nothing enforced the share because exit_normal_prelude tests whether a
baseline landed and nothing else, so the phase ran to whatever its contents
happened to cost. PRELUDE now decides before it spends: prelude_can_afford
bounds an optional arm by the tighter of its own share and what the
optimization phases have reserved, and two arms consult it -- the initial
roofline/profile (81 minutes in one run, 45% of the session, and known-degraded
at second zero because the vLLM version patch had failed) and the baseline's
measured round (59 minutes in the other, to move the anchor 0.2%; dropping it
leaves the single-round baseline the codebase already supports rather than a
new half-measured state). Both estimates come from what the session itself
measured, because the catalog's p50s are calibrated on small models and a guard
reading them would have admitted both arms.

time_exhausted_during_prelude also finally gets a producer: it was in the
terminal-reason vocabulary and in the report's glossary, but nothing ever
assigned it. And PRELUDE's normal exit now states whether the budget it leaves
behind still funds one benchmark round.

A report the session can still write (#1167)

CLOSE reached for a report task by asking "was one enqueued?" when it needed
"is one runnable?", so a cancelled row came straight back and run_task's
queued -> running failed the registry's terminal check with IllegalTransition.
The session that hits this is the one whose report matters most, and it lost
final.md entirely.

This lands with the enforcement layers on purpose: cancelling in-flight work at
the deadline is the point of those layers, and it widens the set of ways a
report task can be sitting in cancelled when CLOSE arrives. A dead row now
falls through to a fresh enqueue under a suffixed idempotency key, and
_run_close_task reports a terminal row instead of handing it to run_task
again.

Two bugs found on the way

The new sentinel returncode first landed on _RAY_ACTOR_DIED_RC, which would
have had every Ray actor death read as a spent budget. The guard test written
to catch that then found a pre-existing overlap: AGENTX_PREFLIGHT_RETURNCODE
and _ACTOR_TIMEOUT_RC were both -912 and both leave _run_magpie through the
same return channel, so a hung actor was recorded as a missing aiperf. Fixed
here as well -- it is the same attribution problem these layers exist to solve.
The Ray code moved rather than the AgentX one, because downstream and every
record written so far already read -912 as the preflight failure.

A field run also crashed on AttributeError: _record_explore_variant_failures,
a _DELEGATED entry naming a method that no longer exists. The test added here
requires every delegated name to resolve on its collaborator; it found two more
stale entries (_resolve_issue_canonical, _wait_for_task_terminal), both
removed. These are now caught at test time rather than mid-session.

Test plan

• test_session_time_budget.py -- 43 new tests: the fit rule, the usable
  budget accessor, the dispatcher gate, all three intent paths, the
  pre-dispatch backstop, the in-flight handles, the cancellation and its
  closing-action carve-out, the lane released on cancel, the task row
  landing terminal, and the pump / shutdown / Coordinator.stop triggers.
• Session reaper and sentinel uniqueness guards in
  test_kill_spawned_server.py; clamping in test_explore_executor.py,
  test_grid_runner.py, test_framework_agent_executor.py and
  test_integrate_patch_coverage_unit.py.
• PRELUDE spend policy in test_phase_state_machine.py (affordability
  bounds, the time-exhausted exit, a landed baseline outranking the guard,
  and the viability sentence on normal exit), the analysis arm in
  test_warm_replay.py, and the measured round in
  test_baseline_warmup_double_run.py.
• Report-task state guard and the delegation-map check in
  test_close_phase_sequencer.py and
  test_coordinator_async_batch2_unit.py.
• Full non-e2e suite: 13193 passed. The 16 failures reproduce unchanged on
  a clean tree (subprocess tests needing an installed package) or pass in
  isolation (cross-test pollution).

Inline actions

run_action_now abandons its future once the inline wait elapses -- the action
keeps running by design so the agent's turn is not held hostage -- which
abandoned the only handle on it. It now publishes its own task for the duration,
so the same cancellation reaches it. Its sync bridge also names cancellation
before its generic handler, which had been filing a deliberate stop as
errored.

[zoroyihan7]

Pushed two commits that extend this PR at the head end. The field data on #1146 — now two independent sessions — showed that stopping on time is necessary and not sufficient, and one of them showed that the enforcement layers here make an existing crash easier to hit.

8c2cdef94 — the CLOSE report guard (#1167), first on purpose

close.py reached for a report task by asking "was one enqueued?" when it needed "is one runnable?". Both reuse paths read task.state only in order to log it, so a cancelled row came straight back and run_task's queued -> running failed the registry's terminal check. The session that hits this is the one whose report matters most, and it lost final.md outright.

This has to land with the enforcement layers rather than after them: a1074febe adds an asyncio.CancelledError handler that transitions in-flight tasks to cancelled, which is exactly the point of the in-flight layer, and it widens the set of ways a report task can be sitting in cancelled when CLOSE arrives. Merging without the guard makes the crash easier to hit, not harder.

A dead row now falls through to a fresh enqueue under a suffixed idempotency key — shared by the report and session_breakdown steps rather than written twice — and _run_close_task reports a terminal row instead of running it, so no path can hand one to run_task again.

While adding the delegation entries this needed, I added the test that would have caught a field-run AttributeError on _record_explore_variant_failures (map entry present, method absent, crash only when the reap loop reached for it): every _DELEGATED name must resolve on its collaborator. It found two stale entries on main, _resolve_issue_canonical and _wait_for_task_terminal, both naming methods that no longer exist anywhere. Removed.

9433b3937 — PRELUDE stops spending the optimization phases' budget

The four layers already here bound the tail. Neither field session was lost in the tail.

PRELUDE's budget share is 3%, and nothing enforced it — exit_normal_prelude tests whether a baseline landed and nothing else, so the phase runs to whatever its contents cost. Two sessions with nothing in common (different model, quantization, and PRELUDE composition — one baseline-dominated, one split with a TraceLens roofline) spent 73.8% and 72.8% of a three-hour budget there, and both reached FRAMEWORK_AGENT with ~47 minutes against its 108-minute threshold. Apply this PR as it stood to either run and you get a session that honours its three hours and delivers a report about a baseline.

So PRELUDE now decides before it spends. prelude_can_afford bounds an optional arm by the tighter of PRELUDE's own share (40%) and what the optimization phases have reserved (50%), and two arms consult it:

• the initial roofline/profile. In one run it took 81 minutes — 45% of the session — and its own log declared the result degraded at second zero, because the vLLM 0.26 patch had failed (see vLLM version specific patch application; use new profiler configs #1157).
• the baseline's measured round. In the other it cost 59 minutes to move the anchor 0.2% (cold 514.4 vs hot 513.5 tok/s). Dropping it leaves the single-round baseline this codebase already supports, not a new half-measured state.

Both estimates come from what the session measured, not from the action catalog. That is the part I would most like a second opinion on: the catalog says baseline costs 5 minutes and roofline 8, while these runs measured 51 / 125 and 81, so a catalog-anchored guard admits both arms and the whole thing is decorative. The analysis arm boots its own server and runs the same benchmark under a profiler, so I use one measured baseline round as a floor on its cost; the measured round re-attaches to a hot server, so I use the warmup's own runtime as an upper bound on it.

Two smaller things in the same commit:

• time_exhausted_during_prelude finally has a producer. It was in the terminal-reason vocabulary and in the report's glossary, but a search over src/ finds nothing that ever assigned it — the state machine had a word for this failure and no way to reach it.
• PRELUDE's normal exit now records whether the budget it leaves behind still funds one benchmark round. That is the plain sentence neither field session ever got; instead each later phase declined in turn, each for its own local reason.

Not in this PR

The progress-visibility work (#1145, #1168) is going in separately — same root cause as each other, none with this one.

[zoroyihan7]

/retest

The e2e run for 9433b3937 was cancelled 43s after it started, by the comment posted alongside the push — the same ci-e2e concurrency defect that has now eaten four runs across this PR and #1161. #1173 fixes it; until it lands, /retest is the only comment on a PR that is safe while a run is in flight, because it is the only one whose group membership is intended.

Verified the two new commits locally: 331 passed across test_close_phase_sequencer, test_phase_state_machine, test_baseline_warmup_double_run, test_warm_replay, test_coordinator_async_batch2_unit and test_session_time_budget; 179 passed on the two suites carrying the _DELEGATED guard; ruff clean on all five changed modules.

[chaojhou]

Blocking items only. Everything below is verified at PR head against the code, not read off the description.

1. Layer 4 cancels the coroutine; the work keeps running

Every benchmark executor blocks in asyncio.to_thread, and a thread that has already started cannot be cancelled. cancel_inflight_actions gets a clean CancelledError on the await while run_with_session_kill runs on to its own timeout=.

                proc = await asyncio.to_thread(
                    run_with_session_kill,
                    cmd,
                    env=env,
                    cwd=str(output_dir),
                    timeout=timeout_sec,

Same shape at baseline.py:2811 and _grid_runner.py:1626, :1851, :1886. Consequences: Coordinator.stop() cancels, gathers a clean return, then closes SQLite — the exact thing the new docstring says it prevents, with the thread still running; the finally blocks release the GPU specialist lease and the lanes while the benchmark still owns the GPU; and SIGTERM latency is unchanged, since concurrent.futures.thread joins workers at interpreter exit. Where work does stop today is run_grid's local path, and only because layer 3 fires inside the thread — that is layer 3 doing layer 4's job.

The whole cancellation suite passes because _never_finishes is a bare await asyncio.sleep(3600), which no real action resembles. An executor doing await asyncio.to_thread(time.sleep, 30), asserting the thread is gone before cancel_inflight_actions returns, fails today and is the test this layer needs.

2. Layer 3 is dropped on the Ray serving path, which is the single-node default — and the clamp then manufactures false timeouts

_run_magpie returns through the lease and session_deadline_sec is not among the arguments:

        return serving_lease.run_session_kill(
            cmd,
            env=env,
            cwd=cwd,
            timeout=timeout_sec,
            soft_deadline_sec=soft_deadline_sec,

ServingLease.run_session_kill has no such parameter (_ray_serving.py:385-395), and it cannot trivially gain one: session_deadline_sec is a time.monotonic() instant, meaningless in the actor's process. The representation blocks propagation, not just the plumbing. With INFERENCE_OPTIMIZER_RAY_EXEC unset on a single node, the lease is created (_ray_backend.py:52-61), so this is the default path.

The failure is worse than "no reaper", because the clamp still applies. After a warmup that overran, _round_timeout_sec returns max(1, negative + 15) = 1 second (_grid_runner.py:1392-1393); the round is granted 1s, the lease converts the actor timeout into TimeoutExpired, and run_grid files it as a verdict about the variant:

                    status="failed",
                    error=f"timeout: {exc}",
                    error_class="magpie_timeout",

That is the "ledger full of spurious timeouts" the clamp's own docstring exists to prevent. The test pinning the clamp (test_grid_runner.py:1635) patches run_with_session_kill, so it only covers the local path.

3. baseline — the hole #1146 names as the largest — is covered by none of the four layers

BASELINE_DEFAULT_TIMEOUT_SEC = 7800  # WARM-start cap, 130 min
BASELINE_COLD_START_TIMEOUT_SEC = 9000  # COLD-start cap, 150 min (includes ~20 min cuda graph capture)

Admission admits it on a catalog cost_minutes_p50 of 5.0; _resolve_timeout (baseline.py:995-1074) never consults the budget; run_with_session_kill is called without session_deadline_sec; and it blocks in to_thread. session_deadline_sec appears nowhere in baseline.py. What the PR adds is a PRELUDE affordability gate on the second round only (baseline.py:2211-2231) — the 3941s MiniMax warmup that motivated the work, 36% of a 180-minute budget, stays unguarded. profile.py (PROFILE_DEFAULT_TIMEOUT_SEC = 14400, longer than the whole budget) is not in the file list at all, and #1146 names baseline and profile together as the two largest holes.

Relatedly, layer 1 is anchored on the static catalog, which this PR's own code documents as an order of magnitude low:

        Anchored on this session's own baseline round rather than the action
        catalog. The catalog's estimates (``baseline`` 5 min, ``roofline``
        8 min) are calibrated on small models: the two sessions that motivated
        this guard measured 51 and 125 minutes of baseline and an 81-minute
        roofline, so a catalog-anchored guard admits an arm it cannot pay for.

The PRELUDE guard uses measured cost; layer 1 uses the catalog, and would not have stopped either field run. SharedState.baseline_runtime_sec is already in scope there.

---

Two coordination notes, since these are decisions rather than defects. Closes #1146 overstates what lands: issue items 1 (one deadline), 2 (baseline/profile), 4 (report timeout and a measured close reserve) and 5 (an end-to-end assertion) are not addressed — Refs would be accurate. And this branch conflicts with #1177: both add an import at baseline.py:33 and insert into BaselineExecutor at 2206/2207, and both modify the same lines of sub_agent_runner.py. Against #1178 the shared files (baseline.py, machine_state.py) touch disjoint regions and should auto-merge.

[github-actions]

CI E2E report — ✅ Succeeded

item	value
result	✅ Succeeded
model	Qwen/Qwen3-0.6B (dense)
resources	1× GPU, TP=1
PR branch	fix/1146-time-budget-enforcement
commit	f2c241cbc32ee9f020858c118de289ebe0ea29d5
session_id	1813b9e8-dfdf-485a-9a35-1388161ef5f2
queue → dispatch	0s
run time	168m 14s
total	168m 14s

details

[zoroyihan7]

All three blocking items are fixed, plus a fourth defect that surfaced while verifying them and would have been the worst of the set. Both coordination notes are accepted; one of them turns out to be understated.

1. Layer 4 now stops the work, not just the coroutine — a52e39122

The test you specified came first and was red: an executor doing await asyncio.to_thread(run_with_session_kill, ["sleep", 30], ...), asserting the thread is finished before cancel_inflight_actions returns.

orchestrator/actions/cancel_channel.py carries a CancelScope (a threading.Event, first reason wins, listener count) published through a ContextVar. The dispatcher opens the scope before the action takes off; asyncio.to_thread copies the context, so the 0.5s poll loop in _communicate_with_soft_deadline sees it at the same point it checks the session deadline, kills the process group, and raises a distinct exception landing on ORCHESTRATOR_CANCELLED_RETURNCODE = -917. -916 was taken by the Ray actor timeout; the collision-guard test caught that on the first attempt.

Two additions beyond the sketch. run_with_session_kill registers as a listener before Popen, so cancel_inflight_actions can tell work that can hear it from work that cannot — the former gets a 10s grace window, the latter is cancelled immediately, instead of every asyncio.sleep coroutine costing a shutdown 10 seconds. And the five identical sentinel handlers in _subprocess_kill collapsed into one _ReapedByWatchdog base, so a new stop reason no longer means a sixth copy.

To prove red/green rather than assert it, I stubbed CancelScope.cancel to a no-op (equivalent to the pre-fix behaviour) and re-ran: test_the_work_is_over_before_the_cancel_returns fails with "the cancel returned while the thread was still running", and the attribution test fails on a missing returncode.

Scope limit, stated rather than hidden: a ContextVar does not cross a process boundary, so Ray-managed work is not reachable by this signal and still relies on the remaining budget passed to it. test_code_outside_an_action_finds_no_scope pins that. Carrying orchestrator cancel through the Ray boundary needs an actor-level control channel — a separate PR.

2. The Ray serving path carries the session clock — d78e0ccc0

You identified the real blocker: the representation, not the plumbing. A time.monotonic() instant means nothing in the actor's process. So the deadline is converted at the boundary — session_deadline_to_remaining_sec() on the caller side, session_remaining_to_deadline_sec() reconstructing a local monotonic deadline on the worker — and the parameter is named session_remaining_sec so nobody is tempted to pass an absolute instant across.

Worth recording why the suite was green: _should_use_ray_backend() returns False under pytest, so every existing test exercised the local path. Added coverage drives a mock actor and asserts the forwarded remaining seconds.

3. Baseline and profile are covered — 4bd6bad2e, and layer 1 in 4b8502144

Three things together, each harmful without the others: _run_single_benchmark resolves the deadline per round (a baseline runs up to three, and an overrun warmup has already spent what the later ones assumed); session_clamped_timeout_sec clamps the 130/150-minute hang backstop to what remains; and a stopped_by_the_run branch attributes the sentinel returncodes to session_time_exhausted / orchestrator_cancelled rather than server_init_dead. That third one matters as much as the first: a reaped round leaves evidence identical to a server that never came up — no workspace, no report, non-zero returncode — so without it the ledger records budget exhaustion as a verdict about the model.

The clamp and the classifier were hoisted out of the grid rather than copied; _round_timeout_sec's body became the shared helper and grid kept only its own log line. ProfileExecutor subclasses BaselineExecutor and overrides only config parsing and the trace pipeline, so it inherits all of it — but it is covered as its own test arm (4-hour default against a tight budget) rather than on the strength of "it should inherit".

On layer 1 being catalogue-anchored: fixed, and PRELUDE's rule is now the only definition. expected_action_cost_minutes() takes this session's measured baseline and uses it as a lower bound for actions that run benchmark rounds — a grid is never cheaper than one round, and only the catalogue knows how many rounds. Which actions those are is read from requires_lanes (benchmark_lane / profile_lane) rather than a name list. The test is the case you described: with 30 minutes left, a baseline priced at 5 catalogue minutes is admitted; after this session measures 51 minutes, the same call is refused and the message says 51.

4. The one that would have shipped silently: layer 1 was reading a field that no longer exists

main removed cost_minutes_p50 from ACTION_CATALOGUE in a708deca6 as unused. This PR added the first reader. Neither side is wrong alone; merged, getattr(meta, "cost_minutes_p50", 0.0) starts returning 0.0 and the admission gate admits everything regardless of budget — the gate that is item 1 of this PR, disabled by a merge. Reproduced at the merge commit: 8 red tests. fd99d0545 reads typical_runtime_min (which carries the same small-model values) through a helper, with a guardian test asserting no action in the catalogue prices out at zero.

Coordination

Closes #1146 → Refs #1146, updating the description now; items 1, 2, 4 and 5 of the issue are not all closed by this.

On the #1177 collision, your list is right and there is one more that git will not tell you about. #1177 renames _pulse_after_variant to _after_variant and rewrites all 13 call sites; this PR keeps the old name and adds a 14th, inside the new reaped-variant branch. The two regions do not overlap, so the merge is reported clean and lands a NameError on precisely the path that records a budget-reaped variant as skipped — this PR's headline behaviour, converted into a crash, in a tree where both branches' own suites are green. I merged the four branches in a scratch worktree to find it. It is fixed on #1177's side (2a131a4be, keeping the old name), so the merge is now safe in either order; the four textual conflicts remain and are all genuinely orthogonal. The integration tree runs 1101 passed / 0 failed and adds no new ruff finding over main.