Skip to content

fix(eval): make execution boundaries authoritative - #2928

Merged
M4n5ter merged 7 commits into
mainfrom
fix/eval-execution-boundary-correctness
Aug 13, 2026
Merged

fix(eval): make execution boundaries authoritative#2928
M4n5ter merged 7 commits into
mainfrom
fix/eval-execution-boundary-correctness

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Make the shared Harbor/Pier Eval execution boundary authoritative, bounded, and race-safe.

Full benchmark runs exposed coupled infrastructure failures: 15 verifiers were cancelled by Eval's private 20-second completion deadline despite task verifier timeouts of 3600–12000 seconds; 11 completed Maka executions were classified indeterminate when the Runtime Host exceeded its default 15-second settlement budget under 24-group load; and cancellation could write to a closed relay socket, causing EPIPE, runner crashes, or stale writer locks. The frozen Terminal-Bench 2.1 revision also contains task images whose Docker WORKDIR is /app/personal-site, /workspace, or /app/dclm, disproving /app as an executor authority.

This PR fixes those failures at the existing shared Eval seam:

  • normal completion has no Eval-owned deadline; Harbor/task remains the normal agent and verifier timeout authority;
  • cancellation, unused-trial termination, peer loss, and framework timeout use one bounded OS-signal teardown path with structured escalation evidence;
  • the relay discovers cwd from the container environment and no execute request or experiment spec carries a second cwd authority;
  • Harbor and Pier's merged Docker output is treated as an untrusted carrier containing one bounded, digest-checked result frame;
  • raw external stdout, stderr, and ZCode JSONL logs are not persisted as Eval artifacts; relay evidence is limited to categories, byte counts, digests, exit facts, and delivery status;
  • staging credentials and process-group control files are removed idempotently;
  • structured external results are restricted to the canonical bundled wrapper; generic external commands use the exit-code contract;
  • both DeepSeek V4 Flash benchmark specs explicitly freeze hostSettlementTimeoutMs: 120000, forwarded through the existing Eval shim to the Runtime Host client.

Runtime Host remains the sole Maka terminal-result authority. No Runtime implementation, polling, public protocol, framework-specific executor, full-evaluation result, or VM configuration is included.

The framing nonce and PID file are correctness mechanisms for normal merged-stream noise and lifecycle control, not a security boundary against an actively malicious same-UID/root child. That stronger threat model requires a separate sandbox/UID/PID-namespace or trusted-sidecar infrastructure change.

AI contribution disclosure: Codex materially authored the implementation, tests, analysis, commit messages, and this PR description. Claude and K3-256k performed read-only adversarial reviews. A human contributor must review the final diff, provenance, behavior, and submission decision before merge.

Verification

  • npm --workspace @maka/eval run typecheck — passed
  • npm --workspace @maka/eval run build — passed
  • focused Node tests: lifecycle boundaries, external subject, provider admission — 24 passed
  • Python relay contract/lifecycle tests — 22 run: 20 passed, 2 skipped because macOS lacks GNU setsid --wait
  • focused Biome check — passed
  • git diff --check — passed

Not run: full evaluation, paid VM execution, repository-wide tests, full-repository build, or real Harbor/Pier container lifecycle.

Review focus

  • normal completion delegates timeout authority to Harbor/task;
  • host cancellation and Harbor framework timeout remain distinguishable;
  • an execution terminal fact observed before cancellation is not overwritten;
  • Python teardown fits within the Node watchdog and has a bounded destroy fallback;
  • frame noise, invalid UTF-8, duplicate/missing/oversized frames, clean EOF, RST, and EPIPE remain bounded infrastructure evidence;
  • no containerCwd, task-workspace harness scratch, raw process log, credential value, or duplicate execution authority remains on the changed path.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Keep normal Harbor and Pier completion under the framework task timeout while representing cancellation, used-trial abort, and unused-trial termination as explicit bounded phases. Allow relay settlement before TERM/KILL escalation and persist structured cleanup evidence so indeterminate outcomes remain attributable.

Validated with focused fake-clock lifecycle tests for the removed 20-second completion deadline, preparation cancellation, and used-trial abort.

