Skip to content

fix(checkpoint): preserve typed capture failure diagnostics - #7763

Open
songoow wants to merge 1 commit into
mainfrom
codex/checkpoint-capture-diagnostics-20261101
Open

songoow wants to merge 1 commit into
mainfrom
codex/checkpoint-capture-diagnostics-20261101

Conversation

@songoow

@songoow songoow commented Oct 31, 2026

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

Checkpoint capture failures currently lose useful typed diagnostics at two boundaries. An already-expired capture window is raised as a generic error and becomes unexpected_handler_error; the benchmark supervisor then retains only the Python exception class even when the runtime supplied a safe diagnostic code.

This PR gives the expired-window rejection a stable typed code and preserves the runtime's existing error kind/code in the supervisor reply. Operators can distinguish known deadline rejection from other capture failures without receiving raw remote exception text.

中文:保留 checkpoint 失败的类型与诊断码,让已过期的捕获窗口得到明确分类,并保持现有超时、恢复和隐私边界。

Outcome basis: reproduced failures through the real Effect RPC client and the actual supervisor reply path. Intended base: loopx-project/loopx:main at 21367015cce45294dcf99f54a3211d66f5d842c5.

Author Declaration

Written by: model_agent — GPT-6 family, OpenAI; independently reviewed and exercised by a second model agent.

Implemented against

  • The existing Effect runtime typed-error contract, checkpoint artifact owner and benchmark checkpoint documentation at the pinned base above.
  • The known deadline is already enforced by the existing withinDeadline boundary. This patch changes its error classification, not the capture budget.
  • The supervisor already receives typed EffectRuntimeRemoteError instances. This patch projects their existing safe fields; it does not introduce a second error classifier.
  • The edited documentation records the repaired behavior and is not treated as an independent oracle.
Criterion Disposition Owner / decisive evidence
Expired capture window has a stable request-rejection code implemented Existing TypeScript artifact owner; actual RPC regression expects checkpoint_capture_window_closed
Preserve typed runtime failure details in supervisor receipts implemented Existing supervisor error handler emits runtime_failure.error_kind and runtime_failure.diagnostic_code
Keep compatibility and safe error projection implemented Existing top-level error_kind retained; no raw message/body/path/stack added to the reply; untyped spoofed attributes ignored
Keep capture cleanup and failed-result behavior preserved Actual worker/supervisor path exercises unpause, close and reply publication; failed captures publish no checkpoint
Keep retries, authority and timing policy with their existing owners preserved No new retry, deadline extension or authority policy; the same ready digest is not retried

Self-check: reviewed the full five-file diff and both active boundaries. A separate reviewer reran the focused Python integration cases and found no blocking issue. Related #7743 concerns startup recovery; #7754 concerns context read recovery and does not repair these running-checkpoint boundaries.

Changes

  • Raise the existing EffectRuntimeRequestError for an expired capture window, with fixed code checkpoint_capture_window_closed.
  • Add a two-field runtime_failure object only for a genuine EffectRuntimeRemoteError.
  • Retain the existing top-level exception-class field and private error-file behavior.
  • Add focused regression coverage and document the additive reply fields.

Scope And Continuation

Complete within checkpoint error classification and safe diagnostic preservation. This does not establish the root cause of historical benchmark capture failures or claim that those failures are fixed. It does not change HTTP error-body redaction, capture deadlines, retry policy, Docker behavior, benchmark scoring or evaluation performance.

Validation

Published revision: 8c30ce4027b46419eefc323cf9c17df9e4952ca8. Its Git tree exactly matches the locally tested and independently reviewed source tree.
Run state: finished. Input classes: synthetic; real local Effect RPC and supervisor entrypoints are exercised, while Docker operations are controlled test doubles.

