fix(sandbox): health-check on connect in the OpenSandbox provider

[hemildesai]

What

The OpenSandbox provider health-checks by default now:

• connect() and the reconnect in _connect_after_create() no longer hardcode skip_health_check=True; both derive it from configuration, whose default is False.
• Two shipped configs that turned the check off (litmus_agent.yaml, and the example in the mini_swe_agent_2 README) are flipped back on.

Why

A sandbox id only proves the workload exists, not that its exec daemon is listening. The server reports a sandbox ready once its pod is Running with an IP, which happens before the daemon binds its port. A handle returned without a health check defers that gap to the first real call, where it surfaces as:

502 {"code":"GENERAL::UNKNOWN_ERROR","message":"Could not connect to the backend sandbox endpoint=..."}

Because connect() skipped the check unconditionally, the setting was effectively opt-in rather than opt-out, and the two paths disagreed: create() honoured skip_health_check while connect() ignored it.

_verify_created_handle() did not cover this either — it is a no-op unless probe.command is configured.

This mirrors the reasoning already documented in the provider's own reference config (configs/opensandbox.yaml), which sets skip_health_check: false; this PR brings the code and the remaining configs in line with it.

Behaviour change

connect() now waits for the sandbox to answer instead of returning immediately, so it can raise where it previously returned an unusable handle. That is the intent: fail or wait at connect, rather than error on the first command.

The opt-out is preserved via skip_health_check for callers that deliberately want an unchecked handle.

Both updated configs also raise create.timeout_s above their ready_timeout_s, following the guidance in the reference config: that timeout bounds the whole create call, which now includes the readiness wait, so leaving the two equal would turn the wait into a timeout.

Testing

• Added test_connect_health_checks_by_default and test_connect_honours_skip_health_check_opt_out.
• Updated two existing assertions that encoded the old hardcoded value.
• tests/unit_tests/test_opensandbox_provider.py and tests/unit_tests/test_sandbox.py: all pass except test_resolve_provider_config_named_reference, which fails on ModuleNotFoundError: No module named 'omegaconf' and was confirmed pre-existing by reproducing it with this change stashed.

Local runs used a minimal virtualenv: this repo pins a Python version the local toolchain could not fetch, and one dependency does not build on the newer interpreter available. Full-matrix verification is left to CI.

🤖 Generated with Claude Code

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[hemildesai]

/claude review

[hemildesai]

/ok to test 6e6a63f

[claude]

SHIP — no reliability concerns.

Tightens OpenSandbox reconnect correctness. connect() previously hardcoded skip_health_check=True, so a reattach returned a handle whose exec daemon might not be listening yet — deferring the startup gap to a 502 on the first call. It now respects create.skip_health_check (default False), so reconnects health-check by default. Verified:

• connect() (provider.py:677) and _connect_after_create() (provider.py:846) now read from config. The latter is only reachable when skip_health_check is True (provider.py:927-928), so its effective value is unchanged — no regression on the post-create path.
• YAML timeout_s: 1500 > ready_timeout_s: 1200 is correct: the outer asyncio.wait_for(Sandbox.create(...), timeout=timeout_s) (provider.py:904) bounds the whole create including the SDK's readiness wait, so it must exceed ready_timeout_s. Comment reasoning holds.
• Both YAML configs (litmus_agent, mini_swe_agent_2 README) and the two provider tests plus the two sandbox tests are updated consistently with the new default.

Correctness-improving change with matching test coverage. No async, API-compat, or config-convention concerns.

[hemildesai]

/claude review

[claude]

SHIP — LGTM, no reliability concerns.

Correct, well-scoped fix. The OpenSandbox provider's reattach paths (connect() and _connect_after_create) previously hardcoded skip_health_check=True, which hands back a handle before the pod's exec daemon is listening — the first command then races and 502s, killing the rollout. The change threads self._create.skip_health_check (default False) through both, so reattach now health-checks unless explicitly opted out.

Checked and clean:

• OpenSandboxCreateConfig.skip_health_check already defaults to False (provider.py:425) — no config/default divergence, defaults live in YAML as required.
• _connect_after_create is only reached inside the skip_health_check is True branch of _create_once (provider.py:927), so passing the config value there preserves prior behavior for opt-out callers — no regression to the create-then-reconnect flow.
• Config comments assert create timeout_s (1500) must exceed spec ready_timeout_s (1200) since it now bounds the readiness wait; verified consistent in both litmus_agent.yaml and the mini_swe_agent_2 README, and matches the already-shipped default opensandbox.yaml.
• Tests cover both the default (health-check) and opt-out paths, and the two updated assertions in test_sandbox.py correctly track the new default.

No async, verifier, or public-API concerns.