Skip to content

fix(runtime): reject stale batched source fingerprints - #5367

Merged
huangruiteng merged 2 commits into
mainfrom
codex/fix-runtime-fingerprint-snapshot-race
Oct 1, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/fix-runtime-fingerprint-snapshot-race

Conversation

@Duang777

Copy link
Copy Markdown
Collaborator

Problem

The bounded reader added in #5323 can prefetch a later runtime source before an earlier worker deletes it. Every read then succeeds, so _runtime_fingerprint() returns a digest that includes a source file which is already absent. This caused the runtime fingerprint and readiness tests to fail intermittently on unrelated PRs such as #5338.

Fix

  • Re-scan the exact TS/JSON metadata snapshot after each uncached read batch.
  • Do not cache a digest when the post-read snapshot differs. Reuse the observed snapshot for one bounded retry.
  • Preserve the existing two-attempt packaged_runtime_source_unstable failure contract.
  • Make deletion-after-prefetch and persistent churn tests deterministic with thread events.

Verification

  • Red: the deletion-after-prefetch regression failed on the unfixed implementation because only later.ts and first.ts were read and no retry occurred.
  • Green: 39 focused runtime fingerprint, readiness, and shared file-reader tests passed.
  • Ruff passed for both changed files.
  • Repository mypy passed for all 19 strict roots.
  • Semantic vocabulary and CLI output smokes passed.
  • Standard diff-driven premerge passed 10 selected checks with no failures or holds.
  • Same-path microbenchmark: cached control flow stayed equivalent at about 5.6 ms. An uncached validated batch measured about 24.0 ms versus 19.0 ms without validation, preserving bounded concurrent reads while paying one metadata verification scan.

The staged diff received a pre-commit bits-code-guard review with no P0-P2 findings. This PR changes control-plane behavior and must be merged by a maintainer.

Concurrent prefetch could read a later source before an earlier read removed it, so the batch returned a fingerprint for a topology that no longer existed. Revalidate the snapshot after each uncached batch and retry once, while deterministic event-based tests cover one-shot and persistent churn.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Copy link
Copy Markdown
Collaborator Author

Exact-head CI attribution for f5995f102:

  • All four Python test shards passed, including the runtime fingerprint churn regressions.
  • The required pytest aggregate failed only while combining coverage: a shard artifact referenced /tmp/pytest-of-runner/.../test_cli_inspects_disjoint_ind0/loopx/a.py, which no longer exists on the aggregation runner.
  • That temporary synthetic checkout comes from the mainline semantic probe tests added by feat(semantics): prompt vocabulary decisions from the current diff #5317, not this PR. Shard 1 and shard 2 both recorded temporary loopx/a.py paths.

The isolated baseline repair is #5375. It prevents the synthetic CLI subprocess from inheriting pytest/coverage startup variables without changing production code or lowering the coverage threshold. This branch remains unchanged pending that baseline fix; no merge action was taken.

@Duang777

Copy link
Copy Markdown
Collaborator Author

CI update: both original runtime-readiness failures are fixed; test shards 1 and 4 pass on exact head f5995f102802a3261d559989ae27f2aeef4925f0.

The final pytest aggregation exposed a separate main-baseline regression introduced by #5317: its synthetic-checkout CLI test inherited pytest-cov state and published a temporary loopx/a.py path that does not exist on the aggregate runner. I reproduced the artifact path locally and opened the isolated test-only fix in #5376. No #5367 code change is needed for this failure.

@Duang777

Copy link
Copy Markdown
Collaborator Author

Dependency correction: #5376 was a duplicate of #5375 and has been closed. The canonical coverage-artifact isolation fix is #5375 at 086f2643355d36b62ccf74cbdac6200ca31a498f. This does not change the runtime fix or its exact-head shard evidence; no #5367 code change was made.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

动机

合入 #5323 的并行读取后,后面的文件可能已经预读成功,随后被另一个读取任务删除或修改。旧代码仍能返回一个长度正确、但不再对应当前源码集合的指纹。这里要修的是运行时身份的真实正确性,不是增加测试数量,也不是宣称关闭整个运行时可靠性或性能路线图。

改动思路

在现有 Python 文件系统身份适配器内,对未命中缓存的整批读取做一次读取后快照核验。变化时用已观察到的新快照重试一次,持续变化则沿用既有 source-unstable 错误。顺序读取仍会遇到读后变化,取消缓存或新增全局锁/状态 owner 又更宽;当前方案复用已有 owner、类型和两次尝试预算,不另建 TypeScript 决策的 Python 副本。

具体改动

