Preserve a re-attached worktree's uncommitted work before the R6 reset (#3639)

docs/architecture/on-demand-agent-lifecycle.md

@@ -183,7 +183,76 @@ Under orchestrator ownership the worktree becomes a hot path.
    resuming agent with no session memory can find and resume its prior
    work instead of silently re-deriving it (falls back to log-only when
    `pipeline_id` context is unavailable, or record-only when the salvage
-   push itself fails).
+   push itself fails). **Uncommitted work is snapshotted first**
+   ([#3639](https://github.com/jwbron/egg/issues/3639)): a dirty tree is
+   committed (`git add -A` plus a `[salvage] pre-reset working-tree state`
+   commit) *before* the `reset --hard`, so it becomes an ordinary orphan
+   that the salvage + record path above recovers. Without that step a
+   session that worked for hours without committing had nothing for the
+   orphan detector to find and lost everything on a routine respawn. The
+   snapshot does not relax the residue policy: the tree still hard-resets
+   to the origin tip, so the successor inherits nothing uncommitted; it
+   only makes the discarded state recoverable. Ignored files are excluded,
+   and a failed snapshot logs at WARNING with the file count and proceeds
+   with the reset rather than blocking reuse. The snapshot is skipped
+   entirely when the re-attach carries no `branch`: with no origin tip to
+   reset to and no salvage target, the commit would simply become the
+   successor's HEAD — un-vetted residue promoted to committed state, which
+   is what R6 exists to prevent. When the salvage push fails the bus
+   record says the snapshot was *not* pushed and asks for escalation
+   rather than reassuring the successor that nothing was lost. The ask
+   also scales with *what* the snapshot captured: when every captured path
+   is a **state file the next event regenerates and some other store
+   durably holds**, the record softens to "read it if you need it" so a
+   routine respawn does not train the #3509 message into background noise.
+   The membership test is regeneration, not authorship — the dominant
+   member is written by the *sandbox* on the agent's own tool call and
+   holds agent-authored prose. The allowlist is
+   `.egg-state/agent-outputs/*/brc-memory*.md` (rewritten by
+   `sandbox/egg_agent_tools/handlers/brc_memory.py` on every
+   `brc_ack`/`brc_nack`, with the orchestrator message history as the
+   durable backstop — see
+   [brc-memory.md](brc-memory.md)),
+   `.egg-state/agent-outputs/consensus-confirmed`, and
+   `.egg-state/agent-outputs/<pipeline-id>-apply-handoff.json`;
+   matching is segment-wise so `*` does not cross `/`. Agent *output* in
+   the same directory — `<pipeline>-wontdo.json`,
+   `<identifier>-tester-output.json` — is deliberately excluded: nothing
+   rewrites it on the next event and no other store holds it, so losing it
+   warrants the imperative. Anything else —
+   including an unrecognised or unknown file set — keeps the imperative
+   "inspect it before starting work", as does a snapshot flagged partial
+   (a truncated capture's path list omits whatever failed to stage, so it
+   cannot establish that the snapshot holds nothing but state files). The
+   bus record's metadata carries the inputs *and* the outcome as separate
+   fields — `wip_paths` (capped), `wip_partial`,
+   `wip_machine_state_only` (the path predicate alone) and `wip_softened`
+   (whether the body actually softened) — so a triage consumer can
+   reconstruct the decision instead of regexing the prose; the two derived
+   fields diverge whenever a machine-state-only path set is disqualified
+   by a commit stack, a truncated capture, or a failed salvage push. The
+   threshold selects wording only; the snapshot itself is always taken —
+   including when the path list cannot be read at all. The staged-path
+   read uses `-z` so `wip_paths` carries real bytes rather than
+   `core.quotePath` C-quoted tokens, which means a filename that is not
+   valid UTF-8 would be undecodable under `subprocess`'s strict `text=True`
+   decode; the read passes `errors="replace"`, so one bad name costs one
+   name (a U+FFFD in `wip_paths`) rather than the whole path set. That
+   replacement does not move the softening decision in either direction:
+   every non-`*` character in the softening globs is ASCII and replacement
+   only substitutes non-ASCII for non-ASCII, so a replaced path matches
+   exactly the globs its raw bytes would. Anything that still defeats that read — a
+   timeout on a large staged set, a non-zero `diff` against a locked index
+   — logs a WARNING and commits blind (`wip_paths`/`wip_files` become
+   `null`, so the record takes the imperative) rather than letting a
+   metadata read cost the working tree. A
+   snapshot whose `git add -A` did not complete cleanly is marked incomplete in
+   both its commit message and the bus record, since a truncated snapshot
+   is otherwise indistinguishable downstream from a complete one. The same
+   marker rides the #2807 crash-salvage commit
+   (`commit_working_tree`), which pushes to `egg/recovered/…` with no bus
+   record at all — there the commit message is the only channel a triager
+   ever sees.
 3. **Before handing off to spawn:** translate the validated paths from
    orchestrator-local (under `WORKTREE_BASE_DIR`) to host paths, matching
    what the create path already gets from the gateway. An untranslated

docs/reference/agent-recovery.md

@@ -305,6 +305,8 @@ Agent restart preserves the agent's git worktree, including any committed work o
 
 **Uncommitted work is also captured before respawn** ([#2807](https://github.com/jwbron/egg/issues/2807)): auto-salvage runs with `salvage_uncommitted=True`, which stages and commits the agent's dirty working tree (using identity `egg-salvage <egg-salvage@localhost>` and commit message `[salvage] pre-crash working-tree state (#2807)`) before pushing to `egg/recovered/…`. By the time the gateway's subsequent `git reset --hard` runs during worktree reuse, that state has already been committed and pushed — so the reset only abandons the synthetic salvage commit locally, and it remains recoverable via the pushed `egg/recovered/…` ref.
 
+The **event-loop respawn** path (worktree re-attach, not operator restart) gets the same protection from [#3639](https://github.com/jwbron/egg/issues/3639): `_clean_reused_worktree` commits a dirty tree as `[salvage] pre-reset working-tree state (#3639)` (same identity, so one `[salvage]` grep finds either snapshot) before its `reset --hard`, which feeds the existing [#3509](https://github.com/jwbron/egg/issues/3509) recovery-ref push and bus record. Until that landed, this path was the one gap in the preservation story: it salvaged commits only, so a session that worked for hours without committing lost the entire working tree on the next respawn, logged at INFO as `cleaned and synced`.
+
 **Implementation detail:** `spawn_agent_container()` always calls the gateway to create (or reuse) the per-agent worktree when `repos` is provided, regardless of whether `repo_volumes` was passed by the caller. This ensures both the initial spawn path and the restart path (which does not pass `repo_volumes`) correctly mount the agent's worktree. See issue [#1597](https://github.com/jwbron/egg/issues/1597) for the fix that resolved a bug where the restart path skipped worktree creation.
 
 ## Phase-Level Restart
@@ -360,7 +362,7 @@ When an agent's pushes to its assigned branch are wedged — gateway branch-allo
 | **API (read)** | `GET /api/v1/pipelines/{id}/local-commits[?agent_role=&slice_id=]` — list unpushed commits per worktree (read-only) |
 | **API (write)** | `POST /api/v1/pipelines/{id}/salvage[?agent_role=&slice_id=]` — push HEAD to `egg/recovered/...` |
 | **MCP tool** | `list_agent_local_commits(task_id, agent_role?, slice_id?)` and `salvage_agent_commits(task_id, agent_role?, slice_id?)` |
-| **Auto-salvage** | Best-effort, automatic — runs from `kubernetes_spawner.cleanup_pipeline` (skipped when `preserve_worktrees=True`, since the worktree survives and there's nothing to mirror), from `restart_phase` (always runs against the worktrees of the roles being restarted), from **agent restart** (`restart_agent_job`, [#2807](https://github.com/jwbron/egg/issues/2807)) with `salvage_uncommitted=True` — which also commits the dirty working tree onto the work branch before the recovery push, so uncommitted edits survive the respawn's `git reset --hard` — and from **worktree re-attach** (`_clean_reused_worktree`'s dirty-discard reset, [#3509](https://github.com/jwbron/egg/issues/3509)): `salvage_discarded_tip` pushes the doomed HEAD to a recovery ref *before* the hard-reset runs (the tip is otherwise unreachable to every other salvage path once the reset moves the worktree branch), and a message-bus system message durably records the discarded tip + recovery ref so a resuming agent with no session memory can find its prior work. Requires `pipeline_id` (plus `agent_role`/`slice_id` for ref scoping) and the pipeline's real gateway `mode` — omitting `mode` risks a "public" push being denied on a private-mode pipeline, silently degrading to record-only. Failures are logged and never block cleanup or restart |
+| **Auto-salvage** | Best-effort, automatic — runs from `kubernetes_spawner.cleanup_pipeline` (skipped when `preserve_worktrees=True`, since the worktree survives and there's nothing to mirror), from `restart_phase` (always runs against the worktrees of the roles being restarted), from **agent restart** (`restart_agent_job`, [#2807](https://github.com/jwbron/egg/issues/2807)) with `salvage_uncommitted=True` — which also commits the dirty working tree onto the work branch before the recovery push, so uncommitted edits survive the respawn's `git reset --hard` — and from **worktree re-attach** (`_clean_reused_worktree`'s dirty-discard reset, [#3509](https://github.com/jwbron/egg/issues/3509)): a dirty tree is first committed as a `[salvage] pre-reset working-tree state (#3639)` snapshot so uncommitted work is salvageable at all ([#3639](https://github.com/jwbron/egg/issues/3639)), then `salvage_discarded_tip` pushes the doomed HEAD to a recovery ref *before* the hard-reset runs (the tip is otherwise unreachable to every other salvage path once the reset moves the worktree branch), and a message-bus system message durably records the discarded tip + recovery ref so a resuming agent with no session memory can find its prior work. Requires `pipeline_id` (plus `agent_role`/`slice_id` for ref scoping) and the pipeline's real gateway `mode` — omitting `mode` risks a "public" push being denied on a private-mode pipeline, silently degrading to record-only. Failures are logged and never block cleanup or restart |
 
 ### Recovery Workflow
 
@@ -378,6 +380,17 @@ git switch <target-branch>
 git cherry-pick <recovered-base>..recovered/<scope>
 ```
 
+**Review before replaying: a recovery ref may hold un-reviewed working-tree residue.** Both working-tree snapshot paths ([#2807](https://github.com/jwbron/egg/issues/2807) restart, [#3639](https://github.com/jwbron/egg/issues/3639) re-attach) stage with `git add -A`, so a snapshot commit contains everything the agent left in the worktree that is not `.gitignore`d — scratch dumps, logs, stray state files — with no agent or human intent behind any of it. Recovery refs are a preservation mechanism, not an endorsement: read the diff before cherry-picking, and expect a snapshot-only ref to sometimes hold nothing you want. **A snapshot may also be incomplete**: when `git add -A` did not complete cleanly the commit holds only what reached the index, and its message carries an ``INCOMPLETE: `git add -A` did not complete cleanly while staging`` paragraph saying so — `git log -1 <ref>` before you conclude the ref holds everything the working tree did. On the [#3639](https://github.com/jwbron/egg/issues/3639) re-attach path a bus record repeats that warning, but on the [#2807](https://github.com/jwbron/egg/issues/2807) crash-salvage path (`commit_working_tree`) there is no bus record at all, so the commit message is the only channel a triager gets. To find truncated snapshots from either path, fetch the recovery namespace first — both paths push to `egg/recovered/*` on **origin**, so a fresh clone has no local ref for them and `git log --all` alone would report zero:
+
+```bash
+git fetch origin 'refs/heads/egg/recovered/*:refs/remotes/origin/egg/recovered/*'
+git log --all --grep 'INCOMPLETE: `git add -A`'
+```
+
+The grep token — the leading `INCOMPLETE:` plus the backticked `git add -A` — is byte-identical in both paths' suffixes (`_WIP_COMMIT_PARTIAL_SUFFIX`, `_UNCOMMITTED_SALVAGE_PARTIAL_SUFFIX`) and is pinned against this file by `test_partial_suffixes_share_one_grep_token`. Keep the backticks: they are part of the commit message, so a pattern without them matches nothing and the zero results read as "no truncated snapshots".
+
+Reading the diff before replaying is the control here: **nothing in this repo enables GitHub push protection**, so do not assume a snapshot containing a secret is stopped on the way out. (Where push protection *is* enabled on the receiving repo it rejects such a push rather than leaking it, and the discard is then recorded with `salvage_error` set instead of a recovery ref — the failure branch described above.)
+
 Operators may delete `egg/recovered/*` refs manually after replay (`git push origin --delete <ref>`). For automatic cleanup of refs left behind by replays that never came, see [Recovery Ref Cleanup](#recovery-ref-cleanup) below.
 
 ### Recovery Ref Cleanup