[mabry1985]

Summary

An oracle-timeout block now keeps the card's worktree so board_salvage_feature can publish green work (#538).

When coder.solve() trips the acceptance-oracle timeout breaker (#459), the card is blocked oracle-timeout. A timeout is not a verdict on the code — the build it judged is often green (bd-b4pr: a dependency bump whose build and tests passed). The drive used to _discard_tree that worktree (saving it to a stranded/… branch and removing it), after which board_salvage_feature — which publishes only from a live worktree — refused with "no worktree of has changes vs main."

• loop/drive.py: on the OracleTimeout path, replace the _discard_tree call with a new _keep_tree_for_salvage helper that keeps the worktree and writes a card comment naming the tree path (falling back to the card's feature_worktrees, since the solve offload reaps its own candidate trees and the drive's wt is usually None here) and the salvage that can publish it (board_salvage_feature / POST /features/<id>/salvage), or a cancel if the work is unwanted. This is the same keep-the-work stance the closed-source-issue guard takes (protoAgent#3832). Nothing reaps the tree meanwhile: the health sweep's reaper only takes done/cancelled/gone trees, and oracle-timeout is not in _SELF_HEALING_BLOCKS, so the blocked sweep neither requeues nor removes it — it escalates to a human and waits for salvage or cancel.
• Every other block class (e.g. SolveExhausted) keeps its existing _discard_tree behaviour, and the oracle budget (coder_solve_test_timeout_s) is unchanged.

tests/test_oracle_timeout_keeps_tree_538.py (new) covers all five requirements with real git worktrees: the drive keeps the tree and names the salvage (r1), loop.salvage publishes the kept tree instead of refusing (r2), the health-sweep reaper and the blocked sweep both leave the tree in place while the card stays blocked (r3), and a non-oracle SolveExhausted still discards (r4). changelog.d/538.md (new) records the change.

Fixes #538

[protoreview]

QA panel review — PASS

code-review-structural · head 9527f0217e07 · formal

Low-risk PR: the oracle-timeout path now keeps the worktree instead of discarding it, enabling salvage. All finders agreed the logic is correct; the only actionable items are a stale doc row in docs/lifecycle.md that contradicts the new keep-instead-of-discard behavior, and a dead base parameter. Verification confirmed both findings with no changes. No gaps.

Findings

	Severity	Location	Finding	Verified
🟡	minor	docs/lifecycle.md:550	The 'Edge | What it does with such a tree' table states a drive's terminal block 'saves the drive's own tree, removes it', but this PR makes the oracle-timeou…	confirmed
⚪	nit	loop/drive.py:457	_keep_tree_for_salvage accepts a base keyword argument that is never used in its body — a dead parameter copied from _discard_tree's signature.	confirmed

findings JSON (machine-readable)

[
  {
    "file": "docs/lifecycle.md",
    "line": 550,
    "severity": "minor",
    "category": "conventions",
    "claim": "The 'Edge | What it does with such a tree' table states a drive's terminal block 'saves the drive's own tree, removes it', but this PR makes the oracle-timeout terminal block KEEP the tree, so the doc now contradicts the code for that class and the exception is not noted.",
    "evidence": "| a drive's terminal block, or an operator cancel | saves the drive's own tree, removes it, and says so on the card.",
    "verdict": "confirmed",
    "note": "Read docs/lifecycle.md at head: the table row says 'saves the drive's own tree, removes it' for all terminal blocks; the diff replaces _discard_tree with _keep_tree_for_salvage for oracle-timeout, so the doc's blanket 'removes it' is now wrong for that class. No exception noted in the doc."
  },
  {
    "file": "loop/drive.py",
    "line": 457,
    "severity": "nit",
    "category": "conventions",
    "claim": "_keep_tree_for_salvage accepts a `base` keyword argument that is never used in its body \u2014 a dead parameter copied from _discard_tree's signature.",
    "evidence": "async def _keep_tree_for_salvage(self, store, fid: str, repo: str, wt, branch, *, base: str = \"\") -> None:",
    "verdict": "confirmed",
    "note": "Read loop/drive.py lines 457\u2013488 at head: `base` appears only in the signature; the body (trees/kept/salvage/text/_comment) never references it. Call site passes base=base but the function ignores it."
  }
]

---

1 finding(s) excluded from the verdict by in-diff confinement (file not among this PR's changed paths):

• docs/lifecycle.md (minor) — The 'Edge | What it does with such a tree' table states a drive's terminal block 'saves the drive's own tree, removes it', but this PR makes the oracle-timeout

[protoreview]

Promoting the PASS verdict for head 9527f0217e07: all checks terminal-green, zero unresolved review threads. (approve-on-green)

Open findings carried by this approval — non-blocking, but they did not go away:

• minor docs/lifecycle.md:550 — The 'Edge | What it does with such a tree' table states a drive's terminal block 'saves the drive's own tree, removes it', but this PR makes the oracle-timeout terminal block KEEP the tree, so the doc
• nit loop/drive.py:457 — _keep_tree_for_salvage accepts a base keyword argument that is never used in its body — a dead parameter copied from _discard_tree's signature.

Approving a WARN does not resolve its findings (issue #22).

[mabry1985]

Review findings (blocking)

1 finding(s): 1 major.

Major

• loop/drive.py:2358 [correctness] — confirmed: The new keep-the-tree branch can't find a tree in production. OracleTimeout is raised inside coder_seam.dispatch(), whose except Exception handler force-removes every candidate worktree and deletes its branch before re-raising. So when this call runs, wt is None and feature_worktrees() finds nothing. The card always gets the 'No worktree ... survived' comment, board_salvage_feature still refuses (the bd-b4pr / coder.solve oracle still times out on green dependency-bump cards; finished worktree is cleaned up so salvage can't publish it #538 failure), and if a stale feat- tree from an earlier run exists, the comment calls it the green build. Flagged independently by both the correctness and cross-file reviews.
  • evidence: await self._keep_tree_for_salvage(store, fid, repo, wt, branch, base=base)
  • verifier: Re-read at PR head: verify() raises OracleTimeout inside solve(), and dispatch()'s except Exception removes every adapter.candidates tree and re-raises. Promotion to feat- only happens on a win, so no live tree is left for _keep_tree_for_salvage (drive.py:2358) to find. A stale feat- tree would get the 'this build is often green' comment. One sub-detail I didn't check: whether remove_worktree saves to a branch before removing. That doesn't change the defect, since salvage needs a live tree.

The coder is re-driving a fix for these findings.

#545