Skip to content

fix(file-lock): preserve diagnostics for invalid UTF-8 holder records - #6762

Merged
songoow merged 3 commits into
loopx-project:mainfrom
steven-kid:roadmap-auto/20261017-lock-holder-utf8
Oct 19, 2026
Merged

songoow merged 3 commits into
loopx-project:mainfrom
steven-kid:roadmap-auto/20261017-lock-holder-utf8

Conversation

@steven-kid

Copy link
Copy Markdown
Collaborator

Goal And Delivered Outcome

Damaged UTF-8 in advisory lock-holder records currently makes liveness reads and contended acquisition raise UnicodeDecodeError, masking the typed timeout and recovery guidance. Treat decoding failure as unreadable metadata in the existing shared filesystem reader. Keep valid alternate records, kernel exclusion, incident recording and normal recovery after the actual holder exits.

Source-checked against upstream main 84dfa63c26a6b03a3c5e6039108f1540e8b2b1bb; the failing baseline and unchanged reader are identified below. This is a bounded S10 reliability/diagnostics fix, not broader roadmap completion. No duplicate issue is created for this self-contained defect. Intended target: loopx-project/loopx:main. Draft requested by the contributor.

Author Declaration

Written by: model_agent — OpenAI Codex (GPT-6).

Specification: docs/reference/protocols/file-lock-acquisition-v0.md, Holder And Incident Records / Operator Recovery, at 84dfa63c26a6b03a3c5e6039108f1540e8b2b1bb.

Criterion Disposition Owner / evidence
Preserve typed timeout and incident implemented Existing shared holder reader; real subprocess contention regression
Metadata cannot grant kernel ownership implemented Waiter refused while holder lives; readback leaves damaged bytes unchanged
Keep valid current/legacy alternate record implemented Both Windows layout directions tested
Recover after actual holder exit implemented Real reacquisition and released-state readback

Scope And Continuation

Complete within the reproduced defect. Retains the existing Python OS/filesystem adapter; no parallel domain decision owner, new capability, state vocabulary or public API shape. Both consumers already share one reader, so no additional refactor is warranted. The existing protocol and repair-pattern catalog capture the same failure/recovery contract. No independent successor needed for this repair.

Validation

Tested revision: 8d956fd08ffdaef3838985c1b3abbfeeb8a90319 (same source content tested before the local commit). Run state: finished. Input classes: synthetic, public_fixture.

Check kind Result Evidence / limits
regression_parity passed python -m pytest -q tests/test_file_lock.py -k invalid_utf8 --tb=short on baseline 73aa2bc: 5 new cases fail with UnicodeDecodeError; production reader and regression source are unchanged at this PR base; all pass on the candidate
real_backend / integration / unit passed python -m pytest -q -rs tests/test_file_lock.py tests/test_file_lock_cross_process.py tests/test_collaboration_mcp.py tests/test_repair_pattern_lookup.py: 45 passed, 2 skipped in 16.36s; disposable Linux kernel locks, competing subprocess, incident/recovery and stdio collaboration
static passed python -m ruff check loopx/file_lock.py tests/test_file_lock.py; git diff --check
static passed Public-boundary scan of all four changed files; two missing-registry warnings in an unconfigured checkout
static passed python scripts/check_rfc_bilingual.py --base 84dfa63c26a6b03a3c5e6039108f1540e8b2b1bb --staged: no RFC pair changed
integration not_run Native Windows: two existing Win32 extended-path skips; sidecar layout exercised via existing fixture on Linux
integration not_run Full repository suite, full canary, packaged frontend/Lark journeys

Python 3.12.14, pytest 9.1.1, MCP 1.30.0, Node 24.19.0. Current checkout test dependencies and npm lockfile installed. No live user data or provider accounts used.

Frontend / Visual Evidence

UI impact: none. Existing timeout envelope and operator action preserved; no frontend/Lark/CLI command shape change. Visual evidence and shared-authority RFC fixture promotion: not applicable.

Type of Change

  • Bug fix
  • Test update
  • Documentation update

Boundary Checklist

  • No private state, credentials, raw traces, internal links or local machine paths in this proposal.
  • No benchmark duplication or new operational permissions.
  • Scope limited to the reproduced defect.
  • UI impact none.
  • DCO Signed-off-by included.

