Skip to content

[Harbor 3/4] Mode B (nested harbor run) + the vero harbor build compiler#5

Open
varunursekar wants to merge 5 commits into
harbor-2-sidecarfrom
harbor-3-compiler
Open

[Harbor 3/4] Mode B (nested harbor run) + the vero harbor build compiler#5
varunursekar wants to merge 5 commits into
harbor-2-sidecarfrom
harbor-3-compiler

Conversation

@varunursekar

@varunursekar varunursekar commented Jun 24, 2026

Copy link
Copy Markdown

Draft · Stack 3 of 4 — targets harbor-2-sidecar.

  • Mode B (runner.py): HarborRunner, an EvalStrategy that — per candidate — runs a nested harbor run of the agent over the selected Harbor tasks (e.g. on Modal) and collates the verifier rewards into vero SampleResults. One Harbor task = one sample; inference is delegated, scoring comes from Harbor.
  • The compiler (build/): vero harbor build renders a BuildConfig into a runnable Harbor task — a Docker Compose env (optimizer workbench + eval sidecar + 3 volumes), two Dockerfiles, instruction.md, tests/test.sh, seed/solve scripts — baking the dataset/scorer/baseline repo and the sidecar's ServeConfig. Mode A and Mode B.

Review focus: the nested-run + collation, and the trust-boundary plumbing baked into the generated compose (volumes, root:600 token, secrets → sidecar only). Also un-ignores src/vero/harbor/build/ (the repo's build/ rule was hiding the package).

Stack: [1/4] core → [2/4] sidecar → this → [4/4] docs.

🤖 Generated with Claude Code

Greptile Summary

This PR adds Mode B (nested harbor run) as an EvalStrategy in runner.py and introduces the vero harbor build compiler (build/) that renders a BuildConfig into a fully-wired Harbor task directory — Dockerfiles, docker-compose, seed/solve/test scripts, and a baked ServeConfig. The previous-thread fixes (best-trial selection, --n-attempts/--max-retries flag emission) are implemented and tested in TestReviewFixes.

  • HarborRunner (runner.py): per-candidate nested harbor run that filters already-done samples on resume, collates result JSONs using a deterministic best-trial ranking (clean > has-rewards > finished_at > mtime), and persists SampleResults.
  • Compiler (build/): compile_task materialises the agent baseline via git archive, registers the dataset into a baked VERO_HOME, emits a ServeConfig, and renders all task artifacts; secrets reach the sidecar container only via compose ${VAR:?} guards, never main.
  • Trust-boundary: admin token is root:600 on a volume mounted read-only to main, so the optimizer agent (agent user) cannot read it; read_only_paths advisory chmod is documented as non-authoritative.

Confidence Score: 4/5

Draft PR; do not merge until the git archive pipeline fixes from #9 are backported into this stack.

The compiler's _prepare_baseline_repo still carries the communicate()/stdout-close bug that breaks git-archive builds for the normal case — acknowledged in prior review threads as landing in #9. The test_baseline_archive_failure_raises test may not reach its expected RuntimeError until that fix lands. Both runner fixes (best-trial ranking, --n-attempts/--max-retries flag emission) are correctly implemented and tested in this PR.

vero/src/vero/harbor/build/compiler.py — the git archive pipeline (lines 89-111) needs the communicate()/stdout-close fix from #9 before the compiler is usable for git-tracked agent repos.

Important Files Changed

Filename Overview
vero/src/vero/harbor/build/compiler.py Core compiler: git archive pipeline, dataset registration, ServeConfig emission, template rendering. Archive cleanup/ValueError bug (flagged in previous threads) is still present and tracked for #9; tempfile and returncode-check fixes also pending in #9.
vero/src/vero/harbor/runner.py HarborRunner: Mode B EvalStrategy. Previously-flagged issues (last-write-wins, missing --n-attempts/--max-retries) are now correctly fixed with best-rank selection and explicit flag emission; tests in TestReviewFixes cover both.
vero/src/vero/harbor/build/config.py BuildConfig Pydantic model: clean schema covering Mode A/B, split access specs, budgets, secrets, and image bases; relative-path resolution in from_file is correct.
vero/src/vero/harbor/build/templates/docker-compose.yaml.j2 Trust-boundary compose: secrets reach eval-sidecar only via ${VAR:?} fail-fast guards; token volume is ro to main (root can read it for the verifier, agent user cannot); admin volume never mounted to main.
vero/tests/test_harbor_runner.py Comprehensive unit tests covering command building, reward extraction, collation with missing trials, resume semantics, and the two previously-flagged fixes (best-trial ranking, flag emission).
vero/tests/test_harbor_build.py Build compiler tests exercise structure, ServeConfig validation, file parsing, pyproject path rewrite, shared baseline SHA, secret-check early-fail, and archive-failure detection; archive-failure test may not reach RuntimeError in current code due to the communicate() bug tracked in #9.
vero/examples/gaia-optimization/build.yaml Well-documented Mode B example with CAVEAT explaining why held-out partition IDs are readable by the optimizer when agent_repo is git-tracked; correct secrets, splits, budgets, and reward config.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Agent as Optimizer Agent
    participant SC as eval-sidecar
    participant HR as HarborRunner
    participant IH as Inner Harbor
    participant VF as Verifier

    Agent->>SC: vero harbor eval --split train
    SC->>HR: produce_sample_results
    HR->>IH: harbor run -n N -i task1 -i task2
    IH-->>HR: jobs_dir/ts/trial/result.json
    HR->>HR: _load_trials best rank per task
    HR->>SC: save SampleResult
    SC-->>Agent: aggregate score and budget

    Note over Agent,SC: optimizer iterates within budget

    VF->>SC: vero harbor finalize with admin token
    SC->>HR: produce_sample_results hidden test
    HR->>IH: harbor run test tasks
    IH-->>HR: result.json files
    HR->>SC: SampleResults
    SC-->>VF: reward.json final score
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Agent as Optimizer Agent
    participant SC as eval-sidecar
    participant HR as HarborRunner
    participant IH as Inner Harbor
    participant VF as Verifier

    Agent->>SC: vero harbor eval --split train
    SC->>HR: produce_sample_results
    HR->>IH: harbor run -n N -i task1 -i task2
    IH-->>HR: jobs_dir/ts/trial/result.json
    HR->>HR: _load_trials best rank per task
    HR->>SC: save SampleResult
    SC-->>Agent: aggregate score and budget

    Note over Agent,SC: optimizer iterates within budget

    VF->>SC: vero harbor finalize with admin token
    SC->>HR: produce_sample_results hidden test
    HR->>IH: harbor run test tasks
    IH-->>HR: result.json files
    HR->>SC: SampleResults
    SC-->>VF: reward.json final score
Loading

Comments Outside Diff (2)

  1. vero/src/vero/harbor/runner.py, line 739-752 (link)

    P1 n_attempts and max_retries silently dropped from harbor run

    HarborConfig exposes n_attempts and max_retries as typed fields (not extra_args pass-throughs), signalling that callers expect to configure retry behavior. Neither field is translated to a CLI flag in _build_command, so every harbor run invocation uses whatever the harbor defaults are regardless of what the caller configured. A user who sets max_retries=0 to disable retries on a budget-sensitive run will get the default retry behavior and may blow their budget.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: vero/src/vero/harbor/runner.py
    Line: 739-752
    
    Comment:
    **`n_attempts` and `max_retries` silently dropped from harbor run**
    
    `HarborConfig` exposes `n_attempts` and `max_retries` as typed fields (not `extra_args` pass-throughs), signalling that callers expect to configure retry behavior. Neither field is translated to a CLI flag in `_build_command`, so every `harbor run` invocation uses whatever the harbor defaults are regardless of what the caller configured. A user who sets `max_retries=0` to disable retries on a budget-sensitive run will get the default retry behavior and may blow their budget.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Cursor Fix in Claude Code Fix in Codex

  2. vero/src/vero/harbor/runner.py, line 799-813 (link)

    P1 Last-write-wins on duplicate task_name when harbor retries a task

    rglob("result.json") visits all result files across all timestamp directories under jobs_dir. If harbor's n_attempts or max_retries causes the same task to produce multiple trial entries with the same task_name, trials[task_name] = data simply overwrites with whichever file rglob happens to yield last — filesystem iteration order is not defined. This can cause a passing retry to be silently replaced by a failing one (or vice versa), yielding an incorrect score for that sample.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: vero/src/vero/harbor/runner.py
    Line: 799-813
    
    Comment:
    **Last-write-wins on duplicate `task_name` when harbor retries a task**
    
    `rglob("result.json")` visits all result files across all timestamp directories under `jobs_dir`. If harbor's `n_attempts` or `max_retries` causes the same task to produce multiple trial entries with the same `task_name`, `trials[task_name] = data` simply overwrites with whichever file `rglob` happens to yield last — filesystem iteration order is not defined. This can cause a passing retry to be silently replaced by a failing one (or vice versa), yielding an incorrect score for that sample.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Cursor Fix in Claude Code Fix in Codex

Reviews (4): Last reviewed commit: "Merge pull request #6 from scaleapi/harb..." | Re-trigger Greptile

@varunursekar
varunursekar requested a review from a team June 24, 2026 18:15
@varunursekar
varunursekar marked this pull request as ready for review June 24, 2026 18:22
Comment thread vero/src/vero/harbor/build/compiler.py
Comment thread vero/src/vero/harbor/build/compiler.py
# Execute
# ------------------------------------------------------------------

def _build_command(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

HarborConfig.n_attempts / max_retries are typed fields, which signals they are honored, but _build_command never emits flags for them — so every nested run uses harbor defaults. On a budget-graded run that is a real cost/correctness bug. Either map them to the real harbor run flags here, or drop the fields. (Confirms Greptile's P1.)

continue
task_name = data.get("task_name")
if task_name:
trials[task_name] = data

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

trials[task_name] = data is last-write-wins over an unordered rglob; with retries or a reused jobs_dir, a failing attempt can clobber a passing one, feeding a wrong score into auto_best. Collect all matches and pick deterministically (latest mtime / highest attempt index), and/or namespace jobs_dir per attempt. (Confirms Greptile's P1.)

values = [float(v) for v in rewards.values()]
return sum(values) / len(values) if values else 0.0

def _existing(self, params: EvaluationParameters, sample_id: int) -> SampleResult | None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

_existing treats a persisted error sample as 'done', so a sample whose nested run failed once is permanently skipped on resume and silently degrades the candidate's score. Either don't persist error samples, or make the pending/skip predicate treat error results as not-done.

Comment thread vero/src/vero/harbor/build/compiler.py Outdated
["tar", "xf", "-", "--strip-components", str(strip)],
cwd=dest, stdin=archive.stdout, check=True,
)
archive.wait()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

archive.wait()'s return code is discarded; if git archive fails, tar can still exit 0 on a truncated stream and you commit a near-empty baseline whose SHA gets baked into both images. Assert archive.wait() == 0 (and reap the Popen on the tar-failure path). (Confirms Greptile.)

environment:
VERO_HOME_DIR: "/opt/vero_home"
{% for secret in secrets %}
{{ secret }}: "${{ '{' }}{{ secret }}{{ '}' }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This renders SECRET: "${SECRET}", resolved from the run host's env, not anything the compiler validates. An unset var interpolates to empty and the sidecar comes up credential-less, failing every eval opaquely. Consider failing the build / emitting a required-secrets manifest when a declared secret is missing. Note test_rendered_files_parse only asserts the literal ${...} string, which hides this.

# ...except locked paths (e.g. the scorer): root-owned + unwritable.
if [ -e "/work/agent/{{ p }}" ]; then
chown -R root:root "/work/agent/{{ p }}"
chmod -R a-w "/work/agent/{{ p }}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This chown root:root + chmod -R a-w applies to the working tree under /work/agent, but the verifier never executes the working tree: _transfer_commit fetches git blobs and the evaluator checks the commit out into a fresh temp copy. The agent can git add -f a modified scorer and commit it regardless of working-tree perms. So read_only_paths is not a tamper control — please document it as advisory-only and move scorer-provenance enforcement into the sidecar (see the Mode A scorer-provenance comment on #4).

varunursekar and others added 4 commits July 13, 2026 15:51
- docs/harbor/architecture.md — what the integration is, the compiled-task topology,
  the two evaluation modes, the component map, and the leaderboard-integrity model.
- docs/harbor/tutorial.md — build and run an optimization task end to end (both modes,
  the agent-side protocol), and a Harbor section in the README.
- examples/gaia-optimization — a Mode-B example optimizing a GaiaAgent (a thin Terminus2
  subclass with an editable prompt) on gaia/gaia via a nested harbor run on Modal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Mode B (runner.py): `HarborRunner`, an `EvalStrategy` that — for each candidate —
  runs a *nested* `harbor run` of the agent over the selected Harbor tasks (e.g. on
  Modal) and collates the verifier rewards into vero `SampleResult`s. One Harbor task
  = one sample; inference is fully delegated, scoring comes from Harbor's verifier.
- The compiler (build/): `vero harbor build` renders a `BuildConfig` into a runnable
  Harbor task directory — a Docker Compose environment (optimizer workbench `main` +
  the eval sidecar + three volumes), two Dockerfiles, instruction.md, tests/test.sh,
  and the seed/solve scripts — baking the dataset/scorer/baseline repo and the
  sidecar's ServeConfig. Supports Mode A (local dataset/scorer) and Mode B (a registry
  or local Harbor benchmark, passed through to the HarborConfig).
- `.gitignore`: un-ignore src/vero/harbor/build/ (the repo's `build/` rule was hiding
  the compiler package).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…xample, by-mode scorer

Documentation accuracy fixes (review findings on PR #6):

- architecture: soften the intro from a hard guarantee ("the optimizer cannot
  read hidden labels, modify the scorer, or bypass its budget") to best-effort,
  OS/process-level language describing what is actually enforced.
- gaia build.yaml: correct "never reaches the optimizer". Because agent_repo is
  "." and build.yaml is git-tracked, the validation task ids ARE seeded into the
  optimizer's repo; only the per-sample scores are withheld. Acceptable for a
  public benchmark, with a caveat + mitigations for secret-identity benchmarks.
- examples: gsm8k-agent is cited as the Mode A example but ships no build.yaml;
  repoint to gaia-optimization as the complete runnable example and pair
  gsm8k-agent with the tutorial's Mode A snippet.
- architecture: document the current fail-open default for unlisted splits (and
  that it becomes fail-closed once the protocol fix lands), and split the
  "scorer is sidecar-only" claim by mode (true for Mode B; Mode A keeps the
  scorer in the agent's editable repo until the serve.py fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mkdtemp cleanup

Mode B runner + build-compiler robustness (review findings on PR #5):

- runner: emit --n-attempts / --max-retries (the typed HarborConfig fields were
  silently dropped); pick the best trial per task deterministically instead of
  last-write-wins over an unordered rglob; treat a persisted error sample as
  not-done so a transient failure is re-run on resume.
- compiler: assert git archive exited 0 (and reap it) so a truncated stream
  cannot bake a near-empty baseline; validate declared secrets at build time and
  render compose secrets with a fail-fast guard so an unset host var fails loudly
  instead of producing a credential-less sidecar.
- seed.sh: document that read_only_paths is advisory only and scorer provenance
  is enforced sidecar-side.
- compiler: stage the dataset in a cleaned-up TemporaryDirectory (Greptile:
  the mkdtemp scratch dir was leaking, datasets can be gigabytes).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread vero/src/vero/harbor/build/compiler.py
[Harbor 4/4] architecture docs, tutorial, and the GAIA example
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.

2 participants