两文件,68 行增加、26 行删除:生产代码 +24/-11,现有运行时测试 +44/-15。没有新增 CLI、schema、持久状态、权限、依赖或安装行为。

_RuntimeSourceChanged(effect_runtime.py:61)

私有异常携带既有类型化元数据快照,不是新公共协议。它让不稳定批次退出被缓存函数,而不是把不合格摘要当成功结果写入 LRU。

_runtime_fingerprint_for_snapshot(:283)

仍按排序后的相对文件名和原始字节计算 SHA256;并行完成顺序不改变摘要顺序。新分支在读取结束后重新观察完整元数据集合,只有与输入快照相同才返回。稳定缓存命中不新增字节读取;冷批次多一次元数据扫描。

_runtime_fingerprint(:305)与真实调用方

单一两次尝试循环取代嵌套的 FileNotFound 重试。快照变化携带新观察值;文件消失则重新扫描;耗尽后仍是 packaged_runtime_source_unstable。readiness 与 effect_runtime_request 共用这个 owner,错误必须发生在 locator/进程创建之前。

现有测试改用 Event 保证实际预读发生后再变动文件,覆盖一次恢复和持续变化;我没有只接受“摘要长64字符”的断言。

对主干的风险

相同固定输入在不可变 base 67930ab 和 head f5995f1 上执行:通过真实文件、线程池、原始字节读取和公共 readiness/request 边界,独立计算当前文件集合的完整 SHA256。删除、修改、新增,以及两次持续变化在 base 上出现五个预期 oracle 失败,在 head 上均正确;稳定重复调用仍复用字节缓存。真实 Node 24.21.0 的 deep readiness、ping 与 settlement identity 在两端通过。临时 source/locator 是输入隔离和观察,不提供摘要、ready 或成功后置条件;未修改任何活动 Goal 来测试。

本地源码验证:运行时五组测试 113 passed,Mypy 的19个配置内源文件、改动文件 Ruff、DCO 与 diff hygiene 通过。语义 coinage advisory 没有发现受支持的新词汇载体;它不是语义等价证明。首次选错测试路径及旧参数的命令在测试执行前失败,随后用实际存在的文件和受支持参数重跑,没有将这些调用错误归责 PR。另外按不可变 base 到精确 head 的 diff 执行 premerge:diff/compile/公共私有边界直接检查、10 项 catalog canary 和8项风险 profile smoke 均通过,0失败、0manual hold。该结果不授予合并权限。

语义、覆盖及恢复限制

这是已有默认路径的缺陷修复,不声称 default-off:稳定调用保持,变化批次现在被核验/拒绝。复用已有 source-unstable 与 readiness 契约,无新状态分类、权限或调度义务;错误措辞仍属于通用运行时。前端、Lark、配置编辑器和 CLI 入口均未改,现有 runtime 调用直接采用同一适配器,因此没有新增配置 companion。

元数据核验不是原子文件系统冻结,最终观察之后的变化和不可检测的 metadata ABA 不在保证内。现有 package-invalid 推荐仍是泛化的 Node 修复提示,不应把它理解成已新增源码专用恢复提示。未运行原生 Windows/Linux 主机、全仓测试或持续延迟资格验证;未查询/等待远端 CI,也不继承作者 CI 结论。

我的整体评价

APPROVE,适用于 f5995f1。没有发现当前 head 的阻断问题。长期维度改善:不稳定摘要不会作为成功缓存,重试有界,源码稳定后的下一次真实请求可恢复;用户不必新增配置或手动启动 daemon。未来演进检查已体现在单一 retry owner 与既有快照类型复用中,不需扩大为锁服务/新框架。保留上述原子性、平台和性能边界;生产运行时变更的合并由维护者决定,本批不合并。

English verdict: APPROVE - f5995f1: the prefetched stale-source defect fails the independent base oracle and passes at head; 113 native runtime tests and real Node readiness passed, with metadata/OS/performance limits disclosed.

Signed-off-by: duanjialing.777 <duanjialing.777@bytedance.com>
@Duang777

Duang777 commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

Synced with main@f49b4a00870604d39fa4318da24d6dd35e72bb6e using signed merge commit 58972157b805f95a2175a85c2c4431deb699357d. The merge was conflict-free, and the PR diff remains limited to loopx/control_plane/effect_runtime.py and tests/control_plane/test_turn_journal_runtime_readiness.py.

Focused validation on the new exact head: tests/control_plane/test_turn_journal_runtime_readiness.py passed all 17 tests. The branch was pushed normally without history rewriting. The existing untracked uv.lock was not modified or staged. CI has restarted; no merge action was taken.

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