Skip to content

fix(e2e): close gaps a review found in the paired harness - #1853

Merged
Teingi merged 5 commits into
oceanbase:masterfrom
Fengzdadi:fix/e2e-paired-review-findings
Oct 7, 2026
Merged

Teingi merged 5 commits into
oceanbase:masterfrom
Fengzdadi:fix/e2e-paired-review-findings

Conversation

@Fengzdadi

@Fengzdadi Fengzdadi commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Part of #1705. It fixes defects in the paired harness merged through #1747, #1779, #1800, #1816, #1835, and #1851.

Rationale for this change

A review of the merged harness found three ways its results or evidence could be wrong:

  1. Bub's session files stay readable. Bub 0.4.2 keeps every session's messages as JSONL under $BUB_HOME/tapes. The harness sets BUB_HOME inside the container and Harbor does not clear it between the steps of a trial. A recall agent in either arm could read the capture session's prompt from those files and answer without PowerContext. OpenCode (feat(e2e): run paired continuation workloads with OpenCode #1835) and Pi (feat(e2e): run paired continuation workloads with Pi #1851) already clear their own session output; Bub did not.
  2. A failed model request on Pi is graded as a failed attempt. Harbor runs Pi with --mode json. In that mode Pi 0.82.1 exits 0 after a failed or aborted model request, so Harbor sees no failure, the verifier finds no answer, and the arm scores 0 and enters the success rate and the paired difference. Pi's text mode exits 1 on the same condition. Codex and Claude Code exit non-zero, and Harbor's OpenCode agent reads OpenCode's error events.
  3. Harbor's job files keep secrets the harness passes to agents. Harbor writes each agent's environment to harbor-jobs/**/config.json and related files. It keeps the first four and last three characters of a literal under a sensitive name, and writes a literal under any other name in full. For the ON arm's POWERCONTEXT_BUB_API_TOKEN that is seven characters of the token, and for POWERCONTEXT_<HOST>_AUTHORIZATION it is the token's last three. The agents' proxy URL, which can carry credentials, goes under HTTP_PROXY and similar names, which Harbor does not consider sensitive, so it was written in full. The harness does not redact Harbor's files, while the README said every final evidence sink is redacted whatever the token's length.

What changes are included in this PR?

  • Bub tapes: before each session, in both arms and for acceptance runs too, the Bub agent removes "${BUB_HOME:?}/tapes". Bub does not search another session's tape, so this removes only what an agent could read from the files. The removal runs after the agent sets the step-failure marker, which the verifiers read as a failed step, so a failed removal, including an unset BUB_HOME, fails the step instead of scoring it 1 and letting a fail-fast batch continue.
  • Pi model failures: after a session, the Pi agent reads the last message_end event in Pi's output. If it is an assistant message that stopped on error or aborted, the agent raises Harbor's NonZeroAgentExitCodeError, which the paired command classifies as an error and leaves out of success rates and paired differences. This is the condition Pi's own text mode uses for exit code 1, so a session that failed once and then recovered still counts as an attempt. The check runs before Harbor downloads the agent's logs, so it reads Pi's output through the bind mount of Harbor's Docker environment; when the output is not on the host, the agent stops with an error rather than counting the session as an attempt.
  • Secrets in Harbor's job files: Harbor writes a ${NAME} reference as it is, whatever the name, and resolves it from the harness process's environment when it starts the agent. The harness now holds each value it derives for an agent in its own environment under the POWERCONTEXT_E2E_AGENT_SECRET_ prefix and gives Harbor the reference. The job files record ${POWERCONTEXT_E2E_AGENT_SECRET_BUB_API_TOKEN}, ${POWERCONTEXT_E2E_AGENT_SECRET_<HOST>_AUTHORIZATION}, and ${POWERCONTEXT_E2E_AGENT_SECRET_PROXY_URL}, and no part of the values. No integration reads that prefix as a native setting, so later jobs never read a derived value back, and HTTP_PROXY stays out of the harness's own environment. Every variable under the prefix is an evidence secret, which also covers the full Bearer <token> header.
  • Secret names: evidence redaction also covers a variable that names a token in the middle, such as AWS_BEARER_TOKEN_BEDROCK, which Harbor's Claude Code agent forwards, and matches names in any case, because settings accept powercontext_client_api_token. A plural such as MAX_THINKING_TOKENS stays a count, and a name ending in _FILE, _PATH, or _URL, such as AWS_WEB_IDENTITY_TOKEN_FILE, holds where a token is, so its value stays in the evidence.
  • README: states what the harness clears for Bub, how a failed model request is classified on each host and that the Pi check relies on the mounted logs, how the token and the proxy URL reach Harbor, which names count as secrets, and that the harness redacts the files it writes while Harbor writes harbor-jobs/ itself.
  • Tests: a conftest.py gives each harness test its own copy of the process environment, because the harness now writes to it.

