APR-262 Add Harbor verifiers support for v1#1401
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a91f41ae52
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if verifier_mode == VERIFIER_MODE_SEPARATE and verifier_environment is None: | ||
| verifier_sandbox = {**sandbox, "command_timeout": int(test_timeout)} |
There was a problem hiding this comment.
When a task sets [verifier] environment_mode = "separate" but does not provide [verifier.environment], this branch advertises support by creating a fresh verifier sandbox from the agent sandbox config. That fresh sandbox never receives the task's tests/ directory: run_separate_harbor_verifier only creates an empty /tests directory and transfers /logs/artifacts plus configured artifacts before running bash /tests/test.sh. In this supported mode the script is absent unless the base agent image happens to contain it, so otherwise-valid Harbor tasks always get reward 0.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Harbor PR #1655 does support environment_mode = "separate" without [verifier.environment]; upstream resolves it to a fresh copy of the top-level environment. The difference is that Harbor can build the verifier image from tests/, while this v1 sandbox path cannot. I fixed the fallback by marking that case with verifier_upload_tests and uploading the task tests into the fresh verifier sandbox before running /tests/test.sh, while explicit [verifier.environment] continues to expect a pre-baked verifier image.
ApprovabilityVerdict: Needs human review Unable to check for correctness in f3f955b. This PR introduces substantial new Harbor verifier functionality with separate sandbox modes, affecting task execution and reward scoring pipelines. Multiple unresolved P1 review comments identify potential runtime bugs including incorrect working directories and changed reward file precedence. You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f02768b6d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
xeophon
force-pushed
the
apr-262-harbor-v1-verifier-modes
branch
from
8ae240a to
05d4739
Compare
xeophon
force-pushed
the
apr-262-harbor-v1-verifier-modes
branch
2 times, most recently
from
a953660 to
f23a006
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f23a0066c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "if [ -s /logs/verifier/reward.json ]; then " | ||
| "cat /logs/verifier/reward.json; " | ||
| "elif [ -s /logs/verifier/reward.txt ]; then " | ||
| "cat /logs/verifier/reward.txt; fi" |
There was a problem hiding this comment.
When both /logs/verifier/reward.txt and /logs/verifier/reward.json are present, this command now reads JSON first, which can score from stale/diagnostic JSON or return 0 for invalid JSON even if reward.txt contains the valid final score. That changes previously working Harbor tasks and diverges from the existing Harbor implementations in this repo that prioritize reward.txt first. Fresh evidence in this commit: HARBOR_REWARD_COMMAND checks reward.json before reward.txt, and the new test test_harbor_reward_command_prefers_json_reward explicitly locks in that order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f23a006. Configure here.
xeophon
force-pushed
the
apr-262-harbor-v1-verifier-modes
branch
from
f23a006 to
9daf4b2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9daf4b2aee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| lease.id, | ||
| state, | ||
| command="bash /tests/test.sh", | ||
| working_dir=None, |
There was a problem hiding this comment.
In separate verifier mode, the test runner now invokes bash /tests/test.sh with working_dir=None, which changes execution semantics versus the existing Harbor path (bash test.sh in /tests). Any verifier script that uses relative paths (for example ./fixtures/... or python helper.py) will resolve against the sandbox default workdir (often /app or image WORKDIR) instead of /tests, causing false failures and reward 0 in otherwise valid tasks. This is specific to verifier_mode = "separate" and is introduced by the new separate-sandbox path.
Useful? React with 👍 / 👎.
xeophon
force-pushed
the
apr-262-harbor-v1-verifier-modes
branch
from
9daf4b2 to
a063a69
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a063a69bff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "if [ -s /logs/verifier/reward.json ]; then " | ||
| "cat /logs/verifier/reward.json; " | ||
| "elif [ -s /logs/verifier/reward.txt ]; then " | ||
| "cat /logs/verifier/reward.txt; fi" |
There was a problem hiding this comment.
HARBOR_REWARD_COMMAND now reads /logs/verifier/reward.json before /logs/verifier/reward.txt, which can mis-score rollouts when both files exist (for example, a stale/diagnostic JSON alongside the final scalar reward in reward.txt). In that case valid tasks can return the wrong reward or 0 if JSON is malformed, despite a correct text reward being present, so this ordering can silently corrupt evaluation results.
Useful? React with 👍 / 👎.
xeophon
force-pushed
the
apr-262-harbor-v1-verifier-modes
branch
from
a063a69 to
c43b51c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c43b51cc5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| verifier_env = harbor.get("verifier_env") or {} | ||
| if not isinstance(verifier_env, Mapping): | ||
| raise TypeError("[verifier].env must be a mapping.") | ||
| verifier_env = {str(key): str(value) for key, value in verifier_env.items()} |
There was a problem hiding this comment.
The new verifier env plumbing passes [verifier.env] values through as raw strings ({"KEY": "${KEY}"} stays literal) instead of resolving host-backed placeholders. Harbor tasks commonly use ${VAR} here for API keys (e.g., LLM-judge verifiers), so in those tasks the verifier process receives the literal token ${ANTHROPIC_API_KEY} rather than the actual secret, causing authentication failures and reward 0 despite valid host configuration.
Useful? React with 👍 / 👎.
xeophon
force-pushed
the
apr-262-harbor-v1-verifier-modes
branch
from
c43b51c to
071c2ba
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3f955ba2f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sandbox: ConfigData = { | ||
| "image": environment.get("docker_image") or config.docker_image, | ||
| "cpu_cores": parse_number(environment.get("cpus"), config.cpu_cores), | ||
| "memory_gb": parse_gb(memory, config.memory_gb), | ||
| "disk_size_gb": parse_gb(storage, config.disk_size_gb), | ||
| "timeout_minutes": config.timeout_minutes, | ||
| "command_timeout": command_timeout, |
There was a problem hiding this comment.
For Harbor tasks whose [environment] or new [verifier.environment] requests GPUs with gpus = 1 / gpu_types = [...], this sandbox mapping drops those fields, so create_sandbox() falls back to gpu_count=0 and the verifier/agent starts without the required accelerator. This is especially visible for the new separate verifier path because Harbor documents [verifier.environment] as the same schema as [environment], but GPU-backed graders will now fail or be scheduled on CPU-only sandboxes despite the task config requesting GPUs.
Useful? React with 👍 / 👎.
1 participant
Summary
[verifier].environment_modeand[verifier.environment], including shared/separate validation and separate-mode sandbox configenvironment_mode = "separate"fallback without[verifier.environment]/logs/artifactsplus configured Harbor artifacts into verifier sandboxes, including explicit/logs/artifactsentries withexcludepatterns and source-path handling for artifactdestinationmappingsreward.jsonbeforereward.txtand documenting the v1 Harbor verifier lifecycleVerification
uv run --frozen pytest tests/test_v1_harbor_cli.py tests/test_opencode_harbor.py -quv run --frozen ruff format .uv run --frozen ruff check --fix .uv run --frozen ty check verifiersuv run --frozen --group policy semgrep --metrics=off --disable-version-check --config .semgrep/verifiers.yml --error --quietNote
Medium Risk
Changes Harbor scoring to create ephemeral verifier sandboxes and move grading inputs across sandboxes; host env substitution for verifier secrets adds operational sensitivity, though behavior is covered by new tests.
Overview
Adds Harbor v1 support for
[verifier]settings fromtask.toml: shared (tests in the agent sandbox, unchanged default) vs separate (dedicated verifier sandbox).[verifier.environment]implies separate mode; explicitenvironment_mode = "separate"without a verifier env reuses the agent[environment]and setsverifier_upload_testssotests/are uploaded when the image has no baked tests. Invalid combos (e.g. shared +[verifier.environment]) fail at load time.Separate-mode scoring spins up a lease via
create_sandbox_lease, optionally uploads tests, packs/logs/artifactsplus taskartifacts(withexcludepatterns) from the agent sandbox into a tarball, extracts them on the verifier, runsbash test.shwith resolved[verifier].env(${VAR}/${VAR:-default}from the host), then reads reward viareward.jsonbeforereward.txt. Shared mode still uploads tests to the agent sandbox but uses the same reward command order.Also extends
SandboxConfig/create_sandboxwithgpu_typeandvm(defaultvmwhengpu_count > 0), and documents the verifier lifecycle in BYO Harness docs.Reviewed by Cursor Bugbot for commit f3f955b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add separate verifier sandbox support to Harbor v1 taskset
separateverifier mode in harbor.py: when[verifier.environment]is declared in a task's TOML, a dedicated verifier sandbox is created, inputs/artifacts are transferred from the agent sandbox, tests are run, and the sandbox is deleted on completion.harbor_verifier_envto resolve[verifier].envvalues from host environment variables with${VAR:-default}template support, raising if required variables are missing.HARBOR_REWARD_COMMANDto prefer/logs/verifier/reward.jsonover/logs/verifier/reward.txtwhen reading reward output.SandboxConfigandcreate_sandboxto carrygpu_typeandvmfields, defaultingvm=Truewhengpu_count > 0.environment_mode="shared"together with[verifier.environment]now raisesValueErrorat task-row construction time.Macroscope summarized f3f955b.