Skip to content

Fix file mutation lock cleanup - #8409

Merged
huangruiteng merged 2 commits into
mainfrom
codex/delegation-lock-release-20261109
Nov 9, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/delegation-lock-release-20261109

Conversation

@loopx-agent

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

Delegation history repeatedly releases shared file locks. The Python release path re-read claims whose file identity it already held, and a claim containing invalid UTF-8 could interrupt cleanup and leave the file behind.

Cleanup now uses the captured identity for an owned claim. Cleanup by pathname still checks the PID and token. Marker release still checks the token and file identity before retirement, and transient failures removing a claim or a retired marker still get a retry.

Intended base: main.

Author Declaration

  • Written by: model_agent, OpenAI GPT-6.
  • Implemented against: the existing cross-runtime marker and claim behavior in loopx/file_lock.py and loopx/control_plane/effect_runtime_io.ts at 71190b6b4ef4e146f3bf4a10d31ce61ac42fd021. There is no new specification.
Criterion Disposition Symbol / path Validation
Only the caller's captured file may be removed during claim cleanup implemented _release_effect_mutation_claim Invalid UTF-8 cleanup and replacement with the same token
Preserve token and file identity checks on marker release implemented _release_effect_mutation_lock Replacement token, current owner and stale owner recovery tests
Preserve cleanup after a transient unlink failure implemented _release_effect_mutation_lock Independent fault injection for claim and retired marker removal
  • Self-check before submission: reviewed claim ownership and both cleanup retries, and exercised the affected HTTP read.

Scope And Continuation

The patch changes Python file cleanup and its regression tests. It removes half the owner-record reads in a matched inventory read. End-to-end timings varied, so production latency remains a separate verification step. No new lock protocol, storage format, setting, or user action is needed.

Validation

  • Tested revision: 817d6ec053106936169124087c665c8dbaf35212.
  • Run state: finished for the focused checks.
  • Input classes: synthetic, authorized_private_read_only.
Check kind Result Public-safe evidence
static passed Ruff and the repository-configured mypy targets.
unit passed tests/test_file_lock.py: 30 passed; 2 platform skips on macOS.
real_entrypoint passed Production inventory HTTP handler in isolation; File and SQLite authority fixtures preserve the full result, provider rejection and retry.
regression_parity passed Invalid UTF-8 fails on the base and passes on the head. A replacement claim and temporary cleanup failure stay safe. Matched isolated HTTP reads preserved the response, authorization rejection and recovery.
static passed The standard local premerge check completed with a valid exact-scope receipt and no manual holds.
unit blocked The supplemental full-repository pytest run was stopped early. The focused lock and File/SQLite integration runs completed.

The results apply to the proposed commit and the isolated HTTP read. They do not establish production latency or installed behavior.

Frontend / Visual Evidence

  • UI impact: none.
  • First viewport and preview: N/A. The HTTP read and its response contract are unchanged.

Type of Change

  • Bug fix
  • Test update

LoopX Area

  • Control plane (runtime)

Boundary Checklist

  • The diff and description contain public-safe evidence.
  • Scope is limited to lock release and its regression coverage.
  • No paired RFC changed.
  • UI impact is marked none.
  • The commit includes a DCO sign-off.

Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>

@loopx-agent loopx-agent 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.

Reviewer: model_agent · gpt-6.1-sol · OpenAI · runtime_reported · max

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

动机 / Motivation

用户在 LoopX 打开「委派 → 全部记录」时,服务会逐批读取历史并释放用于保护这些读取的锁。

每次释放会建立一个临时声明,以保证同一时刻只有一个释放者。原来,释放者清理自己创建的声明时会重读文件;一旦声明内容损坏成无效 UTF-8,清理会中断并留下文件。改动后,释放者按创建时捕获的文件身份清理自己的声明;如果同一路径已换成另一个文件,则保留它。

在同源的八对隔离 HTTP 读回里,所有者记录读取每次从 1,696 次减为 848 次;历史结果一致,撤销权限和非法分页仍被拒绝,恢复后也能读回同一结果。

此 PR 交付声明清理和减少重复 I/O,未改变委派权限、历史排序和分页,也未改变页面呈现。 八对里五对总耗时缩短,但两组各自的中位耗时没有下降,仍需在正式入口重复验证长尾和端到端耗时。

改动思路 / Approach

锁的排他、所有权复核和跨运行时持久化仍是必要约束;用现有文件身份清理本进程声明,能移除额外解析且不用另设一套锁。 本次只改 Python 释放适配和回归测试;剩余耗时由原委派读取路径负责,正式验收须同时比较同源结果、拒绝和恢复。