Check kind Result Public-safe evidence / limitation
regression_parity passed The same new diagnostic tests on the original 04c5799 baseline fail four decisive cases and pass the untyped privacy control.
unit / integration passed New diagnostics plus the full checkpoint artifact transport suite: 8 passed, including the 8,192-member transport case.
refreshed-base integration passed After rebasing onto 2136701, all five diagnostic tests plus the large-RPC-allowance deadline case passed again: 6 passed, zero skipped. The five changed file hashes are unchanged by the rebase.
independent integration passed Separate reviewer: the same six focused cases passed with zero skipped; actual worker cleanup and supervisor publication were inspected.
TypeScript passed After rebase, node --no-warnings --experimental-sqlite --experimental-strip-types --test --test-isolation=none tests/control_plane_ts/checkpoint_artifacts.test.ts: 18 passed, zero skipped.
typecheck passed npm run -s typecheck:control-plane passed before rebase. The changed checkpoint source is byte-identical after rebase; the refreshed-base runtime tests above passed.
static passed Diff hygiene and bilingual-RFC gate passed; the five-file public-boundary scan completed with ok: true.
registry verification unavailable The encompassing loopx check command exited 1 because its explicitly isolated registry was absent. Its completed public-boundary subscan passed. Missing-runtime and an existing nonliteral credential-reference warning are disclosed; no live registry was read.
semantic advisory bounded One changed supported source was inspected; no supported carrier was reported. This is advisory scope, not full semantic qualification.
environment qualification not_run No live Docker capture, native benchmark/model run or full repository CI was run locally.

The tests retain the generic redaction for unexpected JSON errors, reject spoofed typed-error attributes, preserve source bytes, and check the existing pause/probe/unpause/close/reply sequence. They do not qualify a long-running evaluation.

Frontend / Visual Evidence

UI impact: none. No screen or rendering change. Source data: synthetic. Screenshots and preview: N/A.

Type of Change / LoopX Area

Bug fix, focused regression tests and contract documentation; checkpoint runtime/benchmark diagnostics. No routing, permission, scoring or default activation change. Shared-authority RFC fixture impact: N/A.

Boundary Checklist

  • The diff and PR contain no credentials, raw private trajectories, internal links or host-local evidence paths.
  • No live benchmark or model run was launched.
  • Dedicated codex/ worktree, latest target main verified, explicit staging and DCO sign-off.
  • No paired RFC changed; the bilingual gate passed.
  • UI impact is none.
  • Published Git tree matches the tested tree byte-for-byte.

Signed-off-by: song <22676124+songoow@users.noreply.github.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-31T18:54:30.113969Z 8c30ce4 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@songoow songoow left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Request changes conclusion (author-owned PR; GitHub blocks formal self-review)

Reviewer: model_agent | GPT-6 Astra Pro | OpenAI | self_reported

审阅固定在 8c30ce4027b46419eefc323cf9c17df9e4952ca8,基线 21367015cce45294dcf99f54a3211d66f5d842c5。这是作者所属 PR 的 COMMENT 形式结论,不声称已提交 GitHub 正式 Request changes 事件。

未发现新的具体阻断性代码缺陷;当前保留验证 hold。 最新 Windows 验证中,locator live-lock 诊断应为 mutation_lock_timeout,实际得到 runtime_startup_timeout。补丁没有改该启动路径,但还没有同平台基线证据,不能直接宣布该失败无关。

动机 / Motivation

受影响的是使用可选 checkpoint 捕获的 benchmark 维护者,以及收到捕获失败回复后继续原任务的 worker。 捕获窗口已经过期时,旧运行时把正常拒绝包装成内部错误;即使其他失败已有安全诊断码,supervisor 也只留下 Python 异常类,维护者难以区别过期、文件缺失与内部故障。 本次为过期窗口保留明确的拒绝码,并把已由运行时分类的两个安全字段送回 worker,让失败原因可判断而原工作仍可继续。 范围是已有失败分类与回复投影,不延长捕获或实验预算、不重试同一身份、不把快照升级为研究资格或执行权限。

验收边界来自基线 benchmark/README.md,revision 21367015cce45294dcf99f54a3211d66f5d842c5。文档没有给下面三条现有要求编号,本审查用 capture-window、writer-release、first-attempt 标识它们,以便分别核对;本PR新添的诊断说明不充当自己的独立验收标准。

改动思路 / Approach

运行时继续拥有失败分类,Python supervisor 只投影既有 typed error 的安全字段;释放 writer barrier、deadline 与首次 attempt 所有权仍由原 owner 执行。 当前 PR 的独立边界是从真实 Effect Runtime 失败到 worker 安全回复的诊断链路,正常 capture、预算和原生 Docker 操作语义不扩展。 仅修改TS抛错会继续在Python回复边界丢失code,单改Python又无法修正过期被判内部错误,所以两处生产修改组成最小完整链。相比复制exception全文或分析message,这种复用既有错误类型的方式保留机器语义,也限制跨边界泄露。没有新重试策略、CLI选项、预算或持久权威。

