fix(executor): validate writes before disk, wire rollback, emit validation_failed

[ceilf6]

Linked Issue Or Context

• Fixes validation_failed event never fires — guard effectiveness is unobservable #388 (validation_failed never fires).
• Partially addresses Guard validates after the write and does not roll back by default — invalid files land on disk #387 (guard validates after the write and does not roll back by default) — deliberately not Fixes, see below.

Why #387 is only partly closed

#387's acceptance is "a create/patch step whose content is syntactically invalid must not leave the file on disk". This PR blocks the subset that can be detected reliably: a markdown fence in a code file — which is the failure the 2026-07-12 eval actually produced (TS1127: Invalid character).

The general case needs a trustworthy syntax check, and the existing one is not. Measured against the built guard, checkSyntaxValidity counts quotes per line and returns severity: 'block' for export const msg = "it's fine";, for multi-line template literals, and for JSX text with a contraction. An earlier revision of this PR gave that check write-veto and rollback-trigger power; it would have made any file containing an apostrophe unwritable, and could have deleted a legitimate file via a create snapshot rollback (unlinkSync).

Tracked as #413. Fixing it means parsing rather than counting, which means typescript as a runtime dependency of the guard — a packaging decision that does not belong in this PR.

Summary

Content that can be known before the write is now validated before the write, the interception event has an emit site, and rollback has a terminal state.

1. Pre-write validation, narrowly scoped

create_file content, and the whole-file replace patch that codegen produces for every apply_patch step, are validated before callTool. A failure means the tool is never invoked — the bad file does not reach disk at all, so rollback is not the mechanism it depends on.

Only a markdown fence can veto a write (detectMarkdownFence). Every other verdict keeps its old post-write meaning — including import_validity, which blocks on relative imports of modules a later step in the same plan creates and on path aliases like @/components/x; as a write veto that alone would have made multi-file plans unable to produce their first file. See "Why #387 is only partly closed" above for why the syntax check is not used here either.

The same narrowing gates rollback. That symmetry is load-bearing: a create snapshot rollback is unlinkSync, so triggering it on an import verdict would delete the very file the veto scoping deliberately let through.

2. apply_patch is covered, and content cannot shadow the patch

The plan schema emits no patches, so every modify step goes through codegen, which produces a single whole-file replace (startLine: 1, endLine: <original>). That content is fully known before the write. resolveWriteContent recognises exactly that shape, using collectedContext.files for the original line count.

Resolution is dispatched by action. Plan params are passed through verbatim and the apply_patch skill spreads them, so {path, content, patches} is reachable; a cross-action content fallback would have validated a string that never gets written while the real patch content went unchecked. Partial-line patches still fall through to post-write validation.

Since the pre-write result is computed on the same content, it is reused as the post-write verdict rather than re-running the guard.

3. validation_failed distinguishes interceptions from tool errors

validateAfterExecution returns results: [] on a tool failure. The event is emitted only when at least one real check failed, so the count is usable as an interception count — which is what #388 is for.

4. Rollback has a terminal state

rollback_failed joins the event union and the desktop reducer, and the reason is appended to stepResult.error. Previously a denied rollback — the default non-interactive case, i.e. the eval harness — left a dangling rollback_started and no way to tell whether the bad file was still on disk. Executor.rollback also normalises its return shape: it promised { success, message } while the security layer's denial returns { success: false, error }.

5. needsRollback / writeLeftOnDisk

needsRollback keeps its abort semantics (a real check failing aborts remaining steps in progress-enforcement) but excludes plain tool failures, which previously made one failed read_file abort an entire plan. "A write is still on disk" moved to a separate writeLeftOnDisk field — conflating a disk-state fact with a scheduling signal is what made the field ambiguous.

An earlier draft of this description claimed LLM plans leave validation empty and that this was #387's mechanism. That is wrong: planner.ts:325 overwrites it via getDefaultValidation, which returns two required: true rules for both write actions.

Impact Scope

• packages/core/src/executor/executor.ts — pre-write stage, veto narrowing, patch resolution, rollback outcome, rollback() normalisation.
• packages/core/src/types.ts — rollback_failed event, writeLeftOnDisk.
• packages/core/src/agent/agent.ts — 3 lines wiring emitEvent.
• apps/desktop/src/state/executionReducer.ts — the rollback_failed case.
• Tests: executor.test.ts, agent.test.ts.

Known behaviour changes

• A tool failure that produced a snapshot no longer triggers rollback (the trigger is veto-class only), so no new approval prompt appears in interactive mode on that path.
• benchmarks/eval/report.mjs counts validation_failed; this PR makes that count non-zero for the first time.

GitNexus Impact Summary

• Risk level: MEDIUM
• Critical skeleton changes: yes — packages/core/src/executor/, with tests in the same package.
• GitNexus impact: needsRollback's only consumer is progress-enforcement.ts:42; the agent's main path goes through executeStepsWithErrorFeedback/phase-runner, which do not read it, so the blast radius is Executor.executeSteps' direct embedders. The AgentEvent union gains one member; apps/cli/src/ui/bridge.ts and apps/vscode/src/state.ts both have default branches, and the desktop reducer has a matching case. ExecutorOutput.writeLeftOnDisk is optional and additive.
• Verification: pnpm quality:precommit exit 0.

Verification

• pnpm quality:precommit — exit 0.
• The pre-write test uses a real HallucinationGuard and the actual failing artifact from the eval run (a markdown fence in a .tsx), and asserts the file is absent from disk.
• Coverage for the cases where the scoping must not fire: a forward-reference import and a path-alias import are both written, with the write tool returning a snapshotId and rollback permitted, asserting no rollback occurs.
• An apply_patch step carrying both content and patches asserts the patch content is what reaches the guard.
• A denied rollback asserts rollback_failed is emitted and the error says the file is still on disk.

Checklist

• I have linked an issue or explained why this PR stands alone.
• I have kept the diff focused on the stated change.
• I have run pnpm quality:precommit, or explained why it could not run.
• I have run pnpm quality:local for critical skeleton changes, or explained why it could not run.
• I have updated docs or tests when behavior, public APIs, or Harness contracts changed.
• For critical skeleton changes, I have filled the GitNexus impact summary with concrete results.

🤖 Generated with Claude Code

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 中
处理建议: 请求修改
决策摘要: 方向正确且执行路径上确实生效，但两条链路未达关联 issue 的验收口径：apply_patch（生产上几乎全部走 LLM 全文件替换）仍然先落盘后校验，#387 的首选修法只覆盖了 create_file；同时 validation_failed 现在也会在普通工具失败时触发，直接污染 #388 想要建立的「拦截可计数」指标。两处修复都很小，建议本轮改完再合。

级联分析

• 变更符号: Executor.executeStep、Executor.validateAfterExecution（私有，新增带默认值参数）、新增私有 resolveWriteContent / rollbackFailedWrite、ExecutorConfig.emitEvent（可选，additive）、ExecutorTraceStage.name 新增 'validate_content'（additive）、FrontAgent 构造函数中的 executor 配置。
• 受影响流程: 步骤执行（pre-write 校验 → call_tool → post-write 校验 → 回滚）、agent 事件流、执行 trace 采集、进度强制引擎的中止判定。
• 变更集外调用方 (text search):
  • packages/core/src/executor/progress-enforcement.ts:42 — needsRollback 唯一消费者，语义被放宽后行为改变。
  • packages/core/src/executor/phase-runner.ts — 真实 agent 路径（agent.ts:813 → executeStepsWithErrorFeedback）根本不读 needsRollback，所以放宽只影响 Executor.executeSteps 的使用者（如 benchmarks/frontagent-flow-benchmark.mjs:206）。
  • apps/desktop/src/state/executionReducer.ts:279/286/289 — 三个事件的 handler 已存在，形状匹配，无需改动。
  • benchmarks/eval/run-eval.mjs:97 + benchmarks/eval/report.mjs:75 — 按 event.type 计数并直接印在评测报告里，受下面第 1 条影响。
  • packages/mcp-file/src/tools/create-file.ts:88 / apply-patch.ts:136 确实返回 snapshotId，agent.ts:243 已注册 rollback 工具映射，回滚链路可达。
• 置信度: medium（无代码图谱输出，结论来自 Read/Grep 全仓核查；私有方法签名变更与 additive 联合类型不构成外部破坏）

问题发现

1. [高] validation_failed 现在也覆盖普通工具失败，#388 想要的「拦截计数」失真

   • 证据: executor.ts:228-231 对任意 !postValidation.pass 发事件；而 validateAfterExecution（executor.ts:614-623）在 result.success === false 时就返回 pass:false, results:[], blockedBy:[工具报错]。于是 run_command、read_file、create_file 撞到「文件已存在」等普通失败都会发 validation_failed。benchmarks/eval/run-eval.mjs:97 按 type 计数、report.mjs:75 把这个数字当作「校验是否起作用」的证据；桌面端 executionReducer.ts:283 会把工具报错渲染成「校验失败: ...」。
   • 受影响调用方/流程: 评测报告的 guard 有效性口径、桌面 UI 日志、后续重跑 benchmark 的结论。
   • 最小可行修复: 只在失败确由 guard 产生时发事件（例如判 postValidation.results.length > 0，或让 validateAfterExecution 区分「工具失败」与「guard 拦截」两种返回并只对后者上报）。issue #388 还要求带 check type/path，当前 payload 只有 ValidationResult，可顺带确认是否足够。

2. [高] apply_patch 仍是先落盘后校验，#387 的首选修法只覆盖 create 路径

   • 证据: resolveWriteContent（executor.ts:564-584）只认 toolParams.content，而 apply_patch 的内容在 patches[0].content。生产上这条路几乎全部走 LLM 生成：planner.ts:465 固定发 patches: []（注释「补丁由 Executor 生成」），提示词 plan-generation.ts:423 要求 apply_patch 用 changeDescription + needsCodeGeneration，executor-skills.ts:278-288 随后生成 {operation:'replace', startLine:1, endLine:原文件行数, content: 全文件}——这正是「写盘前已完全可知的完整文件内容」，也正是 markdown 围栏幻觉出现的地方。结果：#387 证据里的 .tsx 围栏若来自修改类任务，仍会先写进磁盘，只能靠回滚兜底；而回滚在无 approvalHandler 的非交互 CLI 会被 tool-call-handler.ts:187-196 直接拒绝（评测侧因 run-eval.mjs:94 自动批准不受影响，PR 描述中「eval harness 会被安全层挡住」这点与仓库现状不符）。
   • 受影响调用方/流程: 所有 modify/bugfix/refactor 任务的写盘路径；#387 的验收项「create/patch 步骤的非法内容不得留在磁盘上」未对 patch 成立。
   • 最小可行修复: 在 resolveWriteContent 中补一条窄分支——apply_patch 且 patches 为单个整文件 replace（startLine === 1 且 endLine >= 原文件行数，原文件可从 collectedContext.files 取）时返回该 content。附带收益：这条补上后，validateAfterExecution 里的 post-write 内容校验分支与 contentValidatedBeforeWrite 这个 mode flag 都可以整体删掉，少一条并行校验路径。

3. [中] needsRollback 放宽的范围比 PR 描述更大，且无测试

   • 证据: executor.ts:244 现在对任何 !postValidation.pass 置 true，其中包含纯工具失败（见 finding 1 的同一分支），不只是「post-write 校验失败」。唯一消费者 progress-enforcement.ts:42 会把剩余步骤全标 skipped 并 break，因此在 Executor.executeSteps 路径上，一次 read_file/run_command 失败就会中止整个剩余计划。真实 agent 路径走 PhaseRunner（不读该标志），所以爆炸半径限于 executeSteps 的直接使用者，但这与 PR 里「whenever a step fails post-write validation」的描述不一致。
   • 受影响调用方/流程: benchmarks/frontagent-flow-benchmark.mjs:206 及任何以 executeSteps 为入口的嵌入方。
   • 最小可行修复: 把标志收窄到校验类失败（与 finding 1 用同一判据），或保留现语义但在 PR 描述与测试中明确覆盖「普通工具失败也会中止后续步骤」。

4. [中] 回滚失败是静默的，UI 会停在「回滚开始」

   • 证据: executor.ts:599-605 只在成功时发 rollback_completed，失败仅 debugWarn（debug 关闭时完全无输出）。安全层拒绝时返回的是 {success:false, error}，而这里读的是 result.message，日志会打成 undefined。桌面端已记了 rollback_started（executionReducer.ts:287）却永远等不到终止事件。
   • 受影响调用方/流程: 非交互运行的可观测性、桌面日志一致性——恰恰是本 PR 想解决的那类盲区。
   • 最小可行修复: 失败时也上报一个终止事件（新增 rollback_failed，或复用现有告警事件），并同时读取 message ?? error。

行级发现

• [packages/core/src/executor/executor.ts:229] 这里对任何 postValidation 失败都发 validation_failed，包含 validateAfterExecution 对纯工具失败返回的 pass:false；建议加判据（如 postValidation.results.length > 0）只上报 guard 拦截，否则评测计数与 UI 文案都会把工具报错算成校验拦截。
• [packages/core/src/executor/executor.ts:244] needsRollback: !postValidation.pass 把普通工具失败也纳入，progress-enforcement.ts:42 会因此中止剩余全部步骤；建议与上一条用同一判据收窄到校验类失败。
• [packages/core/src/executor/executor.ts:577] 只从 toolParams.content 取内容，导致 apply_patch 恒返回 null；而 executor-skills.ts:278-288 生成的是覆盖整文件的单个 replace patch，内容在写盘前已完全可知，建议补一条整文件 replace 分支，让 #387 的前置校验也覆盖修改路径。
• [packages/core/src/executor/executor.ts:604] 回滚失败只走 debugWarn，且安全层拒绝返回的是 {success:false,error}（message 为 undefined）；建议补一个终止事件并读取 message ?? error，避免 UI 停在「回滚开始」。
• [packages/core/src/executor/executor.test.ts:794] 回滚用例全程 mock callTool，只断言调用参数，没有验证磁盘上的文件被恢复；#387 的验收项要求的是「文件不留在磁盘上」，建议改成真实 SnapshotManager + 临时目录并断言文件内容已还原。

Karpathy 评审

• 假设: 隐含假设「apply_patch 的完整内容写盘前不可知」——在 codegen 路径上不成立（planner.ts:465 + executor-skills.ts:288）。另一处隐含假设是 postValidation 失败等价于 guard 拦截，实际混入了工具失败。PR 描述中关于 eval harness 无审批通道的说法与 run-eval.mjs:94 不符。
• 简洁性: WRITE_ACTIONS 常量、resolveWriteContent、rollbackFailedWrite 拆分合理，命名与既有风格一致，没有多余抽象。contentValidatedBeforeWrite 是一个为兼容旧路径而存在的 mode flag；若 finding 2 落地，这个 flag 与 post-write 内容校验分支可整段删除，属于能真正减少概念数的简化。
• 结构质量: 无错误分层、无重复 helper、无文件规模跨界（executor.ts 847 行）；trace 阶段与配置字段均为 additive，对下游无破坏。
• 变更范围: 5 个文件都服务于三个既定缺陷，无无关重构或格式噪声。范围本身克制，问题在于覆盖不足而非过宽。
• 验证: 新增用例针对性强（pre-write 用例使用真实 guard + 真实失败样本 + 断言磁盘无文件，这点很好）。但回滚用例完全 mock，且测试自身注释（executor.test.ts:781）承认刻意绕开了 LLM codegen 分支——而那正是生产上 apply_patch 的主路径。

