Skip to content

fix(agents): run the no-web JSON policy on the provisioned Node runtime - #1048

Open
tulerfeng wants to merge 1 commit into
benchflow-ai:mainfrom
tulerfeng:fix/1047-node-json-policy-merge
Open

fix(agents): run the no-web JSON policy on the provisioned Node runtime#1048
tulerfeng wants to merge 1 commit into
benchflow-ai:mainfrom
tulerfeng:fix/1047-node-json-policy-merge

Conversation

@tulerfeng

@tulerfeng tulerfeng commented Aug 22, 2026

Copy link
Copy Markdown

Summary

Agent-owned pre-launch setup runs inside the task image, but the no-web JSON settings merge shelled out to python3 — something no task image is required to ship. On a Python-free task with allow_internet = false, all four JS agents (claude-agent-acp, gemini, opencode, mimo) died at exit 127 before ACP ever started. I moved the merge onto the Node runtime BenchFlow provisions itself and kept the output byte-identical, verified across 40 base-vs-head combinations and end-to-end on both docker and daytona with a real Gemini key.

Fixes #1047

Reproduction

I reproduced it with a real bench eval run. tests/examples/hello-world-task already ships a Python-free image (ubuntu:24.04 + curl), so all I had to change was network_mode = "no-network":

RuntimeError: Failed to apply no-web policy for gemini: exit code 127;
command: export BENCHFLOW_AGENT_HOME=/home/agent; python3 -c '...'
error_category: other

I got a byte-identical failure on --sandbox docker and --sandbox daytona, which matches the report: an infrastructure fault surfacing as a rollout error rather than a model failure.

The fix

I moved the merge onto /opt/benchflow/node/bin/node — the absolute path _js_agent_install already provisions — and translated the four mutators to JavaScript. The ordering holds by construction: install_agent() is rollout/__init__.py:1195, apply_web_tool_policy is :1228, so the runtime is always in place before the policy needs it. I left the path as an unquoted shell word so the surrounding bash expands $BENCHFLOW_AGENT_HOME, which is what os.path.expandvars did before.

Output stays byte-identical, including non-ASCII. This is the one place I could have made the change silently lossy. json.dumps defaults to ensure_ascii=True; JSON.stringify emits raw UTF-8. Semantically equivalent, but not the same bytes — and an agent home can arrive pre-populated from the host, since gemini copies ~/.gemini/settings.json in via subscription_auth. A user with non-ASCII values would have had that file rewritten on their first upgraded run, breaking exactly the idempotence this merge promises. So I re-escape from U+007F up, matching json.dumps byte for byte. I start at U+007F rather than U+0080 because JSON.stringify has already escaped in-string control characters, while the newlines and indent it emits must stay literal.

I also kept the semantics strict rather than quietly "improving" them: if(!('k' in d)) reproduces setdefault instead of d.k ||= {}, so a config with "tools": null still fails loudly rather than being silently repaired. Same fail-closed behaviour, same error surface.

The ownership boundary is now asserted, not implied. The reason a python3 dependency could sit in a JS agent's policy unnoticed is that nothing tied the two together. I added test_node_backed_policies_are_guaranteed_by_their_own_install, which walks the whole registry and requires that any policy reaching for the Node prefix belongs to an agent whose own install_cmd provisions it — so the next agent that grows a JSON policy either brings its runtime or fails in CI.

End-to-end verification

Both canaries are live runs, not stubs: I used a real Gemini API key on gemini-3.5-flash-lite and a real Daytona cloud sandbox. Same task and flags throughout — tests/examples/hello-world-task with network_mode = "no-network", --agent gemini, --trials 1.

Sandbox Version Result
docker 62cc7e41 (previous version) Dies at apply_web_tool_policyexit code 127, python3: command not found. Never reaches ACP
docker this PR Clears the policy; ACP connection initialised and _configure_acp_session completed. Stops further downstream at enforce_agent_egress_firewall (acp/runtime.py:692) — iptables: Permission denied, the container lacked NET_ADMIN. Unrelated to this change and strictly after the step it fixes
daytona 62cc7e41 (previous version) Same exit 127 at the same step — the failure is backend-independent
daytona this PR Full graded rollout: 1/1 passed, mean reward 1.00, errors=0, 56.6k tokens, telemetry 100% — the agent actually solved the task, with model traffic flowing through the in-sandbox LiteLLM proxy

Beyond the canaries I A/B'd the merge itself directly: 4 agents × 10 scenarios, previous version vs this PR, byte-for-byte identical in all 40 — absent file, empty file, unrelated keys, partly-applied config, already-applied (idempotence), Chinese text, emoji (surrogate pair), mixed non-ASCII with quotes and backslashes, null where an object is expected (both fail), malformed JSON (both fail). I ran the previous version in python:3.12-slim and this PR in node:22-slim, with no Python present at all.

Acceptance criteria

Criterion Evidence
Setup depends only on runtimes BenchFlow provisions Generated commands for all four agents contain no python3 and start with the provisioned Node prefix; a registry-wide invariant test ties the dependency to the installer
Preserve JSON merge, idempotence, ownership repair, fail-closed Merge, idempotence and fail-closed: the 40 byte-identical combinations above. Ownership repair covers a different code path — the chown that apply_web_tool_policy appends with && — so I exercised it separately: with BENCHFLOW_AGENT_HOME=/home/agent and a root-owned pre-existing config, the policy leaves both the directory and the file agent:agent and writable by the sandbox user
Regression in a Python-free image after runtime provisioning test_no_web_policies_apply_in_a_python_free_task_image: asserts ubuntu:24.04 ships no python3, runs _NODE_INSTALL, applies each policy twice, checks the resulting config content. Fails on the previous version with exit 127 python3: command not found, passes on this PR
Docker no-network canary reaches ACP start The docker / this PR row above
Daytona equivalent reaches ACP start, confirming backend parity The daytona / this PR row above. Parity holds in both directions: the previous version fails identically on both backends, this PR clears the policy on both

Regression coverage

I added three tests to tests/test_internet_policy.py: the two invariants above plus the containerised regression (skipif no Docker, in line with how the repo guards container/docker tests).

I also had to fix six existing tests, which is the report's own point — _run_setup_cmd executes policy commands on the developer host, where Python is always present, so it could never have caught this. It now retargets the sandbox Node prefix at the host's node and skips when there is none; real task-image coverage lives in the new container test rather than in a host shell.

Full suite: 5742 passed, zero regressions against 62cc7e41 — I checked that the failure set is identical with the change stashed. ruff check and ruff format --check clean.

Agent-owned pre-launch setup executes inside the task image, which owes
BenchFlow no interpreter. The no-web JSON merge shelled out to python3 for
four JS agents whose installs guarantee Node, so a valid Python-free task
with allow_internet = false aborted at exit 127 before ACP launch.

Reimplement the merge on /opt/benchflow/node/bin/node, the absolute path
_js_agent_install already provisions, and translate the four mutators to
JavaScript. Output stays byte-identical: the escape pass reproduces the
ensure_ascii behaviour of json.dumps, without which upgrading would rewrite
non-ASCII values in a settings file copied in from the host via
subscription_auth and break the idempotence this merge promises.

Fixes benchflow-ai#1047

@devin-ai-integration devin-ai-integration Bot left a comment

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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

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.

Agent pre-launch policy assumes undeclared runtimes in task images

1 participant