Are there any user-facing changes?

  • A Pi run whose model request fails is now reported as an error and makes the paired command exit non-zero, instead of counting as a failed attempt.
  • Bub runs, including acceptance runs, start each agent invocation without the tapes of earlier invocations.
  • Harbor's job files hold references to the ON arm's Server token and to the agents' proxy URL instead of a partly masked token and the full URL.
  • AWS_BEARER_TOKEN_BEDROCK and similar names, and lower-case secret names, are now redacted from the files the harness writes; names such as AWS_WEB_IDENTITY_TOKEN_FILE are not.

How was this change tested?

Real runs

Against a local Server with POWERCONTEXT_SERVER_ACCESS_MODE=enforced, on commit a2bd739:

  • Pi, one paired trial (make harness-paired ARGS='--host pi --trials 1', openrouter/z-ai/glm-5.3): OFF 0/1, ON 1/1, no errors or integration failures. Harbor's files hold "POWERCONTEXT_PI_AUTHORIZATION": "${POWERCONTEXT_PI_AUTHORIZATION}" four times and neither the token nor a masked form of it. The ON arm captured and requested context, so Harbor resolved the reference for the agent.
  • Pi with an invalid OPENROUTER_API_KEY: both arms are error, the report shows OFF 0/0 and ON 0/0 with one error each, and the command exits non-zero. Before this change the same run scored both arms as failed attempts.
  • Bub, one acceptance workload (make harness-acceptance ARGS='--id project-database-decision'): passed. The tape removal ran in the real container before both steps, and Harbor's files hold "POWERCONTEXT_BUB_API_TOKEN": "${POWERCONTEXT_BUB_API_TOKEN}" and no part of the token.

The defects, reproduced

  • Pi exit code: in a node:22-bookworm container with Pi 0.82.1 and an invalid key, pi --print --mode json exits 0 with "stopReason":"error" and a 401 message, and pi --print exits 1. With the same invalid key, Claude Code 2.1.284 and Codex 0.153.4 exit 1.
  • Masked token: serializing a job configuration with the 10-character token Zq7Zq7Zq7x gave Zq7Z****q7x for Bub and Bear****q7x for the plugin hosts before this change, and the ${NAME} references after it.
  • Proxy URL: Harbor's sensitive-name pattern is KEY|SECRET|TOKEN|PASSWORD|CREDENTIAL|AUTH, which HTTP_PROXY does not match, so a literal proxy URL is serialized in full.
  • Bub tapes: established from Bub 0.4.2's source (FileTapeStore(directory=bub.home / "tapes")) and by running that store locally with BUB_HOME set, which wrote the prompt text to tapes/*.jsonl.

Checks

  • make check, make harness-check, and make unit-test pass.
  • New tests, each failing without its fix:
    • For all five hosts, the serialized job configuration holds neither a 10-character Server token nor its first four or last three characters, the agent still receives the token once Harbor resolves the reference, and the host integration's native environment never returns the token.
    • A proxy URL with credentials appears nowhere in the serialized job configuration, every proxy variable resolves to it, and the harness's own environment has no HTTP_PROXY or HTTPS_PROXY.
    • A Pi session whose last message stopped on an error raises; one that failed and then recovered does not; one whose output is not on the host raises.
    • The Bub agent's command runs in a real bash against a seeded Bub home, for both arms: the earlier tape is gone when the session starts. Without BUB_HOME, the removal fails, Bub does not start, and the step-failure marker is in place.
    • AWS_BEARER_TOKEN_BEDROCK and a lower-case powercontext_client_api_token are redacted in every final evidence file, and neither a MAX_THINKING_TOKENS count nor the paths in AWS_WEB_IDENTITY_TOKEN_FILE and HF_TOKEN_PATH rewrite the evidence.