Reapply the retained repair from 47e1be3a98d77fd293418f0603e7f3ec3aeea546 to current upstream for local validation. No additional behavior changes.

Signed-off-by: Yuchao Wang <stevenkid404@gmail.com>
Signed-off-by: Yuchao Wang <stevenkid404@gmail.com>
@mergify

mergify Bot commented Oct 18, 2026

Copy link
Copy Markdown

This pull request has merge conflicts with main and cannot be merged
until they are resolved. Please rebase or merge the base branch, @steven-kid.

Choose the remote for the base repository, not an out-of-date fork.
For a fork clone, first inspect git remote -v; upstream must point
to https://github.com/loopx-project/loopx.git. If it is absent, add it
with git remote add upstream https://github.com/loopx-project/loopx.git.
Then run:

git fetch upstream
git rebase upstream/main
# Resolve each conflict, git add the resolved files, then git rebase --continue.
git push --force-with-lease origin HEAD

For a same-repository clone whose origin points to
https://github.com/loopx-project/loopx.git, use origin instead of
upstream for fetch/rebase. If you prefer merging the base, use
git merge <base-remote>/main and push normally.

Keep the DCO Signed-off-by trailer on every commit when you rebase.
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase Mergify: the pull request has merge conflicts with its base branch label Oct 18, 2026
Signed-off-by: Yuchao Wang <stevenkid404@gmail.com>
@mergify mergify Bot removed the needs-rebase Mergify: the pull request has merge conflicts with its base branch label Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Resolved the conflict reported in this comment with signed-off merge commit a2ee229fc616d0bcd3148ea4a98b0024e5bdb42a, incorporating upstream main at a0fb547af9aac4e4d36f59fb05cb64eb56fd750f. Pushed normally; published history is preserved. GitHub readback now reports mergeable=true; the PR remains draft.

The only conflict was adjacent additions in skills/loopx-self-repair/references/repair-patterns.md. Both the upstream scheduler entry and the lock-holder diagnostic entry are retained. Relative to the incorporated upstream, the PR remains the original four-file fix (+76/-1); production and regression-test patch content is unchanged.

Validation on this exact merge content:

  • python -m pytest -q -rs tests/test_file_lock.py tests/test_file_lock_cross_process.py tests/test_collaboration_mcp.py tests/test_repair_pattern_lookup.py tests/test_chat_process_recovery.py: 60 passed, 2 existing Win32 extended-path skips (49.32s).
  • python -m ruff check loopx/file_lock.py tests/test_file_lock.py: passed.
  • Standard loopx canary premerge, using the incorporated upstream as the base and the four PR paths explicitly: all six direct checks, five catalog smokes, eight risk-profile smokes and the public-boundary scan passed.

Full repository tests, local browser journeys and native Windows qualification were not rerun locally. Fresh hosted CI is pending; the previously diagnosed upstream browser failure was not modified or blindly rerun.

@steven-kid
steven-kid marked this pull request as ready for review October 19, 2026 03:24

@loopx-agent loopx-agent 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-6.1-sol · OpenAI · runtime_reported · reasoning_effort=max

Exact head: a2ee229fc616d0bcd3148ea4a98b0024e5bdb42a

动机

未发现阻塞性实现问题。

运行本地命令或持续任务的操作者,在查询锁持有者或等待另一个进程释放锁时会遇到这个问题。

例如持有者记录含有损坏的 UTF-8 字节:旧版本直接抛出解码异常,遮住锁超时和恢复指引;修复后继续报告已有的不可读状态或带检查指引的超时。

本轮在真实 POSIX 内核锁上验证了超时、事件记录、持有者退出后的重新获取与释放;损坏记录不会被诊断读取改写。

范围仅限既有共享读取器的解码容错,不自动终止进程、不删除锁,也不增加新的命令或权限。

改动思路

在共享文件系统读取器捕获解码失败,两个既有消费者即可共同保留诊断;无需第二套状态、扫描器或恢复决策源。

本 PR 的完整边界是损坏持有者元数据的诊断、内核互斥和正常恢复;现有协议及修复目录同步解释这一边界。

我比较了不修复、在两个消费者分别兜底以及扩展共享读取器三种方案。第一种保留可重复的故障,第二种重复错误处理;当前一处异常边界是最小有效改动。持有者记录仍是诊断信息,内核仍独立决定锁归属;既有 current/legacy 兼容有实际旧进程消费者,继续保留合理。没有新配置、人工确认或独立修复流程。

具体改动

完整 base a0fb547af9aac4e4d36f59fb05cb64eb56fd750f 到当前 head 为四路径 +76/-1:生产处理器 +1/-1、测试67行、协议与修复目录8行。

关键代码讲解

  • _read_latest_holder_record:在读取 UTF-8 时捕获解码失败,继续看有效的另一个记录;字段过滤和按 acquired_at 选择最新记录的规则不变。它被 liveness 与 timeout 两处共用,诊断读取没有写入动作。

  • _timeout_error:只在真实内核获取失败、期限已到后组装超时。现在损坏记录能走到空 holder、一次事件和既有检查指引;exclusive_file_lock 的加锁与实际持有者退出后的恢复顺序没有变化。

规范为 docs/reference/protocols/file-lock-acquisition-v0.md,spec_revision=a0fb547af9aac4e4d36f59fb05cb64eb56fd750f,使用修改前文本作为独立依据,新段落仅是本次审查内容。LOCK-KERNEL implemented:实际持锁时两个获取入口均未绕过;LOCK-TIMEOUT implemented:类型化超时、一次事件及完整 operator_action;LOCK-LEGACY implemented:两种损坏方向保留有效记录,原有最新时间选择用例继续通过;LOCK-RECOVERY implemented:实际持有者退出后重新获取并读回 released。

修复目录是既有安装技能的可检索参考,增加的是这一真实故障的复现和安全恢复说明,没有新权限或重复决策源。

对主干的风险

最强反例是损坏元数据导致错误地获得锁,或者诊断成功但实际超时仍崩溃。本轮同一独立脚本在 base/head 各运行六类场景:base 五类抛 UnicodeDecodeError,head 六类全过;真实 POSIX flock 验证互斥、single-flight 拒绝、一次 incident、原字节保留及退出后的正常恢复。Windows 两文件名布局仅用传输夹具模拟,未声称执行 Win32 内核。

五个相关文件测试60 passed、2条未改动的 Win32 extended-path 条件 skip;Ruff、差异空白与六项 direct 检查通过。标准 canary 初次13/14 selected通过,语义 smoke 因复用工作树缺少 TypeScript npm 依赖失败;正常 npm ci --ignore-scripts 后同一个完整 smoke 通过,原失败保留,未改源码、断言或阈值。其余检查无须重复。

语义与 CI 对齐

复用既有 unreadable/timeout 词汇与单一读取 owner;新文字保留检查、确认权限、持有者退出后重试及禁止删锁的要求。语义扫描保留其未解析 producer 等覆盖限制,不是全局语义等价证明。遵循 wait_for_ci=false,没有查询、轮询、等待或触发托管 CI。Goal 的 exact-scope change-quality receipt_missing 仍是独立合并资格 hold,不能把本地 review 称为已可合并。

我的整体评价

problem_context=goal_achieved 仅指本次损坏诊断修复;long_horizon=improved、user_experience=improved,依据是实际恢复继续工作以及现有指引替代崩溃,不宣称长期全量收益。observable_semantics=intentional_change_validated:正常、malformed、替代记录和内核行为保留,invalid UTF-8 故障有意修正。结构与规模合理,兼容保留有实际消费者,单一共享 owner 已是适当简化,无需额外框架。剩余范围包括原生 Windows、完整仓库测试及安装用户采纳;本次审查不授予合并或升级权限。

English verdict: APPROVE at a2ee229fc616d0bcd3148ea4a98b0024e5bdb42a. The shared reader fixes five reproduced baseline decoding failures; real POSIX contention, incident preservation and recovery pass without weakening kernel ownership. Native Windows and whole-repository qualification remain outside this run. The missing managed change-quality receipt is a separate merge hold.

@songoow
songoow merged commit 024a12d into loopx-project:main Oct 19, 2026
56 of 84 checks passed
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.

3 participants