fix(harbor): make generated tasks safe on remote sandboxes (e2b) - #346
Open
vaibhavdabas16 wants to merge 3 commits into
Open
fix(harbor): make generated tasks safe on remote sandboxes (e2b)#346vaibhavdabas16 wants to merge 3 commits into
vaibhavdabas16 wants to merge 3 commits into
Conversation
Setup accepted any 200 from /api/status as "the runtime is ready". The runtime server starts answering before its CDP handler attaches and sets eval_interceptor_ready, so a task could begin in that window with interception inactive and silently fail to score Stage 1 — the failure looks like an agent miss, not a harness fault. The step healthcheck in task.toml already checked the right condition, so the two now share one `runtime_ready_command()` and cannot drift: runtime server up, request interceptor armed, CDP endpoint live. The TOML form is escaped once via json.dumps and the shell form is used verbatim, which also fixes the escaping that would have made a literal copy of the healthcheck string never match inside a shell script. The wait is also raised from 60s to a configurable 180s, and a timeout now tails the runtime-server log instead of exiting with one opaque line — remote sandboxes (TIGER-AI-Lab#331 §3) provision slower than a local daemon and were the case 60s was never sized for.
start-runtime.sh slept a fixed 1-2s after starting Xvfb, uvicorn, and Chromium before moving on. Those numbers encode local container-daemon timing; on a remote sandbox the next step can run against a service that is not listening yet, and in remote-browser mode the script declared the CDP bridge ready after a bare `sleep 1`. Each step now polls for the thing it needs — the X socket, the runtime server, Chromium's CDP endpoint — with a bounded timeout that reports which component never came up. CLAWBENCH_RUNTIME_WAIT_TIMEOUT_S overrides the 60s default for slow providers.
Adds a `-e e2b` section covering what the generated tasks assume about a sandbox (no bind mounts outside the task dir, no local X11 or GPU, polled readiness), the rule that every credential stays an env reference resolved by --env-file/--ve, and the per-trial resource floor. States plainly that the end-to-end e2b smoke run in TIGER-AI-Lab#331 is still open. Tests assert the healthcheck and setup script share one readiness contract in both local and kernel modes, that startup polls rather than sleeps, and that a generated task carries no host paths or baked secrets.
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.
Advances #331 §3. Does not close it — the end-to-end
-e e2bsmoke run is still open work, and I have no e2b account to do it with.Scoping
#331 says §1–§2 are largely done on
feat/harbor-hub-registryand that "§3 (e2b) and the parity run in §4 are the open work", so this deliberately touches neither the task format nor the registry — no conflict with that branch. Of §3's checklist:--ve/--env-fileonly; resource floor documentedharbor-0.20-port-notes.mdconclusion (E2B needs no local disk) folded intodocs/harbor.mdharbor run ... -e e2bsmoke task — not done, needs an accountThe bug this fixes
Setup accepted any 200 from
/api/statusas "runtime ready":But
/api/statusstarts answering as soon as uvicorn binds, whileeval_interceptor_readyonly flips totrueafter the CDP handler connects and sendsTarget.setAutoAttach(runtime-server/server.py:296) — a loop that retries on 1s sleeps. A task that starts in that window runs with interception inactive and cannot score Stage 1, and the result reads as an agent miss rather than a harness fault. The window is small on a warm local daemon and much wider on a cold remote sandbox, which is why it surfaces here.task.toml's[steps.healthcheck]already checked the right condition. The two now share oneruntime_ready_command(), so they cannot drift. That also fixed a latent escaping trap: the healthcheck string is written into a TOML basic string where\"unescapes to", but the same literal pasted into a shell script keeps the backslash and thegrepwould never match. The shell form is now canonical andtask_tomlescapes it once withjson.dumps— the emittedtask.tomlis byte-identical to before (verified againstmain).Also raises the setup wait from 60s to a configurable 180s, and makes a timeout tail the runtime-server log instead of printing one opaque line.
Fixed sleeps → polling
start-runtime.shslept 1–2s after starting Xvfb, uvicorn, and Chromium. Those constants encode local daemon timing; in remote-browser mode the script declared the CDP bridge ready after a baresleep 1. Each step now polls for the thing it needs with a bounded timeout that names the component that never came up.CLAWBENCH_RUNTIME_WAIT_TIMEOUT_Soverrides the 60s per-component default.Testing
Five new cases in
tests/test_harbor_adapter.py: the healthcheck and setup script share one readiness contract in bothlocalandkernelmodes and are escaped correctly for their respective consumers; the setup wait is remote-sized and reports what failed; startup polls rather than sleeps (no baresleep <n>left standing in for a readiness check); and a generated task carries no host paths and no baked secrets — every credential intask.tomlis an env reference.Full suite passes locally (
280 passed, 10 skipped);ruffandpyrightclean.Not verified against a live Harbor run — these are static assertions on the generated dataset plus a shell-syntax check. Worth one local
harbor runon the smoke dataset before merge, since the setup gate is now strictly stricter than it was.