fix(delegate): don't delete a subagent's uncommitted work when git inspection fails

[kshitijk4poor]

Summary

A delegated subagent's uncommitted work is no longer destroyed when a git inspection probe fails — and the parent agent is now told the preserved worktree needs a look.

Salvage of #88125 by @liuhao1024 (authorship preserved via cherry-pick) onto current main, plus a follow-up commit closing a caller-visibility gap found during review.

Root cause: with delegation.worktree_isolation: true, finalize_subagent_worktree() pre-seeded its result payload with commits: 0, dirty: False, then ran two probes (git rev-list --count, git status --porcelain) that only overwrote those values on a zero exit. A non-zero exit left the defaults untouched — and the prune condition read them as proven clean, running git worktree remove --force + git branch -D. Untracked/uncommitted child work was irrecoverable, and the entry even reported pruned: true.

Changes

Commit 1 — @liuhao1024: fail closed on inspection uncertainty

• tools/subagent_worktree.py: an inspection_ok flag; a non-zero rev-list/status exit skips the destructive prune and keeps the worktree + branch with a WARNING naming both.
• tests/tools/test_subagent_worktree.py: regression test using a real repo + worktree whose index is corrupted so git status --porcelain really exits 128.

Commit 2 — follow-up: make the preserved worktree visible to the parent

• tools/subagent_worktree.py: one _unproven() helper stamps inspection_failed: True + a note naming the worktree/branch, warns, and returns. Both unproven exits route through it so they can't drift apart again.
• tools/subagent_worktree.py: the pre-existing except Exception path (timeout, OSError, non-numeric rev-list stdout) produced the same unproven payload but logged at DEBUG — effectively silent. Identical outcomes now get identical reporting.
• tools/delegate_tool.py: the caller's finalize-raised fallback assigned the creation-side metadata dict (path/branch/repo_root/base_commit) — a disjoint schema missing commits/dirty/pruned. It now emits the same flagged shape, at WARNING.
• Docs + docstring + module contract state that pruning requires affirmative proof, so a future cleanup doesn't "fix" the preserved worktree by restoring the unconditional prune and reintroducing this P1.

Commit 3 — review fold: assert the contract, not its prose

• The distinguishability test asserted the failure payload was byte-identical to the genuinely-clean one, which froze the ambiguity as a required property — a future commits: None for "unknown" would improve exactly what [Bug]: subagent worktree cleanup deletes uncommitted work when Git inspection fails #88113 is about and fail the test. Now asserts what the parent depends on: both keep the worktree, only the flag separates them.
• assertNotIn("inspection_failed", ok_payload) pinned key absence, forbidding an always-present-but-False flag (a stabler JSON contract). Now tolerant of that refactor.
• Dropped assertIn("UNKNOWN", note) prose coupling — the two producers' notes disagreed on the token anyway. Tests now assert the note names the worktree and branch (the actionable part), and both producers' wording was aligned.
• The raises test never proved its patched seam ran; now checks call_count and mirrors its sibling's branch-survival leg.
• New WorktreePayloadSchemaTests: AST-parses delegate_tool's real fallback dict and compares it against live finalize_subagent_worktree() output, so the two producers of this schema can't drift and the pre-fix leak (repo_root/base_commit, missing commits/dirty/pruned) can't return.
• Docs/docstring drift: the flag has a second trigger (finalization itself raising) and the module docstring omitted note. Both fixed.

Commit 4 — Phase 2c fold: extract the factory, drop the source-reading test

• The previous commit's schema guard AST-parsed delegate_tool's source. AGENTS.md:1514 bans that outright ("Never read source code in tests") — it passes when the implementation is subtly broken and fails on a correct refactor. Extracting the shared factory the rule prescribes also removes the duplication the AST test existed to police, so one change fixed both.
• subagent_worktree: new mark_worktree_payload_unproven() + unproven_worktree_payload(); both producers call them, so the schema can't drift and the note string exists once. delegate_tool's fallback is now 3 lines instead of 19, with a guarded re-import (the outer except can be entered because the import failed, which would otherwise raise NameError and lose the flag).
• Replaced the AST test with a behavioral one that calls the real factory and compares against live finalize_subagent_worktree() output.
• Closed one more fail-open path of the same bug class: with no base_commit the rev-list probe never ran, so commits kept its unproven 0 and a clean tree still reached git worktree remove --force. finalize is public and takes a caller-supplied dict, so this now fails closed too.
• Per-probe diagnostics: the note named neither which probe failed nor why. It now carries the probe name, exit code, and a bounded git stderr tail.

Commit 5 — /simplify-code residual: disclaim only what actually failed

• The note hard-coded "commits and dirty are UNKNOWN", but the probes fail independently: a bad base_commit fails rev-list while git status still succeeds, so dirty was a real measurement being reported as unknown. Safety was never affected (preserved either way), but telling the parent a measured value is untrustworthy is its own misreport. mark_worktree_payload_unproven() now takes unmeasured; the raising path still disclaims both, since which probe raised is unknowable there.

Purely additive — the happy-path payload shape is unchanged, so no existing reader can break.

Why commit 2 was needed

The fix stopped the deletion but still reported unproven values as facts, making the failure case byte-identical to "the child did nothing":

	commits	dirty	pruned
inspection FAILED, uncommitted work kept	0	false	false
inspected OK, child produced nothing	0	false	false

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 — it cannot read logs (no in-repo code reads the key back). So the parent's rational reading of the failure case was "no work produced": the rescued worktree survives, but nobody is ever told to recover it.

Validation

Bug reproduced on main and verified fixed, with real git repos/worktrees — no mocks:

	main @ 979ca57	this branch
worktree survives	no	yes
uncommitted work survives	no — destroyed	yes
branch survives	no	yes
pruned reported	true (wrong)	false
parent can tell inspection failed	n/a	inspection_failed: true + note
proven-clean tree still pruned	yes	yes (unchanged)

• Tests: 22/22 tests/tools/test_subagent_worktree.py; 127+ passed across test_delegate.py, test_delegate_batch_validation.py, test_delegate_control_actions.py, test_delegate_subagent_timeout_diagnostic.py. Ruff clean.
• Mutation-checked (guards have teeth): neutering the flag fails all 3 new assertions; reverting the production file to pre-fix main fails all 3 (re-run on the final 3-commit stack). Reverting delegate_tool's fallback to the pre-fix dict(_worktree_info) shape fails the new schema guard. All restores checksum-verified. Final stack: all 6 guards fail under both mutations.
• Lint/types: ruff clean; ty clean on subagent_worktree.py and 64-vs-64 unchanged on delegate_tool.py (all pre-existing, verified against the base commit).
• Coverage beyond the original test: the untested rev-list-failure path (bogus base, status exit 0, committed child work) also preserves correctly; the raising path is covered too.
• Edge case ruled out: base_commit == "" would skip the rev-list probe and prune on an unproven count, but it's unreachable — the same bad HEAD that fails rev-parse also fails worktree add, so create_subagent_worktree() returns None and finalize never runs.
• Whole bug class: cli.py's sibling probes (_worktree_is_dirty, _worktree_has_unpushed_commits) already return True on non-zero exit; this file was the last gap. The cli.py startup pruner independently classifies a corrupt-index worktree as dirty, so preserved trees are kept and surfaced in its 7-day stale-work WARNING rather than accumulating silently.

Windows creationflags on _run_git is deliberately out of scope — already owned by open #85243, no overlap with these hunks.

Closes #88125
Fixes #88113

Co-authored-by: liuhao1024 sunsky.lau@gmail.com