Limits

  • Real runs and later changes: the real runs above predate the POWERCONTEXT_E2E_AGENT_SECRET_ names, the proxy reference, the marker order, and the stricter Pi output check. Unit tests cover those; a real run would record the prefixed references in Harbor's files.
  • Bub tapes inside a container: the tape contents were not inspected in a real container, which Harbor removes after the trial. The real run shows only that the removal succeeds there.
  • Bub and failed model requests: whether Bub's ACP server reports a failed model request as a failure was not checked.
  • Pi on other environments: the Pi failure check needs an environment that mounts the agent's logs. The harness runs only Harbor's Docker environment, which does.
  • Harbor's own files are still not redacted: every host's output under harbor-jobs/ can contain arbitrary command output, such as an ON agent printing its environment. Design note (non-blocking): the harness could redact that directory after each job, at the cost of rewriting Harbor's output.
  • Process environment: the harness writes the values it derives for agents into its own process environment under POWERCONTEXT_E2E_AGENT_SECRET_. Nothing else in the harness process reads that prefix, and the OFF arm's agent environment stays empty.
  • Unchanged on purpose: in the Bub OFF arm, the step marker /logs/agent/powercontext-step-failed and the ACP agent ID still carry the name PowerContext. Six acceptance verifiers with pinned checksums read that marker, and neither gives access to anything.
  • Not rerun: the Bub, Codex, Claude Code, and OpenCode paired pilots, and the fixed Compose harness.

AI usage statement

This PR was developed with Claude Code (Claude Fable 5.1 and Claude Opus 5.5). Independent Claude review sessions found the defects; Claude verified each one, wrote the fixes and tests, and ran the checks and the real runs above. The author requested the reviews, approved the scope of the fixes, reviewed the change, and provided the run environment.

🤖 Generated with Claude Code

Bub keeps every session's messages as JSONL in its home, which Harbor
leaves in place between the steps of a trial, so a recall session in
either arm could read what the user said during capture. The agent now
removes Bub's tapes before each session, as OpenCode and Pi already
clear their own session output.

In the JSON mode Harbor uses, Pi exits 0 after a failed model request,
so the arm was graded as an attempt that did not answer. The agent now
reads Pi's last message and raises, which makes the arm an error that
is left out of success rates and paired differences.

Harbor writes each agent's environment to its job files and keeps the
first four and last three characters of a sensitive literal, which is
most of a short token. The harness now holds the Server token it
derives for the ON arm in its own environment and gives Harbor a
reference, so the job files record `${NAME}` and no part of the token.

Evidence redaction also covers a variable that names a token in the
middle, such as AWS_BEARER_TOKEN_BEDROCK, and matches names in any
case. The README states that the harness redacts the files it writes
and does not redact Harbor's own files.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Bub agent touched the step-failure marker only after removing the
tapes, so a failed removal left no marker and the verifier scored the
crashed step 1, letting a fail-fast batch run on. The marker now comes
first.

Harbor writes a literal under a name it does not consider sensitive in
full, so the agents' proxy URL, which can carry credentials, reached
its job files. The harness now passes the proxy by reference as well,
and holds every value it derives for an agent under its own
POWERCONTEXT_E2E_AGENT_SECRET_ prefix, which no integration reads back
as a native setting and which keeps HTTP_PROXY out of the harness's
own environment.

A name with _TOKEN_ in the middle that ends in _FILE, _PATH, or _URL,
such as AWS_WEB_IDENTITY_TOKEN_FILE, holds where a token is, so its
value is no longer redacted from evidence.

The Pi failure check reads Pi's output before Harbor downloads the
agent's logs. It now stops with an error when the output is not on the
host instead of treating the session as an attempt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked the Pi result-classification issue against 5944341; it remains reproducible.

"agent's logs, which requires an environment that mounts /logs"
)
last: dict[str, Any] = {}
for line in output.read_text(encoding="utf-8", errors="replace").splitlines():

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Split Pi JSONL records on LF only

