Skip to content

[STEPPING] Delete gates that never fired, and show the planner what it was blind to - #1056

Draft
ZhengGong-amd wants to merge 23 commits into
mainfrom
feat/zgong/explore-opt-11
Draft

[STEPPING] Delete gates that never fired, and show the planner what it was blind to#1056
ZhengGong-amd wants to merge 23 commits into
mainfrom
feat/zgong/explore-opt-11

Conversation

@ZhengGong-amd

Copy link
Copy Markdown
Collaborator
Added Removed Net
Source +802 −530 +272
Tests +357 −448 −91
Total +1,159 −978 +181

42 files. policy/gate.py alone is +72/−277. The source net is positive
because the second half adds a pull tool, a prompt block, an intent and a
bidirectional file channel; the first half is pure deletion.

Regressions: zero. The failure set is byte-identical to a stashed baseline
of the same tree on the same machine — 17 failures (15 under
inference_optimizer/tests, 2 under agents/robustness/tests), all from shell
environment leakage (ANTHROPIC_BASE_URL / ANTHROPIC_MODEL / a gateway key)
and one venv-discovery test. Each batch was separately baseline-diffed before
any failure was attributed to the change, and CI re-establishes this
independently.

Why the gates went

Each removal below was verified unreachable or non-load-bearing at the source,
not inferred from a grep.

The free-form red-line scan matched destructive shell in
params.task_description. It scanned that one field, while notes,
research_hints, arch_notes and gap_symptom reach the identical specialist
system prompt unscanned — the same text passes by moving fields. After dispatch
nothing enforces it: the subprocess runs under bypassPermissions with Bash,
Write and Edit, and no PreToolUse hook exists. Its own comment said it was
not a security boundary. Its only reliable effect was costing the planner a
tick for prose that describes a destructive command rather than running one.

The specialist_done validator (R3) never saw a real payload. Production
specialists write specialist_done.json; the dispatcher reads it and calls
_record_specialist_result directly, bypassing _handle_intent and therefore
validate_intent. It was also redundant: runner._finalize re-stamps
gap_canonical_id and domain from the dispatch params and defaults
proposal_set, empty and summary, so every field it checked was already
guaranteed by code the specialist cannot influence.

Five scope/tag denials became observations. resolve_specialist_profile is
documented never to raise — it re-infers the scope from the tag count — and
SpecialistRunner synthesizes a well-formed empty result for an anchor it
cannot resolve. Denying these converted a graceful degradation into a lost
tick.

Dead or self-limiting checks went with them: the wave shape checks
(non-list, non-dict entry, empty description) are all re-checked in
_fan_out_specialist_wave and intent_router; a negative max_turns yields
an empty turn range; the gpu_count type check is re-parsed by the dispatcher
with the same default.

Task-registry surface with no counterpart: failed -> running (auto-retry
creates a new row under an -autoretryN key), needs_manual_review (no
production writer), and Task.attempts (no production reader — the retry cap
is driven by params["_auto_retry_attempt"]).

What was deliberately kept

  • max_turns upper bound. The in-process backend's turn loop has no
    wall-clock check, so this is the only bound on that path.
  • Wave size cap (16). research_lane capacity bounds concurrency, not
    total spend. One turn can authorize N claude subprocesses that outlive the
    turn that asked for them.
  • params must be a dict. Without it the AttributeError fires inside
    validate_intent, escapes the PolicyDenied-only handler, and aborts the
    turn's remaining intents while incrementing the emergency-stop crash count.
  • gpu_count <= 0. On the default single-node Ray path this does not
    livelock — try_acquire_ray_observation succeeds and the dispatcher writes
    ROCR_VISIBLE_DEVICES=100000 into the specialist, which then measures
    garbage and reports success.

The blindness this exposed

A specialist SIGKILLed at its wall-clock cap rendered byte-identically to a
fully successful one
. Three things combined: the executor never raises, so
the failure rides inside the result envelope with SubAgentResult.error left
None; the envelope reports runner_status, which was absent from the
renderer's status key list; and the reaper's error text was folded into bare
constants. The planner read kind='specialist' state='succeeded' and nothing
else — and could not reach the reason even by pulling, because
get_recent_outcomes re-renders through the same formatter.

