diff --git a/e2e/bub/README.md b/e2e/bub/README.md index 61811a1a6..fa69bb170 100644 --- a/e2e/bub/README.md +++ b/e2e/bub/README.md @@ -139,7 +139,9 @@ Each selected workload writes the same layout: Shared runs write the same v1 files per source task under `batch-/tasks//`, plus one aggregate evaluation and report at `batch-/`. `collect-all` reports every failed task; `fail-fast` stops only that shared Harbor trial at its first failed step. Runtime batch steps are flat and task-prefixed. Each agent invocation starts an -independent ACP session and Bub tape. +independent ACP session and Bub tape. Bub keeps every tape as a file in its home, which Harbor does not clear between +steps, so before each invocation the harness removes the earlier tapes: Bub does not search another session's tape, but +an agent could read the files. ## Compare PowerContext off and on @@ -176,7 +178,9 @@ therefore runs only against a Server that requires authentication: it stops befo its Scopes to a client without a token. Start the Server with `POWERCONTEXT_SERVER_ACCESS_MODE=enforced` and a `POWERCONTEXT_SERVER_AUTH_TOKEN`, and give the harness the same value as `POWERCONTEXT_CLIENT_API_TOKEN`; the harness passes it to the ON arm's integration as `POWERCONTEXT_BUB_API_TOKEN` or `POWERCONTEXT__AUTHORIZATION`. The -token still lets an ON agent read other Scopes on the same Server, including earlier trials'. +harness holds that value in its own environment and gives Harbor a reference to it, so Harbor's job files record the +reference and no part of the token. The token still lets an ON agent read other Scopes on the same Server, including +earlier trials'. After each ON session the harness records the Scope's Server statistics. When another session follows, it first flushes the Scope, standing in for the time that passes between real sessions, and repeats the flush until the Scope @@ -192,7 +196,10 @@ and that the integration asked PowerContext for context during it. Otherwise it flush creates Memory and whether recall returns content are PowerContext's own behavior, so the snapshots record them but a run that gets nothing useful still counts as an ON attempt. Integration failures and harness or infrastructure errors are reported but left out of success rates and paired -differences. An agent timeout counts as a failed attempt in either arm. +differences. A session whose model request failed is such an error on every host: Codex and Claude Code exit non-zero, +Harbor reads OpenCode's error events, and the harness reads Pi's last message, because Pi exits 0 in the JSON mode +Harbor uses. The harness reads Pi's output through the logs that Harbor's Docker environment mounts and stops with an +error when the file is not there. An agent timeout counts as a failed attempt in either arm. The harness Client waits for each flush, which runs the Server's generation model, so raise its 10-second default timeout; the Bub plugin also flushes during a session. @@ -356,8 +363,8 @@ forwards other native `BUB_*` values without translating them. If the agent task container requires an outbound proxy, set `POWERCONTEXT_E2E_AGENT_PROXY_URL` to a URL reachable from that container. In the fixed nested-container harness, `host-gateway` addresses the harness container, so a -proxy exposed there can be passed as `http://host-gateway:`. The typed setting is also treated as a secret when -evidence is written. +proxy exposed there can be passed as `http://host-gateway:`. The URL can carry credentials, so the harness +treats it as a secret when evidence is written and gives Harbor a reference to it rather than the value. The agent container sees only the repository files that installation needs: the `powercontext` package and the host integration, and none of them in a paired OFF arm. Workload files, answer keys, and benchmark data stay on the host, @@ -397,8 +404,12 @@ The harness does not mirror PowerContext Server, PowerContext Client, Bub, Harbo loads its native parameters, and the adapter only forwards the native values needed across the nested-container boundary. The Bub plugin uses Bub's Pydantic settings extension and accepts the same fields in the `powercontext` section of `bub.yml`. Every `*_API_KEY`, `*_TOKEN`, `*_AUTHORIZATION`, and `*_SECRET_ACCESS_KEY` value in the harness -environment, including Bub's API keys and the PowerContext Client token, is redacted at every final evidence sink, -whatever its length. Only common placeholders for local model servers, such as `1` or `ollama`, are left in place, -because they protect nothing and redacting them by substring would rewrite the evidence. CI scans evidence with -TruffleHog before publishing it. Native ACP artifacts can contain arbitrary command output and should be reviewed before -sharing. +environment, including Bub's API keys and the PowerContext Client token, is redacted in every file the harness writes, +whatever its length. A name with `_TOKEN_` in the middle, such as `AWS_BEARER_TOKEN_BEDROCK`, counts too, and names +match in any case, but a name ending in `_FILE`, `_PATH`, or `_URL`, such as `AWS_WEB_IDENTITY_TOKEN_FILE`, says +where a token is and is left alone. Only common placeholders for local model servers, such as `1` or `ollama`, are +left in place, because they protect nothing and redacting them by substring would rewrite the evidence. CI scans +evidence with TruffleHog before publishing it. Harbor writes the files under `harbor-jobs/` itself, and the harness +does not redact them: the job configuration holds references to secrets rather than their values, but every host's +own output there can contain arbitrary command output, such as an agent printing its environment, and should be +reviewed before sharing. diff --git a/e2e/bub/src/powercontext_e2e/harbor_agent.py b/e2e/bub/src/powercontext_e2e/harbor_agent.py index e18ce6f35..0b7b3b7fd 100644 --- a/e2e/bub/src/powercontext_e2e/harbor_agent.py +++ b/e2e/bub/src/powercontext_e2e/harbor_agent.py @@ -28,6 +28,8 @@ AGENT_ID = "powercontext-bub-acp" REMOTE_BIN_DIR = "/installed-agent/bin" REMOTE_BUB_HOME = "/installed-agent/bub-home" +# Bub keeps every session's messages as JSONL in its home, and nothing else clears them between the steps of a trial. +BUB_TAPES = '"${BUB_HOME:?}/tapes"' REMOTE_BUB_PROJECT = "/installed-agent/bub-project" REMOTE_CODEX_AUTH = "/run/agent-auth/codex-auth.json" REMOTE_CODEX_HOME = "/installed-agent/codex" @@ -41,7 +43,10 @@ class PowerContextBubAcpAgent(harbor_acp.AcpAgent): - """Install Bub through its supported uv tool and plugin commands.""" + """Install Bub through its supported uv tool and plugin commands, and start every session without Bub's tapes. + + Bub does not search another session's tape, but an agent can read the files, so each session starts without them. + """ def __init__(self, **kwargs: Any) -> None: self._invocation_scopes = tuple(kwargs.pop("invocation_scopes", ())) @@ -64,7 +69,10 @@ def __init__(self, **kwargs: Any) -> None: @override async def run(self, instruction: str, environment: BaseEnvironment, context: AgentContext) -> None: try: + # The marker comes first: the verifier reads a missing marker as a passed step, so every failure + # after this line, including a failed removal, must leave it in place. await environment.exec(command=f"touch {STEP_FAILURE_MARKER}") + await self.exec_as_agent(environment, command=f"rm -rf {BUB_TAPES}") if not self._invocation_scopes: await super().run(instruction, environment, context) else: diff --git a/e2e/bub/src/powercontext_e2e/harbor_pi.py b/e2e/bub/src/powercontext_e2e/harbor_pi.py index 9b804683e..3591ecc6c 100644 --- a/e2e/bub/src/powercontext_e2e/harbor_pi.py +++ b/e2e/bub/src/powercontext_e2e/harbor_pi.py @@ -16,8 +16,10 @@ from __future__ import annotations +import json from typing import Any, override +from harbor.agents.installed.base import NonZeroAgentExitCodeError from harbor.agents.installed.pi import Pi from harbor.environments.base import BaseEnvironment from harbor.models.agent.context import AgentContext @@ -40,7 +42,7 @@ class PowerContextPiAgent(Pi): As on the other hosts, the OFF arm runs without the package. The package reads its Server URL, Scope, consent, and Server token from its own environment, which only the ON arm receives. Harbor runs Pi without a saved session, and each session starts without the tool output an earlier session saved, so neither arm can read an earlier session - from Pi's own files. + from Pi's own files. A session whose model request failed is an error rather than an attempt at the task. """ def __init__( @@ -85,6 +87,43 @@ async def install(self, environment: BaseEnvironment) -> None: async def run(self, instruction: str, environment: BaseEnvironment, context: AgentContext) -> None: await self.exec_as_agent(environment, command=f"rm -f {PI_TOOL_OUTPUT}") await super().run(instruction, environment, context) + if (failure := self._model_failure()) is not None: + raise NonZeroAgentExitCodeError(f"Pi's model request failed: {failure}") # noqa: TRY003 + + def _model_failure(self) -> str | None: + """Return why the session's last model request failed, or nothing when it completed. + + Harbor runs Pi in JSON mode, where Pi exits 0 after a failed or aborted model request. Pi's text mode exits 1 + on the same condition: the last message is an assistant message that stopped on an error. The output must be + on the host when this runs; see below. + """ + + output = self.logs_dir / self._OUTPUT_FILENAME + if not output.is_file(): + # Harbor's Pi agent writes the file in the container and downloads the agent's logs only after this + # method runs, so the check reads it through the bind mount of Harbor's Docker environment. An + # environment without that mount would otherwise pass every failed session as an attempt. + raise RuntimeError( # noqa: TRY003 + f"Pi's output {output} is not on the host: the harness reads it before Harbor downloads the " + "agent's logs, which requires an environment that mounts /logs" + ) + last: dict[str, Any] = {} + # Pi ends each record with LF. str.splitlines would also split at U+2028 and the other separators, which + # JSON leaves unescaped inside a string, and each half of a record split there is dropped as invalid JSON. + for line in output.read_text(encoding="utf-8", errors="replace").split("\n"): + try: + event = json.loads(line) + except json.JSONDecodeError: + continue + if ( + isinstance(event, dict) + and event.get("type") == "message_end" + and isinstance(event.get("message"), dict) + ): + last = event["message"] + if last.get("role") == "assistant" and last.get("stopReason") in ("error", "aborted"): + return str(last.get("errorMessage") or f"request {last['stopReason']}") + return None def install_plugin_command() -> str: diff --git a/e2e/bub/src/powercontext_e2e/hosts.py b/e2e/bub/src/powercontext_e2e/hosts.py index f5dfc5108..5e5cac0f1 100644 --- a/e2e/bub/src/powercontext_e2e/hosts.py +++ b/e2e/bub/src/powercontext_e2e/hosts.py @@ -31,6 +31,7 @@ from .harbor_opencode import OPENCODE_VERSION from .harbor_pi import PI_VERSION from .settings import ( + agent_secret, bub_environment, codex_auth_path, powercontext_bub_environment, @@ -135,7 +136,7 @@ def agent_config( "POWERCONTEXT_BUB_SCOPE_ID": scope_id, }) if (token := server_api_token()) is not None: - env["POWERCONTEXT_BUB_API_TOKEN"] = token + env["POWERCONTEXT_BUB_API_TOKEN"] = agent_secret("POWERCONTEXT_BUB_API_TOKEN", token) if invocation_scopes is not None: env.pop("POWERCONTEXT_BUB_SCOPE_ID") kwargs["invocation_scopes"] = invocation_scopes @@ -201,7 +202,8 @@ def agent_config( env = {**self._plugin_environment(), f"{self.plugin_prefix}SCOPE_ID": scope_id} if (token := server_api_token()) is not None: # Each plugin sends this value as its Authorization header. - env[f"{self.plugin_prefix}AUTHORIZATION"] = f"Bearer {token}" + authorization = f"{self.plugin_prefix}AUTHORIZATION" + env[authorization] = agent_secret(authorization, f"Bearer {token}") return AgentConfig( import_path=self.agent_import_path, model_name=self.agent_model(), diff --git a/e2e/bub/src/powercontext_e2e/runner.py b/e2e/bub/src/powercontext_e2e/runner.py index 2da622774..bd87053e8 100644 --- a/e2e/bub/src/powercontext_e2e/runner.py +++ b/e2e/bub/src/powercontext_e2e/runner.py @@ -61,7 +61,7 @@ TaskObservation, ) from .report import render_evaluation_summary -from .settings import HarnessSettings, ModelNotConfiguredError +from .settings import HarnessSettings, ModelNotConfiguredError, agent_secret FailurePolicy = Literal["fail-fast", "collect-all"] TaskStatus = Literal["completed", "failed", "skipped"] @@ -439,7 +439,8 @@ def _job_config( invocation_scopes=invocation_scopes if runtime is not None else None, ) if settings.agent_proxy_url is not None: - proxy_url = settings.agent_proxy_url.get_secret_value() + # Harbor writes a literal under these names to its job files in full, and the URL can carry credentials. + proxy_url = agent_secret("PROXY_URL", settings.agent_proxy_url.get_secret_value()) agent.env.update({ "HTTP_PROXY": proxy_url, "HTTPS_PROXY": proxy_url, diff --git a/e2e/bub/src/powercontext_e2e/settings.py b/e2e/bub/src/powercontext_e2e/settings.py index d8694610f..509455503 100644 --- a/e2e/bub/src/powercontext_e2e/settings.py +++ b/e2e/bub/src/powercontext_e2e/settings.py @@ -55,6 +55,33 @@ def server_api_token() -> str | None: return None if token is None else token.get_secret_value() +# Values the harness derives for an agent are held under this prefix, which no integration reads as its own +# setting, so reading an integration's native environment later never returns a derived value. +_AGENT_SECRET_PREFIX = "POWERCONTEXT_E2E_AGENT_SECRET_" # noqa: S105 - part of a variable name + + +def agent_secret(name: str, value: str) -> str: + """Hold a value the harness derives for an agent in the harness's own environment, and return a reference to it. + + ``name`` is the variable the agent reads, such as ``POWERCONTEXT_BUB_API_TOKEN``, or a short name for a value + that reaches the agent under several variables, such as ``PROXY_URL``. + + The value stays in this process's environment for the rest of the run, where every child process of the harness + inherits it. That exposes nothing new: Harbor resolves a reference from the host environment and nowhere else, + and every value held here derives from a setting that reaches the harness only through that same environment, + as ``POWERCONTEXT_CLIENT_API_TOKEN`` or ``POWERCONTEXT_E2E_AGENT_PROXY_URL``, which those children inherit + already. + Harbor writes each agent's environment to its job files. It keeps the first four and last three characters of a + sensitive literal, which is most of a short token, and writes a literal under any other name in full. It writes a + ``${NAME}`` reference as it is, whatever the name, and resolves the reference from this process's environment when + it starts the agent. A value held here is also an evidence secret. + """ + + held = f"{_AGENT_SECRET_PREFIX}{name.removeprefix('POWERCONTEXT_')}" + environ[held] = value + return f"${{{held}}}" + + def codex_auth_path() -> Path: """Resolve Codex's native authentication document location.""" @@ -62,12 +89,27 @@ def codex_auth_path() -> Path: _SECRET_SUFFIXES = ("_API_KEY", "_AUTHORIZATION", "_TOKEN", "_SECRET_ACCESS_KEY") +# A token can also be named in the middle, as in AWS_BEARER_TOKEN_BEDROCK. A plural, as in MAX_THINKING_TOKENS, is a +# count, and a name ending in one of _LOCATION_SUFFIXES, as in AWS_WEB_IDENTITY_TOKEN_FILE, says where a token is: +# its value is a path or an address, which redacting by substring would rewrite in the evidence. +_SECRET_INFIX = "_TOKEN_" # noqa: S105 - part of a variable name +_LOCATION_SUFFIXES = ("_FILE", "_PATH", "_URL") # Local model servers accept any key, and the placeholders commonly passed to them are ordinary words and numbers. # They protect nothing, and redacting them by substring would rewrite the evidence. Any other value is redacted, # however short. _PLACEHOLDER_CREDENTIALS = frozenset({"1", "true", "none", "null", "empty", "dummy", "ollama", "lm-studio"}) +def _names_a_secret(name: str) -> bool: + # Settings read their variables in any case, so POWERCONTEXT_CLIENT_API_TOKEN may be set in lower case. + name = name.upper() + if name.startswith(_AGENT_SECRET_PREFIX): + return True + if name.endswith(_LOCATION_SUFFIXES): + return False + return name.endswith(_SECRET_SUFFIXES) or _SECRET_INFIX in name + + class ModelNotConfiguredError(RuntimeError): """Report model-backed workloads whose host lacks its runtime model or another required setting.""" @@ -118,7 +160,7 @@ def evidence_secrets(self) -> tuple[str, ...]: values = { value for name, value in environ.items() - if name.endswith(_SECRET_SUFFIXES) and value and value.lower() not in _PLACEHOLDER_CREDENTIALS + if _names_a_secret(name) and value and value.lower() not in _PLACEHOLDER_CREDENTIALS } if self.agent_proxy_url is not None and (proxy_url := self.agent_proxy_url.get_secret_value()): values.add(proxy_url) diff --git a/e2e/bub/tests/conftest.py b/e2e/bub/tests/conftest.py new file mode 100644 index 000000000..aad1555ba --- /dev/null +++ b/e2e/bub/tests/conftest.py @@ -0,0 +1,28 @@ +# Copyright (c) 2026 OceanBase. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import annotations + +import os +from collections.abc import Iterator +from unittest.mock import patch + +import pytest + + +@pytest.fixture(autouse=True) +def harness_environment() -> Iterator[None]: + # The harness holds the secrets it derives for agents in its own environment, so each test gets its own copy. + with patch.dict(os.environ): + yield diff --git a/e2e/bub/tests/test_evidence_redaction.py b/e2e/bub/tests/test_evidence_redaction.py index 09bde2106..3e09e81bb 100644 --- a/e2e/bub/tests/test_evidence_redaction.py +++ b/e2e/bub/tests/test_evidence_redaction.py @@ -73,6 +73,10 @@ def test_resolved_instruction_evidence_matches_harbor_acp_summaries( "HF_TOKEN", "AWS_SECRET_ACCESS_KEY", "POWERCONTEXT_PI_AUTHORIZATION", + # Harbor's Claude Code agent forwards this one, which names the token in the middle. + "AWS_BEARER_TOKEN_BEDROCK", + # Settings read their variables in any case. + "powercontext_client_api_token", ) ), # The Client and the Server accept a token of any length. @@ -152,8 +156,25 @@ def test_short_placeholder_credentials_do_not_corrupt_evidence(monkeypatch) -> N monkeypatch.setenv("LOCAL_API_KEY", "1") monkeypatch.setenv("OLLAMA_API_KEY", "ollama") monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-provider-secret") - evidence = json.dumps({"reward": 1, "provider": "ollama", "error": "rejected sk-or-provider-secret"}) + # A plural names a count, which Harbor's Claude Code agent also forwards. + monkeypatch.setenv("MAX_THINKING_TOKENS", "8192") + # CI sets these to where a token is, not to the token: a path CI logs and an agent can print. + monkeypatch.setenv("AWS_WEB_IDENTITY_TOKEN_FILE", "/var/run/secrets/eks.amazonaws.com/serviceaccount/token") + monkeypatch.setenv("HF_TOKEN_PATH", "/home/runner/.cache/huggingface/token") + evidence = json.dumps({ + "reward": 1, + "provider": "ollama", + "max_bytes": 8192, + "error": "rejected sk-or-provider-secret", + "stderr": "open /var/run/secrets/eks.amazonaws.com/serviceaccount/token: no such file", + }) redacted = json.loads(redact(evidence, HarnessSettings())) - assert redacted == {"reward": 1, "provider": "ollama", "error": "rejected [REDACTED]"} + assert redacted == { + "reward": 1, + "provider": "ollama", + "max_bytes": 8192, + "error": "rejected [REDACTED]", + "stderr": "open /var/run/secrets/eks.amazonaws.com/serviceaccount/token: no such file", + } diff --git a/e2e/bub/tests/test_harbor_job_config.py b/e2e/bub/tests/test_harbor_job_config.py index b9fe81248..293e50a66 100644 --- a/e2e/bub/tests/test_harbor_job_config.py +++ b/e2e/bub/tests/test_harbor_job_config.py @@ -17,25 +17,36 @@ from __future__ import annotations import asyncio +import json import os from pathlib import Path from typing import NamedTuple import pytest +from harbor.agents.installed import acp as harbor_acp +from harbor.agents.installed.base import NonZeroAgentExitCodeError from harbor.agents.installed.opencode import OpenCode from harbor.agents.installed.pi import Pi from harbor.environments.base import ExecResult from harbor.models.agent.context import AgentContext from harbor.models.job.config import JobConfig +from harbor.utils.env import resolve_env_vars +from powercontext_e2e import harbor_agent from powercontext_e2e.catalog import E2ETask, load_tasks +from powercontext_e2e.harbor_agent import PowerContextBubAcpAgent from powercontext_e2e.harbor_claude_code import PowerContextClaudeCodeAgent from powercontext_e2e.harbor_codex import PowerContextCodexAgent from powercontext_e2e.harbor_opencode import PowerContextOpenCodeAgent from powercontext_e2e.harbor_pi import PowerContextPiAgent from powercontext_e2e.hosts import host_adapter from powercontext_e2e.runner import _job_config, prepare_runtime_task, require_runtime_models, run_tasks -from powercontext_e2e.settings import HarnessSettings, ModelNotConfiguredError +from powercontext_e2e.settings import ( + HarnessSettings, + ModelNotConfiguredError, + powercontext_bub_environment, + prefixed_environment, +) _REPOSITORY = Path(__file__).resolve().parents[3] _TASKS = load_tasks(_REPOSITORY / "e2e" / "bub" / "tasks") @@ -146,16 +157,30 @@ def test_codex_auth_is_mounted_only_for_model_tasks(isolated_host_environment: P assert _CODEX_AUTH_TARGET not in _mount_targets(non_model_config) -def test_agent_proxy_is_forwarded_only_when_configured(monkeypatch, tmp_path: Path) -> None: +def test_agent_proxy_is_forwarded_by_reference_only_when_configured(monkeypatch, tmp_path: Path) -> None: + # Harbor does not treat the proxy names as sensitive and would write a literal URL, credentials included, to + # its job files. task = _task("project-database-decision") assert "HTTPS_PROXY" not in _agent_env(_config(task, tmp_path)) - monkeypatch.setenv("POWERCONTEXT_E2E_AGENT_PROXY_URL", "http://proxy.invalid:3128") - env = _agent_env(_config(task, tmp_path)) + proxy_url = "http://user:pass@proxy.invalid:3128" + monkeypatch.setenv("POWERCONTEXT_E2E_AGENT_PROXY_URL", proxy_url) + # A developer's or CI's own proxy settings, which the harness's requests follow, stay as they are. + host_proxy = _host_proxy_settings() + config = _config(task, tmp_path) + env = _agent_env(config) + assert proxy_url not in config.model_dump_json() + assert "proxy.invalid" not in config.model_dump_json() + resolved = resolve_env_vars(env) for name in ("HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy"): - assert env[name] == "http://proxy.invalid:3128" + assert resolved[name] == proxy_url assert "powercontext" in env["NO_PROXY"].split(",") + assert _host_proxy_settings() == host_proxy + + +def _host_proxy_settings() -> dict[str, str]: + return {name: value for name, value in os.environ.items() if name.lower() in ("http_proxy", "https_proxy")} def test_batch_job_binds_scopes_per_invocation_instead_of_per_job(monkeypatch, tmp_path: Path) -> None: @@ -204,7 +229,7 @@ def test_off_arm_runs_the_host_without_powercontext( assert _mount_targets(off) == [_CODEX_AUTH_TARGET] assert not _powercontext_mounts(off) assert _powercontext_mounts(on) - assert on_agent.env["POWERCONTEXT_BUB_API_TOKEN"] == "server-token" # noqa: S105 - test value + assert resolve_env_vars(on_agent.env)["POWERCONTEXT_BUB_API_TOKEN"] == "server-token" # noqa: S105 - test value assert off_agent.kwargs == {"powercontext": False} assert off_agent.env["BUB_MODEL"] == on_agent.env["BUB_MODEL"] == "provider:model" assert on_agent.env["POWERCONTEXT_BUB_SCOPE_ID"] == "scope-1" @@ -277,12 +302,37 @@ def test_plugin_host_off_arm_has_nothing_of_powercontext(monkeypatch, tmp_path: assert off_agent.env == {} assert on_agent.env[host.scope_id] == "scope-1" assert on_agent.env[host.insecure_http] == "true" - assert on_agent.env[host.server_url.replace("SERVER_URL", "AUTHORIZATION")] == "Bearer server-token" + authorization = host.server_url.replace("SERVER_URL", "AUTHORIZATION") + assert resolve_env_vars(on_agent.env)[authorization] == "Bearer server-token" assert off_agent.import_path == on_agent.import_path assert off_agent.model_name == on_agent.model_name == "model-test" assert off_agent.kwargs == {**on_agent.kwargs, "powercontext": False} +@pytest.mark.parametrize("host", ["bub", "codex", "claude-code", "opencode", "pi"]) +def test_job_files_hold_no_part_of_a_short_server_token(monkeypatch, tmp_path: Path, host: str) -> None: + # Harbor writes the job configuration to its job files and keeps the first four and last three characters of a + # sensitive value, which is most of a token this short. + token = "QzJ9QzJ9Qz" # noqa: S105 - test value + monkeypatch.setenv("BUB_MODEL", "provider:model") + for plugin_host in _PLUGIN_HOSTS: + monkeypatch.setenv(plugin_host.model, "model-test") + monkeypatch.setenv(plugin_host.server_url, "http://host-gateway:8000") + monkeypatch.setenv("POWERCONTEXT_CLIENT_API_TOKEN", token) + + config = _paired_config(tmp_path, host, "scope-1") + + written = config.model_dump_json() + assert not [part for part in (token, token[:4], token[-3:]) if part in written] + (agent,) = config.agents + assert [value for value in resolve_env_vars(agent.env).values() if value in (token, f"Bearer {token}")] + # The harness holds the value under its own name: an integration's native environment, which a later job reads + # again, never returns it. + adapter = host_adapter(host) + native = powercontext_bub_environment() if host == "bub" else prefixed_environment(adapter.plugin_prefix) + assert not [name for name, value in native.items() if token in value] + + @pytest.mark.parametrize("agent_class", [PowerContextCodexAgent, PowerContextClaudeCodeAgent]) @pytest.mark.parametrize("powercontext", [True, False]) def test_plugin_agents_install_the_plugin_only_for_on(tmp_path: Path, agent_class: type, powercontext: bool) -> None: @@ -354,7 +404,7 @@ class _ShellEnvironment: default_user = None - def __init__(self, root: Path, *, sessions: str = "") -> None: + def __init__(self, root: Path, *, sessions: str = "", env: dict[str, str] | None = None) -> None: self.home = root / "home" self.tmp = root / "tmp" bin_dir = root / "bin" @@ -364,7 +414,7 @@ def __init__(self, root: Path, *, sessions: str = "") -> None: opencode = bin_dir / "opencode" opencode.write_text(f"#!/bin/sh\nprintf '%s' '{sessions}'\n") opencode.chmod(0o755) - self._env = {"HOME": str(self.home), "TMPDIR": str(self.tmp), "PATH": f"{bin_dir}:/usr/bin:/bin"} + self._env = {"HOME": str(self.home), "TMPDIR": str(self.tmp), "PATH": f"{bin_dir}:/usr/bin:/bin", **(env or {})} async def exec(self, command: str, **_: object) -> ExecResult: # Harbor runs agent commands with bash, and prefixes them with `set -o pipefail`, which dash rejects. @@ -467,6 +517,7 @@ def test_pi_sessions_start_without_tool_output_an_earlier_session_left( async def run_pi(self, instruction, environment, context) -> None: started.append({path.name for path in environment.tmp.iterdir()}) + _leave_pi_output(self) monkeypatch.setattr(Pi, "run", run_pi) @@ -480,6 +531,7 @@ def test_pi_sessions_start_when_no_earlier_tool_output_exists(monkeypatch, tmp_p async def run_pi(self, instruction, environment, context) -> None: started.append(instruction) + _leave_pi_output(self) monkeypatch.setattr(Pi, "run", run_pi) @@ -491,12 +543,133 @@ async def run_pi(self, instruction, environment, context) -> None: def test_pi_runs_without_a_saved_session(tmp_path: Path) -> None: # Pi saves every session unless told not to, so the harness relies on Harbor passing `--no-session`. environment = _RecordingEnvironment() + agent = _pi_agent(tmp_path, powercontext=False) + _leave_pi_output(agent) - asyncio.run(_pi_agent(tmp_path, powercontext=False).run("task", environment, AgentContext())) + asyncio.run(agent.run("task", environment, AgentContext())) assert any("pi --print" in command and "--no-session" in command for command in environment.commands) +def _leave_pi_output(agent: Pi, *lines: str) -> None: + # Harbor's Pi agent tees Pi's events to this file in the container, which Harbor's Docker environment mounts. + (agent.logs_dir / "pi.txt").write_text("".join(f"{line}\n" for line in lines), encoding="utf-8") + + +def _pi_message(stop_reason: str, **fields: str) -> str: + # Pi serializes with JSON.stringify, which leaves U+2028 and other non-ASCII characters unescaped. + return json.dumps( + {"type": "message_end", "message": {"role": "assistant", "stopReason": stop_reason, **fields}}, + ensure_ascii=False, + ) + + +def _run_pi_with_output(monkeypatch, tmp_path: Path, *lines: str) -> None: + async def run_pi(self, instruction, environment, context) -> None: + _leave_pi_output(self, *lines) + + monkeypatch.setattr(Pi, "run", run_pi) + asyncio.run(_pi_agent(tmp_path, powercontext=False).run("task", _RecordingEnvironment(), AgentContext())) + + +def test_pi_session_whose_model_request_failed_is_an_error(monkeypatch, tmp_path: Path) -> None: + # In the JSON mode Harbor uses, Pi exits 0 after a failed model request, which would otherwise be graded as an + # attempt that did not answer. + with pytest.raises(NonZeroAgentExitCodeError, match="401: invalid key"): + _run_pi_with_output(monkeypatch, tmp_path, _pi_message("error", errorMessage="401: invalid key")) + + +def test_pi_session_whose_output_is_not_on_the_host_is_an_error(monkeypatch, tmp_path: Path) -> None: + # The failure check reads Pi's output before Harbor downloads the agent's logs, so it depends on Harbor's Docker + # environment mounting them. Without the file, every failed session would otherwise count as an attempt. + async def run_pi(self, instruction, environment, context) -> None: + pass + + monkeypatch.setattr(Pi, "run", run_pi) + + with pytest.raises(RuntimeError, match="mounts /logs"): + asyncio.run(_pi_agent(tmp_path, powercontext=False).run("task", _RecordingEnvironment(), AgentContext())) + + +def test_pi_session_that_recovered_from_a_failed_model_request_is_an_attempt(monkeypatch, tmp_path: Path) -> None: + _run_pi_with_output( + monkeypatch, + tmp_path, + "Warning: not an event", + _pi_message("error", errorMessage="429: rate limited"), + _pi_message("stop"), + ) + + +# JSON leaves these unescaped inside a string, and str.splitlines would split a record at each of them. +_LINE_SEPARATORS = ["\u2028", "\u2029", "\u0085"] + + +@pytest.mark.parametrize("separator", _LINE_SEPARATORS, ids=lambda s: f"U+{ord(s):04X}") +def test_pi_message_text_cannot_change_the_run_classification(monkeypatch, tmp_path: Path, separator: str) -> None: + # A retry answered with this text would otherwise lose its message_end, and the stale 429 would exclude the arm. + _run_pi_with_output( + monkeypatch, + tmp_path, + _pi_message("error", errorMessage="429: rate limited"), + _pi_message("stop", content=f"The team chose OceanBase.{separator}It runs 12 shards."), + ) + + with pytest.raises(NonZeroAgentExitCodeError, match="invalid key"): + _run_pi_with_output( + monkeypatch, + tmp_path, + _pi_message("stop", content="An earlier turn."), + _pi_message("error", errorMessage=f"401: invalid key{separator}request id 7"), + ) + + +@pytest.mark.parametrize("powercontext", [True, False]) +def test_bub_sessions_start_without_the_tapes_an_earlier_session_left( + monkeypatch, tmp_path: Path, powercontext: bool +) -> None: + # Bub keeps every session's messages in its home, which Harbor leaves in place between the steps of a trial, so + # a later session could otherwise read what the user said in an earlier one. + started: list[bool] = [] + bub_home = tmp_path / "bub-home" + tape = bub_home / "tapes" / "session.jsonl" + tape.parent.mkdir(parents=True) + tape.write_text("The team chose OceanBase with 12 shards.") + environment = _ShellEnvironment(tmp_path, env={"BUB_HOME": str(bub_home)}) + + async def run_bub(self, instruction, environment, context) -> None: + started.append(tape.exists()) + + monkeypatch.setattr(harbor_acp.AcpAgent, "run", run_bub) + monkeypatch.setattr(harbor_agent, "STEP_FAILURE_MARKER", str(tmp_path / "step-failed")) + + agent = PowerContextBubAcpAgent(logs_dir=tmp_path, powercontext=powercontext) + asyncio.run(agent.run("task", environment, AgentContext())) + + assert started == [False] + + +def test_bub_step_whose_tapes_could_not_be_removed_is_marked_failed(monkeypatch, tmp_path: Path) -> None: + # The verifier reads a missing marker as a passed step, so a step that fails before Bub starts must still leave + # the marker, or Harbor would score it 1 and a fail-fast batch would run on. + started: list[bool] = [] + marker = tmp_path / "step-failed" + environment = _ShellEnvironment(tmp_path, env={}) # no BUB_HOME: the removal refuses to run + + async def run_bub(self, instruction, environment, context) -> None: + started.append(True) + + monkeypatch.setattr(harbor_acp.AcpAgent, "run", run_bub) + monkeypatch.setattr(harbor_agent, "STEP_FAILURE_MARKER", str(marker)) + + agent = PowerContextBubAcpAgent(logs_dir=tmp_path, powercontext=True) + with pytest.raises(RuntimeError, match="BUB_HOME: parameter null or not set"): + asyncio.run(agent.run("task", environment, AgentContext())) + + assert marker.exists() + assert started == [] + + @pytest.mark.parametrize("host", _PLUGIN_HOSTS, ids=lambda host: host.name) def test_plugin_host_requires_a_model_and_the_server_url_before_any_run(monkeypatch, host: _PluginHost) -> None: adapter = host_adapter(host.name) diff --git a/e2e/bub/tests/test_paired.py b/e2e/bub/tests/test_paired.py index a28848d2a..85d4dabab 100644 --- a/e2e/bub/tests/test_paired.py +++ b/e2e/bub/tests/test_paired.py @@ -206,7 +206,7 @@ def test_continuation_tasks_cannot_gate_the_recall_step_behind_an_earlier_reward ({"exception_types": ("EnvironmentStartTimeoutError",)}, "error"), ({"harness_failed": True, "reward": 1.0}, "error"), ({"treatment_failures": ("no context",), "reward": 1.0}, "integration_failed"), - # A timed-out ON run has no final snapshot; it still counts as a failed attempt, as it would with OFF. + # A timed-out ON run counts as a failed attempt, as it would with OFF, even when it also missed the treatment. ({"exception_types": ("AgentTimeoutError",), "treatment_failures": ("not observed",)}, "timeout"), ], )