Repository navigation
fix(status): fail closed on unreadable retry journals - #7693
Conversation
Composition retry readback treated every OSError as an empty journal, hiding persisted pending work from status. Preserve missing-file behavior but raise a path-neutral error for other read failures, with a regression test covering both receipt and projection reads. Signed-off-by: Duang777 <duangjl007@gmail.com>
Signed-off-by: Duang777 <duangjl007@gmail.com>
|
Exact-head validation refreshed for
This validation does not grant merge authority; maintainer review remains required. |
|
CI triage for exact head
I am not mixing that unrelated self-update test change into this recovery-journal PR. The remaining checks continue to run; after the owning fix reaches Failed job: https://github.com/loopx-project/loopx/actions/runs/40918492419/job/123609945350 |
|
Additional CI triage for
Failed job: https://github.com/loopx-project/loopx/actions/runs/40918492419/job/123609944854 |
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=max
[P2] 目录权限错误仍绕过新的 fail-closed 分支。 当前 head 4eb3f713eee62dcaded04022801b4ff28750f370 的 collect_pending_composition_retry_projection 仍先执行 journal_path.is_file()。在 Python 3.14 中，已有 journal 的父目录不可访问会使这个检查返回 false，随后直接跳过 reader。真实 File/SQLite 的 status 均返回 exit 0、ok=true 并省略 pending receipt；恢复目录权限后，同一条记录重新出现。移除 collect_pending_composition_retry_projection 的 is_file 预检查，或显式传播非缺失的 stat 错误；让现有 reader 统一区分缺失与不可读，并补真实 status 权限恢复回归。
动机 / Motivation
维护者或 Agent 通过 status 查找已提交写回后的待重试工作。
日志文件失去读取权限时，旧版将失败当作空记录；本版返回明确错误，但父目录失去访问权限的情况仍会隐藏记录。
独立 File/SQLite 检查确认文件读取拒绝已显式失败，目录访问拒绝仍返回 ok=true，恢复权限后同一条记录重新出现。
范围为重试记录读取与状态反馈；写回、扣额、hook 和持久格式保持原合同。跨平台和持续性能未验。
改动思路 / Approach
读取失败由现有 journal IO owner 处理，CLI 复用既有错误投影。is_file 预检查重复了存在性决策，却会提前吞掉权限错误，应收敛到同一次读取。 当前 PR 只需补齐同一条重试发现路径，无需新增协议、配置或通用恢复器。
原写回 owner 生成有稳定身份的 append-only receipt；reader 折叠每条 receipt 的最新状态，状态命令负责公开呈现待恢复工作。修复应让“没有文件”和“没有读到文件”保持区别。保留一个额外的存在性判断没有独立合同收益，反而把本 PR 要修复的假健康路径留下。私有进程只删除该预检查后，两个真实后端的目录拒绝都进入既有错误分支；这是一项最小修复验证，没有改写当前提案。
| 选定 Goal 的实际状态 | 不可变基线 | 当前 head |
|---|---|---|
| 可读 pending journal | 成功，1 条 pending | 同样成功，1 条 pending |
| 文件确实缺失 | 成功，无 pending | 保留原行为 |
| 文件读取权限拒绝 | 错误地成功、无 pending | 正确返回不可读错误、exit 1 |
| 父目录访问权限拒绝 | 错误地成功、无 pending | 缺口仍在 |
| 恢复访问权限 | 原 pending 重新出现 | 原 pending 重新出现 |
具体改动 / Changes
实际不可变基线 7c6fadd6fe20429468c979eec77ccb1445304dae。当前完整差异两文件、+43/-1：生产 +5/-1，原测试模块 +38。没有新增配置、schema、存储 writer 或 actor；错误分类使用异常类型，既有 retryable/settled 状态和幂等身份继续由原 owner 维护。
关键代码讲解
_iter_composition_retry_rows现在仅把FileNotFoundError视为缺失，其他OSError包装为不带路径的RuntimeError。真实文件读取拒绝已验证这项改进，原全量折叠和校验逻辑保留。collect_pending_composition_retry_projection负责 Goal/Agent 过滤与展示。第 463–464 行的前置is_file()使目录拒绝无法到达新分支，是当前最小修复位置。handle_status_command使用已有status_collection_failed和 exit 1 监督收集错误；无须另加恢复协议。
规格检索范围是当前 docs、公开故障与上述 reader/status 调用链，未找到独立规定该 journal 读取错误的书面规格，采用 no_spec。预期来自可独立复现的不变量：读取失败不能证明没有耐久恢复记录；新增测试不是它自己的规格。旧接口的调用者、compaction、原身份 replay 与 settle 路径一并检查，没有理由把可选 worker tracing 或 quota ledger 并为同一个 reader。
对主干的风险 / Risks
新增测试只故障注入 Path.read_text，所以前面的 is_file() 仍成功；该测试通过时，真实目录权限反例仍能失败。这解释了为什么已有 35 项 retry/status 测试全部通过，仍不足以批准完整修复。此遗漏在基线也存在，但它属于本 PR 要完成的同一发现路径，不能归为无关历史红灯。
本轮在同一合成 canonical File/SQLite 状态上执行 20 次不可变基线/当前 CLI，另有两次私有最小修复对照。保留全部命令、完整包、fixture 和观察指纹；8 组正常、缺失、目录拒绝和恢复包除经校验的 observation clock/派生 source age 外一致。目录拒绝的相等恰好证明缺口保留，不代表行为正确。全部原 fixture 内容保持；没有清空 journal、重放业务或扣减额度。
PASS：35 focused tests、Ruff、py_compile、配置内17文件 mypy、两变更文件的公开边界检查。FAIL：当前 standard premerge 的 semantic-vocabulary-drift-smoke 在原120秒上限超时；6项 direct 通过、10项 selected 中9项通过。该轮与私有权限探针/文档构建有重叠；未完成同不可变基线归因，不能猜测原因、称作已修复或标为无关。不为获取绿色结果重复 gate，也未放宽限制。未获取、轮询或等待远端 CI，作者评论中的 CI 状态仍只是作者来源。
语义与 CI 对齐
差异 advisory 没有发现受支持的新 vocabulary carrier；新增文字是中性的读取错误，既有 schema、actor authority 和 replay 说明未改。该 advisory 不证明全部语义覆盖，更不抵消真实目录拒绝和本地 gate 失败。没有新增 default-off 能力或激活声明。原收集失败是确定性错误，恢复说明不授予重试、业务或合并权。
我的整体评价 / Verdict
REQUEST_CHANGES。 局部文件读取修复有价值，体量合理；当前完整目标的 long_horizon 和 user_experience 均为 not_yet_proven，因为后续调用仍可看见错误的成功空视图。读取失败由现有 journal IO owner 处理，CLI 复用既有错误投影。is_file 预检查重复了存在性决策，却会提前吞掉权限错误，应收敛到同一次读取。 当前 PR 只需补齐同一条重试发现路径，无需新增协议、配置或通用恢复器。
已检查 status CLI 和既有投影消费者；此差异没有首屏、布局或可操作页面变更，视觉审查不适用。 视觉未验证范围：未执行浏览器或原生 Windows 资格；没有受影响的视觉布局。全 Goal 聚合 CLI、原生 Windows、PostgreSQL 和持续性能未在本轮独验；没有 storage/schema 变更，不借其它矩阵冒充覆盖。
请先补齐上述前置 stat 分支及真实权限恢复回归，再重验受影响状态路径，并解决或独立归因当前所需的本地超时。无需为此补丁完成整个父 roadmap，也无需新框架。
English verdict: REQUEST_CHANGES — 4eb3f71. Real File/SQLite status still reports a healthy empty view when the existing retry journal's parent directory is inaccessible. Route that failure through the existing reader and cover permission recovery. The file-read-denial fix works; 35 focused tests pass, while the current standard semantic smoke times out at its unchanged 120-second limit.
Signed-off-by: Duang777 <duangjl007@gmail.com>
The first fix hardened the journal read but left an is_file preflight that bypassed it for inaccessible parent directories. Remove that duplicate decision and cover real File and SQLite status denial plus recovery so missing and unreadable journals keep distinct outcomes. Signed-off-by: Duang777 <duangjl007@gmail.com>
|
Exact-head validation for
Non-blocking baseline observations reproduced unchanged on clean |
loopx-agent
left a comment
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent | model=gpt-6.1-sol | provider=OpenAI | reasoning_effort=max | source=runtime_reported
动机 / Motivation
依赖 status 判断是否还有重试工作的操作者与后续 Agent。重试日志存在但不可读时，旧读取器会把它当作没有待办；Python 3.14 下父目录不可访问还会被 is_file 预检查静默跳过，后续工作因此可能漏掉应恢复的写回。本轮验证了缺失日志仍可正常读取，不可读日志明确失败且诊断不泄露路径，恢复权限后原重试项重新可见，已结算项也不会再次出现。本次仅修复现有重试日志读取及状态投影，不新增配置、重放权限或自动执行，也不把审查通过视为合并或整个项目完成。
改动思路 / Approach
复用现有日志读取器与 status 失败出口，在实际 read_text 一处区分缺失与其他 IO 错误；移除会吞掉目录权限错误的投影预检查，避免维护两套文件可读性判断。当前 PR 的可用边界是读者得到真实的重试状态或明确失败，权限恢复即可继续；无需新状态、重放机制或更大迁移。读取失败交由既有 status 外层变成可见的失败结果；真正缺失仍走正常空投影。这是观察真实性的不变量，日志重放和主要写回仍由原 owner 管理。删除预检查是同域且可回滚的简化，不需要引入新状态、开关或第二套文件可读性机制。
具体改动 / Changes
审查 head bc7d115a4114e51d92b6bdd0d9bd87ea4e797b1e，不可变迁移前 base f6d8e001376c8e682fed6e325512c2144988d4aa。本轮先读取此前审查及作者回复；frame revision 4eb3f713eee62dcaded04022801b4ff28750f370。全 PR 三个文件包含一个现有读取器修复、一个冗余预检查删除和两处测试扩展。没有新增公共协议、配置、持久字段或执行权限。JR-MISSING、JR-UNREADABLE、JR-RECOVERY 均 implemented：分别由真实缺失、文件/父目录拒绝及权限恢复、终态与超显示上限案例验证。此前 review 的 commit 仍是旧 head，不能把 GitHub inline 自动映射误称为当时已审查当前 head。
关键代码讲解
_iter_composition_retry_rows：以真实读取结果区分缺失与不可读；保留逐行格式过滤。pending_composition_retry_receipts_for_path：先折叠每个 receipt 的最新状态，再筛选尚待重试项。collect_pending_composition_retry_projection：不再预先用 is_file 跳过权限错误；既有 Agent 过滤和显示上限不变。
对主干的风险 / Risks
未发现尚未解决的阻塞性 finding。 本轮不是只确认删除 is_file，而是复核整个当前 diff、读取/折叠/投影调用链及此前 finding。真实 File/SQLite、Python 3.12 与3.14 共40次主 CLI 调用证明：健康、缺失和恢复的完整输出与 base 一致；文件/父目录不可读时 head 统一 exit1、返回路径无关的明确错误。旧 base 在3.14仍 exit0且漏掉 pending，因此独立 oracle 对旧缺陷敏感。另16次真实调用覆盖格式异常/外来 schema、settled、26个待办只显示20个但保留完整计数，以及重新发现原待办；全部通过，日志原字节保持，未产生 spend。
聚焦 Python 37 PASS，Ruff PASS，标准 premerge PASS：13个所选 canary/risk smoke 通过，原生工具保留一个既有 maintainability advisory。此前 head 的语义 smoke 超时仍是历史失败，不被本次新 head 的通过追溯改写。POSIX 非 root 权限是本轮真实边界，Windows 没有被这些结果覆盖。当前 wait_for_ci=false，未获取、轮询或等待远端 CI。
语义与 CI 对齐
复用原 retryable/settled vocabulary 和异常类型，不用标题、说明文字或子串判定权限。唯一有意行为变化是不可读不再等于空，并且去掉实际路径；缺失、终态、Agent 过滤、日志 fold 与原重放权限不变。此变更无需 opt-in，新错误也不是新增重放义务。标准本地门通过仍不提供合并权限，旧 REQUEST_CHANGES 的有效状态需在批准发布后单独读取并如实报告。
我的整体评价 / Verdict
这是 goal_achieved，仅限既有重试观察缺陷：正向、反向与恢复路径都证明了有用结果，机制与问题相称。long_horizon 为 preserved，恢复承诺不会被错误空状态抹去；user_experience 为 improved，同一读取任务无需新增准备或确认轮次。复用现有日志读取器与 status 失败出口，在实际 read_text 一处区分缺失与其他 IO 错误；移除会吞掉目录权限错误的投影预检查，避免维护两套文件可读性判断。当前 PR 的可用边界是读者得到真实的重试状态或明确失败，权限恢复即可继续；无需新状态、重放机制或更大迁移。已检查 status 的既有投影与错误消费者；本次没有页面布局、控件或第一屏呈现改动。仅验证源码 CLI；Windows 权限语义与已安装 Host 未独立验证。批准只针对这个 head 的代码与限定验收，实际合并和旧 review 状态仍是独立 gate。
English verdict: APPROVE — head bc7d115a4114e51d92b6bdd0d9bd87ea4e797b1e; the prior unreadable-parent defect is resolved, real File/SQLite permission/recovery cases and standard premerge pass; merge authority remains separate.
Signed-off-by: Duang777 <duangjl007@gmail.com>
|
Exact-head validation for The previous CI failures were stale-base failures outside this PR's three changed files:
PR-specific evidence remains green: 37 focused retry/status tests passed; Ruff, diff hygiene, bilingual policy, and the semantic advisory passed; clean standard premerge selected the exact three changed files and passed all 13 checks with no blocking failures or manual holds. The known maintainability advisory does not name a changed file. The new remote checks are queued. Maintainer review and merge authority remain separate. |
Signed-off-by: Duang777 <duangjl007@gmail.com>
|
Exact-head refresh for The eight new main commits do not touch this PR's three files. Focused retry/status tests pass 37/37, mypy reports no issues across 17 source entries, Ruff and committed diff checks pass, and clean standard premerge selects exactly three changed files and passes all 13 checks with no blocking failures or manual holds. The inherited maintainability advisory still names no changed file. GitHub currently retains |
Summary
Problem
_iter_composition_retry_rows()caught everyOSErrorand returned an empty row set. If a persisted retry journal became temporarily unreadable, status interpreted the read failure as "no pending retry receipt" and hid durable recovery work.The fix handles
FileNotFoundErroras the existing empty-journal case and raises a path-neutral error for other I/O failures. The status entrypoint then returns its existingstatus_collection_failedresponse instead of publishing a healthy empty projection.Validation
DID NOT RAISE RuntimeError, then passed after the exception handling changeuv run --extra test pytest -q tests/control_plane/test_post_writeback_composition_retry.py(21 passed)uv run --extra test pytest -q tests/control_plane/test_status_receipt_retry_projection.py(14 passed)loopx statusfault injection: exit1,ok=false,status_collection_failed, path-neutral erroruv run --extra test ruff check loopx/control_plane/post_writeback_composition_retry.py tests/control_plane/test_post_writeback_composition_retry.pyuv run --extra test python scripts/generate_semantic_inventory.py --changed-from upstream/main(no new vocabulary carriers)uv run --extra test loopx canary premerge --from-git-diff --git-diff-base upstream/main(10/10selected catalog and risk-profile smokes passed)Sibling sweep
The same fail-open shape was checked in
worker_bridge.pyand quota slot accounting. The former is optional diagnostic tracing; the latter preserves spend denial when settlement evidence is absent. Neither is the same recovery-projection defect, so this PR leaves them unchanged.