Skip to content

fix(webui,local-runtime): 撤销/重做改成全有或全无 + 工作区包含性检查(批次二 BUG-3) - #150

Open
stevenjj33 wants to merge 3 commits into
webuifrom
fix/webui-diff-mutation-precheck
Open

stevenjj33 wants to merge 3 commits into
webuifrom
fix/webui-diff-mutation-precheck

Conversation

@stevenjj33

Copy link
Copy Markdown
Collaborator

基线

webui @ 6a455a2(动手前 git ls-remote 自查确认),独立分支、独立 PR。第二个提交是 cherry-pick 的 composer 草稿持久化修复(与批次一 PR #149 同一提交)——原因见文末「关于 CI」。

BUG-3 · D-1:撤销/重做两处真实缺陷

roadmap E 区记 🟡 的原因不是提示缺失(#20 已做失败上报),而是功能本身不成立。两处缺陷都在 applyLocalTurnDiffSnapshotMutation(packages/local-runtime/src/turns/file-changes.ts),按 DiffCard 顶部注释的线索逐条对源码核实:

3a 越界路径被放行。 normalizeCapturePath 对解析到工作区之外的路径返回绝对路径本身(这个返回值是 capture 层记录「工具写了工作区外」的方式),而 mutation 经 safeCapturedPath 照单全收——于是一轮 revert/reapply 会 fs.rm/writeFile 工作区外的文件。返回 undefined 的反而是工作区内被过滤的路径(.git/、.harness-def/——注意:任务书提到的 node_modules 不在本基线的过滤集里,实测确认,未擅自扩大 capture 过滤范围)。

3b 检查坐在写循环体内。 安全/完整性检查在循环里逐条做,前面的条目已写完或 fs.rm 掉,后面的才被判不安全——留下半应用状态,而用户只被告知「操作已中断」。

修法

mutation 改为先全量预检、再落盘:

  1. 预检每条 entry:resolveMutationTargetPath 解析目标路径,不在工作区内(含被过滤路径)→ unsafe_path;快照不可还原(exists 但无 content)→ not_undoable;
  2. 只读的 conflict 预检(原有逻辑,前移保证也在写之前);
  3. 才进写循环,用预检解析好的绝对路径。

预检拒绝时磁盘零改动。webui 侧 unsafe_path 人话文案同步改为诚实的新事实(「操作未执行,磁盘文件均未改动」)——旧文案描述的是已不存在的半应用行为;da70088/3222a14 建的人话表本身保留,conflict 等其余条目不动。

已知边界(写进注释):包含性是词法的;工作区内指向外部的 symlink 仍会逃逸,堵它要对每个目标做 realpath 并处理尚不存在的文件,大于本次范围。

证伪(任务书第 4 节)

把函数改回旧形态(检查坐回写循环)后跑新测试:3 failed / 5 passed。三条红的正是缺陷本身——越界夹具旧代码返回 { success: true }(它真的写了工作区外的文件),不可还原第二条目时第一个文件已被改写。五个「行为保持」用例(过滤路径仍拒、conflict 仍拒、happy path、嵌套目录、空 undo)新旧都绿——这正是它们作为「保持性」断言诚实的证明。恢复后 8/8。

测试

新增 packages/local-runtime/test/unit/diff-mutation-precheck.test.ts 8 条(该函数此前没有任何直接测试覆盖,diff-capability 的 rewind 路径同样没有,本文件是首个):

  • 验收 ①:相对/绝对两种越界写法均拒;
  • 验收 ②:「第二个文件不安全」夹具跑完断言第一个文件仍是原内容(以及工作区外文件未被碰);
  • 3b:第二条目快照不可还原 → not_undoable 且第一个文件未动;
  • 保持性:.git/、.harness-def/ 仍拒;conflict 仍拒且不写;revert/reapply 全安全路径含删除文件重建;嵌套目录;
  • 验收 ④:diff-mutation-error.test.tsx 现有断言(失败后卡片仍可操作、不谎称能力缺失)未动且全绿。

边界遵守

  • 没动 roadmap 满足度/统计表;没碰设置页;没认领新区;没动 desktop/browser-core/agent-tools provider。
  • 没有为 CI 改断言/删用例/skip——所有断言变红都是因为产品代码被改回缺陷形态。
  • capture 层过滤范围未动(node_modules 的出入已在上面说明)。

Validation(本地全过)

  • pnpm build:webui
  • 全量单测:370 files / 8093 passed(本分支 8085 基线 + 8 新)
  • pnpm typecheck(standalone)+ pnpm typecheck:webui-full:0 错误
  • pnpm verify:22 gates PASS on darwin
  • pnpm test:webui-browser:97 passed
  • 清单:source-inventory.mjs --write 幂等;新测试已进两份清单

关于 CI(重要,非代码问题)

本仓库 Actions 已被仓库级关闭:gh workflow run 返回 HTTP 422: Actions has been disabled for this repository;全仓最近一次成功运行是 10-07(voice-shortcuts 分支),其后该分支 10-09 的新提交也无任何 run;本 PR 与 #149 均收不到 check。需要仓库管理员恢复 Actions 后补跑。在上述证据成立前,请以本 PR 的本地全量证据为准。

e40a63f moved composer-store ownership into the application and made the
shell adopt the composition root's application (A4-s2). The root built
its application without a `composer` dep, so `create-application` fell
back to `createWebuiComposerStore()` — storage-less by design, because
the application layer may not import infrastructure. The shell's own
storage-backed store (`providedApplication?.composer ?? create(...)`)
became dead code on the live path, and with it every draft: typing
still worked and switching sessions still restored the in-memory draft,
but nothing ever reached localStorage, so a reload lost them all.

Verified on webui @ 6a455a2 before this change: the drafts spec failed
deterministically (3 runs, same failure — after reload the composer is
empty and `webui.composer.state.v1` is null even before the reload), and
a probe showed the key was never written. With the injection: 7/7 in
composer.spec.mjs, 97/97 in the full browser suite.

The composition root now injects the same storage adapter the shell
would have built, exactly as the shell already does for `unreadStorage`
when it builds its own application. The unread controller has the same
gap at the same call site (no failing gate covers it); left for a
follow-up rather than widening this fix.
…-nothing and workspace-contained

Two real defects in applyLocalTurnDiffSnapshotMutation, both found through
the DiffCard header comment and confirmed against the runtime:

1. No containment check. normalizeCapturePath hands back the ABSOLUTE path
   for anything resolving outside the workspace — that return is how the
   capture layer records out-of-workspace writes — and the mutation
   accepted it via safeCapturedPath, so a revert or reapply would write or
   fs.rm a file outside the workspace. A revert of a turn whose tool wrote
   ../outside.ts acted on it.

2. The safety and completeness checks sat inside the write loop, so a
   refused entry left the run half-applied: earlier entries were already
   written or removed when a later one was judged unsafe or
   un-snapshottable, and the user was told the run was "interrupted"
   without being told half of it had already happened.

The mutation now resolves and validates every target in a full precheck
before the first write (containment, snapshot completeness), then the
read-only conflict precheck, then the writes. A refusal touches nothing
on disk. The webui copy for unsafe_path is updated to say that plainly —
the old text described the half-applied behaviour that no longer exists.

Falsified: with the old loop-body checks restored, the new runtime tests
go 3 failed / 5 passed — the out-of-workspace fixtures return
{ success: true } (the old code really did write outside the workspace)
and the un-snapshottable second entry leaves the first file already
rewritten. The five behaviour-preserving tests stay green on both, which
is what makes them honest "kept" claims. The DiffCard retry path
(diff-mutation-error.test.tsx) is untouched and still green.
@stevenjj33 stevenjj33 closed this Oct 14, 2026
@stevenjj33 stevenjj33 reopened this Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant