Skip to content

Add a pinned Harbor adapter for S-Code - #130

Merged
weiddeng merged 10 commits into
mainfrom
codex/issue-129-harbor-adapter
Oct 20, 2026
Merged

weiddeng merged 10 commits into
mainfrom
codex/issue-129-harbor-adapter

Conversation

@weiddeng

Copy link
Copy Markdown
Collaborator

Closes #129.

Adds a pinned Harbor 0.17 adapter so S-Code can run as an agent on Harbor benchmarks such as TBLite and Terminal-Bench, plus the S-Code settings that unattended runs need. Phase 0 results are in this comment.

Commits

  1. feat: make unattended runs configurable. Turn limits move into configuration (turn.*, S_CODE_TURN_*). New settings: turn.disabled_tools, model request timeouts (model.*_timeout_seconds), an opt-in model.merge_system_messages, and a --raw-events-jsonl output mode.
  2. feat(harbor): build a pinned Linux runtime bundle. integrations/harbor/build-bundle.sh builds checksummed Linux x86-64 binaries from a clean commit, and runtime.py validates them.
  3. feat(harbor): add the Harbor 0.17 adapter. integrations.harbor.agent:SCodeAgent runs one S-Code turn per trial within the task's Harbor deadline, and reports runs that never reach the model as infrastructure errors.
  4. docs(harbor): document the adapter and run its tests in CI.

Backward compatibility

Nothing changes for existing users unless they opt in.

  • The new defaults equal the previous built-in values: the turn limits, and model timeouts of 10, 45 and 180 seconds.
  • No tools are disabled by default.
  • System messages are merged only when model.merge_system_messages is set.
  • The text, --jsonl and --stream-json output modes behave as before.

Testing

  • cargo fmt and cargo clippy -D warnings are clean. Tests pass for the CLI, config, model gateway and daemon.
  • Python tests pass: adapter (17), bundle (9) and CI plan (19). The adapter and bundle tests now run in the benchmarks CI job.
  • A bundle built from this branch's head (0f98212) passes the deterministic Harbor fixture as root and as a non-root agent user.
  • In Phase 0, run on an earlier build of this branch, 4 TBLite tasks with Qwen3.6-27B on vLLM all reached verification without infrastructure errors, and 3 got full credit.

Follow-ups

  • Configurable temperature and output length, to match baseline sampling.
  • An audit that flags trials invalidated by infrastructure failures in each benchmark run.
  • A glibc 2.31 bundle for Terminal-Bench 2.1, and matched baselines.

Benchmark harnesses such as Harbor run S-Code with no one present, often
against self-hosted models. Add settings for those runs. Every default
keeps today's behavior.

- Turn limits move into configuration: turn.max_model_calls,
  max_tool_calls, max_elapsed_seconds, model_stream_idle_seconds,
  max_cost_micros and max_total_tokens, with S_CODE_TURN_* overrides. The
  defaults are the previous built-in values.
- turn.disabled_tools (S_CODE_TURN_DISABLED_TOOLS) removes tools from the
  model's tool list and rejects them if they are called anyway, including
  from Code Mode.
- model.connect_timeout_seconds, read_timeout_seconds and
  request_timeout_seconds (S_CODE_MODEL_*_TIMEOUT_SECONDS) bound requests
  to endpoints set in the configuration file or environment, between 1 and
  3600 seconds. The defaults stay 10, 45 and 180 seconds.
- model.merge_system_messages (S_CODE_MODEL_MERGE_SYSTEM_MESSAGES) sends
  an OpenAI-compatible request's system messages, in order, as one leading
  message, for chat templates such as Qwen3.6's that reject later ones. It
  is off by default and, like the timeouts, applies to endpoints set in the
  configuration file or environment.
- A new --raw-events-jsonl output mode emits every raw event as versioned
  JSON Lines. In that mode a question for the user ends the run instead of
  waiting for --timeout. Other output modes are unchanged.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
Add integrations/harbor/build-bundle.sh. It refuses a checkout with any
uncommitted or untracked file and builds the Linux x86-64 s-code,
s-code-cli, s-code-daemon and s-code-pdf-worker binaries from HEAD. It then
runs their self-tests and writes a gzip tar archive with a SHA-256 sidecar.
The archive's manifest records the source commit and tree, the lockfile
digests, the tool versions, and each file's digest, size and mode.

integrations/harbor/runtime.py validates an archive against its SHA-256
digest and manifest without extracting or running anything. It uses only
the Python standard library.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
integrations.harbor.agent:SCodeAgent runs one non-interactive S-Code turn
per Harbor trial against an OpenAI-compatible endpoint. It is pinned to
harbor==0.17.0.

- Install: validates the bundle, uploads it, checks its digest inside the
  task container, and runs each binary's self-test with fresh S-Code state.
