From a1bdd68b50451ad3aaa2b314a8cc19a2af97d703 Mon Sep 17 00:00:00 2001 From: Jay Shen Date: Sun, 6 Sep 2026 23:29:20 +0800 Subject: [PATCH] fix: close real-model planning feedback and review-order gaps --- .../luowang-scenario-design-quality/plan.md | 29 ++- .../luowang-scenario-design-quality/spec.md | 10 +- resources/agent-roles/main-planning.md | 15 +- resources/agent-roles/reviewer-audit.md | 8 +- src/server/repository/git-repository.ts | 13 +- src/server/repository/scenario-patch.ts | 4 +- src/server/runs/agent-session.ts | 5 +- src/server/runs/orchestrator.ts | 74 +++++++- src/server/runs/review-order.ts | 58 ++++++ tests/acceptance/local-model-protocol.ts | 7 +- .../acceptance/scenario-evaluation-status.ts | 32 ++++ tests/closure6-production-pi.test.ts | 16 +- tests/phase3.test.ts | 55 +++++- tests/scenario-quality-feedback.test.ts | 166 ++++++++++++++++++ 14 files changed, 459 insertions(+), 33 deletions(-) create mode 100644 src/server/runs/review-order.ts create mode 100644 tests/acceptance/scenario-evaluation-status.ts create mode 100644 tests/scenario-quality-feedback.test.ts diff --git a/docs/changes/luowang-scenario-design-quality/plan.md b/docs/changes/luowang-scenario-design-quality/plan.md index 0c813a3..2f9f9cb 100644 --- a/docs/changes/luowang-scenario-design-quality/plan.md +++ b/docs/changes/luowang-scenario-design-quality/plan.md @@ -1,7 +1,7 @@ # 场景维护与初始化质量改进 Plan - 日期：2026-09-05 -- 状态：审核发现的四项工程缺口已修复并通过回归；Phase 5 真实模型质量对比未运行（blocked） +- 状态：首轮 Qwen 72 Session 矩阵已运行但未通过质量验收；真实输出反馈修复与复评进行中 - 依据：[intent.md](./intent.md)、[spec.md](./spec.md) 实施分支证明（2026-09-05）：分支为 `feat/scenario-design-quality`，基于 @@ -27,6 +27,23 @@ 命令日志保存在本机 `.cynos/acceptance/sdq-review/fix-quality.log`、`fix-acceptance.log`；未运行真实模型或外部官网写入。本次证明仅关闭上述四项工程缺口，不宣称 Phase 5 已完成或真实模型质量已提升。 +## 真实模型反馈修复（2026-09-06） + +用户授权使用 Pi 中显式选择的 `aliyun-coding-plan/qwen3.7-plus`，Thinking off，仅作离线设计比较，不加载用户扩展/Skills，不切换按量接口或发布目标仓库。首轮冻结 `09dbed0` / `d5c4d00` 两版本和 `cynos-ai/cynos-website@46f971a65b9a28a4961ee7f9bfbf27eb21ca8be7` 派生夹具，完成 72 次 Session 尝试（893 次请求）；此前 pilot 单独保留。初始化使用固定合成侦察，Reviewer 使用明确标记的合成证据，不代表现场官网测试。 + +原始汇总为 58 completed / 14 failed，但不等于质量通过率：两例请求超时后恢复产出仍被驱动标为 failed；新版 17 个 completed 普通规划中五例有 draft/deprecated 清单或无效 patch。12 个初始化尝试均未形成可信完整交接；新版 Reviewer 有一次必需覆盖错误放行和一次提前读取草稿。完整日志、冻结规则及 109 次本地 patch 重放、24 份新版计划检查见 `.cynos/acceptance/sdq-evaluation/{full-output,audit}/`，不改写旧结果，独立人工最终评分仍未运行。 + +修复分支 `fix/scenario-quality-feedback` 从最新 `origin/develop@d5c4d00` 建立，范围为： + +- 完整五字段示例、ID/状态语义、完整 patch 与末尾换行约定，以及限长/不泄露 stderr 的校验提示。 +- Main 在同一 Session 内最多两次接收联合校验反馈；复用既有 patch、工作树和执行清单 validator，结束清理临时应用，不增加角色或 Session。 +- Reviewer 工具约束计划/原始图片/草稿顺序，图片失败保留 blocked；明确执行清单不能豁免本批必需覆盖。 +- 评测驱动区分请求恢复、角色终态、工件检查、质量判断；拒绝的读取请求不算实际读到内容，旧结果及失败全部保留。 + +本轮工程验证：Docker quality 镜像中 format/lint/typecheck 及 26 文件 162/162 tests 通过；local acceptance（包含 build、headless E2E、生产 Pi 专项）通过，live/release 仍 blocked。最终证据为 `.cynos/acceptance/sdq-evaluation/audit/fix-release-check.log` 和 `.cynos/acceptance/sdq-feedback-quality/2026-09-06T15-25-43-681Z-local/report.json`，该 acceptance 覆盖 162-test 全量回归、联合修正反馈、Reviewer 顺序约束及评测状态分类。测试使用 lockfile 匹配的既有 quality 镜像并挂载当前发布源码/测试/角色资源/文档/CI，不冒称已构建新镜像。宿主机缺少 SQLite native binding，因此最终以容器结果为准；初次挂载遗漏 CI workflow、误把被忽略评测脚本纳入 lint 的失败日志也保留。 + +Qwen 复评在工程回归后冻结新 commit，沿用 `full-manifest.json` 的 12 类夹具、评分规则、原始基线 `09dbed0`、模型和 Thinking，每例每版三次。结果另存 `retest-output/`，不修改首轮 `full-output/`；本轮请求上限 1200、单例 24，遇额度/权限错误停止，不切按量接口。复评驱动及状态分类单测分开记录请求恢复、角色终态、工件有效性和被拒绝读取；独立人工质量评分仍待完成，不把修复后的模板示例或评分答案注入夹具，也不预先宣布质量验收通过。 + ## 1. 实施原则与顺序 本计划以 `09dbed0` 为代码检查基线。实现时从最新 `develop` 建立 `feat/scenario-design-quality`，若下列 owner 已有增量修改，先核对调用关系并在原职责边界内集成，不另起一套规划或场景系统。 @@ -188,7 +205,7 @@ Docker 证明使用 Dockerfile/CI 中的 pinned Node digest、npm 镜像、Playw 脱敏评测输入清单、版本标识、全部输出、逐例复核和对比结果保存于 `.cynos/acceptance/`，分别标注规划与 Reviewer 的证据，在本计划补充非敏感证据位置和实际状态。AC-SDQ-09 的真实质量证明必须来自固定工件的真实 Reviewer 输出和读取记录；未运行这组评测时，即使规划对比或本地流程通过，该 AC 仍保持未完成。凭据、预算或外部条件缺失时记录未运行/受阻及原因，Phase 5 保持未完成，不用本地协议模拟出质量通过。 -实际状态：Phase 5 未运行（`not_run/blocked`）。`npm run test:acceptance:live` 在输入检查阶段退出码 1；固定 `cynos-ai/cynos-website` 的不可变快照、非生产 URL/合成账号、GitHub/Provider/OSS 受控凭据、视觉 Reviewer 条件以及明确模型调用预算均未提供。本地 acceptance 报告中的工程 proof 全部通过，但 AC-SDQ-04/05/06/09/12 的真实模型部分仍为 `not_run`；没有基线/新版八类输入三次重复计数、Reviewer 固定工件读取记录或人工复核结论，因此本变更不声称真实场景设计质量已经改善。 +实际状态：已完成首轮真实 Qwen 72 Session 离线矩阵，发现质量及评测统计缺口（见本文“真实模型反馈修复”）；不是现场 live Run 验收。修复后复评和独立人工计数未完成，Phase 5 保持未通过，不声称稳定质量改善。原先因输入不足退出的 live 验收记录是历史事实，不再作为本轮离线模型评测未运行的理由。 对应：AC-SDQ-04、AC-SDQ-05、AC-SDQ-06、AC-SDQ-09 的实际输出质量，以及 AC-SDQ-12。 @@ -209,11 +226,11 @@ Docker 证明使用 Dockerfile/CI 中的 pinned Node digest、npm 镜像、Playw | 验收项 | 实施/证明阶段 | 当前状态 | | ------------------------------- | ------------- | -------- | | AC-SDQ-01、AC-SDQ-02、AC-SDQ-03 | Phase 1、4 | 工程通过 | -| AC-SDQ-04、AC-SDQ-05 | Phase 2、4、5 | 工程通过；真实模型部分未运行 | -| AC-SDQ-06、AC-SDQ-09 | Phase 3、4、5 | 工程通过；真实模型部分未运行 | +| AC-SDQ-04、AC-SDQ-05 | Phase 2、4、5 | 首轮真实模型有缺口；修复复评中 | +| AC-SDQ-06、AC-SDQ-09 | Phase 3、4、5 | 首轮真实模型有缺口；修复复评中 | | AC-SDQ-07、AC-SDQ-10 | Phase 3、4 | 工程通过 | | AC-SDQ-08 | Phase 2、3、4 | 工程通过 | | AC-SDQ-11 | Phase 2、3、4 | 工程通过 | -| AC-SDQ-12 | Phase 4、5 | 工程/本地通过；真实评测未运行 | +| AC-SDQ-12 | Phase 4、5 | 首轮矩阵已运行；最终人工计数未完成 | -PR #56 已合入 `develop`，其 Phase 0–4 原工程证明存在的四项遗漏已在上述修复分支完成回归，修复合并状态以 PR 为准；Phase 5 因外部输入缺失保持 `not_run/blocked`。即使工程 PR 已合并，缺少真实质量证据时也不能宣称提示词或场景设计质量已提升。 +PR #56 和四项工程遗漏修复 PR #58 已合入 `develop`；本轮真实反馈修复尚在独立分支。Phase 5 已运行首轮矩阵但未通过质量验收，修复复评与独立人工评分待完成，不因工程检查通过就宣称场景设计质量已提升。 diff --git a/docs/changes/luowang-scenario-design-quality/spec.md b/docs/changes/luowang-scenario-design-quality/spec.md index b98d9fe..c6ea9cd 100644 --- a/docs/changes/luowang-scenario-design-quality/spec.md +++ b/docs/changes/luowang-scenario-design-quality/spec.md @@ -84,7 +84,7 @@ Main 按“固定范围 → 理解契约 → 查看变化与相关调用关系 沿用默认布局中的模板，文件平铺在 `docs/scenario-testing/scenarios/`，frontmatter 仅包含 `id`、`name`、`description`、`status`、`tags`。新增 ID 使用现有解析器允许的大写字母、数字和连字符。 -内置指令提供一份完整的最小模板和少量正反例，正文包括目的、前置条件、步骤、期望、需要记录。字段名固定，name、description、正文和普通标签值遵循配置语言；`core` 等约定标签不翻译。 +内置指令提供一份包含全部五字段 frontmatter 的完整最小模板和少量正反例；即使 target 没有场景，也不得要求模型猜字段。schema 错误反馈明确完整字段、类型和 ID 规则。patch 必须以换行结尾，格式错误只返回脱敏定位和修复提示，不暴露原始 Git stderr，也不自动改写 patch；每次提交替换整个 patch。正文包括目的、前置条件、步骤、期望、需要记录。字段名固定，name、description、正文和普通标签值遵循配置语言；`core` 等约定标签不翻译。 ### 5.2 质量要求 @@ -143,15 +143,17 @@ Harness 依据场景变更的审批状态和更新后的执行清单分流，不 Harness、Runner 和进度工具共同使用该区域，其他区域的引用、排除理由、历史记录和 draft 候选不构成选中。拒绝重复标题、重复 ID、不存在 ID、deprecated/draft ID 和与已应用场景不一致的清单；沿用工件错误处理，不回退为全文 ID 搜索。初始侦察计划无需声明正式清单；候选 Main 负责补齐。 +日常规划和初始化候选 Main 结束时联合核对实际 patch、应用后场景状态和清单；临时应用只用于固定 target 校验，结束必须清理，不发布。错误在原隔离 Session 内反馈，最多两次修正提示，仍不一致则失败，不增加 Session 或用旧工件通过。初始静态侦察不强制最终清单。 + 声明、开始、完成与最终场景结果必须对应清单，不能只执行任意子集却报告全部通过。缺失执行按现有阻塞/错误语义处理。旧 Run 只读展示保持兼容，不回写历史工件。 ### 7.2 Reviewer 的可复核输入 计划为每个选中场景保留角色、前置、关键步骤、明确期望、依据定位和预期证据的必要摘要，使 Reviewer 在既有工具边界内可核对。新建/修改场景同时以本次 patch 核对，缺少必要摘要时不能自行补造。 -Reviewer 先读固定计划与期望、存在的场景 patch 和原始证据，再对照 Runner 执行记录与草稿。统一角色指令与 Orchestrator 输出契约的顺序，避免“先读草稿”和“原始证据优先”同时出现。 +Reviewer 先读固定计划与期望、存在的场景 patch 和原始证据，再对照 Runner 执行记录与草稿。统一角色指令与 Orchestrator 输出契约的顺序，避免“先读草稿”和“原始证据优先”同时出现。工具在当前 Reviewer Session 内拒绝先于 plan/存在的场景 patch 的图片读取，以及先于 plan/存在的场景 patch/本次已上传原始图片核对的 execution/draft 读取和审核提交。被拒绝的调用不暴露内容、不算已读取；图片尝试失败成为阻塞事实，但允许读取后续工件以如实报告失败；无图片的 Run 不要求不存在的图片。 -审核增加场景设计检查：断言依据是否独立于实现、是否因失败放宽期望、是否漏掉已识别核心能力、是否重复或错误合并场景、执行是否符合清单。不能仅因 patch 路径和 YAML 合法就确认质量，也不能越权补测或读取其他 Session 对话。 +审核增加场景设计检查：断言依据是否独立于实现、是否因失败放宽期望、是否漏掉已识别核心能力、是否重复或错误合并场景、执行是否符合清单。执行清单只限制实际执行，不豁免已识别的本批必需能力；必需能力遗漏或计划矛盾未消解时，已执行场景可单独 passed，但整体必须 blocked。不能仅因 patch 路径和 YAML 合法就确认质量，也不能越权补测或读取其他 Session 对话。 ## 8. 指令组织与质量评测 @@ -171,7 +173,7 @@ Reviewer 先读固定计划与期望、存在的场景 patch 和原始证据， Reviewer 对比另设固定工件案例，至少分别包含无依据断言、已识别核心能力覆盖遗漏、计划与执行结果不一致，以及一例依据和执行一致的无错误对照。每例冻结计划、必要的场景 patch、原始证据、execution 和 draft-report；两版本使用同一组输入，不能分别使用各自规划输出而混淆审核能力与上游输入变化。工件与受控证据必须足以独立判定预置问题；预置问题清单和评分答案只供人工复核，不注入 Reviewer 上下文。通过生产 Reviewer Session、实际工件/证据读取工具和 `write_review` 收集审核结果及读取顺序，不扩大 Reviewer 权限，也不运行目标环境操作。 -规划与 Reviewer 的每个案例每个版本均重复三次，保留全部输出，不能挑选最好的一次。由人工按同一标准复核，模型评分仅作辅助。规划比较无依据断言、遗漏必需风险、语义重复、不必要修改、不可执行场景及计划不一致的次数；Reviewer 单独比较已知问题漏检、无证据误报（包括无错误对照）及读取顺序违约的次数。两组均记录工具调用、耗时和可取得的 token 用量，不把规划输出质量当作 Reviewer 检出能力。 +规划与 Reviewer 的每个案例每个版本均重复三次，保留全部输出，不能挑选最好的一次。由人工按同一标准复核，模型评分仅作辅助。规划比较无依据断言、遗漏必需风险、语义重复、不必要修改、不可执行场景及计划不一致的次数；Reviewer 单独比较已知问题漏检、无证据误报（包括无错误对照）及读取顺序违约的次数。两组均记录工具调用、耗时和可取得的 token 用量，不把规划输出质量当作 Reviewer 检出能力。请求瞬态错误及恢复、Session 终态、工件有效性、质量评分和预算中止分开记录；HTTP 层零重试不等于 SDK Session 不重试。拒绝的工具请求不算实际内容读取，原始失败和旧汇总保留，不静默重分类成质量通过。 工程正确性与质量效果分开报告：权限/固定范围/交接等硬错误必须为零；规划与 Reviewer 的质量项分别逐项报告基线与新版计数，均纳入改善判断。声称“质量改善”需至少一项质量错误总数下降，其余不增加，并解释逐案例退步；若基线已无错误，只能报告维持质量。未达到条件则记录未证明改善，修订后重评，不以流程通过代替。 diff --git a/resources/agent-roles/main-planning.md b/resources/agent-roles/main-planning.md index 2a52b5a..e864aec 100644 --- a/resources/agent-roles/main-planning.md +++ b/resources/agent-roles/main-planning.md @@ -42,9 +42,18 @@ ## 场景写作最小模板 -每个场景围绕一个可判断的业务结果，frontmatter 只使用既有五字段。正文至少包含：目的、前置条件（角色/初始状态/合成数据/依赖）、可重复步骤、可观察的关键期望、每条关键期望的决定性证据、Run 标记数据的登记/清理/删除后核验。不要把 Run ID、临时地址、Secret、DOM ref 或命令输出写进长期场景；`approved` 只表示期望依据明确，不表示产品当前已经通过。 +每个场景围绕一个可判断的业务结果，frontmatter 必须且只能包含 `id`、`name`、`description`、`status`、`tags`。`id` 使用大写字母/数字和连字符（如 `AUTH-LOGIN-001`），不是小写 slug；`name` 是非空场景名称，不是文件名；`description` 非空；`tags` 是字符串数组，可为 `[]`。不要添加 title、owner、version、时间戳或置信度字段。正文至少包含：目的、前置条件（角色/初始状态/合成数据/依赖）、可重复步骤、可观察的关键期望、每条关键期望的决定性证据、Run 标记数据的登记/清理/删除后核验。不要把 Run ID、临时地址、Secret、DOM ref 或命令输出写进长期场景；`approved` 只表示期望依据明确，不表示产品当前已经通过。 ```markdown +--- +id: AUTH-LOGIN-001 +name: 登录后恢复当前用户状态 +description: 验证有依据的登录与会话恢复契约 +status: approved +tags: + - core +--- + ## 目的 <一个完整业务结果> @@ -77,7 +86,9 @@ - SCENARIO-ID-002 ``` -若确有依据无需测试，区域中不得列 ID，必须写“无需场景测试”及理由。场景 patch 必须是标准 git unified patch。有场景变更时，在当前计划的 `## scenario_review_summary` 下用生成语言简述候选范围、依据和覆盖缺口；只写必要业务事实，不含代码块、原始 diff、Secret、临时路径或证据地址。人工审核特殊报告保留此摘要，不复制整份计划。 +若确有依据无需测试，区域中不得列 ID，必须写“无需场景测试”及理由。场景 patch 必须是完整的标准 git unified patch，最后一行必须有换行符；不要输出代码围栏，不猜测或反复修改 blob hash。每次 writer 替换整个 patch，不是追加，不能逐文件覆盖丢失先前候选。收到格式错误时按行号和 schema 修正，不把字段报错当作目录权限问题。 + +执行清单只能列出应用 patch 后实际存在且为 `approved` 的场景。新场景有已确认契约依据时可直接设 approved，不必等产品运行通过；不能把有明确依据的场景全部设 draft 再安排执行。期望确实不明时保持 draft 并记录必需覆盖缺口；已废弃场景保留文件但从执行清单移除，必要替代验证用有依据的 approved 场景。结束前核对清单和完整 patch；收到 Harness 联合校验错误后在同一 Session 修正两者，不能通过删掉必需能力或虚称无需测试规避错误。有场景变更时，在当前计划的 `## scenario_review_summary` 下用生成语言简述候选范围、依据和覆盖缺口；只写必要业务事实，不含代码块、原始 diff、Secret、临时路径或证据地址。人工审核特殊报告保留此摘要，不复制整份计划。 ## 失败规则 diff --git a/resources/agent-roles/reviewer-audit.md b/resources/agent-roles/reviewer-audit.md index 74be10e..1ae6e87 100644 --- a/resources/agent-roles/reviewer-audit.md +++ b/resources/agent-roles/reviewer-audit.md @@ -15,16 +15,18 @@ ## 顺序 -1. 先读取计划、唯一执行清单、固定场景期望和 Harness 阻塞事实。 +1. 先读取计划、唯一执行清单、固定场景期望、存在的场景 patch 和 Harness 阻塞事实。 2. 再独立读取原始命令/API/截图/清理证据，判断其是否与当前 Run、场景和断言对应。 3. 最后读取执行记录和 Runner 草稿，检查遗漏、偏差、证据链和 Bug 判断。 4. 核对场景 patch 边界、清单完整性、零场景理由、confirmed Bugs 和测试数据清理。 -5. 审核断言是否有独立依据、是否因失败放宽期望、是否漏掉已识别核心能力、是否错误重复或合并场景。 +5. 审核断言是否有独立依据、是否因失败放宽期望、是否漏掉已识别核心能力、是否错误重复或合并场景。`execution_scenarios` 只限制 Runner 实际允许执行哪些场景，不证明计划覆盖完整。计划或已确认依据要求本批验证的必需能力，即使没有列入清单，也必须检查并指出遗漏；不能将明确的必需能力降格成背景文字。 + - 已执行场景可单独 passed，但只要已识别的本批必需结果缺少验证，整体必须 blocked，不能因清单已执行完而整体 passed。 + - 若计划要求与清单矛盾尚未消解，维持 blocked；不能自行缩小范围、修改要求或越权补测。 6. 写入明确同意或拒绝最终结果的 `review.md`。 ## 输出契约 -审核必须列出已读取的决定性证据、辅助证据、无法复核项、偏差、清理结论和建议结果；不能仅复述 Runner 草稿。 +审核必须列出已读取的决定性证据、辅助证据、无法复核项、偏差、清理结论和建议结果；不能仅复述 Runner 草稿。先读取 plan.md 和存在的 scenario-changes.patch，再通过受控工具实际读取本次原始图片，最后才可读取 execution.md/draft-report.md 或提交审核；提前读取会被拒绝，拒绝不等于已经读到内容。图片读取失败时记录 blocked，不能当作无证据需求。 ## 失败规则 diff --git a/src/server/repository/git-repository.ts b/src/server/repository/git-repository.ts index 862ba2d..a2a1599 100644 --- a/src/server/repository/git-repository.ts +++ b/src/server/repository/git-repository.ts @@ -882,6 +882,11 @@ export class GitRepository { patch: string, ): Promise { const metadata = validateScenarioPatchText(patch); + if (!patch.endsWith('\n')) { + throw new ScenarioPatchError( + 'patch 缺少末尾换行：请在完整 unified diff 最后一行后添加换行再提交；Harness 不自动改写待发布内容', + ); + } const baseFiles = await this.readScenarioFilesAtCommit(baseCommit); const patchDirectory = await mkdtemp(join(tmpdir(), 'luowang-scenario-patch-')); const patchPath = join(patchDirectory, 'changes.patch'); @@ -892,7 +897,13 @@ export class GitRepository { await this.run(['apply', '--recount', '--whitespace=nowarn', patchPath]); } catch (error) { if (error instanceof GitCommandError) { - throw new ScenarioPatchError('patch 无法干净地应用到固定 target，未发布任何部分变更'); + // Never expose raw Git stderr: it can contain patch text, secrets or local paths. + const line = error.stderr.match( + /(?:corrupt patch at line |patch fragment without header at line )(\d{1,8})\b/, + )?.[1]; + throw new ScenarioPatchError( + `patch 无法干净地应用到固定 target，未发布任何部分变更；${line ? `diff 第 ${line} 行格式错误，` : ''}请核对完整 hunk、上下文及末尾换行；不要猜测 blob hash 或目录权限`, + ); } throw error; } diff --git a/src/server/repository/scenario-patch.ts b/src/server/repository/scenario-patch.ts index 50e91cb..d5eb779 100644 --- a/src/server/repository/scenario-patch.ts +++ b/src/server/repository/scenario-patch.ts @@ -128,7 +128,9 @@ export function validateScenarioContents( scenario = parseScenarioMarkdown(content, path); } catch (error) { if (error instanceof RepositoryError) { - throw new ScenarioPatchError(error.message); + throw new ScenarioPatchError( + `${error.message.slice(0, 1000)}；场景 frontmatter 必须且只能包含 id、name、description、status、tags：前三项非空，id 为大写字母/数字和连字符组成的稳定 ID，status 为 draft/approved/deprecated，tags 为字符串数组（可为空）；不要把 name 当作文件名`, + ); } throw error; } diff --git a/src/server/runs/agent-session.ts b/src/server/runs/agent-session.ts index 6816429..10fc957 100644 --- a/src/server/runs/agent-session.ts +++ b/src/server/runs/agent-session.ts @@ -137,7 +137,10 @@ export function createArtifactWriterTool( ): ToolDefinition { const parameters = Type.Object({ content: Type.String({ - description: '要写入的完整 Markdown 文本', + description: + name === 'write_scenario_patch' + ? '要原样保存的完整标准 git unified diff（不是 Markdown 围栏），最后一行必须以换行符结束；本次内容替换整个 patch' + : '要写入的完整 Markdown 文本', maxLength: 4 * 1024 * 1024, }), }); diff --git a/src/server/runs/orchestrator.ts b/src/server/runs/orchestrator.ts index a6799a5..5122198 100644 --- a/src/server/runs/orchestrator.ts +++ b/src/server/runs/orchestrator.ts @@ -30,7 +30,7 @@ import { supportsVision, type BrowserMcpAdapter, } from '../browser/playwright-mcp.js'; -import { createOssAdapter, type OssAdapter } from '../storage/oss.js'; +import { contentTypeFor, createOssAdapter, type OssAdapter } from '../storage/oss.js'; import { buildSessionInput, createArtifactWriterTool, @@ -52,6 +52,7 @@ import { createProviderAdapter, type ProviderAdapter } from './provider.js'; import { createIssueCandidateController, createRunHistoryTool } from './run-history.js'; import { createScenarioProgressController, type ProgressScenario } from './scenario-progress.js'; import { scenarioReviewSummary } from './scenario-review-summary.js'; +import { createReviewReadOrder } from './review-order.js'; import { assertScenarioResultsMatchPlan, ExecutionPlanError, @@ -563,7 +564,10 @@ class DefaultRunOrchestrator implements RunOrchestrator { 'write_scenario_patch', '写入场景变更 patch', '只写入 docs/scenario-testing/scenarios/** 范围内的标准 git unified patch；不能直接修改目标仓库或其他目录。', - (content) => workspace.writer('main-a').writeScenarioPatch(content), + async (content) => { + await repository.validateScenarioPatch(context.targetCommit, content); + await workspace.writer('main-a').writeScenarioPatch(content); + }, ), ]), ]; @@ -576,6 +580,10 @@ class DefaultRunOrchestrator implements RunOrchestrator { mainAUserMessage(context), mainAOutputContract(context), context.initialization, + [], + context.initialization + ? undefined + : () => this.validatePlanningOutput(workspace, repository, context), ); await assertArtifact(workspace, 'plan.md'); if (!context.initialization) await parseExecutionScenarioPlan(await workspace.read('plan.md')); @@ -640,6 +648,7 @@ class DefaultRunOrchestrator implements RunOrchestrator { '只写入 docs/scenario-testing/scenarios/** 范围内的标准 git unified patch；不能直接修改目标仓库或创建 suite、catalog、journey 或能力图文件。', async (content) => { patchWriteAttempted = true; + patchWriteSucceeded = false; try { await repository.validateScenarioPatch(context.targetCommit, content); } catch (error) { @@ -662,6 +671,14 @@ class DefaultRunOrchestrator implements RunOrchestrator { initializationCandidateUserMessage(context), initializationCandidateOutputContract(), true, + [], + async () => { + if (!planWriteSucceeded) + throw new ExecutionPlanError('候选 Main 未成功更新 plan.md，不能使用旧静态计划'); + if (patchWriteAttempted && !patchWriteSucceeded) + throw new ExecutionPlanError('最后一次候选 patch 未成功写入，必须修正后重新提交'); + await this.validatePlanningOutput(workspace, repository, context); + }, ); if (!planWriteSucceeded) { throw new RunOrchestratorError( @@ -679,6 +696,24 @@ class DefaultRunOrchestrator implements RunOrchestrator { parseExecutionScenarioPlan(await workspace.read('plan.md')); } + private async validatePlanningOutput( + workspace: RunWorkspace, + repository: GitRepository, + context: RunContext, + ): Promise { + const patch = await readOptionalScenarioPatch(workspace); + try { + await repository.checkoutTarget(context.targetCommit); + const scenarioChanges = + patch === undefined + ? undefined + : await repository.applyScenarioPatch(context.targetCommit, patch); + await this.progressScenarios(workspace, repository, { ...context, scenarioChanges }); + } finally { + await repository.cleanWorkspace(); + } + } + private async prepareScenarioPatch( workspace: RunWorkspace, repository: GitRepository, @@ -1199,16 +1234,23 @@ class DefaultRunOrchestrator implements RunOrchestrator { evidenceStore: RunEvidenceStore | undefined, ): Promise { this.setPhase(state, 'reviewer', 'Reviewer 正在独立审核执行结果'); - const tools = [ - createReadArtifactTool((name) => + const readOrder = createReviewReadOrder( + (name) => readAllowedArtifact(workspace, name, [ 'plan.md', 'execution.md', 'draft-report.md', 'scenario-changes.patch', ]), - ), - ...(evidenceStore ? createReviewerEvidenceTools(evidenceStore) : []), + context.evidence + .filter((item) => contentTypeFor(item.filename).startsWith('image/')) + .map((item) => item.filename), + () => this.addBlockingReason(context, 'Reviewer 原始图片读取失败，不能确认通过'), + await workspace.exists('scenario-changes.patch'), + ); + const tools = [ + createReadArtifactTool(readOrder.readArtifact), + ...(evidenceStore ? createReviewerEvidenceTools(evidenceStore).map(readOrder.wrap) : []), ...createReviewerTestDataTools( this.options.testData ?? createTestDataManager(), context.runId, @@ -1218,7 +1260,10 @@ class DefaultRunOrchestrator implements RunOrchestrator { 'write_review', '写入独立审核', '写入本次 Run 的完整 review.md。必须独立核对执行证据和零场景判断。', - (content) => workspace.writer('reviewer').writeReview(content), + (content) => { + readOrder.assertReady(); + return workspace.writer('reviewer').writeReview(content); + }, ), ]; await this.invoke( @@ -1348,6 +1393,7 @@ class DefaultRunOrchestrator implements RunOrchestrator { outputContract: string, initialization: boolean, extensionFactories: InlineExtension[] = [], + validateOutput?: () => Promise, ): Promise { let session: AgentSession | undefined; try { @@ -1366,6 +1412,20 @@ class DefaultRunOrchestrator implements RunOrchestrator { ); session = await this.sessions.create(input); await session.prompt(input.userMessage); + if (validateOutput) { + // Keep correction in the same isolated Session; never launch extra roles or loop unboundedly. + for (let attempt = 0; ; attempt += 1) { + try { + await validateOutput(); + break; + } catch (error) { + if (attempt >= 2) throw error; + await session.prompt( + `规划工件联合校验失败：${safeMessage(error)}\n请在当前 Session 修正完整 plan.md/场景 patch 后结束。execution_scenarios 只能引用应用后实际存在的 approved 场景；draft/deprecated 不可执行。不要通过删去必需覆盖来掩盖问题，也不要重复发送未修正工件。`, + ); + } + } + } } finally { if (session) await session.dispose(); } diff --git a/src/server/runs/review-order.ts b/src/server/runs/review-order.ts new file mode 100644 index 0000000..ad5d900 --- /dev/null +++ b/src/server/runs/review-order.ts @@ -0,0 +1,58 @@ +import type { ToolDefinition } from '@earendil-works/pi-coding-agent'; +import { createTextResult } from './agent-session.js'; + +/** Session-local read order. A denied call never exposes the artifact body. */ +export function createReviewReadOrder( + read: (name: string) => Promise, + requiredImages: readonly string[], + onImageFailure: () => void, + requirePatch = false, +) { + let planRead = false; + let patchRead = false; + const attemptedImages = new Set(); + const assertReady = () => { + if (!planRead || (requirePatch && !patchRead)) + throw new Error( + 'Reviewer 必须先读取 plan.md 和存在的 scenario-changes.patch，再核对原始证据', + ); + if (requiredImages.some((name) => !attemptedImages.has(name))) { + throw new Error( + 'Reviewer 必须先通过 read_evidence_image 逐一核对本次原始图片，再读 execution.md/draft-report.md 或提交审核', + ); + } + }; + return { + assertReady, + async readArtifact(name: string): Promise { + if (name === 'execution.md' || name === 'draft-report.md') assertReady(); + const content = await read(name); + if (name === 'plan.md') planRead = true; + if (name === 'scenario-changes.patch') patchRead = true; + return content; + }, + wrap(tool: ToolDefinition): ToolDefinition { + if (tool.name !== 'read_evidence_image') return tool; + return { + ...tool, + async execute(...args) { + if (!planRead || (requirePatch && !patchRead)) + return createTextResult( + 'Reviewer 必须先读取 plan.md 和存在的 scenario-changes.patch，再核对原始图片', + { error: true }, + ); + const filename = (args[1] as { filename: string }).filename; + const result = await tool.execute(...args); + attemptedImages.add(filename); + // Failed evidence remains a blocking fact, but may be described in the review. + if ( + (result.details as { error?: boolean } | undefined)?.error || + !result.content.some((part) => part.type === 'image') + ) + onImageFailure(); + return result; + }, + }; + }, + }; +} diff --git a/tests/acceptance/local-model-protocol.ts b/tests/acceptance/local-model-protocol.ts index f43827b..731aeed 100644 --- a/tests/acceptance/local-model-protocol.ts +++ b/tests/acceptance/local-model-protocol.ts @@ -451,7 +451,12 @@ function nextTool( } if (has('write_review')) { - const unreadArtifact = nextUnreadArtifact(['plan.md', 'execution.md', 'draft-report.md']); + const unreadArtifact = nextUnreadArtifact([ + 'plan.md', + 'scenario-changes.patch', + 'execution.md', + 'draft-report.md', + ]); if (unreadArtifact) return unreadArtifact; if (count('write_review') === 0) { return tool('write_review', { diff --git a/tests/acceptance/scenario-evaluation-status.ts b/tests/acceptance/scenario-evaluation-status.ts new file mode 100644 index 0000000..b724f79 --- /dev/null +++ b/tests/acceptance/scenario-evaluation-status.ts @@ -0,0 +1,32 @@ +/** Request recovery is not the same fact as role completion or artifact quality. */ +export function classifyEvaluationSession(input: { + requestErrors: readonly string[]; + lastModelStopReason?: string; + orchestratorError?: string; + stopReason?: string; + aborted: boolean; +}) { + const terminalModelError = ['error', 'aborted'].includes(input.lastModelStopReason ?? ''); + const failed = Boolean( + input.orchestratorError || input.stopReason || input.aborted || terminalModelError, + ); + return { + sessionStatus: failed ? ('failed' as const) : ('completed' as const), + requestStatus: + input.requestErrors.length === 0 + ? ('clean' as const) + : !terminalModelError && !input.aborted && !input.stopReason + ? ('recovered' as const) + : ('unrecovered' as const), + requestErrorCount: input.requestErrors.length, + reason: + input.orchestratorError ?? + input.stopReason ?? + (input.aborted + ? 'session_timeout' + : terminalModelError + ? `model_${input.lastModelStopReason}` + : undefined), + // Callers must validate artifacts and obtain independent quality scores separately. + }; +} diff --git a/tests/closure6-production-pi.test.ts b/tests/closure6-production-pi.test.ts index 9b6fb34..c0b897c 100644 --- a/tests/closure6-production-pi.test.ts +++ b/tests/closure6-production-pi.test.ts @@ -184,7 +184,7 @@ describe('Closure 6 local production Pi path', () => { assert.equal(result.status, 'failed'); assert.equal(result.result, null); assert.match(result.errorMessage ?? '', /approved 场景未纳入执行清单：ONBOARD-OMITTED-002/); - assertSessionSequence(context.model, ['main-a', 'runner', 'main-a']); + assertSessionSequence(context.model, ['main-a', 'runner', 'main-a'], [1, 1, 3]); }); it('rejects initialization when a modified approved scene is omitted', async () => { @@ -197,7 +197,7 @@ describe('Closure 6 local production Pi path', () => { assert.equal(result.status, 'failed'); assert.equal(result.result, null); assert.match(result.errorMessage ?? '', /approved 场景未纳入执行清单：CORE-STATE-001/); - assertSessionSequence(context.model, ['main-a', 'runner', 'main-a']); + assertSessionSequence(context.model, ['main-a', 'runner', 'main-a'], [1, 1, 3]); }); it('allows unselected draft candidates without treating them as passed', async () => { @@ -500,7 +500,11 @@ tags: }; } -function assertSessionSequence(model: LocalModelProtocol, expected: string[]): void { +function assertSessionSequence( + model: LocalModelProtocol, + expected: string[], + promptCounts = expected.map(() => 1), +): void { assert.deepEqual( model.sessions.map((session) => session.role), expected, @@ -509,9 +513,9 @@ function assertSessionSequence(model: LocalModelProtocol, expected: string[]): v model.sessions.every((session) => session.disposed), true, ); - assert.equal( - model.sessions.every((session) => session.prompts.length === 1), - true, + assert.deepEqual( + model.sessions.map((session) => session.prompts.length), + promptCounts, ); assert.equal(new Set(model.sessions.map((session) => session.id)).size, expected.length); if (model.sessions.some((session) => session.roleInstructionVersions.length > 0)) { diff --git a/tests/phase3.test.ts b/tests/phase3.test.ts index a0bfe15..b380f9b 100644 --- a/tests/phase3.test.ts +++ b/tests/phase3.test.ts @@ -299,6 +299,44 @@ describe('Phase 3 agent run', () => { assert.equal(context.sessions.created.includes('reviewer'), false); }); + it('repairs a rejected candidate plan inside the same Main Session before Runner starts', async () => { + const fixture = await createGitFixture(); + const context = await createRunContext( + fixture, + ['passed', 'passed'], + undefined, + undefined, + '', + '\n', + false, + false, + { + candidate: { repairPlanOnce: true }, + }, + ); + const result = await context.orchestrator.run({ + request: '修正候选清单后再验证', + trigger: 'manual', + initialization: true, + }); + assert.equal(result.status, 'completed', JSON.stringify(result)); + assert.deepEqual(context.sessions.created, [ + 'main-a', + 'runner', + 'main-a', + 'runner', + 'reviewer', + 'main-b', + ]); + assert.equal( + context.sessions.messages.filter((message) => message.includes('规划工件联合校验失败')) + .length, + 1, + ); + assert.doesNotMatch(result.artifacts['plan.md'] ?? '', /INIT-MISSING-001/); + assert.match(result.artifacts['plan.md'] ?? '', /INIT-HOME-001/); + }); + it('isolates repeated Main and Runner Sessions during initialization', async () => { const fixture = await createGitFixture(); const context = await createRunContext(fixture, ['passed', 'passed']); @@ -724,6 +762,7 @@ interface CandidateTestOptions { planWriteFailureOnly?: boolean; patchValidationFailureOnly?: boolean; planScenarioId?: string; + repairPlanOnce?: boolean; patchScenarioId?: string; } @@ -871,7 +910,10 @@ class RecordingSessionFactory implements AgentSessionFactory { assert.equal(rejected.details.error, true); return; } - const planScenarioId = this.candidateOptions.planScenarioId ?? 'INIT-HOME-001'; + const planScenarioId = + this.candidateOptions.repairPlanOnce && !message.includes('规划工件联合校验失败') + ? 'INIT-MISSING-001' + : (this.candidateOptions.planScenarioId ?? 'INIT-HOME-001'); await invokeTool(input, 'write_plan', { content: `# Initialization candidate plan\n\n侦察发现首页入口需要正式验证。\n\n## execution_scenarios\n\n- ${planScenarioId}\n`, }); @@ -887,6 +929,11 @@ class RecordingSessionFactory implements AgentSessionFactory { content: 'not a git patch', }); assert.equal(rejected.details.error, true); + const missingNewline = await invokeTool(input, 'write_scenario_patch', { + content: initializationScenarioPatch(this.candidateOptions.patchScenarioId).trimEnd(), + }); + assert.equal(missingNewline.details.error, true); + assert.match(commandText(missingNewline), /缺少末尾换行/); } await invokeTool(input, 'write_scenario_patch', { content: initializationScenarioPatch(this.candidateOptions.patchScenarioId), @@ -932,7 +979,13 @@ class RecordingSessionFactory implements AgentSessionFactory { content: `# Draft\n\n结果：${outcome}\n`, }); } else if (input.role === 'reviewer') { + const premature = await invokeTool(input, 'read_run_artifact', { + name: 'draft-report.md', + }); + assert.equal(premature.details.error, true); + assert.doesNotMatch(commandText(premature), /# Draft/); await invokeTool(input, 'read_run_artifact', { name: 'plan.md' }); + await invokeTool(input, 'read_run_artifact', { name: 'scenario-changes.patch' }); await invokeTool(input, 'read_run_artifact', { name: 'execution.md' }); await invokeTool(input, 'read_run_artifact', { name: 'draft-report.md' }); await invokeTool(input, 'write_review', { diff --git a/tests/scenario-quality-feedback.test.ts b/tests/scenario-quality-feedback.test.ts new file mode 100644 index 0000000..5c6f72f --- /dev/null +++ b/tests/scenario-quality-feedback.test.ts @@ -0,0 +1,166 @@ +import assert from 'node:assert/strict'; +import { readFile } from 'node:fs/promises'; +import { describe, it } from 'vitest'; +import { Type } from 'typebox'; +import { createReviewReadOrder } from '../src/server/runs/review-order.js'; +import { createTextResult } from '../src/server/runs/agent-session.js'; +import { parseScenarioMarkdown } from '../src/server/repository/markdown.js'; +import { validateScenarioContents } from '../src/server/repository/scenario-patch.js'; +import { classifyEvaluationSession } from './acceptance/scenario-evaluation-status.js'; + +describe('scenario quality feedback', () => { + it('does not count recovered request errors as a failed role', () => { + const status = classifyEvaluationSession({ + requestErrors: ['timeout'], + lastModelStopReason: 'stop', + aborted: false, + }); + assert.equal(status.sessionStatus, 'completed'); + assert.equal(status.requestStatus, 'recovered'); + assert.equal(status.requestErrorCount, 1); + }); + + it('keeps terminal errors and budget interruption distinct from recovery', () => { + const terminal = classifyEvaluationSession({ + requestErrors: ['timeout'], + lastModelStopReason: 'error', + aborted: false, + }); + assert.equal(terminal.sessionStatus, 'failed'); + assert.equal(terminal.requestStatus, 'unrecovered'); + const budget = classifyEvaluationSession({ + requestErrors: [], + lastModelStopReason: 'stop', + stopReason: 'session_request_limit', + aborted: false, + }); + assert.equal(budget.sessionStatus, 'failed'); + assert.equal(budget.reason, 'session_request_limit'); + }); + + it('does not hide invalid role output behind request recovery', () => { + const status = classifyEvaluationSession({ + requestErrors: ['timeout'], + lastModelStopReason: 'stop', + orchestratorError: 'invalid plan', + aborted: false, + }); + assert.equal(status.sessionStatus, 'failed'); + assert.equal(status.requestStatus, 'recovered'); + assert.equal(status.reason, 'invalid plan'); + }); + + it('provides a complete parser-valid template even when the target has no scenarios', async () => { + const role = await readFile('resources/agent-roles/main-planning.md', 'utf8'); + const template = role.match(/```markdown\n([\s\S]*?)```/)?.[1]; + assert.ok(template); + const scene = parseScenarioMarkdown( + template, + 'docs/scenario-testing/scenarios/AUTH-LOGIN-001.md', + ); + assert.equal(scene.status, 'approved'); + assert.deepEqual(Object.keys(scene), ['id', 'name', 'description', 'status', 'tags']); + }); + + it('reports the full schema instead of forcing one-field-at-a-time guesses', () => { + assert.throws( + () => + validateScenarioContents( + new Map([ + [ + 'docs/scenario-testing/scenarios/TEST-001.md', + '---\nid: TEST-001\nstatus: draft\n---\n', + ], + ]), + ), + /name.*frontmatter.*id、name、description、status、tags.*不要把 name 当作文件名/, + ); + }); + + it('denies premature body access and requires every original image before draft or review', async () => { + const exposed: string[] = []; + let imageCalls = 0; + const order = createReviewReadOrder( + async (name) => { + exposed.push(name); + return name; + }, + ['a.png', 'b.png'], + () => assert.fail('unexpected image failure'), + ); + const image = order.wrap({ + name: 'read_evidence_image', + label: 'image', + description: 'image', + parameters: Type.Object({ filename: Type.String() }), + async execute() { + imageCalls += 1; + return { content: [{ type: 'image', data: 'AA==', mimeType: 'image/png' }], details: {} }; + }, + }); + const view = (filename: string) => + image.execute('id', { filename }, undefined, undefined, {} as never); + await assert.rejects(order.readArtifact('draft-report.md'), /先读取 plan/); + await view('a.png'); + assert.equal(imageCalls, 0); + assert.deepEqual(exposed, []); + await order.readArtifact('plan.md'); + await view('a.png'); + await view('unknown.png'); + await assert.rejects(order.readArtifact('execution.md'), /原始图片/); + assert.throws(order.assertReady, /原始图片/); + await view('b.png'); + await order.readArtifact('execution.md'); + await order.readArtifact('draft-report.md'); + order.assertReady(); + assert.deepEqual(exposed, ['plan.md', 'execution.md', 'draft-report.md']); + }); + + it('keeps failed evidence blocking while allowing an honest review of the failure', async () => { + let failures = 0; + const order = createReviewReadOrder( + async (name) => name, + ['bad.png'], + () => { + failures += 1; + }, + ); + const image = order.wrap({ + name: 'read_evidence_image', + label: 'image', + description: 'image', + parameters: Type.Object({ filename: Type.String() }), + async execute() { + return createTextResult('unavailable', { error: true }); + }, + }); + await order.readArtifact('plan.md'); + await image.execute('id', { filename: 'bad.png' }, undefined, undefined, {} as never); + order.assertReady(); + await order.readArtifact('execution.md'); + assert.equal(failures, 1); + }); + + it('allows a zero-image review only after the plan, without requiring nonexistent evidence', async () => { + const order = createReviewReadOrder( + async (name) => name, + [], + () => assert.fail(), + ); + assert.throws(order.assertReady, /plan/); + await order.readArtifact('plan.md'); + await order.readArtifact('execution.md'); + order.assertReady(); + const withPatch = createReviewReadOrder( + async (name) => name, + [], + () => assert.fail(), + true, + ); + await withPatch.readArtifact('plan.md'); + await assert.rejects(withPatch.readArtifact('draft-report.md'), /scenario-changes.patch/); + assert.throws(withPatch.assertReady, /scenario-changes.patch/); + await withPatch.readArtifact('scenario-changes.patch'); + withPatch.assertReady(); + }); +});