核对 Python 旧实现,发现声明内容正常和内容无法识别的分支最终都调用相同的文件身份校验;读 JSON 没有决定是否删除。只捕获解码异常可以补上一个坏文件场景,却不会去掉普通路径的冗余 I/O。现有原生 TypeScript 释放端继续使用相同的磁盘契约,不需要一起改。

具体改动 / Changes

核对的提交是 817d6ec053106936169124087c665c8dbaf35212,基线是 71190b6b4ef4e146f3bf4a10d31ce61ac42fd021。远端 diff 与本地审查 diff 的稳定 patch ID 一致。

关键代码讲解

  • _release_effect_mutation_claim:拿到本进程独占创建时的文件身份,便直接复用受保护的删除函数;只有拿路径找回声明时,才读进程 ID 与 token。即使替换文件复制了相同的 token 和内容,其文件身份也不同,不能删除。
  • _release_effect_mutation_lock:拿到独占声明后仍重新检查原标记的 token 和身份,匹配才改名退出活跃路径。第一次删除已退役标记成功才跳过重复删除;如果遇到短暂 I/O 失败,原回退仍会再试一次。清理临时声明的外层重试也保留。
  • test_cross_runtime_claim_cleanup_removes_own_corrupted_claim:原先只测可解码但非法的 JSON。补上不可解码字节后基线确实失败,当前提交通过。同文件另有同内容替换和两类临时删除失败测试;去掉外层声明重试的中间版本会留下声明并触发失败。

对主干的风险 / Risks

最需防的是清理时误删接任者的声明,或删除失败后留下一把实际上已完成的锁。现有 token 与身份复核、独占声明和原子改名均保留。测试覆盖同内容替换、坏字节、活跃/过期持有者、错误 token,以及两种临时文件删除各失败一次。持续 I/O 故障及身份检查后遭外部程序改写仍由既有恢复边界处理,此改动不扩张锁模型。

本地锁测试 30 项通过、2 项平台跳过;File/SQLite 原委派 HTTP 测试 2 项通过,静态检查和本地预合并通过。全仓补充测试提前停止,不能当作全量通过。隔离配对只支持减少读取次数;生产长尾未验证。此评审按当前规则使用本地验证,未查看远端 CI。

我的整体评价 / Verdict

改动和证据足以支持这个范围内的清理修复:坏文件可恢复,替换文件与原重试仍受保护,原委派读回未变。后续再单独验证正式入口是否提速。此为作者复核,独立评审与合并资格仍待确认。

English verdict: APPROVE this bounded lock cleanup at the reviewed commit; production latency and independent approval remain open.

Signed-off-by: huangruiteng <huangrt01@163.com>

@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.

Reviewer: model_agent, GPT-5, OpenAI, self_reported

Independent approval

动机 / Motivation

通过委派历史等 HTTP/CLI 读取路径进入跨运行时 mutation lock 的本地进程,以及依赖这些读取正常清理锁文件的运维者。旧路径在释放自己创建的 claim 时重新读取并解析 claim JSON;如果 claim 被损坏成 invalid UTF-8,解析异常会中断 cleanup,留下锁协调文件。普通 cleanup 还多了一次不需要的 owner 读取。base 上创建 claim 后写入 b'\xff' 并释放,release 抛 UnicodeDecodeError 且 claim 仍存在;head 上同一场景 claim 被删除。若路径被替换为同内容的新 inode,head 保持 replacement 不动。

head 的锁和 cross-process 测试 33 项通过、2 项平台跳过;delegation/lock 路径 69 项通过;base/head 手工对照确认坏 claim 从残留变为清理,且 replacement inode 未被误删。

本 PR 不证明生产 HTTP 长尾 latency 或所有 Windows 文件系统的 identity 行为;这些需要正式环境的补充测量。

改动思路 / Approach

入口仍是 Python 的 cross-runtime mutation lock release,权威状态是 kernel lock、token-bound claim path、创建时捕获的文件 identity、owner record 和 retired marker。_release_effect_mutation_claim 在 owned-claim 分支只使用创建时捕获的 inode identity,因此即使内容不可解析也能清理本进程文件;pathname/token recovery 仍读取 owner 并核对 PID/token。资格是无持久状态的可复用投影之外的内部 cleanup 规则:本进程独占创建 claim 才获得 owned branch,文件 identity 不匹配就保留 replacement。

