Uh oh!
There was an error while loading. Please reload this page.
fix(eval): restore headless tools and enforce benchmark egress - #2947
Conversation
hqhq1025
commented
Aug 13, 2026
Real-host external seven-arm canary completed on the PR code line (
Evidence:
This confirms the earlier |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Superseded by the same review posted as a non-blocking comment.
Astro-Han
left a comment
There was a problem hiding this comment.
AI-assisted review: Claude Code (primary), with Codex and kimi-k3-256k run as independent adversarial reviewers. Findings verified against source in an AI-assisted pass; the severity split and the merge/follow-up decision are mine.
Summary
The profile mechanism, fail-closed bindToolsByName, egress URL normalization, and WAL-checkpoint artifact capture are all the right shape.
The blocker is narrower than the finding count suggests: the profile freezes tool names only. The system prompt, tool schemas, and execution boundary are still product defaults, so a product change still moves scores under an unchanged profile version. Items 1–4 are symptoms of that gap plus one placement decision — patching them individually leaves the structure intact.
Must fix before merge
1. Freeze the system prompt.interactive-run-composer.ts:199 only branches on childInstruction, so a profiled run still gets product identity, personalization, skills, and workspace instructions. assembleMainSessionSystemPrompt already takes identity?: boolean (main-session-prompt.ts:34). Without this, README.md:29's "freeze" claim does not hold.
2. Strip run_in_background / pty from Bash in this profile. Scoring correctness, not resources: the normal path calls waitForResidenciesExcept('runtime-resource') (hosted-execution-runner.ts:63), so a background process keeps mutating /app after the execution returns and the verifier races — silently. The cancel path waits for a process that cannot be stopped and burns the cell. Adding StopBackgroundTask is insufficient; it still relies on the model cleaning up.
3. Add a wire-level contract test. Pin a hash of the system prompt and full tool schema from the first provider request of a real hosted.execution.start. Composer-level assertions miss it: routeApplyPatchTools and bindToolResultArchiveDecoder (ai-sdk-backend.ts:1129) rewrite the tool set after binding.
4. Move toolProfile into SessionCreateInput. Reason is protocol cost, not present risk — once it ships next to maxSteps, relocating it is breaking; today it is a field move that also deletes HostedExecutionToolProfileRegistry (~100 net lines). It closes two findings by construction: the registry dies in the runner's finally while the session is durable, so a second turn.start or a refreshIdleBackends rebuild silently restores the full surface (hosted-execution-tool-profile.ts:50); and resolveAvailableToolNames / resolveNewSessionToolNames (execution-composition.ts:674, :740) rebuild a composer without the binding, so the projected and executed surfaces disagree. Reading from backendContext.header next to permissionMode fixes both.
Follow-up issues
All three are silent when they fail, and none trigger under the current config:
- Pin the egress image by digest.
docker-compose-egress-proxy.yaml:17uses a mutable local tag and the executor does not verify image identity, so a stale same-tag image applies different rules. apply_subject_egress_policymust fail closed.run_trial.py:135returns silently whenMAKA_EVAL_EGRESS_ALLOWED_HOSTis unset — enforcement skipped, no error, no artifact, run still completes.- Delete
webTools. Its tool-surface job is nowtoolProfile's, andwebSearch.enabled: falseduplicates the core default (runtime-policy.ts:182). What is left — incognito and thenetworkProxyconfig — has nothing to do with the name. Write the policy unconditionally.
On keeping this as one PR
Splitting means re-running the canary that already passed, so it is not worth it. Two consequences: independent revert is lost (an egress rollback takes the profile fix with it), and packages/runtime-host/** is a public contract change needing independent human review — suggest stating in the review record that the subtree was read line by line, so the requirement is met by the record rather than the PR boundary. If item 4 touches more callers than expected, splitting the Runtime Host part out at that point is the only cut worth making.
Trade-offs worth recording in the PR body
- The egress filter is a blocklist;
google.com/search?q=terminal+benchstays open (+does not match). State the threat model — accidental contamination vs. deliberate lookup — so it is not read as a complete guarantee. test_relay_artifacts.pyexits 127 on macOS for lack ofsetsid.
中文对照
AI 辅助审查：Claude Code 为主，Codex 与 kimi-k3-256k 作为独立对抗性审查方。finding 已在 AI 辅助的流程中对照源码核对；严重程度划分与合并/后续的取舍由我决定。
结论
profile 机制、fail-closed 的 bindToolsByName、egress URL 归一化、WAL checkpoint 的 artifact 捕获，形状都是对的。
阻断项比 finding 数量显示的更集中：profile 只冻结了工具名。 system prompt、工具 schema、执行边界仍是产品默认值，所以 profile 版本不变时，产品侧的改动依然会移动分数。第 1–4 项是这个缺口加上一个位置选择的症状，逐个打补丁会让 CI 变绿但结构问题原样保留。
合并前必修
1. 冻结 system prompt。interactive-run-composer.ts:199 只判断 childInstruction，所以 profile 命中的运行仍会拿到产品 identity、personalization、skills 和 workspace instructions。assembleMainSessionSystemPrompt 已经支持 identity?: boolean（main-session-prompt.ts:34）。不做这条，README.md:29 宣称的 "freeze" 不成立。
2. 在此 profile 内裁掉 Bash 的 run_in_background / pty。 这是评分正确性问题而非资源问题：正常完成路径调用 waitForResidenciesExcept('runtime-resource')（hosted-execution-runner.ts:63），后台进程会在 execution 返回后继续修改 /app，verifier 观察到竞态——且是静默的。取消路径则会等待一个无法停止的进程，烧掉整格。补 StopBackgroundTask 不够，它仍依赖模型主动清理。
3. 补 wire 级契约测试。 拦截真实 hosted.execution.start 的第一条 provider request，对 system prompt 和完整工具 schema 取 hash 钉死。composer 层断言盖不住：routeApplyPatchTools 和 bindToolResultArchiveDecoder（ai-sdk-backend.ts:1129）会在绑定之后改写工具集。
4. 把 toolProfile 移进 SessionCreateInput。 理由是协议成本而非当前风险——一旦作为 maxSteps 的兄弟字段发布，之后再移就是 breaking change；现在做只是移动一个字段，同时删掉 HostedExecutionToolProfileRegistry（净减约 100 行）。它还顺带从构造上消灭两条 finding：registry 在 runner 的 finally 里销毁而 session 是持久实体，所以第二次 turn.start 或 refreshIdleBackends 重建会静默恢复完整工具面（hosted-execution-tool-profile.ts:50）；而 resolveAvailableToolNames / resolveNewSessionToolNames（execution-composition.ts:674、:740）重建 composer 时不带绑定，导致投影面与执行面不一致。改从 backendContext.header 读（与 permissionMode 并列）两者同时消失。
转 issue 跟进
三条的共同点：当前配置下不触发，而失效时是静默的。
- egress 镜像按 digest 固定。
docker-compose-egress-proxy.yaml:17用的是可变本地 tag，executor 也不校验镜像身份，机器上存着同 tag 的旧镜像就会应用不同规则。 apply_subject_egress_policy必须 fail closed。run_trial.py:135在MAKA_EVAL_EGRESS_ALLOWED_HOST缺失时静默返回——强制被跳过，无报错、无 artifact，运行照常完成。- 删掉
webTools。 它的工具面职责现在归toolProfile，而webSearch.enabled: false与 core 默认值重复（runtime-policy.ts:182）。剩下的 incognito 和networkProxy配置与这个名字毫无关系。policy 应无条件写入。
关于不拆 PR
拆分意味着重跑已经通过的 canary，不划算。两个代价：失去独立 revert 能力（回滚 egress 会把 profile 修复一起带走）；packages/runtime-host/** 属公共契约变更，需要独立人工评审——建议在 review 记录里注明该子树已逐行阅读，用记录而非 PR 边界满足评审强度要求。如果第 4 项牵动的调用方超出预期，那时把 Runtime Host 部分拆出来是唯一值得拆的一刀。
建议写进 PR 正文的取舍
- egress filter 是 blocklist，
google.com/search?q=terminal+bench仍然开放（+不匹配正则）。建议写明威胁模型——防意外污染还是防主动查找——避免被读成完备保证。 test_relay_artifacts.py在 macOS 因缺setsid退出码 127。
Freeze a versioned HostedExecution tool ceiling for Maka benchmark subjects while keeping Runtime Host as the sole execution authority.\n\nGenerated-by: Codex
d78a983 to
11a9470Comparehqhq1025
commented
Aug 13, 2026
@Astro-Han Addressed the review on the latest Merge blockers
Follow-ups from the review
Validation
Real-host Maka smoke on the reviewed implementation:
|
Astro-Han
left a comment
There was a problem hiding this comment.
AI-assisted review: Claude Code (primary), with Codex and kimi-k3-256k run as independent fresh-eye adversarial reviewers on the updated branch. Findings verified against source in an AI-assisted pass; the severity calls and the merge decision are mine.
Approving. All four blockers from the previous round are addressed, and the wire-level contract test is stronger than what I asked for — pinning the full provider request across two turns is the strongest available assertion that the profile actually holds. The remaining findings are P2 and below; two of them are net deletions, so they are worth doing here rather than in a follow-up.
Worth doing before merge
egress_filter.py:25never sees the hostname.loweredis built from path and query only, sohttps://terminal-bench.io/tasks/answersandhttps://sub.tbench.ai/xboth returnNone— only the exacttbench.aimatch fires. Fix islowered = f"{host}{path_query}".lower()plus a suffix match fortbench.ai. Worth noting the audit log is archived by sha256 inharness-executor.ts:836as complete evidence, so the gap also overstates what the artifact proves.- Delete
create_harbor_trial(run_trial.py:147). It reimplementsTrial.createthrough two private methods and has already drifted — the copy drops thesource_trial/ RegradeTrial branch.resolve_trial_network_planreadsself.task.configduringrun(), sotrial = await trial_type.create(config)followed by mutatingtrial.taskis equivalent and removes the fork. Net deletion. - Roll back the
economychange forboundTools(interactive-run-composer.ts:159).mainapplies!process.env.MAKA_DISABLE_DEFERRED_TOOLSunconditionally; the new ternary also disables deferred loading on the product sub-agent path, which is a user-visible change with no test and no stated reason.economy: input.boundToolNames ? false : ...keeps the eval ceiling exact and leaves sub-agents alone. If the broader change is intended, it deserves its own commit. - Suppress and record artifact collection failures on the success path (
relay_agent.py:114,:153). The same call is wrapped incontextlib.suppresson the exception paths at:196and:204, and_persist_subject_outputsraises whenmkdirfails — so a scorable attempt becomes an infra failure if/logsis not writable. The asymmetry reads as an oversight rather than a decision. Suppress plus a structuredcollection-errorrecord (the shape already exists atmaka-artifacts.ts:98) keeps both properties. - Reject
egressProxyfor pier at decode time (harness-executor.ts:791).decodeOptionsaccepts it for any framework, but the pier branch atrun_trial.py:129never callsapply_subject_egress_policy— the proxy runs and the env vars are injected while namespace enforcement silently does not exist. No such spec exists today; a throw keeps it that way. - Freeze the turn tail. The profile replaces the base system prompt but still reuses the product
turnTailPrompt, which appends date, cwd, and git state to the user message (ai-sdk-backend.ts:1721). The persisted fingerprint (execution-model-composition.ts:265) covers only the base prompt and tool catalog, so the same profile version can produce different provider input across dates or branches — which is exactly the property this PR exists to guarantee.
One structural suggestion
RunCompositionSnapshot (core/src/run-composition.ts) already carries baseSystemPromptHash / toolCatalogHash / toolNames and is already written per Run by execution-model-composition.ts:276. Extending it to cover the full provider-visible prefix and surfacing it through the hosted execution projection into the Attempt Result would make "are these two runs comparable" a query rather than an archaeology exercise against a test constant. Worth doing because a hash pinned only in a test file invites fixing a red diff by editing the constant while the profile is still called v1. If it touches more callers than expected, that is the one cut worth splitting out.
Follow-up issues
Pin the egress image by digest; mount only mitmproxy-ca-cert.pem into the subject rather than the whole state volume (the mitmproxy confdir puts the CA private key there); drop the unconditional ICMP accept in network-policy:47-48 or narrow it to a stated destination; write a truncation marker when the audit log hits MAX_AUDIT_BYTES; treat a missing audit file as a failure rather than passing verification. Also some cleanup: hostedExecutionToolNames has no production caller, and the compose-file regex assertions in lifecycle-boundaries.test.ts assert YAML text shape rather than semantics.
Deliberately not suggesting
Collapsing the profile into a single application point, and versioning the egress topology into one egressPolicy field. Both are real — the profile applies at three separate places today, and the six egress fields in the experiment JSON are all bound to hardcoded values — but there is one profile and one topology, so the right time is when the second one arrives and the shape is known.
中文对照
AI 辅助审查：Claude Code 为主，Codex 与 kimi-k3-256k 作为独立 fresh-eye 对抗性审查方，针对更新后的分支重跑。finding 已在 AI 辅助的流程中对照源码核对；严重程度判断与合并决定由我负责。
approve。上一轮四项必修全部落实，wire 级契约测试比我要求的更强——钉住跨两轮 turn 的完整 provider request，是"profile 确实生效"这件事最强的可用断言。其余 finding 均为 P2 及以下，其中两条是净删代码，值得在本 PR 里顺手做掉而非另开。
合并前值得做
egress_filter.py:25从不看主机名。lowered只由 path 和 query 构造，因此https://terminal-bench.io/tasks/answers与https://sub.tbench.ai/x都返回None，只有精确匹配的tbench.ai会命中。修法是lowered = f"{host}{path_query}".lower()并把tbench.ai改为后缀匹配。附带一提：审计日志的 sha256 在harness-executor.ts:836被当作完整证据归档，所以这个缺口也让该 artifact 的可信度标注高于实际。- 删掉
create_harbor_trial（run_trial.py:147）。 它用两个私有方法重写了Trial.create，且已经发生漂移——副本丢掉了source_trial/ RegradeTrial 分支。resolve_trial_network_plan是在run()期从self.task.config现算的，所以trial = await trial_type.create(config)之后再改trial.task完全等价，并消除这个 fork。净删代码。 - 回退
boundTools的economy改动（interactive-run-composer.ts:159）。main上是无条件!process.env.MAKA_DISABLE_DEFERRED_TOOLS；新的三元式把产品子代理路径的 deferred 加载也一并关掉了，属于用户可见变更，无测试、无理由说明。economy: input.boundToolNames ? false : ...既保住 eval 侧的精确天花板，又不动子代理。若确实想改子代理行为，它值得单独一个 commit。 - 正常路径的 artifact 采集改为 suppress + 记录（
relay_agent.py:114、:153）。 同一调用在:196和:204的异常路径上被contextlib.suppress包着，而_persist_subject_outputs在mkdir失败时会 raise——于是/logs不可写时，一个本可评分的 attempt 会变成 infra failure。这个不对称更像疏漏而非设计。suppress 加一条结构化collection-error记录（maka-artifacts.ts:98已有该形状）可以两者兼得。 - decode 期拒绝 pier 的
egressProxy（harness-executor.ts:791）。decodeOptions对任何 framework 都接受它，但run_trial.py:129的 pier 分支从不调用apply_subject_egress_policy——代理照跑、环境变量照注，而命名空间级强制静默不存在。当前没有这样的 spec，一个 throw 可以保证以后也没有。 - 冻结 turn tail。 profile 替换了基础 system prompt，但仍复用产品的
turnTailPrompt，后者会把日期、cwd 和 git 状态追加到 user message（ai-sdk-backend.ts:1721）。持久化的 fingerprint（execution-model-composition.ts:265）只覆盖基础 prompt 与工具 catalog，因此同一 profile 版本在不同日期或分支下会产生不同的 provider 输入——而这正是本 PR 要保证的性质。
一条结构性建议
RunCompositionSnapshot（core/src/run-composition.ts）已经携带 baseSystemPromptHash / toolCatalogHash / toolNames，也已由 execution-model-composition.ts:276 逐 Run 写入。把它扩展到覆盖完整的 provider-visible 前缀，并经 hosted execution projection 落进 Attempt Result，可以让"这两次跑分可比吗"变成一次查询，而不是对着测试常量做考古。值得做的理由是：哈希只钉在测试文件里时，diff 变红最省事的修法就是改常量，而 profile 仍然叫 v1。如果它牵动的调用方超出预期，那是唯一值得拆出去的一刀。
转 issue 跟进
egress 镜像按 digest 固定；只把 mitmproxy-ca-cert.pem 挂进 subject，而不是整个 state 卷（mitmproxy 的 confdir 会把 CA 私钥放在里面）；去掉 network-policy:47-48 无条件的 ICMP accept，或收窄到明确目标；审计日志达到 MAX_AUDIT_BYTES 时写一条截断标记；审计文件缺失时判为失败而非照常通过。另有一些清理：hostedExecutionToolNames 没有生产调用方；lifecycle-boundaries.test.ts 里对 compose 文件的正则断言检查的是 YAML 文本形状而非语义。
刻意不建议的
把 profile 收敛成单点施加，以及把 egress 拓扑版本化成一个 egressPolicy 字段。两者都是真实的——profile 目前在三处分别生效，experiment JSON 里那六个 egress 字段也全绑死在硬编码值上——但现在只有一个 profile、一个拓扑，正确的时机是第二个出现、形状明确的时候。
Uh oh!
There was an error while loading. Please reload this page.
Summary
headless-coding-v1Hosted Execution profile for Maka Eval without reviving the removed Headless packageBash,Read,Glob,Grep,apply_patch, and runtime-ownedArchiveRead; exclude memory, task, goal, skill, web, automation, and parent-agent toolsWebSearch,WebFetch, andFetchURLfrom all eight harness request surfaces, with structural filtering at the external metering proxy--noproxyr.jina.aiwrappersruntime.sqlitewith WAL contents, runtime policy, stdout/stderr, manifests, byte counts, and SHA-256 across settled, timeout, cancellation, and failure pathsRoot causes
main-based tool-profile branch; additionally, relay cleanup deleted the fixed stdout artifact path.Validation
Local:
8925d4353git diff --checkReal host (
dhb):apply_patch,ArchiveRead,Bash,Glob,Grep,Readcurl --noproxy '*'failed with exit 35runtime.sqlitewith SHA-256 plus runtime policy, stderr, manifest, and empty bounded egress audit; the final stdout retention fix has a dedicated regression testmaka-eval-egress-proxy:12.2.3image built successfully on the VMSS hostDeployment boundary
Eval semantics and enforcement live in this PR. The machine-local prerequisite is only the pinned
maka-eval-egress-proxy:12.2.3image; the former shared systemd proxy experiment was removed. Harbor download and verifier networking remain unchanged; onlyAgent.run()is forced through the per-cell proxy.