Generated-by: Codex
Use the shared Harbor/Pier relay as the single boundary for task working-directory discovery, subject process settlement, transport delivery, and bounded infrastructure evidence. Remove executor containerCwd authority and workspace capture files, prefer observed terminal execution over simultaneous cancellation, and make peer-close handling idempotent without persisting raw stderr or credentials.

Forward the benchmark-specific 120-second settlement budget through the existing Eval shim to the Runtime Host client without changing Runtime Host authority or polling. Replace remaining provider cwd literals on this path with the relay-confirmed task cwd.

Validated with focused Node lifecycle, external subject, and provider-admission tests; Python relay contract and portable transport/race tests; @maka/eval typecheck; Biome; and git diff checks.

Generated-by: Codex
Adapt the shared Harbor/Pier relay to their merged process stream with an exact-one bounded result frame, while retaining only structured diagnostics and removing raw external process logs. Make OS-signal teardown single-owner and deadline-bounded, preserve Harbor timeout and observed terminal facts, and clean staging/control artifacts idempotently. Restrict structured external results to the canonical bundled wrapper, require the explicit Maka host settlement budget in both benchmark specs, and remove the superseded bounded-log path.

Validated with @maka/eval typecheck/build, 20 focused Node tests, 17 Python relay tests (2 GNU setsid cases skipped on macOS), Biome, and git diff --check.

Generated-by: Codex
Refuse to start a subject when the relay cannot publish its process-group control file. Distinguish setup failure from subject exit, require valid scope evidence before verification, and retain only structured failure evidence.

Validated with focused command, full relay lifecycle, external subject, typecheck, build, and formatting checks.

Generated-by: Codex
@Astro-Han
Astro-Han force-pushed the fix/eval-execution-boundary-correctness branch from d7a404a to 02876b3 Compare August 12, 2026 20:54
@Astro-Han
Astro-Han marked this pull request as ready for review August 12, 2026 21:09
@Astro-Han

Copy link
Copy Markdown
Contributor Author

I exercised this PR’s problem space in a live Terminal-Bench 2.1 run and observed several concrete failures on the pre-#2928 Eval relay implementation:

  1. prove-plus-comm failed because the relay attempted to recover stdout through /tmp/maka-eval-<token>.stdout, but the file no longer existed when Harbor ran docker compose cp. The cell therefore became an infra failure without a verifier result. Retrying reproduced the same failure. fix(eval): make execution boundaries authoritative #2928 appears to address this directly by replacing the container-side temporary stdout file with the authoritative environment.exec() carrier and a structured result frame.

  2. hf-model-inference failed three times with exit code 133 and zero captured stdout. The current implementation can only classify this as empty-output; I cannot distinguish a missing result carrier, an invalid result, an execution-scope failure, or a shim/runtime crash. fix(eval): make execution boundaries authoritative #2928’s structured result frame and diagnostic categories are needed to preserve that distinction. The underlying exit 133 may still require separate Runtime Host diagnosis after the transport ambiguity is removed.

  3. I also observed repeated Runtime Host did not exit cleanly indeterminate outcomes on install-windows-3.11, rstan-to-pystan, and train-fasttext. These cases need fix(eval): make execution boundaries authoritative #2928’s explicit settlement budget, authoritative process scope, and bounded teardown evidence so the execution outcome is not conflated with cleanup failure.

  4. Separately, a peer-closed relay socket emitted an unhandled EPIPE and crashed the entire experiment runner. I applied a focused local fix (a938f18eb, fix(eval): tolerate closed relay sockets) with a regression test. fix(eval): make execution boundaries authoritative #2928’s idempotent peer-close handling appears to supersede this local patch.

I excluded all affected cells from scoring and retried only infra-failed or indeterminate cells. The repeated failures show that retries alone do not make the pre-#2928 execution boundary reliable.

AI assistance disclosure: Codex helped diagnose the failures, prepare the local EPIPE fix and regression test, and draft this report. I verified the referenced attempt artifacts and failure traces.

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English review

The direction of this PR is sound and directly addresses failures observed in real benchmark runs. I found two issues that remain tightly coupled to the PR's stated goals of race-safe cancellation, trustworthy teardown evidence, and reliable bounded result transport.

