fix(adapters): Toolathlon spawn-time token resolution + exit-code verifier scoring#887
fix(adapters): Toolathlon spawn-time token resolution + exit-code verifier scoring#887bingran-you wants to merge 13 commits into
Conversation
…ifier scoring Follow-up to #885. Makes the materialized Toolathlon tasks actually runnable. 1. Token spawn-time resolution. The ${token.X}->${TOOLATHLON_X} env rewrite could never resolve per-task or runtime-computed tokens (Drive folder ids written by preprocess, groundtruth allowlists, per-student Canvas tokens): MCP env/args pass through verbatim and args cannot be resolved from env at all. New _toolathlon_container.py ships into each container: `write-config` bakes the global token_key_session.py from injected TOOLATHLON_* secrets; `launch` wraps every MCP server and resolves ${token.X}/${config}/paths across argv AND env at spawn, mirroring upstream utils/mcp/tool_servers.py (global merged with per-task override). Dep-free via an addict shim; absolute interpreter + PATH fallback so FastMCP can spawn it. 2. Pre-create declared server dirs. arxiv_local (--storage-path) and the emails server create working dirs lazily; an agent that reaches the goal another way leaves them absent and evaluators that os.listdir them crash. The launcher mkdir -p's an explicit allow-list of directory-valued flags. 3. Exit-code verifier scoring. Upstream evaluators score by exit code and several signal task-failure by raising (e.g. raise ValueError("Some tests FAILED")) rather than returning False. Treat any non-zero evaluator exit as reward 0 (healthy fail); only escalate to a verifier error on signatures that cannot be an agent failure (ModuleNotFoundError/ImportError/PermissionError). 4. Tolerate task_config entries naming local harness tools (e.g. web_search) with no MCP yaml instead of failing the whole adaptation. Validated on Daytona+OpenHands+Azure gpt-5.5: 37/58 headless tasks healthy on first pass; find-alita-paper and google-cloud/maps/github/hf probes all reward 1.0. 9 adapter tests, ruff + ty clean.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7deb4b2cfa
ℹ️ 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".
| ("kubeconfig_path", None, "deployment/k8s/configs/cluster1-config.yaml"), | ||
| ("emails_config_file", None, "configs/example_email_config.json"), |
There was a problem hiding this comment.
Use absolute paths for local config defaults
For tasks using the emails or k8s MCPs with the generated defaults, these values are passed as --config_file/KUBECONFIG_PATH to servers whose upstream YAMLs do not set cwd; BenchFlow starts the agent/MCP session in /workspace/agent_workspace, so the relative paths resolve under /workspace/agent_workspace/... while the cloned Toolathlon files live under /workspace/.... Those MCP servers will fail to start unless these defaults are absolute (or the server cwd is forced to /workspace).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2fe7eba3: the generated Toolathlon global token defaults now use absolute /workspace/... paths for both kubeconfig_path and emails_config_file, so MCP servers no longer depend on being spawned from /workspace. Covered by test_toolathlon_container_write_config_bakes_secrets; focused validation passed (tests/test_source_adapters.py 14 passed, ruff, ty, format check). GitHub CI is still running on the new head.
| timeout=30, | ||
| ) | ||
|
|
||
| def test_toolathlon_container_write_config_bakes_secrets(tmp_path: Path) -> None: |
There was a problem hiding this comment.
Add required regression-test docstrings
The root AGENTS.md says regression tests must name the PR/commit they guard in the docstring, but this new Toolathlon regression test has no docstring at all. Please add docstrings to the newly added regression tests that cite this fix (for example 44087fa or the PR) so future maintainers know which regression they protect.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is already addressed on the PR head: the PR #887 regression tests now have docstrings naming the guard, including the Toolathlon verifier scoring and container-helper regressions. I also added two more PR #887 regression docstrings in 2fe7eba3 for the launch-time comment-only cases. Focused validation passed (tests/test_source_adapters.py 14 passed, ruff, ty, format check).
… access Upstream preprocess/eval read all_token_key_session via attributes (all_token_key_session.github_token) because upstream builds it with addict.Dict. The generated global token_key_session.py used a plain dict, so every github/HF/woocommerce/notion/snowflake task whose preprocess reads a token that way crashed with AttributeError. Emit a self-contained attribute-accessible dict subclass.
…ass launch_time Two upstream-parity fixes to the preprocess/verifier invocation: - Run as tasks.finalpool.<task>.preprocess.main (and .evaluation.main) from /workspace, exactly as upstream (uv run -m <dotted path>), so 'from ..utils' relative imports resolve. Running it as a bare 'preprocess.main' broke them. - Pass --launch_time (a single value shared by preprocess and the verifier via /workspace/.toolathlon/launch_time.txt), matching upstream, but only to scripts that declare it (argparse errors on unknown args otherwise).
…al config The notion preprocess (utils.app_specific.notion.notion_remove_and_duplicate) reads source_notion_page_url / eval_notion_page_url off the global all_token_key_session, but the container helper only baked the two integration keys — the URLs resolved to None and every notion task's setup crashed before duplicating the target subpage. Forward TOOLATHLON_NOTION_SOURCE_PAGE_URL and TOOLATHLON_NOTION_EVAL_PAGE_URL through the token env allowlist and add both to _GLOBAL_TOKENS. Bumps adapter version to invalidate the materialization cache.
… credential
Snowflake tasks need configs/snowflake_rsa_key.p8 for keypair auth, but the
credential-injection path validated every payload as JSON and chmod'd 0644 —
wrong for a PEM private key. Add content_format ('json'|'pem') and file_mode to
the credential spec: PEM payloads skip JSON validation and land at 0600. Register
the .p8 spec (b64_env TOOLATHLON_SNOWFLAKE_RSA_KEY_B64, trigger server snowflake).
Removes the last adapter blocker for the 4 Snowflake tasks (account still needs
a human trial signup).
|
Daily scan update (2026-07-03): pushed commit |
|
Users Simulation review (2026-07-03): blocked. Simulation passed the targeted static/unit surface, but reproduced a real Toolathlon user failure:
Blocker: No model-backed trajectory was generated in this smoke, so there was no |
|
Automation follow-up (2026-07-03): pushed What changed:
Validation on the pushed head: GitHub CI is currently running on |
|
Automation status update (2026-07-03): GitHub checks are green on |
|
Users Simulation automation review (2026-07-04): ready on current head PR-scoped validation re-ran the Toolathlon launch-time regression surface:
Shared sweep evidence: Docker OpenHands + DeepSeek on |
Resolve Toolathlon adapter conflicts with the latest runtime sidecar work while preserving PR #887 launch-time and verifier-scoring regressions.
|
Automation refresh (2026-07-05): pushed Resolution kept the latest main-side Toolathlon service/runtime work while preserving the PR #887 fixes:
Local focused validation on the pushed head: GitHub now reports the PR as mergeable, but CI is still running on |
|
Automation follow-up (2026-07-05): pushed Fix in
Local validation after the follow-up:
Keeping |
|
Automation status update (2026-07-05): CI is now green on Current check state:
I moved the PR from |
|
Automation refresh (2026-07-05): Conflict/update notes:
Local validation on
Temporarily moved back to |
|
Automation status update (2026-07-05): refreshed CI is green on Current check state:
Moved the PR from |
|
Users Simulation automation review (2026-07-05): ready on head PR-specific validation passed in an isolated worktree: CLI help surfaces, SDK/import config smoke, focused source-adapter/provider/process/ACP/eval tests ( Shared gates passed: uv sync, ruff, ty, 71-test CLI/task/usage slice, bench help/task check/SDK smoke. Live Docker/OpenHands + Gemini flash-lite canary is healthy: ACP+LLM trajectories, results.jsonl training_ready=1, reward 1.0, 24,869 tokens, $0.00079715, 69.8s at /tmp/benchflow-users-simulation-20260705/shared-openhands-gemini-flash-lite-default/2026-07-05__05-11-14/hello-world-task__40312589. OpenHands max-effort was quarantined because that ACP agent does not declare reasoning_effort=max. Labels: keep |
|
Users Simulation automation review (2026-07-06): ready by simulation on head Evidence:
Caveat: the CI GLM user-endpoint artifact has |
|
Users Simulation automation review (2026-07-08): ready by simulation on head Evidence:
Thermo-nuclear review: no structural blocker found; the change remains localized to adapter behavior and regression coverage. Metadata caveat: GLM artifact token/timing coverage is healthy, but Labels: |
|
Users Simulation automation review (2026-07-10): ready by targeted simulation; one maintainability follow-up on head Verified:
The live model-backed Codex rollout attempt did not produce a publishable artifact because the local auth/effort path failed ( Thermo-nuclear maintainability note: the new Toolathlon subprocess/verifier smoke helpers were added to |
|
Users Simulation automation review (2026-07-11): ready by simulation on head Scope checked: Toolathlon spawn-time token resolution, exit-code verifier scoring, source/provider selection, and CLI/SDK regression risk. Evidence:
Artifact health:
Thermo review: no production-code blocker. The fix stays in the adapter/container-helper layer, uses focused helpers, and avoids broad special-case branching. Non-blocking maintainability note: the Toolathlon subprocess/verifier tests now grow the already-large source-adapter test module; splitting them later would improve scanability. |
fix(adapters): resolve Toolathlon MCP tokens at spawn + exit-code verifier scoring
Follow-up to #885. Makes the materialized Toolathlon tasks actually runnable.
Token spawn-time resolution. The ${token.X}->${TOOLATHLON_X} env rewrite
could never resolve per-task or runtime-computed tokens (Drive folder ids
written by preprocess, groundtruth allowlists, per-student Canvas tokens):
MCP env/args pass through verbatim and args cannot be resolved from env at
all. New toolathlon_container.py ships into each container:
write-configbakes the global token_key_session.py from injected TOOLATHLON* secrets;
launchwraps every MCP server and resolves ${token.X}/${config}/pathsacross argv AND env at spawn, mirroring upstream utils/mcp/tool_servers.py
(global merged with per-task override). Dep-free via an addict shim; absolute
interpreter + PATH fallback so FastMCP can spawn it.
Pre-create declared server dirs. arxiv_local (--storage-path) and the emails
server create working dirs lazily; an agent that reaches the goal another way
leaves them absent and evaluators that os.listdir them crash. The launcher
mkdir -p's an explicit allow-list of directory-valued flags.
Exit-code verifier scoring. Upstream evaluators score by exit code and
several signal task-failure by raising (e.g. raise ValueError("Some tests
FAILED")) rather than returning False. Treat any non-zero evaluator exit as
reward 0 (healthy fail); only escalate to a verifier error on signatures that
cannot be an agent failure (ModuleNotFoundError/ImportError/PermissionError).
Tolerate task_config entries naming local harness tools (e.g. web_search)
with no MCP yaml instead of failing the whole adaptation.
Validated on Daytona+OpenHands+Azure gpt-5.5: 37/58 headless tasks healthy on
first pass; find-alita-paper and google-cloud/maps/github/hf probes all reward
1.0. 9 adapter tests, ruff + ty clean.
Context
Follow-up to #885 (which materialized the tasks but left token resolution non-functional and misclassified raise-to-fail evaluators as infra errors). Validated live on Daytona + OpenHands + Azure gpt-5.5.
Test
Latest automation fix (2026-07-03)
Pushed
2fe7eba3to clear the current Toolathlon user-simulation blockers:launch_timewith AST-based detection of an actualargparse.add_argument("--launch_time", ...)declaration before injecting the flag in preprocess/evaluator scripts;/workspace, so MCP servers do not depend on the agent workspace cwd;Local validation on the PR head:
GitHub CI is running on the new head and should be checked before moving this out of
status:blocked.