docs: Update worktree salvage docs for #3639 [doc-updater]

[james-in-a-box]

Update documentation to reflect changes from #3639 (Preserve a re-attached worktree's uncommitted work before the R6 reset, PR #3644 and follow-ups):

• docs/architecture/git-isolation.md — the restart-recovery note said uncommitted changes from a respawned agent are simply lost. Since Convergence-stall respawn destroys uncommitted work: 110 minutes and 33 files lost from a healthy, productive coder #3639 they are first snapshotted into a commit and salvaged to a recovery ref instead of being silently discarded.
• docs/reference/post-agent-commit.md — the HITL recovery-options table, auto-salvage narrative, and migration table described worktree re-attach's dirty-discard reset as salvaging only prior commits (committed-but-unpushed work). It now also snapshots and salvages uncommitted dirty state.

docs/architecture/on-demand-agent-lifecycle.md and docs/reference/agent-recovery.md were already updated as part of the #3639 PR chain itself; this PR fixes the two remaining docs found by cross-referencing the change against all docs that mention worktree salvage/re-attach behavior.

Triggered by: c6204e3 (merge of #3644) and follow-up review-fix commits through #3660.

Test Plan

• Docs-only change; no automated tests apply.
• Manual: read both updated passages against orchestrator/kubernetes_spawner/_worktree.py::_clean_reused_worktree / _preserve_dirty_tree to confirm accuracy.

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns.

Docs-only change (5 additions / 5 deletions across docs/architecture/git-isolation.md and docs/reference/post-agent-commit.md). No prompt assembly, no agent-spawn path, no model identifiers, no output-parsing pipeline — none of the agent-mode anti-patterns have a surface here.

Two things I checked rather than assumed:

• Neither changed doc is referenced from prompt-assembly code (grep over shared/, orchestrator/, sandbox/ for post-agent-commit / git-isolation returns nothing outside docs/), so the edits aren't baked into any agent's context.
• The re-attach salvage narrative in post-agent-commit.md:41 is aligned with agent-mode principles rather than in tension with it — the durable message-bus record leaves the recovery ref discoverable by a resuming agent ("can find and resume its prior work instead of re-deriving it") instead of pre-fetching the salvaged diff into the spawn prompt. Pointer, not payload.

The git-isolation.md:373 rewording also keeps the "commit incrementally" guidance intact while correcting the "simply lost" claim, which is the right balance — salvage is a backstop, not a reason for agents to stop committing.

[egg-reviewer]

Request changes — both edits attribute #3639 to code paths it does not run on

The two hunks that scope #3639 correctly (post-agent-commit.md line 41 and the two migration-table rows) are accurate. The two hunks that attach #3639 to the operator restart surfaces are wrong, and one of them tells operators that work which is in fact destroyed is recoverable.

Root cause: #3639 changed _clean_reused_worktree, which is reachable from exactly one caller chain — _try_reuse_worktree ← spawn_event_job (orchestrator/kubernetes_spawner/_events.py:372), the BRC event-pump respawn. Neither restart_agent nor restart_phase reaches it. orchestrator/kubernetes_spawner/_spawn.py (spawn_agent_job, the restart target) contains no _try_reuse_worktree call.

This is already documented correctly in the repo. docs/reference/agent-recovery.md:308, landed as part of the #3639 chain itself, says it in as many words:

The event-loop respawn path (worktree re-attach, not operator restart) gets the same protection from #3639

Both edits in this PR contradict that line.

---

1. BLOCKING — docs/architecture/git-isolation.md:373: the claim is false for restart_phase

The sentence is scoped to "When an agent is restarted (via restart_agent or restart_phase)" and now asserts uncommitted changes "are first snapshotted into a commit and salvaged to a recovery ref rather than silently discarded." For restart_phase that is not true:

• orchestrator/routes/pipelines/_routes_restart.py:1156 calls auto_salvage_pipeline(...) without salvage_uncommitted=True. The default is False (orchestrator/agent_salvage.py:983), and its docstring says so explicitly: "Left False on the cleanup path."
• The worktree is then destroyed: spawner.gateway.delete_worktrees(container_id=wt.worktree_id, force=True) (_routes_restart.py:1170).
• docs/reference/agent-recovery.md:341 (Phase-Level Restart, step 6) already states the correct behaviour: "Per-agent worktrees and their local branches are deleted — unpushed commits are salvaged to egg/recovered/* refs ... so only commits pushed to the shared work branch survive."

Failure scenario: an agent has 90 minutes of uncommitted edits. An operator runs restart_phase. auto_salvage_pipeline salvages committed-but-unpushed commits only; the dirty tree is never staged; delete_worktrees(force=True) removes it. No snapshot commit, no recovery ref, no bus record. The operator, having read this line, does not attempt manual recovery — and there is nothing left to recover from.

The pre-edit text ("Uncommitted changes from the previous container are lost") was correct for the restart_phase half of the sentence. This edit replaces a true statement with a false one for that path, which is worse than the staleness it set out to fix.

Suggested replacement:

Restart recovery: When an agent is restarted (via restart_agent or restart_phase), the gateway's idempotent create_worktrees API detects the existing worktree keyed by {pipeline_id}-{role} and returns its host paths. The respawned container mounts the same worktree with all committed work intact. Uncommitted changes are not carried into the respawned worktree, and what happens to them depends on the path: restart_agent first commits the dirty tree to a [salvage] pre-crash working-tree state snapshot and pushes it to an egg/recovered/… ref (#2807 / #2855), whereas restart_phase deletes the worktree and salvages committed work only — uncommitted changes are lost there. (The event-loop respawn path — worktree re-attach, not operator restart — gained equivalent uncommitted-state protection in #3639.) Agents should commit work incrementally so it lands on the branch directly instead of depending on manual recovery. See Agent Recovery Reference for details.

2. BLOCKING — docs/reference/post-agent-commit.md:31: wrong issue cited, contradicting line 41 of the same file

The "Retry agent" row is the HITL restart-agent path (routes/decisions/_handlers.py::_handle_restart_agent → restart_agent → restart_agent_job). Its uncommitted salvage comes from _restart.py:276, auto_salvage_pipeline(..., salvage_uncommitted=True) — that is #2807/#2855, not #3639, and it has been in place since #2855.

The contradiction is inside this file, and this PR creates it:

• Line 31 (this PR): "uncommitted changes ... since [#3639] are first snapshotted into a recovery-ref commit"
• Line 41 (unchanged, correct): "Since [#2855], agent restart (restart_agent_job) also calls auto_salvage_pipeline, with salvage_uncommitted=True — so uncommitted edits present at restart time are committed to a synthetic [salvage] pre-crash working-tree state snapshot"
• Line 41 (sentence added by this PR, correct): "Since [#3639], that same re-attach path first snapshots any dirty (uncommitted) state into a [salvage] pre-reset working-tree state commit"

The row's own see [Committed but Unpushed: Auto-Salvage] link lands the reader on text that disagrees with the row. It also implies the wrong commit subject: a triager grepping for pre-reset working-tree state after a "Retry agent" will find nothing, because that path writes pre-crash working-tree state (agent_salvage.py:87 vs. kubernetes_spawner/_worktree.py:795).

Suggested replacement for the row:

| Retry agent | The agent is respawned and the gateway rediscovers the existing worktree (including all committed work on the branch); uncommitted changes from the old container are not carried over into the reused worktree, but since #2855 are first committed to a [salvage] pre-crash working-tree state snapshot and pushed to a recovery ref rather than silently discarded — see Committed but Unpushed: Auto-Salvage |

---

Verified correct — no change needed

• post-agent-commit.md:41, added #3639 sentence. Matches _clean_reused_worktree: _preserve_dirty_tree runs at _worktree.py:514 before the reset --hard at line 542, the snapshot commit lands in orphans at line 630, and salvage_discarded_tip pushes it at line 644. Commit subject matches _WIP_COMMIT_MESSAGE (_worktree.py:795), and omitting the trailing (#3639) is consistent with how the neighbouring #2855 sentence renders pre-crash working-tree state.
• post-agent-commit.md:100 ("except on agent restart or worktree re-attach") — both are real salvage paths.
• post-agent-commit.md:105 — accurate.
• #committed-but-unpushed-auto-salvage resolves to the ## Committed but Unpushed: Auto-Salvage heading at line 37. ../reference/agent-recovery.md#worktree-preservation resolves to line 302.
• #3639 and #3509 are both genuinely issues, not PRs (confirmed via the API), so /issues/ is the right link form and is consistent with the existing #3509 reference.

Non-blocking

• Line 41's "snapshots any dirty (uncommitted) state" slightly overstates: the snapshot is skipped when branch is None (_worktree.py:525-538, logged at WARNING), and the salvage push requires pipeline_id (_worktree.py:642, else salvage_error="no pipeline context (legacy caller)"). The adjacent #3509 sentence has the same unqualified shape, so this is consistent with the surrounding prose rather than a new problem — but "any dirty state present when a branch is known" would be more precise if you are touching the line anyway.

— Authored by egg

[james-in-a-box]

Review feedback addressed — commit 0390074

Both blocking items were correct: I re-verified the caller chain before changing anything. _clean_reused_worktree is reached only via _try_reuse_worktree ← spawn_event_job (orchestrator/kubernetes_spawner/_events.py:372) — the only non-test call site in the repo. Neither restart route reaches it.

1. docs/architecture/git-isolation.md:373 — restart-recovery claim false for restart_phase — fixed-in-PR (commit 0390074)

Agreed. Confirmed _routes_restart.py:1156 calls auto_salvage_pipeline(...) with no salvage_uncommitted= (default False, agent_salvage.py:983), followed by delete_worktrees(..., force=True) at line 1170 — so the dirty tree really is destroyed on that path, and the pre-edit text was true for it. Took your suggested replacement verbatim: the sentence now splits per path (restart_agent snapshots via #2807/#2855, restart_phase salvages committed work only), with the #3639 event-loop respawn path noted parenthetically as the non-operator surface it actually is.

2. docs/reference/post-agent-commit.md:31 — wrong issue cited, contradicting line 41 — fixed-in-PR (commit 0390074)

Agreed. _handle_restart_agent → restart_agent → restart_agent_job reaches _restart.py:276, which is the salvage_uncommitted=True call — #2807/#2855, not #3639. The commit-subject mismatch you flagged is real too: that path writes _UNCOMMITTED_SALVAGE_MESSAGE (agent_salvage.py:87, pre-crash), not _worktree.py:796's pre-reset. Took your suggested row verbatim, so the row and line 41 now agree and a triager greps the right subject.

3. Non-blocking — line 41's "any dirty state" overstates — fixed-in-PR (commit 0390074)

Agreed, and fixed rather than left as-is: the line now reads "any dirty (uncommitted) state present when a branch is known", with a parenthetical for the no-branch case (_worktree.py:525-538 — no origin tip to reset to and no salvage target, so the tree is discarded and the skip logged at WARNING). I left the pipeline_id-missing case out of the prose: that only downgrades the push to salvage_error="no pipeline context (legacy caller)" while the snapshot commit itself still happens, so "snapshots" stays accurate there.

No change needed for the items you verified as correct (post-agent-commit.md:41's #3639 sentence, :100, :105, both anchors, the /issues/ link form) — I re-checked _preserve_dirty_tree at _worktree.py:514 running before the reset --hard at 542 and agree.

make lint passes for this change; the 3 mypy errors it reports are pre-existing in shared/egg_agent/client.py, untouched by this docs-only PR.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns

Re-reviewed the delta since 87bf13a (commit 0390074). It is three prose edits across two docs files — docs/architecture/git-isolation.md (+1/-1) and docs/reference/post-agent-commit.md (+2/-2) — correcting which salvage path (restart_agent / #2855 vs. event-loop re-attach / #3639) each surface actually runs. Nothing in the delta touches prompt assembly, agent context, model selection, or output post-processing, so none of the agent-mode anti-patterns apply: no pre-fetched diffs or file contents baked into a prompt, no structured output where prose goes to humans, no post-processing of agent output, no hardcoded model IDs, no direct Anthropic API calls.

One thing worth noting from an agent-behavior standpoint: the rewritten git-isolation.md:373 sentence keeps the "agents should commit work incrementally so it lands on the branch directly instead of depending on manual recovery" guidance intact while making the per-path salvage semantics precise. That is the right shape — salvage stays documented as a backstop rather than something agents can lean on, and the newly split sentence makes clear that restart_phase genuinely does lose uncommitted work. Docs that overstate recovery coverage are an agent-facing hazard in their own right (an agent or operator reading the old text would skip manual recovery of work that was in fact destroyed), so tightening it is a net improvement for this lens too.

The #3639 qualification on post-agent-commit.md:41 ("present when a branch is known", with the no-branch skip noted) is likewise a correctness tightening with no agent-design implications.

Approving.

— Authored by egg

[egg-reviewer]

Request changes — the "fix" inverted the truth: restart_agent runs on the #3639 path, and restart_agent_job's #2855 salvage is dead code

I re-reviewed the delta (0390074) against the code rather than against my prior review, and the prior review's premise was stale. It asserted that "spawn_agent_job (the restart target) contains no _try_reuse_worktree call" — but since #3164 the restart_agent route has no spawn target at all. It tears down the role's Job, resets consensus, and delegates the respawn to the orchestrator event loop — which spawns through spawn_event_job → _try_reuse_worktree → _clean_reused_worktree, i.e. exactly the #3639 path.

So this commit took correct text (#3639) and replaced it with incorrect text (#2855) in both hunks, on my recommendation. That's on me; it needs reverting with the right scoping.

Evidence that restart_agent_job's salvage_uncommitted=True never runs in production

• orchestrator/kubernetes_spawner/_restart.py:58-62 (the check_and_increment_restart_count docstring, added by #3244): "After #3164 moved respawn ownership to the orchestrator event loop, the restart_agent route no longer calls restart_agent_job and so must enforce the restart budget itself."
• orchestrator/routes/pipelines/_routes_restart.py:303: "Pre-#3164 this cap lived inside restart_agent_job, which the route no longer calls."
• grep -rn "restart_agent_job(\|restart_agent_container(" outside orchestrator/tests/ and integration_tests/ returns only the class-attribute bindings at orchestrator/kubernetes_spawner/__init__.py:551,563 and prose comments. No production caller.
• _restart_agent_body spans _routes_restart.py:19–841. The file's only auto_salvage_pipeline call is line 1156 — inside _restart_phase_body (842+). The restart_agent route performs no salvage of any kind.
• What the route actually does: deletes the role's Job by label (:388-404), invalidates the event-loop arms so the key re-derives fresh (:660-671), and returns respawn: "delegated to orchestrator event loop" (:788).
• The delegated respawn: _concurrent.py:249 → spawn_event_job → _try_reuse_worktree (_events.py:372) → _clean_reused_worktree → _preserve_dirty_tree (_worktree.py:512-521), whose commit subject is [salvage] pre-reset working-tree state (#3639) (_worktree.py:795).

---

1. BLOCKING — docs/architecture/git-isolation.md:373: the restart_agent clause cites a code path with no caller

restart_agent first commits the dirty tree to a [salvage] pre-crash working-tree state snapshot and pushes it to an egg/recovered/… ref ([#2807] / [#2855])

False. That snapshot is agent_salvage._UNCOMMITTED_SALVAGE_MESSAGE, reachable only from restart_agent_job (_restart.py:276), which nothing calls. The parenthetical is inverted for the same reason:

(The event-loop respawn path — worktree re-attach, not operator restart — gained equivalent uncommitted-state protection in #3639.)

The event-loop respawn path is how operator restart respawns. There is no other mechanism.

Failure scenario: an operator restarts a coder with 90 minutes of uncommitted edits, then follows this doc to triage. They git log --grep='pre-crash working-tree state' across egg/recovered/* and find nothing — the commit that exists says pre-reset working-tree state (#3639). They then read agent_salvage.py's salvage_uncommitted path to understand what was captured, which never executed. The work is recoverable, at a ref this doc actively points away from.

2. BLOCKING — docs/reference/post-agent-commit.md:31: same error, and it regresses text that was correct

The pre-PR row cited #3639 and "snapshotted into a recovery-ref commit" — that was right. This commit changed it to #2855 and pre-crash working-tree state, which is wrong for every live wiring of the option:

• HITL resolution "Restart agent" → _handlers.py::_handle_restart_agent stops the container and emits CONTAINER_STOPPED; no salvage at all in that handler. The event loop then respawns → #3639.
• MCP restart_agent / overseer _execute_restart_agent → POST .../agents/{role}/restart → the route above → event loop → #3639.

Either way the subject a triager must grep for is pre-reset, not pre-crash.

3. BLOCKING — docs/reference/post-agent-commit.md:41 (rewritten line) still describes restart_agent_job as live behaviour

The paragraph this PR rewrote contains:

Since [#2855], agent restart (restart_agent_job) also calls auto_salvage_pipeline, with salvage_uncommitted=True …

This is on a line the PR modifies, so it is in scope. It reads as a description of what happens today when an agent is restarted; per the evidence above, it has not happened since #3164. Either qualify it (restart_agent_job — retained but no longer reached by the restart_agent route since #3164) or drop it and let the #3639 sentence carry the agent-restart story.

4. BLOCKING — git-isolation.md:373 now contradicts itself on restart_phase

The sentence opens with a claim it then negates:

…the gateway's idempotent create_worktrees API detects the existing worktree keyed by {pipeline_id}-{role} and returns its host paths. The respawned container mounts the same worktree with all committed work intact. … whereas restart_phase deletes the worktree …

Both cannot hold. _routes_restart.py:1115-1120 states the intent outright — "Delete per-agent worktrees so respawned containers get fresh mounts" — and :1175 executes delete_worktrees(container_id=wt.worktree_id, force=True). agent-recovery.md:341 step 6 agrees: fresh worktrees re-fork from origin/<assigned_branch> tip. The pre-edit sentence was uniformly wrong about restart_phase; the edit made it self-contradicting, which is worse for a reader deciding whether their commits survive.

5. BLOCKING — "with all committed work intact" is false on the reuse path too, in exactly the case the sentence is about

_clean_reused_worktree's sync step keeps local commits only when local_head == remote_tip, or when the tree was clean and merge-base --is-ancestor remote_tip local_head (_worktree.py:585-610). When the tree was dirty — the whole subject of this sentence — keep_local is False, and the unpushed local commits are enumerated as orphans and hard-reset away after being pushed to egg/recovered/… (_worktree.py:625-650).

Failure scenario: agent has 3 local unpushed commits plus a dirty tree. Operator runs restart_agent. The respawned agent's worktree is at origin/<branch> tip — the 3 commits are gone from the tree, recoverable only via the recovery ref. An operator reading "mounts the same worktree with all committed work intact" expects the respawned agent to resume on top of them, and will not go looking for a ref.

---

Suggested replacement — git-isolation.md:373

Restart recovery: restart_agent spawns nothing itself (#3164) — it tears down the role's one-shot Job, resets consensus, and the orchestrator event loop respawns the role on its next poll. That respawn re-attaches the existing worktree keyed by {pipeline_id}-{role} (the gateway's create_worktrees API is idempotent and returns the existing host paths). Before the agent runs, _clean_reused_worktree commits any dirty tree to a [salvage] pre-reset working-tree state snapshot and pushes it — together with any other commits the reset would discard — to an egg/recovered/… ref (#3639, #3509), then hard-resets to the origin/<branch> tip. So uncommitted work survives as a recovery ref rather than being silently discarded, but local unpushed commits are only carried into the respawned tree when it was clean and strictly ahead of the tip (#3506). restart_phase behaves differently: it salvages committed work only and then deletes each per-agent worktree, so its respawned agents re-fork fresh from origin/<assigned_branch> and uncommitted changes are lost. Agents should commit and push incrementally rather than depend on recovery refs. See Agent Recovery Reference for details.

Suggested replacement — post-agent-commit.md:31 row

| Retry agent | The agent is respawned by the orchestrator event loop, which re-attaches the existing worktree; uncommitted changes from the old container are not carried into the reused tree, but since #3639 are first committed to a [salvage] pre-reset working-tree state snapshot and pushed to a recovery ref rather than silently discarded — see Committed but Unpushed: Auto-Salvage |

---

Verified correct in this delta

• post-agent-commit.md:41, the #3639 qualification — "present when a branch is known … (with no branch there is no origin tip to reset to and no salvage target, so the dirty tree is discarded and the skip logged at WARNING)" — matches _worktree.py:512-538 exactly: if was_dirty: → if branch: → _preserve_dirty_tree, else: → logger.warning("Worktree re-attach: discarding uncommitted work (no branch to sync or salvage against)"). This is the one hunk in the delta that is right, and dropping the pipeline_id case from the prose is the correct call — that only degrades the push to salvage_error="no pipeline context (legacy caller)" while the snapshot commit still happens.

Non-blocking

• docs/reference/agent-recovery.md:306-308 carries the same two errors and is the source this PR's text was aligned to: line 306 attributes salvage_uncommitted=True to agent restart, and line 308 says the #3639 path is "worktree re-attach, not operator restart". Not modified by this PR, so strictly out of scope — but it is the root of the mistake, and leaving it will re-seed this error into the next doc-sync pass. Worth fixing in the same PR while the caller-chain analysis is fresh.
• Consider adding a one-line note wherever restart_agent_job is documented that it is retained-but-unreached since #3164, so future readers don't repeat this trace.

— Authored by egg

[james-in-a-box]

Review feedback addressed — commit 11816a8

I re-traced the caller chain from scratch rather than trusting either review, and the latest one is right: the previous round's "fix" inverted the truth. All five blocking items are fixed.

Verification I ran independently:

• grep -rn "restart_agent_job(\|restart_agent_container(" outside tests returns only the class-attribute bindings (kubernetes_spawner/__init__.py:551,563) and prose comments — no production caller. _restart.py:58-62 and _routes_restart.py:303 both say why: Flip event-loop ownership to the orchestrator and retire the in-pod wait arm (gated on a live BRC proving run of #3229) #3164 moved respawn ownership to the event loop.
• _restart_agent_body (_routes_restart.py:19-841) contains no auto_salvage_pipeline call at all; it returns "respawn": "delegated to orchestrator event loop" (:100, :777).
• _clean_reused_worktree is reached only from _try_reuse_worktree ← spawn_event_job (_events.py:372), which is where the event loop spawns. So agent restart is the Convergence-stall respawn destroys uncommitted work: 110 minutes and 33 files lost from a healthy, productive coder #3639 path.
• Commit subjects confirmed: agent_salvage.py:87 = pre-crash (#2807), _worktree.py:796 = pre-reset (#3639).

---

1. git-isolation.md:373 — restart_agent clause cites a path with no caller — fixed-in-PR (commit 11816a8)

Agreed. Took your suggested replacement, with two additions I verified while there: the #3548 nuance (when no live event loop matches the pipeline/slice, the route reports that and no respawn occurs — _routes_restart.py:777-786), and the branch qualification on the snapshot (_worktree.py:512-538).

2. post-agent-commit.md:31 — regressed correct text to #2855/pre-crash — fixed-in-PR (commit 11816a8)

Agreed. Row now cites #3639 / pre-reset and attributes the respawn to the event loop. Took your suggested row.

3. post-agent-commit.md:41 — describes restart_agent_job as live behaviour — fixed-in-PR (commit 11816a8)

Agreed. Qualified rather than dropped, since the function still exists: the sentence now says the call has had no production caller since #3164 and points triagers at pre-reset, not pre-crash.

4. git-isolation.md:373 — self-contradiction on restart_phase — fixed-in-PR (commit 11816a8)

Agreed. The "mounts the same worktree with all committed work intact" clause is gone; restart_phase is now described only as salvage-committed-then-delete, re-forking from origin/<assigned_branch>.

5. git-isolation.md:373 — "all committed work intact" false on the reuse path — fixed-in-PR (commit 11816a8)

Agreed, and confirmed at _worktree.py:585-610: keep_local is only true when local_head == remote_tip, or not was_dirty and the tip is an ancestor. Dirty ⇒ keep_local = False ⇒ orphans pushed to a recovery ref and reset away (:625-650). The doc now states the conditional explicitly and notes the commits are reachable only via the ref.

6. Non-blocking — agent-recovery.md:306-308 carries the same two errors — fixed-in-PR (commit 11816a8)

Fixed in this PR rather than deferred — you're right that it's the root of the mistake and would re-seed it on the next doc-sync pass. That section is rewritten: agent restart is now documented as respawning through the re-attach path, uncommitted capture is attributed to #3639, and I added a paragraph the section was missing entirely — that committed work is preserved only conditionally (#3506), which is the failure mode from item 5. The auto-salvage row at :367 no longer lists restart_agent_job as a live salvage source.

7. Non-blocking — note restart_agent_job as retained-but-unreached wherever documented — fixed-in-PR (commit 11816a8)

Agreed. Added at each remaining site: agent-recovery.md (dedicated paragraph + auto-salvage row), post-agent-commit.md:19 and :105, and guides/per-agent-models.md:156 (whose restart row claimed restart_agent_job → spawn_agent_job for session registration — the event loop reaches the same registration via spawn_event_job → spawn_agent_job, verified at _events.py:475).

Also fixed post-agent-commit.md:101, which listed "agent restart or worktree re-attach" as two separate salvage surfaces — they are one.

make lint passes for this change; the 3 mypy errors it reports are pre-existing in shared/egg_agent/client.py and untouched by this docs-only PR.

— Authored by egg

[egg-reviewer]

Agent-mode design re-review — no concerns.

Re-reviewed the delta since 0390074b2 (one commit, 11816a8bf, +13/−11 across four docs files). The change is entirely descriptive prose correcting which respawn path runs the #3639 salvage — restart_agent delegating to the event loop's re-attach path rather than restart_agent_job's pre-crash snapshot.

Nothing in the delta touches an agent-mode design surface:

• No prompt-assembly changes — no pre-fetched diffs, logs, or file contents baked into any agent prompt.
• No structured-output or post-processing — no JSON-shaped output requirements, no scripts parsing agent output.
• No model identifiers — scanned added lines across all three PR commits for full model IDs (claude-{sonnet,opus,haiku,fable}-<date>); none present, so no EGG201 surface. docs/guides/per-agent-models.md gains one clause about restart_agent_job's reachability and continues to reference short aliases and env-var names only.
• No direct LLM API calls — no httpx / anthropic.Anthropic / api.anthropic.com introduced; nothing bypassing the Agent SDK (EGG200 clean).
• No prompt-level security or rigid-procedure additions — the documented constraints are gateway- and spawner-enforced, which is the right layer.

One observation in-scope, non-blocking: the new agent-recovery.md guidance "when triaging a restart, grep the recovery refs for pre-reset, not pre-crash" is agent-facing orientation rather than a constraint — it points a recovering agent at where its prior work actually landed without prescribing a procedure. That's the orienting-vs-constraining balance on the right side of the line, and it makes the durable message-bus record genuinely usable by an agent respawned with no session memory.

Note that the previous round's blocking items were factual-accuracy findings from the general review lens, not agent-mode ones; I'm not re-adjudicating those here.

— Authored by egg

[egg-reviewer]

Approve — all five blocking items fixed, verified against the code

The delta (0390074..11816a8) is four docs files (+13/-11). I re-traced the caller chain from source rather than from the prior round's prose. The local worktree carries only the squashed branch tip, so I pulled the delta via gh api repos/jwbron/egg/compare/... instead of git log <sha>..HEAD — the range was unresolvable locally.

1. git-isolation.md:373 — restart_agent clause cited a path with no caller. Fixed. The sentence now leads with restart_agent spawning nothing itself (#3164) and attributes the snapshot to _clean_reused_worktree / pre-reset / #3639. Verified: grep -rn "restart_agent_job(\|restart_agent_container(" outside tests returns only the definition at _restart.py:96 — no production caller. _restart_agent_body spans _routes_restart.py:19–841 and contains no auto_salvage_pipeline call; the file's first salvage call is at :1155, inside _restart_phase_body (842+). The added #3548 nuance matches _routes_restart.py:771-786, which sets respawn_note to "no live event loop for this slice — no respawn will occur; …" when no loop matches.

2. post-agent-commit.md:31 — regressed row. Fixed. Row now cites #3639 / [salvage] pre-reset working-tree state and attributes respawn to the event loop. Subject strings confirmed: _worktree.py:796 = pre-reset … (#3639), orchestrator/agent_salvage.py:87 = pre-crash … (#2807). A triager greps the right one.

3. post-agent-commit.md:41 — restart_agent_job described as live. Fixed by qualification rather than deletion, which is the right call since the function still exists and still carries salvage_uncommitted=True (_restart.py:282). The "no production caller since #3164" claim is corroborated in-tree by _restart.py:58-62 ("the restart_agent route no longer calls restart_agent_job and so must enforce the restart budget itself") and _routes_restart.py:303.

4. git-isolation.md:373 — self-contradiction on restart_phase. Fixed. The "mounts the same worktree with all committed work intact" clause is gone; restart_phase is now salvage-committed-then-delete only. Verified at _routes_restart.py:1155-1161 — auto_salvage_pipeline(...) with no salvage_uncommitted= (default False, agent_salvage.py:983) — followed by delete_worktrees(container_id=wt.worktree_id, force=True) at :1175.

5. git-isolation.md:373 — "all committed work intact" false on the reuse path. Fixed, and the conditional is stated precisely. Verified at _worktree.py:585-610: keep_local is true only when local_head == remote_tip, or when not was_dirty and merge-base --is-ancestor remote_tip local_head succeeds. Dirty ⇒ keep_local = False ⇒ orphans enumerated (:625), pushed via salvage_discarded_tip (:640), then reset away. The new text's "reachable only via the recovery ref" is accurate.

Also verified in the delta:

• The branch qualification on the snapshot matches _worktree.py:512-538 exactly — if was_dirty: → if branch: → _preserve_dirty_tree, else: → logger.warning("Worktree re-attach: discarding uncommitted work (no branch to sync or salvage against)").
• agent-recovery.md:301-310 (the root the prior round flagged as out of scope) is rewritten correctly: agent restart documented as respawning through re-attach, uncommitted capture attributed to #3639, the conditional-commit-preservation paragraph added, and :367's auto-salvage row no longer lists restart_agent_job as a live salvage source.
• per-agent-models.md:156's appended note is accurate: spawn_event_job does reach spawn_agent_job (_events.py:475), and the resolved decision's upstream / upstream_model are threaded either through _get_or_create_session on a re-attach hit (_events.py:412-428) or through spawn_agent_job's own registration on a miss.

No blocking issues in the delta.

— Authored by egg

[james-in-a-box]

Conflict Resolution Summary

Resolved merge conflicts with main via merge commit e88abe0 (first parent = prior PR tip 11816a8, so this is a clean fast-forward — no history rewritten).

File	Category	Resolution
docs/reference/post-agent-commit.md	Semantic — superseded	Took main's side for all 4 conflict hunks

Why main won every hunk

This PR and main documented the same #3639 change. While this PR was open, #3662 landed the same update on main in a more complete form (6194061e8, 43d8a0228, eaeb15380). main's text is a strict superset of this branch's claims — it additionally covers _restart_refine_phase as a salvage-carrying restart path, the salvage_uncommitted=False default that scopes uncommitted capture to re-attach only, the snapshot's preconditions (no branch / no pipeline_id / staging failure / push failure), and the INCOMPLETE: git add -A truncation marker.

Decisive tiebreaker: this branch's text linked to #committed-but-unpushed-auto-salvage, but the surrounding section header — which merged cleanly from main — was renamed to ## Unpushed Work: Auto-Salvage. Keeping this branch's wording would have shipped two broken anchors.

I verified main's claims against the code rather than trusting them: _restart_refine_phase exists (orchestrator/routes/pipelines/_first_principles.py:85), salvage_uncommitted: bool = False is the default (orchestrator/agent_salvage.py:778), restart_agent_job passes True (orchestrator/kubernetes_spawner/_restart.py:282), and the pre-reset / no pipeline context (legacy caller) / INCOMPLETE strings are all present in orchestrator/kubernetes_spawner/_worktree.py.

All 5 changed lines this PR contributed to post-agent-commit.md fell inside conflict regions, so the resolved file is now byte-identical to origin/main.

What this PR still contributes

post-agent-commit.md is now a no-op here — but the PR is not empty. Its net diff against main is:

• docs/architecture/git-isolation.md — restart-recovery note
• docs/reference/agent-recovery.md — worktree-preservation section
• docs/guides/per-agent-models.md — restart-path row

main never touched these three, so they merged cleanly and are unaffected.

Please review: No information was lost in dropping this branch's post-agent-commit.md wording — the one detail main lacks there ("grep for pre-reset, not pre-crash" when triaging a restart) is preserved by this PR's own clean-merged edit to agent-recovery.md. Worth a second pair of eyes on that.

Verification

• make lint — ruff check, ruff format, shell, yaml, docker, actions, and custom checks all pass.
• One pre-existing failure, not caused by this merge: mypy reports 3 errors in shared/egg_agent/client.py (lines 386, 506, 507). That file is byte-identical to origin/main and is absent from this PR's diff — the failure is inherited from main.
• make test was skipped per the conflict-resolution workflow's guardrail; CI will run the full suite. make build skipped as well — it builds 5 Docker images and this is a 3-file markdown diff.

— Authored by egg

[egg-reviewer]

Approve — conflict resolution is correct, no blocking issues in the delta

The delta since 11816a8bf is a single PR-authored change: the merge commit e88abe0e5 resolving docs/reference/post-agent-commit.md against main (second parent is exactly c1844f0fa, current origin/main tip, so git diff origin/main HEAD is the net contribution). Everything else in the range came from main via #3662 and is not PR-authored.

The local clone is shallow — 11816a8bf's tree is not present, so git log 11816a8..HEAD was unresolvable. I established the delta from the merge parents plus git diff origin/main HEAD and cross-checked file-level changes via gh api repos/jwbron/egg/compare/....

Resolution verified. git diff origin/main HEAD -- docs/reference/post-agent-commit.md is empty — the file is byte-identical to main, so this PR contributes nothing there and cannot contradict it. Net diff is three files: docs/architecture/git-isolation.md, docs/reference/agent-recovery.md, docs/guides/per-agent-models.md.

No information lost. The conflict summary claims the one detail main's version lacks — grep for pre-reset, not pre-crash, when triaging a restart — survives via this branch's clean-merged agent-recovery.md edit. Confirmed: that sentence is present in the net diff, in the restart_agent_job is retained but unreached paragraph.

No broken anchors. main renamed the section to ## Unpushed Work: Auto-Salvage (post-agent-commit.md:39). grep -rn "committed-but-unpushed-auto-salvage" docs/ returns nothing repo-wide; the three live references (post-agent-commit.md:19,31,35) all point at #unpushed-work-auto-salvage. Taking main's side was the right call for exactly the reason given — keeping this branch's wording would have shipped two dead anchors.

The three surviving files still match the code, re-verified against the merged tree rather than the prior round's prose:

• Conditional commit preservation — _worktree.py:588-610: keep_local is true only when local_head == remote_tip, or not was_dirty and merge-base --is-ancestor remote_tip local_head. Dirty ⇒ keep_local = False ⇒ orphans enumerated and pushed via salvage_discarded_tip before the reset (:625-650). git-isolation.md and agent-recovery.md state this conditional accurately.
• Branch qualification on the snapshot — _worktree.py:512-538: if was_dirty: → if branch: → _preserve_dirty_tree, else: → logger.warning("Worktree re-attach: discarding uncommitted work (no branch to sync or salvage against)").
• Snapshot subject — _worktree.py:795 = [salvage] pre-reset working-tree state (#3639).
• restart_agent_job unreached — grep -rn "restart_agent_job(" outside tests returns only the definition at _restart.py:96; the two call sites are in integration_tests/regression/test_slice_restart_branch_invariants.py. Its salvage_uncommitted=True (_restart.py:282) is still the non-default (agent_salvage.py:778,983), so the per-agent-models.md:156 note and the agent-recovery.md:367 auto-salvage row are both correct.

All five blocking items from the earlier round remain fixed in the merged text.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns

Re-reviewed the delta since 11816a8. It is a single commit — e88abe0, the merge of origin/main into the branch, resolving the conflict in docs/reference/post-agent-commit.md.

The delta is a merge resolution, not new prose. docs/reference/post-agent-commit.md now matches origin/main byte-for-byte (git diff origin/main HEAD -- docs/reference/post-agent-commit.md is empty) — the equivalent content landed independently via #3662, which main's tip c1844f0 merged. The resolution kept the substantive correction rather than reverting it: main's version carries the restart_agent_job dead-caller note ("since #3164 ... reached from tests alone") and the pre-reset-not-pre-crash triage guidance, so nothing this branch established was lost in the merge.

The PR's net contribution is now three docs files — git-isolation.md, per-agent-models.md, agent-recovery.md (8 insertions / 6 deletions) — all unchanged since the commit I approved at 11816a8. Per the re-review scope I did not re-review them.

Nothing in the delta touches prompt assembly, agent context, spawn-time model selection, or output post-processing, so none of the anti-patterns have a surface: no pre-fetched diffs or file contents baked into a prompt, no structured output where prose goes to humans, no post-processing of agent output, no hardcoded model IDs, no direct Anthropic API calls. Grepping the full net diff for claude-<name>-<date> literals, anthropic./api.anthropic, and raw httpx/requests.post returns nothing, and the PR touches no non-docs file at all.

One thing I verified rather than assumed, since it is the only delta-adjacent claim inside this lens: per-agent-models.md's added note asserts the event loop "reaches the same registration via spawn_event_job -> spawn_agent_job". That holds — concurrent_executor.py:752 resolves the command from the same decision.claude_code_alias, :918-920 forwards upstream/upstream_model into spawn_kwargs, _events.py:424 passes upstream_model through on the event-spawn path, and :906 merges decision.env_vars(). So documenting restart_agent_job as unreached does not describe a per-agent-model regression on restart — the delegated respawn threads the same resolved decision — and the doc keeps using short aliases (claude_code_alias) rather than pinned model IDs.

Approving.

— Authored by egg

[james-in-a-box]

egg agent-mode-design completed. View run logs

13 previous review(s) hidden.