1. Cancellation and teardown cleanliness are modeled as the same fact

Locations:

  • packages/eval/src/harness-executor.ts:117-202
  • packages/eval/src/harness-executor.ts:783-826
  • packages/eval/src/runner.ts:283-305

When the host aborts while verification is pending, onAbort starts teardown but does not record cancellation as a semantic attempt outcome. If the supervisor exits after TERM, clean becomes true, verification continues, and the attempt may be persisted as infra_failed or even completed rather than indeterminate.

I reproduced the latter case end to end: after AbortSignal fired during verification, the attempt was stored as completed with score: 1 and became non-replaceable.

The same state conflation affects escalation: trialExitEvidence() marks every non-completion exit as clean, including outcome: "killed". A SIGKILL only proves that the Python supervisor exited. It does not prove that Harbor completed environment.stop(delete=true) or removed the container and staged control/credential files.

This conflicts directly with the PR's central contract: host cancellation must remain distinguishable, and teardown evidence must describe what was actually confirmed.

Suggested direction:

  • Model normal completion, cancellation source, and cleanup outcome as separate facts.
  • If host cancellation occurs before verification has reached a confirmed terminal result, return an indeterminate attempt even when resource cleanup succeeds.
  • Do not treat SIGKILL as clean unless a host-owned cleanup path independently confirms resource removal.
  • Add regressions for cancellation during verification and TERM timeout followed by SIGKILL.

2. The bounded stream classifier drops valid large terminal records

Location:

  • packages/eval/src/harbor-external-subject.ts:422-450

classifyStream() clears buffered once an unterminated line exceeds 1 MiB. The remainder of that line is then not valid JSON, so its terminal marker is lost.

This affects a frozen, supported path rather than only malformed output. Pi's JSON-mode agent_end record contains the complete messages[] payload. I reproduced the issue with the PR's built wrapper:

  • provider admission succeeded;
  • the child exited with code 0;
  • it emitted a valid 2,097,220-byte agent_end record;
  • the wrapper returned status: "failed" with pi execution failed.

The previous implementation accepted up to 16 MiB of trajectory output, so this is a regression introduced by the new bounded classifier.

Suggested direction:

  • Incrementally extract only the bounded top-level terminal fields required by each profile while continuing to hash/count the complete stream.
  • If an oversized record cannot be classified safely, report an explicit infrastructure diagnostic instead of silently turning it into a subject failure.
  • Add an oversized agent_end regression test.

Validation

I independently ran:

  • Eval build and typecheck — passed
  • Node Eval tests — 23 passed
  • Python relay tests — 21 passed
  • Biome and git diff --check — passed
  • Focused reproductions for both issues above — reproduced

I intentionally excluded concerns outside this PR's stated threat model and normal benchmark paths.

简体中文审查

这个 PR 的方向是正确的,也确实针对真实 benchmark 中观察到的故障进行了修复。不过,我确认了两个仍然直接影响 PR 核心目标的问题:取消竞态安全、teardown 证据可信,以及有界结果传输的可靠性。

1. 取消语义与资源清理成功被建模成了同一个事实

位置:

  • packages/eval/src/harness-executor.ts:117-202
  • packages/eval/src/harness-executor.ts:783-826
  • packages/eval/src/runner.ts:283-305

当 host 在 verifier 等待期间发出取消时,onAbort 会启动 teardown,但不会把“attempt 已被取消”记录成独立的语义事实。如果 supervisor 在 TERM 后退出,clean 会变成 true,verification 继续执行,最终 attempt 可能被保存为 infra_failed,甚至 completed,而不是 indeterminate

我端到端复现了后一种情况:在 verification 期间触发 AbortSignal 后,attempt 仍被保存为 completedscore: 1,并成为不可替换结果。

同一状态混淆也影响升级路径:trialExitEvidence() 会把所有非 completion 阶段的退出都标记为 clean,包括 outcome: "killed"。SIGKILL 只能证明 Python supervisor 已退出,不能证明 Harbor 已经完成 environment.stop(delete=true),也不能证明容器及暂存的控制文件和凭据文件已经删除。

这直接违反了 PR 的核心契约:host cancellation 必须保持可区分,teardown evidence 必须只描述已经确认的事实。