缺失覆盖

• apply_patch 走 codegen 全文件替换、内容非法时的端到端用例：断言磁盘上不留下（或已还原）非法文件，这是 #387 明确要求的回归测试且当前 patch 路径未覆盖。
• 「普通工具失败不应发 validation_failed」的负向断言（配合 finding 1 的修复）。
• Executor.executeSteps 路径下，一次非校验类工具失败对剩余步骤的影响（finding 3 的行为变更目前无任何测试锁定）。
• 非交互配置（无 approvalHandler、无 permissions.allow: ['rollback']）下回滚被安全层拒绝时的事件与日志行为。

[github-actions]

这里对任何 postValidation 失败都发 validation_failed，包含 validateAfterExecution 对纯工具失败返回的 pass:false；建议加判据（如 postValidation.results.length > 0）只上报 guard 拦截，否则评测计数与 UI 文案都会把工具报错算成校验拦截。

[github-actions]

needsRollback: !postValidation.pass 把普通工具失败也纳入，progress-enforcement.ts:42 会因此中止剩余全部步骤；建议与上一条用同一判据收窄到校验类失败。

[github-actions]

只从 toolParams.content 取内容，导致 apply_patch 恒返回 null；而 executor-skills.ts:278-288 生成的是覆盖整文件的单个 replace patch，内容在写盘前已完全可知，建议补一条整文件 replace 分支，让 #387 的前置校验也覆盖修改路径。

[github-actions]

回滚失败只走 debugWarn，且安全层拒绝返回的是 {success:false,error}（message 为 undefined）；建议补一个终止事件并读取 message ?? error，避免 UI 停在「回滚开始」。

[github-actions]

回滚用例全程 mock callTool，只断言调用参数，没有验证磁盘上的文件被恢复；#387 的验收项要求的是「文件不留在磁盘上」，建议改成真实 SnapshotManager + 临时目录并断言文件内容已还原。

[ceilf6]

Follow-ups from this PR are now filed as issues:

• [Bug] Executor never calls guard.validate() — SDD compliance checking never runs on the real execution path #404 — executor never calls guard.validate(), so SDD compliance checking never runs on the real execution path. This is the residual of hallucinationGuard.enabledChecks is dead config on the executor path #386 that PR fix(guard): honor disabled checks in executor validation #392 could not reach (the sddCompliance check exists only inside validate(), not in validateFilePath/validateCode).
• [Bug] HallucinationGuardConfig.enabled is required but never read — the guard cannot be disabled #405 — HallucinationGuardConfig.enabled is a required config field that nothing reads, so the guard cannot be disabled. This is also why the 2026-07-12 ablation run degraded from a two-factor (SDD + guard) ablation to an SDD-only one, and it means pre-fix(guard): honor disabled checks in executor validation #392 and post-fix(guard): honor disabled checks in executor validation #392 ablation arms are not directly comparable.

Both are the same defect class this PR addresses for validation_failed — a contract declared in types/schema and never wired to behaviour. A third instance (filesense navigate writeMode) is fixed separately in #403.

Still open from the PR description and not filed as an issue, because it is a policy decision rather than a defect: SecurityManager classifies rollback as ask/high-risk, so the auto-rollback wired here is denied in non-interactive runs unless the user sets permissions.allow: ['rollback'].

[ceilf6]

Correction to my earlier follow-up comment: #405 was a duplicate of #400 (which predates it and already covers hallucinationGuard.enabled being dead config). I have closed #405 and moved its one additional point — that pre-#392 vs post-#392 ablation arms are not comparable — onto #400 as a comment.

Revised follow-up list for this PR:

• [Bug] Executor never calls guard.validate() — SDD compliance checking never runs on the real execution path #404 — executor never calls guard.validate(), so SDD compliance checking never runs on the real execution path (residual of hallucinationGuard.enabledChecks is dead config on the executor path #386).
• hallucinationGuard.enabled is dead config on the agent path #400 — hallucinationGuard.enabled is dead config (pre-existing).
• [Bug] Guard syntax check passes prose-as-code — TS1127 corruption reaches disk unflagged #406 — new, and now the binding constraint: the guard's syntax check is bracket-matching plus regexes, so it returns pass: true on natural-language prose. Verified directly against two files an agent run produced today, both pure Chinese prose, both written to disk, both TS1127: Invalid character under tsc.

The #406 finding matters for how this PR should be read: the executor plumbing here blocks writes that the guard flags, and it demonstrably does so (see the before/after probe in the benchmark report PR). But when the checker itself passes the content, the write still lands. Interception rate is now gated on checker strength, not on the executor.

[ceilf6]

Pushed 1a54e25: validation_failed now only fires for real check failures.

The first version of this PR had a metric defect I want to name explicitly, because it would mislead anyone reading the number. The event was emitted from three places, and the third was not an interception at all:

1. pre-execution structural block — real check
2. pre-write content block — real interception
3. post-write validation failure — includes pure tool failures, because validateAfterExecution returns { pass: false, results: [], blockedBy: [toolError] } when resultObj.success === false

So an EACCES from the write tool counted as a "validation failure". Anyone using the counter as an interception count would have been wrong, and filtering after the fact by results[].type is a footgun.

Fix: emit only when at least one real check reports pass: false (results.some(r => !r.pass)). The metric is now clean by construction. Rollback still keys off !postValidation.pass — a tool failure that already produced a snapshot must still be undone.

New test: does not emit validation_failed when only the tool itself failed.

Re-verified end to end after the change, through the real runFrontAgentTask path with a deterministic bad-content stub: still validation_failed: 1, file not on disk, error Pre-write validation failed: Syntax errors found in .... Gate green (pnpm quality:precommit exit 0, turbo 25/25, workflows 34/34).

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 高
处理建议: 请求修改
决策摘要: 方向正确且证据扎实，但前置校验把 guard 的 block 判定升级成了"写盘否决权"——其中 import_validity 对"后续步骤才创建的相对导入"必然判 block，会让本来能落盘的文件彻底不写；同时 apply_patch（真实计划里全部 modify 步骤）没有拿到任何前置保护，#387 在默认无审批配置下未闭环。建议收窄前置门禁的检查范围并补上 patch 路径后再合。

级联分析

• 变更符号: Executor.executeStep、Executor.validateAfterExecution（签名 +1 参数）、新增 resolveWriteContent / emitValidationFailed / rollbackFailedWrite、ExecutorConfig.emitEvent、ExecutorTraceStage.name 联合类型、FrontAgent 构造。
• 受影响流程: executeStep → validate_content → call_tool；Executor.executeSteps → progress-enforcement；phase-runner 的失败/恢复回路；agent 事件流 → apps/desktop/src/state/executionReducer.ts；benchmarks/eval/report.mjs 对 validation_failed 的计数口径。
• 变更集外调用方: packages/core/src/executor/progress-enforcement.ts:42（needsRollback 唯一消费者，语义被动过但文件未改）、apps/desktop/src/state/executionReducer.ts:279-290（validation_failed / rollback_started / rollback_completed 三个 case 现在首次会真的收到事件）、packages/core/src/skills/executor-skills.ts:278-288（apply_patch 代码生成路径）、packages/core/src/llm/schemas.ts:19-37（决定 toolParams 实际能带哪些字段）。
• 置信度: medium（本地只读代码 + 文本检索确认调用方，未自行跑 GitNexus；PR 提供的图谱结果与我读到的调用关系一致）

问题发现

1. [高] 前置门禁把 import_validity 的 block 判定变成了"不写盘"，会吞掉合法文件

   • 证据: executor.ts:181-205 调用的是完整 guard.validateCode，而 validateCode 会跑 checkAllImports（packages/hallucination-guard/src/guard.ts:196-203）；相对导入解析不到时返回 severity: 'block'（packages/hallucination-guard/src/checks/import-validity.ts:84-90），非 .// 开头的路径别名（@/components/x）会走 checkPackageExists 判成"未安装的包"（同文件 36-53、96-126）。改动前这些判定发生在写盘后：step 标记失败，但文件仍在磁盘上，等后续步骤把被导入模块建出来后仍可自洽；改动后该文件根本不存在。
   • 受影响调用方/流程: 多文件计划中"先建页面、后建组件"的 create_file 步骤；随后 phase-runner.ts:271-275 记录 phaseErrors → runPhaseRecovery。恢复步骤若是 apply_patch，会命中 Cannot apply patch: file does not exist，而该错误在 executor-skills.ts:291-298 被判为可跳过 → 记为成功；onPhaseComplete 的 tsc --noEmit（agent/phase-checks.ts:99-105）也看不到一个"根本不存在的文件"的导入错误，于是 phase-runner.ts:369-376 会把原本 failed 的步骤翻成 completed。净效果：文件没写、run 报成功。
   • 最小可行修复: 前置阶段只用写盘前可靠的检查（syntax_validity）做否决，import_validity 结果保留为非阻塞或仍留在写盘后判定；若要保留 import 门禁，用已有的 config.getCreatedModules?.()（executor/types.ts:23）把"计划内待建模块"排除出 block 集合。并补一条回归测试：内容引用同计划后续步骤才创建的相对模块时，文件必须照常写入。

2. [高] apply_patch 全程没有前置保护，#387 的 patch 侧验收未闭环

   • 证据: PR 描述称"apply_patch 只带补丁片段所以返回 null"，但 packages/core/src/llm/schemas.ts:19-37 的 STEP_PARAMS_SCHEMA 既没有 content 也没有 patches 字段，因此 LLM 计划里的 apply_patch 步骤必然走 executor-skills.ts:226-289 的代码生成分支，产出的是覆盖全文件的单个 replace 补丁（startLine: 1, endLine: originalLines, content: modifiedCode）——写盘后的完整内容在写之前就是已知的。而 resolveWriteContent（executor.ts:573-584）只认 toolParams.content，对该路径恒返回 null。
   • 受影响调用方/流程: 所有 modify 类任务。它们只能落到写盘后校验 + rollbackFailedWrite，而 rollback 在 security.ts:283-292 被判为 ask/high，非交互运行直接拒绝（PR follow-up 1 已自述）。issue #387 明确要求"a create/patch step whose content is syntactically invalid must not leave the file on disk"，patch 侧在默认配置下仍会留下坏文件——正是评测所处的 headless 配置。
   • 最小可行修复: 在 resolveWriteContent 里识别"单个 replace 且覆盖 collectedContext.files.get(path) 全部行"的补丁，直接用 patch.content 作为前置校验内容；其余补丁形态继续返回 null。

3. [中] needsRollback 现在等价于"步骤失败"，语义与字段名脱节

   • 证据: validateAfterExecution（executor.ts:626-635）对任何 result.success === false 都返回 pass:false，因此 executor.ts:245 的 !postValidation.pass 对 EACCES、命令失败等纯工具错误同样为 true——与 !stepResult.success 完全同义；同时写盘失败时 rollbackFailedWrite 已经在 executor.ts:228-232 尝试过回滚，标志位却仍报"需要回滚"。PR 描述把影响表述为"post-write validation 失败时"，实际范围是所有失败。
   • 受影响调用方/流程: progress-enforcement.ts:42（Executor.executeSteps 这一公开 API：任何一步失败即把余下步骤全部置 skipped）。主链路走 executeStepsWithErrorFeedback/phase-runner，不读该标志，所以生产影响有限，但 SDK 消费者的 run shape 变了，且变化面比 PR 描述更宽。
   • 最小可行修复: 让该标志表达"有内容已落盘且未被成功撤销"（结合 snapshotId 与 rollbackFailedWrite 的返回值），或明确删除它并让消费者直接读 stepResult.success；同时在 PR 描述里更正影响范围。

4. [中] 回滚失败没有任何终态信号，恰好在最需要观测的 headless 场景失声

   • 证据: executor.ts:611-617 先发 rollback_started，成功才发 rollback_completed，失败只走 debugWarn。AgentEvent 联合（packages/core/src/types.ts:793-795）没有失败态；apps/desktop/src/state/executionReducer.ts:286-290 会留下一条永远等不到收尾的"回滚开始"日志。默认非交互配置下 rollback 被安全层拒绝，正是这条静默分支。
   • 受影响调用方/流程: 桌面端执行日志；benchmarks/eval/report.mjs:74-75 依赖事件计数评估 guard 有效性——"拦截到了但没能撤销"与"撤销成功"不可区分。
   • 最小可行修复: 补一个终态（扩 AgentEvent 加 rollback_failed，或至少把回滚失败原因追加进 stepResult.error），使"坏文件是否仍在磁盘上"可判定。

5. [中] 纯工具失败也会触发回滚，交互式运行会多出审批弹窗

   • 证据: executor.ts:228-232 在 postValidation.pass === false 时无条件调用 rollbackFailedWrite，而该分支包含工具自身报错（executor.ts:626-635）。只要工具返回了 snapshotId，就会发起一次 rollback，在注册了 approvalHandler 的桌面/CLI 交互模式下触发 snapshot_rollback_requires_approval 审批（security.ts:283-292）。行内注释说明这是有意的，但这属于用户可见行为变化，PR 描述未覆盖。
   • 受影响调用方/流程: 桌面端与交互式 CLI 的失败路径。
   • 最小可行修复: 要么把执行器发起的补偿性回滚单独归类（PR 已把它列为 maintainer 决策，可先只在校验驱动的失败上触发），要么在描述与文档里显式说明新增审批点。

行级发现

• [packages/core/src/executor/executor.ts:183] 这里用完整 validateCode 做写盘否决，import_validity 的 block 判定（含"后续步骤才创建的相对导入"和路径别名）会导致文件根本不落盘；前置门禁建议只依据 syntax_validity，import 检查保留在写盘后或用 getCreatedModules() 排除计划内模块。
• [packages/core/src/executor/executor.ts:573] WRITE_ACTIONS 含 apply_patch，但计划 schema 不产出 content，apply_patch 恒返回 null；应识别"覆盖全文件的单个 replace 补丁"并取其 content，否则所有 modify 步骤没有前置保护。
• [packages/core/src/executor/executor.ts:245] !postValidation.pass 对纯工具错误同样为 true，使该字段与 !stepResult.success 同义且与"已在上方尝试过回滚"矛盾；建议收窄为"有落盘且未成功撤销"。
• [packages/core/src/executor/executor.ts:616] 回滚失败只 debugWarn，rollback_started 没有终态；在安全层拒绝回滚的 headless 运行里这正是最需要上报的分支，建议补失败事件或把原因写进 stepResult.error。
• [packages/core/src/executor/executor.ts:231] 工具自身失败也会发起 rollback，交互模式下会新增一次审批弹窗；请确认该行为并在描述中说明，或只在校验驱动的失败上触发。

Karpathy 评审

• 假设: "apply_patch 只携带补丁片段"这一核心假设与实际实现不符（skill 生成的是全文件 replace），据此得出的"无法前置校验"结论不成立；另一处隐含假设是"guard 的 block 判定足够可靠到可以否决写入"，但 import 检查存在已知误报类别。
• 简洁性: 三个新私有方法职责清晰、体量合适；validate_content 阶段对所有 step（含 read_file）都会记一条空 stage，属可接受的噪声。emitValidationFailed 用 results.some(r => !r.pass) 区分"校验拦截"与"工具失败"是隐式契约，但注释已写明，可接受。
• 结构质量: 无明显退化，没有跨层泄漏或薄 wrapper；emitEvent 作为可选出口与既有 onSecurityDecision 模式一致。
• 变更范围: 聚焦，agent.ts 仅 3 行接线，无无关重构。
• 验证: 3 个新执行器测试 + 1 个 agent 契约测试方向正确，但只覆盖真阳性；两个关键测试 mock 了 guard，回滚测试还额外授予 permissions.allow: ['rollback']，因此验证的是接线而非默认配置下的结果。新门禁的假阳性代价（合法文件不落盘）完全没有测试。

缺失覆盖

• create_file 内容引用"同计划后续步骤才创建"的相对模块时，文件必须照常写入（针对发现 1 的回归测试）。
• 路径别名导入（如 @/components/Card）不应触发写盘否决。
• apply_patch 步骤内容语法无效时，磁盘上不应留下坏文件——按 issue #387 的验收口径断言文件状态，而不只断言 callTool('rollback', ...) 被调用。
• 默认（未授予 permissions.allow: ['rollback']）非交互配置下回滚被拒的路径：断言失败可观测，且不会出现只有 rollback_started 的悬空事件序列。
• Executor.executeSteps 在纯工具失败（非校验失败）时是否应中止余下步骤——progress-enforcement 的新语义目前无测试。

[github-actions]

这里用完整 validateCode 做写盘否决，import_validity 的 block 判定（含"后续步骤才创建的相对导入"和路径别名）会导致文件根本不落盘；前置门禁建议只依据 syntax_validity，import 检查保留在写盘后或用 getCreatedModules() 排除计划内模块。

[github-actions]

WRITE_ACTIONS 含 apply_patch，但计划 schema 不产出 content，apply_patch 恒返回 null；应识别"覆盖全文件的单个 replace 补丁"并取其 content，否则所有 modify 步骤没有前置保护。

[github-actions]

!postValidation.pass 对纯工具错误同样为 true，使该字段与 !stepResult.success 同义且与"已在上方尝试过回滚"矛盾；建议收窄为"有落盘且未成功撤销"。

[github-actions]

回滚失败只 debugWarn，rollback_started 没有终态；在安全层拒绝回滚的 headless 运行里这正是最需要上报的分支，建议补失败事件或把原因写进 stepResult.error。

[github-actions]

工具自身失败也会发起 rollback，交互模式下会新增一次审批弹窗；请确认该行为并在描述中说明，或只在校验驱动的失败上触发。

[Copilot]

Pull request overview

This PR fixes executor validation/rollback ordering and makes validation outcomes observable by emitting validation_failed (plus rollback lifecycle events) into the agent event stream, with regression tests to ensure invalid content is blocked before disk when possible and rolled back otherwise.

Changes:

• Add a pre-write validate_content trace stage that validates fully-resolvable write content before invoking file-write tools.
• Wire rollback on post-write validation failure (using tool snapshots) and emit validation_failed / rollback_started / rollback_completed / rollback_failed.
• Add executor + agent contract tests and surface rollback failure status in the desktop UI reducer.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
packages/core/src/types.ts	Extends AgentEvent with rollback_failed for rollback terminal-state observability.
packages/core/src/executor/types.ts	Adds ExecutorConfig.emitEvent outlet and new validate_content trace stage.
packages/core/src/executor/executor.ts	Implements pre-write content validation, rollback-on-failed-validation, and event emission.
packages/core/src/executor/executor.test.ts	Adds regression tests for pre-write blocking, single-pass validation, rollback behavior, and event emission semantics.
packages/core/src/agent/agent.ts	Wires executor emitEvent into the agent’s event stream via this.emit(...).
packages/core/src/agent/agent.test.ts	Adds contract test ensuring executor validation_failed surfaces through agent events.
apps/desktop/src/state/executionReducer.ts	Logs rollback_failed so rollback has a terminal UI state (not just rollback_started).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 高
处理建议: 请求修改
决策摘要: 前置门禁（#387 主体）与事件出口（#388）本身是对的，但写盘后判定复用了未收窄的完整校验结果去触发回滚——import_validity 对「后续步骤才创建的模块」和路径别名必然判 block（packages/hallucination-guard/src/checks/import-validity.ts:42,87），一旦按 PR 自己推荐的 permissions.allow: ['rollback'] 打开自动回滚，刚写好的合法文件会被 create 快照回滚直接 unlink 删除（packages/mcp-file/src/snapshot.ts:113-123）；这与本 PR 收窄前置否决权的理由自相矛盾，且现有测试因 mock 未返回 snapshotId 而观察不到。修掉回滚触发口径后可再评。

级联分析

• 变更符号: Executor.executeStep、Executor.validateAfterExecution（新增第 4 参）、新增 resolveWriteContent / emitValidationFailed / rollbackFailedWrite / narrowToPreWriteVeto / resolveFullFileReplaceContent、ExecutorConfig.emitEvent、ExecutorTraceStage.name 新成员 validate_content、AgentEvent 新成员 rollback_failed、FrontAgent 构造期事件接线、consoleReducer。
• 受影响流程: executeStep → validate_content → call_tool → validate_after → rollbackFailedWrite → callTool('rollback') → SecurityManager.evaluate（packages/core/src/security.ts:283 判 ask）；emitEvent → FrontAgent.emit → 桌面 reducer / CLI bridge / VS Code state；Executor.rollback 由零调用方变为执行路径上的活代码。
• 变更集外调用方:
  • packages/core/src/executor/progress-enforcement.ts:42 —— needsRollback 唯一消费者，语义变更直接改变它的中止条件（text search + 直接读取确认）。
  • packages/core/src/executor/phase-runner.ts / step-feedback-runner.ts —— 不读该字段；而 agent 实际走的是 executeStepsWithErrorFeedback（packages/core/src/agent/agent.ts:813 → executor.ts:865），所以 needsRollback 的改动在 agent 主路径上不生效，PR 描述中「更多运行会中止剩余步骤」的影响面被高估。
  • AgentEvent 新增成员：apps/cli/src/ui/bridge.ts:100、apps/vscode/src/state.ts:413 都有 default 分支，联合类型扩展不破坏这两处；桌面 reducer 已同步补 case。
  • 事件消费者 benchmarks/eval/report.mjs:74 依赖 validation_failed 计数，本 PR 会首次让它非零。
• 置信度: medium（无代码图谱工具输出，结论来自仓库内直接读取 + 文本检索；validate_content/needsRollback/rollback 的调用方已逐个打开确认，未确认的是各 app 里非 switch 形式的事件订阅者）

问题发现

1. [致命] 写盘后回滚触发口径未收窄，合法文件会被删除

   • 证据: executor.ts:292 把未收窄的 rawContentValidation 传给 validateAfterExecution，executor.ts:737-739 原样返回，于是 postValidation 含 import_validity 的 block 项；executor.ts:297-301 只看 !postValidation.pass 就调用 rollbackFailedWrite。而 import-validity.ts:42 把路径别名 @/components/Card 判为「包未安装 / block」，import-validity.ts:87 把尚未创建的相对模块判为 block——这正是 executor.ts:249-251 与测试 executor.test.ts:860-862 明确认定「合法、应当落盘」的两类内容。create_file 成功时必返回 snapshotId（packages/mcp-file/src/tools/create-file.ts:88），snapshot.ts:113-123 对 create 快照的回滚是 unlinkSync。
   • 受影响调用方/流程: 所有走 executeStep 的引擎（phase-runner、progress-enforcement、langgraph runner），即 agent 主路径也在内。默认 headless 下 SecurityManager（security.ts:283）拒绝 rollback，表现为每个含别名/前向 import 的写入都刷 rollback_started + rollback_failed 且 needsRollback=true；一旦按本 PR follow-up #1 推荐配置 permissions.allow: ['rollback']，同样的场景变成刚创建的合法文件被删除，后续步骤在文件不存在的状态上继续。
   • 最小可行修复: 让回滚触发口径与否决口径一致——用 narrowToPreWriteVeto(postValidation).pass === false（或同一 PRE_WRITE_VETO_CHECKS 过滤）作为 rollbackFailedWrite 的前置条件，import 类失败仍照旧只让步骤失败、不撤销落盘。

2. [高] apply_patch 步骤上的 content 参数会顶掉真实补丁内容，反而使写入内容完全不被校验

   • 证据: executor.ts:660-662 对两个写动作一律优先取 toolParams.content ?? step.params.content，只有在取不到时才回落 resolveFullFileReplaceContent。LLM 计划的 params 是原样透传的（packages/core/src/planner.ts:323），apply_patch 技能又整体 spread 原 params（packages/core/src/skills/executor-skills.ts:286-289），因此 {path, content, patches:[生成的补丁]} 是可达状态。此时前置校验校的是那份不会被写入的 content，通过后 executor.ts:737-739 又把这份结果当作写盘后结论复用——真正落盘的 patches[0].content 一次都不会过 guard。
   • 受影响调用方/流程: 所有 apply_patch 步骤；效果是 #387 想堵的漏洞在这条分支上被静默放大（原实现至少还会在写盘后校验 result.content）。
   • 最小可行修复: 按动作分派——create_file 用 content，apply_patch 只用 resolveFullFileReplaceContent，不接受 content 兜底。

3. [中] needsRollback 的语义改动建立在一个与仓库不符的前提上

   • 证据: PR 描述称「LLM 计划普遍 validation: []，所以旧的 some(v => v.required) 几乎恒假，这就是 #387 的机制」。但 packages/core/src/planner.ts:325 对 LLM 计划的每一步都用 getDefaultValidation(action) 覆写 validation，planner.ts:367-374 对 create_file / apply_patch 固定返回两条 required: true——写步骤上旧的第二个合取项恒真。validation: [] 只出现在 step-callbacks.ts 生成的修复/恢复步骤上，而这些步骤走 phase-runner，根本不读 needsRollback。
   • 受影响调用方/流程: progress-enforcement.ts:42 是唯一消费者。按新语义，「写盘后校验失败 + 回滚成功」从「中止剩余步骤」变成「继续执行」，而此时 create_file 的回滚意味着文件已被删除，后续依赖步骤会在缺文件状态下继续（executor-skills.ts:291-298 还会把 file not found in context 判为可跳过，步骤反而记成功）。
   • 最小可行修复: 要么保留「写盘后校验失败即中止」的中止语义、另用独立字段表达 leftOnDisk，要么在描述中更正前提并说明为何「回滚成功后继续」对 create_file 也安全；无论哪种，PR 描述里已不存在的 needsRollback = !postValidation.pass 段落需要改写。

4. [中] Executor.rollback 的返回类型是错的，新调用方用 cast 绕过而不是修掉

   • 证据: executor.ts:947 声明返回 { success: boolean; message: string }，但安全层拒绝时 tool-call-handler.ts:62-69 返回的是 { success:false, error }，没有 message。executor.ts:710 因此写成 result.message ?? (result as { error?: string }).error ?? 'unknown error'。类型契约与实际形状不符，下一个调用方还会踩同一个坑。
   • 受影响调用方/流程: rollbackFailedWrite，以及未来任何 Executor.rollback 的消费者（该方法此前零调用方，本 PR 让它上线）。
   • 最小可行修复: 在 rollback() 内把工具返回归一化为 { success, message }（把 error 并入 message），或把签名改成 { success: boolean; message?: string; error?: string }，让调用点不再 cast。

行级发现

• [packages/core/src/executor/executor.ts:300] 回滚以未收窄的 postValidation 为触发条件，import_validity（前向引用 / 路径别名，必判 block）会导致刚落盘的合法文件被回滚删除；应改用与 PRE_WRITE_VETO_CHECKS 一致的收窄结果作为回滚触发条件。
• [packages/core/src/executor/executor.ts:661] content 兜底对 apply_patch 同样生效，会校验一份不会被写入的内容并把该结果复用为写盘后结论，导致真实补丁内容零校验；按动作分派：create_file 用 content，apply_patch 只用 resolveFullFileReplaceContent。
• [packages/core/src/executor/executor.ts:322] 新语义的依据（旧 step.validation 恒空）与 planner.ts:325/367-374 不符，写步骤旧条件恒真；此改动实际把「校验失败即中止」放宽为「回滚成功即继续」，而 create 回滚会删文件，需重新论证或改用独立字段。
• [packages/core/src/executor/executor.ts:710] 这里 cast 出 error 是在绕开 Executor.rollback 声明错误的返回类型（executor.ts:947 承诺 message: string，安全层拒绝时只有 error）；应在 rollback() 内归一化返回形状而不是在调用点断言。
• [packages/core/src/executor/executor.test.ts:891] 该用例的 callTool mock 返回 { success: true } 且无 snapshotId，恰好绕开 rollbackFailedWrite，所以观察不到「合法前向引用文件被回滚」这一真实行为；mock 应返回 snapshotId 并断言未发生 rollback 调用。

Karpathy 评审

• 假设: 两处关键假设未成立——「LLM 计划 validation 恒空」被 planner.ts:325/367-374 证伪；「写盘后判定时文件还在磁盘、后续步骤补齐即可自洽」（executor.ts:249-251）被同一 PR 新接的回滚破坏。另外描述中的参数名 contentValidatedBeforeWrite、表达式 needsRollback = !postValidation.pass 与最终代码不一致，需同步。
• 简洁性: narrowToPreWriteVeto / resolveFullFileReplaceContent 是纯函数、边界清楚，是合理的抽象；问题不在层数，而在收窄逻辑只用在写盘前、没用在回滚触发点，形成了同一份结果的两套口径。
• 结构质量: 新增逻辑集中在 executor 内、复用了既有 rollback()/事件出口，未见薄 wrapper 或文件规模跨界（executor.ts 约 960 行）；扣分项是 executor.ts:710 的 cast 掩盖了 rollback() 的错误契约，以及 resolveWriteContent 用「动作无关」的取值顺序处理两个语义不同的写动作。
• 变更范围: 与 #387/#388 目标相符，无无关重构；rollback_failed 事件 + 桌面 reducer 是补齐终态的必要配套，不算范围蔓延。
• 验证: 事件序列、单次校验、前置拦截都有直接断言，pre-write 用例还用了真实 guard 和评测样本，质量高；但两条「不否决」用例（891、927）只断言了写工具被调用和错误文案，未断言步骤结果与是否发生回滚，恰好放过了本报告的第 1 项。

缺失覆盖

• 合法前向引用 / 路径别名 + 写工具返回 snapshotId + permissions.allow: ['rollback']：断言文件仍在磁盘上、未调用 rollback、needsRollback 为 false。
• apply_patch 步骤 params 同时带 content 与生成的 patches：断言被校验的是补丁内容而非 content。
• progress-enforcement 层面的端到端用例：写步骤校验失败且回滚成功后，剩余步骤是继续还是跳过——当前只有 executor 单步断言，中止语义变更没有任何调用方级覆盖。
• Executor.rollback 在安全层拒绝时的返回形状用例（锁住 message/error 归一化后的契约）。

[github-actions]

回滚以未收窄的 postValidation 为触发条件，import_validity（前向引用 / 路径别名，必判 block）会导致刚落盘的合法文件被回滚删除；应改用与 PRE_WRITE_VETO_CHECKS 一致的收窄结果作为回滚触发条件。

[github-actions]

content 兜底对 apply_patch 同样生效，会校验一份不会被写入的内容并把该结果复用为写盘后结论，导致真实补丁内容零校验；按动作分派：create_file 用 content，apply_patch 只用 resolveFullFileReplaceContent。

[github-actions]

新语义的依据（旧 step.validation 恒空）与 planner.ts:325/367-374 不符，写步骤旧条件恒真；此改动实际把「校验失败即中止」放宽为「回滚成功即继续」，而 create 回滚会删文件，需重新论证或改用独立字段。

[github-actions]

这里 cast 出 error 是在绕开 Executor.rollback 声明错误的返回类型（executor.ts:947 承诺 message: string，安全层拒绝时只有 error）；应在 rollback() 内归一化返回形状而不是在调用点断言。

[github-actions]

该用例的 callTool mock 返回 { success: true } 且无 snapshotId，恰好绕开 rollbackFailedWrite，所以观察不到「合法前向引用文件被回滚」这一真实行为；mock 应返回 snapshotId 并断言未发生 rollback 调用。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 中
处理建议: 请求修改
决策摘要: 前置写盘门禁（#387 的首选方案）实现正确且用真实 guard 验证过，可以合并；但"回滚"这半边在仓库内两个真实 file client 上不可达，其两条测试只因 mock 返回了真实工具从不返回的 content 字段才通过，且 PR 描述中 needsRollback 的表达式与合入代码不一致——建议先补齐这两点再合。

级联分析

• 变更符号: Executor.executeStep、Executor.validateAfterExecution(私有，1 调用方)、新增 resolveWriteContent/rollbackFailedWrite/emitValidationFailed/narrowToPreWriteVeto/resolveFullFileReplaceContent、Executor.rollback(返回形状归一化)、ExecutorConfig.emitEvent、ExecutorOutput.writeLeftOnDisk、AgentEvent 联合新增 rollback_failed、ExecutorTraceStage 新增 validate_content。
• 受影响流程: Agent.execute → executeStepsWithErrorFeedback → PhaseRunner（生产主路径，读 stepResult.success，不读 needsRollback）；Executor.executeSteps → progress-enforcement（benchmark 路径，唯一 needsRollback 消费者，progress-enforcement.ts:42）；FileMCPClient.callTool（packages/runtime-node/src/mcp-clients.ts:33、apps/cli/src/mcp-client.ts:42）；desktop consoleReducer。
• 变更集外调用方: progress-enforcement.ts:42（needsRollback，语义已变，未同步更新，见发现 2）；apps/desktop/src/state/executionReducer.ts 已处理新事件；未发现其它 AgentEvent 穷举 switch 消费者（text search）。Executor.rollback 返回形状归一化与 SnapshotManager.rollback（packages/mcp-file/src/snapshot.ts:106，返回 {success,message}）及安全层拒绝形状 {success:false,error}（tool-call-handler.ts:63）均对得上——已核实无回归。
• 置信度: medium（无代码图谱可用，以上均为 Read/Grep 文本核查；被改符号的调用方数量少且已逐一确认）

问题发现

1. [高] 写盘后校验 + 回滚这条链路在仓库内两个真实 file client 上不可达，两条"证明回滚生效"的测试建立在虚构的工具返回形状上

   • 证据: validateAfterExecution 的写盘后内容来源是 result.content ?? toolParams.content ?? step.params.content（executor.ts:752-756）。而 createFile 返回 {success, path, snapshotId}（packages/mcp-file/src/tools/create-file.ts:85），applyPatch 返回 {success, diff, validation, snapshotId}（apply-patch.ts:132），都不返回 content；两个真实 client 均直接透传这两个对象（packages/runtime-node/src/mcp-clients.ts:37-47、apps/cli/src/mcp-client.ts:42-54）。局部行补丁的 toolParams.content/step.params.content 也不存在，于是写盘后 validateCode 根本不会被调用，postValidation 恒为 {pass:true}。反过来，能解析出前置内容的场景（create_file、整文件 replace）语法失败已在写盘前 return，写盘后只可能剩 import_validity 失败，而 executor.ts:303 的收窄门禁又刻意不为它触发回滚。合起来：rollbackFailedWrite 在这两个 client 下没有可达触发点。executor.test.ts:772-776 与 1130 附近的 mock 通过返回 content: 'export const a = {' 人为制造了这条路径。
   • 受影响调用方/流程: Agent.execute 全部写步骤；#387 中"补丁写入的非法内容不得留在磁盘"这一条对局部行补丁仍不成立；PR follow-up #1 把"回滚不生效"归因于安全层，实际即使授予 permissions.allow:['rollback'] 也不会触发。
   • 最小可行修复: 在 validateAfterExecution 的写步骤分支里，当没有前置内容且工具未返回 content 时从磁盘读回已写文件再校验（路径已知、快照已存在，改动约 3 行）；并把两条回滚测试的 mock 换成真实工具的返回形状（{success, snapshotId} + 磁盘上确有坏文件），否则它们证明不了任何生产行为。若决定不做，请把 rollback 部分明确降级为"面向外部 MCP client 的兜底"并在 PR 描述里更正 follow-up #1 的归因。

2. [中] needsRollback 的合入语义与 PR 描述不一致，且顺带改变了非写步骤的中止语义

   • 证据: 描述称新表达式为 !postValidation.pass（更宽），合入代码是 postValidation.results.some((r) => !r.pass)（executor.ts:328，更窄）。工具自身失败时 validateAfterExecution 返回 results: []（executor.ts:740-744），因此纯工具失败一律不再中止。planner 给 read_file 的默认校验是 [{type:'file_exists', required:true}]（planner.ts:376），旧条件下这类步骤的非 skippable 工具失败会中止剩余计划（progress-enforcement.ts:42），现在不会。描述里"Behavioural change worth reviewer attention"一节讲的是相反方向的放宽。同样过时的还有"apply_patch … returns null and keeps the old post-write path"（实际已支持整文件 replace 前置校验）和 contentValidatedBeforeWrite 布尔参数（实际是 preWriteContentValidation: ValidationResult）。
   • 受影响调用方/流程: progress-enforcement.ts:42（benchmark 与 Executor.executeSteps 的外部调用方）；主 agent 路径不读该字段，影响有限。
   • 最小可行修复: 更新 PR 描述为合入表达式，并显式写明"纯工具失败不再中止剩余步骤"这一方向的变化；read_file 工具失败后续步骤继续执行的新行为建议补一条 executeSteps 级别的测试（目前只在单步层面断言 needsRollback === false）。

3. [中] ExecutorOutput.writeLeftOnDisk 是新增公共契约字段，但没有任何生产消费者

   • 证据: 全仓仅 types.ts:741（定义）、executor.ts:329（赋值）与测试引用（Grep 全仓）。唯一读调度信号的 progress-enforcement.ts:42 只看 needsRollback，主路径的 PhaseRunner 两个字段都不读。"坏文件仍在磁盘"这一事实对调用方只能通过 stepResult.error 的子串（executor.ts:314-316）获得。
   • 受影响调用方/流程: 任何依赖 ExecutorOutput 的下游都要多理解一个永不被读的字段；与 #386 "dead config" 属同一类问题。
   • 最小可行修复: 二选一——让 progress-enforcement/PhaseRunner 在 writeLeftOnDisk 为真时中止（这恰是最该中止的一种失败），或删除该字段，保留 error 文案与 rollback_failed 事件。

4. [中] validation_failed 无法区分"写盘前拦截"与"写盘后失败但文件仍在"，而这正是 #388 要统计的量

   • 证据: 三个发射点（executor.ts:206、255、298）都发同一形状 {type:'validation_failed', result}（emitValidationFailed，executor.ts:688-692）。#388 要求带 check type、path 与 blocking reason；type/reason 在 results[].type/blockedBy 里可取，path 只存在于 message 字符串与 details 中，且没有 stage/stepId。于是 benchmarks/eval/report.mjs:75 的 sumEvent(..., 'validation_failed') 统计出的数字会把"被前置门禁挡下的真拦截"和"import 前向引用导致的步骤失败（有意放行落盘）"混在一起。
   • 受影响调用方/流程: 评测报告脚本、desktop 日志、任何以该事件度量 guard 有效性的消费者。
   • 最小可行修复: 事件加一个判别字段（如 stage: 'pre_execution' | 'pre_write' | 'post_write'）与 path/stepId，desktop reducer 与 report 脚本按 stage 分开计数。

5. [低] agent 侧"契约测试"绕过公共 API，测不到它声称的契约

   • 证据: agent.test.ts:240 用 (agent as unknown as { executor: Executor }).executor 取私有字段后直接调 executeStep。若将来 emitEvent 的接线从构造期移走、或 executor 字段改名，该 cast 仍能编译，测试仍会绿。
   • 受影响调用方/流程: #388 的核心验收（"执行器事件能否浮到 agent 事件流"）没有真正的回归保护。
   • 最小可行修复: 通过 agent.addEventListener + 公开执行入口驱动一次写步骤；若成本过高，至少断言 executor config 上的 emitEvent 与 agent.emit 同源。

6. [低] executor.test.ts 由 664 行增至 1151 行，跨过可扫描边界

   • 证据: 新增三个内聚的 describe（write validation / pre-write veto scope / rollback observability），共约 480 行。
   • 最小可行修复: 抽到 executor.validation.test.ts，与既有 makeConfig/makeStep helper 共享同一 fixture 模块。

行级发现

• [packages/core/src/executor/executor.ts:752] 这条写盘后分支依赖 result.content/toolParams.content，但真实 create_file/apply_patch 都不返回 content、局部行补丁也没有 content 参数，因此局部补丁写盘后实际不做任何内容校验；建议在此处按已知 path 从磁盘读回内容再 validateCode。
• [packages/core/src/executor/executor.ts:303] 收窄后的回滚门禁只认 syntax_validity，而语法失败已在写盘前 return，所以这个分支在仓库内两个 file client 下没有可达触发点；修复发现 1 的写盘后内容来源后此门禁才有意义，否则请注明它只服务于外部 MCP client。
• [packages/core/src/executor/executor.ts:328] 合入表达式 results.some(r => !r.pass) 比 PR 描述的 !postValidation.pass 窄：纯工具失败（results: []）不再中止剩余计划，而 planner 给 read_file 的 file_exists 是 required:true，旧条件下会中止；请更新描述并补 executeSteps 级别回归测试。
• [packages/core/src/executor/executor.ts:690] 三个发射点共用同一事件形状，写盘前拦截与写盘后失败不可区分；建议加 stage 与 path 字段，否则 benchmarks/eval/report.mjs 统计出的拦截数会混入有意放行的 import 失败。
• [packages/core/src/types.ts:741] 新增的 writeLeftOnDisk 没有任何生产读者（progress-enforcement.ts:42 只读 needsRollback，PhaseRunner 两者都不读）；要么让调度层消费它，要么删除并保留 error 文案与 rollback_failed 事件。
• [packages/core/src/executor/executor.test.ts:774] mock 让写工具返回 content，真实 applyPatch/createFile 从不返回该字段，因此这条"回滚生效"的用例证明不了生产行为；请改用 {success, snapshotId} 并让坏文件真实落盘。
• [packages/core/src/agent/agent.test.ts:240] 通过 cast 取私有 executor 字段绕过了公共 API，接线方式变更时该测试不会失败；建议改由公开执行入口 + addEventListener 驱动。

Karpathy 评审

• 假设: 前置门禁默认"整文件 replace 是 modify 步骤的唯一形态"，这一点对 codegen 路径成立（executor-skills.ts:278-288），但 resolveFullFileReplaceContent 依赖 collectedContext.files 中的原文件行数与工具端一致——两者同源，已核实。未成立的假设是"局部补丁会落回写盘后判定"（见发现 1）。
• 简洁性: narrowToPreWriteVeto 单成员白名单 + 两处复用是合理的收敛点，注释解释了为何只给 syntax_validity 否决权，可接受；resolveWriteContent 按动作分派、不做 content 跨动作兜底是正确取舍。回滚那一支则是当前不可达的额外机制 + 一个新事件 + 一个无消费者的输出字段，属于净增复杂度。
• 结构质量: executor.ts 从 ~750 增至 973 行，未越界；两个模块级纯函数抽取得当。退化点集中在 ExecutorOutput 多了一个无人读的字段，以及事件语义未分层。
• 变更范围: 大体聚焦；needsRollback 对非写步骤中止语义的改动超出 #387/#388 的目标，且未在描述中如实说明。
• 验证: 前置门禁用真实 HallucinationGuard + 评测集真实失败样本验证，质量高（executor.test.ts 的 fence/.tsx、别名 import、前向引用三例都切中要害）。回滚相关两例建立在虚构返回形状上，置信度低。

缺失覆盖

• 局部行补丁写入语法非法内容后的真实端到端用例：用真实 FileMCPClient 执行，断言磁盘上的文件内容（当前无任何测试覆盖"补丁写入的坏内容被发现或被撤销"）。
• Executor.executeSteps 层面的中止语义回归：read_file 工具失败后，后续步骤是否应继续（新行为）——目前只有单步 needsRollback 断言。
• create_file 覆写既有文件（overwrite: true，快照类型为 modify）时前置门禁拦截后原文件保持不变的断言。
• desktop consoleReducer 对 rollback_failed 分支的 reducer 测试（新增 case 无测试覆盖）。

[github-actions]

这条写盘后分支依赖 result.content/toolParams.content，但真实 create_file/apply_patch 都不返回 content、局部行补丁也没有 content 参数，因此局部补丁写盘后实际不做任何内容校验；建议在此处按已知 path 从磁盘读回内容再 validateCode。

[github-actions]

收窄后的回滚门禁只认 syntax_validity，而语法失败已在写盘前 return，所以这个分支在仓库内两个 file client 下没有可达触发点；修复发现 1 的写盘后内容来源后此门禁才有意义，否则请注明它只服务于外部 MCP client。

[github-actions]

合入表达式 results.some(r => !r.pass) 比 PR 描述的 !postValidation.pass 窄：纯工具失败（results: []）不再中止剩余计划，而 planner 给 read_file 的 file_exists 是 required:true，旧条件下会中止；请更新描述并补 executeSteps 级别回归测试。

[github-actions]

三个发射点共用同一事件形状，写盘前拦截与写盘后失败不可区分；建议加 stage 与 path 字段，否则 benchmarks/eval/report.mjs 统计出的拦截数会混入有意放行的 import 失败。

[github-actions]

新增的 writeLeftOnDisk 没有任何生产读者（progress-enforcement.ts:42 只读 needsRollback，PhaseRunner 两者都不读）；要么让调度层消费它，要么删除并保留 error 文案与 rollback_failed 事件。

[github-actions]

mock 让写工具返回 content，真实 applyPatch/createFile 从不返回该字段，因此这条"回滚生效"的用例证明不了生产行为；请改用 {success, snapshotId} 并让坏文件真实落盘。

[github-actions]

通过 cast 取私有 executor 字段绕过了公共 API，接线方式变更时该测试不会失败；建议改由公开执行入口 + addEventListener 驱动。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 高
处理建议: 请求修改
决策摘要: ** 事件接线、回滚终态、needsRollback 拆分都是对的，但把 syntax_validity 升级为写盘否决权 + 回滚触发权时没有校准该检查的可靠性——它是逐行引号奇偶 + 括号栈的启发式（packages/hallucination-guard/src/checks/syntax-validity.ts:190-216），对 "it's"、多行模板字符串、JSX 文本必然误判 block，合并后这类合法文件将根本写不出来，且已落盘的合法内容会被回滚删除/还原。

级联分析

• 变更符号: 原始模型未提供结构化级联字段。
• 受影响流程: 原始模型未提供结构化级联字段。
• 变更集外调用方: unknown
• 置信度: degraded

问题发现

1. [高] 写盘否决权授予了启发式语法检查，会阻断合法文件落盘
   • 证据: PRE_WRITE_VETO_CHECKS 只放行 syntax_validity（executor.ts:32），而该检查的实现是逐行引号奇偶判定 + 朴素括号栈：syntax-validity.ts:194-216 对任意单行内奇数个 '/"/` 直接产出 severity: 'block'。const msg = "it's fine";、<p>Don't</p>、跨行模板字符串首行都会命中；checkCommonPatterns 自己的 message 都写成 "Possible unclosed …"。HallucinationGuard 默认 syntaxValidity: true（guard.ts:69），agent 生产路径直接使用（agent/agent.ts:93-97）。
   • 受影响调用方/流程: 每一个 create_file 与 codegen 整文件 replace。改动前这类误判发生在写盘后（文件仍在磁盘、后续步骤/tsc 可裁决），改动后文件根本不产生；LLM 重生成同样内容会再次命中同一启发式，构成确定性失败环。FrontAgent 的主要产物是 React/TSX 组件，命中面很宽。
   • 最小可行修复: 把否决资格从「check type」细化到「错误类别」——只对确定性证据放行否决（checkBrackets 的未匹配/未闭合括号、JSON.parse 失败、以及 #387 举证的 markdown 围栏 TS1127 类），排除 checkCommonPatterns 产出的 "Possible unclosed …" 启发式项；details.errors[].message 已带足够信息可判别。
2. [高] 同一误判类别现在获得回滚（删除/还原）权
   • 证据: executor.ts:303-304 用同一个 narrowToPreWriteVeto 作为回滚触发口径，配合新增的写盘后读回（executor.ts:788），局部行补丁的最终内容首次进入 validateCode。一次引号奇偶误判即触发 rollbackFailedWrite，create 快照回滚是 unlinkSync、modify 快照回滚是覆盖还原（packages/mcp-file/src/snapshot.ts:106）。
   • 受影响调用方/流程: 交互模式（CLI/desktop）与评测。注意 benchmarks/eval/run-eval.mjs:94 传入 onApprovalRequest，packages/runtime-node/src/run.ts:182 据此把 interactive 置为 true，因此评测里 rollback 会被自动批准并真正执行——PR 描述中「默认非交互、即评测环境会拒绝回滚」的前提对这个 harness 不成立，评测将实际发生文件删除/还原。
   • 最小可行修复: 与 finding 1 共用同一份收窄后的「确定性错误类别」；在此之前不要让回滚由该检查单独驱动。
3. [高] 前置否决后不再中止计划，且后续步骤会被记为「跳过成功」
   • 证据: 否决路径固定返回 needsRollback: false（executor.ts:266），与本 PR 自述的「真实检查失败 → 中止」不一致。改动前同一失败在写盘后判定，step.validation 对写步骤恒为 required: true，progress-enforcement.ts:45 会中止剩余步骤；改动后计划继续，而目标文件并不存在。后续针对该文件的 apply_patch 会拿到工具的 Cannot apply patch: file does not exist，被 skills/executor-skills.ts:291-299 判为可跳过，进而由 buildSkippedStepOutput 记成 success: true（executor.ts:355-357）。
   • 受影响调用方/流程: Executor.executeSteps 的直接嵌入方——一次运行可能在「零文件产出」的情况下整体呈现为成功。
   • 最小可行修复: 否决路径返回 needsRollback: true（磁盘无残留，writeLeftOnDisk 保持 false），中止语义与其余真实检查失败一致。
4. [中] 复用前置结果作为写盘后判定，会跳过对「误判为整文件」补丁的实际落盘内容校验
   • 证据: executor.ts:773-775 一旦存在前置结果就直接返回，读回逻辑（executor.ts:788）被短路。整文件判定依赖 collectedContext.files.get(path) 的行数（executor.ts:674、resolveFullFileReplaceContent），而 step-callbacks.ts:86-106 只在 read_file / create_file 后刷新该 Map——apply_patch 成功后不刷新。同一计划内二次修改同一文件时，若真实文件行数多于上下文快照，mcp-file 会按 startLine:1..staleLines 做局部替换并保留尾部（apply-patch.ts:97,114），最终落盘内容 ≠ 被校验的 patch.content，而写盘后校验已被复用结果跳过。
   • 受影响调用方/流程: 多次修改同一文件的计划；#387 中「非法补丁内容不得留在磁盘」在该分支不成立。
   • 最小可行修复: 复用前先比对 readWrittenFile(path) 与已校验内容，不一致时按读回内容重新校验（读回逻辑本 PR 已具备，成本仅一次同步读）。
5. [低] validation_failed.path 与实际校验对象可能不一致
   • 证据: executor.ts:698 取 step.params.path，而校验路径解析用的是 toolParams.path ?? step.params.path（executor.ts:662）。技能改写过 path 的步骤，事件里的 path 会缺失或指向旧值。
   • 受影响调用方/流程: #388 的拦截遥测与 desktop 日志中的 [path] 前缀。
   • 最小可行修复: 把已解析的 writeContent.path 传入 emitValidationFailed。

行级发现

• [packages/core/src/executor/executor.ts:32] syntax_validity 整类获得写盘否决权，但该检查含逐行引号奇偶启发式（"Possible unclosed …"），"it's"`/多行模板/JSX 文本会被判 block；请把否决资格细化到确定性错误类别（括号未闭合、JSON 解析失败、围栏类），排除 "Possible" 启发式项。
• [packages/core/src/executor/executor.ts:266] 前置否决返回 needsRollback: false，使最严重的失败类别不再中止剩余步骤，后续 apply_patch 会被判为「跳过成功」；应返回 true（磁盘无残留由 writeLeftOnDisk: false 表达）。
• [packages/core/src/executor/executor.ts:304] 回滚触发沿用同一收窄集合，因此同一启发式误判会触发 unlinkSync/覆盖还原；请与否决口径一起收窄到确定性错误类别后再赋予回滚权。
• [packages/core/src/executor/executor.ts:773] 复用前置结果会跳过读回校验；当 collectedContext 相对磁盘过期时，被判为「整文件 replace」的补丁实际是局部替换，落盘内容从未被校验——复用前应与 readWrittenFile(path) 比对，不一致则改用读回内容重校。
• [packages/core/src/executor/executor.ts:698] 事件用 step.params.path，与校验实际使用的 toolParams.path ?? step.params.path 不同源；传入已解析的 writeContent.path 更准确。

Karpathy 评审

• 假设: 模型输出需要归一化为固定 Markdown 契约。
• 简洁性: 已提取 summary、finding、evidence 与 fix；原始 prose 不再附在评论中，避免占用下游解析与代理上下文。
• 变更范围: 原始模型未提供结构化范围字段。
• 验证: 需要查看 CI、测试或人工 CR 证据补强合并信心。

缺失覆盖

• 输出未命中 Repo Guard Markdown 契约；建议补充真实模型质量评估覆盖。

[github-actions]

syntax_validity 整类获得写盘否决权，但该检查含逐行引号奇偶启发式（"Possible unclosed …"），"it's"`/多行模板/JSX 文本会被判 block；请把否决资格细化到确定性错误类别（括号未闭合、JSON 解析失败、围栏类），排除 "Possible" 启发式项。

[github-actions]

前置否决返回 needsRollback: false，使最严重的失败类别不再中止剩余步骤，后续 apply_patch 会被判为「跳过成功」；应返回 true（磁盘无残留由 writeLeftOnDisk: false 表达）。

[github-actions]

回滚触发沿用同一收窄集合，因此同一启发式误判会触发 unlinkSync/覆盖还原；请与否决口径一起收窄到确定性错误类别后再赋予回滚权。

[github-actions]

复用前置结果会跳过读回校验；当 collectedContext 相对磁盘过期时，被判为「整文件 replace」的补丁实际是局部替换，落盘内容从未被校验——复用前应与 readWrittenFile(path) 比对，不一致则改用读回内容重校。

[github-actions]

事件用 step.params.path，与校验实际使用的 toolParams.path ?? step.params.path 不同源；传入已解析的 writeContent.path 更准确。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 高
处理建议: 请求修改
决策摘要: ** 主体方向正确且测试扎实，但 apply_patch 写盘后校验仍会被残留 content 参数遮蔽，导致本 PR 为 #387 声称的补丁路径覆盖在该状态下失效；再加上新公开字段 writeLeftOnDisk 的语义与命名/注释不符，建议这两处修好再合。

级联分析

• 变更符号: 原始模型未提供结构化级联字段。
• 受影响流程: 原始模型未提供结构化级联字段。
• 变更集外调用方: unknown
• 置信度: degraded

问题发现

1. [高] apply_patch 的写盘后校验仍被从未落盘的 content 遮蔽
   • 证据: executor.ts:811-815 的取值链是 result.content ?? toolParams.content ?? step.params.content ?? readWrittenFile(path)。resolveWriteContent（executor.ts:685-692）刻意按 action 分派以避免这个陷阱，注释写明「校验的就是一份不会被写入的内容……比修复前更糟」，但同一份危害在写盘后路径原样保留。planner.ts:323 的 params: llmStep.params as Record<string, unknown> 是自由形状，executor-skills.ts:286-289 的 apply_patch 分支返回 {...params, patches}，所以 {path, content, patches} 可达——这正是本 PR 自己论证并写了测试的状态。
   • 受影响调用方/流程: 所有走 validateAfterExecution 的写步骤；直接削弱 #387 在补丁路径上的验收口径。
   • 最小可行修复: 写盘后同样按 action 分派——apply_patch 直接用 readWrittenFile(path)，只对 create_file 保留 toolParams.content/step.params.content。
2. [中] writeLeftOnDisk 表达的是「回滚尝试失败」，不是「写入仍在磁盘」
   • 证据: 该字段只在 rollbackFailedWrite 返回 leftOnDisk: true 时为真，而回滚仅在落盘内容含围栏时才触发（executor.ts:320-323）。写盘后校验因其他原因判失败（未解析的相对 import、guard 语法启发式）时，文件原样留在工作区，字段却是 false。types.ts:736-741 的注释「已落盘的写入未被成功撤销——坏文件仍在工作区里」和 progress-enforcement.ts:42-44 的「写入卡在磁盘上没被撤销」都描述了这个更宽的事实，实现只覆盖了子集。
   • 受影响调用方/流程: 当前无功能性 bug（progress-enforcement.ts:45 与 needsRollback 做了 OR），但这是 ExecutorOutput 上的新公开字段，下一个消费者（例如评测侧统计「有多少坏文件留在盘上」）会按注释理解并读到系统性偏低的值。
   • 最小可行修复: 二选一——改名为 rollbackFailed 使名实相符；或在「写工具产生了 snapshotId 且写盘后判失败但未回滚」时也置 true。
3. [中] PR 描述与最终实现不符，且 Fixes #387 会关闭尚未满足验收标准的 issue
   • 证据: 描述称「Only syntax_validity can veto a write (PRE_WRITE_VETO_CHECKS)」，但仓内不存在 PRE_WRITE_VETO_CHECKS；实际否决判据是 detectMarkdownFence（executor.ts:43-65），只拦 markdown 围栏。收窄本身有充分依据——checks/syntax-validity.ts:190-216 逐行数引号奇偶，"it's fine" 会被判 severity: 'block'，把它作为写盘否决权确实比缺陷本身更糟，最新提交 c1c6d79 正是为此回退。问题在于评审者据描述作出的合并判断与实际代码不是同一个变更。
   • 受影响调用方/流程: #387 的验收标准是「a create/patch step whose content is syntactically invalid must not leave the file on disk」；当前只有围栏子集被拦，其余非法内容照旧落盘且不回滚。Fixes #387 合并即自动关闭。
   • 最小可行修复: 更新描述以匹配围栏判据，把关联关系降级为 Relates to #387（或明确保留 issue 并链接 #413 追踪启发式误判），并在描述里显式写出剩余缺口。
4. [低] 否决路径上白跑一次完整 guard
   • 证据: executor.ts:254-262 先 await validateCode（含会命中文件系统的 import 解析），executor.ts:263-266 才算围栏否决；否决触发时 rawContentValidation 被直接丢弃。
   • 受影响调用方/流程: unknown
   • 最小可行修复: 先算 buildFenceVeto 并提前返回，再对未被否决的内容跑 validateCode。
5. [低] validation_failed 计数口径未随事件语义更新，#388 的「拦截数」会被前向引用 import 淹没
   • 证据: benchmarks/eval/report.mjs:29 的 sumEvent 按事件类型直接累加，不区分 stage。而 emitValidationFailed('post_write', ...)（executor.ts:316）会在复用的 rawContentValidation 上触发，checks/import-validity.ts:84-90 对「同一计划里后续步骤才创建的相对模块」返回 severity: 'block'——这是多文件计划的常态。PR 描述称「the count is usable as an interception count」，但按当前口径统计出的数会由这类良性判定主导。
   • 受影响调用方/流程: unknown
   • 最小可行修复: 事件已带 stage，在 report 侧按 stage === 'pre_write' 计拦截数，或在 PR 描述中写明该计数需要按 stage 拆分后才可用。
6. [低] executor.test.ts 的全部改动是两个未使用 import
   • 证据: existsSync（第 1 行）与 AgentEvent（第 8 行）在该文件内再无引用（全文搜索仅命中 import 行），用例应是被搬到了 executor.validation.test.ts。biome.json:24-27 把 noUnusedImports 设为 warn，所以 quality:precommit 退出 0 与此并不矛盾。
   • 受影响调用方/流程: unknown
   • 最小可行修复: 整体还原 executor.test.ts，让该文件不出现在本次 diff 中。

行级发现

• [packages/core/src/executor/executor.ts:815] 新增的读回兜底位于 toolParams.content/step.params.content 之后，apply_patch 步骤只要携带残留 content 就永远走不到这里，落盘补丁内容不受校验；按 action 分派，apply_patch 直接用 readWrittenFile(path)。
• [packages/core/src/types.ts:741] 字段实际只表示「回滚尝试且失败」，非围栏类写盘后失败时文件仍在盘上却为 false；改名为 rollbackFailed，或在「有 snapshotId、判失败、未回滚」时同样置 true。
• [packages/core/src/executor/executor.ts:254] 完整 validateCode（含文件系统 import 解析）在围栏否决判定之前无条件执行，否决路径上结果被丢弃；先算 buildFenceVeto 并提前返回。
• [packages/core/src/executor/executor.test.ts:1] existsSync与第 8 行的AgentEvent` 在本文件内均未使用，本文件的唯一改动是这两个残留 import，建议整体还原。

Karpathy 评审

• 假设: 模型输出需要归一化为固定 Markdown 契约。
• 简洁性: 已提取 summary、finding、evidence 与 fix；原始 prose 不再附在评论中，避免占用下游解析与代理上下文。
• 变更范围: 原始模型未提供结构化范围字段。
• 验证: 需要查看 CI、测试或人工 CR 证据补强合并信心。

缺失覆盖

• 输出未命中 Repo Guard Markdown 契约；建议补充真实模型质量评估覆盖。

[github-actions]

新增的读回兜底位于 toolParams.content/step.params.content 之后，apply_patch 步骤只要携带残留 content 就永远走不到这里，落盘补丁内容不受校验；按 action 分派，apply_patch 直接用 readWrittenFile(path)。

[github-actions]

字段实际只表示「回滚尝试且失败」，非围栏类写盘后失败时文件仍在盘上却为 false；改名为 rollbackFailed，或在「有 snapshotId、判失败、未回滚」时同样置 true。

[github-actions]

完整 validateCode（含文件系统 import 解析）在围栏否决判定之前无条件执行，否决路径上结果被丢弃；先算 buildFenceVeto 并提前返回。

[github-actions]

existsSync与第 8 行的AgentEvent` 在本文件内均未使用，本文件的唯一改动是这两个残留 import，建议整体还原。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 中
处理建议: 请求修改
决策摘要: ** 主体设计是对的（前置校验 + 收窄否决口径 + 回滚终态），级联面也确实很小；但新增的写盘否决绕过了 hallucinationGuard.enabledChecks（本仓已有测试与消融基准明确依赖 guard 可关），且对 .yaml 存在真实误伤路径，合并前需要给否决加语言与配置门控。

级联分析

• 变更符号: 原始模型未提供结构化级联字段。
• 受影响流程: 原始模型未提供结构化级联字段。
• 变更集外调用方: unknown
• 置信度: degraded

问题发现

1. [中] 写盘否决绕过 guard 的 enabledChecks，使 guard 不再可关闭
   • 证据: executor.ts:258 的 buildFenceVeto 是模块级纯函数，不查 this.config.hallucinationGuard 的任何开关；packages/hallucination-guard/src/guard.ts:62 的 enabledChecks 为 private 且只有 setCheckEnabled 无读取入口。仓内 executor.test.ts:413 正是「honors a fully disabled hallucination guard on the executor validation path」这一契约的守卫测试——它只是因为用例内容不含围栏才继续通过。benchmarks/eval/report.mjs:67-69 记录的头号缺陷就是「guard 无法通过公开配置关闭，导致消融臂失效」。
   • 受影响调用方/流程: 任何以 enabledChecks: { syntaxValidity: false } 甚至全关配置构造 guard 的调用方；消融基准的 ablation 臂从此再也拿不到干净的「guard 关」对照。
   • 最小可行修复: 给 guard 加只读访问器（如 isCheckEnabled('syntaxValidity')），或在 ExecutorConfig 上加显式 preWriteVeto?: boolean（默认 true），在 executor.ts:257-259 处门控；并补一条「全关 guard + 围栏内容照常写入」的用例，把这条契约真正锁住。
2. [中] 否决判据被应用到 yaml，块标量里的围栏是合法内容
   • 证据: executor.ts:702 只要 detectLanguage 非 null 就返回 writeContent，而 phase-ordering.ts:144-159 的返回值包含 json 与 yaml；detectMarkdownFence 的正则 /^\s*```/ 对 YAML 块标量（如 issue template 的 value: | 内嵌 markdown 代码块）会命中。而 executor.ts:35-38 的文档注释自己写的适用范围是「.ts/.tsx/.js」——代码比注释宽。json 不受影响（裸 ``` 无法出现在行首）。
   • 受影响调用方/流程: 写 .yaml/.yml 的 create_file 步骤会被否决且步骤判失败；在 progress-enforcement 路径下还会连带中止剩余计划。叠加问题 1，用户没有任何配置手段绕开。
   • 最小可行修复: 在 resolveWriteContent 或否决调用点把 veto 限定为 language === 'typescript' || language === 'javascript'，让 yaml/json 保持原有的写盘后判定语义。
3. [中] #388 的验收「interceptions are countable」只完成了一半
   • 证据: 事件已带 stage（types.ts:809-814），但唯一的计数消费者 benchmarks/eval/run-eval.mjs:97 是 events[e.type] = (events[e.type] ?? 0) + 1，只按 type 聚合；report.mjs:77-79 新增的说明自己承认「必须按 stage 分开计数」，却没有任何代码这么做，而 report.mjs:75 的裸计数照旧输出。结果是：这一轮之后该数字第一次非零，但按报告自己的口径它不能当拦截数用。
   • 受影响调用方/流程: benchmarks/eval/report.mjs 生成的效果报告；#388 想回答的「guard 到底拦下了什么」。
   • 最小可行修复: run-eval.mjs:97 改为按 e.stage ? \${e.type}:${e.stage}` : e.type聚合，并把report.mjs的那行统计拆成pre_write`（真拦截）与其余两类分别输出，替换当前的散文提醒。
4. [中] needsRollback 新口径把「写步骤工具失败」也一并排除出中止条件
   • 证据: executor.ts:350 改为 postValidation.results.some(r => !r.pass)，而 validateAfterExecution:789-793 对工具失败返回 results: []。PR 描述给出的理由是 read_file（planner.ts:376 确认 read_file 默认 file_exists: required，旧条件确实会中止全盘），但同一改动同样覆盖 create_file/apply_patch：写工具报 EACCES 之后，progress-enforcement.ts:45 不再中止，计划继续跑。后续若恰好是针对该文件的 apply_patch，会在 executor-skills.ts:242 抛错兜住；但若后续步骤只是「引用该模块的另一个文件」，则整轮会以「缺模块但步骤全绿」的形式收尾。
   • 受影响调用方/流程: Executor.executeSteps 的直接嵌入方（progress-enforcement）。主 agent 路径不读该字段，不受影响。
   • 最小可行修复: 把「排除纯工具失败」限定在只读动作上，例如工具失败时对 WRITE_ACTIONS 仍返回 needsRollback: true，其余动作返回 false；并补一条 progress-enforcement 层的用例锁住这条分支。
5. [低] 回滚触发在读回失败时静默降级为「什么都没发生」
   • 证据: executor.ts:324-327 以 readWrittenFile(step.params.path) 的结果为门；readWrittenFile:765-774 在路径越界、文件不可读、或写工具的根目录与 config.projectRoot 不一致时一律返回 undefined，于是既不尝试回滚，rollbackFailed 也保持 false，validateAfterExecution:802-805 还会复用写盘前结论——三处都表现为「无异常」。
   • 受影响调用方/流程: headless 运行下「坏文件是否还在磁盘上」的可判定性，正是本 PR 第 4 节要解决的问题。
   • 最小可行修复: 当 landed === undefined 且 toolResult 带 snapshotId 且动作属 WRITE_ACTIONS 时，至少 debugWarn 一条并在 stepResult.error 注明「未能读回写入内容，回滚未尝试」。
6. [低] PR 描述与实现漂移
   • 证据: 描述第 5 节与 GitNexus 小结均称新增 ExecutorOutput.writeLeftOnDisk，实际落地字段是 rollbackFailed（types.ts:744）；Impact Scope 列了 executor.test.ts 为改动测试，但变更文件清单里没有它（新增的是 executor.validation.test.ts）。
   • 受影响调用方/流程: 评审者与后续读 changelog 的人。
   • 最小可行修复: 更新描述，或在合并说明里指出字段最终改名的提交（644a10b）。

行级发现

• [packages/core/src/executor/executor.ts:258] 否决判据不查 guard 的 enabledChecks，syntaxValidity: false 乃至全关配置也照样否决写盘；给 guard 加只读访问器或在 ExecutorConfig 上加显式开关来门控这一行。
• [packages/core/src/executor/executor.ts:702] 这里会把 yaml/json 也交给围栏否决，而 YAML 块标量里的 ``` 是合法内容；把 veto 限定为 typescript/`javascript`，其余语言维持写盘后判定。
• [packages/core/src/executor/executor.ts:59] 前置否决路径下文件并未落盘，错误文案却写「written into」，该串会随 stepResult.error 进入 recovery 反馈；按是否已落盘区分措辞（如 blocked before write / written into）。
• [packages/core/src/executor/executor.ts:325] 读回失败时这一门会静默跳过回滚且 rollbackFailed 仍为 false；补一条 WRITE_ACTIONS + 有 snapshotId 却读不回时的告警与 error 注记。
• [packages/core/src/executor/executor.ts:350] 该口径同时排除了写步骤的工具失败，progress-enforcement 不再中止；建议只对只读动作排除工具失败。
• [benchmarks/eval/report.mjs:77] 这里用散文说明「必须按 stage 分开计数」，但 run-eval.mjs:97 仍只按 e.type 聚合；把计数键改为 type:stage 并拆分本节统计，否则 #388 的指标依旧不可用。

Karpathy 评审

• 假设: 模型输出需要归一化为固定 Markdown 契约。
• 简洁性: 已提取 summary、finding、evidence 与 fix；原始 prose 不再附在评论中，避免占用下游解析与代理上下文。
• 变更范围: 原始模型未提供结构化范围字段。
• 验证: 需要查看 CI、测试或人工 CR 证据补强合并信心。

缺失覆盖

• 输出未命中 Repo Guard Markdown 契约；建议补充真实模型质量评估覆盖。

[github-actions]

否决判据不查 guard 的 enabledChecks，syntaxValidity: false 乃至全关配置也照样否决写盘；给 guard 加只读访问器或在 ExecutorConfig 上加显式开关来门控这一行。

[github-actions]

这里会把 yaml/json 也交给围栏否决，而 YAML 块标量里的 ``` 是合法内容；把 veto 限定为 typescript/`javascript`，其余语言维持写盘后判定。

[github-actions]

前置否决路径下文件并未落盘，错误文案却写「written into」，该串会随 stepResult.error 进入 recovery 反馈；按是否已落盘区分措辞（如 blocked before write / written into）。

[github-actions]

读回失败时这一门会静默跳过回滚且 rollbackFailed 仍为 false；补一条 WRITE_ACTIONS + 有 snapshotId 却读不回时的告警与 error 注记。

[github-actions]

该口径同时排除了写步骤的工具失败，progress-enforcement 不再中止；建议只对只读动作排除工具失败。

[github-actions]

这里用散文说明「必须按 stage 分开计数」，但 run-eval.mjs:97 仍只按 e.type 聚合；把计数键改为 type:stage 并拆分本节统计，否则 #388 的指标依旧不可用。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 高
处理建议: 请求修改
决策摘要: #388 的接线（stage 判别、拦截计数、rollback 终态）实现扎实且可合并，但 apply_patch 首次被纳入内容校验，直接把 #413 里已实测会误判的 checkSyntaxValidity（severity block）接到「步骤失败 / 计划中止」路径上，且未在 Known behaviour changes 里披露；另有 228 个与本变更无关、仓库内零引用的 fixture-deep 夹具文件和一个解析不到目标的 node_modules 链接。

级联分析

• 变更符号: Executor.executeStep（新增 pre-write 阶段与回滚判定）、Executor.validateAfterExecution、Executor.rollback（返回形状归一）、resolveWriteContent / buildFenceVeto / readWrittenFile（新增）、ExecutorOutput.rollbackFailed、AgentEvent（validation_failed 增 stage/path/stepId，新增 rollback_failed）、HallucinationGuard.isCheckEnabled（新公开 API）、ExecutorConfig.emitEvent。
• 受影响流程: create_file / apply_patch 写盘路径；写盘后校验与回滚触发；progress-enforcement 的中止判定；agent 事件流与三个前端消费者；消融评测两臂计数。
• 变更集外调用方: apps/cli/src/ui/bridge.ts:100 与 apps/vscode/src/state.ts:413 的 switch(event.type) 均有 default 分支，事件 union 扩展非破坏性（text search 已确认）。Executor.rollback 仅有 executor.ts:785 一个调用方，返回形状归一无外部影响。needsRollback 仅 progress-enforcement.ts:45 消费；agent 主路径走 executeStepsWithErrorFeedback（agent.ts:813），不读该字段——但主路径仍会看到步骤 success:false 并进入 recovery。enabledChecks 经 agent.ts:96 由 config.hallucinationGuard.checks 传入，run-eval.mjs:33 的 ablation 臂能真正关掉写盘否决，可消融性成立（graph 不可用，以上为 Read/Grep 直接查证）。
• 置信度: medium

问题发现

1. [高] apply_patch 首次被已知会误判的语法检查判定成败，且未披露

   • 证据: 改动前 validateAfterExecution 取 result.content ?? toolParams.content ?? step.params.content；codegen 的 apply_patch 分支只产出 patches（skills/executor-skills.ts:286-289，不设 content），因此补丁步骤此前从不进入 validateCode，恒为 pass:true。本 PR 后两条路径都会校验：整文件 replace 走 executor.ts:294 的 pre-write validateCode 并复用为写盘后结论，局部补丁走 executor.ts:862 读回磁盘再校验。validateCode（guard.ts:201-214）包含 checkSyntaxValidity，其 checkCommonPatterns（syntax-validity.ts:186-217）逐行数引号奇偶，失败时 severity:'block'（同文件 45-53 行）——即 issue #413 已实测的 "it's fine"、多行模板字符串、JSX 撇号全部判 block。
   • 受影响调用方/流程: 所有 modify 步骤。文件不会被删（回滚仍只由围栏触发），但步骤被判 success:false；executor.ts:386-388 使 needsRollback:true，progress-enforcement.ts:45 会跳过剩余全部步骤；agent 主路径则消耗 recovery 次数。同时 validation_failed:post_write 计数被误报污染，削弱 #388 想要的可观测性。PR 自己论证「这条检查不可靠到不能否决写盘」，却让它决定步骤成败与调度，两处口径不一致；#413「Interim state」也要求调用方不要依赖它。
   • 最小可行修复: 让 apply_patch 的写盘后结论与否决口径一致——把补丁路径的内容判定同样收敛到 buildFenceVeto（或从补丁路径的 results 中剔除 syntax_validity 条目）直到 #413 落地；并补一条断言：内容含 "it's fine" 的整文件 replace 补丁 stepResult.success === true。若确认是有意扩大校验面，至少写进 Known behaviour changes 并说明 recovery/中止的代价。

2. [中] 228 个与本变更无关的 fixture-deep 文件，且 node_modules 条目在检出中解析不到

   • 证据: 全仓库 grep fixture-deep 零命中，run-eval.mjs:12 仍指向 join(HERE, 'fixture')；benchmarks/eval/fixture-deep/* 下没有任何顶层文件（无 package.json / tsconfig.json / sdd.yaml，与 benchmarks/eval/fixture/ 的结构不同），因此它现在也无法作为评测工作区使用；benchmarks/eval/fixture-deep/node_modules 在本检出中读取报「不存在」（悬空链接）。
   • 受影响调用方/流程: 无运行时影响，但占本 PR 241 个文件中的 228 个，diff 无法聚焦；仓库多出一份无人引用、无 .gitignore、依赖入口悬空的夹具树，后续容易被误当作可用夹具。
   • 最小可行修复: 从本 PR 移出 benchmarks/eval/fixture-deep/**，随真正使用它的评测改动一起提交，并在那时补上 package.json / .gitignore，不要把 node_modules 链接提交进仓库。

3. [中] PR 描述与实现不符：writeLeftOnDisk 并不存在

   • 证据: 描述第 5 节、Impact Scope 和 GitNexus 影响摘要都称 ExecutorOutput.writeLeftOnDisk；types.ts:736-744 实际新增的是 rollbackFailed，其注释明确写着它不等于「写入仍在磁盘」（无快照时不会尝试回滚，文件照样留着而该字段为 false）。
   • 受影响调用方/流程: 按描述核对契约的评审者与后续接入方会找不到该字段，或误以为存在一个「文件是否残留」的直接信号。
   • 最小可行修复: 更新 PR 描述为 rollbackFailed，并同步其语义说明。

行级发现

• [packages/core/src/executor/executor.ts:862] 这条读回分支让每个补丁步骤首次进入 validateCode，把 syntax_validity 的 block 级误判（#413）变成步骤失败与计划中止信号；建议此处改用与写盘否决相同的高精度判据，或剔除 syntax_validity 结果，直到 #413 修复。
• [packages/core/src/executor/executor.ts:340] 写盘前用 toolParams.path ?? step.params.path 解析路径，这里的回滚判定只读 step.params.path；一旦技能改写 path（emitValidationFailed 的注释正是这么假设的），读回会落空、回滚静默不触发。建议把已解析路径从 resolveWriteContent 传下来复用。
• [packages/core/src/executor/executor.validation.test.ts:677] 该用例只断言「未被写盘前置校验拦截」，未断言 stepResult.success；按 syntax-validity.ts:194-216 的逐行引号计数，"it's fine" / 多行模板字符串 / JSX 撇号三例在写盘后仍会被判 block，步骤实际为失败。补一条 expect(result.stepResult.success).toBe(true) 才能锁住这个 describe 声称的契约。

Karpathy 评审

• 假设: 「围栏是高精度判据、其余校验维持旧的写盘后语义」这一前提对 create_file 成立，对 apply_patch 不成立——补丁路径此前根本没有写盘后内容语义，本 PR 是新建而非保留。这个隐含假设是问题 1 的根源。
• 简洁性: 否决判据小而独立，isCheckEnabled 让执行器侧门禁受同一份配置管辖（避免重演 #386），是正确的所有权归位。rollback() 返回归一消除了调用方各自 cast，方向正确。
• 结构质量: resolveWriteContent 按 action 分派、拒绝跨动作 content 兜底，是合理的边界选择；但同一份「按 action 取内容」的逻辑在 validateAfterExecution:856-862 又写了一遍，两处口径靠注释而非代码保持同步，后续容易漂移，可考虑合并为单一解析函数。executor.ts 已达约 1100 行，本 PR 新增 ~340 行集中在 executeStep 与校验辅助上，写盘校验这一簇（围栏判据、补丁解析、读回、回滚编排）具备自然的模块边界，建议后续拆分。
• 变更范围: 核心 diff 聚焦；benchmarks/eval/fixture-deep/** 的 228 个文件属于范围蔓延（问题 2）。
• 验证: 前置拦截、别名/前向引用放行、补丁内容优先于残留 content、回滚被拒的终态事件——这几条都用真实 HallucinationGuard 与真实磁盘断言，质量高。缺的是「合法代码在写盘后不应被判失败」这条正向断言（见行级发现 3），而它恰好是问题 1 会暴露的地方。

缺失覆盖

• 整文件 replace 补丁，内容含 "it's fine" 或多行模板字符串：断言 stepResult.success === true 且不产生 validation_failed（当前无覆盖，是问题 1 的回归门）。
• 局部行补丁写入合法代码后读回：断言步骤成功、无 validation_failed、无 rollback。
• executeStepsWithProgressEnforcement 层面的用例：一个内容合法的 modify 步骤失败时不应跳过后续步骤（验证新 needsRollback 口径不会因误判连锁中止整轮计划）。

[github-actions]

这条读回分支让每个补丁步骤首次进入 validateCode，把 syntax_validity 的 block 级误判（#413）变成步骤失败与计划中止信号；建议此处改用与写盘否决相同的高精度判据，或剔除 syntax_validity 结果，直到 #413 修复。

[github-actions]

写盘前用 toolParams.path ?? step.params.path 解析路径，这里的回滚判定只读 step.params.path；一旦技能改写 path（emitValidationFailed 的注释正是这么假设的），读回会落空、回滚静默不触发。建议把已解析路径从 resolveWriteContent 传下来复用。

[github-actions]

该用例只断言「未被写盘前置校验拦截」，未断言 stepResult.success；按 syntax-validity.ts:194-216 的逐行引号计数，"it's fine" / 多行模板字符串 / JSX 撇号三例在写盘后仍会被判 block，步骤实际为失败。补一条 expect(result.stepResult.success).toBe(true) 才能锁住这个 describe 声称的契约。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 中
处理建议: 请求修改
决策摘要: 核心方向正确（写盘前否决判据高精度、事件接线到位、回滚终态补齐），但写盘后的围栏判定作用在「整份落盘文件」而非本步骤写入的内容上，会误判并回滚对既有含围栏文件的合法补丁；同时 dropUnreliableSyntaxVerdicts 删除了全部写盘后 syntax_validity 判定，这与 PR 描述「其余判定保持原有写盘后语义」相矛盾且未列入行为变更；executor.ts 由约 770 行跨到约 1140 行，新增逻辑有天然模块边界。

级联分析

• 变更符号: Executor.executeStep / validateAfterExecution / rollback；新增 resolveWriteContent、emitValidationFailed、rollbackFailedWrite、readWrittenFile、buildFenceVeto、dropUnreliableSyntaxVerdicts；AgentEvent（+rollback_failed、validation_failed.stage）；ExecutorOutput.rollbackFailed；ExecutorConfig.emitEvent；HallucinationGuard.isCheckEnabled。
• 受影响流程: 写盘前后校验、首次真正触发的自动回滚、progress-enforcement 中止判定、desktop 日志、eval 事件计数。
• 变更集外调用方:
  • needsRollback 唯一消费点确为 packages/core/src/executor/progress-enforcement.ts:45（text search，全仓仅此一处读取）。
  • AgentEvent 新成员安全：apps/cli/src/ui/bridge.ts:100、apps/vscode/src/state.ts:413、apps/desktop/src/state/executionReducer.ts:342 均有 default 分支。
  • isCheckEnabled 在 executeStep 中对每个 step 无条件调用；ExecutorConfig.hallucinationGuard 是类类型，生产侧（agent.ts:107、runtime-node/src/run.ts:173）传真实实例，仅 executor.test.ts:341 的鸭子类型 mock 未补该方法——该用例走 pre-validation 早返回，未触达调用点，属潜在脆弱点而非当前破坏。
  • 前置判据的高精度已核实：/^\s*```/ 对全仓 .ts/.tsx/.js 无命中，'```' 形式的字符串常量（如 packages/sdd/src/workflow/phases/tasks.ts:42）不匹配；resolveFullFileReplaceContent 的形状与 packages/core/src/skills/executor-skills.ts:278-289 实际产出一致；planner.ts:325 + getDefaultValidation 确实给两个写动作发 required: true；安全层拒绝形状确为 {success:false,error}（tool-call-handler.ts:62-69），rollback() 归一化必要且正确。
• 置信度: medium — 工作区检出落后于 PR 头（缺 dropUnreliableSyntaxVerdicts 与 effectivePostValidation），这两处只能依据 diff 评审；我未运行测试，pnpm quality:precommit 结果按作者陈述采信。

问题发现

1. [高] 写盘后的围栏判定作用于整份落盘文件，且不区分工具是否成功

   • 证据: executor.ts 新增 366-377 行——landedContent 在 callTool 之后无条件读回（不看 toolResult.success），effectivePostValidation = landedFenceVeto.pass ? postValidation : landedFenceVeto 整体覆盖原判定；395 行据此触发 rollbackFailedWrite。
   • 受影响调用方/流程: 局部行 apply_patch（resolveFullFileReplaceContent 只覆盖 startLine === 1 的整文件替换，其余全部落到这条路径）。三个后果：① 目标文件在本步骤之前就含围栏时（正是 #387 证据里那批已落盘的 .tsx），任何合法局部补丁都会被判失败并回滚到同样含围栏的快照——修复这些文件的路径被堵死；② 工具报错（如 EACCES）时若磁盘上的旧文件含围栏，stepResult.error 会被替换成 "Markdown code fence written into …"，真实工具错误被掩盖，且仍会尝试回滚（交互模式下多一次审批弹窗）；③ 事件里的 path/line 归因到一个并未写过这些行的步骤。
   • 最小可行修复: 仅在 toolResult 成功时启用 landedFenceVeto；对局部补丁，用 collectedContext.files.get(path) 的原始内容先做一次 detectMarkdownFence，原本就有围栏则不归因本步骤（保留 postValidation 与工具错误）。

2. [中] dropUnreliableSyntaxVerdicts 删除全部写盘后 syntax_validity 判定，属未申报的范围扩张

   • 证据: executor.ts 新增 66-76 行，validateAfterExecution 的两个出口（复用分支与读回分支）都套用它。guard.validateCode 里 syntax_validity 是唯一的语法信号（guard.ts:201-204）。
   • 受影响调用方/流程: 真正的语法破损（#413 表中判定正确的 export const a = {）从此不再让步骤失败，也不再产生 validation_failed:post_write。PR 描述第 1 节称「Every other verdict keeps its old post-write meaning」，与此实现直接冲突；「Known behaviour changes」也未列。此外本 PR 同时在给消融基准加分阶段计数，而 syntaxValidity 开/关两臂的差异被这次删除进一步抹平，会影响正在建立的拦截率指标解释。
   • 最小可行修复: 保留结果条目但把 severity 降级为 warn（pass 计算不变，遥测与错误反馈仍可见），或维持删除但在 PR「Known behaviour changes」与 report.mjs 的阶段说明里写明「post_write 不再包含语法判定」。

3. [中] executor.ts 跨过 1000 行，新增逻辑有天然模块边界

   • 证据: 基线约 770 行（检出为 1091 行、含本 PR 大部分改动；diff stat +385/−14），PR 头约 1140 行。新增部分是一个内聚单元：WRITE_ACTIONS、detectMarkdownFence、buildFenceVeto、FENCE_VETO_LANGUAGES、dropUnreliableSyntaxVerdicts、resolveFullFileReplaceContent、resolveWriteContent、readWrittenFile、rollbackFailedWrite。同时 executeStep 主体膨胀到约 180 行，控制流被 20 余行论证性注释切断（286-292、311-313、360-364、419-425）。
   • 受影响调用方/流程: 后续改动写盘校验策略（#413 落地要换 parser、删掉 dropUnreliableSyntaxVerdicts）都要在这个文件里做，回归面无法隔离测试。
   • 最小可行修复: 抽出 packages/core/src/executor/write-validation.ts（纯函数：围栏判据 + 判定过滤 + 补丁内容解析），executor 只保留编排；readWrittenFile/rollbackFailedWrite 可留在类内。

4. [中] 新测试文件 848 行，脚手架成片复制，且含一对功能等价用例

   • 证据: executor.validation.test.ts:154-212 与 :543-601 是同一场景（同样的 apply_patch startLine:2,endLine:2、同样落盘 ```ts、同样断言回滚并恢复原文），只是断言侧重不同；另有 6 处约 40 行的 mkdtempSync + new Executor + registerMCPClient + registerToolMapping 逐字复制。
   • 受影响调用方/流程: 判据一旦调整（第 1、2 条修复都会动），需要同步修改 6 处近似副本；重复用例还会让「哪条锁的是哪个契约」难以判断。
   • 最小可行修复: 提取 makeGuardedExecutor({ projectRoot, events, allowRollback }) 辅助函数，合并那对重复用例，把两条断言并入一个用例。

5. [低] PR 描述与实现的字段名不一致

   • 证据: 描述第 5 节与 GitNexus 摘要多处写 ExecutorOutput.writeLeftOnDisk，实现是 rollbackFailed（types.ts:744），其文档还明确说明「刻意不叫 writeLeftOnDisk」。
   • 受影响调用方/流程: 依据描述评估爆炸半径的维护者会去找一个不存在的字段，也会误以为「写入是否仍在磁盘」已被单一字段表达（实际需要 rollbackFailed 与 needsRollback 并集）。
   • 最小可行修复: 更新 PR 描述第 5 节与 GitNexus 摘要中的字段名。

行级发现

• [packages/core/src/executor/executor.ts:377] effectivePostValidation 用整份落盘文件的围栏判定覆盖 postValidation，且上游 landedContent 不校验 toolResult.success；应仅在工具成功时启用，并对局部补丁与 collectedContext 里的原始内容比对，避免把既有围栏归因给本步骤、掩盖真实工具错误。
• [packages/core/src/executor/executor.ts:66] dropUnreliableSyntaxVerdicts 整类删除 syntax_validity，真实语法破损从此不再让步骤失败；建议降级为 warn 保留结果条目，或在行为变更清单中明写。
• [packages/core/src/executor/executor.ts:384] 该分支的错误文案「could not read back …; rollback was not attempted」会附加到任何读不回文件的写盘后失败上，但按新口径这类失败本就不触发回滚；应改为「无法读回落盘内容，未能核验」之类的表述，避免暗示漏掉了一次回滚。
• [packages/core/src/executor/executor.ts:53] 这行 doc comment 描述的是 buildFenceVeto，却挂在 dropUnreliableSyntaxVerdicts 的文档块之前，形成悬空注释；移到 buildFenceVeto 定义处或删除。
• [packages/core/src/executor/executor.ts:365] landedPath 取 step.params.path，而写盘前 resolveWriteContent 取 toolParams.path ?? step.params.path，emitValidationFailed 的注释还写着「技能可能改写过 step.params.path」；当前技能只做 ...params 透传所以两者相等，但同一路径应统一取值口径，否则未来任何改写 path 的技能都会让读回静默失效。
• [packages/core/src/executor/executor.validation.test.ts:543] 该用例与 154 行用例是同一场景同一断言目标，合并为一条并抽出共用的 executor 脚手架。

Karpathy 评审

• 假设: 「codegen 的 apply_patch 必为 startLine:1 整文件 replace」已在 executor-skills.ts:278-289 核实；但 resolveFullFileReplaceContent 依赖 collectedContext.files 的行数快照，而该 Map 只在 create_file 成功后刷新（step-callbacks.ts:98-106），同一计划内二次修改同一文件时前置校验会静默降级到写盘后——PR 在复用分支里处理了不一致，但没有测试锁住这条降级路径。另一未言明的假设是 rollback 工具已注册；未注册时 callTool 抛错被 rollback() 吞成 rollbackFailed: true，从而中止整个剩余计划。
• 简洁性: 围栏判据本身是正确的「窄而确定」取舍，值得保留。但写盘后一段同时存在三条内容来源（复用的 pre-write 结果、读回内容、landedFenceVeto）和三次同文件 readFileSync（366、复用分支、create_file 兜底），概念数偏多；抽成一个「解析本次落盘内容 + 判定」的单一函数可以让分支自然消失。
• 结构质量: 见问题 3、4——executor.ts 跨过 1000 行、测试文件 848 行且成片复制，是本轮最明确的退化项；判据逻辑本身是可独立测试的纯函数，拆分成本很低。
• 变更范围: 与既定目标基本吻合；dropUnreliableSyntaxVerdicts 是唯一超出 #388/#387 的行为变更（问题 2）。report.mjs/run-eval.mjs 的阶段计数属于 #388 的合理配套。
• 验证: 新增用例质量高——用真实 HallucinationGuard、真实落盘、并专门覆盖了「不该否决」的四类合法代码与 yaml 块标量，ablatable 用例锁住了 enabledChecks 管辖关系，这些是本 PR 最扎实的部分。缺口见下。

缺失覆盖

• 局部补丁作用在本来就含围栏的既有文件上：应断言合法补丁不被判失败、不触发回滚（对应问题 1 的核心风险，也是修复 #387 遗留脏文件的实际路径）。
• 写工具失败（success:false）且目标文件已含围栏：应断言 stepResult.error 保留工具错误、不发起回滚。
• dropUnreliableSyntaxVerdicts 的真阳性方向：export const a = { 这类真实破损写盘后现在会通过，没有任何用例记录这一既定行为。
• 同一计划内二次修改同一文件（collectedContext 行数已过期）：应断言复用分支检测到落盘内容不一致后按读回内容重判。
• rollback 工具未注册时的路径：当前会 rollbackFailed: true 并中止剩余计划，无用例说明这是预期。

一点说明：本次评审对 dropUnreliableSyntaxVerdicts 与 effectivePostValidation 只能依据 PR diff——工作区检出落后于 PR 头，不含这两处；其余结论均已在仓库中核对到具体行。

[github-actions]

effectivePostValidation 用整份落盘文件的围栏判定覆盖 postValidation，且上游 landedContent 不校验 toolResult.success；应仅在工具成功时启用，并对局部补丁与 collectedContext 里的原始内容比对，避免把既有围栏归因给本步骤、掩盖真实工具错误。

[github-actions]

dropUnreliableSyntaxVerdicts 整类删除 syntax_validity，真实语法破损从此不再让步骤失败；建议降级为 warn 保留结果条目，或在行为变更清单中明写。

[github-actions]

该分支的错误文案「could not read back …; rollback was not attempted」会附加到任何读不回文件的写盘后失败上，但按新口径这类失败本就不触发回滚；应改为「无法读回落盘内容，未能核验」之类的表述，避免暗示漏掉了一次回滚。

[github-actions]

这行 doc comment 描述的是 buildFenceVeto，却挂在 dropUnreliableSyntaxVerdicts 的文档块之前，形成悬空注释；移到 buildFenceVeto 定义处或删除。

[github-actions]

landedPath 取 step.params.path，而写盘前 resolveWriteContent 取 toolParams.path ?? step.params.path，emitValidationFailed 的注释还写着「技能可能改写过 step.params.path」；当前技能只做 ...params 透传所以两者相等，但同一路径应统一取值口径，否则未来任何改写 path 的技能都会让读回静默失效。

[github-actions]

该用例与 154 行用例是同一场景同一断言目标，合并为一条并抽出共用的 executor 脚手架。

[github-actions]

🛡️ ceilf6/repo-guard

代码评审报告: fix(executor): validate writes before disk, wire rollback, emit validation_failed

风险等级: 高
处理建议: 请求修改
决策摘要: 前置围栏否决 + validation_failed 接线这两块是对的且有真实证据支撑，但同一改动顺带把整套 guard（含 PR 自己论证为高误报的 import_validity）第一次接到了 apply_patch 的成败判定上，而 needsRollback 会因此中止整个剩余计划——这条新增的阻塞面没有任何测试覆盖，合并前需要先定性。

级联分析

• 变更符号: Executor.executeStep / validateAfterExecution / resolveWriteContent（新）/ rollbackFailedWrite（新）/ readWrittenFile（新）/ Executor.rollback；ExecutorOutput.needsRollback+rollbackFailed；AgentEvent.validation_failed（新增必填 stage）与 rollback_failed（新成员）；HallucinationGuard.isCheckEnabled（新）。
• 受影响流程: 所有 create_file / apply_patch 步骤的写盘判定；executeStepsWithProgressEnforcement 的中止调度；desktop 控制台日志；benchmarks/eval 的事件计数。
• 变更集外调用方: packages/core/src/executor/progress-enforcement.ts:45 是 needsRollback 的唯一消费者（已随本 PR 更新）；apps/cli/src/ui/bridge.ts、apps/cli/src/run-logger.ts:111（原样透传，无 switch 遗漏）、apps/vscode/src/state.ts:303 均无 validation_failed 分支，事件联合新增成员不会破坏它们；packages/runtime-node/src/run.ts:173,443 传入的是 HallucinationGuard 实例，新增方法对它们是加法。ExecutorConfig.hallucinationGuard 是具体类而非接口，所有 mock 均在包内，已同步。（证据来源：仓库内 Read/Grep，非代码图谱）
• 置信度: medium

问题发现

1. [高] apply_patch 首次继承 import_validity 的 block 判定，会让合法的修改步骤失败并中止整个计划

   • 证据: 改动前 validateAfterExecution 对 apply_patch 取 result.content ?? toolParams.content ?? step.params.content；planner 对 modify 步骤只发 params: { path, patches: [] }（packages/core/src/planner.ts:465），codegen 技能返回 {...params, patches:[patch]}（skills/executor-skills.ts:286-289），三个来源全为 undefined ⇒ 补丁路径实际上从不做内容校验。本 PR 后：整文件 replace 走 resolveWriteContent → rawContentValidation → executor.ts:884 复用为写盘后结论；局部补丁走 executor.ts:904 读回磁盘再跑 validateCode。两条路都会带上 import_validity。而 checkImportValidity（packages/hallucination-guard/src/checks/import-validity.ts:36-53,84-90）对 @/components/x 判「包未安装 → block」，对后续步骤才创建的相对模块判「无法解析 → block」——正是 PR 描述里点名不敢给写盘否决权的那两类误报。
   • 受影响调用方/流程: 每个 modify 步骤。失败后 executor.ts:426-428 的 results.some(r => !r.pass) 为真 ⇒ progress-enforcement.ts:45 把剩余步骤全部置 skipped；agent 主路径则改为消耗 recovery 次数去「修」一个本来正确的 import。
   • 最小可行修复: 对写盘后判定采用与写盘否决相同的收窄口径——只让确定性判据（围栏）决定 apply_patch 的成败，import_validity 的 block 在写动作上降为 warn（保留在 results 里供遥测），或至少显式声明「modify 步骤现在也受 import 检查阻塞」并补一条整文件 replace + 别名 import 的用例把预期钉死。现有的 does not veto a write over a path-alias import 只断言了没有 Pre-write validation failed，该步骤实际仍以失败收尾（走 executor.ts:384 的 unreadable 分支），测试名与真实行为不符。

2. [中] dropUnreliableSyntaxVerdicts 整条剔除 syntax_validity，真语法错误从此不再让任何步骤失败，且从事件载荷里消失

   • 证据: executor.ts:67 用 filter(r => r.type !== 'syntax_validity') 丢弃结果本身，而非只把它排除出 blockedBy。export const a = { 这类 #413 表格里判定正确的样本（block、真错），改动前会让 create_file 步骤失败，改动后 pass 为 true、步骤成功、文件留在磁盘上，围栏判据也覆盖不到它。
   • 受影响调用方/流程: 所有写步骤的写盘后判定；validation_failed:post_write 的载荷（validation.results 已被裁剪）；消融基准中 guard 臂的实际检出能力。#413 的「Interim state」只要求「不要给它写盘否决权」，本 PR 额外取消了它的写盘后判定，这一点没有出现在「Known behaviour changes」里。
   • 最小可行修复: 保留 results 条目、只把 syntax_validity 排除出 pass/blockedBy 计算（等价于就地降级为 warn），并在 PR 的 Known behaviour changes 里补一句「真语法错误不再让步骤失败」。

3. [中] executor.ts 越过 1000 行（761 → 1132），executeStep 单函数约 235 行，同目录已有可复用的拆分惯例

   • 证据: executor.ts 现为 1132 行；executeStep（executor.ts:212-447）里串起参数校验、写前否决、guard 调用、工具调用、读回、二次围栏判定、回滚编排、错误串拼装（executor.ts:399-414 的三层嵌套三元）八件事。同目录已有 progress-enforcement.ts、phase-ordering.ts、tool-call-handler.ts、step-trace-recorder.ts 这类按关注点拆出的兄弟模块。
   • 受影响调用方/流程: 后续任何改动写盘判定的人都要在这个函数里重新推演一遍「哪份内容、读了几次、谁触发回滚」。
   • 最小可行修复: 把 detectMarkdownFence / buildFenceVeto / dropUnreliableSyntaxVerdicts / resolveFullFileReplaceContent / readWrittenFile / rollbackFailedWrite 移到同目录新建的 write-validation.ts，executeStep 只保留编排。

4. [中] 同一次写盘最多读回同一文件三次，且写前/写后的 path 解析口径不一致

   • 证据: executor.ts:880（复用一致性比对）、executor.ts:366（landed 围栏判定）、executor.ts:903/904（内容取值）各读一次。写前用 toolParams.path ?? step.params.path（executor.ts:762），写后固定用 step.params.path（executor.ts:365,889）；emitValidationFailed 的注释（executor.ts:792）恰恰说明作者预期技能可能改写路径，两处口径就不该分叉。（现有技能都不改写 path，所以今天不会出错，属于隐性假设而非当前缺陷。）
   • 受影响调用方/流程: 写盘后判定路径；将来任何会重写 path 的技能会让读回落到错误文件、静默退化为「读不回」分支。
   • 最小可行修复: 在 executeStep 里读一次 landed 内容与一份 resolvedPath，作为参数传给 validateAfterExecution，删掉后两处独立读取。

5. [低] PR 描述与 GitNexus 影响摘要引用了不存在的字段 writeLeftOnDisk

   • 证据: 描述第 5 节、Impact Scope、GitNexus 摘要均写 ExecutorOutput.writeLeftOnDisk；代码里落地的是 rollbackFailed（packages/core/src/types.ts:744），语义也不同（后者明确「不等于文件还在磁盘上」）。
   • 受影响调用方/流程: 依赖影响摘要做合入判断的评审者。
   • 最小可行修复: 更新描述；顺带考虑把 needsRollback 改名为它现在真正的含义（是否中止剩余步骤）——回滚已由执行器内部完成，该字段与回滚无关。

6. [低] report.mjs 新增的分阶段表格放在一个断言「事件从未触发」的小节标题下

   • 证据: benchmarks/eval/report.mjs:74 的标题仍是「validation_failed 事件从未触发」，而本 PR 让第 82-84 行的计数首次可能非零，同一小节会自相矛盾。
   • 受影响调用方/流程: 后续评测报告的可读性。
   • 最小可行修复: 把标题改为中性表述（如「validation_failed 拦截计数」），或按 sum 是否为 0 分支输出结论句。

行级发现

• [packages/core/src/executor/executor.ts:774] 这里让整文件 replace 的补丁内容进入 validateCode，于是 apply_patch 第一次带上 import_validity 的 block 判定（别名 @/x、后续步骤才创建的相对模块），步骤失败并经 needsRollback 中止整个计划；建议对写盘后判定沿用与写盘否决相同的收窄口径（import 判定降为 warn），或补测试钉死预期行为。局部补丁路径在 :904 有同样问题。
• [packages/core/src/executor/executor.ts:67] filter 把 syntax_validity 条目整条删掉，真语法错误（export const a = {）从此既不失败也不出现在 validation.results 里；改为保留条目、只将其排除出 pass/blockedBy 计算，保住遥测可见性。
• [packages/core/src/executor/executor.ts:365] landedPath 只取 step.params.path，与写前的 toolParams.path ?? step.params.path（:762）口径不一致；按同一解析结果取一次路径并向下传递。
• [packages/core/src/executor/executor.ts:880] 这是同一文件在一次步骤内的第二次读回（第三次在 :903）；把 landed 内容读一次后作为参数传入 validateAfterExecution。
• [benchmarks/eval/report.mjs:77] 新表格挂在「事件从未触发」这一标题下，本 PR 后计数会非零；把标题改为中性表述或按 sum 分支输出。

Karpathy 评审

• 假设: 「计划 schema 不产出 patches，所有 modify 都走整文件 replace」经 planner.ts:465 + executor-skills.ts:286 核实成立；但 resolveFullFileReplaceContent 额外依赖 collectedContext.files 里有原文件——缺失时前置校验静默跳过，#387 对补丁路径的保护是有条件的，这一点没写进描述。另一处隐性假设是「技能不改写 path」（见发现 4）。
• 简洁性: 围栏判据本身是恰当的最小手术；但 dropUnreliableSyntaxVerdicts 是一个为绕开 #413 而生的临时层，它与「写前否决」「写后再判围栏」「复用写前结果」共同构成了四条彼此牵制的规则，读者必须同时持有四条才能推断一次写盘的结局。#413 落地后至少要删掉两条。
• 结构质量: executor.ts 越过 1000 行、executeStep 约 235 行、错误文案三层嵌套三元、同一文件读三次——同目录已有按关注点拆分的先例，应抽出 write-validation.ts（见发现 3、4）。needsRollback 保留旧名但语义已变为调度信号（types.ts 注释自己承认），属于名实不符。
• 变更范围: 主体聚焦。benchmarks/eval/* 的分阶段计数与 guard.isCheckEnabled 是必要配套。Executor.rollback 的返回归一化虽属顺带，但与 rollback_failed 直接相关，可接受。
• 验证: 新增用例扎实且用了真实 guard 与评测里的真实失败样本，pnpm quality:precommit 已跑。但覆盖的是「不否决写盘」，没有覆盖「步骤最终成败」——两个 import 相关用例都只断言了 Pre-write validation failed 不出现，掩盖了发现 1 的行为。

缺失覆盖

• 整文件 replace 的 apply_patch，内容含 @/x 别名或前向引用相对 import：断言步骤成败与 needsRollback，把发现 1 的预期行为钉死。
• create_file 写入真语法错误（如 export const a = {，非围栏）：断言剔除 syntax_validity 后的预期结局，防止发现 2 的能力退化被静默改回或永久遗忘。
• 集成层用例：写前否决返回 needsRollback: true 后，executeStepsWithProgressEnforcement 确实把剩余步骤置为 skipped（当前只在单步层断言了该字段）。
• apply_patch 且 collectedContext.files 缺少目标文件时前置校验被跳过的路径，确认降级到写盘后判定符合 #387 的验收口径。

[github-actions]

这里让整文件 replace 的补丁内容进入 validateCode，于是 apply_patch 第一次带上 import_validity 的 block 判定（别名 @/x、后续步骤才创建的相对模块），步骤失败并经 needsRollback 中止整个计划；建议对写盘后判定沿用与写盘否决相同的收窄口径（import 判定降为 warn），或补测试钉死预期行为。局部补丁路径在 :904 有同样问题。

[github-actions]

filter 把 syntax_validity 条目整条删掉，真语法错误（export const a = {）从此既不失败也不出现在 validation.results 里；改为保留条目、只将其排除出 pass/blockedBy 计算，保住遥测可见性。

[github-actions]

landedPath 只取 step.params.path，与写前的 toolParams.path ?? step.params.path（:762）口径不一致；按同一解析结果取一次路径并向下传递。

[github-actions]

这是同一文件在一次步骤内的第二次读回（第三次在 :903）；把 landed 内容读一次后作为参数传入 validateAfterExecution。

[github-actions]

新表格挂在「事件从未触发」这一标题下，本 PR 后计数会非零；把标题改为中性表述或按 sum 分支输出。