fix(delegate): keep the worktree when git inspection fails

[liuhao1024]

What does this PR do?

Stops finalize_subagent_worktree() from permanently deleting a delegated child's uncommitted work when its Git inspection probes fail. With delegation.worktree_isolation: true, the function ran git rev-list --count and git status --porcelain via _run_git(check=False); on a non-zero exit it silently kept the payload defaults (commits: 0, dirty: False) and then read those defaults as proven clean state — running git worktree remove --force and git branch -D, so untracked/uncommitted files were irrecoverably lost and the result even reported pruned: true (#88113, P1 data loss).

The documented contract — only a worktree with zero commits and a clean tree is pruned, "anything holding work is kept" — is now enforced on the failure path too: a destructive cleanup requires affirmative proof. Any non-zero inspection result keeps the worktree and branch for manual review, with a WARNING naming both. The existing exception handler was already fail-safe; this closes the ordinary non-zero-exit gap beside it.

Related Issue

Fixes #88113

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/subagent_worktree.py — finalize_subagent_worktree() tracks an inspection_ok flag; a non-zero rev-list or status exit clears it, and a non-inspectable worktree returns un-pruned with a warning (worktree + branch named) instead of falling through to the prune condition.
• tests/tools/test_subagent_worktree.py — new regression reproducing the issue's deterministic offline scenario: a real git repo + worktree whose index is corrupted (making git status --porcelain exit 128) with uncommitted work on disk must stay intact — worktree present, WIP file present, branch not deleted, pruned false. The _git helper gained a check parameter for the non-zero-exit sanity probe.

How to Test

1. python -m pytest tests/tools/test_subagent_worktree.py -q
2. Observed result: 16 passed (15 pre-existing + 1 new). A/B guard: with only the tools/subagent_worktree.py hunk stashed, the new test FAILS (worktree force-removed, WIP file gone, branch deleted — the data loss).
3. python -m pytest tests/tools/test_delegate_subagent_timeout_diagnostic.py -q — Observed result: 2 passed, 2 failed; both failures are pre-existing on clean main (verified via git stash A/B run) and unrelated to this change.
4. The issue's standalone reproduction script now reports worktree_exists: true, wip_exists: true, branch_exists: true, pruned: false.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS 15 (arm64)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

$ pytest tests/tools/test_subagent_worktree.py -q                    # with fix
16 passed in 2.21s
$ git stash -- tools/subagent_worktree.py && pytest ... -q           # A/B guard
1 failed  (worktree force-removed; uncommitted work irrecoverable)

[kshitijk4poor]

Thanks @liuhao1024 — this is a genuinely nasty P1 and your diagnosis was exactly right: the payload was pre-seeded with commits: 0, dirty: False, and a non-zero probe exit left those defaults in place where the prune condition read them as proven clean. I reproduced the data loss on main before touching anything (work destroyed, pruned: true reported) and confirmed your fix closes it.

Merged via #88419, with your commit cherry-picked so your authorship is preserved in git log (rebase merge, not squash).

Three follow-up commits on top of yours, all from review — none of them a correction to your fix:

1. The parent agent was never told. Your fix stops the deletion, but the failure payload still reported unproven commits: 0, dirty: False — byte-identical to "inspected fine, the child left nothing". I verified those two cases produced the same dict. The only failure signal was a logger.warning, and the sole consumer of this payload is the parent agent reading the serialized delegate_task entry, which cannot read logs. So the worktree survived but nothing ever told anyone to look at it. Added inspection_failed: true + a note naming the worktree and branch.
2. Test-contract cleanup on my own commit, plus aligning both producers of the payload.
3. One more fail-open path of the same bug class: with no base_commit the rev-list probe never runs, so commits keeps its unproven 0 and a clean tree still reached git worktree remove --force. Narrow to reach in practice, but finalize_subagent_worktree is public and takes a caller-supplied dict, so it now fails closed too — same principle as your fix, one site further out.

Also worth recording: your instinct to make the exception path fail-safe was already correct in the original code, and the sibling probes in cli.py (_worktree_is_dirty, _worktree_has_unpushed_commits) already returned True on a non-zero exit — so this file was the last gap in that bug class. Your PR completed the pattern.

Verification on the final stack: 21/21 in tests/tools/test_subagent_worktree.py, all 6 guards mutation-checked (they fail both when the flag is neutered and when production is reverted to pre-fix main), and E2E against real git repos confirming a clean tree still prunes.

Thanks again — this was a well-written report with a deterministic offline reproduction, which made it fast to verify. Credit to @Adkid-Zephyr for the original issue (#88113) too.