fix(manager): name the refused read argument instead of a bare failure - #4839
Conversation
A manager read that fails validation returned only invalid_arguments. The caller is a model that can repair its own tool call, so a bare refusal makes it retry blind and the steward answer degrades into an unexplained failure. The reader now derives its allowlist and ranges from the published tool schema and returns every rejected entry as <argument>:<what it must be>, next to the allowed arguments, allowed views and a repair instruction naming the tool the caller actually used. Legal reads keep their existing shape. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Cover each rejection rule, the schema-derived allowlist, the multi-argument case, the called-tool name in the repair instruction, and the unchanged shape of a legal read. Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
Exact head: 75d7f076919d4c147643bdc52044b7e40909cbb5 (re-read immediately before publishing).
动机
管家回答问题时,模型会调用 loopx_manager_read(项目会话里是 loopx_context_read)取证据。参数写错时,读工具只回 {"ok": false, "error": "invalid_arguments"}(基线 09f10c4e3 的 inspection.py:156/:173)。调用方是模型,它能自己改工具调用,但拒绝里没有任何可改的信息,于是只能盲重试或放弃;放弃就会变成用户看到的“管家处理失败”,且本次不会自动重放。
这是「拒绝必须说明缺什么输入」这条已记录要求的直接落点,范围限定在管家回答路径上真正会被点到的那个读工具,属于一个有完整验收边界的切片:失败路径变得可自修复,成功路径一字不变。
改动思路
入口是 app-server 的工具调用闸门 CodexChatAgentSession._check_server_gate(loopx/chat_agent.py:677),它把模型实际调用的工具名和参数交给 read_tool_handler;该 handler 在 loopx/chat_coordination.py:126 被装成 ManagerInspection.read。真正的判定与拒绝都在 read 里,这个 PR 只改这一层。
关键取舍是不要让 schema 和校验规则各说一套。工具已经通过 READ_TOOL["inputSchema"] 向调用方公布了参数集、view 取值和数值范围,所以新增的 rejected_read_arguments 直接从这份 schema 派生 READ_ARGUMENT_NAMES / READ_VIEWS / READ_LIMIT_RANGE / READ_DAYS_RANGE,把原来内联的那一大串重复条件删掉。仓库现有实现里没有可复用的同类拒绝载荷;相邻的 loopx/chat_loopx_mode.py:621-638 是另一种工具形状(action 枚举、抛 ValueError),把它一起抽象会得到一个参数化的跨工具校验器,对两个各自演进的调用方并不划算,因此留在本模块,并作为 P3 记录。没有新增模块、状态、CLI 或持久化契约。
具体改动
loopx/capabilities/manager_context/inspection.py(+78/-25,生产)与 tests/test_chat_manager_inspection.py(+89,测试)。
关键代码讲解
rejected_read_arguments(inspection.py:82):纯函数,逐条产出<argument>:<what it must be>。先报unknown_argument:<name>(排序稳定),再报view、以及依赖view的参数(request_id/include_stopped/days)、offset/limit、goal_id、source_id。合法默认值(offset=0、limit=8)不会被报出来。返回空列表就是放行。ManagerInspection.read(inspection.py:208):原来两处裸拒绝合并成一处,返回同一invalid_arguments错误码,外加rejected_arguments、allowed_arguments、allowed_views和一段detail。detail用调用方实际传进来的工具名,因此项目会话不会被指去调管家专属工具名。接受路径与manager.evidence_read事件记录完全不变。CodexChatAgentSession._check_server_gate(chat_agent.py:677,未改动):确认它转发的是params["tool"],这是「拒绝能说出是哪个工具」的前提;线程/轮次/命名空间不匹配时仍走tool_turn_mismatch,异常仍走read_tool_unavailable,不会被这条新逻辑吞掉。
对主干的风险
最强的回归假设是「提示与 schema 漂移,模型照着改反而更错」。allowed_arguments/allowed_views 由 schema 常量派生,并且测试断言它们等于已公布的 properties 与 view.enum,所以这一半结构上不会漂;逐参数规则仍是 schema 旁边的代码,可能落后,已按 P3 记录最小修复(出现第二个读工具时把规则挪到 schema 常量旁并做差集比对)。
负路径按证据逐条走:越权 Goal 仍在读 Core 之前被拒(test_invalid_or_out_of_scope_reads_do_not_touch_core 未改且通过),授权中途失效仍是 authorization_changed,远程读的 invalid_remote_read/source_outside_available_scope 仍是各自错误码,没有被改写成“参数建议”。合法读形状由 test_a_valid_read_keeps_its_existing_shape 钉住。
scope_fit:生产调用点就是 chat_coordination.py:126 → chat_agent.py:693,当前即可达,不是只在测试里存在的模块。change_proportionality:约 53 行净生产代码换掉一类不可自修复的失败,且同时删掉重复规则;反方最强论点(收益是给模型的建议文本、尚未线上量化)写在残留风险里,不构成阻断。default_off_isolation 判定 not_applicable:本 PR 没有 opt-in 或 default-off 声明,也验证了不存在被 gate 的路径。semantic_alignment 判定 not_applicable:未触碰共享词汇、owner、consumer、投影或持久化值,错误码与 schema 本身都没动。authority_semantics:新增字段只描述拒绝本身,不声称任何新的 actor 生命周期或授权。
验证(本地,按仓策略不查远端 CI):ruff check 通过;tests/test_chat_manager_inspection.py 25 passed;相邻五套件 98 passed;基线/头对照脚本在 origin/main 09f10c4e3 与头部分别执行,基线只回裸拒绝,头部回命名载荷;/tmp/mutation_probe.py 用独立 oracle 在基线 matches_oracle=false、头部 true。
未执行:真实 Codex app-server 进程里的整链工具调用(适配层只读未跑,且本 PR 未改它);两个 view 相关参数同时非法时的拒绝优先级只有单测覆盖。
我的整体评价
正向且成比例。它把管家回答路径上一类「拒绝但不说为什么」的失败变成可机械修正的拒绝,删掉重复的规则副本,不新增状态、权限或用户面变化,失败路径之外可观测语义等价。P2 及以上的阻断项:无。两条 P3 保留在正文(逐参数规则仍可能落后于 schema;相邻的 loopx_collaboration 派发器仍是裸拒绝),不阻断合入。
作者是 PR 所有者,GitHub 不允许自我 approve,因此以 COMMENTED review 记录同一结论。
English verdict: APPROVE - the refusal on the steward read path now names each rejected argument and offers the schema-derived allowlist; validation semantics, the error code and every legal read are unchanged, and no blocking finding or authority/state change was found at head 75d7f07.
Problem
A steward manager read that fails argument validation returns only
{"ok": false, "error": "invalid_arguments"}(loopx/capabilities/manager_context/inspection.py). The caller is a model that can repair its own tool call, but a bare refusal tells it nothing: it retries blind, or the failure surfaces to the user as an unexplained "管家处理失败".Change
The reader now takes its allowlist and ranges from the published tool schema (
READ_TOOL["inputSchema"]) instead of restating them, and refuses with the offending arguments named:{"ok": false, "error": "invalid_arguments", "rejected_arguments": ["unknown_argument:path"], "allowed_arguments": ["view", "source_id", "days", "goal_id", "request_id", "include_stopped", "offset", "limit"], "allowed_views": ["sources", "portfolio", "todos", "deliveries", "handoffs"], "detail": "resend loopx_manager_read with only the allowed arguments; each rejected entry names the argument and what it must be"}Each entry is
<argument>:<what it must be>so the correction is mechanical. The repair instruction names the tool the caller actually used, so a project conversation on the neutralloopx_context_readname is not told to call the manager-only name.Validation
uv run --extra test ruff checkclean.tests/test_chat_manager_inspection.py25 passed; adjacent suites (test_chat_manager_context,test_chat_manager_report,test_manager_ssh_evidence,test_manager_context_roundtrip,test_manager_context_handoff) 98 passed.origin/main09f10c4e3: the same two calls return the bareinvalid_argumentson base and the named payload on head.Risk
Additive on the refusal path only. Validation semantics are unchanged (any invalid argument still refuses), no consumer branches on the error string, and a legal read keeps its existing shape (pinned by a test).