Uh oh!
There was an error while loading. Please reload this page.
fix(eval): harden external prompt and admission handling - #2963
Conversation
a3dadfc to
82c22a0Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for chasing these down — the seven-arm matrix failing before model admission is a real problem and the diagnosis is sound. Two things block this one for me, both because they change numbers we publish rather than because the code is wrong in the abstract.
Before anything else: the description covers two of the five commits.5f920fa66 and 82c22a0f2 are described; 3dd99bb20, cfa20fe34 and eeec9529a are not, and they carry the largest behavior change in the PR — stream persistence, a usage checkpoint recovered through the relay, and a second place where admission decides scoring. The validation list almost certainly predates them. Whatever else changes, the body needs to describe the diff.
P0 — only Pi's prompt was modified. Laying the eight arms side by side:
| arm | args tail |
|---|---|
| codex, claude-code, reasonix, opencode | -- {{task.input}} |
| zcode | --prompt={{task.input}} |
| kimi-code | --prompt {{task.input}} |
| pi | Task instruction:\n{{task.input}} |
Six arms receive context.taskInput verbatim. Pi receives it with a prefix. That is not a difference in argv syntax, it is a difference in the model's input — different first tokens, different framing of the instruction — and it lands in the spec that produces cross-arm comparison numbers. lifecycle-boundaries.test.ts:684 then freezes it as an assertion, so nobody downstream will read it as a defect.
P0 — admission now counts responses that never produced billable inference.origin/main had if (upstream.ok && parsed.admitted). This PR makes it parsed.admitted || isSuccessfulInferenceResponse(upstream.status, projected.model), which drops the upstream.ok guard from the first branch entirely — so the body's "keeps non-2xx provider and transport failures classified as infrastructure failures" no longer holds. A 500 carrying a choices array is now admitted. And isSuccessfulInferenceResponse itself only asks for a 2xx plus a model string somewhere in the request body, ignoring the URL path and the response content; a live proxy run admits 200-wrapping-an-error, 200-with-empty-body, 204 No Content, and /v1/messages/count_tokens — which claude-code issues routinely, which returns 200, and which bills nothing. Each of those flips an attempt from infra_failed (not scored, runner.ts:289) to failed (verifier runs, reward 0 recorded, runner.ts:296).
Root cause, and it is the same one twice. Both halves substitute a proxy signal for the fact actually needed. Admission needs "the provider produced billable output" — that fact lives in usageParser, and usageParser is what is broken: it splits on \n and JSON.parses each line, so a non-streaming, pretty-printed JSON body fails on every line and yields neither admission nor usage. The fallback makes the symptom disappear while the tokens and cost stay lost — a real billed pretty-JSON response still comes back usage: null, costUsd: null, usageComplete: false. Prompt delivery needs "every arm receives byte-identical task input", an invariant that is nowhere stated, so each arm's argv drifted on its own and one of them drifted into changing the prompt.
Minimal path. Restore the upstream.ok guard on the first branch. Narrow the fallback so it only applies when usage was parsed, which keeps the Reasonix cases while removing every row in that table. Give Pi the prompt unmodified — = form if it has such a flag, otherwise stdin (one change in the wrapper at harbor-external-subject.ts:429, currently stdio: ['ignore', 'pipe', 'pipe']), and failing both, drop Pi from the matrix rather than comparing it. Fix kimi-code's --prompt {{task.input}}, which is the exact shape you just declared broken for zcode. Replace the per-arm literal assertions with one parity assertion: after stripping flag syntax, every arm's prompt element equals {{task.input}}.
Cleanest path. Fix the parser instead of guarding around it: have finish() retry a whole-body JSON.parse when line-wise parsing produced nothing, which requires keeping the raw text rather than discarding buffered. That admits the Reasonix runs and recovers their usage and cost, and isSuccessfulInferenceResponse can be deleted rather than narrowed. Make prompt parity a spec-level invariant checked once for all arms instead of a literal per arm. And split this: the argv changes are self-evident and low risk, while the admission change rewrites scoring attribution and wants independent human review under AGENTS.md — right now it is spread across four commits and thirteen files, so it cannot be reverted on its own.
Two smaller notes. The new admission branch has no coverage — the integration children all send body: '{}' with no model field, so it never fires; reverting harbor-external-subject.ts:762-763 to main's line leaves provider-admission-integration.test.js 4/4 green. And "escape" in the title is misleading: argv reaches the container through shlex.join plus shlex.quote (relay_agent.py:296, :305) and is spawned as an execve argv array, so no shell is involved and there is no injection surface — what these changes do is disambiguate each CLI's own option parser.
Review assistance: Claude Code (Opus) ran parallel passes over prompt delivery, admission attribution, and test quality, reproduced the misclassification table against a live metering proxy, and drafted this comment. I verified independently the parts it turns on: the eight-arm args table by decoding the spec, the upstream.ok deletion against origin/main, the five commits against the description, and the second admission consumer in external-subject.ts. The live reproduction rows and the mutation results are AI output that I have not re-run myself.
中文
先谢谢你把这些追出来——七臂矩阵在模型 admission 之前就失败是真问题，诊断也站得住。有两条我认为阻塞合并，原因不是代码抽象层面写错了，而是它们改的是我们会对外发布的数字。
先说一件事：描述只覆盖了五个提交里的两个。5f920fa66 和 82c22a0f2 写了；3dd99bb20、cfa20fe34、eeec9529a 一个字没有，而它们承载了本 PR 最大的行为变更——流持久化、经 relay 回收的 usage checkpoint，以及第二处由 admission 决定计分的地方。Validation 里的数字几乎可以肯定早于这三个提交。无论其他怎么改，body 都需要如实描述这个 diff。
P0 —— 只有 Pi 的 prompt 被改了。 八臂并排看：
| arm | args 末尾 |
|---|---|
| codex, claude-code, reasonix, opencode | -- {{task.input}} |
| zcode | --prompt={{task.input}} |
| kimi-code | --prompt {{task.input}} |
| pi | Task instruction:\n{{task.input}} |
六臂拿到的是逐字的 context.taskInput，Pi 拿到的多了一段前缀。这不是 argv 语法差异，是模型输入的差异——首 token 不同、指令的框架化程度不同——而且它落在产出跨臂对比数字的那份 spec 里。lifecycle-boundaries.test.ts:684 又把它冻结成断言，之后不会再有人把它读成缺陷。
P0 —— admission 现在会把从未产生计费推理的响应算进去。origin/main 是 if (upstream.ok && parsed.admitted)，本 PR 改成 parsed.admitted || isSuccessfulInferenceResponse(upstream.status, projected.model)，第一支的 upstream.ok 守卫被整个去掉了——所以正文里「keeps non-2xx provider and transport failures classified as infrastructure failures」不再成立，一个带 choices 的 500 现在算 admitted。而 isSuccessfulInferenceResponse 本身只要求 2xx 加上请求体里任意位置有个 model 字符串，不看 URL path、不看响应内容；真实起代理跑下来，200 包错误、200 空 body、204 No Content、以及 /v1/messages/count_tokens（claude-code 常规发送、返回 200、不计费）全部被判 admitted。每一条都会把 attempt 从 infra_failed（不计分，runner.ts:289）翻成 failed（跑 verifier、把 reward 0 写进 attempt，runner.ts:296）。
根因，而且是同一个根因出现了两次。 两半都用一个代理信号替换了真正需要的事实。admission 需要的事实是「provider 确实产出了可计费输出」——这个事实在 usageParser 里，而坏掉的正是 usageParser：它按 \n 切行、逐行 JSON.parse，于是非流式、被格式化过的 JSON 响应体每一行都失败，既拿不到 admission 也拿不到 usage。兜底让症状消失，token 和成本却仍然全丢——一次真实计费的 pretty JSON 响应回来仍是 usage: null, costUsd: null, usageComplete: false。prompt 传递需要的事实是「每一臂收到逐字节相同的任务输入」，而这条不变量没有被写在任何地方，于是各臂的 argv 各自漂移，其中一臂漂到了改 prompt 上。
最小方案。 恢复第一支的 upstream.ok 守卫。把兜底收窄成「仅当 usage 已解析成功时生效」，这样既保住 Reasonix 那两例，又消掉上表每一行。让 Pi 拿到未经修饰的 prompt——有 = 形式的 flag 就用，没有就走 stdin（改一处 wrapper，harbor-external-subject.ts:429 现在是 stdio: ['ignore', 'pipe', 'pipe']）；两条都不行，就把 Pi 从矩阵里摘出来而不是拿它对比。修掉 kimi-code 的 --prompt {{task.input}}，那正是你刚判定 zcode 要修的形态。把逐臂冻结字面量换成一条 parity 断言：剥掉 flag 语法后，每一臂的 prompt 元素都等于 {{task.input}}。
最彻底方案。 修解析器而不是在它外面加护栏：让 finish() 在逐行解析没有产出时，对累积的完整 body 再整体 JSON.parse 一次（需要保留原始文本，目前 buffered 被丢弃）。这样 Reasonix 那几次既能正确 admitted，usage 和成本也一起修好，isSuccessfulInferenceResponse 可以直接删掉而不是收窄。把 prompt parity 做成 spec 级不变量、对所有臂检查一次，而不是每臂一条字面量。另外这个 PR 该拆：argv 那部分自证、风险低，而 admission 改的是计分归因，按 AGENTS.md 需要独立人审——它现在摊在四个提交、十三个文件里，没法单独 revert。
两条小的。新的 admission 分支没有覆盖——集成测试的 child 全部发 body: '{}'、没有 model 字段，所以它从未触发；把 harbor-external-subject.ts:762-763 还原成 main 那一行，provider-admission-integration.test.js 仍是 4/4 全绿。另外标题里的 "escape" 有误导：argv 经 shlex.join + shlex.quote（relay_agent.py:296、:305）进容器，再以 execve argv 数组 spawn，全程没有 shell，不存在注入面——这些改动做的是消解各 CLI 自己的选项解析歧义。
评审协助说明：Claude Code (Opus) 并行跑了 prompt 传递、admission 归因与测试质量三轮审查，用真实 metering proxy 复现了那张误判表，并起草了本条评论。结论所依赖的部分我独立核实过：解码 spec 得到的八臂 args 表、对照 origin/main 确认 upstream.ok 被删、五个提交与描述的对照，以及 external-subject.ts 里第二处 admission 消费点。表中的实时复现行与变异测试结果属于 AI 输出，我没有自己重跑。
| "--thinking", | ||
| "max", | ||
| "{{task.input}}" | ||
| "Task instruction:\n{{task.input}}" |
There was a problem hiding this comment.
P0 — this is the only arm whose prompt content changed. The other six pass {{task.input}} verbatim; this one prepends Task instruction:\n, which changes the model's first tokens and how framed the instruction is. This file is the source of the cross-arm comparison, so Pi's column stops being comparable with the rest.
lifecycle-boundaries.test.ts:684 freezes it as assert.equal(pi.config.args?.at(-1), 'Task instruction:\n{{task.input}}'), so from here it reads as contract rather than defect.
If Pi really does treat dash-leading positionals as options even after --, the options that preserve the prompt are a =-form flag (as zcode now uses), or stdin — the wrapper spawns with stdio: ['ignore', 'pipe', 'pipe'] at harbor-external-subject.ts:429, so that is one central change rather than a third per-arm workaround. If neither works, dropping Pi from the matrix is more honest than comparing it.
P2, same file — kimi-code at :237-238 is still "--prompt", "{{task.input}}", byte-identical to the shape you just declared broken for zcode and fixed at :260. It is not in the failing-arm list because the eighth arm was not run; enabling it fails the same way. One line.
| } finally { | ||
| const parsed = parser.finish(); | ||
| if (upstream.ok && parsed.admitted) { | ||
| const admitted = |
There was a problem hiding this comment.
P0 — two things, one line.
origin/main:660 was if (upstream.ok && parsed.admitted). The new expression drops upstream.ok from the parsed.admitted branch, so a non-2xx response whose body happens to carry a recognized envelope is now admitted — isInferenceAdmissionEvent matches on Array.isArray(event.choices) alone, so a 500 with a choices body qualifies. That directly contradicts "keeps non-2xx provider and transport failures classified as infrastructure failures".
And isSuccessfulInferenceResponse asks only for 2xx plus a model string in the request body — no URL path, no response content. A live proxy run admits 200-wrapping-an-error, 200-with-empty-body, 204 No Content, and /v1/messages/count_tokens, which claude-code issues routinely and which bills nothing. Each flips infra_failed (not scored) to failed (reward 0 recorded).
There is also a quieter cost: usageComplete requires usageRequests === admittedRequests (:705, :792), so every newly-admitted request with no parseable usage pushes usageComplete false and drops costUsd to null at :101.
The narrow fix is (upstream.ok && parsed.admitted) || isSuccessfulInferenceResponse(...) plus requiring parsed usage before the fallback applies. The fix I would rather see is below at usageParser — the Reasonix cases are a parse gap, not an admission gap.
| } | ||
| } | ||
| function usageParser(anthropic: boolean): { |
There was a problem hiding this comment.
P2 — this is where the Reasonix cases actually fail. Splitting on \n and JSON.parse-ing each line only works for SSE and JSONL; a non-streaming, pretty-printed JSON body fails on every line, so the request yields neither admission nor usage.
The fallback added above makes the admission half disappear while the usage half stays broken — a genuinely billed pretty-JSON response still returns usage: null, costUsd: null, usageComplete: false. Retrying a whole-body JSON.parse in finish() when line-wise parsing produced nothing fixes both, and lets isSuccessfulInferenceResponse be deleted rather than narrowed. It needs the raw text kept rather than buffered being discarded line by line.
Worth attaching the two Reasonix responses to the PR — without the actual envelopes there is no way to tell whether widening admission is even the right shape for them.
| status: context.signal?.aborted | ||
| ? ('indeterminate' as const) | ||
| : ('infra_failed' as const), | ||
| : recovered && recovered.admittedRequests > 0 |
There was a problem hiding this comment.
P2 — a second place where admission decides scoring, and it inherits everything above: any of the misclassified 2xx cases makes admittedRequests > 0 here and turns a result-transport failure into a scored failed.
Test coverage does not reach it. The new external-subject.test.ts case loops two executions, but the first has termination: 'framework_timeout' and returns at :89 before ever reaching this branch — both paths happen to produce 'failed', so the assertion passes while only testing one. Mutating :137-139 to a constant 'failed' leaves that file 8/8 green. A case with admittedRequests: 0 expecting infra_failed would close it.
| return false; | ||
| } | ||
| export function isSuccessfulInferenceResponse(status: number, model: string | undefined): boolean { |
There was a problem hiding this comment.
P2 — provider-admission.test.ts:24 asserts 204 is admitted. 204 No Content means the provider returned no output at all, so this writes a wrong semantic into the contract; it should be false, and the predicate narrowed to 200.
The rest of that test restates the two-line implementation — a range check and a non-empty string check — rather than exercising a behavior. The coverage that is missing is at the integration level: every child there sends body: '{}' with no model field, so this predicate never returns true in any integration case, and reverting harbor-external-subject.ts:762-763 to main's line leaves provider-admission-integration.test.js 4/4 green.
| } | ||
| assert.equal(zcode.config.args?.includes('{{task.cwd}}'), true); | ||
| assert.equal(zcode.config.args?.at(-1), '--prompt={{task.input}}'); | ||
| assert.equal(pi.config.args?.at(-1), 'Task instruction:\n{{task.input}}'); |
There was a problem hiding this comment.
P3 — freezing each arm's literal separately is what let Pi's prompt drift without anyone noticing; the assertions record what each arm does rather than what all arms must share.
One parity assertion would carry the invariant instead: strip the flag syntax (-- separator, --flag= prefix) and require every arm's prompt element to equal {{task.input}}. That fails loudly on the next arm that improvises, and it is the natural place to state the rule — nothing in the repo currently says the task input must reach every subject unmodified.
hqhq1025
commented
Aug 13, 2026
Real-machine validation update (VMSS, Terminal-Bench 2.1):
Validation:
Remaining non-Eval gaps reproduced with complete evidence: Reasonix adaptive/gpt2 and Kimi adaptive fail before model admission, so no provider usage exists to recover. Validated deployment commit: |
Astro-Han
commented
Aug 13, 2026
Writing as a downstream consumer of The problem being solved is real. The fact you want is "the provider produced billable output", and that fact currently lives in
if(upstream.ok&&parsed.admitted)The PR requires either of two, neither of which is that: constadmitted=parsed.admitted||isSuccessfulInferenceResponse(upstream.status,projected.model)Two separate widenings. Three consumers in my arm, and two of them decide whether an attempt is retried.
The first two are the same boundary, and that boundary is the difference between an infrastructure retry and a recorded result. Widening admission moves an attempt from "retry it" to "the subject failed" — which scores zero and does not retry. A provider 500 whose stream carried an admission event, or a 2xx that yielded no usage, becomes a silent zero rather than a retried cell. The third one moves the other way: The part I most want on the record: in the head-to-head cohort this is one-sided. I am not asking for a particular fix, and I do not think the answer is to revert to
Happy to adapt on my side instead if you would rather the semantics change and the consumers follow — but then I would want to land that after #2971, so the arm is not written against a definition that is mid-flight. Separately, and unrelated to the above: this PR and #2971 both read |
Astro-Han
commented
Aug 18, 2026
/agentic_review |
Code Review by Qodo
1. Trusts subject usage checkpoint |
| await environment.download_file(source, destination) | ||
| value = json.loads(destination.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
1. Trusts subject usage checkpoint 🐞 Bug⛨ Security
Fix-now: the relay accepts a schema-valid checkpoint from the subject-controlled /logs/agent path, which the external CLI can overwrite before exit, and the adapter trusts its admittedRequests, usage, and cost to change a missing-result-frame outcome from infra_failed to failed. A task-controlled agent can therefore forge model-admission and billing evidence, violating the attribution boundary the recovery path is intended to preserve.
Agent Prompt
## Issue description
The recovery path accepts a schema-valid checkpoint from a subject-controlled location without proving that the metering proxy produced it. Do not trust this file as accounting or admission evidence without host-owned integrity protection, because a forged checkpoint can alter reported usage, cost, and inference-admission classification.
## Issue Context
The wrapper creates the checkpoint before launching the external CLI, and the relay downloads it after that CLI exits, allowing the same external process to replace the file in between. Path validation and structural schema checks constrain only the checkpoint's name and shape, not its provenance or contents.
Reuse the existing relay result token as the checkpoint-authentication secret; it is already shared between the wrapper and relay, avoiding another secret, configuration field, or authority. Have the wrapper MAC a canonical checkpoint payload, and make the relay reject recovery checkpoints whose MAC is absent or invalid before forwarding their data.
## Fix Focus Areas
- packages/eval/harbor/relay_agent.py[324-339]
- packages/eval/src/external-subject.ts[195-235]
- packages/eval/src/harbor-external-subject.ts[695-713]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| await environment.download_file(source, destination) | ||
| value = json.loads(destination.read_text(encoding="utf-8")) |
There was a problem hiding this comment.
2. Bounds recovery checkpoint reads 🐞 Bug☼ Reliability
Fix-now: _read_recovery downloads, decodes, and JSON-parses an arbitrarily large subject-controlled file selected from the writable log directory without a size or regular-file check. A child can replace the checkpoint before timeout, teardown, or result-frame recovery, causing unbounded host temporary-disk, memory, and JSON-parsing work; the fixed pathname regex does not constrain the target’s type or size.
Agent Prompt
## Issue description
Recovery downloads and parses an arbitrarily large subject-controlled checkpoint, allowing host temporary-disk, memory, and JSON-parsing exhaustion during timeout, teardown, or result-transport failure handling. Reject missing, non-regular, symlinked, and oversized checkpoint data before decoding JSON.
## Issue Context
The path regex protects only the pathname; it does not constrain the type or size of the file the external process leaves at that path. The existing result-carrier limit cannot be reused because it applies only after stdout is already present in relay memory, while this recovery path uses `environment.download_file`. Introduce a small dedicated checkpoint byte limit, reject non-regular, symlinked, or oversized sources before transfer where the environment API permits, and enforce the same limit while reading the downloaded file so resource exhaustion cannot prevent finalization.
## Fix Focus Areas
- packages/eval/harbor/relay_agent.py[33-38]
- packages/eval/harbor/relay_agent.py[324-339]
- packages/eval/harbor/test_relay_artifacts.py[67-84]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| costUsd: | ||
| checkpoint.costUsd === null | ||
| ? null | ||
| : nonnegative(checkpoint.costUsd, 'external checkpoint cost'), |
There was a problem hiding this comment.
3. Reports incomplete checkpoint costs 🐞 Bug≡ Correctness
Fix-now: decodeUsageCheckpoint accepts usageComplete: false but still returns costUsd, so timeout and result-frame-failure paths report an estimate made from only a subset of admitted requests. This contradicts the wrapper’s normal-result contract, which deliberately returns costUsd: null until all admitted requests have usage.
Agent Prompt
## Issue description
Do not expose checkpoint cost when its `usageComplete` flag is false. Recovered partial usage may remain useful, but its estimated cost is not authoritative.
## Issue Context
The normal wrapper result already suppresses cost unless all admitted requests have usage. Recovery should preserve that same accounting invariant rather than interpreting the checkpoint estimate differently.
## Fix Focus Areas
- packages/eval/src/external-subject.ts[218-235]
- packages/eval/src/harbor-external-subject.ts[99-102]
- packages/eval/src/harbor-external-subject.ts[697-705]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
jackwener
left a comment
There was a problem hiding this comment.
Automated review of exact head eeec9529ad69dd0be90408f8e35deaaea871181e against current main@d62857a8357e9160926726a2a13096bc2dc2b91d.
The reported failures still exist on current main: external arguments interpolate dash-leading task text without a profile-aware option boundary, and admission still depends only on recognized response envelopes. The repair direction is therefore relevant, but this revision is not safe to transplant as written.
P1 — recovered metering is subject-forgeable and unbounded. The wrapper writes ${profile}.provider-usage.json inside /logs/agent, the same writable environment in which the measured external CLI runs. After the child exits, relay_agent.py:_read_recovery downloads and JSON-parses that path with no authentication, regular-file proof, or byte cap, and external-subject.ts:decodeUsageCheckpoint trusts its usage, cost, and admittedRequests to decide billing and whether a missing result is an infrastructure retry or a scored subject failure. A task-controlled process can replace the file, forge a non-retryable zero/result, or exhaust host disk/memory during download. Current main already has the correct single authority: signed, bounded, atomic metering-checkpoint data keyed by meteringSecret; rebasing must preserve it rather than reintroduce this older plain-JSON protocol.
P1 — non-2xx responses can become admitted attempts.harbor-external-subject.ts:762-764 uses parsed.admitted || isSuccessfulInferenceResponse(...); the first branch no longer requires upstream.ok. An error response carrying a recognizable event therefore increments admittedRequests, moves recovery from infra_failed to failed, suppresses retry, and can record a zero. Keep success status as a conjunct for the retry boundary; if “billable” and “admitted” need different widening, represent them as different facts instead of sharing one counter.
Required conclusions:
- Optimal for the actual problem: no; useful prompt/artifact changes are coupled to a superseded metering authority.
- Production code to delete: delete the PR's duplicate plain-JSON checkpoint/recovery path after rebasing; reuse current main's signed checkpoint.
- Tests to delete/replace: replace fixture-written trusted checkpoint tests with adversarial forged/oversized/non-regular checkpoint cases against the real signed seam.
- Deeper refactor: rebase first, preserve the current metering authority, then transplant the remaining prompt escaping, artifact inventory, and narrowly defined admission change.
- Ready to merge: no; it is also currently conflicting with main.
- Residual risks/gaps: Eval scoring, retry classification, and usage/cost are protected contract behavior and require independent human review plus exact-head CI and real-host evidence after rebase.
Astro-Han
commented
Aug 23, 2026
Hi — this PR conflicts with current I tested a rebase onto current
These are real source conflicts, so they need your judgement rather than a mechanical rebase — please rebase onto current Thanks for the contribution — happy to help if any conflict is unclear. AI-assisted maintenance note, not a review. It does not count as the required human review under |
Astro-Han
left a comment
There was a problem hiding this comment.
NO-GO at exact head eeec9529ad69dd0be90408f8e35deaaea871181e. One integration P1 is inline. Multiple existing, non-outdated unresolved threads already cover the prompt-parity, admission, parser, retry-classification, and recovered-metering defects on this head; the existing CHANGES_REQUESTED review remains current. The branch is 334 commits behind current main. A real five-commit rebase probe conflicts in packages/eval/src/harbor-external-subject.ts; conflict resolution must retain current main's signed, bounded v2 metering/checkpoint authority and transplant only orthogonal fixes. The old exact-head CI is green, but it predates and does not validate that integration.
| models: [...requestModels].sort(), | ||
| toolNames: [...observedToolNames].sort(), | ||
| }; | ||
| checkpointWrites = checkpointWrites.then(() => writeJsonAtomic(checkpointPath, value)); |
There was a problem hiding this comment.
[P1] Resolve the rebase conflict by deleting this unsigned v1 checkpoint authority. This snapshot is written without a MAC and the PR's recovery path later trusts it from subject-writable /logs/agent, so choosing or blending this side would let the measured process forge usage, cost, and admittedRequests. Current main already has the canonical replacement: signMeteringCheckpoint(..., resultToken), a bounded O_NOFOLLOW regular-file read, v2 settlement/in-flight fields, and shared derivation of completeness/cost. Preserve that entire authority and transplant only the independent prompt/parser/artifact changes; do not revive this parallel schema or its fixture-trusted tests.
There was a problem hiding this comment.
Thanks — one additional correctness reason to preserve the current-main checkpoint authority here: this branch chains checkpointWrites = checkpointWrites.then(() => writeJsonAtomic(...)) without containing a single write rejection. After an ENOSPC, EACCES, or rename failure, every later persistCheckpoint() stays rejected; both proxy.settle() and close() await that poisoned chain, so a provider request that already completed successfully is ultimately reported as infra_failed because an auxiliary recovery snapshot could not be refreshed. Current main deliberately catches each atomic-write failure and keeps the previous truthful lower-bound snapshot. Please preserve that behavior during the rebase as well. If checkpoint persistence is intended to be a hard run gate rather than best-effort recovery evidence, please push back and clarify that invariant.
中文摘要
再补充一个保留 current-main checkpoint authority 的正确性原因：这里一次原子写失败会永久污染 promise chain，随后让本已成功的运行在 settle/close 时误报 infra_failed。current main 明确把单次写失败收敛为保留旧的真实下界快照；重基时也请保留这个不变量。若 checkpoint 写入本来就是硬门禁，也欢迎直接说明。
AI-assisted review disclosure: Codex ran an independent analysis lane; Astro-Han independently verified the exact head, production path, and severity, and owns this review.
Fixes two Eval-side attribution failures found while completing the seven-arm Terminal-Bench matrix.
--prompt=argument and keeps its cwd task-derived--Real-host evidence:
pytorch-model-recoverypreviously failed before model admission across Codex, Claude Code, Reasonix, ZCode, and Pi because its instruction begins with-adaptive-rejection-samplerandgpt2-codegolfactually consumed model output but were misreported as pre-admission because the response envelope was not recognizedValidation:
git diff --check