feat(agent): add governed analysis protocol runtime#79
Conversation
bb68ec2 to
90e9b8d
Compare
iancaoo
left a comment
There was a problem hiding this comment.
代码审查:针对「正确性」与终态收尾路径的若干问题。以下为行内评论。
| runId, | ||
| agentAssembly.protocol.segmentId | ||
| ); | ||
| if (protocolState.protocolId === "general-task" && lastAssistantMessageId) { |
There was a problem hiding this comment.
【正确性】无助手文本的 general-task 运行会被收尾为 partial
这里用 lastAssistantMessageId 合成 general.answer.commit,但 lastAssistantMessageId 仅在 TEXT_MESSAGE_* 事件上更新(见 assistantMessageIdFromEvent)。两类场景会漏掉它:
- 仅靠工具输出就到达
RUN_FINISHED、未产生任何助手文本的运行; - 文本是在前一个 segment(切换前)发出、当前 segment 无新文本就结束的运行。
此时 reduceGeneralTaskAction 因 messageId 为空而不设置 answerMessageId,completionPolicy 返回 continue,forceTerminal 把它强制成 partial —— 即使该运行已产出有效产物/文件。
请确认这是有意为之;如不是,建议在 lastAssistantMessageId 缺失时回退到 metadata store 中该 run 的最新助手消息,而不是依赖运行内可变变量。
| return state; | ||
| } | ||
| const messageId = recordString(result, "messageId"); | ||
| return messageId ? { ...state, answerMessageId: messageId } : state; |
There was a problem hiding this comment.
【正确性】空 messageId 时静默不更新状态
这行是上一条 (server.ts 合成 commit) 的根因落点:当 messageId 为空时直接 return state,answerMessageId 不变。结合 completeProtocolRun 的 forceTerminal,等价于「无助手文本 → partial」。
如果「无文本也允许 completed」是预期行为,应在此处或 completionPolicy 显式处理空消息场景;否则建议对空 messageId 抛错或记一条 GENERAL_ANSWER_MESSAGE_MISSING,避免静默降级为 partial。
| return false; | ||
| } | ||
| const filePath = directString(input, "path") ?? directString(input, "filename") ?? ""; | ||
| return /\.md$/iu.test(filePath.trim()); |
There was a problem hiding this comment.
【正确性】报告提交门仅拦截 .md,可被扩展名绕过
assertRequirementsCommittedBeforeReport 仅在 isMarkdownFileAction 为真时触发,而判定仅匹配 /\.md$/。模型可把最终报告写成 .txt / .html / .markdown,或用带尾斜杠/空白的路径,从而绕过「未提交声明不得写报告」这一门。
策略明确要求「最后两步留给 task_check 与结束语」,非 .md 扩展名不应打开绕过通道。建议改用更宽的「报告写入」信号(例如 synthesis 阶段内的任意 write_file/edit_file,或内容标记),而非仅扩展名。
| const latestContextPackageRef = ( | ||
| current: ContextPackageRef, | ||
| candidate: ContextPackageRef | ||
| ): ContextPackageRef => current.packageId === candidate.packageId && current.revision > candidate.revision |
There was a problem hiding this comment.
【正确性】revision 相等时静默切换到 candidate
current.revision > candidate.revision ? current : candidate —— 当 revision 相等时返回 candidate。若工具返回了一个 revision 相同但 packageId 不同(可能已过期)的 context package 引用,运行时会静默采用它。
鉴于引用是服务端生成,可能无害,但相等情形值得显式 packageId 检查,或改用 >= 以保留 current,避免在等价修订下被替换。
| ...(input.contextPackageExists ? { contextPackageExists: input.contextPackageExists } : {}), | ||
| onEvent: (event) => { | ||
| if (!protocolEventsReady) { | ||
| deferredProtocolEvents.push(event); |
There was a problem hiding this comment.
【正确性】延迟事件缓冲的确认路径需防重放
在 protocolEventsReady 之前,onEvent 返回 false(未确认)并缓冲到 deferredProtocolEvents;flushProtocolEvents 再重放并调用 protocol.acknowledgeEvent。该路径本身自洽。
但 server.ts 中的 replayPendingProtocolEvents 在 createRunAgentAssembly 之前运行,直接通过 emit 推流。若某 run 被恢复,且边界自身启动路径又重放了任何未确认事件,就会出现重复发出。
恢复时边界会跳过 startEvents(仅 !persistedState 时设置),所以应当安全 —— 但这是一个微妙的不变式,建议补一条测试:恢复时不会重新发出已入日志(已确认)的事件。
| agentAssembly.protocol.segmentId | ||
| ); | ||
| } | ||
| const terminalState = agentAssembly.protocol.protocolRuntime.proposeCompletion({ |
There was a problem hiding this comment.
【健壮性】completeProtocolRun 在 answer 阶段调用 proposeCompletion 可能死锁式失败
若 general.answer.commit 在协议已处于 answer 阶段时被调用,assertActionAllowedInState 会因 answer 阶段 allowedActions: [] 抛 ACTION_NOT_ALLOWED_IN_PHASE,completeProtocolRun 随即抛错,收尾失败 → RUN_ERROR。
响亮失败本身可接受,但因 completeProtocolRun 是终态路径,请确认该异常会经 finalization.then(..., (error) => subscriber.error(error)) 暴露为干净的 RUN_ERROR,而非未处理的 Promise 拒绝。建议补一条测试覆盖此路径。
No description provided.