建议修复方向:

  • 分别建模正常完成、取消来源和 cleanup outcome。
  • 如果 host cancellation 发生时 verification 尚未确认终态,即使资源清理成功,也应返回 indeterminate attempt。
  • 除非 host-owned cleanup 路径能够独立确认资源已删除,否则 SIGKILL 不能被视为 clean。
  • 增加 verification 期间取消,以及 TERM 超时后升级到 SIGKILL 的回归测试。

2. 有界流分类器会丢弃合法的大型终态记录

位置:

  • packages/eval/src/harbor-external-subject.ts:422-450

当一个尚未结束的单行超过 1 MiB 时,classifyStream() 会直接清空 buffered。之后只剩该行的尾部,无法再解析成合法 JSON,终态 marker 因此丢失。

这会影响已经冻结并明确支持的正常路径,而不只是异常输出。Pi JSON mode 的 agent_end 记录包含完整的 messages[]。我使用本 PR 构建出的 wrapper 复现了问题:

  • provider admission 成功;
  • child exit code 为 0;
  • 输出合法的 2,097,220-byte agent_end
  • wrapper 最终返回 status: "failed"pi execution failed

旧实现允许最高 16 MiB trajectory,因此这是新有界分类器引入的回归。

建议修复方向:

  • 在继续对完整流进行 hash 和 byte count 的同时,增量提取各 profile 所需的、有界的顶层终态字段。
  • 如果 oversized record 无法安全分类,应报告明确的 infrastructure diagnostic,而不能静默转成 subject failure。
  • 增加 oversized agent_end 回归测试。

验证

我独立执行了:

  • Eval build 和 typecheck:通过
  • Node Eval tests:23 项通过
  • Python relay tests:21 项通过
  • Biome 和 git diff --check:通过
  • 上述两个问题的针对性复现:均已复现

我有意排除了不属于本 PR 威胁模型以及正常 benchmark 路径的问题。

AI disclosure / AI 披露: This review was prepared with assistance from OpenAI Codex, manually reviewed by me, and I take responsibility for its conclusions. / 本次审查由 OpenAI Codex 辅助完成,已经我人工复核,其结论由我负责。

Track host cancellation, verified terminal state, and finalization evidence as independent facts. Attempts cancelled before verification settles now remain replaceable even when TERM cleanup succeeds, while SIGKILL records only supervisor termination rather than confirmed Harbor cleanup.

Validated with focused verifier-cancellation and TERM-to-KILL lifecycle tests.

Generated-by: Codex
Restore the frozen 16 MiB JSONL record contract without persisting raw output. Valid large Pi terminal records remain classifiable, while records beyond the bound continue to be hashed and counted and produce explicit infrastructure evidence instead of a subject failure.

Validated with 2 MiB Pi settlement and 17 MiB oversized-record wrapper tests.

Generated-by: Codex
@Astro-Han

Astro-Han commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Addressed both findings in 53ea274a5 and d9afb798a.

  1. Cancellation semantics are now independent from teardown confirmation.

runHarnessAttempt no longer collapses normal completion, host cancellation, and cleanup into a single clean flag. It records whether host cancellation was observed, whether verification was confirmed before that cancellation, and the finalization evidence independently.

  • Cancellation before a confirmed verification terminal now always produces a replaceable indeterminate attempt with score: null, even when TERM cleanup completes normally.
  • A normal code-0 supervisor exit after TERM is recorded as confirmed.
  • A signal termination, SIGKILL, or detached supervisor is not treated as confirmed Harbor cleanup.
  • Added end-to-end regressions for cancellation while verification is pending and TERM timeout followed by SIGKILL.
  1. Large external terminal records remain bounded without losing supported Pi results.

The wrapper now preserves the previous 16 MiB per-record contract while continuing to hash and count the complete stream without persisting raw stdout or stderr.

  • A 2 MiB Pi agent_end record is classified as completed.
  • A record beyond 16 MiB becomes an explicit infra_failed result with record-too-large, the configured limit, observed byte count, and SHA-256 evidence.
  • Oversized records are no longer silently converted into provider subject failures.

I did not introduce a general incremental JSON parser because the existing frozen contract already defines a bounded 16 MiB record. Keeping that explicit bound is smaller and avoids a second provider-specific parsing path.

Focused validation:

  • Eval build and typecheck passed.
  • 24 focused Node tests passed.
  • 10 Python relay contract tests passed.
  • Focused Biome lint/format and git diff --check passed.

Could you please re-review the latest head, d9afb798a?

AI assistance disclosure: Codex materially authored the fixes, tests, commit messages, and this response. I reviewed the resulting diff and focused validation.

简体中文对照

已在 53ea274a5d9afb798a 中处理两项审查意见。

  1. 取消语义现在与 teardown 是否确认完成相互独立。

runHarnessAttempt 不再将正常完成、host cancellation 和 cleanup 压缩成单一的 clean 标志。现在会分别记录是否观察到 host cancellation、verification 是否在 cancellation 之前得到确认,以及最终的 finalization evidence。

  • 如果 cancellation 发生在 verification terminal 得到确认之前,即使 TERM cleanup 正常完成,attempt 也始终为可替换的 indeterminate,且 score: null
  • TERM 后 supervisor 以 code 0 正常退出时,记录为 confirmed
  • signal termination、SIGKILL 或 detached supervisor 均不会被视为已确认 Harbor cleanup。
  • 新增了 verification 等待期间 cancellation,以及 TERM 超时后升级到 SIGKILL 的端到端回归测试。
  1. 外部 subject 的大型 terminal record 现在保持有界,同时不会丢失受支持的 Pi 结果。

Wrapper 恢复了此前每条 record 最高 16 MiB 的合同,同时继续对完整 stream 计算 hash 和 byte count,并且不持久化 raw stdout 或 stderr。

  • 2 MiB 的 Pi agent_end record 会被正确分类为 completed。
  • 超过 16 MiB 的 record 会得到明确的 infra_failed,并保留 record-too-large、配置上限、观察到的 byte count 和 SHA-256 evidence。
  • oversized record 不再被静默转换成 provider subject failure。

我没有引入通用增量 JSON parser,因为当前冻结合同已经定义了有界的 16 MiB record。保留这一明确上限更简单,也避免建立第二套 provider-specific parsing path。

聚焦验证:

  • Eval build 和 typecheck 通过。
  • 24 项聚焦 Node 测试通过。
  • 10 项 Python relay contract 测试通过。
  • 聚焦 Biome lint/format 和 git diff --check 通过。

请重新审查最新 head d9afb798a

AI 协助披露:Codex 实质性参与了修复、测试、commit message 和本回复的编写。我已复核最终 diff 和聚焦验证结果。

@Astro-Han
Astro-Han requested a review from M4n5ter August 13, 2026 06:41

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English follow-up review

Thanks for addressing the original findings. The large terminal-record issue is resolved, and cancellation during verification now correctly produces an indeterminate attempt. One production lifecycle contract gap remains.

Production teardown cannot currently produce the new confirmed outcome

Locations:

  • packages/eval/src/harness-executor.ts:827-857
  • packages/eval/harbor/run_trial.py:141-165
  • packages/eval/src/runner.ts:319-325

waitForTrial() now treats a supervisor exit after TERM as confirmed only when it exits with code 0. However, the production supervisor does not currently have that exit behavior.

On SIGTERM, run_trial.py cancels its main task. Harbor's Trial.run() performs _finalize() in finally and then re-raises CancelledError. Consequently, even when teardown completes successfully, the uncaught cancellation causes the Python supervisor to exit with code 1.

I reproduced this through the real run_trial.main() signal path with a run_trial() stub that records completion from its finally block:

teardown-finally-ran
cleaned=True
exit_code=1

This affects a normal supported path:

subject returns infra_failed
→ runner skips verification
→ executor sends TERM during finalization
→ Harbor completes teardown, but the supervisor exits 1
→ cleanup is classified as terminated rather than confirmed
→ the original infra_failed result becomes indeterminate

The new lifecycle tests do not cover this production behavior because their fake Node supervisors explicitly call process.exit(0) from the SIGTERM handler.

This is directly within the PR's goal of making teardown evidence authoritative, so I think it should be resolved here rather than deferred.

