Tell the agent where its salvaged work went (#3684)

[jwbron]

Closes #3684.

The gap

The salvage half of the re-attach discard path works — #3639/#3644 built it and the issue observed 8 successful salvages in one pipeline. The telling half was missing, and the cost of missing it is the whole session: the observed coder correctly diagnosed that its files were gone, had no path back to them, and started re-implementing 8 commits / 3072 insertions that were on the recovery ref the entire time. Restoring was a two-command fast-forward.

Root cause

The issue's read was that the bus message never named the ref. It does — since #3509 the body has said The full commit stack is preserved on remote ref .... The failure is one level down: the record was unreadable.

_record_discarded_tip built its Message without phase, leaving it at the None default. Both readers behind mcp__brc__read_peer_artifact — the live /brc-transcript route (routes/messages.py:541) and _write_brc_history (routes/pipelines/_brc_history.py:326) — select with:

[m for m in messages if m.message_type in BRC_HISTORY_TYPES and m.phase == phase]

So the record matched neither filter. Written every time; readable never. It was the only STATUS emitter in the tree that skipped the field — every other one threads phase=....

What changed

Pull channel. phase is threaded spawn_event_job → _try_reuse_worktree → _clean_reused_worktree → _record_discarded_tip → Message.phase, so the record reaches the transcript. integration_tests/regression/test_unpushed_commit_salvage.py pins the coupling from both ends: the emitter sets a phase, and a phaseless record is dropped by the exact filter its readers apply.

Push channel. The re-attach appends one notice per discard ({repo, recovery_ref, tip_sha, reset_to, n_commits, fast_forward, wip_*, salvage_error}) to a caller-supplied recovery_out list. spawn_event_job serialises it into EGG_WORKTREE_RECOVERY on the successor pod, and routes/event_prompt/_render_recovery.py renders it as the first section of that pod's prompt — ahead of the event banner, because an agent that reads the event, opens the tree, and finds its files missing has already formed the "I must re-implement" conclusion by the time a later section could correct it. Shape and contract mirror #3537's EGG_EVENT_RELEASE_CONTEXT; the key is unset on every ordinary spawn, so the common path is byte-stable.

The pull channel alone would not have fixed this. A coder facing an empty worktree does not think to go read a BRC transcript.

The instructions are now executable. The message used to say "build on it (cherry-pick or reset)". git reset --hard <recovery tip> cannot work: pipeline sessions enforce an off-lineage-reset guard (gateway/gateway/_git_execute.py:468) that requires the target be an ancestor of HEAD, and a recovery tip is a descendant by construction — it 403s every time. The issue asked whether the gateway permits the recovery; the answer is that git fetch is unrestricted and git merge --ff-only is allowlisted and unguarded, but the one command we were recommending is the one that is refused.

The discard path now probes merge-base --is-ancestor <reset target> <doomed tip> (not already known on the dirty arm, which is the dominant discard) and emits:

• fast-forwardable → git fetch origin <ref> + git merge --ff-only <sha>
• diverged → git log --oneline <range> + git cherry-pick <range>
• either way → an explicit "do NOT git reset --hard onto it, the gateway 403s off-lineage resets"

Wording. Subject was Unpushed commits discarded on re-attach — the one line every summary view renders, saying "discarded" about work that was preserved. Now Unpushed commits PRESERVED on a recovery ref after re-attach; the body opens with NOTHING WAS LOST and the opening clause says "removed from your worktree". The salvage-failed arm keeps "discarded" (there it is true) and still emits a push notice — that case needs the agent more, because re-deriving burns the reflog window the recovery depends on.

Deliberately not done

Automatic restore of a fast-forwardable tip (the issue's "Better"). The topology is now computed, so it would be easy — but the R6 residue policy hard-resets a dirty pre-reset tree on purpose, so the successor never inherits a killed-mid-event working set. Reversing that is a policy decision with its own review history (#3506/#3639), not a drive-by. Noted in the docs and left on the issue.

Ratchets

Two allowlist edits, both deliberate:

• test_wait_instruction_ratchet — pure line-number shift; the new section sits ahead of the existing entry in _compose.py.
• test_prompt_sync_ratchet — a new justified entry. Design: coordination state is served, not replicated — retire git/prompt choreography for agent state exchange #3077 retired agents self-syncing to see peer proposals, a read the wrapper's sync_to_proposals merge and the served-read verbs now perform. There is no served read for this case: the agent's own commits were moved onto a recovery ref by the orchestrator, and no egg-artifact / read_peer_artifact path resolves one. Fetching it is the only recovery there is.

Verification

• make lint clean (ruff, ruff-format, mypy, and every repo check).
• make test: 22749 passed. 3 failures (test_reap_stale_egg_images ×2, test_git_client::test_worktrees_parent_detected) reproduce identically on an unmodified HEAD worktree — pre-existing, unrelated.
• New coverage: 6 spawner tests for the two channels + subject wording + the env size cap, 3 for spawn_event_job's env injection and phase threading, 11 composer/CLI tests for the rendered section, 4 integration regressions for the transcript filter.

[egg-reviewer]

Code Review — PR #3689 (Tell the agent where its salvaged work went, #3684)

Reviewed all 13 changed files against the full diff, traced both delivery channels end-to-end, and verified the load-bearing claims in the PR body against the code they cite. The design is right and most of it holds up under tracing. One blocking defect, four advisory items.

What I verified as correct (so it doesn't get re-litigated)

• The pull channel genuinely un-hides the record. "STATUS" is in BRC_HISTORY_TYPES (orchestrator/routes/pipelines/_brc_history.py:40), the live /brc-transcript filter is m.message_type in BRC_HISTORY_TYPES and m.phase == phase (orchestrator/routes/messages.py:541), _apply_delphi_filter only redacts CONSENSUS_PROPOSE and passes STATUS through (messages.py:368-412), and brc_read_peer_artifact merges the live route with disk (sandbox/egg_agent_tools/handlers/brc.py:1178-1200) — so this is readable mid-phase, not only after _write_brc_history runs at phase completion. Threading phase is not a cross-module no-op.
• phase is always populated on the production path. The sole spawn_event_job caller sets common_kwargs["phase"] = _pkg._resolve_live_phase(...) (_concurrent.py:180-275), and _resolve_live_phase normalises the enum to str | None (_concurrent.py:113-156), so Message.phase gets a plain string. _events.py reads it with .get and does not pop it, so it still reaches spawn_agent_job.
• The fast-forward probe direction is right. git merge-base --is-ancestor <remote_tip> <local_head> returns 0 iff the doomed tip is a strict descendant of the reset target — which is exactly the condition under which git merge --ff-only <tip> succeeds from the post-reset position. The Do NOT git reset --hard onto it warning matches the gateway's off-lineage guard (gateway/gateway/_git_execute.py:468), and git fetch / git merge --ff-only are both unguarded, so the advised commands are actually executable in a pipeline session.
• Both ratchet allowlist edits are accurate, and the regex only matches fetch|merge|pull, so the cherry-pick / log / reset strings in _render_recovery.py correctly need no entries. _compose.py:182 still holds the pinned egg-orch message wait-loop line.
• No missed call sites. _try_reuse_worktree has one production caller; compose_event_prompt has one (_cli.py:263). The new kwargs are keyword-only with defaults, so no external breakage.
• The tests drive production code. TestDiscardRecordReachesTheAgent runs the real _clean_reused_worktree against a seeded temp worktree, and the integration tests use the real BRC_HISTORY_TYPES / SLICE_ID_PATTERN rather than hand-rolled copies. No self-seeding goldens, no fixture that bypasses the production path.
• Double-backticks in emitted prompt text (_render_recovery.py:126) match existing convention (_render_memory.py:88-92) — not a finding.

---

Blocking

1. The 2 KiB env cap silently drops the entire notice — and the only notice that can overflow is the salvage-FAILED one

_recovery_env_json (orchestrator/kubernetes_spawner/_events.py:294-300) drops whole trailing entries until the JSON fits. With a single oversized notice, kept.pop() empties the list and the function falls out of the loop to return "[]".

The docstring justifies the cap with:

Every field is a sha, a count, or a ref name, so a single notice is well under 300 bytes and the cap only bites on a pathological many-repo discard.

That is not true of salvage_error. Tracing it:

• _worktree.py:794 puts "salvage_error": salvage_error into the notice verbatim.
• salvage_error is salvage.error (_worktree.py:718) or str(e) (:720).
• On a failed salvage push, agent_salvage.salvage_discarded_tip returns error=push_result.describe() (orchestrator/agent_salvage.py:861, :952).
• PushResult.describe() is f"{category}: {detail}" (gateway_client/_models.py:66-73), and detail = stderr — the raw, unbounded git push stderr (gateway_client/_push.py:186-199).

Git push stderr carries every remote: line the server echoes (pre-receive hook output, policy rejection bodies, error dumps). Two-plus KiB is entirely ordinary there.

Failure scenario. Worktree re-attach discards N unpushed commits; the salvage push is rejected with a >2 KiB stderr. Then:

1. _recovery_env_json returns "[]".
2. _events.py:484 still sets EGG_WORKTREE_RECOVERY="[]", and :485-491 logs "Event spawn: injecting worktree-recovery notice into pod env" with recovery_refs computed from the uncapped recovery_notices — the operator log claims delivery that did not happen.
3. _cli.py:241-253: "[]" is truthy after .strip(), parses cleanly, is a list, so recovery_context = [] with no stderr warning.
4. _render_recovery_section([]) hits if not recovery: return "" (_render_recovery.py:37-38).
5. The prompt renders with no recovery section at all.

Note which case this is. recovery_ref is None exactly when salvage_error is set, so the oversized notice is always the salvage-FAILURE arm — the one where the commits exist only in the pod-local object store until gc, where the notice would have said "Do NOT start re-deriving the work", and where the PR's own docstring argues the notice matters most:

It is rendered with the same prominence — the case with no ref is the case where silently re-deriving costs the most.

The agent finds its files gone, is told nothing, and re-derives. That is the #3684 incident this PR exists to prevent, reproduced by the PR's own cap.

This is also the cross-module silent-no-op shape: the producer emits a synthetic value ("[]") that the consumer's guard excludes, and every layer in between treats it as success.

Test gap. test_env_json_drops_whole_entries_rather_than_truncating (test_kubernetes_spawner.py:5544-5562) asserts 0 < len(decoded) < 40 over 40 notices of ~250 bytes each. By construction it can never reach the return "[]" at _events.py:300. The all-dropped path has no coverage.

Suggested fix (any of these closes it; I'd do all three):

• Clamp at the producer: "salvage_error": (salvage_error or "")[:400] or None at _worktree.py:794. The full string is already in the WARNING log (_worktree.py:731) and the bus record, so nothing is lost for the operator.
• Make _recovery_env_json degrade to a minimal notice (repo / recovery_ref / tip_sha / reset_to) instead of "[]" when a single entry cannot fit — the ref is the whole point of the payload.
• Don't set the env key, and don't log the success line, when serialisation collapsed to []; log a WARNING instead. Right now the failure is invisible on both sides.

---

Non-blocking

2. docs/architecture/orchestrator.md composer table is now wrong

docs/architecture/orchestrator.md:1036-1050 documents the composer's section order and the signature. This PR adds a section that leads the whole prompt (_compose.py prepends recovery_section before event_section) and a new recovery_context kwarg, and neither appears. The table already lists a "Top | Park-release delta (#3537)" row for the exactly-analogous EGG_EVENT_RELEASE_CONTEXT, so the precedent for documenting it is right there.

The signature line is also stale in a way this PR widens — it lists task_description / iteration_feedback but not release_context (pre-existing, from #3537) and now not recovery_context. Worth fixing both while you're in there.

3. _cli.py swallows a well-formed non-list without a signal

_cli.py:242-253 prints to stderr on JSONDecodeError, but a value that parses to a dict, string, or number falls through the isinstance(decoded_recovery, list) guard and is dropped with no message at all. The malformed-JSON path is the less likely one; a future caller that serialises a single notice as an object rather than a one-element list would hit the silent arm. Add an else: that prints the same style of warning.

4. The salvage-FAILED arm's git reflog advice can be false

_render_recovery.py:95-100 tells the agent the tip "survives only in this worktree's local git object store" and to ask an operator to recover it via git reflog. But recovery_out.append at _worktree.py:776-802 runs before _git(d, "reset", "--hard", ...) at :797, and if that reset raises, _clean_reused_worktree returns False (:812) and the caller falls back to create-with-retry (_events.py:419-425) — a fresh worktree whose object store does not contain the tip. The env key is still injected.

This requires two compounding failures (salvage push fails and the reset fails), so I'm not treating it as blocking, but the guidance in that arm is the only thing standing between the operator and a real loss. Either move the append below the reset, or soften the wording to name the worktree by id rather than asserting "this worktree".

5. Remote-controlled text is interpolated into the agent prompt

salvage_error is git push stderr, which includes remote: lines echoed from the server, and it lands verbatim in the prompt at _render_recovery.py:88-94. GitHub is the remote so the practical risk is low, but a hostile or misconfigured pre-receive hook gets a free write into a "READ FIRST" prompt section. The clamp in finding #1 bounds the blast radius; consider also stripping newlines so the injected text can't fabricate its own markdown structure.

---

The core idea — push the ref into the prompt rather than hoping the agent pulls a BRC transcript — is the right read of the #3684 incident, and the "PRESERVED, not lost" framing plus the explicit merge --ff-only command is exactly what that coder needed. Fix the cap so the notice actually survives the case it was written for.

— Authored by egg

[james-in-a-box]

Review feedback addressed in commit e2738d9.

1. _recovery_env_json can drop the entire notice, and the spawn logs claim delivery anyway — fixed-in-PR (commit e2738d9)

All three of the suggested layers, since a lone oversized notice defeats any one of them alone:

• Producer clamp — _clamp_salvage_error (kubernetes_spawner/_worktree.py) collapses whitespace and caps salvage_error at 400 chars before the notice is ever built, so the one unbounded field can no longer be what pushes a notice over budget.
• Never drop the last entry — the trim loop in _recovery_env_json (kubernetes_spawner/_events.py) now stops at len(kept) > 1. If the single survivor still doesn't fit, it degrades to _WORKTREE_RECOVERY_MINIMAL_FIELDS (repo, recovery_ref, tip_sha, reset_to, fast_forward) rather than returning []. Those are the fields the restore command is built from — everything droppable is dropped first.
• Don't claim what wasn't delivered — spawn_event_job no longer sets ENV_WORKTREE_RECOVERY when the encoder returns "[]"; it emits a logger.warning naming the pipeline, the role, and every recovery_ref that did not fit, so an operator can hand-deliver. On the success path it now decodes what it is about to set and logs notices_delivered / notices_total plus the refs actually in the env — the pre-fix log listed the refs from the input list, which is exactly the "silently drops... logs as if it succeeded" case.

Tests: test_a_lone_oversized_notice_degrades_instead_of_vanishing, test_producer_clamps_the_one_unbounded_notice_field, test_event_job_does_not_claim_delivery_it_did_not_make, test_event_job_logs_the_refs_it_actually_delivered.

2. docs/architecture/orchestrator.md doesn't document the new prompt section — fixed-in-PR (commit e2738d9)

The compose_event_prompt signature line now carries recovery_context, and the composition table gains a First (ahead of the banner) row covering the section, the EGG_WORKTREE_RECOVERY env var, the 2 KiB cap, the minimal-fields degradation, the salvage_error clamp, and why the section leads the prompt.

3. _cli silently drops a well-formed-but-wrong-shape EGG_WORKTREE_RECOVERY — fixed-in-PR (commit e2738d9)

An elif decoded_recovery is not None arm now prints the decoded type to stderr and states the notice will not be rendered. Test: test_cli_warns_on_well_formed_recovery_env_of_the_wrong_shape.

4. The salvage-FAILED arm claims the tip is in "this worktree" — it may not be — fixed-in-PR (commit e2738d9)

You're right that the notice is appended before the reset --hard, so a reset failure sends the caller to create-with-retry and the successor may be running somewhere else entirely. I fixed it by making the text accurate rather than by moving the append below the reset: moving it would drop a valid notice whenever salvage succeeded but the reset then failed — the egg/recovered/... remote ref survives worktree recreation, so that notice is still correct and still worth delivering. Losing it to fix a wording bug is the worse trade.

The rendered text now names the predecessor worktree by worktree_id (added to the notice dict), says the tip "may still be in the local git object store of worktree <id>", and adds: "Note the worktree you are running in now may not be that one: a failed re-attach falls back to a fresh worktree, whose object store does not carry the tip." The docstring records the same reasoning. Test: test_recovery_section_names_the_worktree_rather_than_asserting_this_one.

5. Remote-controlled salvage_error renders unescaped inside a "READ FIRST" section — fixed-in-PR (commit e2738d9)

Two-sided. Producer side: _clamp_salvage_error (above) collapses whitespace and caps at 400 chars. Renderer side: _sanitize_free_text in _render_recovery.py collapses all whitespace to single spaces and caps at 500 chars, so remote: stderr cannot introduce its own headings, list items, or a fenced block that swallows the instructions below it. The renderer cap is deliberately looser than the producer's so a producer-clamped value renders with its own truncation marker instead of being truncated twice; it does not trust the writer, since the value arrives through a pod env var. Test: test_recovery_section_flattens_remote_controlled_error_text asserts a payload with embedded headings and fences renders as one flat line and that no line in the recovery section starts with # or a fence.

Verification. make lint — ruff and ruff-format clean; mypy's 3 errors are all in shared/egg_agent/client.py, untouched by this PR and failing identically on the base. Targeted suites: test_compose_event_prompt.py, test_prompt_sync_ratchet.py, test_wait_instruction_ratchet.py → 184 passed (the ratchet allowlist line pins for _render_recovery.py were updated to 105/106/112 after the edits shifted them). test_kubernetes_spawner.py gains the 4 new tests as passes; its 46 failures are the pre-existing sandbox gateway git init policy-env failures, identical with and without this change (verified by stashing). CI on this HEAD is the ground truth.

— Authored by egg

[egg-reviewer]

Agent-mode design review — no concerns.

The two delivery channels this PR adds both move toward the guidelines rather than away:

• Pull channel — threading phase into _record_discarded_tip's Message (orchestrator/kubernetes_spawner/_worktree.py) makes the discard record readable through mcp__brc__read_peer_artifact. That is restoring the agent's ability to fetch context for itself, which is the direction guideline 1 asks for.
• Push channel — EGG_WORKTREE_RECOVERY carries a fixed set of shas, a ref, and counts, capped at 2 KiB (_WORKTREE_RECOVERY_ENV_MAX_BYTES) with the free-text salvage_error clamped to 400 chars. That is orientation metadata in the "you last reviewed at commit X" sense, not a baked-in diff or file contents. The key is unset on ordinary spawns, so the common prompt path is unchanged.

On the one item that could read as guideline 4 (prefer what over how): _render_recovery.py names exact commands — git fetch + git merge --ff-only, or git cherry-pick <range> — and explicitly warns off git reset --hard. That falls under the guideline's own carve-out for "procedural context the agent can't easily discover." I confirmed the underlying claim rather than taking the PR description's word for it: the off-lineage reset guard at gateway/gateway/_git_execute.py:463 runs merge-base --is-ancestor <target> HEAD and 403s when the target is not an ancestor, and a recovery tip is a descendant by construction. An agent has no way to learn that from inside the sandbox except by burning a turn on the 403. Naming the permitted command is informing about a sandbox-enforced constraint, not substituting prompt text for enforcement.

Also checked and clear: no structured-output requirement on anything the agent emits, no script parsing agent output, no direct Anthropic API calls or claude --print, no pinned model identifiers.

Non-blocking observation: _sanitize_free_text collapsing remote-controlled git push stderr before it lands in a section headed "READ FIRST" is the right instinct for untrusted text in the highest-priority prompt position — flagging it as good, not as something to change.

Both ratchet allowlist edits are justified in-comment: test_wait_instruction_ratchet is a pure line shift, and the test_prompt_sync_ratchet entry correctly distinguishes fetching one's own orchestrator-moved commits (no served read exists) from the peer-proposal self-sync #3077 retired.

— Authored by egg

[egg-reviewer]

Re-review — PR #3689 (delta 4e0557a → e2738d9)

Approving. The blocking item from my previous review is fixed at every layer it needed to be, and the delta introduces nothing blocking.

Note on method: git log 4e0557a..HEAD is not resolvable in this worktree (HEAD is grafted with no parent), so I reconstructed the delta from the GitHub commit API.

Blocking item 1 — the 2 KiB cap collapsing a lone notice to "[]" — CONFIRMED FIXED

I re-ran the exact _recovery_env_json control flow rather than reading it, over the realistic payloads:

Input	Result
salvage-FAILED notice, salvage_error producer-clamped	802 B, full notice delivered including worktree_id, n_commits, wip_*
same, producer clamp bypassed ("remote: rejected\n" × 400)	degrades to 179 B minimal — and critically recovery_ref: None is retained, so _render_recovery.py:98 takes the salvage-FAILED arm and the agent still reads "Do NOT start re-deriving the work"
[]	"[]" — unreachable in production, guarded by if recovery_notices: at _events.py:504
40 fat notices	8 delivered, first entry intact

All three layers hold independently:

• Producer clamp. _clamp_salvage_error (_worktree.py:21-51) is applied at the sole recovery_out.append site (_worktree.py:813) — I grepped; there is only one. 400 chars + a 46-char marker, whitespace collapsed. The unbounded string still reaches _record_discarded_tip (_worktree.py:797) and the WARNING log unclamped, so test_kubernetes_spawner.py:4029's "gateway down" in msg.metadata["salvage_error"] assertion is untouched and the operator loses nothing.
• Serialiser floor. while len(kept) > 1 plus the _WORKTREE_RECOVERY_MINIMAL_FIELDS degradation (_events.py:307-323). The empty-input arm is preserved by the explicit if not kept: return "[]", so the removed while kept: behaviour is re-established — Angle B clean.
• Honest logging. _events.py:506-536 no longer sets the key on "[]", and the success line now decodes what it is about to set and reports notices_delivered / notices_total with the delivered refs. That closes the second half of the finding — the log claiming a delivery that did not happen.

The new coverage exercises the production functions directly (_recovery_env_json, _clamp_salvage_error, real spawn_event_job with a patched _try_reuse_worktree). test_a_lone_oversized_notice_degrades_instead_of_vanishing deliberately feeds an unclamped salvage_error so the serialiser's own floor is proven without the producer clamp propping it up — that is the right construction, since the two layers would otherwise mask each other.

Delta scan — no new blocking issues

• Renderer. _sanitize_free_text returns "" for None, so ... or "unknown error" preserves the pre-delta fallback exactly. The 500-char renderer cap sits above the producer's 446-char clamped output, so a clamped value renders once, not twice.
• Degraded-notice rendering. The minimal field set drops worktree_id, n_commits and salvage_error; the renderer reads all three through .get() with fallbacks ("the predecessor worktree", "commits", "unknown error"), and the first is asserted in test_recovery_section_names_the_worktree_rather_than_asserting_this_one. No KeyError path.
• CLI. The new elif decoded_recovery is not None arm cannot double-warn: the JSONDecodeError handler sets decoded_recovery = None first.
• Cross-file. The new worktree_id key is additive; no consumer validates the notice schema strictly, and the one equality assertion (test_kubernetes_spawner.py:2301) builds its own notice dict. _clamp_salvage_error resolves _pkg._SALVAGE_ERROR_NOTICE_MAX_CHARS at call time, so the barrel's define-before-import ordering is not load-bearing.
• Ratchet. I checked the three test_prompt_sync_ratchet.py pins against the current file: line 105 git fetch origin {ref}, 106 git merge --ff-only {tip}, 112 git fetch origin {ref}. All three land on the intended lines — the edit is a real re-pin, not a blind bump.

Per the review conventions I did not run make test; CI on this HEAD is the ground truth.

— Authored by egg

[egg-reviewer]

Re-review — PR #3689 (delta e2738d9 → 87951dd)

No agent-mode design concerns. Approving.

The delta is one clean merge, not new work

git log e2738d9..HEAD --not origin/main resolves to exactly one commit — the merge of origin/main (87951dd), whose PR-side patch is empty. I did not take that at face value, since a merge commit can smuggle hand edits through conflict resolution:

• PR source is untouched. git diff e2738d9 87951dd --stat does not list orchestrator/routes/event_prompt/, orchestrator/kubernetes_spawner/, or any of the four test files. Every producer and renderer I reviewed at e2738d9 is byte-identical.
• PR doc content is untouched. The two docs that do appear in that stat were changed on main's side (1fde992e, 8b6d1558). I isolated the PR's own contribution at both points — git diff <old-merge-base> e2738d9 vs git diff origin/main...HEAD, restricted to those files — and the added lines are identical, 14 either way. The merge took main's edits and preserved the PR's; nothing was rewritten in the resolution.

(git merge-tree is gateway-blocked, so the per-file comparison above is the substitute for a tree-equality check.)

Merged main commits don't touch this PR's channel

Worth confirming rather than assuming, because #3688 lands in the re-attach area this PR reads from. The merged commits change only routes/pipelines/ — git diff --name-only <merge-base> 38b029d9 matches nothing under kubernetes_spawner/ or event_prompt/. spawn_event_job remains the sole entry point for the push channel.

The interaction is in fact complementary: _await_reaped_jobs_gone (_pod_liveness.py:135) now blocks until orphaned Jobs are observed gone specifically so _clean_reused_worktree doesn't race a live orphan holding the worktree. That is the same function whose recovery_out.append (_worktree.py:813) produces this PR's notice, so the merge makes the notice's inputs more settled, not less. Nothing goes dark.

Standing assessment unchanged

The findings from my e2738d9 review carry forward as-is: the EGG_WORKTREE_RECOVERY payload is bounded orientation metadata (2 KiB cap, one clamped free-text field), not a baked-in diff; the Message.phase fix restores the agent's pull path through mcp__brc__read_peer_artifact; naming git merge --ff-only over git reset --hard informs about a sandbox-enforced guard rather than substituting for it; no structured-output requirement, no output-parsing script, no direct Anthropic API call, no pinned model ID.

— Authored by egg

[egg-reviewer]

Re-review — PR #3689 (delta e2738d9 → 87951dd)

Approving. The delta contains no PR-authored code changes — the single new commit is a clean merge of origin/main (38b029d, #3685) into the branch. My previous blocking item was already confirmed fixed and approved at e2738d9; nothing in this delta reopens it.

Note on method: e2738d9 was not present in this worktree (shallow clone), so I ran git fetch --deepen first to make the range resolvable.

The merge is not an evil merge

I verified this rather than assuming it, because a clean textual merge is exactly where a silent PR-side revert would hide:

• Main side preserved verbatim — git diff <merge-base> 38b029d and git diff e2738d9 HEAD are identical after normalising blob hashes and hunk offsets.
• PR side preserved verbatim — git diff <merge-base> e2738d9 and git diff 38b029d HEAD are likewise identical. No PR hunk was dropped, reordered, or re-resolved by the merge.

So there is no new code to review; the only real question is whether the merge introduces a semantic conflict. It does not:

Semantic-conflict trace (Angle C)

• No code-file overlap. The two changesets intersect only on docs/architecture/orchestrator.md and docs/reference/agent-recovery.md. Every Python file is disjoint: main touches routes/pipelines/{_pod_liveness,_run_implement,_slice_state}.py, this PR touches kubernetes_spawner/ and routes/event_prompt/.
• No new call sites for the widened signatures. _try_reuse_worktree still has exactly one production caller (_events.py:420); main's references to _clean_reused_worktree (_pod_liveness.py:151, orchestrator.md:141) are prose in a docstring and a doc paragraph, not invocations. Both new parameters (phase, recovery_out) are keyword-with-default, so main's code compiles against them unchanged either way.
• The BLE001 audit ratchet survives. This was the one place both sides plausibly collided: main raised the cap 124 → 126 for two new audited sites, and this PR adds an except Exception:. The PR's site is in kubernetes_spawner/_worktree.py, outside the audit's routes/pipelines rglob scope (test_ble001_narrowing_audit.py:56), so the counts do not stack. Confirmed by running it: test_ble001_narrowing_audit.py passes on the merged tree.
• Both ratchets still pin the intended lines. test_wait_instruction_ratchet is line-number sensitive and this PR's edit to it was a pure shift, so a merge that moved _compose.py would have broken it. Main does not touch _compose.py, and I confirmed empirically — test_wait_instruction_ratchet.py and test_prompt_sync_ratchet.py pass on 87951dd (63 tests green across the three file-scanning suites).
• Docs merged into disjoint sections, no contradiction. Main's additions land in the Layer-C bootstrap-reconciliation section (orchestrator.md:126-160, agent-recovery.md:329); this PR's land in the composer table (orchestrator.md:1056) and the worktree-re-attach section (agent-recovery.md:304-312). Main's new paragraph narrates _clean_reused_worktree's reset --hard as the hazard its bounded teardown wait exists to close — consistent with, not contradicting, this PR's description of the same function. If anything the two compose: reaping the orphaned Job before the successor attaches removes a race that would have produced exactly the kind of discard this PR's notice reports.

Per the review conventions I did not run make test; CI on this HEAD is the ground truth.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

6 previous review(s) hidden.