- Run: uses Full permission mode and --raw-events-jsonl. It disables the
  browser, PDF and interactive-input tools and merges system messages. It
  keeps S-Code's limits inside the task's Harbor deadline.
- Results: keeps events.jsonl, stderr.log, final.txt, provenance.json and
  daemon.log in the trial's agent logs. A run that never gets a model
  response is raised as Harbor's NetworkConnectionError or UnknownApiError,
  so it counts as an infrastructure error and the verifier still runs.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
Add integrations/harbor/README.md: requirements, building the bundle,
running a Harbor job, the agent kwargs and the files kept per trial. It
also explains that the task container must reach the model endpoint
itself.

Route integrations/harbor and its tests to the benchmarks CI job, and run
the adapter and bundle tests there.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
@weiddeng
weiddeng requested a review from sl-7qx as a code owner October 18, 2026 23:37
Benchmarks compare agents at fixed sampling settings. S-Code always samples
at temperature 0. It starts with 8,192 output tokens per model call and
doubles that after a cut-off response, up to 32,768.

Add model.temperature (S_CODE_MODEL_TEMPERATURE, 0 to 2) and
model.max_output_tokens (S_CODE_MODEL_MAX_OUTPUT_TOKENS, 1 to 1000000).
When they are unset, behavior is unchanged. An explicit output cap stays
fixed instead of doubling. The Harbor adapter passes both through as the
temperature and max_output_tokens kwargs and records them in provenance.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
The existing tests cover the sampling settings' validation and the agent's
fixed output cap, but none followed a configured value to the request the
model receives. Add one: it runs a turn without the settings and checks
the defaults (temperature 0, 8,192 output tokens), then with temperature
0.6 and max_output_tokens 4,096 and checks those values arrive.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>

@al2wang al2wang left a comment

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.

I reviewed the whole delta at 0f98212, against base eac5c1a (current main). That covers the adapter, the bundle builder and runtime manifest, the config, gateway, daemon and CLI changes, and the tests. I also read #131 and #132 for context.

The adapter is careful work:

  • The bundle is deterministic, and its manifest is checked without extracting it.
  • Every timeout is derived from Harbor's resolved deadline.
  • Cleanup is bounded, and cancellation survives it.
  • Event validation fails closed.
  • Disabled tools are filtered on every path I could find: the tool list, catalog search, executor preparation, Code Mode children and Chat start_work.
  • The new TurnConfig defaults equal the previous built-in limits.

Testing

CI is green at this head. Locally:

  • cargo fmt --check and clippy -D warnings are clean.
  • Tests pass for config (26), model-gateway (37) and cli (173).
  • s-code-daemon passes 239 of 240. The one failure is code_mode_worker::pdf_worker_runs_through_the_networkless_os_sandbox, which needs bwrap. That machine has none, and the test fails the same way on main there. It passes in CI.
  • The adapter (17) and bundle (9) tests pass on Python 3.12, and test-ci-plan.py (19) passes.
  • An import smoke test against the real harbor==0.17.0 wheel passes.

I did not run Harbor end-to-end with a model. The items below come from code reading, small container probes, and repro scripts that drive the PR's runner and adapter with fake binaries.

There are three themes, all about whether a Harbor score measures S-Code rather than the container or the harness. The first two mostly come from existing runtime behaviour, but this adapter is where they start to change results.

1. Programs that agent commands start don't see the task image's PATH or HOME

The program the model names in run_command is found on the daemon's own PATH (program_search_path, crates/tool-runtime/src/lib.rs:163 and :828-839). Under Harbor, that is the container's PATH. The command itself, though, runs with a cleared environment:

  • PATH becomes <staged program dir>:/usr/bin:/bin:/usr/sbin:/sbin (crates/platform-runtime/src/lib.rs:118, applied at :821-822);
  • besides it, only a per-command TMPDIR, HOME and XDG_CACHE_HOME are set;
  • the one exception is the managed-runtime shim path, which already lists /usr/local/bin (crates/tool-runtime/src/lib.rs:1852-1856).

So program: "python3" works, but anything a command then starts by name does not when it lives under /usr/local/bin or in a directory added by the Dockerfile's ENV PATH. Examples are sh -c "python3 …", a Makefile calling go, pip's console scripts, and npm scripts calling node.

That covers a lot of task images:

  • The official language images install their toolchains under /usr/local: python:*, node:*, golang:* and rust:*.
  • Many Dockerfiles add paths with ENV PATH=..., such as /opt/venv/bin or /root/.cargo/bin.

In python:3.12-slim, here is what a shell started this way sees:

$ env -i PATH=/bin:/usr/bin:/usr/sbin:/sbin /bin/sh -c 'command -v python3'
(exit 127)
$ env -i PATH=/bin:/usr/bin:/usr/sbin:/sbin /bin/bash -lc 'command -v python3'
/usr/local/bin/python3

So sh -c "python3 …" exits 127 unless the model happens to use a login shell. A login shell restores /usr/local/bin from /etc/profile, but it still drops paths that only exist in the Dockerfile's ENV. No run_command path defaults to a login shell. We saw exactly this in internal Terminal-Bench runs on python:*-slim images: shell-invoked python3 exited 127, including in the agent's own check scripts.

HOME is a fresh, empty scratch directory for each command (crates/tool-runtime/src/lib.rs:1831-1838), not the image user's home. So ~/.local (from pip install --user), ~/.config and git config --global don't persist from one command to the next.

The result is a silent shortfall that depends on the image and on the model's command style.

The smallest fix is to put /usr/local/bin (and /usr/local/sbin) ahead of /usr/bin for sandboxed workspace commands, matching what the shim path already does. For Harbor specifically, an adapter-managed opt-in might be better: give commands the agent user's container PATH (and HOME) as captured at install, and keep clearing everything else. A test that runs sh -c 'python3 --version' through run_command in python:3.12-slim would lock in either fix.

2. Files the agent creates are owner-only (0600/0700)

The runner sets umask 077 (integrations/harbor/agent.py:94) before s-code exec. The launcher sets it again in the subshell that starts the daemon (scripts/s-code:175). Nothing in the Rust crates resets it for child processes, so files that shell commands create are 0600 and directories are 0700.

In the same image:

  • With the image default (022), a new index.html is -rw-r--r--.
  • After umask 077, it is -rw------- in a drwx------ directory.
  • The result is the same when a child process with a cleared environment creates the file.

By default Harbor's verifier runs as the image's default user, usually root, so default runs won't show this. Some setups will fail where agents that keep the container's umask would not:

  • tasks whose verifier runs as a different non-root user (Harbor 0.17 has separate [agent].user and [verifier].user);
  • services running as another user (www-data, postgres);
  • tests that check file modes.

Separately, and already true on main, the edit tool does this regardless of umask. On Unix, apply_patch creates a new file through a temporary file opened with an explicit 0600 (crates/tool-runtime/src/lib.rs:1322-1328). Only files that already existed keep their mode (:1302-1310, :1334-1335). So a new script or config file the model creates through the edit tool is 0600 even under 022.

Suggestion: keep the private state protected with explicit modes, as the runner already does with mkdir -m 700. Then:

  • start tool processes with the container's original umask. For example, the runner could record $(umask) first, and platform-runtime could apply that value in pre_exec;
  • create new workspace files with 0666 & ~umask instead of a fixed 0600.

3. How outcomes reach Harbor (retries and the #132 audit)

  • S-Code's own outcomes look retryable. A tool-call or model-call limit stop, or an ordinary failed turn, ends with CLI exit 1. run() then re-raises Harbor's generic NonZeroAgentExitCodeError (agent.py:1107-1108). Under Harbor's default retry filter, that type is not excluded (harbor/models/job/config.py:288-296).
    • With --max-retries > 0, Harbor reruns the trial and deletes the failed trial directory first (harbor/trial/queue.py:200-222). That happens whatever the reward was.
    • The score quietly becomes "keep the last attempt" while provenance shows one attempt.
    • Baselines such as Terminus-2 return normally when their budget runs out.
    • #132 fixes this for timeouts by raising TimeoutError. The limit stops and ordinary failures are still affected.
    • Suggestion: return normally for S-Code-owned terminal outcomes (the verifier still scores them) and raise only for infrastructure. Or at least have the README name --retry-include NetworkConnectionError UnknownApiError.
  • A run that never starts is scored as an agent failure. If the daemon isn't ready within the launcher's 5 s window (scripts/s-code:185-194), the CLI exits 1 with no events and zero model requests. A repro with the real launcher and a daemon that never becomes ready exits 1 after 5.2 s. It also happens if the CLI refuses the workspace (see the / note below).
    • The classifier marks a run as an endpoint problem only when at least one request failed (agent.py:489-495). So this case becomes NonZeroAgentExitCodeError with failure_kind: null.
    • The task is untouched, so it scores 0. #132's audit_trial returns flagged: false for it.
    • Suggestion: treat "no turn started" (empty or absent event stream) as infrastructure.
  • Deterministic rejections are treated as endpoint faults. model_rejected covers every non-2xx response, including deterministic 400s such as a request over the model's context length.
    • It is raised as UnknownApiError, retried, and then excluded by #132's audit.
    • An S-Code prompt that a model cannot accept therefore disappears from the denominator instead of counting as a failure.
    • Suggestion: separate 400/413/422 from 401/403/429/5xx.
  • Token- and cost-budget stops look like crashes. These stops (TurnError::LimitExceeded, for example from record_usage at crates/agent-core/src/lib.rs:854) take the execution-error path. That path publishes turn.failed with only {"status":"failed","error_code":"agent_error"} (crates/daemon/src/lib.rs:15768-15772) and no turn.usage. The adapter records a generic failure with no limit_kind and no token counts.

Smaller notes

  • System-message merging changes behaviour for every model. S_CODE_MODEL_MERGE_SYSTEM_MESSAGES is managed and always true, so it applies to every model, not only to those whose chat templates reject later system messages. agent-core appends a budget-convergence system reminder at 80% of max_model_calls (crates/agent-core/src/lib.rs:662-667). With merging on, it moves into the leading system message, away from the latest turns, and the system prefix changes mid-turn. A kwarg defaulting to on would let other endpoints opt out.
  • Long instructions can't reach S-Code. The instruction is passed as one argv element (agent.py:155-167). At 128 KiB (MAX_ARG_STRLEN) the runner exits 126 with "Argument list too long" before S-Code starts. 131,071 bytes runs; 131,072 bytes does not. Terminal-Bench instructions are far smaller, but other Harbor datasets may not be.
  • A task with no workdir can't start. The runner doesn't set a working directory. A task with no [environment].workdir and no image WORKDIR runs in /. overlaps() in crates/cli/src/commands/workspace.rs treats / as overlapping /installed-agent/s-code-private/*, so the non-interactive CLI refuses the workspace. I found this by code reading; I did not run it.
  • Provenance doesn't identify the adapter. It pins the bundle but records the adapter only as the constant 0.1.0. The adapter decides timeouts, disabled tools, merging and classification, so a hash of integrations/harbor/*.py would make runs from different adapter code distinguishable.
  • Unmanaged settings go unrecorded. Unmanaged S_CODE_* variables in the agent env (S_CODE_CONFIG, S_CODE_MCP_ENABLED, and with #131 S_CODE_MODEL_TEMPERATURE) reach S-Code but not provenance. With #131, setting the temperature through the env records temperature: null. Rejecting unmanaged S_CODE_* names, or recording them, would keep provenance faithful.
  • The model key stays readable through /proc. The model credential stays in the daemon's environment for the whole run. Clearing the environment keeps it out of command environments, but a command running as the same user can still read /proc/<daemon pid>/environ. I checked this with a fixture value in python:3.12-slim. This is the usual trade-off for installed agents. A README note recommending a scoped, budget-limited key would cover it.
  • The adapter tests can't catch drift. They stub every Harbor module and hand-write raw events. A small CI step that imports the adapter against the pinned harbor==0.17.0 wheel would catch drift in the Harbor API. A golden events file captured from s-code exec --raw-events-jsonl would do the same for event names and reason strings.
  • #132's stack base. #132's parent is c0097f3, the first commit of #131, not #131's head 71e08c8. This will matter when the stack is retargeted.

@weiddeng

Copy link
Copy Markdown
Collaborator Author

Thanks for the careful review. Our plan for issue 1:

Under Harbor, S-Code runs in Full mode: run_host clears the environment, sets PATH to the program's directory plus /usr/bin:/bin:/usr/sbin:/sbin, and leaves HOME unset, with the effect you describe.

Fix: a Harbor-only "container environment" mode.

  • A default-off Cargo feature that only build-bundle.sh enables, switched on by the adapter. Normal builds can't activate it.
  • Full-mode commands get the baseline environment of an ordinary Harbor command in the task container, minus --ae variables, S_CODE_* and volatile shell fields. That PATH also finds the program; S-Code adds no PATH, TMPDIR or OPENSSL_CONF.
  • The adapter captures it at install as the agent user; S-Code loads it once and deletes it. Provenance records names and a hash, never values.
  • The sandbox, extensions and non-Harbor builds are unchanged.

We pass the whole baseline environment rather than just PATH and HOME because 6 tasks also need PYTHONPATH or Java, Gradle, Maven or Redis variables. We didn't adopt the global /usr/local/bin change because it would change behaviour for every S-Code user.

Tests cover normal builds, git config via HOME, custom paths and a canary key. Provenance will also record an adapter-code hash and unmanaged S_CODE_* names.

@al2wang al2wang left a comment

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.

Thanks, Wei. This is a follow-up on the new head a0722df; my earlier review was on 0f98212. It replies to the issue-1 plan and covers what changed since then: the #131 merge (c0097f3, 71e08c8; +296/−3 across 10 files).

A correction first

You're right about HOME, and my review was wrong.

  • The adapter runs s-code exec --permission-mode full, so commands go through run_host. It calls execute_host with an empty environment_handles (crates/tool-runtime/src/lib.rs:840-847).
  • After env_clear(), a command gets only PATH, a per-command TMPDIR, OPENSSL_CONF for node, and those empty handles (crates/platform-runtime/src/lib.rs:821-833). HOME is unset.
  • The per-command scratch HOME I cited (:1831-1838) is built only on the sandboxed path (run_with_compatibility).

The effect is the same either way: ~/.local, ~/.config and git config --global don't persist.

The issue-1 plan

I agree with the approach. It is the adapter-managed opt-in I suggested, and passing the whole baseline environment is the right call given the six tasks that need more than PATH. Keeping the global PATH change out of normal builds is fair. The adapter-code hash covers my provenance note.

A few things would make it robust:

  • Fail closed. If the feature is on and the capture file is missing, unreadable or malformed, stop the run with a clear reason. A silent fallback to the cleared PATH would bring back the original shortfall with no signal. A missing-file test would lock this in.
  • Protect the capture file.
    • Write it into the private directory at 0600, as the agent user.
    • Before loading it, have S-Code check that it is a regular file (not a link) with the expected owner and mode.
    • Delete it before the first command runs.
  • Keep the key out by name and by value. Besides subtracting the --ae names, drop the adapter's credential_env name, and any variable whose value equals the credential, in case the task environment carries it under another name. The canary test can check both the capture file and a command's environment.
  • Decide name collisions. If an --ae name also exists in the image's own ENV (say PYTHONPATH), subtracting by name drops the image's value too. Either behaviour is defensible, but it should be chosen and tested.
  • Name the volatile fields. List the exclusions explicitly (for example PWD, OLDPWD, SHLVL, _) and test that list, so a change in how Harbor runs commands can't quietly add entries.
  • Make the switch visible. The variable that turns the mode on should be managed, so the agent env can't override it. Provenance should record whether the mode was on, and with which bundle features, next to the names and hash.
  • Capture the umask in the same step. Recording the container's $(umask) at install is the missing piece for theme 2. platform-runtime could apply it to commands, and the edit tool could create new files with 0666 & ~umask instead of a fixed 0600.
  • Test the original symptom directly. Run sh -c 'python3 --version' through run_command in python:3.12-slim, next to the HOME/git-config test.

Are themes 2 (file modes) and 3 (how outcomes reach Harbor, beyond #132's timeouts) planned for this PR or for follow-ups? Either works; it would just help to know.

The #131 delta

This part looks good, and each layer has its own test:

  • Config bounds model.temperature to 0–2 and model.max_output_tokens to 1–1,000,000. The env parsing rejects non-finite values.
  • agent-core: fixed_max_output_tokens (serde default false) turns off output-token doubling at both growth sites, so a configured cap really is the cap.
  • The daemon applies the settings to agent turns only (execute_turn). Title generation keeps its fixed 0.
  • The adapter's kwargs are validated, sent only when set, and recorded in provenance.

Three small notes:

1. Temperatures go out as widened f32 values. (executed)

  • The daemon narrows the configured f64 to f32 (crates/daemon/src/lib.rs:1359).
  • The gateway puts the f32 into the body with json! (crates/model-gateway/src/lib.rs:433, :485, :558).
  • In serde_json 1.0.151 (the Cargo.lock version, without arbitrary_precision anywhere in the tree), Number::from_f32 stores f as f64 before formatting.

A small program built against that version prints:

0.25 -> {"temperature":0.25}
0.6 -> {"temperature":0.6000000238418579}
0.7 -> {"temperature":0.699999988079071}

Endpoints will treat these as practically the same. But the wire value no longer matches the configured one, or the 0.6 the adapter records. The gateway's f32 field predates #131; it only shows now that the value is configurable. The new gateway test uses 0.25, which f32 represents exactly (crates/model-gateway/src/lib.rs:2140), so it can't catch this. Carrying f64 through to the request, since config already holds one, plus a test with 0.6 would fix it.

2. An env-set temperature still reads null in provenance. (code reading)

  • S_CODE_MODEL_TEMPERATURE and S_CODE_MODEL_MAX_OUTPUT_TOKENS are managed only when their kwarg is set (integrations/harbor/agent.py:720-723). Otherwise a value from the agent env passes through (:727), and provenance records null (:879-880).
  • Your plan to record unmanaged S_CODE_* names would show that the variable was set, but not its value. For these two the value is the setting.
  • I'd refuse the two names in the agent env now that the kwargs exist, or record the value actually used.

3. A fixed cap and reasoning models. (code reading)

  • With a fixed cap, a cut-off or empty response is retried at the same cap, at most twice. Then the turn fails with "model returned repeated empty responses" or the incomplete-response reason (crates/agent-core/src/lib.rs:945-967, :982-1014).
  • That is the intended "fixed" meaning. But a reasoning model whose hidden reasoning uses a low cap will end turns this way, so a sentence in the docs and README would help benchmark users choose the cap.

Testing at this head

CI is red at this head; the previous head was green.

  • linux: tests::fast_durable_question_resume_keeps_tool_limit_fail_closed timed out with "the question must be answerable before its durable worker settles: Elapsed(())" (213 passed, 1 failed).
  • macOS: the CLI end-to-end check failed with "CLI did not exit after Ctrl-C".
  • source-gate fails as their aggregate.

Neither looks caused by this delta:

  • The durable-question test comes from main (c8ee6a4). It busy-polls for up to 5 s for the moment when the turn is AwaitingInput while the durable task is still Running, before the worker pauses itself (crates/daemon/src/lib.rs:33673-33692). A loaded runner can miss that window.
  • The delta doesn't touch the CLI or the interrupt path.
  • With the new settings unset, the agent-turn request is unchanged (temperature 0, 8192 tokens, cap not fixed).

Rerunning the failed jobs should show whether both are flakes.

Locally at a0722df (offline, --locked):

  • config: 27 passed;
  • model-gateway: 37 passed;
  • agent-core: 61 passed;
  • s-code-daemon --lib: 214 passed. That is the same target where CI had one failure, and it includes configured_sampling_settings_reach_the_model_request.
  • The durable-question test alone passed 20 of 20 runs.
  • The Harbor adapter (18) and bundle (9) tests pass on Python 3.12.

I did not run the macOS Ctrl-C check.

Under Harbor, S-Code runs in Full mode, where run_command clears the
environment and sets a minimal PATH and no HOME. Tools in /usr/local/bin
or in Dockerfile ENV PATH directories, PYTHONPATH and JAVA_HOME were
invisible to anything a command started, so scores could reflect the
harness rather than S-Code.

Add a Harbor-only container environment mode:
- A default-off Cargo feature, harbor-container-env, that only
  integrations/harbor/build-bundle.sh enables. The bundle manifest records
  it and the adapter refuses bundles without it, so normal builds cannot
  activate the mode.
- Before it sets anything of its own, the runner saves its environment as
  the agent user, minus every variable Harbor passed to the agent. The
  daemon loads it once at startup, only if it is a private regular file
  this user owns; it drops S_CODE_*, volatile shell fields and any variable
  holding the model credential, deletes the file and writes a summary of
  names and a hash.
- Full-mode commands then run in that environment, with PWD set to their
  directory, and are found on its PATH. S-Code adds no PATH, TMPDIR or
  OPENSSL_CONF. Sandboxed commands, extensions and normal builds are
  unchanged.
- credential_env must start with S_CODE_HARBOR_, and the adapter no longer
  sets TERM and NO_COLOR, which would otherwise reach every command.
- Provenance records the verified mode with the variable names and their
  hash, a hash of the adapter sources in its version, and the names of
  unmanaged S_CODE_* variables.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
A configured temperature was narrowed to f32 on its way to the model, so
0.6 went out as 0.6000000238418579 while provenance recorded 0.6. Carry
it as f64 through the daemon, agent-core and the gateway, and check 0.6
in the gateway test, which f32 cannot represent exactly.

The Harbor adapter now refuses S_CODE_MODEL_TEMPERATURE and
S_CODE_MODEL_MAX_OUTPUT_TOKENS in the agent env, so provenance records
the values actually used, and the docs explain that a fixed output cap
also bounds a reasoning model's hidden reasoning.

Refs #129

Signed-off-by: Wei Deng <wdeng@wustl.edu>
@weiddeng

Copy link
Copy Markdown
Collaborator Author

Thanks. Issue 1 is in a2509ff and your #131 notes in b6232f3.

Issue 1, against your list:

  • Fail closed: S-Code won't start without a snapshot, and the daemon exits if it is missing, unreadable, oversized or malformed.
  • Capture file: the runner writes it 0600 in the private directory, as the agent user, before setting any variable of its own. The daemon opens it without following links, requires a regular file it owns with no group or other access, and deletes it before listening.
  • Key: --ae names are dropped at capture and again in the daemon, along with S_CODE_* and any variable whose value is the model key; credential_env must start with S_CODE_HARBOR_. Canary tests cover the file and a command's environment.
  • Collisions and volatile fields: a task variable sharing an --ae name is dropped (documented, tested). PWD, OLDPWD, SHLVL and _ are an explicit, tested list, and PWD is set to the command's directory.
  • Visibility: the switch is managed. Provenance records host_command_env (container only once the daemon's summary is read), the bundle's Cargo features, and the names and hash.
  • Original symptom: in the Harbor fixture on python:3.13-slim, a run_command's sh finds python3 in /usr/local/bin and sees HOME and an image variable but no TMPDIR or credential, as root and as a non-root user.

Themes 2 and 3 will be follow-up PRs; the umask capture goes with theme 2.

#131: temperature is f64 end to end (gateway test at 0.6), the adapter refuses S_CODE_MODEL_TEMPERATURE and S_CODE_MODEL_MAX_OUTPUT_TOKENS in the agent env, and the docs note that a fixed cap also bounds hidden reasoning.

@weiddeng

Copy link
Copy Markdown
Collaborator Author

Also done, as promised (a2509ff): each run's record now shows which adapter code ran and any extra S-Code settings passed in. We're leaving your other small notes out of this PR.

@al2wang al2wang left a comment

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.

Thanks, Wei. This re-review covers a0722df..b6232f3: a2509ff (issue 1) and b6232f3 (the #131 notes).

The #131 notes (b6232f3)

All three are addressed:

  • Temperature is f64 from config to the request body, and nothing in those crates narrows it any more. The gateway test now checks 0.6, so it fails if anything narrows the value again.
  • The adapter refuses S_CODE_MODEL_TEMPERATURE and S_CODE_MODEL_MAX_OUTPUT_TOKENS unless a kwarg manages them, and provenance names any other unmanaged S_CODE_* variable.
  • The docs and README say the cap also bounds hidden reasoning.

Issue 1 (a2509ff)

This follows the plan and covers my list. What I checked:

  • The loader (crates/platform-runtime/src/container_environment.rs:110-229):
    • it opens with O_NOFOLLOW;
    • it requires a regular file owned by the effective uid, with no group or other bits;
    • it reads at most 1 MiB + 1 byte and deletes the file before any later check can fail;
    • it drops S_CODE_*, the agent names, the four volatile names and exact credential values;
    • it writes the summary with create_new at 0600, after removing a stale one.
  • Where the environment goes. Only run_host calls execute_host (crates/tool-runtime/src/lib.rs:850), so only Full-mode run_command gets the environment.
    • Code Mode has no run_command of its own (crates/daemon/src/code_mode.rs:850).
    • Background terminals and hooks are separate, user-confirmed paths. They keep the minimal environment.
  • The runner's capture. I ran its snippet under dash with a made-up environment. The file was 0600 and the --ae names were gone. PWD was captured, and the daemon drops it.

A few small points, none blocking:

1. Where the fail-closed lives. (code reading, plus an existing test)

  • In a feature build, the daemon loads nothing when S_CODE_HARBOR_COMMAND_ENV_FILE is unset (crates/daemon/src/main.rs:1043-1045; your test asserts this at :2584). Its commands then get the minimal environment. The comment above it (:1030-1032) says a missing setting stops the daemon.
  • In the Harbor flow the runner covers this case. It refuses unless the capture succeeded, and _assert_runner_refuses_without_snapshot tests that with no variable and with a missing file. So I don't see a gap in the flow as written.
  • The result can't tell the two cases apart, though. A completed run whose summary is missing still scores as completed, with host_command_env: "unverified" (test_successful_run_is_one_shot_and_records_usage, tests/test-harbor-adapter.py:1253-1280).
  • Having a feature build refuse to serve without the variable would make the comment true. --self-test already returns before the load (main.rs:1089-1094). Then unverified could only mean the summary was lost. Treating unverified as an infrastructure failure would be the stricter option.

2. A snapshot without PATH loads. (code reading)

  • load accepts an empty snapshot, or one with no PATH. It also skips records without =, while its doc comment says any failure is an error (container_environment.rs:106-108).
  • With no PATH, commands run with no PATH at all, while the program lookup falls back to the daemon's search path (host_search_path, crates/tool-runtime/src/lib.rs:2321-2329).
  • env -0 in a real task container always prints PATH, so a missing one means the capture went wrong. Refusing such a snapshot would match the rest of the design. Otherwise, narrow the comment.

3. The README's credential sentence. (docs) "Commands never receive --agent-env variables" (integrations/harbor/README.md:154) is accurate for what commands inherit. The /proc/<daemon pid>/environ point from my first review still applies, so adding "by inheritance" and the scoped-key line there would keep the sentence exact. I know you're leaving the other notes out; this one is just wording in a paragraph this PR adds.

4. The python:3.13-slim check. I couldn't find this run in the diff, and neither of us ran a container here. If it was a manual Harbor run, please paste a short transcript into the PR: the command, and what the run_command printed as root and as the non-root user. Committing it as an opt-in test would keep it.

Themes 2 and 3 as follow-ups sounds good.

Testing at this head

CI is green at b6232f3, including the new harbor-container-env steps.

Locally at b6232f3 (--locked):

  • Python 3.12: the Harbor adapter tests (25) and bundle tests (11) pass.
  • fmt and clippy: fmt is clean. Clippy with the feature on platform-runtime, tool-runtime and the daemon (--all-targets -D warnings) is clean. evals builds.
  • With the feature, these pass:
    • all 7 container_environment tests;
    • host_lookup_uses_the_container_path_before_the_daemon_path;
    • tests/harbor_container_env.rs;
    • the daemon binary's 20 tests, harbor_command_environment_needs_every_setting_and_drops_agent_names among them.
  • Without the feature, these pass: config 27, model-gateway 37, agent-core 61, s-code-daemon --lib 214, and normal_builds_never_have_a_container_environment.
  • Expected failures on this host: 13 platform-runtime sandbox tests and 1 tool-runtime sandbox test (commands_cannot_read_sensitive_workspace_files) fail with ENOENT, because this host has no bubblewrap.
    • The 13 fail the same way at a0722df. The tool-runtime test is unchanged in this delta.
    • CI, which has bubblewrap, passes them.

Signed-off-by: Wei Deng <wdeng@wustl.edu>
@weiddeng
weiddeng force-pushed the codex/issue-129-harbor-adapter branch from b835451 to c304734 Compare October 20, 2026 16:52
@weiddeng

Copy link
Copy Markdown
Collaborator Author

Thanks for the re-review. I addressed the two small follow-ups in c304734:

  • A Harbor-feature daemon now refuses startup when S_CODE_HARBOR_COMMAND_ENV_FILE is unset; the existing test was updated.
  • The README now clarifies that commands do not inherit --agent-env variables and recommends scoped, budget-limited credentials given same-user /proc access.

The requested python:3.13-slim evidence comes from earlier manual Harbor 0.17 fixture runs against b6232f3. The command recorded the environment to a file, which the verifier checked:

  • root: python3=/usr/local/bin/python3, HOME=/root, image marker present, TMPDIR and credential absent;
  • non-root: identical, with HOME=/home/fixture.

Both runs passed verification with reward 1 and no exception.

Stricter malformed/missing-PATH validation and classification of a missing completion summary remain follow-up hardening because complete fixes require broader behavior and tests. I'm not adding the optional Docker fixture to this PR; the manual runs provide the requested evidence without expanding its CI scope.

These were the last PR-scoped items. I consider #130 ready to merge once CI finishes—could you please approve it?

@al2wang al2wang left a comment

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.

Thanks, Wei. This re-review covers b6232f3..c304734 (9 lines in crates/daemon/src/main.rs and integrations/harbor/README.md).

The two follow-ups

Both are done.

  • The daemon refuses without the snapshot.
    • In a feature build, an unset S_CODE_HARBOR_COMMAND_ENV_FILE is now an error (crates/daemon/src/main.rs:1043-1044). Startup stops with "cannot load the Harbor command environment: S_CODE_HARBOR_COMMAND_ENV_FILE must be set" (:1337-1338), so the doc comment above the function (:1030-1032) is now accurate.
    • The test asserts the exact message (:2583-2586).
    • The callers that start a feature daemon without the variable still return before the load (main.rs:1088-1092). These are the --self-test runs in the adapter's install step (integrations/harbor/agent.py:991) and in build-bundle.sh:166, and they don't reach it. The CLI's --self-test doesn't start a daemon (crates/cli/src/args.rs:261-265).
    • The runner sets the variable for the real run (agent.py:837), so the Harbor flow is unchanged.
    • With this, host_command_env: "unverified" can only mean the summary was lost after a successful load. Classifying that as a follow-up is fine with me.
  • The README. "never inherit" plus the scoped, budget-limited credential and /proc sentence (integrations/harbor/README.md:154, :161-162) make the paragraph exact.

One optional tidy-up, for this PR or later: harbor_command_environment can no longer return Ok(None), so its Option and the if let Some at its call site (:1337) could go.

python:3.13-slim

Thanks for the transcript. That covers it: root and non-root both get /usr/local/bin/python3, the expected HOME, and no TMPDIR or credential, and both runs pass with reward 1. Those runs were at b6232f3. This delta only adds the refusal when the variable is unset, and the runner always sets it, so the result carries over.

The PATH validation and the missing-summary classification as follow-ups, and keeping the Docker fixture out of this PR, both sound good to me.

Testing at this head

CI is green at c304734 (run 39461326985): linux, macos, source-gate and the rest pass, and windows is skipped. The linux job runs the feature's clippy and tests, including cargo test -p s-code-daemon --features harbor-container-env --bin s-code-daemon, which holds the updated test.

Locally at c304734 (offline, --locked):

  • Python 3.12: the Harbor adapter tests (25) and bundle tests (11) pass.
  • fmt is clean, and clippy with the feature on the daemon (--all-targets -D warnings) is clean.
  • The daemon binary's 20 tests pass with the feature, the updated one among them.
  • The built feature daemon, started with no S_CODE_HARBOR_* variables:
    • --self-test exits 0 with "s-code web self-test ok";
    • a plain start exits 1 with Error: "cannot load the Harbor command environment: S_CODE_HARBOR_COMMAND_ENV_FILE must be set".
  • The CLI end-to-end scripts build the daemon without the feature (tests/test-cli-e2e.sh:56, tests/test-first-run.sh:66), so they never pick up the feature binary.

Nothing blocking remains from my reviews.

@weiddeng
weiddeng merged commit 556aa23 into main Oct 20, 2026
12 checks passed
@weiddeng
weiddeng deleted the codex/issue-129-harbor-adapter branch October 20, 2026 21:01
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.

Add a pinned Harbor adapter for S-Code

3 participants