Suggested direction:

  • Give host-requested teardown an explicit success outcome after run_trial() has unwound and finalized successfully.
  • Preserve a nonzero outcome when teardown itself fails; do not generally treat every TERM exit 1 as clean.
  • Exercise the real run_trial.py TERM path in a regression test.
  • Verify that a subject infra_failed result remains infra_failed after confirmed cleanup.

A secondary attribution issue remains: cancellation during verification now correctly becomes indeterminate, but runner.ts describes it as executor cleanup did not settle even when the artifact reports outcome: "confirmed". This wording does not need to be independently merge-blocking, but it would be clearer for the executor outcome to distinguish host cancellation from unconfirmed cleanup.

简体中文复审

感谢处理上一轮审查意见。大型终态记录问题已经解决,verification 期间发生 cancellation 也会正确产生 indeterminate attempt。目前还剩一个生产生命周期契约缺口。

生产 teardown 当前无法产生新增逻辑要求的 confirmed outcome

位置:

  • packages/eval/src/harness-executor.ts:827-857
  • packages/eval/harbor/run_trial.py:141-165
  • packages/eval/src/runner.ts:319-325

waitForTrial() 现在只有在 supervisor 收到 TERM 后以 code 0 退出时,才会确认 cleanup。不过,生产 supervisor 当前不具备这一退出语义。

收到 SIGTERM 后,run_trial.py 会取消 main task。Harbor 的 Trial.run() 会在 finally 中执行 _finalize(),然后重新抛出 CancelledError。因此,即使 teardown 已经成功完成,未捕获的 cancellation 仍会令 Python supervisor 以 code 1 退出。

我通过真实的 run_trial.main() 信号路径进行了复现,并让 run_trial() stub 在 finally 中记录 cleanup 已完成:

teardown-finally-ran
cleaned=True
exit_code=1

这会影响正常且受支持的路径:

subject 返回 infra_failed
→ runner 跳过 verification
→ executor 在 finalization 阶段发送 TERM
→ Harbor 完成 teardown,但 supervisor exit 1
→ cleanup 被分类为 terminated,而不是 confirmed
→ 原本确定的 infra_failed 被降为 indeterminate

新增 lifecycle tests 没有覆盖这一生产行为,因为其中的 fake Node supervisor 会在 SIGTERM handler 中显式调用 process.exit(0)

这个问题直接属于本 PR “让 teardown evidence authoritative”的目标,因此我认为应在当前 PR 中解决,而不是留作 follow-up。

建议修复方向:

  • 当 host-requested teardown 中的 run_trial() 已经成功完成 unwind 和 finalization 后,提供明确的成功 outcome。
  • teardown 本身失败时仍应保持非零 outcome,不能笼统地把所有 TERM exit 1 当成 clean。
  • 使用真实的 run_trial.py TERM 路径增加回归测试。
  • 验证 subject infra_failed 在 cleanup 得到确认后仍保持 infra_failed

另外还存在一个次要的归因问题:verification 期间 cancellation 现在会正确成为 indeterminate,但即使 artifact 显示 outcome: "confirmed"runner.ts 仍将原因描述为 executor cleanup did not settle。这个措辞问题本身不必单独阻塞合并,但如果 executor outcome 能区分 host cancellation 与 cleanup 未确认,语义会更准确。

AI disclosure / AI 披露: This follow-up review was prepared with assistance from OpenAI Codex, manually reviewed by me, and I take responsibility for its conclusions. / 本次复审由 OpenAI Codex 辅助完成,已经我人工复核,其结论由我负责。

Translate only host-requested cancellation after Trial unwind into a zero supervisor exit so the executor can confirm successful teardown. Cleanup exceptions and other cancellation sources remain nonzero.

Carry host cancellation and unconfirmed cleanup as explicit executor outcome causes so persisted attempts describe why they are indeterminate without inferring control state from artifacts.

Validated with real run_trial SIGTERM subprocess tests and focused Eval lifecycle coverage.

Generated-by: Codex
@Astro-Han

Copy link
Copy Markdown
Contributor Author

Addressed the remaining lifecycle contract gap in 442bf82ff.

  1. The production supervisor now produces the cleanup outcome expected by the executor.