Python's splitlines() also splits on U+0085/U+2028/U+2029, which Pi's JSON.stringify(event) leaves unescaped inside valid JSON strings. Using real Pi 0.82.1 and Harbor 0.16.1 with a deterministic test provider, I reproduced a 429 followed by a successful automatic retry whose response contains U+2028. The final message_end is silently discarded, so this raises the stale 429 and excludes the recovered arm as an infrastructure error. Conversely, a terminal error containing U+2028 is missed. Please split on LF only (or iterate over the file's lines), and cover a successful retry with this content so valid message text cannot change the run classification.

str.splitlines also splits at U+2028, U+2029, and U+0085, which Pi's
JSON.stringify leaves unescaped inside a string. A message_end record
containing one was dropped as invalid JSON, so a retry that recovered
from a 429 raised the stale error and a terminal error carrying one was
missed. Cover both directions for each separator.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed the Pi JSONL fix on 514279f by replaying both counterexamples. One test-isolation issue remains.

Comment thread e2e/bub/tests/test_harbor_job_config.py Outdated
assert resolved[name] == proxy_url
assert "powercontext" in env["NO_PROXY"].split(",")
# The harness's own requests do not go through the agents' proxy.
assert not [name for name in os.environ if name.lower() in ("http_proxy", "https_proxy")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Assert that existing host proxy settings remain unchanged

This assertion fails whenever the process already has HTTP_PROXY, HTTPS_PROXY, or their lowercase variants set. The fixture leaves these variables intact, and _job_config() correctly preserves them. I reproduced the failure with only HTTP_PROXY=http://review-proxy.invalid:3128 set; the same test passes after removing it. This makes make harness-check fail in proxy-enabled development or CI environments even when the agent proxy is forwarded correctly. Please snapshot the host proxy settings before constructing the configuration and assert that they remain unchanged, or explicitly isolate these variables in the test.

The assertion that the harness leaves its own HTTP_PROXY and
HTTPS_PROXY alone failed on any host that already sets them. Snapshot
the host's settings first and assert they are unchanged instead.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
"""

held = f"{_AGENT_SECRET_PREFIX}{name.removeprefix('POWERCONTEXT_')}"
environ[held] = value

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Harness-derived credentials land in the harness process environment, where every child process inherits them

held = f"{_AGENT_SECRET_PREFIX}{name.removeprefix('POWERCONTEXT_')}"
environ[held] = value
return f"${{{held}}}"

The reference mechanism itself is the right call and I verified it against harbor==0.16.1: resolve_env_vars only reads os.environ (harbor/utils/env.py:118-119), and templatize_sensitive_env keeps an already-templated value as-is (:68), so the job files never contain the literal. The reason this helper exists is also real — is_sensitive_env_key("PROXY_URL") is False, so without templating the agent proxy URL would be written to the job file in full. Confirmed:

  POWERCONTEXT_CLIENT_API_TOKEN                 sensitive=True
  PROXY_URL                                     sensitive=False
  POWERCONTEXT_E2E_AGENT_SECRET_PROXY_URL       sensitive=True

The part worth tightening is the storage location. environ[held] = value puts every harness-derived credential into the harness process's own environment table, where it is inherited by everything the harness spawns. Measured:

returned reference : ${POWERCONTEXT_E2E_AGENT_SECRET_CLIENT_API_TOKEN}
os.environ holds it: True
child sees plaintext: True
child output        : sk-live-ABCDEFGHIJKLMNOPQRST
in evidence_secrets : True

The harness runs git (settings.py:143), docker compose (Harbor passes env=os.environ into the container process), and the plugin install command as subprocesses, so the token is readable from /proc/<pid>/environ or ps e by anything else on the machine, for the lifetime of the run. This is a wider surface than the one the commit message sets out to fix — it moves the value out of Harbor's output directory but not out of reach. The prefix keeps it from being read as an integration's own setting; it does not keep it from being read by unrelated processes.

Suggested direction: hold these values in a process-local store on HarnessSettings rather than os.environ, and inject them into the environment only at the point Harbor resolves the reference (Harbor reads os.environ inside the same process, so a short-lived injection immediately before create_agent_from_config, restored right after, keeps the mechanism working). If the current shape is kept, at minimum document why process-environment visibility is acceptable for these values, since the same runner can be pointed at a real token.

Test worth adding: assert the harness's own os.environ contains none of the derived values after agent_secret returns (today test_job_files_hold_no_part_of_a_short_server_token checks config.model_dump_json() and the native env, but not os.environ), and that a subprocess spawned from the harness does not see the token.

Harbor resolves a reference from the host environment and nowhere
else, and every value agent_secret holds derives from a setting that
reaches the harness only through that same environment, so a child
process of the harness inherits nothing it did not inherit already.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Teingi
Teingi merged commit 70ed364 into oceanbase:master Oct 7, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants