Repository navigation
fix(goal-channel): keep the real validation reason on a rejected prepare - #6726
Conversation
goal-channel prepare-operation mapped every ValueError from the typed operation owner to blocker=invalid_configuration with a generic summary, so an operator or agent saw "the Goal Channel configuration is invalid" even when the real cause was an invalid executor binding or a payload digest mismatch. Keep the compatibility blocker and add the code-authored validation reason through details.validation_reason. Messages that look path-like, carry control characters, or are unreasonably long collapse to the exception class so private material never reaches the packet. Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com>
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
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
[P2] 不要把未知异常文本当作公开安全的校验原因。 准确审查 head：da7b835b844ef524ce7a521ffa931d3753c2eae5。
动机
通过命令行准备待确认操作的用户与 Agent，需要区分请求校验失败和通道配置故障。
例如执行器绑定无效时，旧版只给出笼统的配置错误；新版能显示具体原因，但未知参数名也会未经脱敏进入同一个公开诊断字段。
独立 CLI 验证确认诊断可用，也确认了新增的信息泄露路径；当前版本尚未同时满足可操作错误和公开安全边界。
本次不验证真实飞书发送、金融执行、安装采纳或长期运行收益，也不授予合并权限。
判断依据是修改前的 docs/architecture/rfcs/goal-channel-collaboration-v0.md，revision d4487a7c0a332365fc80ee3ca43d421bdf510d42：Command Contract 要求紧凑、类型化的失败回包；Lark Provider Binding 要求公开状态不暴露私有标识、原始载荷、路径或凭证。需要保留具体诊断的价值，同时证明其可公开，不能由新过滤规则自行定义“安全”。
改动思路
复用现有错误 packet 合理，但公开文本必须由明确的安全原因契约决定；字符黑名单不能承担来源鉴别。
当前 PR 只需修正这一个诊断投影并补 CLI 负例，保留既有操作校验、授权、幂等和成功路径。
数据流是私有请求 → 原操作归一化器 → ValueError → 新 helper → operation_packet.details。旧版在最后一步只显示通用配置错误；新版保留 blocker 和 summary，再增加具体原因。这是有用的、可兼容的方向，不需要新增命令、状态机、配置项或完整 TypeScript 迁移。但原归一化器还会生成带输入内容的错误，不能把所有 ValueError 都视作固定文案。最小修复是复用原错误 owner 的安全原因代码，或在本边界明确允许固定公开原因；其他情况保持通用回退。
具体改动
完整差异只有两个文件，+52/-1。loopx/cli_commands/goal_channel_operation.py 增加 helper，并在共享的 except ValueError 处添加 details.validation_reason。它先 strip，再拒绝空串、超过240字符、斜杠、反斜杠、波浪号和控制字符，其他文本原样返回。PR 描述说超过长度会截断，但实现实际回退到异常类名；应同步描述。
关键代码讲解
_safe_validation_reason（248行）的关键风险在259–269行：这些字符条件只能识别部分文本形状，无法证明消息是固定代码文本。run_goal_channel_operation（272行）把 helper 用在整个 prepare/deliver 分发的 ValueError 分支，因此错误来源不限于作者列出的两个固定例子。其余执行器漂移、发送阶段未知结果、冲突以及普通异常处理保持不变。
未修改的 ChatActionService._allowed_parameters 会执行 raise ValueError(f"unknown typed action parameter: {sorted(unknown)[0]}")。独立使用真实 source console、隔离 File registry、原生安装/启用/doctor 的 bundled provider，向 prepare-operation 送入未知键 api_key=synthetic-review-sentinel：head 的公开 JSON 原样出现 unknown typed action parameter: api_key=synthetic-review-sentinel，同输入在 d4487a7c0a332365fc80ee3ca43d421bdf510d42 只有通用 summary。这里全部是合成标记，没有使用真实凭据，也没有发出飞书消息或创建操作。将键换成含斜杠的名字则正确回退，说明问题正是字符黑名单遗漏动态文本，而非正常拒绝失效。
tests/extensions/test_lark_goal_channel_operation.py 新增 helper 级断言，覆盖固定绑定/摘要错误、路径、换行和空串。整模块41项全部通过，却仍遗漏上述正常 caller 链；请把未知请求字段及 projection 字段的反例放在完整 dispatcher/CLI 边界验证。
对主干的风险
这是即时错误投影变化，没有新增操作授权或持久化格式。四个对照输入在 base/head 都是 exit1、无 actions store、无外部写入；固定执行器绑定和 digest 错误能显示具体原因，provider 关闭时也仍由原 extension_unavailable 门挡住。已有主体绑定、确认、幂等和未知发送结果语义没有被本差异改写。
独立验证：操作测试41 passed/0 skip，Ruff通过，diff advisory后完整 semantic drift smoke通过；标准 native premerge 为6个直接检查加5个选中检查，全部通过、无人工hold，包含真实 CLI 输出预算、维护性、双语RFC和模块边界检查。首次探针误用不存在的 python -m loopx，改为当前 source console；最初 provider-off 结果未当作校验成功，而是在只启用隔离本地 provider 后重做。一次 inventory --check 调用缺少 output 参数，保留该调用并改用文档指定的 semantic smoke。没有修改活动 Goal 或全局安装，也未查询或等待 CI。
剩余风险是新公开字段可进入后续日志、状态或 Agent 上下文，private_provider_payload_captured=false 不会消除已输出的私有请求文本。真实飞书发送、金融执行、用户安装采纳及长期效果均未验证，也不是证明这个本地回归所需的前置条件。
我的整体评价
结论是 REQUEST_CHANGES。具体诊断能减少无效排查，现有 envelope 与 owner 复用也合理；但固定短消息与用户可控消息尚未分开，真实 CLI 已否证当前安全承诺。请修复该局部公开边界并加入上述反例，再针对新准确 head 复评。未来改动的简化检查已应用于这个边界：优先明确安全原因的单一 owner，不再扩充字符黑名单，不要求额外框架或无关迁移。此次不合并、不撤销其他审查，也不把全绿 smoke 当作隐私证明。
English verdict: REQUEST_CHANGES - da7b835b844ef524ce7a521ffa931d3753c2eae5 adds useful diagnostics but reflects an arbitrary unknown request key into public validation_reason. Reproduced through the real isolated CLI against the immutable base; 41 operation tests and native premerge (6 direct + 5 selected) pass but miss this counterexample. Keep safe fixed diagnostics and fall back for untrusted text.
| code-authored phrase, and fall back to the exception class otherwise. | ||
| """ | ||
|
|
||
| text = str(exc).strip() |
There was a problem hiding this comment.
[P2] 仅公开明确安全的固定校验原因
New head publishes request-derived text while claiming a public-safe packet; the same base input does not.
最小修复：在既有错误 owner 区分固定公开原因与动态详情；仅输出明确允许的固定消息或类型化安全代码，对未知文本回退，并补完整 CLI 回归。
|
The [P2] on publishing request-derived text is addressed in #6743 (merged head predates the fix): the packet now carries only a fixed |
1 participant
What
loopx goal-channel prepare-operationmaps everyValueErrorfrom the typed-operation owner to:{"ok": false, "blocker": "invalid_configuration", "public_summary": "the Goal Channel configuration is invalid"}Real causes such as
operation executor binding is invalid,operation payload_digest does not match payload,operation projection fields are unavailableortyped operation proposal is unavailableall collapse into that single line, so an operator (or an agent driving the CLI) cannot tell a malformed request from a broken channel binding. Reproduced locally while preparing a typed finance operation: the CLI reportedinvalid_configurationwhile the underlying normalizer raisedoperation executor binding is invalid.Change
blockerand summary for compatibility.details.validation_reason: the code-authoredValueErrormessage, truncated at 240 characters./,\\,~, control characters or are empty collapse to the exception class name, so private paths never reach the packet.Verification
No behavior change for successful prepare/deliver/inspect/consume/report paths; the packet only gains a
detailsfield on this error branch.