具体改动 / Changes

关键代码讲解

  • withinDeadline(checkpoint_artifacts.ts:407–410)保持原来的时间条件,在过期时改抛 EffectRuntimeRequestError 和固定 checkpoint_capture_window_closed。真实RPC因此保留request_rejected分类,捕获仍然拒绝且不产生目标目录。capture-window 已由future/expired对照与回归证据覆盖。
  • CheckpointSupervisor._run(checkpoint_supervisor.py:120,新增:174–178)只在 isinstance(error, EffectRuntimeRemoteError) 为真时附加两个安全字段,保留顶层异常类。普通provider异常即使有同名属性,也不会被当作可信runtime错误;message、原始provider正文与credentials不复制到worker receipt。host私有错误日志仍是既有0600路径,不把它误称为对所有秘密做了通用净化。
  • capture_worker(checkpoint_worker.py:231,原finally:330–343)在异常传播前恢复原worker;_reply(supervisor:82)再次检查running/unpaused后发布回复。writer-release 和 first-attempt 对应的源文件不变、无失败产物、原attempt仍占用、同handoff只回复一次,都在同一跨边界测试中断言。

验证与反证

复核本次审查的独立harness和完整原始日志:真实Effect Runtime经过生产worker/supervisor,Docker pause、archive和reply IO由受控替身实现。相同5场景在基线为4 failed / 1 passed,在head为5 passed、exit0;四个失败分别精确暴露过期误分类或缺失runtime_failure,普通untyped异常对照在两边都通过。head事件顺序是pause→probe→unpause→client_close→reply_transfer→reply_publish,并检查receipt不含合成token、admin secret、私有路径和provider正文。这证明本次诊断不变量,不代表真实Docker daemon或native provider已经资格验证。

原focus Python运行有71个断言通过、39个skip,session整体exit1是共享审查进程期间出现默认usage-ping目录触发隔离gate;保留该结果,不写成全绿。独立5场景harness隔离usage观察与runtime临时目录后exit0,覆盖最关键改动。Ruff由本次审查执行记录报告通过;本次另复核immutable-base git diff --check 通过,没有放宽断言或预算。

对主干的风险 / Risks

主要风险是把不可信provider异常当成安全诊断、在捕获失败后遗漏unpause,或为同一capture identity再次采集。现有typed owner、finally和seen去重路径均保留;新增测试明确击中这些边界。顶层error_kind兼容读取仍在,expired从内部错误变为请求拒绝是明确披露的修复。

当前 Windows job 为1 failed / 418 passed / 20 skipped。失败是 test_locator_publication_failure_surfaces_safe_typed_startup_diagnostic[live_lock-False-mutation_lock_timeout],观察到startup timeout。这里没有证据证明它由新增checkpoint诊断导致,也还没有相同Windows环境的不可变base同签名对照。最小补证是同条件定位启动/锁时序并验证当前有效诊断或独立归因;不应靠扩大超时、改预期码或跳过测试拿到绿色。pending状态本身不构成代码缺陷。

我的整体评价 / Verdict

改动解决了可复现的诊断丢失,使用既有owner、生产代码很小,188行主要测试增量为跨真实RPC的五种失败路径,体量与其边界风险相称。当前结论仍是REQUEST_CHANGES的证据hold,针对尚未归因的当前Windows失败,不新增没有复现的代码finding,也不要求此PR修理已证明无关的历史问题。本PR没有渲染页面变化;检查的是运行时RPC、worker回复与benchmark说明,视觉验收不适用。 实际Docker资格与模型对新诊断的采纳不在本次已验证结论内。

English verdict: REQUEST_CHANGES — exact head 8c30ce4; the checkpoint diagnostic fix is supported by a regression-sensitive real Effect Runtime harness, while the current Windows startup-diagnostic failure remains unattributed. This author-owned PR uses a COMMENTED review conclusion.

This branch has not been deployed

No deployments
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.

1 participant