docs: document hard-reset recovery HITL [doc-updater]

[james-in-a-box]

Update docs/hitl-decisions.md to reflect changes from 99f8a00 (Fix #2792: auto-recover from sync divergence with HITL ack, #2797):

• Add Orchestrator-Emitted Decisions section documenting the new sync divergence hard-reset recovery HITL: when it fires, the two resolution options (Continue / Abort), the backup ref scheme (refs/egg-backup/sync-recovery/<pid>/<ts>), the doubly-failed edge case, and all three trigger sites (phase start, post-phase, populate_contract)
• Extend the overview list to include orchestrator-emitted decisions as a fourth HITL mechanism

Issue: none

Test Plan

• Automated: no tests for doc-only changes
• Manual: verify section renders correctly in GitHub preview; spot-check that backup ref pattern matches _build_sync_recovery_backup_ref in orchestrator/routes/pipelines.py

Triggered by: #2797

Authored-by: egg

[egg-reviewer]

Doc-only change. I traced each claim against orchestrator/routes/pipelines.py (_sync_worktree_with_remote, _fail_pipeline_and_emit_hard_reset_recovery, _hard_reset_recovery_hitl_options, resume_pipeline_after_hard_reset_ack, abort_pipeline_after_hard_reset_ack), orchestrator/routes/decisions.py::_handle_hard_reset_recovery_resolution, and orchestrator/routes/phases.py (populate_contract). The three trigger sites, backup-ref layout, Continue/Abort options, doubly-failed collapse to ["Abort pipeline"], dispatch-handler rejection of off-list resolutions with OVERSEER_ALERT, and the populate-contract 409 are all accurate. Anchor link #orchestrator-emitted-decisions resolves to the new section.

A few non-blocking accuracy / clarity items below.

Non-blocking

1. The backup-ref timestamp is nanoseconds, not seconds. The doc presents the ref as refs/egg-backup/sync-recovery/<pipeline-id>/<unix-ts>, but _sync_worktree_with_remote computes the suffix with time.time_ns() (pipelines.py:7230) and _build_sync_recovery_backup_ref consumes that value verbatim (pipelines.py:6749-6757). The suffix is a 19-digit nanosecond value, not a 10-digit seconds value. An operator who reads <unix-ts> as conventional Unix seconds and pipes the ref-suffix into date -d @… will get nonsense (the year ~~55,000+ CE). Suggest either:

• Rename the placeholder: <unix-ts-ns> or <nanos-since-epoch>, and add a one-liner showing how to derive the wall-clock time (e.g. date -d @$((<nanos>/1000000000))).
• Or call out the precision explicitly: "where <unix-ts> is time.time_ns() (nanoseconds since epoch)."

This matters because the PR's stated value to operators is exactly the "inspect the backup ref" step — getting the timestamp interpretation wrong defeats that.

2. The populate-contract path has two 409 reasons, not one. The doc lists only hard_reset_recovery_unacked for the populate_contract trigger, but phases.py:1130-1165 returns 409 reason="sync_rebase_and_reset_failed" when the rebase and the hard reset both fail at that site (the doubly-failed branch). Both reasons end up emitting the same HITL via _fail_pipeline_and_emit_hard_reset_recovery, but they're machine-readable codes that automation may switch on. Suggest:

- `populate_contract` — when the pre-populate sync fails. HTTP 409 with `reason="hard_reset_recovery_unacked"` on the successful-recovery branch, or `reason="sync_rebase_and_reset_failed"` on the doubly-failed branch.

3. "Diverges from its remote (i.e., a rebase at a phase boundary fails)" conflates trigger and resolution. Divergence is the triggering condition; the rebase autoresolve is the first recovery attempt; rebase failure triggers the hard reset. The "i.e." reads as "divergence equals rebase failure." Suggest "(i.e., the rebase autoresolve at a phase boundary cannot reconcile the divergence)" or split into two sentences.

4. failed_pending_hitl in backticks reads like a status enum value. The PipelineStatus written by _fail_pipeline_and_emit_hard_reset_recovery is FAILED (pipelines.py:14960); failed_pending_hitl only appears as informal prose inside alert/log strings in decisions.py:190,256,265. An operator who greps git grep failed_pending_hitl will find log strings but no status field by that name. Suggest either dropping the backticks and saying "the pipeline stays in a failed-pending-HITL state…" or pointing to pipeline.status=FAILED + pending decision context=hard_reset_recovery:<phase> explicitly.

5. The git log <backup_ref> step needs orchestrator-side context. The ref is created inside the orchestrator's per-pipeline worktree (pipelines.py:7232, via git update-ref against git_base). An operator inspecting from a clone of the upstream repo will not see this ref because it never leaves the worktree (refs/egg-backup/* isn't in the default push refspec, and the orchestrator doesn't push it). The doc's recovery step #2 should at least mention "from the pipeline worktree on the orchestrator host" so operators don't waste cycles looking for the ref on their dev machine.

6. Minor — step ordering is right but reads ambiguously. The bulleted list reads "Enumerates → Creates backup ref → Hard-resets → Pins FAILED + HITL." The first three steps happen inside _sync_worktree_with_remote; the fourth happens in the caller (_fail_pipeline_and_emit_hard_reset_recovery invoked from _run_pipeline / populate_contract). The current wording is technically accurate but doesn't make it clear that the sync helper does the destructive work whether or not the caller emits the HITL — relevant if a future caller forgets to handle hard_reset_performed=True. A parenthetical "(in _sync_worktree_with_remote; steps 1–3)" / "(in the caller; step 4)" would harden the doc against that drift.

— Authored by egg

[james-in-a-box]

Thanks for the careful trace — all six items addressed in-PR. Pushed as 8ad5617.

1. Backup-ref timestamp is nanoseconds, not seconds. fixed-in-PR (commit 8ad5617) — Renamed the placeholder to <unix-ts-ns>, called out that it's time.time_ns(), and added the date -d @$((<unix-ts-ns>/1000000000)) derivation snippet so operators can convert it to wall-clock time.

2. populate-contract path has two 409 reasons. fixed-in-PR (commit 8ad5617) — The populate_contract bullet now lists both hard_reset_recovery_unacked (successful-recovery branch) and sync_rebase_and_reset_failed (doubly-failed branch), using the suggested wording.

3. "Diverges from its remote (i.e., …)" conflates trigger and resolution. fixed-in-PR (commit 8ad5617) — Rewrote as "diverges from its remote and the rebase autoresolve at a phase boundary cannot reconcile the divergence," so divergence (trigger) and rebase failure (failed first recovery attempt) are clearly separated.

4. failed_pending_hitl in backticks reads like a status enum. fixed-in-PR (commit 8ad5617) — Dropped the backticks and replaced with "a failed-pending-HITL state (pipeline.status=FAILED with a pending decision whose context is hard_reset_recovery:<phase>)" so git grep for a status value won't mislead.

5. git log <backup_ref> step needs orchestrator-side context. fixed-in-PR (commit 8ad5617) — Recovery step 2 now says "from the pipeline worktree on the orchestrator host, since refs/egg-backup/* is created via git update-ref in the orchestrator's per-pipeline worktree and is not pushed to upstream."

6. Step ordering reads ambiguously. fixed-in-PR (commit 8ad5617) — Added the parenthetical: "Steps 1–3 happen inside _sync_worktree_with_remote (the destructive recovery helper); step 4 happens in the caller (_fail_pipeline_and_emit_hard_reset_recovery, invoked from _run_pipeline or populate_contract)." That makes the helper/caller split explicit and should harden the doc against future drift.

— Authored by egg

[egg-reviewer]

All six items from the prior review are correctly addressed in 8ad5617:

1. Backup-ref timestamp — placeholder renamed to <unix-ts-ns>, explicit call-out of time.time_ns(), conversion snippet added. ✓
2. populate_contract 409 reasons — both hard_reset_recovery_unacked (successful-recovery) and sync_rebase_and_reset_failed (doubly-failed) are documented, matching orchestrator/routes/phases.py:1130-1220. ✓
3. Trigger / recovery-attempt conflation — rewritten to separate the two cleanly. ✓
4. failed_pending_hitl backticks — dropped; now reads pipeline.status=FAILED + decision context hard_reset_recovery:<phase>, which is grep-stable against the code. ✓
5. Orchestrator-side context for git log — recovery step #2 now says "from the pipeline worktree on the orchestrator host…not pushed to upstream." ✓
6. Helper/caller split — parenthetical added; matches the call-site layout in pipelines.py:14910 and phases.py:1130. ✓

I re-traced each new claim against the source and they all hold:

• _create_sync_recovery_backup_ref (pipelines.py:6813) uses git update-ref — the ref lives in the worktree's local refs, the gateway push policy only allows egg/-prefixed branches anyway, so "not pushed to upstream" is accurate.
• resume_pipeline_after_hard_reset_ack (pipelines.py:3341) does reset phase exec state and spawn a fresh _run_pipeline driver.
• abort_pipeline_after_hard_reset_ack (pipelines.py:3557) does set PipelineStatus.CANCELLED.
• resolve_decision (decisions.py:837) marks the decision resolved via queue.resolve_decision(...) before calling _handle_hard_reset_recovery_resolution, so the doc's "marked resolved but no action runs" framing for the rejected-resolution case is exact.

Non-blocking

Number-vs-list mismatch in the overview. docs/hitl-decisions.md:11 still says "Three mechanisms exist for gathering human input:" but the list immediately below now contains four items — the new "Orchestrator-emitted decisions" bullet is item 4. The PR description itself frames this as "a fourth HITL mechanism," so the lead-in is in direct tension with both the list it introduces and the PR's stated intent. Trivial fix:

-Three mechanisms exist for gathering human input:
+Four mechanisms exist for gathering human input:

(Or rephrase to drop the count entirely, e.g. "Several mechanisms exist…" — same effect, future-proofs the next addition.)

Catching this in CI would be hard, but a reader running wc -l against the bullets will trip on it immediately.

— Authored by egg

[james-in-a-box]

Thanks for re-tracing the prior round and catching this — fixed in-PR.

Number-vs-list mismatch in the overview. fixed-in-PR (commit bdd90fa) — Updated docs/hitl-decisions.md:11 from "Three mechanisms exist for gathering human input:" to "Four mechanisms exist for gathering human input:" so the lead-in count matches the four bullets immediately below (Formal HITL, Feedback comments, Phase approval, Orchestrator-emitted decisions). Kept the explicit count rather than rephrasing to "Several mechanisms…" since the bullets are numbered and a concrete count aids readers skimming the section.

— Authored by egg

[egg-reviewer]

Re-review of bdd90fa against 8ad5617.

The delta is a single-line fix to docs/hitl-decisions.md:11:

-Three mechanisms exist for gathering human input:
+Four mechanisms exist for gathering human input:

That exactly addresses the non-blocking number-vs-list mismatch flagged in the prior review. The four numbered bullets immediately below (Formal HITL, Feedback comments, Phase approval, Orchestrator-emitted decisions) now agree with the lead-in count, and the PR description's framing of this as "a fourth HITL mechanism" is consistent.

No other changes in the delta. All blocking-grade and non-blocking items from the prior two reviews remain addressed (verified by re-reading the section that contains the new ## Orchestrator-Emitted Decisions anchor — unchanged since 8ad5617). Nothing else to flag.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

Thanks for the re-review. The latest approval on bdd90fa confirms all prior feedback is addressed and there are no new actionable items.

No new feedback in this round. disagree (no actionable items; the latest review explicitly states "Nothing else to flag" and carries verdict=approve) — the re-review of bdd90fa against 8ad5617 traced the single-line Three → Four fix to docs/hitl-decisions.md:11, confirmed it resolves the prior round's number-vs-list mismatch, and re-verified that all six items from the original review plus the count mismatch remain addressed. No code change required.

For completeness, prior rounds (already addressed before this run):

1. Backup-ref timestamp is nanoseconds → fixed-in-PR (commit 8ad5617)
2. populate_contract has two 409 reasons → fixed-in-PR (commit 8ad5617)
3. Trigger/recovery conflation → fixed-in-PR (commit 8ad5617)
4. failed_pending_hitl backticks → fixed-in-PR (commit 8ad5617)
5. git log <backup_ref> needs orchestrator-side context → fixed-in-PR (commit 8ad5617)
6. Helper/caller step ordering → fixed-in-PR (commit 8ad5617)
7. Three-vs-four mechanism count → fixed-in-PR (commit bdd90fa)

— Authored by egg

[james-in-a-box]

egg feedback addressed. View run logs

7 previous review(s) hidden.