Dynamic executor model routing with cost-aware escalation - #29
Open
saikethan27 wants to merge 2 commits into
Open
Dynamic executor model routing with cost-aware escalation#29saikethan27 wants to merge 2 commits into
saikethan27 wants to merge 2 commits into
Conversation
- Introduced tests for escalation-worthy failures in `test_escalation_failure_signal.py`, covering various scenarios including incomplete reports and integrity issues. - Added tests for executor escalation briefings in `test_executor_escalation_briefing.py`, ensuring that escalated executors receive proper context about previous failures. - Implemented tests for manager escalation behavior in `test_manager_escalation.py`, validating the conditions under which escalation occurs and the effects on executor tiers. - Created end-to-end tests for the escalation flow in `test_manager_escalation_e2e.py`, simulating a complete run from task creation to successful completion with tier transitions. - Developed tests for executor tier selection in `test_manager_executor_tier.py`, ensuring correct routing based on manager choices and executor types. - Added role prompt parsing tests in `test_role_prompts_executor_tier.py` to verify the correct extraction of executor tier information from manager plans.
The harness could not complete a single round on Windows. Agent commands were built as POSIX shell strings and handed to `create_subprocess_shell`, which is cmd.exe there, so `mkdir -p` failed on the first call. Process control used `os.killpg`/`SIGKILL`/`SIGHUP`, none of which exist on Windows, so every timeout and Ctrl+C raised AttributeError. Rather than translate the shell strings per platform, remove the shell from the agent path entirely. Everything it was doing is a real subprocess argument: cd <ws> && ... -> cwd= VAR=value <cmd> -> env= layered onto the scrubbed parent environment ... < prompt.md -> the prompt is written to the child's stdin mkdir -p / chmod -> native pathlib calls on the local environment Commands are argv lists now, so `shlex.quote` is gone and no model-supplied value (model id, path, MCP config) can reach a shell parser. `Environment.exec` stays as an explicit escape hatch for callers that genuinely want a shell. Also fixed, all found by running it for real on Windows: - process_group: platform split, CREATE_NEW_PROCESS_GROUP plus `taskkill /T`, and named operations instead of POSIX-only signal numbers. - MAX_PATH: run directories reach 260 characters easily, and the failure is deceptive because mkdir succeeds on the shorter parent while open() on the file inside raises FileNotFoundError. New utils/paths applies the \?\ prefix, but only to paths that actually exceed the limit. - screenshot(): PowerShell on Windows, screencapture on macOS, the existing X11 tools on Linux. - Console output no longer mangled by cp1252. - Deny rules for the claude_code backend now also emit the drive-letter form, so harness-owned paths are actually hidden from the auditor on Windows. The 0.1.4 hardening layer is POSIX-only in the same way, so the run tree could not be written on Windows at all: `_ensure_dir_nofollow`, `_atomic_bytes_write`, `_append_jsonl` and the event append all require O_NOFOLLOW, O_DIRECTORY and dir_fd. Each grows a Windows branch that keeps what the platform can express -- reparse-point refusal, atomic replace, the hard-link check before truncation, and the same event-id sequencing -- and documents the one guarantee that cannot be reproduced without directory descriptors: the check-then-use window is not anchored. POSIX behaviour is untouched. Verified on Windows against real models: a full run on codex/gpt-5.6-luna completes, and a cross-backend run drives codex.CMD for manager and auditor with claude.EXE as executor in the same run. Against origin/main the suite goes from 74 failed / 94 passed to 68 failed / 308 passed, with no regressions; the six repaired tests are upstream's own. The remaining 68 are the webapi and supervisor POSIX-only paths, which this change does not touch. Adds 41 tests over LocalEnvironment, the process-group primitives and adapter argv construction, none of which had any coverage before. BREAKING: CommandAgentAdapter takes argv=/env= instead of command_template=. `EpisodeResult.metadata["command"]` is now the argv list rather than a string; existing readers already accept both.
saikethan27
force-pushed
the
feat/executor-tier-routing
branch
from
August 12, 2026 12:11
a3cc930 to
0dbae41
Compare
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
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.
Closes #13.
Adds a second, orthogonal routing dimension to the executor: a tier (
cheap/strong) alongside the existing type (gui/cli). Routine work stays on a cheaper backend; the Manager can ask for a stronger one per subtask, and the harness escalates on its own when verification keeps failing.Everything goes through the existing
AgentAdapterabstraction, so it is backend-agnostic — a Codex manager can dispatch to a Claude Code executor and back.1. Executor tiers and cost-aware escalation (#13)
Configuration
Tier tables nest inside the existing role tables, so resolution keeps the current fallback-chain shape. Naming a tier is a deliberate cost decision, so it outranks the older type-level section:
Per-type overrides (
[run.roles.gui_executor.strong]) are supported, so all four of CLI+cheap, CLI+strong, GUI+cheap and GUI+strong are expressible without duplicating the executor implementation. Every field has a matching CLI flag, as elsewhere in the project.Backward compatible. A config with no tier tables resolves every tier to the same executor it uses today. A test asserts a legacy
[run.roles.executor]config produces a byte-identical defaults dict.Manager-selected tier
The Manager may add one optional line after its route:
The issue sketched this as JSON, but the manager protocol is deliberately JSON-free throughout (
MANAGER_INSTRUCTIONS: "Output plain natural language, never JSON"), so it follows the existingNext:line convention instead. The keys map one-to-one. The prompt instructs the Manager to judge the subtask in front of it and explicitly not to decide from task category. Omitting the line accepts the configured default.Escalation
Escalation never bypasses verification: the strong result goes through the normal Auditor flow. A passing audit clears the escalation, so routing returns to the default tier and the expensive model is scoped to the stretch of the run that is actually struggling.
What counts as a failure matters more than it looks. Auditors are instructed to report
incompletewhenever the whole contract is unsatisfied, "even if the local subtask succeeded" — so a perfectly good mid-run round comes backincomplete / clean / aligned. An earlier version of this PR treated that as a failure and escalated on round one of every task, which threw the saving away. Two distinct signals now:blocked,suspect/violation,needs_revision/invalid, or an executor episode that erroredescalate_after_failuresescalate_after_stalled_roundsincomplete+ clean + aligned with a new gap each roundEscalation briefing
Every role episode is a one-shot process — there is no session to resume, and resuming would not be coherent across an escalation that changes backend. A previous round's
executor_outputnever otherwise reaches a later executor, and the Auditor's findings only arrive if the Manager happened to cite those rounds.So an escalated executor is briefed with what the previous tier attempted and why it was rejected. The prior attempt is labelled as that executor's own unaudited claim; the Auditor report is labelled authoritative — the same trust boundary
MANAGER_INSTRUCTIONSalready draws. Bounded to the three most recent failures and char-clipped. Disable withescalation_briefing = false.The Manager is told too, through the existing
harness_feedbackchannel, since repeated failure can mean the decomposition is wrong rather than the executor being too weak.Observability
(type, tier)cell to its agent and modelexecutor_tier+tier_sourceon the executor events; a newexecutor_escalationevent carrying the reason and the failed roundstier=cheapon the console role line, and a dedicated escalation lineexecutor_tierper round inreport.json, plus anexecutor_routingblock recording the policy, per-tier round counts, and the full escalation history (kept even after a run recovers)executor_tierexposed byDashboardStatefor finished and in-progress rounds (written before the executor starts, so a live round already shows it). The badge itself needs re-adding to the newfrontend/, see below.2. Windows support (independent of #13)
The harness could not complete a single round on Windows. Agent commands were POSIX shell strings handed to
create_subprocess_shell, which is cmd.exe there, somkdir -pfailed on the first call. Process control usedos.killpg/SIGKILL/SIGHUP, none of which exist on Windows, so every timeout raisedAttributeError.Rather than translate shell strings per platform, this removes the shell from the agent path. Everything it was doing is a real subprocess argument:
cd '<ws>' && …cwd=VAR=value <cmd>env=layered ontoos.environ… < prompt.mdmkdir -p,chmodpathlibcallsCommands are argv lists, so
shlex.quoteis gone and no model-supplied value can reach a shell parser.Environment.execsurvives as an explicit escape hatch.Also fixed, all found by running it for real:
CREATE_NEW_PROCESS_GROUP+taskkill /T, named operations instead of POSIX-only signal numbersMAX_PATH— run directories reach 260 characters easily, and the failure is deceptive:mkdirsucceeds on the shorter parent whileopen()on the file inside raisesFileNotFoundError. Newutils/pathsapplies the\\?\prefix, but only to paths that actually exceed the limitscreenshot()— PowerShell on Windows,screencaptureon macOS, existing X11 tools on LinuxThe 0.1.4 hardening layer is POSIX-only in the same way, so the run tree could not be written on Windows at all:
_ensure_dir_nofollow,_atomic_bytes_write,_append_jsonland the event append all requireO_NOFOLLOW,O_DIRECTORYanddir_fd. Each grows a Windows branch that keeps what the platform can express — reparse-point refusal, atomic replace, the hard-link check before truncation, and the same event-id sequencing — and documents the one guarantee that cannot be reproduced without directory descriptors: the check-then-use window is not anchored. POSIX behaviour is untouched.Breaking:
CommandAgentAdaptertakesargv=/env=instead ofcommand_template=.EpisodeResult.metadata["command"]is now the argv list rather than a string; the existing reader inmanager.pyalready accepts both. The vendoredcua_harnesscopies undereval/are separate packages and are untouched.Testing
The
lh_harnesspackage had no test suite; this adds one — 208 tests, no paid model calls (scripted adapters and a recording environment).(type, tier)cells across five config shapes, tier-before-type precedence, and a regression lock on the eight pre-existing rolesrounds.jsonl,report.json,events.jsonl, DashboardLocalEnvironmentargv/cwd/env/stdin, timeouts, long lines,MAX_PATH, process-group primitives, adapter argv constructionVerified against real models on Windows
codex/gpt-5.6-lunacodex.CMDmanager + auditor,claude.EXEexecutor, one run, completetier=cheap → --model=gpt-5.6-luna,tier=strong → --model=gpt-5.6-sol, read from the recorded argvExecutor tier: cheapunpromptedRebased onto 0.1.4
Rebased onto
53bc678, resolving conflicts in favour of upstream wherever the two overlapped — the supervised-run plumbing, bootstrap-failure handling,resolve_codex_binary(), theLH_HARNESS_WEB_TOKENscrub, bounded stdout capture, and the symlink-safe trajectory writer are all preserved. The tier badge that used to live indashboard/static/app.jswent away with that file;executor_tieris still exposed byDashboardState, so re-adding it to the newfrontend/is a small follow-up.Measured against
origin/mainon the same machine:No regressions. The six newly-passing tests are upstream's own (
test_cli_isolation,test_manager_hardening,test_trajectory_artifacts), repaired by the Windows branches above. The remaining 68 are pre-existing Windows breakage in thewebapi/supervisorPOSIX-only paths, which this PR does not touch — on Linux CI they are unaffected either way.Known limitations
escalate_after_failuresis raised above 1) is not briefed; only escalated rounds are. Documented at the knob.cheap,strong).0.