Fixed by adding runner_status to the status keys, falling back to the nested
error, surfacing the audit notes (a run whose patches were all dropped as
ungrounded no longer reads as a plain success), and keeping the reaper's
verbatim text after the classifier token so the elapsed and threshold numbers
survive. A checkpoint salvaged after an infra failure now reports partial
rather than succeeded, and is classified non-retryable so a retry cannot
discard what was rescued.

Three further holes closed:

  • Silent abandonment. Both auto-retry bail-outs returned without emitting
    anything, so the planner saw retries 1..N−1 and nothing for the give-up. It
    now broadcasts specialist_auto_retry_exhausted.
  • No view of running work. get_recent_outcomes queries terminal events
    only, and no prompt section carried task rows, so a dispatched task was
    invisible between task_queued and its delegated_result — hours, for a GPU
    specialist. get_running_tasks joins the running rows against the lease and
    GPU-lease tables: elapsed seconds, domain and gap, lease TTL and remaining
    time, held lanes, leased GPU ids, and heartbeat age from the same files the
    reap loop polls.
  • Unjudgeable GPU requests. The prompt asked the planner to reason about
    gpu_count relative to serving TP without rendering the TP or either pool
    size. The === Resource pools === block reports the numbers PolicyGate
    actually admits against, including that the serving-disjoint pool is empty
    whenever serving owns every card.

New capabilities

kill_task from orchestration. The restriction to Robustness was a role
partition, not a safety argument. This required fixing a bug first: killing a
running task destroyed its result, because the cancel makes the row terminal
while the executor is still running, so its closing transition raised
IllegalTransition, escaped run_task, and was dropped by the reap loop — no
delegated_result, no bookkeeping, for up to hours of GPU work. scope stays
task-only.

extend_lease. Nothing ever renewed a lease: heartbeat existed,
CAS-protected, with zero callers, so lease_ttl_sec was fixed at enqueue and a
task that legitimately outran its registry default was failed out from under
live work. The intent refreshes the task TTL, its lane rows and its GPU rows
together, preserving kill <= gpu_lease TTL <= gpu_research_lane TTL.

A two-way specialist channel. Previously fire-and-forget: parent writes
prompt.md, spawns, reads specialist_done.json after death. A specialist
that discovered its mandate was wrong an hour in could only burn the remaining
budget.

  • Uplink: the reap loop already stat'd the workspace every 5s but read
    specialist_done.partial.json only after the process died, as salvage. It
    now parses each rewrite while the specialist is alive and republishes it as a
    specialist_progress observation.
  • Downlink: a send_message addressed to specialist:<task_id> is appended
    to inbox.json in the specialist's workspace, which the prompt tells it to
    read between steps. The reaper ignores that file, so a message steers a live
    run instead of ending it — the missing half, since residual_questions
    previously had no answer path.

Bugs found and fixed during self-review

The final two commits are the result of reviewing the first thirteen. They are
worth reading as a unit, because four of the six are defects the
instrumentation work itself introduced.

  1. EXTEND_LEASE had no _PAYLOAD_REQUIRED entry. validate_envelope
    uses a bare subscript, so it raised KeyError instead of
    IntentValidationError. The backends catch only the latter, so the first
    extend_lease emitted would have been swallowed as an SDK stream failure,
    discarding every intent already collected in that turn. Also mirrored
    into the robustness envelope, whose contract test diffs the two enums and
    was failing.
  2. _transition_resilient swallowed IllegalTransition everywhere,
    including queued -> running where that rejection is the double-spawn
    guard. Now opt-in per call site; only the three terminal transitions
    tolerate it.
  3. extend_lease restamped updated_at, which forgave the elapsed time on
    top of extra_sec and reset the elapsed-seconds readings that both the
    health block and get_running_tasks derive from it.
  4. extend_lease did not refresh gpu_leases, so the GPU reaper could
    still free cards under a live specialist — the exact failure the intent
    exists to prevent.
  5. The specialist inbox was written to the workspace while the prompt
    advertises the worktree. Steering never reached a specialist that had one,
    which is the production case. The test passed only because its environment
    had no worktree.
  6. get_running_tasks reported an arbitrary lane's expiry for a multi-lane
    task; it now reports the soonest, which is when reclaim starts.

