Fix/parser backed prewrite validation - #458
Open
NanluQingshi wants to merge 14 commits into
Open
Conversation
Move line-range patch projection into a reusable pure helper so executor preflight and the file tool evaluate identical projected content. Co-Authored-By: Claude <noreply@anthropic.com>
Replace quote and bracket heuristics with TypeScript parser diagnostics for TypeScript, JavaScript, JSX/TSX, and JSON. Expose syntax-only validation for deterministic pre-write callers while preserving enabled-check behavior. Fixes ceilf6#413 Co-Authored-By: Claude <noreply@anthropic.com>
Preflight create_file and arbitrary apply_patch content with the parser-backed guard, bind patch execution to the validated original hash, and serialize same-target writes so validated projections cannot race. Complete the general syntax-validation path left intentionally narrow in ceilf6#402 and surface validation failures consistently to desktop consumers. Co-Authored-By: Claude <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
代码评审报告: Fix/parser-backed prewrite validation (#413, #387)
风险等级: 高
处理建议: 请求修改
决策摘要: 解析器替换启发式这条主线是对的、覆盖也扎实，但 create_file 的 wx 失败路径会 unlinkSync 掉并发写入者刚创建的文件（数据丢失），且 PR 声称的「file-tool 边界写盘前校验」与「同目标写序列化」在代码里并不存在——合并前需要先修 race 回滚路径并把描述/CHANGELOG 与实现对齐。
级联分析
- 变更符号:
Executor.executeStep/ 新增validateWriteBeforeExecution、PhaseRunner.executePhaseSequential|Parallel、HallucinationGuard.validateSyntax|validateImports|validateCode、checkSyntaxValidity/新增validateSourceSyntax、applyPatch、createFile、新增applyFilePatches、detectLanguage、AgentEvent.validation_failed.stage。 - 受影响流程: agent
execute/planOnly/executeSteps的写盘链路；phase 顺序/并行执行与 recovery；apply_patch/create_fileMCP 边界；desktopconsoleReducer日志；benchmark 遥测聚合。 - 变更集外调用方:
packages/core/src/executor/progress-enforcement.ts:42-47（已有同款 needsRollback 中止语义，本 PR 使 PhaseRunner 与之对齐）；benchmarks/eval/report.mjs:89-90（只渲染pre_execution/post_write两个 stage，未加pre_write）；benchmarks/eval/run-eval.mjs:287-289（按type:stage通用聚合，无需改动）；PatchResult.validation在全仓无消费方（仅apply-patch.test.ts:370）。 - 置信度: medium —（graph 证据来自 PR 内 GitNexus 摘要，未由我复核）我的结论基于仓库只读检查与文本搜索：
needsRollback、syntaxValid、validateCode/Syntax/Imports、collectedContext.files.set均已全仓 grep 确认。
问题发现
-
[高]
create_file的wx失败路径会删除并发写入者刚创建的文件- 证据:
create-file.ts:86用flag: 'wx'写入；EEXIST 落入create-file.ts:95-98的 catch，调用snapshotManager.rollback(snapshotId)；该快照在create-file.ts:69-72因「检查时文件不存在」被建为'create'类型，而SnapshotManager.rollback对'create'快照执行unlinkSync（snapshot.ts:113-123）。 - 受影响调用方/流程: 并行 phase 中两个
create_file步骤命中同一路径（phase-runner.ts:198-205同一波内并发）；任何在 existsSync 检查与写入之间创建该文件的外部进程。 - 最小可行修复: 在 catch 中区分 EEXIST——直接返回既有的
File already exists: ...错误并跳过rollback（本次调用没有创建该文件，无权删除它）；其余错误保持现有回滚。
- 证据:
-
[高] file-tool 边界并未做「写盘前校验」，PR 描述与实现不符
- 证据:
apply-patch.ts:68计算validation后，apply-patch.ts:71-76无条件写盘并在78-83返回success: true；create-file.ts除wx外没有任何语法校验。全仓 grepsyntaxValid显示PatchResult.validation无消费方（仅apply-patch.test.ts:370断言），新加的apply-patch.test.ts:346-375用例本身就把「syntaxValid=false 而 success=true」固化为期望。 - 受影响调用方/流程: 任何不经
Executor.executeStep的写入（@frontagent/mcp-file作为独立 MCP server 被外部 client 调用、hook 路径、未来调用方）仍会把无效内容留在磁盘——正是 #387「no bad state ever exists」要消除的状态。附带成本：mcp-file/package.json:26为此新增@frontagent/hallucination-guard运行时依赖（连带typescript），每次 patch 额外全量解析一遍，收益为零。 - 最小可行修复: 二选一——(a) 在
applyPatch/createFile里真正拦截（syntaxValid === false时不写盘、返回success: false，并给非代码内容留显式 opt-out）；(b) 撤掉 mcp-file 对 guard 的依赖，把 PR 描述与 CHANGELOG 改为「仅 executor preflight 拦截」。
- 证据:
-
[中] JSON 走严格
JSON.parse且已升级为写盘否决权，JSONC 文件（tsconfig.json、.vscode/*.json）会被误拦并连带中止整个 phase- 证据:
executor.ts:577-578只跳过yaml，json进入否决路径；guard.ts:69中syntaxValidity默认true；syntax-validity.ts:178-181用JSON.parse。代码生成 prompt 明确要求产出tsconfig.json（llm/code-generation.ts:100），而主流 TS 模板的 tsconfig 带注释/尾逗号。失败后executor.ts:198置needsRollback: true，进而触发phase-runner.ts:295-300的整段 phase 跳过。 - 受影响调用方/流程: 所有生成配置文件的 create_file 步骤；这与 #413 要消除的 false block 属同一类，只是换了语言。
- 最小可行修复: 对
tsconfig*.json/jsconfig*.json/.vscode/*.json用容忍注释的解析（如ts.parseConfigFileTextToJson），或把 JSON 的 block 严格限定在纯 JSON 路径、其余降级为 warn。
- 证据:
-
[中] 「serialize same-target writes」未实现，描述与代码不一致
- 证据:
packages/core/src/executor下无任何 lock/queue/串行化实现（已 grepmutex|lock|queue|serialize|inFlight|sameTarget）；phase-runner.ts:198-205仍把同一波 ready 步骤全部并发提交。真正起作用的是apply-patch.ts:49-56的 hash 校验，它把竞争变成「后手失败」而非「顺序执行」。 - 受影响调用方/流程: 并行模式下同文件多补丁步骤——其中一个必然以
changed since executor preflight失败，且executor-skills.ts:292-294明确将其定为不可跳过。 - 最小可行修复: 修正描述为「hash 绑定拒绝陈旧基线」，或若确实要两个写都成功，则补一个按目标路径的顺序化调度。
- 证据:
-
[中] PhaseRunner 的 phase 级中止范围超出既定目标，且未在描述/CHANGELOG 说明
- 证据:
phase-runner.ts:220-222、226-235（并行）与295-300（顺序）对任意needsRollback失败中止整段 phase。needsRollback的既有来源包括executeStep内任何抛错（executor.ts:246-260）与 required validation 失败（executor.ts:244）；例如executor-skills.ts:242抛出的Cannot apply patch: file not found in context过去只失败该步，现在会把 phase 内无依赖关系的步骤一并标为skipped（recovery 成功时才可能被重跑，phase-runner.ts:399-423）。方向上与progress-enforcement.ts:42-47一致，可以接受，但属于未声明的行为变更，两个新测试只覆盖了刻意构造的写失败。 - 最小可行修复: 把跳过范围收敛为失败步骤的（传递）依赖方，或在 CHANGELOG 写明 phase-abort 语义并补一个「异常路径同样中止 phase」的测试。
- 证据:
-
[低] 遥测下游未同步：
pre_write阶段在消融报告里不可见- 证据:
benchmarks/eval/report.mjs:89-90只渲染pre_execution与post_write两行；语法拦截已整体迁移到pre_write（executor.ts:187），该表将对新的主要拦截来源恒显 0，与 #388「可计数的拦截量」目标冲突（run-eval.mjs:287-289已通用记录该键，无需改）。 - 最小可行修复: 在 report.mjs 的分阶段表格加一行
pre_write。
- 证据:
行级发现
- [packages/mcp-file/src/tools/create-file.ts:86]
wx的 EEXIST 分支落到 catch 里的snapshotManager.rollback()，而该快照是'create'类型 →unlinkSync掉并发写入者的文件；请在 catch 中识别 EEXIST，返回 already-exists 错误且不回滚。 - [packages/mcp-file/src/tools/apply-patch.ts:74] 上一行算出的
validation从未参与决策，syntaxValid === false时仍然写盘并返回success: true；要么在此之前拦截，要么去掉 mcp-file 对 guard 的运行时依赖并修正 PR 声明。 - [packages/core/src/executor/executor.ts:578] 只排除
yaml，使.json获得写盘否决权，而校验用的是严格JSON.parse；tsconfig/.vscode 这类 JSONC 会被误拦，请对这些路径改用容忍注释的解析。 - [packages/core/src/executor/phase-runner.ts:295]
needsRollback已被既有异常路径广泛置真，这里的break会跳过 phase 内互不依赖的步骤；建议把跳过限定在失败步骤的依赖闭包，或在 CHANGELOG 明确该语义。 - [packages/core/src/executor/executor.ts:707] 写后校验只剩 import 检查；当 preflight 返回
undefined（如create_file的content非字符串）时，语法校验将完全缺失，而此前validateCode会覆盖该情形。
Karpathy 评审
- 假设: 隐含假设「executor 是 file tool 的唯一调用方」——mcp-file 是独立 MCP server，该假设未被任何机制保证（发现 2）。另一处隐含假设是 collectedContext 中的内容与磁盘一致，现由 hash 校验兜底，方向正确。
- 简洁性:
applyFilePatches抽到@frontagent/shared是本 PR 最好的部分——删掉了 apply-patch 内约 130 行重复的边界/重叠校验，并让 preflight 与实际写入共用同一投影，属于真正减少复杂度的重组。反面是 mcp-file 里那条「算了但不用」的校验链路（发现 2），是纯增复杂度。 - 结构质量: guard 的
validateSyntax/validateImports拆分让validateCode变成组合调用，enabledChecks 语义保持一致，是干净的边界改进。syntax-validity.ts由 178 行手写扫描降为解析器驱动，理解成本明显下降。 - 变更范围: 超出既定目标的部分有两处：PhaseRunner 的 phase 级中止（发现 5）、
detectLanguage新增mts/cts（无害且被 preflight 需要）。phase-runner.test.ts:335的r→resolve重命名属无关噪声。 - 验证: 单测覆盖到位（TSX/JSX/正则/撇号/模板、TS-in-JS、stale hash、pre_write 事件、reducer 三阶段）。但没有一条测试覆盖本 PR 新引入的失败路径：EEXIST 回滚、JSONC 拦截、非 dryRun 下 syntaxValid=false 仍写盘。
缺失覆盖
createFile在 existsSync 之后、写入之前被并发创建：断言 EEXIST 后目标文件仍然存在（当前会被删除）。applyPatch非 dryRun、投影结果语法无效：断言磁盘内容与返回值符合期望的契约（现状是写入 +success: true，与 PR 描述矛盾）。create_file写tsconfig.json（含注释）：断言不被 pre-write 拦截，或明确记录为预期拦截。- executeStep 内部抛错（如 apply_patch 缺上下文）时，phase 内无依赖步骤是否应被
skipped——当前无测试固化该新语义。 - #387 的验收证据来自冻结的 30 任务 benchmark（
benchmarks/results/2026-07-12-sdd-ablation.md的 TS1127 落盘）。PR 的验证只列了单测与本地 gate；full 臂（guard 开启，run-eval.mjs:94）重跑一次并给出围栏不再落盘的结果，会显著提高对该 issue 已修复的信心。
| if (overwrite) { | ||
| writeFileSync(safePath.fullPath, content, 'utf-8'); | ||
| } else { | ||
| writeFileSync(safePath.fullPath, content, { encoding: 'utf-8', flag: 'wx' }); |
There was a problem hiding this comment.
wx 的 EEXIST 分支落到 catch 里的 snapshotManager.rollback()，而该快照是 'create' 类型 → unlinkSync 掉并发写入者的文件；请在 catch 中识别 EEXIST，返回 already-exists 错误且不回滚。
| writeFileSync(safePath.fullPath, newContent, 'utf-8'); | ||
| snapshotManager.updateSnapshotContent(snapshotId, newContent); | ||
| if (!existsSync(dir)) mkdirSync(dir, { recursive: true }); | ||
| writeFileSync(safePath.fullPath, projected.content, 'utf-8'); |
There was a problem hiding this comment.
上一行算出的 validation 从未参与决策，syntaxValid === false 时仍然写盘并返回 success: true；要么在此之前拦截，要么去掉 mcp-file 对 guard 的运行时依赖并修正 PR 声明。
| const path = typeof toolParams.path === 'string' ? toolParams.path : undefined; | ||
| if (!path) return undefined; | ||
| const language = detectLanguage(path); | ||
| if (!language || language === 'yaml') return undefined; |
There was a problem hiding this comment.
只排除 yaml，使 .json 获得写盘否决权，而校验用的是严格 JSON.parse；tsconfig/.vscode 这类 JSONC 会被误拦，请对这些路径改用容忍注释的解析。
| onStepComplete(step, output); | ||
| } | ||
|
|
||
| if (!output.stepResult.success && output.needsRollback) { |
There was a problem hiding this comment.
needsRollback 已被既有异常路径广泛置真，这里的 break 会跳过 phase 内互不依赖的步骤；建议把跳过限定在失败步骤的依赖闭包，或在 CHANGELOG 明确该语义。
| ); | ||
| return codeValidation; | ||
| if (language === 'typescript' || language === 'javascript') { | ||
| return this.config.hallucinationGuard.validateImports(content, path); |
There was a problem hiding this comment.
写后校验只剩 import 检查；当 preflight 返回 undefined（如 create_file 的 content 非字符串）时，语法校验将完全缺失，而此前 validateCode 会覆盖该情形。
Use TypeScript JSON parsing for tsconfig, jsconfig, and VS Code settings while keeping package and application JSON strict. Co-Authored-By: Claude <noreply@anthropic.com>
Enforce parser validation at create and patch tool boundaries, preserve dry-run diagnostics, and discard speculative snapshots when exclusive creation loses an EEXIST race. Co-Authored-By: Claude <noreply@anthropic.com>
Defer same-target writes into later parallel waves, keep distinct targets concurrent, and let dependency failures skip only dependents instead of aborting unrelated phase work. Co-Authored-By: Claude <noreply@anthropic.com>
Render the new pre_write telemetry stage and execute the report against distinct arm counts to keep validation metrics complete. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
代码评审报告: Fix/parser-backed prewrite validation (#413, #387)
风险等级: 中
处理建议: 需要人工判断
决策摘要: 工程实现正确、测试扎实，两个 issue 的验收点基本达成；但 #413 明确留给维护者的打包决策在本 PR 中被顺带做掉了——typescript 成为 guard 的运行时依赖后会被整体打进已发布的 CLI bundle（build.mjs 的 external 未同步），且该决策没有任何体积测量或记录，需要维护者签字后再合。
级联分析
- 变更符号:
validateSourceSyntax/checkSyntaxValidity、HallucinationGuard.validateSyntax|validateImports|validateCode、applyFilePatches(新)、applyPatch、createFile、SnapshotManager.discardSnapshot、Executor.executeStep|validateWriteBeforeExecution|validateAfterExecution|getWriteTarget、PhaseRunnerDeps、AgentEvent.validation_failed.stage。 - 受影响流程: executor 写盘前拦截 →
callTool→ 写后 import 校验；phase 并行调度的同目标串行化；desktop 事件日志；benchmark 报表；mcp-file server dispatch；CLI/VSCode 打包图。 - 变更集外调用方（我实际查证，非图谱）:
packages/runtime-node/src/mcp-clients.ts:38-48进程内直传 args，__frontagentExpectedOriginalHash能到达applyPatch✅packages/mcp-file/src/server.ts:90stripInternalArgs会剥掉__frontagent*，走 stdio 的外部 MCP 路径上 stale-base 保护静默失效（非降级失败，是"检查不存在"）⚠️ PhaseRunner全仓仅executor.ts:60一处构造，新增必填getWriteTarget无遗漏调用方 ✅executor-skills.ts:292的新短路先于'Cannot apply patch'命中，stale 错误确实是致命而非静默跳过 ✅（新错误串同时含两个子串，顺序是对的）validateBeforeExecution:406-419会为 apply_patch 自动读文件入 context，因此 preflight 里 "original content unavailable" 分支基本是兜底 ✅HallucinationGuard新增两个 public 方法会结构性收紧ExecutorConfig.hallucinationGuard（本 PR 的 executor 测试 fake 全部要补两个方法即为证据），对@frontagent/core外部使用者是轻微契约扩宽。
- 置信度: medium（无代码图谱输出可复核，结论来自 diff + 仓库文本查证；PR 自述的 GitNexus 数据未独立验证）
问题发现
-
[高]
typescript变成运行时依赖后被整体打进已发布的 CLI 产物- 证据:
packages/hallucination-guard/package.json:24将typescript移到dependencies；packages/hallucination-guard/src/checks/syntax-validity.ts:3是全仓唯一import ts from 'typescript'；build.mjs:15的external仍只有['playwright','ts-morph']，dist/index.mjs是package.json:files中发布的唯一 CLI 产物。ts-morph 此前是 external，所以 TS 编译器此前不在 CLI bundle 里。 - 受影响调用方/流程:
pnpm build:verify→ npm 发布的faCLI；apps/vscode/build.mjs:11同样不 external，vsix 会多带一份 TS。 - 影响: 已发布产物体积量级变化（TS 完整编译器约 8–10MB 未压缩）+ sourcemap，且这正是 #413 写明"是维护者的决定"的那一项，PR 描述里没有任何体积数据或取舍说明。
- 最小可行修复: 二选一并写进 PR 说明——(a)
build.mjs/apps/vscode/build.mjs把typescript加入 external，并在根dependencies声明；(b) 明确接受内联，附上dist/index.mjs变更前后的体积数字。
- 证据:
-
[中]
enabledChecks.syntaxValidity: false不再能端到端关闭语法否决- 证据:
packages/mcp-file/src/tools/create-file.ts:65-68与apply-patch.ts:58-68无条件调用validateFileSyntax；guard 侧的开关（guard.ts:187）只影响 executor preflight。 - 受影响调用方/流程: 任何以
enabledChecks.syntaxValidity=false运行的部署，写盘仍会被文件工具拦截；PR 描述"Preserve guardenabledChecks"与实际端到端行为不符。 - 影响: 解析器一旦对某个合法文件误报，使用者没有任何逃生口（#413 正是因为"检查不可靠时不该有 veto"才被立项）。
- 最小可行修复: 要么把文件工具的校验做成可关闭参数（默认开），要么在 CHANGELOG/README 明确"文件工具层的语法校验不受 guard 配置控制"。
- 证据:
-
[中] JSONC 白名单过窄，
.json写入的新误报会直接否决写盘- 证据:
syntax-validity.ts:212-219仅放行tsconfig*/jsconfig*与.vscode/*.json，其余.json走严格JSON.parse。 - 受影响调用方/流程: agent 写
.eslintrc.json、.devcontainer/devcontainer.json、turbo.json等带注释/尾逗号的配置时，现在是 block 而不是"写下去后报失败"；空内容.json同样被拒。 - 影响: 拦截误报的代价从"误导性失败"升级为"写不进去"，属于 #413 关心的方向。
- 最小可行修复: 把白名单扩到这几类公认 JSONC 文件并补测试，或对非 manifest 类
.json的解析失败降级为 warn。
- 证据:
-
[中] preflight 的管线失败被伪装成
syntax_validityblock，污染pre_write拦截指标- 证据:
executor.ts:613-628把 "patches must be an array" / "original content unavailable" 包成type: 'syntax_validity', severity: 'block'，随后由emitValidationFailed('pre_write', ...)（executor.ts:194）发射；benchmarks/eval/report.mjs:90新增行会把它们计进"写盘前内容拦截"。 - 受影响调用方/流程: #388 建立的"拦截量可计数"语义——
emitValidationFailed上方的注释正是说工具/管线失败不能算拦截。 - 最小可行修复: 给这类失败一个独立
type（如preflight_unavailable），或在这条路径上不发射validation_failed。
- 证据:
-
[中] stale-hash 由缓存 context 单点决定，且没有自愈路径
- 证据:
executor.ts:649-654用collectedContext.files里的内容算哈希；validateBeforeExecution:406只在缺失时才自动读盘；executor-skills.ts:292把 stale 错误标为致命不可跳过。 - 受影响调用方/流程: 若同一 run 中
run_command或外部进程改了该文件，apply_patch 会硬失败，而重试仍复用同一份陈旧缓存 → 重复失败直到 recovery 次数耗尽。 - 影响: 相比旧行为（静默按行号打到错内容上）更安全，但把可恢复情形变成了不可恢复。
- 最小可行修复: 收到 stale 错误时清除
collectedContext.files中该条目（或 apply_patch preflight 前强制重读），让重试有机会成功。
- 证据:
-
[低] 新增包边界上的松类型与重复映射
- 证据:
packages/mcp-file/src/syntax-validation.ts:11-19通过'errors' in result.details+as Array<{...}>把 guard 结果强转，而 guard 已导出SyntaxErrorDetail；同文件 43-49 行又复制了一份扩展名→语言映射，与phase-ordering.ts:144-164并行存在（本 PR 为了.mts/.cts同时改两处，正是漂移成本的实证）。 - 最小可行修复: guard 侧导出返回
SyntaxErrorDetail[]的类型化入口；把detectLanguage提到@frontagent/shared供两侧复用。
- 证据:
行级发现
- [packages/hallucination-guard/package.json:24]
typescript进 dependencies 后会被build.mjs内联进发布的dist/index.mjs（external 只有 playwright/ts-morph）；要么把 typescript 加入 external 并在根 dependencies 声明，要么在 PR 中记录产物体积变化。 - [packages/mcp-file/src/tools/create-file.ts:65] 这里的语法校验对所有调用方无条件生效，
enabledChecks.syntaxValidity: false无法关闭；补一个可选开关或明确文档化该层不可配置。 - [packages/hallucination-guard/src/checks/syntax-validity.ts:179] JSONC 放行仅限 tsconfig/jsconfig/.vscode，
.eslintrc.json、devcontainer.json、turbo.json等带注释配置会被直接否决写盘；扩展白名单并补测试。 - [packages/core/src/executor/executor.ts:624] "original content unavailable" 属于管线失败而非语法拦截，却用
syntax_validity/block上报，会被计进pre_write拦截指标；换独立 type 或此路径不发射事件。 - [packages/core/src/executor/executor.ts:651] 哈希取自缓存 context，命中 stale 后错误被标为致命且缓存不刷新，重试必然复现；失败时清除该文件的 context 条目。
- [packages/mcp-file/src/syntax-validation.ts:13]
result.details.errors as Array<{...}>在新包边界上绕过类型；改为 guard 侧导出返回SyntaxErrorDetail[]的入口。 - [packages/mcp-file/src/syntax-validation.ts:43] 与
phase-ordering.ts:detectLanguage重复的扩展名映射，本 PR 已被迫同步两处；下沉到@frontagent/shared单点维护。
Karpathy 评审
- 假设: 隐含假设"executor 与文件工具永远同进程"——
runtime-node成立，但 stdio server 路径上stripInternalArgs会让 stale-base 保护静默消失，这一点没有在代码或文档中言明。另一隐含假设是collectedContext始终等于磁盘现状（见问题 5）。 - 简洁性: 主体是净简化——
applyPatch从 275 行降到约 110 行，投影逻辑收敛到@frontagent/shared单一实现，executor 与工具共用同一投影；启发式括号/引号计数被解析器取代，删除的是真复杂度而非搬家。保留的checkOuterMarkdownFence有正当理由（```ts…```会被 TS 当作合法 tagged template 解析通过），是必要例外而非残留。 - 结构质量: 新增
syntax-validation.ts属于薄适配层，可接受，但带一处强转和一份重复的扩展名映射（见问题 6）。validateCode现已无生产调用方，仅测试引用——作为公共 API 保留合理，但值得在文档里标注新入口。 - 变更范围: 基本贴合目标。
phase-runner的同目标写串行化与create_file的wx独占写严格说超出 #413 的范围，但它们是"写盘前投影"成立的前提条件，属于必要配套；benchmarks/eval/report.mjs+ 新 workflow 测试是可观测性配套，可接受。 - 验证: 覆盖到位——#413 表格里四个用例（
it's、多行模板、JSX 撇号、正则）都有断言，.ts中 JSX 与.js中类型注解的反向用例也有；file tool 层验证了"拒写不留快照不落盘"；applyFilePatches有独立单测。缺口见下。
缺失覆盖
- 没有端到端用例证明
enabledChecks.syntaxValidity: false时写盘仍被文件工具拦截（当前测试只覆盖 guard facade 本身），而 PR 描述声称保留了该契约。 - 没有对发布产物体积的断言或记录，
typescript内联进dist/index.mjs属于不可见回归。 - stdio MCP server 路径（
stripInternalArgs剥掉哈希）下的 stale-base 行为无测试，保护缺失是静默的。 .vue/.svelte/.html/.css写入仍完全不校验（detectLanguage返回 null），#387 描述的"围栏内容落盘"在这些扩展名上依旧可复现；建议加一条固化该有意缺口的测试或在 CHANGELOG 标注范围。.json空内容与常见 JSONC 配置（.eslintrc.json、devcontainer.json）被拒写的行为没有测试固化，属于问题 3 的验证盲区。
| "@frontagent/shared": "workspace:*", | ||
| "@frontagent/sdd": "workspace:*" | ||
| "@frontagent/sdd": "workspace:*", | ||
| "typescript": "^6.0.3" |
There was a problem hiding this comment.
typescript 进 dependencies 后会被 build.mjs 内联进发布的 dist/index.mjs（external 只有 playwright/ts-morph）；要么把 typescript 加入 external 并在根 dependencies 声明，要么在 PR 中记录产物体积变化。
| return { success: false, error: 'File content must be a string' }; | ||
| } | ||
|
|
||
| const syntaxValidation = validateFileSyntax(content, filePath); |
There was a problem hiding this comment.
这里的语法校验对所有调用方无条件生效，enabledChecks.syntaxValidity: false 无法关闭；补一个可选开关或明确文档化该层不可配置。
| */ | ||
| function checkJsonSyntax(code: string): SyntaxError[] { | ||
| function checkJsonSyntax(code: string, filePath?: string): SyntaxErrorDetail[] { | ||
| if (filePath && isKnownJsoncConfigPath(filePath)) { |
There was a problem hiding this comment.
JSONC 放行仅限 tsconfig/jsconfig/.vscode，.eslintrc.json、devcontainer.json、turbo.json 等带注释配置会被直接否决写盘；扩展白名单并补测试。
| return { | ||
| path, | ||
| content: '', | ||
| validation: this.buildWriteValidationFailure( |
There was a problem hiding this comment.
"original content unavailable" 属于管线失败而非语法拦截，却用 syntax_validity/block 上报，会被计进 pre_write 拦截指标；换独立 type 或此路径不发射事件。
| ), | ||
| toolParams: { | ||
| ...toolParams, | ||
| __frontagentExpectedOriginalHash: createHash('sha256') |
There was a problem hiding this comment.
哈希取自缓存 context，命中 stale 后错误被标为致命且缓存不刷新，重试必然复现；失败时清除该文件的 context 条目。
| const result = validateSourceSyntax({ code: content, language, filePath }); | ||
| const errors = | ||
| result.details && typeof result.details === 'object' && 'errors' in result.details | ||
| ? (result.details.errors as Array<{ |
There was a problem hiding this comment.
result.details.errors as Array<{...}> 在新包边界上绕过类型；改为 guard 侧导出返回 SyntaxErrorDetail[] 的入口。
| return `Syntax validation failed for ${filePath}:${firstError.line}:${firstError.column}: ${firstError.message}`; | ||
| } | ||
|
|
||
| function detectSyntaxLanguage(filePath: string): 'typescript' | 'javascript' | 'json' | undefined { |
There was a problem hiding this comment.
与 phase-ordering.ts:detectLanguage 重复的扩展名映射，本 PR 已被迫同步两处；下沉到 @frontagent/shared 单点维护。
Keep the parser runtime installed beside the published npm CLI instead of embedding the full compiler, while documenting and testing the dependency-free VSIX's intentional inline bundle. Co-Authored-By: Claude <noreply@anthropic.com>
Centralize syntax language detection, expose typed parser diagnostics, honor the mutable guard switch only through trusted in-process metadata, and keep external MCP validation default-on. Co-Authored-By: Claude <noreply@anthropic.com>
Refresh patch context before preparation, reject explicit patches collected against changed content, return machine-readable stale-hash errors, and refresh or invalidate cached content before recovery. Co-Authored-By: Claude <noreply@anthropic.com>
Count failed checks by stage and type so parser interceptions are reported independently from patch projection, stale context, and malformed preflight inputs. Co-Authored-By: Claude <noreply@anthropic.com>
Verify the trusted executor marker observes runtime setCheckEnabled changes rather than only constructor defaults. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
代码评审报告: Fix/parser-backed prewrite validation
风险等级: 中
处理建议: 评论
决策摘要: 核心方向正确且实现扎实（解析器取代引号计数、写盘前投影校验、hash 绑定、内部参数在 stdio 边界被剥离），可以合并；但写盘前 veto 对「原文件本就语法非法」的修复场景一刀切拒绝，JSONC 白名单会误伤目标工程中的合法配置文件，建议合入前或紧随其后补这两处。
级联分析
- 变更符号:
validateSourceSyntax/checkSyntaxValidity（返回类型收窄为SyntaxValidityCheckResult，details由unknown收窄为结构化对象——HallucinationCheckResult.details: unknown，赋值兼容，已确认）、HallucinationGuard.validateSyntax/validateImports/isCheckEnabled（新增）、applyPatch/createFile（新增内部参数与写盘前 veto）、applyFilePatches（shared 新增）、Executor.executeStep、PhaseRunner.selectConflictFreeSteps、detectLanguage（改为detectSyntaxLanguage别名，新增.mts/.cts/.jsonc）。 - 受影响流程: executor 写盘前后校验、apply_patch 上下文刷新、并行 phase 同目标写入串行化、mcp-file stdio server 与两个 in-process adapter（
runtime-node/src/mcp-clients.ts:38-48、apps/cli/src/mcp-client.ts:43-56）、desktopvalidation_failed日志、benchmark report 阶段统计。 - 变更集外调用方:
applyPatch/createFile的三个调用点（server.ts / runtime-node / cli mcp-client）全部只做参数透传，语法 veto 默认开启对它们生效——这是有意的（stripInternalArgs按__frontagent前缀剥离，外部客户端无法关掉，internal-args.ts:10-27已确认，注释也同步更新）。guard.validateCode在生产代码中已无调用方（仅剩自身定义与测试 mock），成为仅供外部使用的公开 API。 - 置信度: medium（我用 Read/Grep 逐一核对了符号、调用方、打包与安全边界，未运行测试；PR 提供的 GitNexus 结论与 diff 一致）。
问题发现
-
[中] 写盘前 veto 只看投影结果，不看原文件是否本来就非法，阻断增量修复路径
- 证据:
apply-patch.ts:71-82与executor.ts:709-713只对projected.content调用语法校验，没有把originalContent的合法性纳入判断。#387 自身证据（benchmarks/results/2026-07-12-sdd-ablation.md：markdown 围栏写进.tsx并落盘）说明仓库里确实存在已落盘的非法文件；用户「帮我修好这个报错文件」也是核心场景。 - 受影响调用方/流程: executor preflight + recovery 循环 + 所有 in-process/stdio 的
apply_patch调用方。文件有多处语法错误、模型分次修复时，每个中间态都会被硬拒（错误信息可读，模型通常能在下一轮一次性补全，因此是「多烧一次 recovery 预算」而不是死锁）。 - 最小可行修复: 先对
originalContent跑一次validateFileSyntax；原文件已非法时不 veto（或仅在错误条数增加时 veto），保持 #387「不引入新的坏状态」的语义。executor preflight 同理（原始内容已在手上）。补一条「patch 一个已非法文件」的回归测试。
- 证据:
-
[中] JSONC 判定是硬编码文件名白名单，落在会阻断写入的路径上
- 证据:
syntax-validity.ts:227-249白名单为*.jsonc/tsconfig|jsconfig*.json/.eslintrc.json/ 5 个.vscode/*/ devcontainer。syntax-validity.test.ts还显式把turbo.json钉成严格 JSON，而 Turborepo 的turbo.json支持注释；同类还有biome.json、.vscode/keybindings.json、nest-cli.json、components.json。 - 受影响调用方/流程: agent 向目标工程写配置文件时被硬失败——正是 #413 要消除的「误判为 block」这一类。名单必然随生态漂移，且分散在 guard 共享路径里。
- 最小可行修复: 反转策略——
JSON.parse失败后用ts.parseJsonText复解，能干净通过时只对少量必须严格的清单（package.json、锁文件、manifest）保留 block，其余降级为 warn；或至少把名单提到可配置的单一常量并补上turbo.json/biome.json。
- 证据:
-
[低] 非预检语言的成功写入不刷新
collectedContext，会触发一次多余的stale_patch_context拦截- 证据:
executor.ts:253-255仅在preflight?.content存在（即 TS/JS/JSON）时回写缓存；.md/.css/.html/.vue走validateWriteBeforeExecution的if (!language) return undefined（executor.ts:644-645）。随后对同一文件的显式 patch 步骤在executor.ts:463-481发现缓存≠磁盘并硬拦截。 - 受影响调用方/流程: 前端任务里常见的「先写 css/md，再 patch 同一文件」。因为第 462 行在返回拦截前已刷新缓存，重试即可通过，代价是一个失败步骤 + 一次 recovery 配额。
- 最小可行修复: 任何
create_file/apply_patch成功后都files.delete(path)（或写回），不要只覆盖预检过的语言。
- 证据:
-
[低]
dryRun下语法非法仍返回success: true- 证据:
apply-patch.ts:74的!dryRun && !validation.syntaxValid，预览态把success=true与validation.syntaxValid=false同时返回，测试固化了该行为。 - 受影响调用方/流程: 只看
success的预览调用方会认为补丁没问题。 - 最小可行修复: 保持不写盘的同时，在 dry-run 结果里补一个明确的
error/errorCode，或在文档里写明 dry-run 的success只表示「投影成功」。
- 证据:
行级发现
- [packages/mcp-file/src/tools/apply-patch.ts:71] 只校验
projected.content；原文件本就语法非法时，任何部分修复补丁都会被拒。建议先校验originalContent，原文件已非法则跳过 veto（或仅在错误增多时拒绝）。 - [packages/core/src/executor/executor.ts:709] preflight 同样只看投影结果，与工具层是同一个盲点；此处已持有
originalContent，判断成本几乎为零。 - [packages/hallucination-guard/src/checks/syntax-validity.ts:227] 白名单缺
turbo.json（Turborepo 允许注释）、biome.json、.vscode/keybindings.json等；建议反转为「JSONC 复解通过即放行，仅少量清单强制严格」，避免名单长期漂移导致误拒写入。 - [packages/core/src/executor/executor.ts:253] 只有预检覆盖的语言才回写文件缓存，导致
.md/.css写入后缓存变陈旧并在后续 patch 步骤触发一次多余拦截；成功写入后统一 delete/refresh 即可。
Karpathy 评审
- 假设: 假设「写盘前内容合法」等价于「写盘后文件合法」——对
create_file成立，对apply_patch依赖 hash 绑定，已用__frontagentExpectedOriginalHash+wx独占创建 + phase 内同目标串行三层兜住，这部分假设是显式化且可验证的。未显式化的假设是「被写文件此前是合法的」（见问题 1）。打包这个 #413 点名的「maintainer call」已在 CHANGELOG 记录（CLI external 7.3MB vs 内联 17.4MB，VSIX 有意内联，apps/vscode/src/package.test.ts:51-57用测试钉住），处理得当。 - 简洁性:
applyFilePatches抽到 shared 让 executor 与工具共用同一投影，是真正删复杂度的重组；apply-patch.ts从 275 行手写括号扫描缩到 57 行是净减。代价是同一份内容在 executor 与 mcp-file 各解析一次（JS 路径还额外跑一次transpileModule全量 emit），属于跨进程边界的合理纵深防御，但值得在 perf 敏感场景留意。detectLanguage保留为别名（phase-ordering.ts:144）是薄包装，但用于兼容既有导入，可接受。 - 结构质量: 分层合理——
shared放纯投影与语言判定，guard 放解析，mcp-file 通过syntax-validation.ts做薄适配（未发现hallucination-guard → mcp-file反向依赖，无环）。唯一的结构隐患是问题 2 的硬编码名单。 - 变更范围: 基本聚焦。
benchmarks/eval/report.mjs+scripts/tests/benchmark-report.test.mjs属于新阶段的遥测配套，pnpm-lock.yaml中 electron-builder 的 peer 展开变化是提升 typescript 到 dependencies 的副产物，均可追溯。 - 验证: 阶段事件（
pre_execution/pre_write/post_write）到 desktop reducer 到 benchmark 报表整条链路都有测试，create_file的并发竞态、快照 discard、stale hash 都有针对性用例，验证强度与「HIGH 风险骨架变更」匹配。
缺失覆盖
apply_patch打到一个原本就语法非法的文件（部分修复 / 完整修复两种），断言是否应被 veto——这是问题 1 的判定用例。- 工具返回
errorCode: 'stale_original_hash'后refreshCollectedFileContext的端到端行为（当前只测了 hash 绑定与工具侧拒绝，没测 executor 侧的上下文刷新与后续重试）。 - 非预检语言（
.md/.css）成功写入后，对同一文件的显式 patch 步骤是否被stale_patch_context误拦（问题 3）。 turbo.json/biome.json这类白名单外 JSONC 的期望行为——当前测试把turbo.json钉为严格 JSON，需要确认这是刻意决策还是疏漏。
| 'original', | ||
| 'modified', | ||
| ); | ||
| const validation = __frontagentSyntaxValidationEnabled |
There was a problem hiding this comment.
只校验 projected.content；原文件本就语法非法时，任何部分修复补丁都会被拒。建议先校验 originalContent，原文件已非法则跳过 veto（或仅在错误增多时拒绝）。
| return { | ||
| path, | ||
| content: projected.content, | ||
| validation: await this.config.hallucinationGuard.validateSyntax( |
There was a problem hiding this comment.
preflight 同样只看投影结果，与工具层是同一个盲点；此处已持有 originalContent，判断成本几乎为零。
| } | ||
| } | ||
|
|
||
| function isKnownJsoncConfigPath(filePath: string): boolean { |
There was a problem hiding this comment.
白名单缺 turbo.json（Turborepo 允许注释）、biome.json、.vscode/keybindings.json 等；建议反转为「JSONC 复解通过即放行，仅少量清单强制严格」，避免名单长期漂移导致误拒写入。
| this.validateAfterExecution(step, toolResult, toolParams, preflight?.content), | ||
| ); | ||
|
|
||
| if (preflight?.content && this.isSuccessfulToolResult(toolResult)) { |
There was a problem hiding this comment.
只有预检覆盖的语言才回写文件缓存，导致 .md/.css 写入后缓存变陈旧并在后续 patch 步骤触发一次多余拦截；成功写入后统一 delete/refresh 即可。
Reject contentless patches, validate JSONC grammar strictly, keep dry runs out of file context, bind non-code patches, reject deferred explicit coordinates, and restore overwrite snapshots on write failures. Co-Authored-By: Claude <noreply@anthropic.com>
Allow incremental repairs when a source file is already invalid, reject invalid dry-run previews, and treat non-manifest JSON configs as JSONC while keeping package, lock, turbo, data, and FrontAgent settings strict. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
代码评审报告: Fix/parser-backed prewrite validation (#413)
风险等级: 中
处理建议: 请求修改
决策摘要: 解析器化 + 写盘前校验的主干设计正确且验证充分，但有两处可确认的误拦截/状态泄漏问题（并发降级标记被写进可持久化的 step.params；JSONC 白名单让合法 JSONC 配置写入被否决），修复量都很小，建议修完再合。
级联分析
- 变更符号:
checkSyntaxValidity/ 新增validateSourceSyntax、HallucinationGuard.validateSyntax|validateImports|isCheckEnabled、applyFilePatches(新, shared)、applyPatch/createFile（新增内部参数与写盘前校验）、Executor.executeStep/validateWriteBeforeExecution/validateAfterExecution、PhaseRunner.selectConflictFreeSteps、AgentEvent.validation_failed.stage。 - 受影响流程: executor 单步执行与并发相位调度、apply_patch/create_file MCP 写入、guard 快速校验、桌面端
validation_failed日志、benchmark 报告生成、CLI/VSIX 打包（typescript 变为运行时依赖）。 - 变更集外调用方:
packages/runtime-node/src/mcp-clients.ts（直接透传 args → 内部参数保留，hash 绑定有效）；packages/mcp-file/src/server.ts经stripInternalArgs剥离__frontagent*（外部 MCP host 默认开启语法校验、无 hash 绑定，属预期降级）；packages/core/src/executor/phase-ordering.ts的detectLanguage已改为detectSyntaxLanguage别名，未产生重复实现；executor-skills.ts:293已显式排除stale_original_hash，避免被旧的Cannot apply patch前缀吞成 skip——这一条是关键且做对了。read_file无截断，executor 侧 sha256 与applyPatch侧 disk 读取一致，hash 绑定成立。 - 置信度: medium（基于仓库只读检查与文本搜索复核；我未自行运行 GitNexus 或测试，PR 提供的图谱与测试结论未独立复现）
问题发现
-
[中] 并发降级标记
__frontagentDeferredSameTargetWrite被写进 step.params 且从不清除- 证据:
phase-runner.ts:233直接对step.params赋值；executor.ts:456-486把该标记当作「内容已变」的充分条件，无视实际内容比对。AgentSessionSnapshot.plan（packages/core/src/types.ts:75-86）序列化整个 plan，含step.params。 - 受影响调用方/流程: 并发相位执行 → 被延后的
apply_patch（显式 patches）在本轮必然以stale_patch_context失败，即使同目标的前一步骤自身失败/被跳过、文件根本没变；标记随快照落盘后，resume场景下该步骤永久失败。同时该 key 会随toolParams一路传给 in-process MCP 适配器（被工具忽略，但把调度状态泄漏进工具参数）。 - 最小可行修复: 把延后信息保存在 PhaseRunner 本地（如
Set<stepId>/ WeakSet），执行后清除；executor 侧只在「刷新后的磁盘内容 ≠ patches 生成时所依据的内容」时拦截，而不是依赖一个粘性布尔值。
- 证据:
-
[中] JSONC 白名单给合法 JSONC 配置带来新的写入否决
- 证据:
syntax-validity.ts:227-249仅放行*.jsonc/tsconfig|jsconfig*.json/.eslintrc.json/ 五个.vscode/*/ devcontainer；其余.json走JSON.parse严格路径且severity: 'block'。syntax-validity.test.ts更明确断言turbo.json必须失败。而turbo.json、biome.json、deno.json、.vscode/keybindings.json等在生态里都合法带注释——本仓库自身就用 turbo/biome。经create_file/apply_patch写入或修改这类已含注释的文件，会被写盘前整体拒绝。 - 受影响调用方/流程: executor 写盘前 veto、
createFile/applyPatch工具层默认开启的校验（外部 MCP host 无法关闭）。方向是 fail-closed 不会损坏文件，但这正是 #413 想消除的「合法代码被判 block」类误报，且现在带写否决权。 - 最小可行修复: 对
.json默认走ts.parseConfigFileTextToJson容忍路径（仅在真正无法解析时拦截），把严格 JSON 限定为确需严格的少数路径（如package.json）；或至少把 turbo/biome/deno/.vscode/*纳入白名单并在注释里写明白名单的维护口径。
- 证据:
-
[低]
dryRun预览被写盘前 veto 吞掉- 证据:
apply-patch.ts:74刻意只在!dryRun时因语法失败返回失败（dry-run 仍返回 diff + validation 供预览）；但executor.ts:645起的validateWriteBeforeExecution不区分dryRun，语法不合格的 dry-run 在callTool之前就被判 step 失败，工具的预览语义在 executor 路径下不可达。 - 受影响调用方/流程: 任何用
dryRun: true做补丁预览的步骤。 - 最小可行修复: preflight 中对
toolParams.dryRun === true只记录不阻断，或显式说明 executor 不支持失败预览。
- 证据:
-
[低] mcp-file 为一个纯函数引入整包 guard 依赖
- 证据:
packages/mcp-file/src/syntax-validation.ts:1从@frontagent/hallucination-guard包根导入validateSourceSyntax，而包根 (index.ts) 会连带guard.js→@frontagent/sdd与全部 checks 进入文件工具/独立 server 的依赖图。 - 最小可行修复: 为 guard 增加
./syntax子路径导出，或把纯解析器下沉到@frontagent/shared，让 mcp-file 只依赖它需要的部分。
- 证据:
行级发现
- [packages/core/src/executor/executor.ts:456]
wasDeferredSameTargetWrite把 PhaseRunner 写入step.params的粘性标记当成「内容已变」的充分条件；标记不会被清除且会随会话快照持久化，导致前置写入未真正改动文件、或 resume 之后，该 apply_patch 步骤仍被永久判为 stale。建议改为 runner 本地状态并按真实内容比对拦截。 - [packages/hallucination-guard/src/checks/syntax-validity.ts:227] JSONC 白名单漏掉了生态中常见的注释型配置（turbo.json、biome.json、deno.json、
.vscode/keybindings.json），这些文件将以severity: 'block'被写盘前否决；建议.json默认走容忍解析、仅对少数路径保持严格 JSON。 - [packages/core/src/executor/executor.ts:645] preflight 未对
dryRun做例外，使apply-patch.ts:74刻意保留的 dry-run 预览语义在 executor 路径下失效；建议 dry-run 只记录校验结果而不阻断调用。
Karpathy 评审
- 假设: 关键假设已被显式绑定——
read_file返回未截断的原文，因此 executor 的 sha256 与工具侧磁盘读取一致（已核对read-file.ts，无 startLine/endLine 时为原文）；内部参数只在 in-process 适配器保留（internal-args.ts+runtime-node/mcp-clients.ts）。未言明的假设是「同目标写入必须串行且延后即视为失效」，这一条被编码成粘性 params 标记而非显式调度状态（见问题 1）。 - 简洁性: 主体是减复杂度的：手写括号/引号扫描被解析器替换，patch 投影收敛为
@frontagent/shared/file-patches.ts单一实现，detectLanguage变为共享实现别名，apply-patch.ts从 275 行删到 57 行净减。反向的复杂度只有 JSONC 白名单这一处 ad-hoc 特例表。 - 结构质量: 分层合理——纯投影在 shared，解析在 guard，写盘前后分别由
validateSyntax/validateImports承担，enabledChecks被贯通。扣分项：包根导入把 guard 全量拖进 mcp-file（问题 4）；调度状态写进step.params破坏了 runner 与 step 数据的边界（问题 1）。 - 变更范围: 与 #413/#387 目标一致，
report.mjs与 benchmark 文案的改动可追溯到新事件分层；无明显无关重构。 - 验证: 覆盖度好——撇号/多行模板/JSX/正则、TSX vs TS vs JSX 判别、fence、hash 绑定与 stale 拒绝、快照不被创建、
wx独占创建竞态、dry-run 不污染上下文、并发同目标串行、桌面端三阶段标签、report 分类计数均有针对性用例。
缺失覆盖
- 缺少「被延后的 apply_patch 在前置同目标步骤失败/文件未变」以及「携带
__frontagentDeferredSameTargetWrite的 plan 经 resume 再执行」的用例——这两条正是问题 1 的失败路径。 - 缺少非白名单 JSONC（
turbo.json/biome.json带注释）经apply_patch修改的用例；当前测试反向锁定了turbo.json必须失败，会把误拦截固化成契约。 apps/vscode/src/package.test.ts:52-53在dist/extension.cjs不存在时直接return，而quality:ci的顺序是test先于build:verify；本 PR 让 typescript 成为 guard 的运行时依赖并依赖「VSIX 内联打包」这一不变量，该断言在标准门禁下实际为空跑。建议改为断言 esbuild 配置的 externals，或在该测试前保证已构建。- 缺少 dry-run 预览路径的 executor 级用例（问题 3）。
| ); | ||
| { | ||
| const cachedContent = context.collectedContext.files.get(path); | ||
| const wasDeferredSameTargetWrite = step.params.__frontagentDeferredSameTargetWrite === true; |
There was a problem hiding this comment.
wasDeferredSameTargetWrite 把 PhaseRunner 写入 step.params 的粘性标记当成「内容已变」的充分条件；标记不会被清除且会随会话快照持久化，导致前置写入未真正改动文件、或 resume 之后，该 apply_patch 步骤仍被永久判为 stale。建议改为 runner 本地状态并按真实内容比对拦截。
| } | ||
| } | ||
|
|
||
| function isKnownJsoncConfigPath(filePath: string): boolean { |
There was a problem hiding this comment.
JSONC 白名单漏掉了生态中常见的注释型配置（turbo.json、biome.json、deno.json、.vscode/keybindings.json），这些文件将以 severity: 'block' 被写盘前否决；建议 .json 默认走容忍解析、仅对少数路径保持严格 JSON。
| } | ||
| | undefined | ||
| > { | ||
| if (step.action !== 'create_file' && step.action !== 'apply_patch') return undefined; |
There was a problem hiding this comment.
preflight 未对 dryRun 做例外，使 apply-patch.ts:74 刻意保留的 dry-run 预览语义在 executor 路径下失效；建议 dry-run 只记录校验结果而不阻断调用。
There was a problem hiding this comment.
代码评审报告: Fix/parser-backed prewrite validation (#413)
风险等级: 高
处理建议: 请求修改
决策摘要: 方向正确、测试扎实，但新的 JSON 分类逻辑把「除白名单外的所有 .json」交给 tsconfig 解析器，会把顶层数组/标量的合法 JSON 判为语法错误——而这个判定现在拥有写盘否决权，属于 #413 要消灭的同一类误拦；另有一处永不清除的调度标志会污染持久化的 step.params。
级联分析
- 变更符号:
checkSyntaxValidity/ 新增validateSourceSyntax、HallucinationGuard.validateSyntax/validateImports/isCheckEnabled、Executor.executeStep（新增validateWriteBeforeExecution）、Executor.validatePreExecution/validateAfterExecution、PhaseRunner.selectConflictFreeSteps、applyPatch、createFile、新增applyFilePatches/detectSyntaxLanguage/SnapshotManager.discardSnapshot。 - 受影响流程: agent
execute/planOnly的每一次create_file、apply_patch写盘；apply_patch 现在每步额外做一次read_file；guardvalidateCode在执行器路径上已被拆分替代；desktop 执行日志 reducer；benchmark 事件统计（validation_failed:pre_write:<check>键，报告与 run-eval 已对齐，scripts/tests/benchmark-report.test.mjs覆盖）。 - 变更集外调用方: 未发现未同步的调用方。
detectLanguage已改为复用detectSyntaxLanguage（phase-ordering.ts:144），无重复实现；validateCode保留为公共 API，执行器不再调用；packages/mcp-file→@frontagent/hallucination-guard新依赖边不构成循环；typescript的运行时依赖已在根package.json与build.mjsexternal 列表、VSIX 打包注释中处理一致。 - 置信度: medium（在 checkout 中用 Read/Grep 逐一核对了被改符号的调用方与打包配置，关联 issue 上下文完整；但 PR diff 被截断 24 个文件，且本环境未安装依赖，无法执行测试或实际跑 TypeScript 解析器验证 JSON 判定）。
问题发现
-
[高]
.json默认走 JSONC 解析器，顶层数组/标量的合法 JSON 会被写盘前否决- 证据:
syntax-validity.ts:253的兜底return baseName.endsWith('.json')让所有不在isStrictJsonPath硬编码清单里的.json进入ts.parseConfigFileTextToJson（:195）。该 API 对非对象根会写入The root value of a 'tsconfig.json' file must be an object诊断并返回error，于是[{"id":1}]、"str"、123这类合法 JSON 被判为语法错误。链路上这已是硬否决：create-file.ts:67-72直接拒绝写盘，executor.ts:199-213在callTool前终止 step。证据等级 (inferred)：依据 TypeScript API 契约推断，本环境未安装typescript，未实际执行验证。 - 受影响调用方/流程: 任何 agent 写 mock 数据 / 列表型 JSON（
items.json、mock.json、i18n 数组文件等）的create_file、apply_patch；外部 MCP 客户端因内部参数被stripInternalArgs剥离，无法绕过。这正是 #413 所警告的「不可靠的检查不应拥有写否决权」。 - 最小可行修复: 在
checkJsonSyntax里先尝试JSON.parse，成功即通过；失败后再按isKnownJsoncConfigPath决定是否用 JSONC 宽松解析。这既保留本分支刻意放宽的 JSONC 支持，又完全消除数组/标量根误拦，同时让package.json等严格路径行为不变。并补一条数组根 JSON 的回归用例。 - 附带（同一根因，无需单列）: 诊断文案对
biome.json等文件也会写成 "tsconfig.json"；反方向的代价是en.json、manifest.json这类严格消费的 JSON 现在允许注释与尾逗号并静默通过，运行时才JSON.parse失败——修复方案同上即可收敛。
- 证据:
-
[中]
__frontagentDeferredSameTargetWrite写入step.params后永不清除，会随会话快照持久化- 证据:
phase-runner.ts:233把调度状态写进step.params；executor.ts:456读取它并在存在显式patches时无条件拦截。没有任何位置删除该键。agent.ts:649-654在 resume 时保留resume.plan.steps的全部 params，仅把非 completed 步骤重置为 pending。 - 受影响调用方/流程: 被同目标去重推迟并失败的 apply_patch 步骤，在 resume 后即使文件与上下文完全一致也会再次被判
stale_patch_context，即永久失败；该键还会随{...step.params}进入callTool参数（进程内适配器不剥离内部键），把调度状态泄漏到工具边界。 - 最小可行修复: 读取即消费——在
executor.ts:456取值后delete step.params.__frontagentDeferredSameTargetWrite；或把推迟标记放进 PhaseRunner 内的运行期WeakSet<ExecutionStep>，不落到step.params。
- 证据:
-
[中] 写盘前校验块把
executor.ts从约 788 行推过 1000 行（现 1017 行）- 证据:
executor.ts:632-760新增validateWriteBeforeExecution/buildWriteValidationFailure/isSuccessfulToolResult，加上:296-313的refreshCollectedFileContext与crypto哈希绑定，共约 150 行独立职责代码；本 PR 净增 +249 行。 - 受影响调用方/流程: 该目录已有
phase-runner.ts、phase-ordering.ts、tool-call-handler.ts的拆分惯例，Executor又是 18 个直接上游依赖的关键骨架，文件继续膨胀会直接抬高后续改动的评审与验证成本。 - 最小可行修复: 把这组函数整体移到
executor/write-preflight.ts（纯函数 + 注入 guard），Executor只保留一次调用与结果分支，符合同目录既有模式，不改变任何行为。
- 证据:
-
[低]
overwrite: true但目标当时不存在时使用wx，竞态下返回失败而非按调用方意图覆盖- 证据:
create-file.ts:98-101仅在overwrite && existedBeforeWrite时走普通写；overwrite: true且检查时不存在则用flag: 'wx'，EEXIST 会被:112-126归为 "File appeared concurrently" 失败。悬空符号链接同样触发（existsSync为 false 但wx报 EEXIST）。 - 最小可行修复:
lostExclusiveRace && overwrite === true时改为普通写重试一次，仅对overwrite: false保留硬失败。
- 证据:
行级发现
- [packages/hallucination-guard/src/checks/syntax-validity.ts:253] 兜底把所有
.json归为 JSONC，使顶层数组/标量的合法 JSON 经parseConfigFileTextToJson被判语法错误并否决写盘；建议先JSON.parse成功即放行，失败再按路径决定是否宽松解析。 - [packages/core/src/executor/executor.ts:456] 该推迟标志由 PhaseRunner 写入
step.params且从不清除，会随会话快照持久化，使 resume 后的显式 patch 步骤永久被判 stale；读取后立即delete，或改为 PhaseRunner 内的运行期 WeakSet。 - [packages/core/src/executor/executor.ts:632] 写盘前校验整块逻辑使本文件越过 1000 行；按同目录
phase-runner.ts/tool-call-handler.ts的既有模式抽到executor/write-preflight.ts，行为不变。 - [packages/mcp-file/src/tools/create-file.ts:101]
overwrite: true且检查时文件不存在也走wx，竞态或悬空符号链接会返回 "appeared concurrently" 而非按调用方意图覆盖；EEXIST 且overwrite === true时应普通写重试。
Karpathy 评审
- 假设: 哈希绑定隐含「
read_file返回逐字节原文」这一未文档化契约（read-file.ts:88-124当前成立）；若日后read_file加行号或截断，所有 patch 都会退化为stale_original_hash，值得在applyPatch的哈希比较处留一行契约注释。另一处隐含假设是「.json多半是配置文件」，即上面的发现 1。 - 简洁性: 共享投影器
applyFilePatches是本 PR 最好的部分——删除了 apply-patch 里 275 行重复的边界/重叠/括号启发式，且用detectSyntaxLanguage统一了语言判定。executor 与工具各自再解析一次原文与投影（每次 patch 共 4 次解析）属于对外部 MCP 调用方的合理纵深防御，不算冗余。file-patches.ts:26/31的content !== undefined分支在 bounds 校验之后已不可达，属可清理的小冗余。 - 结构质量: 主要退化是发现 3（文件越界）与发现 2（调度状态放进数据模型
step.params）。isStrictJsonPath的硬编码 basename 清单是共享路径上的 ad-hoc 特例，会持续漂移，发现 1 的修复方案可同时缓解它。 - 变更范围: 基本聚焦。benchmark 报告与
report.mjs重写由新的分阶段事件键驱动，可接受；pnpm-lock.yaml中 electron-builder 的 peer 解析串重排与本变更无关，属重解析噪音。 - 验证: 焦点测试质量高——写盘前拦截不调用
callTool、快照不创建、stale hash 拒绝、dry-run 不污染上下文、同目标写序列化、enabledChecks贯通、TSX/JSX 与.ts的区分都有断言，方向与 #387/#413 的验收标准一致。缺口集中在下列场景。
缺失覆盖
- 顶层为数组/字符串/数字的合法
.json内容（create_file与 executor preflight 两侧），当前 JSONC 用例全部是对象根。 - 空
.json内容：JSONC 路径判通过、JSON.parse判失败，两条路径行为不一致。 - 会话 resume 场景：携带
__frontagentDeferredSameTargetWrite的 step 重跑仍被拦截。 - 非法
operation（如 LLM 生成"update"）：applyFilePatches的 switch 静默落空，投影等于原文，最终以 success 返回但文件未变——「校验过的投影」在此处会给出假成功。 - 严格 JSON 消费方（i18n/locale、
manifest.json等）写入带尾逗号内容时被放行的回归用例，用于固定放宽策略的边界。
| return true; | ||
| } | ||
|
|
||
| return baseName.endsWith('.json'); |
There was a problem hiding this comment.
兜底把所有 .json 归为 JSONC，使顶层数组/标量的合法 JSON 经 parseConfigFileTextToJson 被判语法错误并否决写盘；建议先 JSON.parse 成功即放行，失败再按路径决定是否宽松解析。
| ); | ||
| { | ||
| const cachedContent = context.collectedContext.files.get(path); | ||
| const wasDeferredSameTargetWrite = step.params.__frontagentDeferredSameTargetWrite === true; |
There was a problem hiding this comment.
该推迟标志由 PhaseRunner 写入 step.params 且从不清除，会随会话快照持久化，使 resume 后的显式 patch 步骤永久被判 stale；读取后立即 delete，或改为 PhaseRunner 内的运行期 WeakSet。
| }; | ||
| } | ||
|
|
||
| private async validateWriteBeforeExecution( |
There was a problem hiding this comment.
写盘前校验整块逻辑使本文件越过 1000 行；按同目录 phase-runner.ts / tool-call-handler.ts 的既有模式抽到 executor/write-preflight.ts，行为不变。
| if (overwrite && existedBeforeWrite) { | ||
| writeFileSync(safePath.fullPath, content, 'utf-8'); | ||
| } else { | ||
| writeFileSync(safePath.fullPath, content, { encoding: 'utf-8', flag: 'wx' }); |
There was a problem hiding this comment.
overwrite: true 且检查时文件不存在也走 wx，竞态或悬空符号链接会返回 "appeared concurrently" 而非按调用方意图覆盖；EEXIST 且 overwrite === true 时应普通写重试。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked Issue Or Context
Summary
apply_patchvalidate the same complete projected content.create_fileand arbitraryapply_patchresults before disk mutation, bind patch execution to the validated original-content hash, and serialize same-target writes so the validated projection cannot race a concurrent write.enabledChecks, validation-failure events, phase accounting, and desktop reducer behavior with regression coverage.Impact Scope
@frontagent/hallucination-guard: parser-backed synchronous syntax validation and syntax/import facades.@frontagent/shared: reusable deterministic patch projection.@frontagent/mcp-file: create/patch pre-write validation and stale-original hash rejection.@frontagent/core: executor preflight, same-target write ordering, internal argument propagation, and validation-failure events.GitNexus Impact Summary
Executor.executeStep,PhaseRunner, and thecreate_file/apply_patchMCP boundary.detect_changes({ scope: "compare", base_ref: "develop" })reported 43 changed symbols across 25 files, 15 affected execution flows, and HIGH aggregate risk.Executorhas 18 direct upstream dependents and affects agentexecute,planOnly,executeSteps, and constructor flows.HallucinationGuardis MEDIUM (6 direct dependents, 2 agent flows);PhaseRunner,applyPatch,createFile,checkSyntaxValidity,validateSourceSyntax, andapplyFilePatcheswere LOW individually.context(Executor.executeStep)confirmed pre-write validation sits beforecallTooland preserves trace/failure paths. Focused executor, phase, guard, file-tool, shared, and desktop tests passed, followed by the full local gate.Verification
pnpm agent:bootstrap— passed.pnpm quality:predev— passed.pnpm quality:precommit— passed (lint, full typecheck, full tests, 38 workflow tests).pnpm quality:local— passed (Contract Guard, all precommit checks, and 14/14 package builds).Checklist
pnpm quality:precommit, or explained why it could not run.pnpm quality:localfor critical skeleton changes, or explained why it could not run.