feat: let a build configure its outer optimizer trial's harbor flags#55
feat: let a build configure its outer optimizer trial's harbor flags#55shehabyasser-scale wants to merge 3 commits into
Conversation
37da8e6 to
bdcf567
Compare
Live result: the fix did not work.
|
| previous run (DinD) | this run (vm_runtime) | |
|---|---|---|
| harbor frame | single_step.py:105 _run_verifier → verifier.py:199 verify |
single_step.py:77 _run_agent → codex.py:1102 run |
| when | after codex finished, during the verifier | during the codex run itself |
| bottom of stack | modal.py:1304 _sdk_exec → StreamTerminatedError |
identical |
Same bottom of stack, different long-running exec. That rules out my hypothesis. This is not about host networking or which exec happens to run at teardown — it is that harbor runs each phase as one exec and streams its stdout for the phase's entire duration, and a ~35-minute stream does not survive. Modal's own _stream_stdio_with_retries (task_command_router_client.py:838) tries to recover and re-raises.
So the original version of this PR had the right instinct and the wrong layer: retry is the applicable lever, it just has to reach the outer trial. The outer harbor run receives no --max-retries, which is why both runs log retries=0 and "Not retrying trial because the maximum number of retries has been reached".
What I propose keeping
optimizer_harbor_argsstays. It is verified working, it is the only way to configure the outer trial at all, and it is now the mechanism the actual fix needs.- tau3's
--ek modal_vm_runtime=trueshould come out, or stay only on its own merits (it does remove the host-networking caveat), not as a fix for this. I am not going to leave a value in a committed config claiming to fix something a live run says it does not fix.
Next experiment, not yet run
optimizer_harbor_args: ["--max-retries", "1"]. Honest caveat before spending it: a retry re-runs the whole ~40-minute codex trial, so worst case is ~80 minutes and ~$2.5, and it only pays off if the stream drop is probabilistic. Two for two so far, at different phases, is weakly consistent with "random drop over a long window" — but two samples is two samples. Flagging the cost rather than burning it unasked.
Four live runs, each ~40 min on Modal, all ending the same way: default DinD 08:13 StreamTerminatedError (verifier exec) modal_vm_runtime 09:18 StreamTerminatedError (codex agent exec) modal_sandbox_v2 10:02 StreamTerminatedError --max-retries 1 10:22 StreamTerminatedError on both attempts So tau3's optimizer trial gets no `optimizer_harbor_args` value. None of the four is a fix, and a committed config value that implies otherwise is worse than an empty one. What the experiments did establish: - The mechanism works. With `--max-retries 1` the trial logged "failed with exception StreamTerminatedError. Retrying in 1.00 seconds" where every earlier run logged "Not retrying ... maximum number of retries has been reached". The outer layer is now configurable, which it was not before. - Retry is not a cure here. The second attempt died the same way, so this converts one guaranteed loss into two. - It is not a networking mode. The failure moved between phases under vm_runtime but kept an identical bottom of stack, and sandbox_v2 changed nothing. - It looks deterministic, not flaky: 39m55s, 39m45s and 38m14s to failure across three independent runs is too tight for a random stream drop. The common factor is that harbor runs each phase as one exec and streams its stdout for that phase's whole duration. No harbor timeout matches ~40 min, so the deadline, if there is one, is not in harbor's config surface. This PR is therefore just the `optimizer_harbor_args` plumbing plus its tests. The tau3 failure belongs upstream with harbor. Refs #55. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Final: all four experiments failed. This PR is now plumbing only.
~$5.30 and four runs. tau3's optimizer trial now has no What the experiments did establishThe mechanism works. Run 4 logged: Every earlier run logged Retry is not the cure. The second attempt died identically. On this workload retry converts one guaranteed loss into two, at double the cost. It is not a networking mode. vm_runtime moved the failure between phases while keeping an identical bottom of stack; sandbox_v2 changed nothing. It looks deterministic, not flaky. 39m55s, 39m45s and 38m14s across three independent runs is far too tight a spread for a random gRPC drop. That is the single most useful thing these runs produced, and it argues against every "add resilience" fix including the one I proposed. Where the real cause probably livesHarbor runs each trial phase as one This should go upstream to harbor as "long-running agent phases die at ~40 min on the Modal DinD/VM exec stream", with these four tracebacks attached. I have stopped spending on it here. Review noteThe one remaining judgement call is whether |
The tau3 optimizer trial kept dying ~22 min in, after codex had finished, with `grpclib StreamTerminatedError: Connection lost` raised from harbor's finalization verifier exec (harbor/verifier/verifier.py:199 -> environments/dind_compose.py -> modal.py `_sdk_exec`). Harbor logs its own remedy at trial start: "DinD mode uses host networking ... Use --ek modal_vm_runtime=true to use the VM runtime instead". There was no way to declare that. `extra_harbor_args` reaches the NESTED `harbor run` that scores a candidate (build/config.py -> HarborBackendConfig -> backend.py `_harbor_command`), not the OUTER `harbor run` that hosts the optimizer trial, which is where the stream drops. Add `optimizer_harbor_args`, forwarded by `vero harbor run` to the outer command ahead of any trailing CLI args (harbor's `--ek` is last-value-wins, so the command line still overrides), and set tau3's to `--ek modal_vm_runtime=true`. This supersedes the previous version of this PR, which raised tau3's `max_retries` 1 -> 3. That knob is the nested per-case retry and was proven not to be the failing layer: the 2026-07-25 run still errored with `retries=0` and "Not retrying trial because the maximum number of retries has been reached", because the outer trial never receives `--max-retries`. Reverted to 1. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Four live runs, each ~40 min on Modal, all ending the same way: default DinD 08:13 StreamTerminatedError (verifier exec) modal_vm_runtime 09:18 StreamTerminatedError (codex agent exec) modal_sandbox_v2 10:02 StreamTerminatedError --max-retries 1 10:22 StreamTerminatedError on both attempts So tau3's optimizer trial gets no `optimizer_harbor_args` value. None of the four is a fix, and a committed config value that implies otherwise is worse than an empty one. What the experiments did establish: - The mechanism works. With `--max-retries 1` the trial logged "failed with exception StreamTerminatedError. Retrying in 1.00 seconds" where every earlier run logged "Not retrying ... maximum number of retries has been reached". The outer layer is now configurable, which it was not before. - Retry is not a cure here. The second attempt died the same way, so this converts one guaranteed loss into two. - It is not a networking mode. The failure moved between phases under vm_runtime but kept an identical bottom of stack, and sandbox_v2 changed nothing. - It looks deterministic, not flaky: 39m55s, 39m45s and 38m14s to failure across three independent runs is too tight for a random stream drop. The common factor is that harbor runs each phase as one exec and streams its stdout for that phase's whole duration. No harbor timeout matches ~40 min, so the deadline, if there is one, is not in harbor's config surface. This PR is therefore just the `optimizer_harbor_args` plumbing plus its tests. The tau3 failure belongs upstream with harbor. Refs #55. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Two tests in test_v05_harbor_http.py build the build-config as a SimpleNamespace carrying only the attributes run_command reads, so adding a field to HarborBuildConfig breaks them with an AttributeError that surfaces only as a non-zero exit code. Same convention as when agent_env was added: the stub grows with the config. The mirror of that also applied to this branch's own test, whose _Config predates agent_env and so broke once both fields were in the same code path. Suite now matches the base branch exactly: 11 pre-existing failures, none new. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
ccc7d07 to
ec708e9
Compare
e189a3f to
28a5331
Compare
|
Restacked. Also needed a test fix that is worth knowing about generally: The PR remains plumbing-only and no benchmark sets the field. The four live experiments behind the original tau3 justification all failed identically (default DinD 39m55s, That leaves the honest question unchanged: whether a config field earns its place with no caller. I would rather you decide that than have me justify it with a value that does not work. Full suite matches the base branch: 11 pre-existing failures, none new. |
What broke
The tau3 optimizer trial dies at teardown, after codex has already finished optimizing. From the 2026-07-25 run (
runs/tau3-rerun/jobs/2026-07-25__07-33-29, ~40 min, $1.57 burned for zero signal):task__cmDWxgt/exception.txtputs it in harbor's finalization verifier, not in the model:Harbor prints the remedy itself, as line 4 of that very job's log:
DinD funnels the whole sandbox through one host-networked gRPC stream, so a single drop takes down a 20+ minute trial with no partial credit.
Why the previous version of this PR was wrong
It raised tau3's
max_retries1 → 3. That knob is the nested per-case retry:build/config.py→HarborBackendConfig.max_retries→ the innerharbor run --max-retriesinbackend.py::_harbor_command. The failing layer is the outer trial, which never gets--max-retriesat all. The live rerun confirmed it: stillretries=0, still"Not retrying trial because the maximum number of retries has been reached". Reverted to 1.Same trap applies to
extra_harbor_args: it also only reaches the nested eval. There was no way for a build to say anything about the environment the optimizer itself runs in.The fix
Add
optimizer_harbor_argstoHarborBuildConfig, forwarded byvero harbor runto the outerharbor run, and set tau3's to["--ek", "modal_vm_runtime=true"].parse_kwargsatcli/utils.py:65is last-value-wins per key).-p,-a,-e,-m), mirroring the existingextra_harbor_argsguard.http://eval-sidecar:8000,http://inference-gateway:8001), which the VM runtime resolves through real Docker DNS rather than the DinDextra_hosts→127.0.0.1remap, so this is the better-supported path for this task shape, not just the more resilient one.Live verification
Plumbing (from the run log, the outer command vero assembles):
An end-to-end tau3 run on Modal with this branch is in flight; it is a straight A/B against
2026-07-25__07-33-29(identical config, only the flag differs). Result posted here when it lands.Tests:
test_harbor_run_forwards_build_declared_optimizer_args(outer command carriesmodal_vm_runtime=true, nested-only flags stay off it, build-declared flags precede CLI args) and anoptimizer_harbor_argscontrolled-flag case intest_load_build_config_rejects_invalid_specs.Refs #55.
Greptile Summary
This PR adds
optimizer_harbor_argstoHarborBuildConfig, a new escape hatch that lets a build inject flags directly into the outerharbor runthat hosts the optimizer trial — distinct from the already-existingextra_harbor_args, which only reaches the nested per-candidate evaluation sub-run. The field is forwarded incli.pybefore the user's trailing CLI args, so a command-line override always wins (harbor's last-value-wins semantics). The tau3 build's comment is notably honest: all four attempted fixes (DinD, VM runtime, sandbox v2, max-retries) failed identically at teardown, so the field is shipped empty there while the upstream root cause is tracked.config.py: Newoptimizer_harbor_args: list[str]field with a Pydanticfield_validatorthat rejects the four flagsvero harbor runcontrols (-p,-a,-e,-m).cli.py: One-line insertion appendsoptimizer_harbor_argsto the outer command betweenagent_env--aeflags and the user's extra args, preserving CLI override priority.test_harbor_run_forwards_build_declared_optimizer_args) confirms the flag reaches the outer command, stays off the nested command, and precedes--yes; existingSimpleNamespacestubs intest_v05_harbor_http.pyare updated to include the new attribute.Confidence Score: 5/5
Safe to merge — the change is additive, the new field defaults to an empty list (no behavior change for existing builds), and the forwarding logic is a single well-tested line.
The change is narrow and well-tested: a new optional list field with a validator, a one-line CLI insertion, and matching test coverage for both the happy path and the rejection case. Existing builds are unaffected by the empty default. The one gap (long-form flag equivalents not in the controlled set) is harmless given last-value-wins ordering.
Files Needing Attention: No files require special attention.
Important Files Changed
optimizer_harbor_argsfield with a validator that blocks the 4 CLI-owned short flags (-a,-e,-m,-p); long-form equivalents are not blocked, but last-value-wins ordering makes this safe in practice.optimizer_harbor_argsbetweenagent_envflags and the user'sextraargs so CLI-provided flags always take final precedence.optimizer_harbor_argsis left empty; infrastructure is present for future use.optimizer_harbor_args, excludesextra_harbor_args, and preserves ordering (build-declared flags before CLI extras).optimizer_harbor_args; covers the controlled-flag guard and a valid--ekcase.SimpleNamespaceconfig stubs to includeoptimizer_harbor_args=[]so they don't crash on the new attribute access incli.py.Reviews (4): Last reviewed commit: "test: teach the run-command config stubs..." | Re-trigger Greptile