run_trial.main() translates a CancelledError into a normal process exit only when host teardown was explicitly requested and run_trial() has already completed exception unwinding and finalization.

  • Host-requested teardown with successful Trial unwind/finalization now exits 0, allowing Node to record outcome: "confirmed".
  • A cleanup exception still propagates and produces a nonzero supervisor exit.
  • A CancelledError without a host teardown request still propagates; cancellation is not generally treated as success.
  • Added subprocess regressions that exercise the real run_trial.main() SIGTERM handler and OS process exit code for both successful and failed cleanup.

This closes the producer/consumer contract without treating arbitrary TERM exit 1 as confirmed.

  1. Indeterminate attribution is now explicit.

ExecutorAttemptOutcome now distinguishes:

  • host-cancelled
  • cleanup-unconfirmed

The runner maps those causes directly instead of inferring control state from cleanup artifacts:

  • cancellation before verification completes → executor cancelled before verification completed;
  • SIGKILL, signal termination, or detached cleanup → executor cleanup did not settle.

A subject infra_failed result remains infra_failed after confirmed cleanup, while unconfirmed cleanup remains replaceable indeterminate.

Validation:

  • Eval build and typecheck passed.
  • 24 focused Node tests passed.
  • Python relay tests: 22 run, 20 passed, 2 skipped because macOS lacks GNU setsid --wait.
  • Focused Biome lint/format and git diff --check passed.
  • All PR checks passed, including Windows recovery.

Could you please re-review the latest head, 442bf82ff?

AI assistance disclosure: Codex materially authored the fix, tests, commit message, and this response. I reviewed the resulting diff and focused validation.

简体中文对照

已在 442bf82ff 中处理剩余的生命周期合同缺口。

  1. 生产 supervisor 现在能够产生 executor 所要求的 cleanup outcome。

run_trial.main() 只有在 host 明确请求 teardown,并且 run_trial() 已经完成异常展开和 finalization 后,才会将 CancelledError 转换为正常进程退出。

  • host-requested teardown 在 Trial unwind/finalization 成功后以 code 0 退出,使 Node 能够记录 outcome: "confirmed"
  • cleanup exception 仍会继续传播,并产生非零 supervisor exit。
  • 如果没有 host teardown request,CancelledError 仍会继续传播;系统不会笼统地把 cancellation 当作成功。
  • 新增 subprocess 回归测试,使用真实的 run_trial.main() SIGTERM handler 和 OS process exit code,分别覆盖 cleanup 成功与失败。

这使 producer/consumer 合同闭合,同时不会将任意 TERM exit 1 误判为 confirmed。

  1. indeterminate 的归因现在是显式的。

ExecutorAttemptOutcome 现在区分:

  • host-cancelled
  • cleanup-unconfirmed

runner 会直接映射这些原因,不再从 cleanup artifact 反向推断控制状态:

  • verification 完成前发生 cancellation → executor cancelled before verification completed
  • SIGKILL、signal termination 或 detached cleanup → executor cleanup did not settle

subject 的 infra_failed 在 confirmed cleanup 后仍保持 infra_failed;cleanup 未确认时则保持可替换的 indeterminate

验证:

  • Eval build 和 typecheck 通过。
  • 24 项聚焦 Node 测试通过。
  • Python relay 测试共运行 22 项:20 项通过,2 项因 macOS 缺少 GNU setsid --wait 而跳过。
  • 聚焦 Biome lint/format 和 git diff --check 通过。
  • PR 全部 checks 通过,包括 Windows recovery。

请重新审查最新 head 442bf82ff

AI 协助披露:Codex 实质性参与了修复、测试、commit message 和本回复的编写。我已复核最终 diff 和聚焦验证结果。

@Astro-Han
Astro-Han requested a review from M4n5ter August 13, 2026 08:38

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the latest head. The teardown confirmation contract and cancellation attribution are now correctly closed, and the focused reproductions, Eval test suite, typecheck, and CI all pass. Approving.

@M4n5ter
M4n5ter merged commit b8f1998 into main Aug 13, 2026
10 checks passed
@M4n5ter
M4n5ter deleted the fix/eval-execution-boundary-correctness branch August 13, 2026 08:46
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.

2 participants