Scaffolding removed in the same pass

The review also stripped guards that cannot fire or that a caller already
provides: a third-layer _rows closure over a read already wrapped twice;
session_dir is None checks on a non-Optional field that is never reassigned;
a runs_dir guard unreachable for a literal action and a uuid task id; a
try/except around three lines of arithmetic; and a resource-pools guard
sitting beside a larger unguarded renderer. A bare except Exception in
_handle_extend_lease was narrowed to the two registry errors a bad task_id
actually produces — it was reporting infra failures back to the planner as its
own mistake. A hand-built Lease that only worked because heartbeat happened
to read two of its fields was replaced with heartbeat_by_task.

One correctness fix came out of this: the Resource pools block read
serving_tp from shared_state.tp while the pool size beside it came from
_serving_tp_for_policy, which also honours the TP env — the block could
report serving_tp=0 next to a carve of four cards.

Review guidance

Read the deletions for reachability claims, not diff size. Every removed
gate is justified above by a specific unreachable path; the useful question is
whether that path is genuinely unreachable in your configuration. The
Ray-vs-non-Ray split matters here: on the default single-node Ray path the
physical GPU mutex is Ray's num_gpus / serving_slot, which is
process-scoped; with INFERENCE_OPTIMIZER_RAY_EXEC=0, on multi-node, or under
pytest, the SQLite lanes are the only mutex and are clock-scoped.

For the new intents, check the wiring is complete rather than the logic.
An intent must land in IntentType, _PAYLOAD_REQUIRED, the role intent sets,
the PolicyGate dispatch, the IntentRouter dispatch table, the emit_intent
tool description, the claude.py enum, and the robustness envelope mirror.
Missing one of those is how finding 1 above shipped.

The two-way channel is the highest-risk addition. Its downlink writes into
a directory the specialist reads concurrently; the tmp+rename swap is
load-bearing. Note that the path had to match worktree or workspace to work
at all, which no test caught.

Behavior worth a release note: orchestration can now cancel a running task,
and the specialist prompt gained an inbox.json contract. Neither changes an
existing interface, but both change what an operator watching a session will
see.

ZhengGong-amd and others added 15 commits July 29, 2026 09:13
…ss safety nets

Render the full unread inbox batch per tick instead of the last-20 tail so
orchestration/robustness never silently lose events that arrive together in a
single tick. The Critic's durable pending-proposal augment is retained: the
read cursor advances per intent, so undecided proposals can scroll past the
cursor and only that path re-delivers them.

Remove the orchestration-memory compaction safety nets and let the soft
water-level + cadence triggers drive compaction directly:
- anti-thrash tick-gap floor and the >=98% emergency ceiling
- the degenerate-summary hard-compaction fallback path
- the now-unused hard water level: is_hard_compaction, context_token_hard,
  DEFAULT_CONTEXT_TOKEN_HARD_FRACTION, deterministic_memory_fallback,
  _checkpoint_min_tick_gap, and the INFERENCE_OPTIMIZER_CTX_HARD_FRACTION knob

The self-summary compaction body, soft-budget/cadence triggers, and the
next_cycle_directive cross-cycle strategy chain are unchanged. Tests that
covered the removed symbols are deleted.

Co-Authored-By: Claude <noreply@anthropic.com>
The scan was self-declared not a security boundary and did not act as one:
it matched only params.task_description, while notes, research_hints,
arch_notes and gap_symptom reach the specialist system prompt verbatim and
unscanned, so the same text passes by moving fields. After dispatch nothing
enforces it either - the specialist subprocess runs under bypassPermissions
with Bash, Write and Edit, and no PreToolUse hook exists.

Its only reliable effect was costing the planner a full tick for prose that
describes destructive commands rather than running them.

Co-Authored-By: Claude <noreply@anthropic.com>
PolicyGate never saw a real specialist payload. The production subprocess
writes specialist_done.json, which the dispatcher reads and hands straight
to _record_specialist_result without going through _handle_intent, so
validate_intent was never called with a specialist:<task_id> identity.

The checks were also redundant with the runner: _finalize re-stamps
gap_canonical_id and domain from the dispatch params and defaults
proposal_set, empty and summary, so every field the validator required was
already guaranteed by code the specialist cannot influence.

Removes _validate_specialist_intent, _validate_specialist_done_payload,
_SpecialistPseudoRole, the specialist branch in validate_intent, and
IntentRouter._handle_specialist_done with its dispatch-table entry. The
specialist Iron Rule now describes the actual consequence of a stray intent
(dropped and recorded as a tool violation) instead of a gate that never ran.

Co-Authored-By: Claude <noreply@anthropic.com>
…hape

resolve_specialist_profile is documented never to raise: it re-infers the
scope from the tag count, and SpecialistRunner synthesizes a well-formed
empty result for an anchor it cannot resolve. Denying these cases converted
a graceful degradation into a lost planner tick without protecting anything.

Downgraded to log-only observations: missing tags under an explicit scope,
out-of-vocabulary tags, an unknown scope value, scope='domains' with one tag,
and scope='domain' with several.

Also dropped as dead or self-limiting:
- the wave shape checks (non-list, non-dict entry, empty description), all
  re-checked defensively in _fan_out_specialist_wave and intent_router
- the max_turns lower bound; a negative value yields an empty turn range
- the gpu_count type check, which the dispatcher re-parses with the same
  default fallback

The max_turns upper bound stays: the in-process backend's turn loop has no
wall-clock check, so it is the only bound on that path. The wave size cap
stays: one turn can authorize N subprocesses that outlive the turn that
asked for them, and orchestration cannot recall them.

Co-Authored-By: Claude <noreply@anthropic.com>
Three pieces of the DelegatedTask state machine had no live counterpart:

- failed -> running: no code path re-dispatches a failed row. The specialist
  auto-retry creates a new task under an -autoretryN idempotency key, and the
  only transitions into running come from queued.
- needs_manual_review: no production writer ever transitions into it; it was
  carried in the state tuple, the CHECK constraint, TERMINAL_STATES and three
  terminal-state literals without anything producing it.
- Task.attempts: no production reader. The retry cap is driven by
  params["_auto_retry_attempt"], so the column and its increment were pure
  telemetry nobody consumed.

Narrowing the CHECK affects new databases only, and dropping attempts from the
INSERT relies on its existing column default, so both directions stay
compatible with session databases created by earlier versions.

Co-Authored-By: Claude <noreply@anthropic.com>
Orchestration dispatches specialists but learned nothing about them between
the task_queued event and the terminal delegated_result, which for a GPU
specialist can be hours apart. The Specialist health block already computed
and rendered exactly the missing signal - in-flight count, stale count, and
per-task running seconds - but was gated to Robustness.

The block is a read of tasks.running() with no concurrency meaning, so both
planners now receive it. kill_task authority is unchanged: the stale-task
line tells Robustness to reap and tells Orchestration to plan around the
stall or escalate.

Co-Authored-By: Claude <noreply@anthropic.com>
Comments and docstrings still described specialist_unknown_domain and the
R2/R3 rule names as live enforcement. Reworded to state what the code now
does, and to record why the max_turns hard cap survived while the rest of
the dispatch shape checks became observations.

Co-Authored-By: Claude <noreply@anthropic.com>
A SIGKILLed specialist rendered byte-identically to a fully successful one.
Three things combined to erase it: the executor never raises, so the failure
rides inside the result envelope with SubAgentResult.error left None; the
envelope reports runner_status, which was absent from the status key list;
and the reaper's error text was folded into bare constants.

- add runner_status to the outcome status keys, and fall back to the nested
  result error when the top-level one is None
- surface the audit notes, so a run whose patches were all dropped as
  ungrounded no longer reads as a plain success
- keep the reaper's verbatim text after the classifier token, so the elapsed
  and threshold numbers survive into the prompt
- report a checkpoint salvaged after an infra failure as partial rather than
  succeeded, and classify it as non-retryable so the rescued proposals are
  not thrown away by a retry
- read the failure reason from the result envelope in the auto-retry
  classifier, which was previously always passed an empty string

