[linletian]

Closes #102

Problem

Deleting a worktree was refused with the flat worktree has uncommitted or untracked changes; delete is refused — zero detail, no force exit. In the reported instance 133 tracked source files had vanished BEFORE the delete attempt, and the refusal presented a damaged workspace as an everyday "you have changes" prompt. The only way out was a manual git worktree remove --force the daemon never noticed (its state record survived). (The file disappearance itself was investigated in the issue and is NOT a myworktree bug.)

Fix (the issue's P1–P3)

• P1 — detailed refusal. Manager.Delete(id, force) returns a structured DirtyWorktreeError: counts by category (one bucket per porcelain line, delete > rename > add > modify, buckets always sum to the total), up to 5 first paths, and the FULL git status --porcelain output. Message now reads e.g. … refused: 133 entries (133 deleted, 0 modified, 0 untracked); first paths: ….
• P2 — force exit everywhere the delete exists. POST /api/worktrees/delete takes force, the MCP worktree_delete verb takes force, CLI grew myworktree worktree delete --force <id>; force passes --force to git worktree remove.
• P3 — gitignored-file warning. The refusal separately counts git status --porcelain --ignored (!! lines the blocking check never sees — 89 ignored files, 64 single-copy, would have silently vanished) and warns that force destroys them unrecoverably.

API: refusal answers 409 with contract code worktree_dirty + message (one-line summary) + structured dirty breakdown (documented in docs/API.md).

UI: deleteWorktree matches the code and opens a dialog: summary, collapsible full git status output, gitignored-file danger warning, and a two-step-confirm Force delete button that resends with force: true.

P4 (state.json's recorded branch can lag a git switch) needed no change: Manager.List already re-resolves the live branch per worktree for display.

Tests (full suite green)

• internal/worktree: TestDeleteDirtyRefusalCarriesDetails / TestDeleteForceRemovesDirtyWorktree (real-git integration), TestDirtyWorktreeErrorSummarizePorcelain / TestCountIgnoredPorcelain / TestDirtyWorktreeErrorMessage (unit).
• internal/app: TestHandleWorktreeDeleteDirtyRefusalIsStructured / TestHandleWorktreeDeleteForceDeletes (API contract, real git).
• internal/ui: TestWorktreeDirtyDeleteDialog (asset anchors).
• CLI --force parsing smoke-tested with a built binary.

CHANGELOG entry under Unreleased.

[linletian]

独立评审（qwen3.8-flash-next）

总体：分层清晰（错误结构 → 409 契约 → UI 对话框），桶互斥不变式（delete>rename>add>modify 恒求和）设计正确，文档与测试扎实。本地复核：go test ./... 全绿，gofmt -l、go vet 干净。以下为发现的问题（P1 建议合并前处理，其余可跟进）。

P1 — CLI 后置 --force 被静默忽略（cli.go:212-219）

Go flag 包在第一个位置参数后停止解析：myworktree worktree delete <id> --force → force=false、NArg=2，用户得到完全相同的拒绝。而拒绝消息恰好写着 "retry with force to delete anyway"——用户自然会把 --force 追加在 id 后面重试，然后继续被拒且毫无线索。已实验复现。建议：fs.NArg() > 1 时报错，或解析前把 --force/-force 提取到位置参数之前。

P2 — MCP worktree_delete 的脏拒绝形态与 HTTP API 不一致（app.go:3203）

/api/worktrees/delete 返回 409 + 结构化 dirty，MCP 路径仍是 writeErr(400) 平铺文本：状态码不一致，MCP 客户端无法程序化识别 worktree_dirty、拿不到 breakdown。代码注释声称 "same semantics as /api/worktrees/delete"，实际只有 message 文本相同。建议透传 409 + dirty 结构（至少统一状态码），并同步 docs/API.md 的 MCP 小节。

P2 — force=true 仍前置运行 git status，且其失败会中止强删（worktree.go:462-465）

force 时该 status 输出只用于拒绝结构，但 git status 失败会直接 return "git status failed"——给"用户已明确接受一切销毁"的 force 路径新增了一个失败模式（还是一次多余调用）。建议：force 时跳过该检查，或容忍其失败继续删除。

P3 — 折叠目录使两类计数严重低估（worktree.go:462/476）

实测：git status --porcelain 把未跟踪目录折叠为单条 ?? node_modules/，--ignored 同样折叠 !! dir/。UI 警示文案恰好是 "build caches, logs, backups"——正中最严重的折叠场景，1 条 entry 可能是上千文件。建议两次 status 均加 -uall，或在消息/文案中说明目录按一条计。

P3 — 非 ASCII 路径显示为八进制转义

实测默认 core.quotePath=true 下，FirstPaths/Porcelain 会带 "\344\270\255\346\226\207.txt" 这类转义进 UI。建议 status 调用加 -c core.quotePath=false。

Nits

• UI forceDeleteDirtyWorktree 失败时展示 e.message——newAPIError 将其设为 body.error，即原始码 "worktree_dirty"；建议 (e.body && e.body.message) || e.message。
• 对话框按 Esc 关闭不会走 closeWorktreeDirtyDialog()，dirtyDeleteID 悬挂；建议给 #modal-wt-dirty 加 cancel 事件同样调用。
• Error() 里 map[bool]string{...}[...] 每次调用构造一个 map 只为单复数；小 nit。

不阻塞项均已核对无误：porcelain v1 解析（status.outputFormat=v2 不影响裸 --porcelain，实测确认）、409 契约与 UI e.body.error 匹配链、两阶段确认、拒绝后 state/路径原样保留、全部 Delete 调用点已更新。

[linletian]

Independent review — PR #105 (issue #102)

Verified locally: gofmt -l . clean, go build ./..., go vet ./..., and the full go test ./... all green. The structured refusal, the API/UI/MCP plumbing, and the message unit tests look solid. Three issues below — F1 undermines the force exit for exactly the damaged-worktree case this PR targets; F2 leaves a silent path open.

F1 — force is gated on a successful git status, so it can't rescue a worktree whose status probe fails. internal/worktree/worktree.go:462 runs git status --porcelain unconditionally, even when force=true, and returns git status failed (line 465) before ever reaching git worktree remove --force (line 483). Verified: corrupt a worktree's index and git status exits 128 while git worktree remove --force still succeeds (exit 0). Force is meant to be the escape hatch for a damaged workspace, so the status probe should be skipped (or its error ignored) when force is set.

F2 — a clean worktree containing only gitignored files is still deleted silently, with no P3 warning. The ignored-file warning lives entirely inside the dirty-refusal branch (porcelain != "" && !force, lines 468–481). A worktree with no tracked/untracked changes but only ignored files looks "clean" to the blocking check, so it is removed without force, and git worktree remove deletes the whole directory — the ignored files vanish with no warning. That is the exact class P3 claims to guard ("would have silently vanished"). Verified: a worktree holding only local.log and build/ (both ignored) is removed with no refusal and no warning. Suggest surfacing the ignored risk (or requiring confirmation) whenever Ignored > 0, including the clean path.

F3 — myworktree worktree delete <id> --force silently drops --force. internal/cli/cli.go:215 uses Go's flag.FlagSet, which stops parsing at the first positional arg: fs.Parse(["wt1","--force"]) leaves force=false and ignores --force (verified empirically). The documented usage is [--force] <id>, but the common post-positional form is a silent no-op — the user then hits the refusal with no hint the flag was discarded. Consider a manual scan for --force or explicit handling.

Minor: the MCP worktree_delete dirty refusal (internal/app/app.go:3203) still returns a generic 400 {"error":"<message>"} rather than the structured worktree_dirty 409, so MCP clients don't receive the breakdown the HTTP contract documents. Intended or not, worth confirming.

No changes made — review only.

[linletian]

回复 @独立评审（qwen3.8-flash-next） — 所有发现均已逐条核实（关键项做了实证复现），全部属实，修复见 da2171a：

• P1 CLI 后置 --force 被静默忽略 — 属实（Go flag 在首个位置参数后停止解析）。已改为手动扫描 --force/-force（位置任意），多余位置参数报 usage 错误。钉住：TestRunWorktreeDeleteTrailingForceFlag / TestRunWorktreeDeleteExtraPositionalFails。
• P2 MCP 脏拒绝形态不一致 — 属实，注释声称 same semantics 名不副实。MCP 路径现复用 writeWorktreeDirtyErr，返回与 HTTP API 相同的 409 + worktree_dirty + 结构化 dirty；成功响应同样带 ignored_destroyed。docs/API.md 已同步。钉住：TestHandleMCPWorktreeDeleteDirtyRefusalIsStructured。
• P2 force 仍前置 git status — 属实且已实证：破坏 worktree 索引后 git status 退出 128，而 git worktree remove --force 成功。force 路径现在完全跳过探测（探测输出只用于构造拒绝信息）。钉住：TestDeleteForceSkipsStatusProbe（真实损坏索引的集成测试）。
• P3 折叠目录低估计数 — 属实。两个 status 探测均加 -uall，未跟踪/忽略目录按文件逐一计数；同时为避免 node_modules 展开后撑爆 API/UI 负载，携带的 porcelain 限行 200 行（带截断标记），计数始终基于全量输出。钉住：TestDeleteUntrackedDirCountsFilesIndividually。
• P3 非 ASCII 八进制转义 — 属实。探测加 -c core.quotePath=false。钉住：TestDeleteRefusalKeepsNonASCIIPathsLiteral（中文文件名保持字面）。
• Nits — 三条均已修：force 失败展示 (e.body && e.body.message) || e.message；#modal-wt-dirty 加 cancel 监听重置悬挂状态；map[bool]string 改为普通 if。

go test ./... 全绿，gofmt/go vet 干净。

[linletian]

Reply to the independent review — every finding was verified locally before fixing (the key one reproduced empirically); all accurate. Fixes in da2171a:

• F1 (force gated on a successful git status) — Confirmed and reproduced exactly as described: with a corrupt worktree index, git status exits 128 while git worktree remove --force succeeds. The probe now runs ONLY without force — its output exists to build the refusal, and force skips it entirely, so the escape hatch works on exactly the damaged workspaces it exists for. Pinned by TestDeleteForceSkipsStatusProbe (real-git integration with a corrupted index).
• F2 (clean worktree with only gitignored files deleted silently) — Accurate observation. I deliberately did NOT take the blocking route: almost every real worktree carries ignored build artifacts, so refusing whenever Ignored > 0 would make force the everyday path and would diverge from git worktree remove's own behavior. Instead the delete now counts what it destroys and surfaces it non-blockingly: ignored_destroyed in the API and MCP success responses, a post-delete notice in the dashboard, and a stderr note on the CLI. Ignored directories count file-by-file (-uall). Pinned by TestDeleteCleanWorktreeReportsIgnoredDestroyed / TestHandleWorktreeDeleteCleanReportsIgnoredDestroyed.
• F3 (trailing --force silently dropped) — Confirmed. The CLI now scans for --force/-force manually (any position) and rejects extra positionals with a usage error. Pinned by TestRunWorktreeDeleteTrailingForceFlag / TestRunWorktreeDeleteExtraPositionalFails.
• Minor (MCP 400 vs 409) — Fixed: the MCP worktree_delete dirty refusal now returns the same 409 + worktree_dirty + structured dirty breakdown as the HTTP API. Pinned by TestHandleMCPWorktreeDeleteDirtyRefusalIsStructured; docs/API.md updated.

Full go test ./... green, gofmt/go vet clean.

[linletian]

Second-round independent review — PR #105

All four prior findings are fixed and pinned by tests that genuinely run. Verified locally: gofmt -l . clean, go build ./..., go vet ./..., and the full go test ./... green; the new tests pass with -v (not skipped).

• F1 (force no longer gated on git status) — fixed. The probe moved inside if !force (internal/worktree/worktree.go:497), so a worktree whose status fails can still be force-removed. TestDeleteForceSkipsStatusProbe corrupts the index (asserts the probe really fails) and pins both directions: force removes, non-force still errors. Correct.
• F2 (clean delete silently destroys gitignored files) — fixed. The destroyed count is now returned as ignored_destroyed and surfaced through every client: HTTP success (internal/app/app.go:1144), MCP success (app.go:3204), a CLI stderr note (internal/cli/cli.go:233), and a dashboard post-delete notice (internal/ui/static/index.html:3368). Pinned by TestDeleteCleanWorktreeReportsIgnoredDestroyed / TestHandleWorktreeDeleteCleanReportsIgnoredDestroyed. Correct.
• F3 (trailing --force silently dropped) — fixed. The CLI scans --force/-force manually and rejects extra positionals (internal/cli/cli.go:217-227); pinned by TestRunWorktreeDeleteTrailingForceFlag / TestRunWorktreeDeleteExtraPositionalFails. Correct.
• Minor (MCP parity) — fixed. The MCP dirty refusal now answers the same 409 + worktree_dirty + dirty breakdown (app.go:3204ff), pinned by TestHandleMCPWorktreeDeleteDirtyRefusalIsStructured. Correct and consistent with the codebase's existing non-200 MCP error convention.

The bonus hardening is also sound: -uall (untracked/ignored dirs count file-by-file) and -c core.quotePath=false (literal non-ASCII paths) on both probes, counts/FirstPaths computed from the full output while only the carried porcelain is capped at 200 lines, and the two UI nits (human body.message on a failed force, cancel listener resetting dangling dialog state).

Remaining minor items — non-blocking, no new defect found:

1. CLI flag parsing is now hand-rolled, so --force=true / --force=false (and unknown flags such as --verbose) become positional args and yield a generic usage: error, and -- is not handled. All of these fail loudly — no silent drop — so this is a UX nit; consider recognizing the --force=<bool> form so it stays consistent with the rest of the CLI's flag-based subcommands.
2. Both -uall --ignored probes swallow their error and are bounded by the 10s GitCommand timeout. On a pathologically large ignored tree a probe timeout leaves Ignored/ignored_destroyed at 0, re-silencing the exact loss this PR exists to surface (on the clean path the delete still proceeds). Edge case, but worth a comment or a large-tree-safe count.
3. By design ignored_destroyed is 0 on the force path (probe skipped), so an API/MCP client calling force:true without a prior refusal gets no signal (documented in docs/API.md). Accepted tradeoff — flagging only to confirm it is deliberate.

No changes made — second-round review only.

[linletian]

第二轮评审（qwen3.8-flash-next）

复核范围：修复提交 da2171a 全量 diff + 整条 PR 重扫。本地验证：go test ./... 全绿（21 包）、gofmt -l/go vet 干净、CLI 参数边界用构建产物实测（--force=true、缺 id 均正确报 usage 错误，非静默）。

第一轮 7 项全部确认修复且各有测试钉住：

• ✅ 后置 --force——手动扫描 + 多余位置参数报 usage（TestRunWorktreeDeleteTrailingForceFlag/_ExtraPositionalFails）
• ✅ force 跳过 status 探针——损坏 index 实测 git status 失败但强删成功（TestDeleteForceSkipsStatusProbe 还钉住了非 force 路径必须透传探针失败，好）
• ✅ MCP 409 parity（TestHandleMCPWorktreeDeleteDirtyRefusalIsStructured）
• ✅ -uall + -c core.quotePath=false——-c 位于子命令前，经 gitx.GitCommand 原样透传，拼写正确（TestDeleteUntrackedDirCountsFilesIndividually/_NonASCIIPathsLiteral）
• ✅ 干净删除的 ignored_destroyed 上报（API/MCP/stderr/dashboard 四处一致，TestDeleteCleanWorktreeReportsIgnoredDestroyed 钉住折叠目录按文件计）
• ✅ UI 两处 nit（body.message 优先、cancel 监听复位；监听在 script 块内、dialog 之后，位置正确）
• ✅ porcelain 200 行封顶且计数在封顶前用全量计算（顺序正确）

本轮新发现（均为 minor/nit，不阻塞）：

1. force 路径 ignored_destroyed: 0 语义混淆（worktree.go 返回契约 / docs/API.md）：force 时探针跳过，0 实为"未统计"，与"真零"不可区分；文档已如实声明，但若想让客户端明确知道，可考虑 force 时省略该字段或置 null。可接受的取舍，留作决定即可。
2. 过期注释（worktree.go:342）：maxDirtyFirstPaths 上方仍写 "the full list is always available in Porcelain"——Porcelain 现已封顶 200 行，建议改为 "up to maxDirtyPorcelainLines lines"。
3. 语法 nit：capDirtyPorcelain 截断 1 行时输出 "1 more lines truncated"。
4. 观测：干净删除现在多付一次 status --ignored -uall 调用（超时上限 10s）——为准确计数值得，仅记录。

结论：建议合并。核心逻辑、契约（409/worktree_dirty/ignored_destroyed）、文档、测试四层一致，边界处理经得起实测。

[linletian]

回复第二轮评审（qwen3.8-flash-next） — 感谢确认。本轮 4 项处理如下（9868fb6）：

1. force 路径 ignored_destroyed: 0 语义混淆 — 采纳改进：force 成功时省略该字段（HTTP + MCP 一致），语义变为"字段存在=已统计、缺失=未统计"，不再依赖文档消歧。钉住：TestHandleWorktreeDeleteForceDeletes 现断言响应不含该字段；非 force 路径仍由 TestHandleWorktreeDeleteCleanReportsIgnoredDestroyed 钉住存在性。docs/API.md 已同步。
2. 过期注释 — 已改：maxDirtyFirstPaths 注释现为 "the remaining paths are in Porcelain (itself capped at maxDirtyPorcelainLines lines)"。
3. 单复数 nit — 已修：截断 1 行时输出 "1 more line truncated"。钉住：TestCapDirtyPorcelain（封顶原样 / +1 单数 / +2 复数三种情形）。
4. 多付一次 status 调用 — 确认属实且值得：干净删除才付这一次 --ignored -uall，换准确计数；已在代码注释中记录 10s 超时边缘（巨型忽略树计数归 0 不阻塞删除）。

另：英文评审提的 --force=<bool> 与 -- 解析一致性也一并补齐（见对那条评审的回复）。

go test ./... 全绿，gofmt/go vet 干净。

[linletian]

Reply to the second-round independent review — thanks for confirming the round-1 fixes. Your three remaining items, all in 9868fb6:

1. Hand-rolled CLI parsing (--force=<bool>, --) — Fixed for consistency with the flag-based subcommands: the scan now honors --force=<bool> in any position (a garbage value fails loudly with a clear error, never a silent default) and treats -- as the end-of-flags terminator. Pinned by TestRunWorktreeDeleteForceEqualsForms / _InvalidForceValueFails / _DashDashTerminator.
2. Ignored-count probe swallowing its error / 10 s timeout — Kept non-blocking deliberately (blocking a legitimate clean delete on a counting failure is worse than occasionally missing the note), and the edge case is now named in a code comment at the clean-path probe. Docs state the same: counting is best-effort, a probe failure never blocks the delete.
3. ignored_destroyed: 0 on the force path — Was indeed only a documented convention; now made structural: a forced success OMITS the field (HTTP and MCP), so "present = counted, absent = not counted" needs no doc caveat. Pinned by TestHandleWorktreeDeleteForceDeletes asserting the field's absence; the non-force presence stays pinned by TestHandleWorktreeDeleteCleanReportsIgnoredDestroyed. docs/API.md updated.

Also folded in the other review's nits (stale maxDirtyFirstPaths comment, singular "line" in the truncation marker — pinned by the new TestCapDirtyPorcelain).

Full go test ./... green, gofmt/go vet clean.