使用已捕获的 inode identity 清理本进程 claim,保留 pathname/token 分支和 marker retry,是最小且与现有锁模型一致的变更。本次只修复本地 lock cleanup 与回归覆盖,不宣称生产 latency 或跨文件系统完成度。_release_effect_mutation_lock 保留 owner token、lock identity、retired marker 和 claim 的 best-effort retry;没有新增锁类型、持久状态、CLI 或权限模型。更小的只捕获 UnicodeDecodeError 方案会让普通路径继续做无意义 I/O,也没有使用已经存在的 identity,因此不采用。

场景 旧行为 当前 head
本进程 claim 内容为 invalid UTF-8 解析抛错,claim 残留 按 captured identity 删除自己的 claim
同一路径替换成同内容新 inode 不能删除 replacement identity 不同,replacement 保留
替换 token 或 owner record 不能删除其他 owner 的 lock pathname/token 分支继续核对 owner
claim 或 retired marker 首次 unlink 瞬态失败 可能留下 marker 内层和外层 cleanup 各提供一次安全重试

具体改动 / Changes

本次核对 e7e9857ac7ba2abb7ccbd6510b096413828a3a4d 相对 9837572bdab00ead43940a23e219d9b32cd6ef3f 的 2 个文件,+64/−38。作者声明没有新的 lock protocol;现有 file-lock acquisition 的 kernel lock 与 holder recovery 是相关语义,但临时 claim cleanup 本身没有独立书面规范,因此本 review 使用已复现的 cleanup defect 和既有 identity invariant 作为验收基准。

关键代码讲解

  • _claim_effect_mutation_lock 在发布 claim 内容前从未关闭的 descriptor 捕获 (dev, ino, birthtime, ctime);这条 identity 是后续只删除本进程文件的依据,写入/fsync 失败仍会在异常路径清理。
  • _release_effect_mutation_claim 对 _EffectMutationClaim 不再解析 JSON,而是直接让 _remove_created_effect_file 比较 identity;传入普通 Path 时才读取 owner 并要求 token 匹配。
  • _release_effect_mutation_lock 保留原 lock-file owner/token/identity 三重检查,并将 retired marker 的首次清理结果记录为 retired_cleaned;失败时在 finally 中再尝试一次,claim 也在外层 finally 做最后一轮 best-effort 清理。

测试覆盖本进程坏 claim、替换 inode、替换 token 以及 claim/retired marker 的一次性 unlink 故障。它没有把多次持续 I/O 故障伪装成成功,也没有扩张 lock recovery API。

对主干的风险 / Risks

没有新的 blocking finding。最强反例是 replacement claimant 在旧 claim 路径复用同一个 token 文件,或者 retired marker 的第一次 unlink 失败后留下锁 artifact;当前实现的 owned branch 只有 captured identity 匹配才删除,pathname/token branch 只有 owner PID 和 token 同时匹配才删除,marker 失败则保留安全重试。测试直接验证 replacement inode 内容不变和一次瞬态故障被吸收;持续 I/O 故障仍由既有 OSError/suppress_errors 边界处理。

exact head 验证:tests/test_file_lock.py 与 cross-process lock 用例 33 passed, 2 skipped;delegation/lock 路径 69 passed;Ruff 和 git diff --check 通过。base/head invalid-UTF8 replay 独立确认 base 抛 UnicodeDecodeError 且 claim 存在,head 删除 own claim、保留 replacement。远端 CI 的 pending/failing 项作为独立 merge-readiness 观察保留;视觉审查不适用,因为这是本地文件锁和 HTTP read adapter,没有渲染页面。

语义与 CI 对齐

本 PR 是 local-only 实现变更,candidate decision 为 local_only;它复用现有 lock vocabulary,没有新增公开 schema、命令或 authority。CI 未决定本 review verdict,changed lock invariant 由 exact-head local suite 和 real delegation path 独立覆盖;与 CI 状态相关的内容继续作为 merge readiness 记录,而不是被声称为通过。

我的整体评价 / Verdict

长程效果是改善的:释放路径少了重复 owner 读取,损坏 claim 不再把后续委派读取拖进不可恢复的 cleanup 状态;用户路径和权限拒绝保持原样。实现复用了已存在的 identity、claim path、owner check 和 retry,没有发现重复 lock authority 或未披露的默认变化。残余风险是生产 latency、安装后行为和平台差异仍未测量,不阻塞这个 bounded cleanup fix。

English verdict: APPROVE for e7e9857. The owned claim now cleans safely from captured file identity even with invalid UTF-8, while replacement inodes, token mismatches, and transient marker cleanup failures remain protected.

@huangruiteng
huangruiteng merged commit 99fdb8c into main Nov 9, 2026
21 of 36 checks passed
@huangruiteng
huangruiteng deleted the codex/delegation-lock-release-20261109 branch November 9, 2026 19:57
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