Co-Authored-By: Claude <noreply@anthropic.com>
Two blind spots around dispatched work:

The auto-retry gave up silently. Both bail-outs - retry cap reached, and
retry slot already taken - returned without emitting anything, so a planner
could see retries 1..N-1 and nothing for the abandonment. It now broadcasts
specialist_auto_retry_exhausted carrying the failure type, verbatim reason,
attempts used and the wall budget the attempt was given.

Nothing showed work still running. get_recent_outcomes only queries terminal
events, and no prompt section carried task rows, so a dispatched task was
invisible between task_queued and its delegated_result - hours, for a GPU
specialist. The new get_running_tasks pull tool joins the running task rows
against the lease and gpu_lease tables and reports elapsed seconds, domain
and gap, idempotency key, lease TTL and remaining time, held lanes, leased
GPU ids, and heartbeat age using the same liveness files the reap loop
polls.

Co-Authored-By: Claude <noreply@anthropic.com>
The prompt asked the planner to reason about gpu_count relative to serving TP
without ever rendering the TP, the pool sizes, or the lane capacities. A
needs_gpu dispatch could therefore only be judged schedulable by emitting it
and reading the denial.

The new Resource pools block reports the same numbers PolicyGate admits
against, including the two distinct pools: bench and framework-authoring
specialists admit against the whole-machine pool, everything else against the
serving-disjoint pool, which is empty whenever serving owns every card.

Co-Authored-By: Claude <noreply@anthropic.com>
kill_task on a running task destroyed its result. The cancel makes the row
terminal while the executor keeps running, so its closing transition raised
IllegalTransition, escaped run_task, and was dropped by the reap loop - no
delegated_result, no bookkeeping, nothing recorded for up to hours of GPU
work. _transition_resilient now swallows that case the way it already
swallows a vanished row, so the result survives the kill.

With that fixed, orchestration can emit kill_task itself. The restriction to
Robustness was a role partition, not a safety argument: orchestration holds
the strategic context for deciding that a specialist it dispatched is chasing
a dead end, and it now has the health block and get_running_tasks to see one.
scope stays task-only; server and process kills still go through Robustness
delegate(recover).

Co-Authored-By: Claude <noreply@anthropic.com>
Nothing ever renewed a lease. ResourceLockManager.heartbeat existed, was
CAS-protected against a lost lease, and had zero callers, so lease_ttl_sec was
fixed at enqueue and the TTL watchdog measured age from updated_at, which only
a state transition moved. A task that legitimately outran its registry default
was failed out from under live work.

extend_lease grows the task's own lease_ttl_sec (what the watchdog reads) and
restamps updated_at, then pushes every lane row the task holds out to the same
horizon through the existing heartbeat, so the two cannot drift. Each step is
bounded and the outcome is broadcast, so a request has to be re-justified
against the live task view rather than granted once as a blanket window.

Co-Authored-By: Claude <noreply@anthropic.com>
A specialist was fire-and-forget: the parent wrote prompt.md, spawned it, and
read specialist_done.json after it died. If it found the mandate was wrong an
hour in, it could only burn the remaining budget.

Uplink: the reap loop already stat'd the workspace every 5s but only read
specialist_done.partial.json after the process was dead, as a salvage
fallback. It now parses each rewrite while the specialist is alive and
republishes it as a specialist_progress observation carrying the summary so
far, proposal count, new findings and residual_questions.

Downlink: a send_message addressed to specialist:<task_id> is now also
appended to inbox.json in the specialist's workspace, which the prompt tells
it to read between steps. The reaper ignores that file, so a message steers a
live run instead of ending it - the missing half of the loop, since
residual_questions previously had no answer path back.

Co-Authored-By: Claude <noreply@anthropic.com>
Correctness:
- EXTEND_LEASE had no _PAYLOAD_REQUIRED entry, so validate_envelope raised
  KeyError instead of IntentValidationError; the backends catch only the
  latter, so the first extend_lease emitted would have discarded every intent
  in that turn. Mirrored into the robustness envelope too, which its contract
  test diffs against.
- _transition_resilient swallowed IllegalTransition at every call site,
  including queued->running where the rejection IS the double-spawn guard.
  Now opt-in per call site; only the three terminal transitions tolerate it.
