fix(steward): answer a rejected manager read with typed evidence - #4787
huangruiteng wants to merge 1 commit into
Conversation
A refused manager read returned {"ok": false, "error": "<code>"} and that code
was the whole payload, so the steward repeated it to the reader: the row
records "invalid_arguments" pasted into the LoopX steward group on 2026-09-14.
The same shape reached the repository's own tests, which asserted the bare
code as the contract.
Every rejection now returns the code, a readable error sentence naming the
repair, and a typed read_failure row built by manager_read_failure_row:
schema manager_read_failure_v0, the code, the source id, a coverage effect
saying the request read no evidence and must not be presented as no progress,
and the next action for that code (seven codes, each with its own repair). A
turn that cannot collect manager context at all carries the same kind of row
as context_failure beside its existing warnings list, so an unavailable
context names what it costs and what has to happen next instead of leaving a
bare reason code for the model to quote.
The ssh remote-source path already reports reason, coverage_effect and
next_action; this brings the local read and context paths to the same standard.
Disclosed contract change: a rejected read is no longer equal to
{"ok": false, "error": code} — it adds code and read_failure, and error becomes
the repair sentence. The three tests that encoded the old shape now assert the
typed row.
Evidence: 917 manager/steward/chat/coordination tests passed, ruff clean, and
loopx canary premerge --from-git-diff reported no failures (the single red
canary is the pre-existing maintainability-ratchet finding for files this
change does not touch).
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
审查对象：PR #4787，exact head f1d794f3e3473eb9ef3f3c04c79c286f461583b8（base main = 361347713）。审查策略 pull_request_review revision 7。
动机
管家读取证据被拒绝时返回 {"ok": false, "error": "<code>"} —— 整个 payload 只有一个码，于是管家把它原样讲给读者：本行 todo_386976ec9aae 记的 2026-09-14 13:20「im read returned invalid_arguments twice」就是这么来的。这不只是回答质量问题：仓库自己的三个测试把「裸码就是契约」写进了断言，所以我这一刀同时要把那个默认改掉并披露。
本行的要求是：每次失败都要以 source id + typed reason + coverage effect + next action 出现，并且「读不到」永远不能被讲成「没有进展」。这是有界增量：覆盖「管家读取工具拒绝 + 上下文不可用」这一族；Lark im 读取返回 invalid_arguments 的底层实现仍在同一行上开着。
改动思路
入口是 ManagerInspection.read：它由 chat_coordination 作为 host 的 read tool handler 安装，所以它的返回值就是模型看到的 tool result。三个决定：
- 一个 helper 管十处拒绝：
_rejected(code)统一返回ok=false+ 机器可读code+ 写明修复的error句子 + typedread_failure行；error与read_failure.next_action由同一行派生，不会互相矛盾。 - 每个 code 自带修复：
MANAGER_READ_FAILURE_REASONS显式映射 7 个 code 的 coverage effect 与 next action；缺 key 就抛 KeyError，新增拒绝点无法绕过「必须给修复」。 - 对齐既有标准：ssh 远端源路径早就在报
reason+coverage_effect+next_action；这次把本机读取路径与unavailable_manager_context（新增context_failure行，warnings保持不变以兼容既有读者）拉到同一水平。
被放弃的更小方案：只加 typed 行、error 仍留裸码（裸码仍是最可被照抄的字符串）；只改十处拒绝点而不管上下文路径（上下文不可用时仍只剩一个裸 reason）。
关键代码讲解
manager_read_failure_row/MANAGER_READ_FAILURE_REASONS（inspection.py:22起）：7 个 code 各自映射 coverage effect + 修复动作；未登记的 code 直接 KeyError（fail fast，而不是静默发出无类型 payload）。ManagerInspection._rejected（inspection.py:197）：十处拒绝的唯一形状；error句子从同一条 typed 行派生，因此读error的人也会读到修复指引。unavailable_manager_context（chat_manager_context.py:492）：warnings逐字保留，新增context_failuretyped 行，把「这轮没有收集到证据、不要讲成没有进展」写进 payload。
具体改动
loopx/capabilities/manager_context/inspection.py（+92/-11）loopx/chat_manager_context.py（+21）tests/test_chat_manager_inspection.py、tests/test_manager_context_tracking.py、tests/test_chat_project_coordination.py、tests/test_chat_manager_context.py（+35/-16）：把裸码断言改成断言 typed 行（code / source_id / coverage effect / next action）
对主干的风险
已披露的契约变化：被拒绝的读取不再等于 {"ok": false, "error": code}。最强的反例是仓库外某消费者用 result["error"] == code 做字符串比较 —— 我在仓库内全量检索过这类用法：只命中了本次更新的三个测试，没有任何其他消费方；并且 code 与 read_failure.code 都保留同一个值，这就是迁移路径。
其次是「新增 code 忘配修复」：这正是 KeyError 想拦的开发者错误，宁可响亮也不要退回无类型 payload。
本轮不涉及 provider 原文：这条路径的码都是 LoopX 自己的判定（原文泄漏路径已由 #4784 处理），因此没有引入新的文本泄漏面。
验证：pytest -k "manager or steward or chat or coordination" 917 passed；四个受影响套件 + ssh 套件 72 passed；ruff 干净；loopx canary premerge --from-git-diff 无失败（唯一红的是 main 上既有 maintainability-ratchet，涉及本改动未触碰的文件）。
我的整体评价
同意合并（author-owned，以 COMMENTED 记录 APPROVE 结论）。它把「管家把裸码当结论」这类回答质量事故从 tool result 这个产生点掐掉，让每次拒绝都自带 source、原因、覆盖影响与修复，并且刻意只覆盖这一族、把 Lark im 底层留在同一行。控制面改动（loopx/**），按仓库规则由维护者合并。
English verdict: APPROVE - head f1d794f; every rejected manager read now returns machine-readable code plus a repair sentence in error plus a typed read_failure row (schema manager_read_failure_v0: code, source_id, a coverage effect stating that no evidence was read and must not be presented as no progress, and a per-code next action across seven codes mapped in one dict whose missing key raises KeyError), and an uncollected turn context carries the same kind of context_failure row beside its unchanged warnings; this removes the bare-code payload the steward repeated as "invalid_arguments" and that three repository tests had encoded as the contract, while the ssh remote path's existing reason/coverage_effect/next_action standard is now matched by the local read and context paths; the shape change is disclosed, all in-repository consumers and tests were updated, and code/read_failure.code preserve the old value as the migration path; validated by 917 manager/steward/chat/coordination tests, 72 focused tests, ruff, and loopx canary premerge --from-git-diff with no failures (the single red canary being the pre-existing maintainability-ratchet finding for untouched files); scope note: the underlying Lark im read and the live answer re-measurement stay open on the same row; control-plane change, maintainer merge only.
1 participant
动机 / Motivation
管家读取证据被拒绝时，返回的是
{"ok": false, "error": "<code>"}—— 整个 payload 只有一个码。于是管家把这个码原样讲给读者：本行todo_386976ec9aae记录的 2026-09-14 13:20「im read returned invalid_arguments twice」就是这样来的。这不只是回答质量问题：连仓库自己的测试也把「裸码就是契约」写进了断言。本行的要求很明确：每次失败都要以 source id + typed reason + coverage effect + next action 出现，并且「读不到」永远不能被讲成「没有进展」。
改动思路 / Approach
manager_read_failure_row(code, source_id)：schemamanager_read_failure_v0，携带 code、source_id、「本次没有读到证据、不得当成没有进展」的 coverage effect，以及每个 code 各自的 next action（7 个 code：unsupported_read_tool、invalid_arguments、authorization_changed、source_outside_available_scope、invalid_remote_read、goal_outside_available_scope、handoff_query_unavailable_or_invalid）。ManagerInspection.read()的 10 处失败返回统一走_rejected(code)：保留ok=false、新增机器可读的code、把error从裸码改成写明修复动作的句子（所以读者即使照抄error也只会抄到修复指引），并附上 typedread_failure行。unavailable_manager_context）同样补上context_failuretyped 行，与既有的warnings并存。reason+coverage_effect+next_action；这次把本机读取与上下文路径拉到同一水平。具体改动 / Changes
loopx/capabilities/manager_context/inspection.py（+92/-11）：typed 行、7 个 code 的修复映射、_rejected()统一 10 处失败返回。loopx/chat_manager_context.py（+21）：context_failuretyped 行（warnings保持不变，兼容既有读者）。tests/test_chat_manager_inspection.py、tests/test_manager_context_tracking.py、tests/test_chat_project_coordination.py、tests/test_chat_manager_context.py（+35/-16）：把编码旧形状的断言改成断言 typed 行（code / source_id / coverage effect / next action）。对主干的风险 / Risk
{"ok": false, "error": code}—— 增加了code与read_failure，error变成修复句子。仓库内所有消费者与测试都已按新形状更新；这是本行要的方向（裸码不该是可被引用的最终文本）。_rejected对 7 个 code 是显式映射，缺 key 会抛KeyError而不是静默降级 —— 新增 code 时必须同时给出 coverage effect 与修复动作，这是有意的强约束。im读取返回invalid_arguments的那次事故，其底层读取实现（而非本工具的拒绝分支）仍留在同一行上；本 PR 覆盖的是「管家读取工具拒绝/上下文不可用」这一族。pytest -k "manager or steward or chat or coordination"917 passed；ruff 干净；loopx canary premerge --from-git-diff无失败（唯一红的 canary 是 main 上既有的 maintainability-ratchet，涉及本改动未触碰的文件）。loopx/**），按仓库规则由维护者合并，作者不自合并。English verdict: APPROVE - every rejected manager read now answers with machine-readable code plus a repair sentence in error plus a typed read_failure row (schema manager_read_failure_v0: code, source_id, a coverage effect stating that no evidence was read and must not be presented as no progress, and a per-code next action across seven codes), and an uncollected context carries the same kind of context_failure row beside its existing warnings; this removes the bare-code payload that the steward repeated as "invalid_arguments" and that the repository's own tests had encoded as the contract; the ssh remote path already reported reason/coverage_effect/next_action and this brings the local read and context paths to that standard; the shape change is disclosed and all in-repository consumers and tests were updated; validated by 917 manager/steward/chat/coordination tests, ruff, and
loopx canary premerge --from-git-diffwith no failures (the single red canary being the pre-existing maintainability-ratchet finding for untouched files); residual scope note: the underlying Lark im read that produced the incident stays open on the same row; control-plane change, maintainer merge only.