diff --git a/.claude/docs/REPO_WALKTHROUGH.md b/.claude/docs/REPO_WALKTHROUGH.md index 63548b51b..bbec02843 100644 --- a/.claude/docs/REPO_WALKTHROUGH.md +++ b/.claude/docs/REPO_WALKTHROUGH.md @@ -211,7 +211,7 @@ rfcs/ │ └── tester.md # Write high-signal, failing tests │ └── hooks/ # Automation scripts - ├── lint.sh # Run ruff format check + ├── lint.sh # Check CI import sort, format, and lint (src/ tests/) ├── test.sh # Run pytest ├── check-debug.sh # Find debug code ├── post-push-pr.sh # Validate PR after push (freshness, CI, conflicts) diff --git a/.claude/hooks/lint.sh b/.claude/hooks/lint.sh index ade5d9751..9e09ef2c7 100755 --- a/.claude/hooks/lint.sh +++ b/.claude/hooks/lint.sh @@ -1,13 +1,6 @@ #!/bin/bash -# Lint check for OpenEnv -# Replicates the exact arc f pipeline from fbsource: -# 1. usort format — sort imports (matches arc f's usort pass) -# 2. ruff format — code formatting, line-length 88 (matches arc f's ruff-api pass) -# 3. ruff check — lint rules (E, F, W) -# -# usort is scoped to src/ and tests/ only. envs/ uses ruff format only -# because standalone usort and pyfmt's usort disagree on import ordering -# inside try/except blocks in some env files. +# Match CI: usort format, then ruff format, then ruff check on src/ and tests/. +# Format disposable copies so uncommitted edits are never overwritten. set -e @@ -19,18 +12,30 @@ if ! command -v uv &> /dev/null; then fi echo "=== Running import sort + format check ===" -# Ask the same tools as arc f whether the tree is formatted, without writing to -# it. Formatting in place and then running `git checkout --` to undo it also -# reverts the author's uncommitted edits in those files, and leaves reformatted -# Markdown behind, because the restore only ever covered *.py. +# CI checks the combined result: ruff can undo an intermediate usort change. +FORMAT_DIR=$(mktemp -d) +trap 'rm -rf "$FORMAT_DIR"' EXIT +cp -RL src tests pyproject.toml "$FORMAT_DIR/" +if [ -f .gitignore ]; then + cp .gitignore "$FORMAT_DIR/" +fi +# Ruff only respects .gitignore inside a Git repository. +git -C "$FORMAT_DIR" init -q +uv run usort format "$FORMAT_DIR/src/" "$FORMAT_DIR/tests/" +uv run ruff format "$FORMAT_DIR/src/" "$FORMAT_DIR/tests/" + FORMAT_FAILED=0 -uv run usort check src/ tests/ || FORMAT_FAILED=1 -uv run ruff format --check src/ tests/ envs/ || FORMAT_FAILED=1 +while IFS= read -r -d '' file; do + if ! cmp -s "$file" "$FORMAT_DIR/$file"; then + echo "$file" + FORMAT_FAILED=1 + fi +done < <(find -L src tests -type f -name '*.py' -print0) if [ "$FORMAT_FAILED" -ne 0 ]; then echo "" echo "ERROR: the files listed above need formatting." - echo "Run: uv run usort format src/ tests/ && uv run ruff format src/ tests/ envs/" + echo "Run: uv run usort format src/ tests/ && uv run ruff format src/ tests/" exit 1 fi echo "Import sort + format check passed!" diff --git a/CLAUDE.md b/CLAUDE.md index 061a14ed1..ab4c3a9ea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -292,7 +292,7 @@ docker build -t echo-env:latest -f envs/echo_env/server/Dockerfile . Scripts in `.claude/hooks/` are used by skills and can be run directly: ```bash -bash .claude/hooks/lint.sh # Run ruff format check +bash .claude/hooks/lint.sh # Check CI import sort, format, and lint (src/ tests/) bash .claude/hooks/test.sh # Run pytest (excludes special envs) bash .claude/hooks/check-debug.sh # Find debug code (print, breakpoint, TODO) bash .claude/hooks/post-push-pr.sh # Validate PR after push (freshness, CI, conflicts) diff --git a/docs/source/environments/harbor.md b/docs/source/environments/harbor.md index 9f1fdc9d6..418b14383 100644 --- a/docs/source/environments/harbor.md +++ b/docs/source/environments/harbor.md @@ -384,7 +384,7 @@ Exit codes: | code | meaning | |---|---| | `0` | success | -| `1` | ran, but failed. `rollout` returns this if **any** rollout in the batch was unusable | +| `1` | ran, but failed. `rollout` returns this if **any** rollout in the batch was unusable; `info --harness` returns it if construction or wiring checks fail | | `2` | usage error: a missing or invalid flag. Nothing ran | The `1` and `2` split matters if you are scripting this: `2` means the command never started, so @@ -392,7 +392,9 @@ retrying it unchanged will fail the same way. ### `openenv harbor info` -Report what this machine can run. Read-only: boots no sandbox, starts no server, makes no rollout. +Report what this machine can run. Boots no sandbox, starts no server, makes no rollout. +With `--harness`, imports and constructs that agent locally using dummy capture credentials and +temporary logs. Only check code you trust; its constructor runs, but its setup and run methods do not. | flag | type | default | meaning | |---|---|---|---| @@ -400,11 +402,13 @@ Report what this machine can run. Read-only: boots no sandbox, starts no server, | `--model` | str | `""` | Served model id. Auto-detected when the endpoint serves exactly one | | `--dataset` | str | none | Dataset spec. Repeatable, or comma-separated | | `--env-file` | path | `""` | dotenv with provider credentials, loaded before the checks | +| `--harness` | str | `""` | Check a seam name or trusted `module:Class`; show its capture wiring and try constructing it | | `--verbose` | flag | off | List all 30 harnesses, not only the 16 validated | | `--json` | flag | off | Emit machine-readable JSON instead of the text report | ```bash openenv harbor info --llm-url $LLM --dataset org/tasks --json +openenv harbor info --harness my_package.agent:MyAgent --model org/model --json ``` The JSON has four top-level keys: `llm`, `sandboxes`, `datasets`, `harnesses`. Each sandbox entry is @@ -415,6 +419,12 @@ openenv harbor info --llm-url $LLM --json \ | jq -r '.sandboxes[] | select(.available) | .name' ``` +`--harness` adds a `harness` object with `ok` and either the dummy wiring or an error in `detail`. +It catches import errors, incorrect subclasses, constructor failures, and host-side agents that +declare neither `api_base` nor `api_key` as constructor parameters or options fields. A successful +check confirms construction only; it does not validate model calls, capture or sandbox installation. +Without `--model` or an endpoint's detected model, the check uses `preflight-model`. + ### `openenv harbor rollout` Run rollouts with no env server involved. This is the debugging path: if `rollout` works and `serve` @@ -552,11 +562,29 @@ ids and logprobs, and where the agent emits a trajectory, its own record agreed Supporting four dialects rather than chat-completions alone is what makes the last four rows work. -`terminus-2` runs in the server process rather than inside the sandbox, so it reaches the proxy on -localhost and needs no public URL. +`terminus-2` and host-side agents of your own run in the server process rather than inside the +sandbox, so they are handed the proxy's localhost URL and need no public one. The other 14 known agents have a seam but are untested; run `openenv harbor info --verbose` to list -them. Anything Harbor supports can be reached with `--harness module:Class`. +them. + +### Your own agent + +Name any Harbor agent class by import path, `--harness my_package.agent:MyAgent`, in `rollout`, in +`serve` rollouts and from a trainer. Naming a class imports its module, so `serve` accepts only the +import paths its operator lists with `--allow-harness my_package.agent:MyAgent`. How the class is +pointed at the proxy depends on what it is: + +| the class | what it gets | +|---|---| +| a subclass of an agent above (its `name()` is a known harness) | that agent's wiring. If OpenEnv runs that agent through its own subclass, such as `openenv.harbor.install_fixes:InterceptPi` for pi, subclass that one instead: the error names it | +| a host-side agent (a `BaseAgent` that calls the model from the server process) | `api_base` (the proxy's `/v1` URL, on localhost) and `api_key` (the rollout's session id) as constructor kwargs | +| any other agent installed into the sandbox | refused: add a seam for it in `openenv/harbor/seams.py` | + +A host-side agent declares both kwargs in its options and sends every model call to `api_base` with +`api_key` as the bearer token; capture, reward and training traces then work as for any other +harness. Nothing reaches the sandbox or the verifier. Options the server has no way to pass, such as +provider extras, are best read from the agent's own environment variables. ## Supported sandboxes diff --git a/docs/source/tutorials/harbor-harness.md b/docs/source/tutorials/harbor-harness.md index 931387436..806b8196e 100644 --- a/docs/source/tutorials/harbor-harness.md +++ b/docs/source/tutorials/harbor-harness.md @@ -20,6 +20,17 @@ openenv harbor rollout \ Against a hosted provider you get an evaluation rollout: the reward and the full trace. Against vLLM or SGLang you also get the token ids and logprobs that training needs. `openenv harbor info` checks first which agents, sandboxes and capture level this machine can use. +Before running an agent of your own, check its wiring and constructor without starting a sandbox: + +```bash +openenv harbor info --harness my_package.agent:MyAgent --model org/model --json +``` + +This imports and constructs the named class with dummy capture credentials; only name code you +trust. It reports errors such as a missing constructor setting or undeclared `api_base`/`api_key`, +and returns exit code 1 on failure. Success confirms construction, while model calls and capture +remain untested. See [Your own agent](../environments/harbor#your-own-agent) for the contract. + ## Train on the Captures Each rollout comes back as a `TrainingTrace`: the token ids, logprobs and loss masks of every model call, plus the task's reward. It doesn't depend on a trainer, so any framework can train on it. TRL has a worked example: `AsyncGRPOTrainer` with a `HarnessRolloutWorker` runs each task to completion, reads back the trace, and syncs the new weights into the same vLLM server. diff --git a/envs/harbor_env/README.md b/envs/harbor_env/README.md index f6b9a3e22..67a6d9899 100644 --- a/envs/harbor_env/README.md +++ b/envs/harbor_env/README.md @@ -396,7 +396,7 @@ Exit codes: | code | meaning | |---|---| | `0` | success | -| `1` | ran, but failed. `rollout` returns this if **any** rollout in the batch was unusable | +| `1` | ran, but failed. `rollout` returns this if **any** rollout in the batch was unusable; `info --harness` returns it if construction or wiring checks fail | | `2` | usage error: a missing or invalid flag. Nothing ran | The `1` and `2` split matters if you are scripting this: `2` means the command never started, so @@ -404,7 +404,9 @@ retrying it unchanged will fail the same way. ### `openenv harbor info` -Report what this machine can run. Read-only: boots no sandbox, starts no server, makes no rollout. +Report what this machine can run. Boots no sandbox, starts no server, makes no rollout. +With `--harness`, imports and constructs that agent locally using dummy capture credentials and +temporary logs. Only check code you trust; its constructor runs, but its setup and run methods do not. | flag | type | default | meaning | |---|---|---|---| @@ -412,11 +414,13 @@ Report what this machine can run. Read-only: boots no sandbox, starts no server, | `--model` | str | `""` | Served model id. Auto-detected when the endpoint serves exactly one | | `--dataset` | str | none | Dataset spec. Repeatable, or comma-separated | | `--env-file` | path | `""` | dotenv with provider credentials, loaded before the checks | +| `--harness` | str | `""` | Check a seam name or trusted `module:Class`; show its capture wiring and try constructing it | | `--verbose` | flag | off | List all 30 harnesses, not only the 16 validated | | `--json` | flag | off | Emit machine-readable JSON instead of the text report | ```bash openenv harbor info --llm-url $LLM --dataset org/tasks --json +openenv harbor info --harness my_package.agent:MyAgent --model org/model --json ``` The JSON has four top-level keys: `llm`, `sandboxes`, `datasets`, `harnesses`. Each sandbox entry is @@ -427,6 +431,12 @@ openenv harbor info --llm-url $LLM --json \ | jq -r '.sandboxes[] | select(.available) | .name' ``` +`--harness` adds a `harness` object with `ok` and either the dummy wiring or an error in `detail`. +It catches import errors, incorrect subclasses, constructor failures, and host-side agents that +declare neither `api_base` nor `api_key` as constructor parameters or options fields. A successful +check confirms construction only; it does not validate model calls, capture or sandbox installation. +Without `--model` or an endpoint's detected model, the check uses `preflight-model`. + ### `openenv harbor rollout` Run rollouts with no env server involved. This is the debugging path: if `rollout` works and `serve` @@ -564,11 +574,29 @@ ids and logprobs, and where the agent emits a trajectory, its own record agreed Supporting four dialects rather than chat-completions alone is what makes the last four rows work. -`terminus-2` runs in the server process rather than inside the sandbox, so it reaches the proxy on -localhost and needs no public URL. +`terminus-2` and host-side agents of your own run in the server process rather than inside the +sandbox, so they are handed the proxy's localhost URL and need no public one. The other 14 known agents have a seam but are untested; run `openenv harbor info --verbose` to list -them. Anything Harbor supports can be reached with `--harness module:Class`. +them. + +### Your own agent + +Name any Harbor agent class by import path, `--harness my_package.agent:MyAgent`, in `rollout`, in +`serve` rollouts and from a trainer. Naming a class imports its module, so `serve` accepts only the +import paths its operator lists with `--allow-harness my_package.agent:MyAgent`. How the class is +pointed at the proxy depends on what it is: + +| the class | what it gets | +|---|---| +| a subclass of an agent above (its `name()` is a known harness) | that agent's wiring. If OpenEnv runs that agent through its own subclass, such as `openenv.harbor.install_fixes:InterceptPi` for pi, subclass that one instead: the error names it | +| a host-side agent (a `BaseAgent` that calls the model from the server process) | `api_base` (the proxy's `/v1` URL, on localhost) and `api_key` (the rollout's session id) as constructor kwargs | +| any other agent installed into the sandbox | refused: add a seam for it in `openenv/harbor/seams.py` | + +A host-side agent declares both kwargs in its options and sends every model call to `api_base` with +`api_key` as the bearer token; capture, reward and training traces then work as for any other +harness. Nothing reaches the sandbox or the verifier. Options the server has no way to pass, such as +provider extras, are best read from the agent's own environment variables. ## Supported sandboxes diff --git a/src/openenv/cli/commands/harbor.py b/src/openenv/cli/commands/harbor.py index 7125fc963..420ef3c7b 100644 --- a/src/openenv/cli/commands/harbor.py +++ b/src/openenv/cli/commands/harbor.py @@ -187,6 +187,67 @@ def _hub_not_found(exc: Exception) -> bool: return getattr(response, "status_code", None) == 404 +def _check_harness(harness: str, model: str) -> dict[str, Any]: + """Construct the rollout's agent with dummy capture credentials, without setup or run.""" + import inspect + import io + + result: dict[str, Any] = {"name": harness, "ok": False} + try: + # Imports, constructors and the process-env warning may print. Keep --json readable. + with ( + contextlib.redirect_stdout(io.StringIO()), + tempfile.TemporaryDirectory() as tmp, + ): + from harbor.agents.factory import AgentFactory + from openenv.harbor import seams + from openenv.harbor.rollout import build_trial_config, process_env + + seam = seams.get(harness) + url, session = "http://127.0.0.1:9", "openenv-harness-preflight" + *_, proc_env = seam.resolve(base_url=url, session=session, model=model) + config = build_trial_config( + task_dir=tmp, + harness=harness, + sandbox="docker", + intercept_url=url, + session_id=session, + model=model, + trial_name="preflight", + trials_dir=tmp, + ) + with process_env(harness, proc_env): + agent = AgentFactory.create_agent_from_config( + config.agent, logs_dir=Path(tmp) + ) + if seam.host_side and agent.name() not in seams.SEAMS: + accepted = set(inspect.signature(type(agent).__init__).parameters) + # Harbor 0.22 predates options models; constructor parameters still work. + options = getattr(agent, "options", None) + if options is not None: + accepted.update(type(options).model_fields) + missing = {"api_base", "api_key"} - accepted + if missing: + raise ValueError( + f"{harness} must declare {', '.join(sorted(missing))} as constructor " + "parameters or options fields; Harbor can silently ignore these kwargs." + ) + result.update( + ok=True, + import_path=seam.import_path, + dialect=seam.dialect, + host_side=seam.host_side, + status=seam.status, + model=config.agent.model_name, + kwargs=config.agent.kwargs, + agent_env=config.agent.env, + process_env=proc_env, + ) + except (Exception, SystemExit) as exc: # noqa: BLE001 - a failed check is report data + result["detail"] = f"{type(exc).__name__}: {exc}" + return result + + @app.command("info") def info( llm_url: Annotated[str, typer.Option("--llm-url", help=_LLM_HELP_OPTIONAL)] = "", @@ -207,6 +268,14 @@ def info( env_file: Annotated[ str, typer.Option("--env-file", help="dotenv file with provider credentials.") ] = "", + harness: Annotated[ + str, + typer.Option( + "--harness", + help="Check a seam name or trusted `module:Class` by constructing it with dummy " + "capture credentials. Runs no agent or sandbox.", + ), + ] = "", verbose: Annotated[ bool, typer.Option("--verbose", help="List every harness, not only validated ones."), @@ -228,11 +297,23 @@ def info( api_key=api_key or None, auth_header=auth_header, ) - print( - json.dumps(caps.to_dict(), indent=2) - if json_output - else caps.render(verbose=verbose) + checked = ( + _check_harness(harness, model or caps.llm.get("model") or "preflight-model") + if harness + else None ) + if json_output: + data = caps.to_dict() + if checked is not None: + data["harness"] = checked + print(json.dumps(data, indent=2)) + else: + print(caps.render(verbose=verbose)) + if checked is not None: + print("\nharness preflight (construction only; capture is untested)") + print(json.dumps(checked, indent=2)) + if checked is not None and not checked["ok"]: + raise typer.Exit(1) @app.command("rollout") @@ -399,6 +480,16 @@ def serve( reward_key: Annotated[ str, typer.Option("--reward-key", help=_REWARD_KEY_HELP) ] = "", + allow_harness: Annotated[ + Optional[list[str]], + typer.Option( + "--allow-harness", + help=( + "Accept this `module:Class` harness in clients' rollouts. Naming a class imports " + "its module on this server, so other import paths are refused. Repeatable." + ), + ), + ] = None, ) -> None: """Serve Harbor tasks over the OpenEnv Task API and MCP. @@ -422,6 +513,8 @@ def serve( ) # The UI's laptop defaults (this machine's token, private URLs) are for a loopback-only server. os.environ["OPENENV_HARBOR_UI_HOST"] = host + if allow_harness: + os.environ["OPENENV_HARBOR_ALLOW_HARNESS"] = ",".join(allow_harness) serve_harbor( llm_url=llm_url, model=model or None, diff --git a/src/openenv/harbor/capabilities.py b/src/openenv/harbor/capabilities.py index 2c0981d7b..7da90b98b 100644 --- a/src/openenv/harbor/capabilities.py +++ b/src/openenv/harbor/capabilities.py @@ -21,11 +21,6 @@ from . import seams -# Harbor agents that run HOST-SIDE, in this server's process, rather than being installed into the -# sandbox. The distinction is not cosmetic: their LLM traffic never leaves the host, so they reach -# the capture proxy on localhost and need no public URL at all. -HOST_SIDE_AGENTS = frozenset({"terminus-2", "computer-1", "oracle", "nop", "dspy-rlm"}) - # Backends worth advertising. Harbor registers 23; these are the ones with a credential story we # check and have exercised. Others still work via `--sandbox `, just unadvertised. KNOWN_SANDBOXES = ("docker", "e2b", "modal", "daytona", "hf-sandbox") @@ -288,7 +283,7 @@ def list_harnesses() -> list[HarnessStatus]: HarnessStatus( name=name, dialect=seam.dialect, - kind="base" if name in HOST_SIDE_AGENTS else "installed", + kind="base" if seam.host_side else "installed", status=seam.status, needs_subclass=seam.import_path is not None, notes=seam.notes, diff --git a/src/openenv/harbor/environment.py b/src/openenv/harbor/environment.py index 50e6963c5..dff376397 100644 --- a/src/openenv/harbor/environment.py +++ b/src/openenv/harbor/environment.py @@ -249,6 +249,18 @@ async def _run_rollout( from .models import HarborRolloutResult from .rollout import run_rollout as _run + # The client names the harness, and a `module:Class` name makes this server import that + # module. Only the import paths the operator allowed at startup are accepted. + allowed = os.environ.get("OPENENV_HARBOR_ALLOW_HARNESS", "").split(",") + if ":" in harness and harness not in allowed: + return HarborRolloutResult( + ok=False, + error=f"harness {harness!r} is not allowed on this server: start it with " + f"`openenv harbor serve --allow-harness {harness}`", + harness=harness, + sandbox=sandbox, + ).model_dump_json() + service = self._service() if service is None: return HarborRolloutResult( @@ -310,6 +322,7 @@ async def _resolve_and_run(): sandbox=sandbox, registry=service.capture.registry, intercept_url=service.public_url, + local_intercept_url=service.local_url, model=served, trials_dir=_trials_dir(), dataset=split, diff --git a/src/openenv/harbor/rollout.py b/src/openenv/harbor/rollout.py index a8d5eb2ff..4dd567f3a 100644 --- a/src/openenv/harbor/rollout.py +++ b/src/openenv/harbor/rollout.py @@ -330,6 +330,7 @@ async def run_rollout( trials_dir: Path | str, dataset: str = "", reward_key: str = "", + local_intercept_url: str = "", require_reward: bool = True, keep_sandbox: bool = False, agent_timeout_sec: float | None = None, @@ -358,6 +359,10 @@ async def run_rollout( The live capture registry; a session is minted here and its id becomes the agent's key. intercept_url (`str`): Public URL of the capture proxy, as the sandbox must reach it. + local_intercept_url (`str`, *optional*): + The proxy's loopback URL. A host-side agent (`Seam.host_side`) is handed this instead of + `intercept_url`: it calls the model from this process, and every public hop is one more + place a long completion can be cut. Empty keeps `intercept_url` for every agent. model (`str`): Served model id. Normalised per harness by the seam. trials_dir (`Path` or `str`): @@ -473,6 +478,16 @@ async def run_rollout( enable_shared_templates() + # A host-side agent calls the model from this process, so it takes the loopback URL. A + # public forward is one more hop that can cut a long completion: a gradio share link + # answers 502 after about a minute while the upstream is still generating, the agent + # retries, and the capture records the same prompt again as a new root. + if ( + local_intercept_url + and seams.get(harness, profile=harness_profile).host_side + ): + intercept_url = local_intercept_url + config_kwargs = dict( task_dir=task_dir, harness=harness, diff --git a/src/openenv/harbor/runner.py b/src/openenv/harbor/runner.py index 93225a361..6b45e491b 100644 --- a/src/openenv/harbor/runner.py +++ b/src/openenv/harbor/runner.py @@ -153,6 +153,7 @@ async def run_batch( sandbox=sandbox, registry=capture.registry, intercept_url=public_url, + local_intercept_url=f"http://127.0.0.1:{port}", model=model, trials_dir=trials_dir, dataset=dataset, diff --git a/src/openenv/harbor/seams.py b/src/openenv/harbor/seams.py index b31d22bf0..99b245f84 100644 --- a/src/openenv/harbor/seams.py +++ b/src/openenv/harbor/seams.py @@ -109,6 +109,10 @@ def _pi(base_url: str, session: str, model: str) -> dict[str, Any]: logger = logging.getLogger(__name__) +# Harnesses already warned about an inexpressible step limit, in this process. The warning is a fact +# about the harness, not the rollout: repeating it for every rollout of a training run buries it. +_STEP_LIMIT_WARNED: set[str] = set() + @dataclass(frozen=True) class Seam: @@ -153,6 +157,9 @@ class Seam: # the loss step while every log line looks healthy. Harbor itself has no step cap — only a # timeout — and an earlier Qwen3-4B run micro-stepped to 451 turns and 10.1M prompt tokens. step_limit: Callable[[int], dict[str, Any]] | None = None + # The agent runs in OUR process (terminus-2, computer-1, a host-side agent of your own) and makes + # its model calls from here, so the proxy's loopback address reaches it: no public forward needed. + host_side: bool = False status: str = "untested" notes: str = "" @@ -183,11 +190,14 @@ def resolve( extra = _deep_merge(extra, self.training_kwargs) if step_limit: if self.step_limit is None: - logger.warning( - "%s has no native step-limit setting; the capture proxy enforces the " - "model-call budget and the agent timeout still applies", - self.name, - ) + if self.name not in _STEP_LIMIT_WARNED: + _STEP_LIMIT_WARNED.add(self.name) + logger.warning( + "%s has no native step-limit setting; the capture proxy enforces the " + "model-call budget and the agent timeout still applies (said once per " + "harness)", + self.name, + ) else: extra = _deep_merge(extra, self.step_limit(step_limit)) return self.model_fmt.format(model=model), extra, agent_env, proc_env @@ -430,6 +440,7 @@ def _terminus(base_url: str, session: str, model: str) -> dict[str, Any]: # --- second wave -------------------------------------------------------- "terminus-2": Seam( name="terminus-2", + host_side=True, status="validated", dialect="openai_chat", model_fmt="hosted_vllm/{model}", @@ -690,6 +701,7 @@ def _terminus(base_url: str, session: str, model: str) -> dict[str, Any]: # reason is RECORDED rather than assumed. "computer-1": Seam( name="computer-1", + host_side=True, dialect="openai_chat", model_fmt="hosted_vllm/{model}", # Host-side like terminus-2: drives litellm from our process with an `api_base` kwarg, so @@ -930,7 +942,88 @@ def _terminus(base_url: str, session: str, model: str) -> dict[str, Any]: ] +def _custom_kwargs(base_url: str, session: str, model: str) -> dict[str, Any]: + """A host-side agent of your own reaches the proxy through two constructor kwargs. + + The names are terminus-2's (`api_base`) and the OpenAI SDK's (`api_key`): an agent declares both + in its options and hands them to whatever client it uses. + """ + return {"api_base": f"{base_url}/v1", "api_key": session} + + +def custom(import_path: str) -> Seam: + """The seam for `--harness module:Class`, an agent Harbor builds from its import path. + + Harbor already constructs agents from an import path. What it cannot know is how to point one + at the capture proxy, and that depends only on what the class is: + + * a subclass of an agent with a seam (its `name()` is in `SEAMS`, e.g. your own opencode + subclass) is wired exactly like that agent. Where the seam's wiring lives in an OpenEnv + subclass (pi's `InterceptPi` writes models.json), yours must subclass that class; + * a host-side agent (a plain `BaseAgent`, like terminus-2) makes its model calls from THIS + process, so it gets `api_base` (the proxy's `/v1` URL) and `api_key` (the capture session + id) as constructor kwargs. Nothing reaches the sandbox or the verifier; + * any other agent installed into the sandbox is refused: there is no way to tell which env + var or config file it reads, and guessed wiring fails as a rollout with no model calls. + + The status is "untested" in every case: OpenEnv has no evidence about an agent it has not run. + """ + import importlib + from dataclasses import replace + + from harbor.agents.installed.base import BaseInstalledAgent + + module, _, attr = import_path.partition(":") + try: + cls = getattr(importlib.import_module(module), attr) + if not isinstance(cls, type): + raise TypeError(f"{attr} is not a class") + except Exception as exc: # noqa: BLE001 - surface why, whatever the import raised + raise KeyError( + f"cannot import harness {import_path!r}: {type(exc).__name__}: {exc}" + ) from exc + try: + parent = cls.name() + except Exception: # noqa: BLE001 - a name() that needs an instance is not a known agent + parent = None + if parent in SEAMS: + wired_by = SEAMS[parent].import_path + if wired_by: + wired_module, _, wired_attr = wired_by.partition(":") + if not issubclass( + cls, getattr(importlib.import_module(wired_module), wired_attr) + ): + raise ValueError( + f"{import_path} subclasses {parent}, which OpenEnv points at the capture " + f"proxy through {wired_by}. Subclass {wired_by} instead." + ) + return replace( + SEAMS[parent], + name=import_path, + import_path=import_path, + status="untested", + notes=f"{import_path} is a {parent} subclass, wired like {parent}.", + ) + if issubclass(cls, BaseInstalledAgent): + raise ValueError( + f"{import_path} is installed into the sandbox, and OpenEnv cannot tell how to point it " + "at the capture proxy. Subclass an agent that has a seam, or add a Seam for it to " + f"openenv.harbor.seams.SEAMS. Known: {sorted(SEAMS)}" + ) + return Seam( + name=import_path, + dialect="openai_chat", + import_path=import_path, + kwargs=_custom_kwargs, + host_side=True, + status="untested", + notes="Host-side agent of your own: gets the proxy as api_base/api_key kwargs.", + ) + + def get(name: str, *, profile: str | None = None) -> Seam: + if name not in SEAMS and ":" in name and profile is None: + return custom(name) if name not in SEAMS: raise KeyError(f"no seam for {name!r}. Known: {sorted(SEAMS)}") if profile is None: diff --git a/src/openenv/harbor/serving.py b/src/openenv/harbor/serving.py index 6c73e9936..36d44feca 100644 --- a/src/openenv/harbor/serving.py +++ b/src/openenv/harbor/serving.py @@ -139,6 +139,12 @@ def start(self) -> str: raise return self.public_url + @property + def local_url(self) -> str: + """The proxy's loopback URL, for agents running in this process. Empty when it is mounted on a + Space's app, whose port this service does not own.""" + return "" if self.mounted else f"http://127.0.0.1:{self.capture.port}" + def stop(self) -> None: """Tear both halves down, even if the first half refuses to go. diff --git a/tests/envs/test_harbor_rollout_contract.py b/tests/envs/test_harbor_rollout_contract.py index 289a61b44..5f8564ddc 100644 --- a/tests/envs/test_harbor_rollout_contract.py +++ b/tests/envs/test_harbor_rollout_contract.py @@ -393,6 +393,61 @@ async def run(self): ) +@pytest.mark.parametrize( + ("harness", "expected"), + [("terminus-2", "http://127.0.0.1:8100"), ("opencode", "https://tunnel.test")], +) +def test_only_a_host_side_agent_takes_the_loopback_url( + monkeypatch, tmp_path, harness, expected +): + """A host-side agent calls the model from this process. Through a public forward, a long + completion can be cut mid-generation (a gradio share link answers 502 after about a minute), + the agent retries and the capture records the prompt twice. Sandboxed agents still need the + public URL: loopback is the sandbox itself.""" + import asyncio + from types import SimpleNamespace + + from openenv.core.harness.capture.sessions import SessionRegistry + + trial_module = pytest.importorskip("harbor.trial.trial") + + class Trial: + @classmethod + async def create(cls, config): + return cls() + + async def run(self): + return SimpleNamespace( + verifier_result=SimpleNamespace(rewards={"reward": 1.0}) + ) + + seen = {} + + def build(**kwargs): + seen["url"] = kwargs["intercept_url"] + + monkeypatch.setattr(trial_module, "Trial", Trial) + monkeypatch.setattr(rollout, "build_trial_config", build) + asyncio.run( + rollout.run_rollout( + task_dir=tmp_path, + harness=harness, + sandbox="e2b", + registry=SessionRegistry(), + intercept_url="https://tunnel.test", + local_intercept_url="http://127.0.0.1:8100", + model="m", + trials_dir=tmp_path, + ) + ) + assert seen["url"] == expected + + +def test_host_side_seams(): + assert seams.get("terminus-2").host_side and seams.get("computer-1").host_side + assert not seams.get("opencode").host_side + + @pytest.mark.parametrize("strict", [True, False], ids=["train", "look"]) def test_ambiguous_rewards_fail_training_but_not_a_rollout_to_look_at( monkeypatch, tmp_path, strict diff --git a/tests/envs/test_harbor_seams.py b/tests/envs/test_harbor_seams.py index f7260a982..dec9478c9 100644 --- a/tests/envs/test_harbor_seams.py +++ b/tests/envs/test_harbor_seams.py @@ -6,6 +6,10 @@ from __future__ import annotations +import sys +import types + +import pytest from openenv.harbor import seams # --- step limits -------------------------------------------------------------------------------- @@ -59,8 +63,11 @@ def test_a_step_limit_merges_instead_of_replacing_a_seams_own_config(): assert kwargs["config"]["agent"]["step_limit"] == 7 -def test_a_harness_that_cannot_cap_steps_warns_rather_than_dropping_it(caplog): +def test_a_harness_that_cannot_cap_steps_warns_rather_than_dropping_it( + caplog, monkeypatch +): """Silently ignoring the cap would let a caller believe its rollouts are bounded.""" + monkeypatch.setattr(seams, "_STEP_LIMIT_WARNED", set()) with caplog.at_level("WARNING"): _, kwargs, _, _ = seams.get("opencode").resolve( base_url="http://proxy", session="s1", model="m", step_limit=20 @@ -69,6 +76,26 @@ def test_a_harness_that_cannot_cap_steps_warns_rather_than_dropping_it(caplog): assert "step_limit" not in str(kwargs) +def test_the_step_limit_warning_is_said_once_per_harness(caplog, monkeypatch): + """A training run resolves a seam for every rollout; one line per harness is enough.""" + monkeypatch.setattr(seams, "_STEP_LIMIT_WARNED", set()) + with caplog.at_level("WARNING"): + for session in ("s1", "s2", "s3"): + seams.get("opencode").resolve( + base_url="http://proxy", session=session, model="m", step_limit=20 + ) + seams.get("qwen-coder").resolve( + base_url="http://proxy", session="s4", model="m", step_limit=20 + ) + warned = [ + r.getMessage() + for r in caplog.records + if "no native step-limit" in r.getMessage() + ] + assert len(warned) == 2 + assert warned[0].startswith("opencode") and warned[1].startswith("qwen-coder") + + def test_copilot_byok_routes_without_a_github_model_service(): from openenv.harbor.seams import SEAMS @@ -93,6 +120,135 @@ def test_grok_primary_and_auxiliary_models_share_capture_endpoint(): assert env["OPENAI_API_KEY"] == "session-key" +@pytest.fixture +def custom_agents(monkeypatch): + """A module of agents to name by import path: one host-side, one opencode subclass, one unknown + installed agent, and subclasses of Harbor's pi and of OpenEnv's `InterceptPi`.""" + # Not bare "harbor": without Harbor (Python 3.11), `examples/harbor` imports under that name + # once another test puts `examples/` on sys.path. + pytest.importorskip("harbor.agents.base") + from harbor.agents.base import BaseAgent + from harbor.agents.installed.base import BaseInstalledAgent + from harbor.agents.installed.opencode import OpenCode + from harbor.agents.installed.pi import Pi + from openenv.harbor.install_fixes import InterceptPi + + module = types.ModuleType("custom_agents_for_test") + + class HostSideAgent(BaseAgent): + @staticmethod + def name() -> str: + return "my-host-side-agent" + + class MyOpenCode(OpenCode): + pass + + class UnknownInstalledAgent(BaseInstalledAgent): + @staticmethod + def name() -> str: + return "my-installed-agent" + + class MyPi(Pi): + pass + + class MyInterceptPi(InterceptPi): + pass + + for cls in (HostSideAgent, MyOpenCode, UnknownInstalledAgent, MyPi, MyInterceptPi): + setattr(module, cls.__name__, cls) + monkeypatch.setitem(sys.modules, module.__name__, module) + return module.__name__ + + +def test_a_host_side_agent_of_your_own_gets_the_proxy_as_kwargs(custom_agents): + """`--harness module:Class` is documented; it used to raise "no seam" for any import path.""" + name = f"{custom_agents}:HostSideAgent" + seam = seams.get(name) + model, kwargs, agent_env, proc_env = seam.resolve( + base_url="https://capture.test", session="session-key", model="Qwen/Qwen3.5-9B" + ) + assert seam.import_path == name + assert seam.status == "untested" + assert model == "Qwen3.5-9B" + assert kwargs == {"api_base": "https://capture.test/v1", "api_key": "session-key"} + # Nothing in the sandbox's or our process's environment, so nothing can reach a grader. + assert agent_env == {} and proc_env == {} + + +def test_a_subclass_of_a_known_agent_is_wired_like_it(custom_agents): + name = f"{custom_agents}:MyOpenCode" + seam = seams.get(name) + model, kwargs, _, _ = seam.resolve( + base_url="https://capture.test", session="session-key", model="m" + ) + assert seam.import_path == name + assert model == seams.SEAMS["opencode"].model_fmt.format(model="m") + provider = kwargs["opencode_config"]["provider"][seams.OPENCODE_PROVIDER] + assert provider["options"]["apiKey"] == "session-key" + + +def test_an_unknown_installed_agent_is_refused_rather_than_guessed(custom_agents): + with pytest.raises(ValueError, match="installed into the sandbox"): + seams.get(f"{custom_agents}:UnknownInstalledAgent") + + +def test_a_subclass_must_keep_the_openenv_class_that_wires_its_agent(custom_agents): + """pi reaches the proxy through `InterceptPi`, which writes models.json in setup. A subclass of + Harbor's own `Pi` would skip that and fail on `intercept_config`, so it is refused.""" + with pytest.raises(ValueError, match="install_fixes:InterceptPi"): + seams.get(f"{custom_agents}:MyPi") + seam = seams.get(f"{custom_agents}:MyInterceptPi") + _, kwargs, _, _ = seam.resolve( + base_url="https://capture.test", session="s", model="m" + ) + assert seam.import_path == f"{custom_agents}:MyInterceptPi" + assert kwargs["intercept_config"]["api_key"] == "s" + + +def test_a_bad_import_path_says_why(): + pytest.importorskip("harbor.agents.base") + with pytest.raises(KeyError, match="cannot import harness"): + seams.get("no_such_module_xyz:Agent") + + +async def test_serve_imports_only_the_harnesses_its_operator_allowed( + monkeypatch, tmp_path +): + """A client names the harness in `run_rollout`, and a `module:Class` name makes the server + import that module. Unless the operator allowed it at startup, it is refused before any import.""" + import json + + from openenv.harbor.environment import HarborEnvironment + from openenv.harbor.serving import HarborService + + (tmp_path / "client_named_agent.py").write_text("class Agent:\n pass\n") + monkeypatch.syspath_prepend(str(tmp_path)) + monkeypatch.setattr(HarborService, "_instance", None) + monkeypatch.delenv("OPENENV_HARBOR_ALLOW_HARNESS", raising=False) + env = object.__new__(HarborEnvironment) + + async def rollout_error() -> str: + result = await env._run_rollout( + split="", + task_index=0, + harness="client_named_agent:Agent", + sandbox="e2b", + reward_key="", + keep_sandbox=False, + force_build=False, + ) + return json.loads(result)["error"] + + assert "--allow-harness client_named_agent:Agent" in await rollout_error() + assert "client_named_agent" not in sys.modules + + monkeypatch.setenv( + "OPENENV_HARBOR_ALLOW_HARNESS", "other:Agent,client_named_agent:Agent" + ) + # Past the check, the rollout stops at the next one: no capture proxy runs here. + assert "no capture proxy" in await rollout_error() + + def test_antigravity_uses_the_existing_native_google_endpoint_setting(): from openenv.harbor.seams import SEAMS diff --git a/tests/scripts/test_lint_hook.py b/tests/scripts/test_lint_hook.py index 38673f492..af689c54a 100644 --- a/tests/scripts/test_lint_hook.py +++ b/tests/scripts/test_lint_hook.py @@ -1,13 +1,10 @@ # SPDX-License-Identifier: BSD-3-Clause -"""`.claude/hooks/lint.sh` must never modify the working tree. +"""The lint hook must match CI without modifying the working tree. -The hook says so itself ("Undo the formatting so the working tree stays -as-is"), and `/alignment-review` and `/pre-submit-pr` run it automatically, so -an author can trigger it without meaning to. It previously formatted in place -and then ran `git checkout --` on whatever changed, which restores from HEAD: -that discarded the formatting *and* any uncommitted edits in the same files, -and left reformatted Markdown behind because the restore only covered `*.py`. +`/alignment-review` and `/pre-submit-pr` run it automatically, so an author +can trigger it without meaning to. It previously formatted in place and then +ran `git checkout --`, discarding uncommitted edits in the same files. The same holds for the git hooks that `install.sh` writes: they run on every commit and push, with the author's uncommitted work in the tree. @@ -31,7 +28,8 @@ # The stub stands in for `uv`. In check mode it reports whether any target file # still carries the marker; otherwise it rewrites them, standing in for a -# formatter that modifies files in place. +# formatter that modifies files in place. PIPELINE_STABLE models a file that +# usort changes but ruff returns to its original formatting. STUB_UV = """#!/bin/bash mode=report for a in "$@"; do @@ -45,6 +43,15 @@ found=1 while IFS= read -r f; do + if [ "$2 $3" = "usort check" ] && grep -q PIPELINE_STABLE "$f"; then + found=0 + elif [ "$2 $3" = "usort format" ]; then + sed -i.bak 's/PIPELINE_STABLE/USORT_INTERMEDIATE/g' "$f" && rm -f "$f.bak" + elif [ "$2 $3" = "ruff format" ] && [ "$mode" != "check" ]; then + sed -i.bak 's/USORT_INTERMEDIATE/PIPELINE_STABLE/g' "$f" && rm -f "$f.bak" + elif [ "$2 $3" = "ruff check" ] && grep -q NEEDSLINT "$f"; then + found=0 + fi if grep -q NEEDSFORMAT "$f" 2>/dev/null; then found=0 if [ "$mode" != "check" ]; then @@ -73,9 +80,9 @@ def repo(tmp_path: pathlib.Path) -> pathlib.Path: (work / "tests").mkdir() (work / "envs" / "demo_env").mkdir(parents=True) - # Committed in a state the formatter wants to change, mirroring the 55 - # unformatted files that sit under envs/ on main. + (work / "pyproject.toml").write_text("[tool.ruff]\nline-length = 88\n") (work / "src" / "mod.py").write_text("# NEEDSFORMAT\nx = 1\n") + (work / "envs" / "demo_env" / "mod.py").write_text("# NEEDSFORMAT\nx = 1\n") (work / "envs" / "demo_env" / "README.md").write_text( "# Demo\n\n```python\n# NEEDSFORMAT\nx = 1\n```\n" ) @@ -137,10 +144,11 @@ def test_hook_preserves_uncommitted_python_edits(repo: pathlib.Path) -> None: """ target = repo / "src" / "mod.py" target.write_text(target.read_text() + "\n" + MARKER) + before = target.read_bytes() _run_hook(repo) - assert "uncommitted work" in target.read_text(), ( + assert target.read_bytes() == before, ( "lint.sh discarded uncommitted edits in src/mod.py. It formats in place " "and then runs `git checkout --` on every changed file, which restores " "from HEAD and takes the author's work with the formatting." @@ -163,12 +171,55 @@ def test_hook_leaves_no_files_modified(repo: pathlib.Path) -> None: ) -def test_hook_still_fails_when_formatting_is_needed(repo: pathlib.Path) -> None: +@pytest.mark.parametrize("scope", ["src", "tests"]) +def test_hook_still_fails_when_formatting_is_needed( + repo: pathlib.Path, scope: str +) -> None: """Not writing must not mean not reporting: the gate still has to fail.""" + (repo / "src" / "mod.py").write_text("x = 1\n") + (repo / scope / "mod.py").write_text("# NEEDSFORMAT\nx = 1\n") + result = _run_hook(repo) assert result.returncode != 0 assert "format" in (result.stdout + result.stderr).lower() + assert "uv run usort format src/ tests/ && uv run ruff format src/ tests/" in ( + result.stdout + ) + assert "envs/" not in result.stdout + + +def test_hook_checks_final_pipeline_output(repo: pathlib.Path) -> None: + """CI accepts files that usort changes and ruff restores.""" + target = repo / "src" / "mod.py" + target.write_text("# PIPELINE_STABLE\nx = 1\n") + before = target.read_bytes() + + result = _run_hook(repo) + + assert result.returncode == 0, result.stdout + result.stderr + assert target.read_bytes() == before + + +def test_hook_ignores_environment_formatting(repo: pathlib.Path) -> None: + """The lint CI job only formats and lints src/ and tests/.""" + (repo / "src" / "mod.py").write_text("x = 1\n") + + result = _run_hook(repo) + + assert result.returncode == 0, result.stdout + result.stderr + assert "NEEDSFORMAT" in (repo / "envs" / "demo_env" / "mod.py").read_text() + + +def test_hook_still_fails_on_lint_errors(repo: pathlib.Path) -> None: + """Formatting success must not bypass the lint gate.""" + (repo / "src" / "mod.py").write_text("# NEEDSLINT\nx = 1\n") + + result = _run_hook(repo) + + assert result.returncode != 0 + assert "Import sort + format check passed!" in result.stdout + assert "Lint check passed" not in result.stdout def test_hook_passes_when_everything_is_formatted(repo: pathlib.Path) -> None: diff --git a/tests/test_cli/test_harbor_info.py b/tests/test_cli/test_harbor_info.py new file mode 100644 index 000000000..fd5d28f31 --- /dev/null +++ b/tests/test_cli/test_harbor_info.py @@ -0,0 +1,191 @@ +"""Harness construction failures should surface before a paid rollout.""" + +import json +import os +import sys +import types +from pathlib import Path + +import pytest +from openenv.cli.commands.harbor import app +from openenv.harbor.capabilities import Capabilities +from typer.testing import CliRunner + + +@pytest.fixture +def info_only(monkeypatch): + # No endpoint, dataset download, sandbox checks or server are needed for these tests. + monkeypatch.setattr("openenv.harbor.startup.prepare", lambda **_: Capabilities()) + + +@pytest.fixture +def agents(monkeypatch): + pytest.importorskip("harbor.agents.base") + from harbor.agents.base import BaseAgent + from harbor.agents.installed.pi import Pi + + module = types.ModuleType("info_test_agents") + module.constructed = [] + + class SilentAgent(BaseAgent): + @staticmethod + def name(): + return "info-test-agent" + + def version(self): + return None + + async def setup(self, environment): + raise AssertionError("preflight must not set up an agent") + + async def run(self, instruction, environment, context): + raise AssertionError("preflight must not run an agent") + + class DeclaredAgent(SilentAgent): + def __init__(self, *, api_base, api_key, **kwargs): + super().__init__(**kwargs) + print("constructor output must not corrupt JSON") + module.constructed.append((api_base, api_key, self.logs_dir)) + + class BrokenAgent(DeclaredAgent): + def __init__(self, *, required_setting, **kwargs): + super().__init__(**kwargs) + + class WrongPi(Pi): + pass + + for cls in (DeclaredAgent, SilentAgent, BrokenAgent, WrongPi): + setattr(module, cls.__name__, cls) + monkeypatch.setitem(sys.modules, module.__name__, module) + return module + + +def test_info_without_a_harness_keeps_the_existing_json(info_only): + result = CliRunner().invoke(app, ["info", "--json"]) + assert result.exit_code == 0 + assert json.loads(result.stdout) == Capabilities().to_dict() + + +def test_info_constructs_with_dummy_capture_wiring(info_only, agents, monkeypatch): + monkeypatch.setenv("OPENAI_API_KEY", "operator-key") + # litellm (which Harbor's agents import) runs load_dotenv() when first imported, and picks up any .env + # above the checkout. Import it first, so the check below covers only what the preflight does to os.environ. + try: + import litellm # noqa: F401 + except ImportError: + pass + + before = dict(os.environ) + result = CliRunner().invoke( + app, + [ + "info", + "--harness", + "info_test_agents:DeclaredAgent", + "--model", + "org/model", + "--json", + ], + ) + assert result.exit_code == 0, result.output + checked = json.loads(result.stdout)["harness"] + assert checked["ok"] is True + assert checked["host_side"] is True + assert checked["status"] == "untested" + assert checked["model"] == "model" + assert checked["kwargs"] == { + "api_base": "http://127.0.0.1:9/v1", + "api_key": "openenv-harness-preflight", + } + assert checked["agent_env"] == checked["process_env"] == {} + assert dict(os.environ) == before + assert "operator-key" not in result.output + url, key, logs = agents.constructed[0] + assert url == checked["kwargs"]["api_base"] + assert key == checked["kwargs"]["api_key"] + assert not Path(logs).exists() + + +def test_info_accepts_declared_options(info_only, agents): + options = pytest.importorskip("harbor.agents.options") + + class Options(options.AgentOptions): + api_base: str + api_key: str + + class OptionsAgent(agents.SilentAgent): + options_model = Options + + agents.OptionsAgent = OptionsAgent + result = CliRunner().invoke( + app, ["info", "--harness", "info_test_agents:OptionsAgent", "--json"] + ) + assert result.exit_code == 0, result.output + assert json.loads(result.stdout)["harness"]["ok"] is True + + +@pytest.mark.parametrize( + ("agent", "detail"), + [ + ("SilentAgent", "must declare api_base, api_key"), + ("BrokenAgent", "required_setting"), + ("WrongPi", "install_fixes:InterceptPi"), + ("Missing", "cannot import harness"), + ], +) +def test_info_reports_bad_harnesses_with_a_failing_exit_code( + info_only, agents, agent, detail +): + result = CliRunner().invoke( + app, ["info", "--harness", f"info_test_agents:{agent}", "--json"] + ) + assert result.exit_code == 1, result.output + checked = json.loads(result.stdout)["harness"] + assert checked["ok"] is False + assert detail in checked["detail"] + + +def test_info_checks_the_installed_agent_using_the_rollout_configuration( + info_only, monkeypatch +): + pytest.importorskip("harbor.agents.base") + monkeypatch.setenv("OPENENV_HARBOR_AGENT_VERSIONS", '{"opencode": "1.18.30"}') + result = CliRunner().invoke(app, ["info", "--harness", "opencode", "--json"]) + assert result.exit_code == 0, result.output + checked = json.loads(result.stdout)["harness"] + assert checked["kwargs"]["version"] == "1.18.30" + assert checked["host_side"] is False + + +def test_info_text_explains_the_limit_of_a_successful_check(info_only, agents): + result = CliRunner().invoke( + app, ["info", "--harness", "info_test_agents:DeclaredAgent"] + ) + assert result.exit_code == 0 + assert "construction only; capture is untested" in result.stdout + + +@pytest.mark.parametrize("fails", [False, True]) +def test_info_restores_process_credentials_after_construction( + info_only, monkeypatch, fails +): + pytest.importorskip("harbor.agents.base") + from harbor.agents.factory import AgentFactory + + original = AgentFactory.create_agent_from_config + + def construct(config, **kwargs): + assert os.environ["ANTHROPIC_API_KEY"] == "openenv-harness-preflight" + if fails: + raise ValueError("constructor failed") + return original(config, **kwargs) + + monkeypatch.setattr(AgentFactory, "create_agent_from_config", construct) + monkeypatch.setenv("ANTHROPIC_API_KEY", "operator-key") + monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) + before = dict(os.environ) + result = CliRunner().invoke(app, ["info", "--harness", "claude-code", "--json"]) + assert result.exit_code == int(fails), result.output + assert json.loads(result.stdout)["harness"]["ok"] is not fails + assert "operator-key" not in result.output + assert dict(os.environ) == before