- extend_lease restamped updated_at, which forgave the elapsed time on top of
  extra_sec and reset the elapsed-seconds readings that both the health block
  and get_running_tasks derive from it.
- extend_lease refreshed the lane rows but not gpu_leases, so the GPU reaper
  could still free cards under a live specialist - the exact failure the
  intent exists to prevent. SpecialistGpuPool.extend keeps the three TTLs
  ordered.
- The specialist inbox was written to the workspace while the prompt
  advertises the worktree, so steering never reached a specialist that had
  one, which is the production case.
- get_running_tasks reported an arbitrary lane's expiry for a multi-lane
  task; report the soonest, which is when reclaim starts.
- Publish partial checkpoints after the exit checks so a finished run does
  not emit one last progress event.

Simplification:
- Replaced the hand-built Lease in the extend path with
  ResourceLockManager.heartbeat_by_task; the fabricated object only worked
  because heartbeat happened to read two of its fields.
- EXTEND_LEASE dropped from the robustness intent set: orchestration owns it.

Docs: trimmed comments that narrated the change rather than the mechanism,
and scrubbed the remaining R2 / R3 / red-line references the earlier docs
commit missed.

Co-Authored-By: Claude <noreply@anthropic.com>
…umentation

Guards that could not fire, or that a caller already provides:
- the _rows closure in the running-tasks reader was a third layer over a read
  already wrapped by the method and by ContextProvider._safe, and leases /
  gpu_leases are created by the same DDL transaction as tasks
- session_dir is non-Optional and never reassigned, so its None checks in the
  heartbeat-age probe and the inbox writer were dead
- runs_dir cannot raise for a literal "specialist" action and a uuid task id
- the resource-pools render sat behind a try/except while the larger
  to_prompt_summary call beside it did not
- _specialist_wall_budget_sec is three lines of arithmetic and cannot raise

Narrowed or simplified:
- _handle_extend_lease caught bare Exception, reporting an infra failure back
  to the planner as its own mistake; it now catches the two registry errors a
  bad task_id actually produces
- _transition_resilient returned a bool no call site read
- the retry-exhausted observation recomputed wall_budget_sec at report time,
  so it described a hypothetical future retry rather than the attempt that
  failed; dropped, which removes a hand-rolled needs_gpu coercion the repo
  already has as coerce_needs_gpu
- ftype is a SpecialistFailureType, not Any, so the getattr dance is gone

Corrected:
- the Resource pools block read serving_tp from shared_state.tp while the pool
  size beside it came from _serving_tp_for_policy, which also honours the TP
  env; the block could report serving_tp=0 next to a carve of 4 cards
- gpu_research_lane_capacity was a literal duplicating DEFAULT_LANE_CAPACITIES

Co-Authored-By: Claude <noreply@anthropic.com>
@ZhengGong-amd
ZhengGong-amd requested a review from a team as a code owner July 30, 2026 08:39
Comment thread src/hyperloom/orchestrator/state/_shared_state/render.py Fixed
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

CI E2E report — ❌ Failed

item value
result ❌ Failed
model Qwen/Qwen3-0.6B (dense)
resources 1× GPU, TP=1
PR branch feat/zgong/explore-opt-11
commit 0491edb82f546344ac5b77df5b06c27c4a9570b6
session_id cba4fef4-2d3b-4a66-acca-127d10d4385f
queue → dispatch 0s
run time 154m 25s
total 154m 25s
reason platform reported Failed: container hyperloom exited with code 1 (Error): pruned_families : []
detail `platform reported Failed: container hyperloom exited with code 1 (Error): pruned_families : []

details

ZhengGong-amd and others added 8 commits July 30, 2026 10:17
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Protect resolved workspace paths when loading dotenv credentials, allow model architecture profiles to be seeded before launch, and include the supporting specialist review notes.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Comment on lines +326 to +331
from ...policy.gate import (
_effective_gpu_specialist_pool_size,
_serving_tp_for_policy,
_whole_machine_pool_size,
gpu_specialist_ceiling,
)
@ZhengGong-amd
ZhengGong-amd marked this pull request as draft July 30, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants