Fix/parser backed prewrite validation - #458
Open
NanluQingshi wants to merge 7 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 单点维护。
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.