Recover context PR on create_context_branch divergence

[jwbron]

Summary

Closes the durability gap called out in PR #2575 review (blocking-severity issue 1).

When the context PR hook partially fails between pushing the artifact commit and persisting contract.pr.context_pr_number, the next tick's create_context_branch raises on branch divergence (existing_sha = base_sha+1 vs base = base_sha). The hook's outer except Exception swallowed it without invoking _recover_existing_context_pr — the pipeline silently never opens a context PR for the rest of its lifetime, even though one is live on GitHub.

The narrow real-world trigger is post-push, pre-contract-persist failure (e.g. save_contract raises after a successful create_pr, or push_worktree_branch raises after a successful push). Severity is doc-only / fail-soft — slices still ship — but the recovery code was specifically designed for this case (see commit message on #2578) and was half-complete relative to its stated purpose.

Changes

• orchestrator/gateway_client.py: new ContextBranchDiverged(GatewayError) subclass. create_context_branch raises this specifically on existing-SHA divergence instead of the bare GatewayError. Callers that broadly catch GatewayError continue to work via the subclass relationship.
• orchestrator/routes/pipelines.py::_open_context_pr_for_pipeline: catch ContextBranchDiverged ahead of the broad except, run _recover_existing_context_pr, persist the salvaged linkage (so subsequent ticks idempotent-skip), and return the branch name. A non-divergence GatewayError still routes through the broad except and fails-soft as today.
• Persistence tail (save_contract + commit + push) extracted into _persist_context_pr_linkage_on_contract so the new recovery path and the existing happy/create_pr-recovery paths share one implementation. No behavior change to the existing paths.
• Tests:
  • TestOpenContextPRRecoverAfterBranchDiverged (4 cases): divergence with a recoverable PR, divergence with no recoverable PR, divergence with mismatched base_ref, and non-divergence GatewayError still fails-soft (no recovery attempted).
  • test_create_context_branch::test_raises_when_existing_branch_diverges: tightened to assert the typed ContextBranchDiverged subclass + its recovery-side metadata (context_branch, existing_sha, base_branch, base_sha), while keeping the broad-catch contract via isinstance(..., GatewayError).

Test plan

• .venv/bin/pytest orchestrator/tests/test_context_pr.py orchestrator/tests/test_create_context_branch.py — 51 passed (40 existing + 4 new context-PR + 7 gateway)
• .venv/bin/ruff check on touched files — clean
• CI green

Out of scope

The PR #2575 review listed two adjacent observations (logging the swallowed KeyError in _recover_existing_context_pr, and tightening the torn-write claim in _commit_slice_brc_history_to_integration_branch's docstring). Keeping this PR scoped to the blocking durability fix — those can be separate.

[egg-reviewer]

Reviewed and verified locally — 51 tests pass. This is a clean, well-scoped fix for the durability gap PR #2575's reviewer flagged.

The design choices are sound:

• ContextBranchDiverged subclasses GatewayError so broad-catch callers keep working — backward-compatible.
• The new except ContextBranchDiverged branch sits before the broad except Exception, so the divergence path takes precedence; a non-divergence GatewayError still falls through to fail-soft. Pinned by test_other_gateway_errors_still_fail_soft.
• Extracting _persist_context_pr_linkage_on_contract removes duplication cleanly — the divergence branch and the post-create_pr happy path share the same persistence tail with no behavioral drift.
• The new path correctly skips the artifact file-copy / _commit_statefiles_to_worktree / push_worktree_branch for the context branch — that work was already done by the prior tick — and only writes the contract linkage on the work branch.
• The four new tests cover the matrix: recovers when PR exists; returns None when none exists; rejects mismatched base_ref; non-divergence GatewayError still fails soft.

I traced the divergence-recovery flow end-to-end and verified the test assertions against the actual control flow. The list_open_prs filter on head_ref + base_ref correctly guards against salvaging an unrelated stale PR.

Non-blocking suggestions

1. Adjacent durability gap: branch pushed but create_pr failed

The PR scopes itself to the "post-create_pr, pre-save_contract" failure. A closely related sub-case is still wedged after this fix:

• Tick 1: create_context_branch succeeds → artifact commit pushed → create_pr raises (or returns no URL) → _recover_existing_context_pr returns None (no PR exists yet) → hook returns None.
• Tick 2: create_context_branch raises ContextBranchDiverged (branch exists at our pushed SHA ≠ base_sha) → new code calls _recover_existing_context_pr → still None → hook returns None again.

The pipeline is stuck — the branch is already on origin, all that's missing is the PR, but the divergence-recovery path never reaches create_pr. The test test_returns_none_when_diverged_and_no_existing_pr explicitly pins this "return None" behavior with the rationale "somebody else pushed to our branch shape" — but because the branch name is pipeline-id-prefixed (egg/<pipeline_id>/context), a more likely cause is our own prior tick's create_pr failure, not third-party interference.

This was already broken before this PR (the broad except Exception did the same thing), and addressing it requires a different code shape (call create_pr directly when divergence + no recoverable PR), so it's reasonably out of scope. Worth tracking as a follow-up issue.

2. Class placement

ContextBranchDiverged is defined at line 3090 but referenced from create_context_branch at line 2180. Forward references work fine in Python because the reference is inside a method body, but co-locating it with GatewayError (right after, ~line 3088) avoids the "where is this class?" hunt. Optional, defer to author preference.

3. Pre-existing pattern note (not in this PR)

While reading surrounding code I noticed except ValueError, TypeError: (gateway_client.py:2298), except KeyError, ValueError, TypeError: (pipelines.py:9484), and similar at pipelines.py:17455/17483. These parse and work on Python 3.14 as implicit-tuple form (verified via dis.dis), but they're not idiomatic — PEP 8 expects except (X, Y, Z): with explicit parens, and the implicit-tuple form is easy to misread as Python 2 except X, name: syntax. Pre-existing across the codebase and not introduced by this PR — purely flagging in case there's appetite for a cleanup pass. The PR's new code uses correct except Exception: / except ContextBranchDiverged: so it's not at risk.

Verification

• Ran pytest orchestrator/tests/test_context_pr.py orchestrator/tests/test_create_context_branch.py — 51/51 passed.
• Traced control flow through _open_context_pr_for_pipeline, _recover_existing_context_pr, _persist_context_pr_linkage_on_contract, and the gateway-side create_context_branch raise path.
• Verified backward compatibility: isinstance(ContextBranchDiverged(...), GatewayError) holds.
• Verified no other production call sites of create_context_branch exist (only the one in _open_context_pr_for_pipeline).
• Confirmed the call-site wrapper at pipelines.py:19689 catches any exception that escapes the hook, so even if _persist_context_pr_linkage_on_contract raised unexpectedly the plan→implement transition is not blocked.

[james-in-a-box]

Thanks for the careful review — verifying the new branch precedes the broad except, threading the list_open_prs filter check, and re-running the 51-test suite end-to-end is exactly the level of scrutiny that catches subtle ordering bugs. Per-item disposition below.

1. Adjacent durability gap (post-create_pr failure → divergence + no recoverable PR)

deferred-to #2582 — you explicitly suggested tracking this as a follow-up issue, and the fix requires a different code shape (call create_pr directly on divergence + no recoverable PR, with corresponding update to the test_returns_none_when_diverged_and_no_existing_pr assertion) that materially expands this PR's scope. #2582 captures the trigger, proposed fix, and acceptance criteria including the test change.

2. Class placement

disagree (already co-located with GatewayError) — ContextBranchDiverged is at line 3090 and GatewayError is at line 3075 (its __init__ body ends at line 3088). The two classes are separated only by one blank line — this is the exact placement you suggested ("right after, ~line 3088"). The "defined at line 3090 but referenced from line 2180" framing is accurate but reflects the file's existing convention of defining error classes at the bottom (immediately before the _gateway_client singleton and get_gateway_client factory), not a placement choice I introduced. Happy to revisit if you had a different target location in mind that I'm misreading.

3. Pre-existing except X, Y, Z: implicit-tuple patterns

disagree (pre-existing, not introduced by this PR) — as you noted, these are pre-existing across the codebase and the PR's new code (except ContextBranchDiverged: / except Exception:) does not introduce any. A targeted cleanup pass is a reasonable refactor candidate but doesn't belong bundled into a durability-gap fix — bundling would dilute the PR's blast-radius story for the reviewer (yourself, future archaeologists, and the orchestrator) and force a re-review of the cleanup alongside the fix.

— Authored by egg

[james-in-a-box]

Feedback contract violation detected.

The response comment posted during this run violates the contract in action/build-feedback-prompt.sh. Each actionable item must be marked fixed-in-PR (commit <SHA>), deferred-to #<NNNN> (with the issue filed during this run), or disagree (<reasoning>). Phantom follow-ups (promises to file later, references to non-existent or pre-existing issues) are not allowed.

Violations:

forbidden phrase(s): tracking this as a follow-up

View run logs

— Authored by egg

[james-in-a-box]

Conflict Resolution: Escalating to human review

This PR has merge conflicts with main that require author judgment rather than auto-resolution.

Root cause

While this PR was open, #2600 was merged to main ("Fix #2582: unify context-PR idempotency at top of hook"). Both PRs target the same _open_context_pr_for_pipeline durability hole — the post-push, pre-contract-persist failure mode — but with incompatible mechanisms:

• This PR (Recover context PR on create_context_branch divergence #2579): catches ContextBranchDiverged, calls _recover_existing_context_pr (via gh pr list) inside the except block, persists linkage if found, returns None if no PR is found.
• Fix #2582: unify context-PR idempotency at top of hook #2600 (already in main): adds a top-of-hook GitHub-state lookup (_lookup_existing_context_pr at "step 2"). On divergence at step 3, falls through to steps 4-5 (push + create_pr) under the rationale that step 2 already ruled out an existing PR, so the divergent SHA must be our own prior tick's artifact push.

The conflicts:

File	Lines	Conflict
orchestrator/gateway_client.py	2176-2182	Comment-only — both raise the same ContextBranchDiverged
orchestrator/gateway_client.py	3094-3116	Docstring on ContextBranchDiverged class — both describe the same class, different recovery model
orchestrator/routes/pipelines.py	10069-10154	Semantic — the entire except ContextBranchDiverged body
orchestrator/routes/pipelines.py	10429-10432	Trailing blank line
orchestrator/tests/test_create_context_branch.py	175-194, 220-246	Docstring + variable-binding differences on the same assertions

The first three test conflicts are docstring/wording only — both versions assert the same behavior on ContextBranchDiverged metadata. The blocking conflict is pipelines.py:10069-10154.

Why I can't auto-resolve

Option A: accept main's version of the except block

This strictly supersedes this PR's mechanism — main handles both partial-failure cases:

• "PR opened but contract not persisted" → step 2 finds the PR, salvages it, returns
• "Branch pushed but PR never created" → step 3 raises divergence, falls through to create_pr (which Recover context PR on create_context_branch divergence #2579's _recover_existing_context_pr would have skipped, returning None)

But it would break this PR's 4 new tests in TestOpenContextPRRecoverAfterBranchDiverged (test_context_pr.py:793):

• test_recovers_when_create_context_branch_raises_diverged — asserts create_pr.assert_not_called() after divergence; main would call it
• test_returns_none_when_diverged_and_no_existing_pr — asserts result is None; main would proceed and return the branch
• test_diverged_recovery_ignores_mismatched_base_ref — asserts no recovery on stale base; under main, step 2's head_only_match covers this earlier
• test_other_gateway_errors_still_fail_soft — would still pass

It would also leave this PR's _recover_existing_context_pr call in the divergence path dead — but main has its own _recover_existing_context_pr (or equivalent via _lookup_existing_context_pr) at step 2.

Option B: accept this PR's version

The top-of-hook lookup main added (step 2) would still run (it's outside the conflict region), and would catch the "PR exists, contract not persisted" case before step 3. So the divergence catch would only fire for the "branch pushed, no PR" case — where this PR's _recover_existing_context_pr returns None (no PR found) and the hook returns None. Net effect: the case #2582 specifically calls out (branch pushed, PR never created) silently fails forever, regressing the fix in #2600.

Why this is a true semantic conflict

This isn't "both sides modify the same logic differently" in a way I can mechanically reconcile — it's "the bug this PR fixes was fixed by a different PR with a fundamentally different mechanism, and choosing either resolution breaks something." The decision belongs to the author.

Suggested paths forward

1. Close this PR if you agree Fix #2582: unify context-PR idempotency at top of hook #2600's mechanism covers your case. The four new test classes in test_context_pr.py could be ported (rewritten against main's fall-through behavior) as a follow-up PR if you want regression coverage for the "branch pushed, no PR" path.
2. Rebase + rework this PR to add only the parts that complement Fix #2582: unify context-PR idempotency at top of hook #2600 — e.g., if the typed ContextBranchDiverged metadata (context_branch, existing_sha, base_branch, base_sha) was something Fix #2582: unify context-PR idempotency at top of hook #2600 didn't expose, that could land here. (I noticed Fix #2582: unify context-PR idempotency at top of hook #2600 already exposes these — please confirm.)
3. Keep this PR's approach by explicitly removing Fix #2582: unify context-PR idempotency at top of hook #2600's top-of-hook check — but that regresses the "PR opened but contract not persisted" case main now handles earlier.

The two adjacent observations from the #2575 review you marked out of scope (logging the swallowed KeyError in _recover_existing_context_pr, tightening the _commit_slice_brc_history_to_integration_branch docstring) are still good candidates for a small follow-up if option 1 is the right call.

I aborted the merge and left the branch untouched. No-op until you decide.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Aborted — needs human judgment

I attempted to merge origin/main into this branch but aborted because the conflicts are semantic, not mechanical: PR #2600 (already merged to main) explicitly supersedes this PR with a different architectural approach to the same wedge.

Conflicting files

File	Category	Issue
orchestrator/gateway_client.py	Additive (docstring)	Both sides add ContextBranchDiverged; conflict is on docstring wording. Auto-resolvable.
orchestrator/routes/pipelines.py	Semantic	Same except ContextBranchDiverged: clause filled with mutually-exclusive recovery strategies.
orchestrator/routes/pipelines.py	Duplicate function	Both PRs added a _persist_context_pr_linkage_on_contract helper with the same name but different implementations. Git auto-merged them as two functions side-by-side (one shadows the other).
orchestrator/tests/test_create_context_branch.py	Semantic-adjacent	Same test, two docstrings explaining different rationales (one references _recover_existing_context_pr, the other references "top-of-hook GH-state check").

Why this is semantic, not mechanical

PR #2600's commit message (863d299) is explicit:

"Supersedes PR #2579, which targeted the same wedge with an additional layered exception handler. The unified top-level check ... replaces three recovery handlers with zero — net less code, no layered recovery..."

The two PRs solve the same #2582 wedge with mutually-exclusive designs:

• This PR (Recover context PR on create_context_branch divergence #2579): On ContextBranchDiverged, run _recover_existing_context_pr to salvage via gh pr list, persist linkage, return early.
• PR Fix #2582: unify context-PR idempotency at top of hook #2600 (on main): Add a top-level list_open_prs idempotency check at the start of the hook. On ContextBranchDiverged, just log and fall through to the push + create_pr flow — the top-level check has already verified no PR exists, so divergence "by elimination" must be our own prior tick's artifact push.

These can't be mechanically merged because:

1. The recovery handler bodies are different (_recover_existing_context_pr + return vs. log + fall through).
2. Both sides added a helper with the same name _persist_context_pr_linkage_on_contract. The merge result has two definitions ~100 lines apart, which is a Python silent-shadow bug, not a working state.
3. The test docstrings reference the two different recovery paths — picking one orphans the other branch's rationale.

What needs to happen

This PR appears to be superseded rather than complementary. Options for the author/reviewer:

1. Close as superseded — PR Fix #2582: unify context-PR idempotency at top of hook #2600 already lands the durability fix for the wedge this PR targets.
2. Rework on top of Fix #2582: unify context-PR idempotency at top of hook #2600 — if the salvage-via-_recover_existing_context_pr path adds something beyond the top-level idempotency check (e.g., an extra safety net for some narrow race), open a fresh PR that adds only that delta on top of the current main.

I cannot pick between these without product/architecture context.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Aborted — PR Superseded by #2600

I attempted to merge origin/main into this branch but aborted because the conflicts represent a semantic supersession, not a routine merge.

Root cause: PR #2600 (commit 863d299, "Fix #2582: unify context-PR idempotency at top of hook") merged into main on 2026-05-11 and its own commit message explicitly states:

Supersedes PR #2579, which targeted the same wedge with an additional layered exception handler.

#2600 fixes the same durability gap this PR addresses, but via a different strategy:

Aspect	This PR (#2579)	main (#2600)
Where divergence is handled	New except ContextBranchDiverged handler runs _recover_existing_context_pr (gh-pr-list salvage) and persists linkage	New top-of-hook _lookup_existing_context_pr does the GitHub-state check; divergence handler just falls through to fast-forward push + create_pr
Recovery layers	Layered (step-3 except handler stacked on top of existing recovery branches)	Unified (single GitHub-state check at the top, the two post-create_pr recovery branches deleted)
Round-trips	None added	One list_open_prs per tick until context_pr_number is persisted
New ContextBranchDiverged typed exception	✅	✅ (same class, same metadata)
_persist_context_pr_linkage_on_contract helper	✅ extracted from inline persistence	✅ extracted (independently)

Conflicts encountered

File	Category	Why I can't auto-resolve
orchestrator/routes/pipelines.py (~line 10215)	Semantic	The PR's _recover_existing_context_pr recovery branch directly contradicts main's "fall through to push + create_pr" branch. They're mutually exclusive control flows in the same except ContextBranchDiverged handler.
orchestrator/routes/pipelines.py (~line 10575)	Additive (from main)	Unrelated additions from #2583 / #2599 / #2607 (auto-ACK seeding, plan→implement transition wiring) — would resolve as accept-main, but only meaningful after the semantic conflict above is decided.
orchestrator/routes/pipelines.py (silent duplicate at lines 9694 + 9799)	Duplicate definition	Both sides independently added _persist_context_pr_linkage_on_contract. Git auto-merged them as additive, producing two definitions of the same function — the second shadows the first at import time. Whichever recovery strategy we keep, only one copy survives.
orchestrator/gateway_client.py (~line 2176)	Additive	Comment-only; PR adds rationale, main has nothing. Trivial to accept the PR's comment.
orchestrator/gateway_client.py (~line 3094)	Formatting/docstring	ContextBranchDiverged docstring differs — PR frames it around _recover_existing_context_pr, main frames it around the top-of-hook fallthrough. Wording follows whichever code strategy we keep.
orchestrator/tests/test_create_context_branch.py (~lines 175 / 220)	Formatting/docstring	Test assertions are equivalent; only docstring wording and import location differ.

The pipelines.py semantic conflict is the load-bearing one — the rest follow from how it's decided.

What I need from you

The decision isn't between "your code" vs "their code" — it's a product/architecture decision:

1. Close this PR as superseded by Fix #2582: unify context-PR idempotency at top of hook #2600 (recommended): main already fixes the documented wedge. Re-resolving the conflicts in favor of Recover context PR on create_context_branch divergence #2579's approach would require reverting _lookup_existing_context_pr from main, which would re-introduce the head-only-mismatch and post-create_pr-no-URL recovery branches that Fix #2582: unify context-PR idempotency at top of hook #2600 deliberately deleted. The remaining delta on this PR after accepting main would be ~0 (the typed exception, the helper extraction, and the comment are already on main).

2. Salvage a non-overlapping piece of this PR: e.g. a follow-up PR for the two adjacent observations the original PR description listed as out-of-scope (logging the swallowed KeyError in _recover_existing_context_pr — though that function no longer exists on main; tightening the _commit_slice_brc_history_to_integration_branch docstring). If you want either of these I can open a fresh, conflict-free PR.

3. Override: if you want Recover context PR on create_context_branch divergence #2579's layered-recovery approach kept in addition to Fix #2582: unify context-PR idempotency at top of hook #2600's top-of-hook check (defense in depth), say so — I'd resolve by keeping both the new step-2 check and the new step-3 recovery handler, and would want sign-off because it adds a redundant gh pr list round-trip on the rare divergence path.

Merge state is fully reset; nothing was pushed. Awaiting direction.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Escalation

I attempted to merge origin/main into this branch but stopped after categorizing the conflicts as semantic / superseded — they need a human decision (likely "close this PR") rather than an auto-resolve.

Conflicts found

File	Category	Notes
orchestrator/gateway_client.py (line ~3094)	Semantic	Both sides rewrote the ContextBranchDiverged docstring to describe two different recovery strategies.
orchestrator/gateway_client.py (line ~2176)	Trivial	A comment block this PR added before the raise ContextBranchDiverged(...); main has no comment. Easy to keep this PR's comment — but downstream divergence makes it moot.
orchestrator/routes/pipelines.py (line ~10215)	Semantic	The except ContextBranchDiverged handler. This PR routes through _recover_existing_context_pr + persist; main falls through to a fast-forward push under the assumption that the top-level list_open_prs check (added in #2600) already proved no PR exists. Mutually exclusive strategies.
orchestrator/routes/pipelines.py (line ~10575)	Additive	main added _derive_producer_roles_with_tasks (#2581) and _maybe_open_base_pr_for_plan_to_implement (#2593) after _persist_context_pr_linkage_on_contract. Auto-resolvable in isolation.
orchestrator/tests/test_create_context_branch.py (lines ~175, ~220)	Semantic	Both sides tightened the divergence test to assert the typed ContextBranchDiverged subclass + recovery metadata, but the assertion shapes diverge in ways that mirror the production-code split above.

Why this needs human review

This PR appears to have been superseded by #2600 (merged 2026-05-11). The merge commit message of #2600 says so explicitly:

Supersedes PR #2579, which targeted the same wedge with an additional layered exception handler. The unified top-level check costs one extra list_open_prs round-trip per tick until contract.pr.context_pr_number is durably persisted (after which the contract-state fast path skips the lookup entirely), and replaces three recovery handlers with zero — net less code, no layered recovery, and the head-only-mismatch + divergence-with-no-PR cases are now first-class branches of one switch rather than emergent properties of nested exception handling.

#2600 ships the same ContextBranchDiverged typed-subclass primitive this PR introduces, plus the tightened test_raises_when_existing_branch_diverges assertion, plus a different (top-of-hook idempotency) recovery strategy that explicitly removes the layered exception handler this PR adds in _open_context_pr_for_pipeline. Trying to combine the two would re-introduce the layered handler #2600 deliberately deleted.

Recommended action

Close this PR — the durability gap from #2575 review issue 1 is fixed in main via #2600. If the operator disagrees and wants to keep #2579's layered-handler approach, the merge needs to be hand-resolved by a human who can decide which of the two recovery strategies to keep, since the choice is not derivable from the diffs alone.

What I did locally

• Ran git fetch origin main and git merge --no-commit origin/main.
• Read each conflict in full file context.
• Aborted the merge (git reset --merge) — branch is back to 1b2830b27, no force-push, no remote changes.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Escalating to Human Review

I attempted to merge origin/main into this branch but found the conflicts are semantic, not mechanical, and the PR's purpose appears to have been independently addressed on main by #2582 (863d299e5: Fix #2582: unify context-PR idempotency at top of hook).

Files with conflicts

File	Category	Notes
orchestrator/gateway_client.py	Additive (docstring)	Two harmless conflicts: a comment block and the ContextBranchDiverged docstring. Both sides agree on the exception class shape and metadata; only the prose differs (HEAD describes _recover_existing_context_pr, main describes "fall through to artifact-push + create_pr").
orchestrator/routes/pipelines.py	Semantic — escalating	See below.
orchestrator/tests/test_create_context_branch.py	Additive (docstring)	Both sides assert the same ContextBranchDiverged shape and metadata fields. Only the docstring narration differs.

Why I'm not auto-resolving

This PR adds a recovery path inside _open_context_pr_for_pipeline:

except ContextBranchDiverged as branch_err:
    # ... call _recover_existing_context_pr, persist linkage, return

Main's #2582 took a different approach to closing the same gap: it added a top-of-hook _lookup_existing_context_pr (via list_open_prs) that runs before create_context_branch, plus a ContextBranchDiverged handler that falls through to push + create_pr (relying on the top-of-hook lookup having already verified no PR exists).

Concretely on main:

• Scenario A (prior tick: push ✓, create_pr ✓, save_contract ✗): top-of-hook lookup sees the open PR → salvages linkage → returns.
• Scenario B (prior tick: push ✓, create_pr ✗): top-of-hook lookup sees no PR → proceeds → create_context_branch raises ContextBranchDiverged → falls through to push (fast-forward no-op) + create_pr → opens the missing PR.

Both scenarios — including the one this PR was specifically designed to fix — are now handled by main's structure. The PR's except ContextBranchDiverged handler:

• Is redundant for Scenario A (top-of-hook lookup already returned).
• Is arguably wrong for Scenario B: it calls _recover_existing_context_pr (which returns None since no PR exists), then returns None and skips. Main correctly proceeds to push + create_pr to open the missing PR.

Mechanically merging would require either:

1. Drop this PR's recovery branch entirely in favor of main's structure (closes the gap via Context PR hook: open PR after divergence when no recoverable PR exists #2582's mechanism — but then this PR has no functional content beyond the typed ContextBranchDiverged subclass, which Context PR hook: open PR after divergence when no recoverable PR exists #2582 also adds independently in its merged version).
2. Layer the PR's recovery after main's top-of-hook lookup — this would only fire in genuinely impossible states (lookup said no PR, then divergence, then suddenly a PR appears) and would still leave Scenario B broken because the recovery returns None.
3. Reshape the PR to do something complementary to Context PR hook: open PR after divergence when no recoverable PR exists #2582 (e.g. log the recovery path differently, add observability) — but that's a different PR.

There's also a non-trivial test rewrite implied: TestOpenContextPRRecoverAfterBranchDiverged (4 tests added by this PR) tests a code path that no longer exists in main's structure. The auto-merged test_context_pr.py already pulled in main's TestOpenContextPRTopLevelIdempotency rename, so the test file is internally inconsistent.

What I need from a human

A scope decision: Is this PR superseded by #2582, and should it be closed? Or is there a defensive layer worth keeping that I'm missing?

If keeping, the resolution probably needs the author to:

• Rebase the recovery onto main's structure (lookup + divergence-fallthrough)
• Decide whether _recover_existing_context_pr should still exist or be replaced by _lookup_existing_context_pr
• Rewrite TestOpenContextPRRecoverAfterBranchDiverged against the new structure

I aborted the merge (git reset --merge); the branch is unchanged.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Aborted — Semantic Supersession

Attempted to merge origin/main into this branch. Aborted because the conflict resolution requires human judgment, not just a mechanical choice between sides.

What happened

Three files conflicted:

File	Category	Notes
orchestrator/gateway_client.py	Semantic (docstring + comment)	Both sides describe ContextBranchDiverged recovery semantics, but via different mechanisms
orchestrator/routes/pipelines.py	Semantic (incompatible recovery designs)	HEAD: catch divergence → call _recover_existing_context_pr → return. Main: catch divergence → log → fall through to push + create_pr (the push is a fast-forward no-op over the prior tick's commit)
orchestrator/tests/test_create_context_branch.py	Docstring / assertion-style	Functionally equivalent; both pin the same ContextBranchDiverged metadata

Additionally, the auto-merge produced two problems even before conflict resolution:

• _persist_context_pr_linkage_on_contract was duplicated (HEAD's version + main's near-identical version both included as separate function defs).
• _recover_existing_context_pr (defined only in HEAD) was dropped from the merged tree, but lines that reference it remained inside the HEAD-side conflict region — so taking HEAD as-written would produce a NameError.

Why this is not auto-resolvable

The two branches solve the same durability gap (post-push, pre-contract-persist partial failure) with different, overlapping designs:

• This PR's design (the targeted fix for the Egg/issue 2548/work #2575 review comment): on ContextBranchDiverged in step 3 of _open_context_pr_for_pipeline, run _recover_existing_context_pr (a gh pr list lookup against head_ref + base_ref), and if a match is found, persist the linkage and return.

• Main's design (introduced after this branch by Context PR hook: open PR after divergence when no recoverable PR exists #2582 / Fix #2593: wire context-PR hook into all plan→implement transition paths #2599 / context_pr.{skipped,failed} events don't reach recent_messages or wait-status (#2599 follow-up) #2611 / Fix #2611: route context_pr.{skipped,failed} to message store + event bus #2621): a new top-of-hook step 2 calls _lookup_existing_context_pr before create_context_branch runs. If a PR is already on GitHub for our head, it's salvaged there. If the subsequent step 3 then sees divergence, that means step 2 already proved no PR exists, so the divergence is by elimination our own prior tick's push — fall through to push + create_pr, which opens the missing PR.

Main's design is a strict superset of this PR's scenarios:

Scenario	This PR	Main
Push succeeded, create_pr succeeded, save_contract failed	✅ recovers in step-3 divergence catch	✅ recovers in step-2 top-of-hook lookup
Push succeeded, create_pr never ran (crash between)	❌ returns None — PR never opens	✅ falls through; idempotent push + create_pr opens the missing PR
Push failed transiently	✅ logs and returns	✅ logs and returns (broad except)

If I take main's version in every conflict (the natural "merge supersedes" resolution), the resulting tree is byte-identical to origin/main, and the PR becomes a no-op merge.

Recommendation

One of:

1. Close this PR as superseded by Context PR hook: open PR after divergence when no recoverable PR exists #2582 / Fix #2593: wire context-PR hook into all plan→implement transition paths #2599. The blocking durability gap called out in the Egg/issue 2548/work #2575 review is addressed by main's top-of-hook lookup + the divergence fall-through, which together cover this PR's scenario plus the push-but-no-PR scenario it didn't.
2. Land an empty merge if you want a paper trail that this PR's intent was fulfilled. I can re-do the merge taking main's version everywhere; the resulting commit will record the resolution but show zero file changes vs main.
3. Re-scope this PR to whatever still isn't covered by main — but from my reading, nothing in this PR's diff adds value that main doesn't already provide.

Awaiting direction.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Escalating — PR superseded by #2600

I attempted to merge origin/main into this branch to resolve conflicts, but the conflict is semantic and mutually exclusive — main already implements the same fix via a different (and explicitly stated as superseding) approach.

The semantic collision

PR #2600 ("Fix #2582: unify context-PR idempotency at top of hook", commit 863d299) landed on main after this PR was opened. Its commit message states:

Supersedes PR #2579, which targeted the same wedge with an additional layered exception handler.

Both PRs target the same _open_context_pr_for_pipeline divergence wedge but take incompatible recovery strategies:

	This PR (#2579)	main (#2600)
Where idempotency lives	After create_context_branch raises	Top of hook (list_open_prs)
On ContextBranchDiverged	Catch → _recover_existing_context_pr → salvage via gh pr list → persist	Log → fall through to push+create_pr (knowing no PR exists, proven by the top-of-hook check)
ContextBranchDiverged class	Added	Already added
Test for divergence path	Asserts recovery via _recover_existing_context_pr	Asserts fall-through to artifact-push + create_pr

The top-of-hook list_open_prs check on main makes this PR's recovery branch unreachable; this PR's recovery branch makes main's fall-through unreachable. They cannot both exist.

Conflicting files

File	Lines	Category	Why escalating
orchestrator/gateway_client.py	~30	Semantic (docstring)	Both sides define ContextBranchDiverged with the same __init__, but the docstrings describe incompatible caller contracts (_recover_existing_context_pr salvage vs. fall-through to push).
orchestrator/routes/pipelines.py	~250	Semantic (logic)	Both sides handle except ContextBranchDiverged: differently. Mutually exclusive recovery strategies; cannot include both.
orchestrator/tests/test_create_context_branch.py	~25	Semantic (test rationale)	Tests assert overlapping but rationale-conflicting metadata; trivial to merge alone, but pointless if pipelines.py conflict is escalated.

Decision needed

This is not a conflict an automated resolver can decide. Options:

1. Close this PR as superseded by the already-merged Fix #2582: unify context-PR idempotency at top of hook #2600 (matches the explicit intent in Fix #2582: unify context-PR idempotency at top of hook #2600's commit message). Recommended.
2. Replace this PR's approach with main's — effectively git checkout origin/main -- <conflicting files> and abandon _recover_existing_context_pr. Equivalent to option 1 with extra steps; commit history would no longer match the PR description.
3. Argue the recovery path is still needed alongside main's top-of-hook check (defensive layering). Would require a re-scoped PR description and likely a different test — main's top-of-hook check already covers the documented trigger.

Per the conventions: semantic conflicts where both sides modify the same logic differently → escalate. I'm leaving the branch unchanged (git reset --merge ran cleanly; no local mutations).

— Authored by egg

[james-in-a-box]

Conflict Resolution: Aborted — Human Review Needed

Attempted a git merge origin/main to resolve the conflicts on this branch. Aborted the merge without pushing because the central conflict is semantic, not mechanical: the architectural gap this PR is fixing has been independently addressed on main by #2600 (Fix #2582: unify context-PR idempotency at top of hook) using a different design.

The conflict

Three files conflicted: orchestrator/gateway_client.py, orchestrator/routes/pipelines.py, orchestrator/tests/test_create_context_branch.py. Two of those are trivial (a comment and a docstring on ContextBranchDiverged, and stylistic test-assertion differences). The blocker is in _open_context_pr_for_pipeline:

This PR (HEAD) — when create_context_branch raises ContextBranchDiverged, call _recover_existing_context_pr (which does gh pr list against the head branch). If a PR is found, persist the linkage and return; if not, skip the hook for this tick.

main after #2600 — does the gh pr list lookup at the top of the hook (step 2, before create_context_branch). If create_context_branch then raises divergence, by elimination no PR exists on the head branch, so the prior tick must have pushed the artifact commit but failed create_pr. The handler falls through to push_worktree_branch (fast-forward no-op) and create_pr opens the missing PR.

#2600's commit message explicitly names this wedge:

Also closes the #2582 wedge: when create_context_branch raises divergence AFTER the top-level check has confirmed no PR exists on our head, the divergence is by elimination our prior tick's artifact push that never reached create_pr. The hook falls through to the existing fast-forward / no-op push and opens the missing PR.

Why the two approaches don't compose

If we keep this PR's recovery at step 3 on top of main's step 2 GH check, the sequence becomes:

1. Step 2 (top-of-hook GH lookup): no PR exists → proceed
2. Step 3 (create_context_branch): raises divergence
3. This PR's handler calls _recover_existing_context_pr → also finds nothing → returns None

That short-circuits the hook before the fallthrough push + create_pr that main's #2600 design relies on to open the missing PR. The "pushed-but-no-PR" case — which is the exact wedge this PR is also targeting — would silently stop recovering.

Files that conflicted

File	Category	Notes
orchestrator/gateway_client.py	Semantic (docstring) + Additive (comment)	Comment at the raise ContextBranchDiverged site is additive. The class docstring conflict reflects the two divergent designs (#2600 vs this PR).
orchestrator/routes/pipelines.py	Semantic — blocking	The ContextBranchDiverged handler in _open_context_pr_for_pipeline is mutually exclusive between this PR's design and #2600's design. Also a large additive block at the tail (_derive_producer_roles_with_tasks, _context_pr_events_emitted, _maybe_open_base_pr_for_plan_to_implement are new on main, none on this branch).
orchestrator/tests/test_create_context_branch.py	Stylistic	Both sides assert the same things on ContextBranchDiverged; differences are docstring framing + assertion ordering.

What to decide

This PR predates #2600 (#2600 merged 2026-05-11; this PR's commit is from 2026-05-10). The blocking durability issue described in the PR body is real, but main already fixes it via a different and incompatible mechanism. Options:

1. Close as obsolete — Fix #2582: unify context-PR idempotency at top of hook #2600 + the top-of-hook check in Context PR hook: open PR after divergence when no recoverable PR exists #2582 cover the wedge. The new helpers this PR introduces (_recover_existing_context_pr already existed on this branch; _persist_context_pr_linkage_on_contract was a refactor extraction) are not needed by main's design.
2. Rebase + reduce scope — drop the step-3 recovery branch (let main's fallthrough run), keep only any pieces that genuinely add value on top of Fix #2582: unify context-PR idempotency at top of hook #2600 (e.g. the ContextBranchDiverged test tightening, the comment at the raise site). The remaining diff would be small.

I can't pick between these without product input, so leaving the branch untouched. No commits pushed.

— Authored by egg

[james-in-a-box]

Conflict Resolution: Escalating — semantic conflict

Tried merging origin/main and hit conflicts in 3 files. After analysis, the resolution requires human judgment because PR #2582 (which landed on main while this PR was open) takes a different architectural approach to the same durability gap. I'm escalating rather than guessing.

Conflicts found

File	Conflict region	Category
orchestrator/gateway_client.py	ContextBranchDiverged docstring (~line 3094)	Semantic — docstring framing differs to match each side's recovery model
orchestrator/gateway_client.py	Comment above the raise ContextBranchDiverged(...) (~line 2176)	Additive — HEAD adds an explanatory comment; main has none
orchestrator/routes/pipelines.py	ContextBranchDiverged catch in _open_context_pr_for_pipeline (~line 10239, ~85 lines)	Semantic — both sides handle divergence differently
orchestrator/routes/pipelines.py	Function boundary after _open_context_pr_for_pipeline (~line 10599, ~228 lines)	Additive on main's side — main inserts _derive_producer_roles_with_tasks and _maybe_open_base_pr_for_plan_to_implement
orchestrator/tests/test_create_context_branch.py	test_raises_when_existing_branch_diverges docstring + assertion comments	Formatting/docstring — assertions are functionally identical

Additionally, git auto-merge produced a duplicate _persist_context_pr_linkage_on_contract definition (lines 9718 and 9823 in the merged file) because both branches added that helper at different positions. Whichever way the semantic conflict is resolved, the duplicate needs cleanup.

Why this is semantic, not mechanical

This PR addresses the durability gap by catching ContextBranchDiverged from create_context_branch and calling _recover_existing_context_pr (which does gh pr list --head <context_branch> and salvages the existing PR).

main (via PR #2582 + #2611 + #2621) addresses the same gap differently:

1. Step 2 of the hook (new on main) calls _lookup_existing_context_pr at the top of the hook, before create_context_branch. It returns matched / head_only_match / error. On matched, the hook salvages the linkage and returns immediately.
2. The divergence catch in main assumes "by elimination, since Step 2 just verified no open PR exists on our head, the divergent SHA must be our own prior tick's artifact push that didn't reach create_pr" — and falls through to push + create_pr (idempotent fast-forward over the prior tick's commit).

HEAD's _recover_existing_context_pr and main's _lookup_existing_context_pr are functionally identical — both call list_open_prs and match head_ref == context_branch && base_ref == base_branch. Main's just runs earlier and is wired into a richer three-state result.

The decision needed

Has main's PR #2582 superseded this PR?

• If yes — close this PR; main's top-of-hook lookup already handles the post-push, pre-contract-persist failure mode (the next tick sees the open PR in Step 2 and salvages it).
• If no — there's something this PR does that main doesn't, and the conflict needs to be resolved by rewriting this PR on top of main's new architecture (e.g., the divergence handler could call _lookup_existing_context_pr as a backup before falling through, in case Step 2 had a transient list_open_prs failure earlier in the same tick).

I'm not equipped to make that product-level call — closing a PR or restructuring it on top of a newer architecture is your decision, not a mechanical merge fix.

What I did

Ran git merge --no-commit origin/main, analyzed the conflicts, then aborted (git reset --merge) to restore the branch to 1b2830b27. No changes pushed.

— Authored by egg

[james-in-a-box]

egg conflict resolution completed. View run logs

[egg-reviewer]

Re-review: nothing left to review. The PR is closed (no merge), and the May 12 merge from main at 9319ad8d took main's side of the conflicts entirely — the merge result tree is identical to its main-side parent cf0f9aa4. The PR's original contributions to gateway_client.py, routes/pipelines.py, tests/test_context_pr.py, and tests/test_create_context_branch.py are no longer present on this branch.

Verification

$ git diff cf0f9aa4 9319ad8d        # main parent of merge → PR head
(empty)

$ git log origin/main..pr-2579 --oneline
9319ad8d Merge remote-tracking branch 'origin/main' into pr-2579

$ git log pr-2579..origin/main --oneline
2eff2820 docs: Update build context docs for #2499 fix [doc-updater] (#2637)

The branch is exactly main-at-cf0f9aa4 (one commit behind current main, with the #2637 doc-updater commit missing). There is no PR-authored code delta to scrutinize.

Context (matches the conflict-resolution comment already on this PR)

#2600 ("Fix #2582: unify context-PR idempotency at top of hook", merged as 863d299e) supersedes this PR with a different mechanism for the same wedge — top-of-hook list_open_prs check, then fall through to push + create_pr on ContextBranchDiverged (push is idempotent over the prior tick's commit). The ContextBranchDiverged class and its raise site in create_context_branch are present on main — they originated in this PR's predecessor work but landed via #2600's slightly different docstring shape.

If the author still wants the four divergence-recovery tests as regression coverage for the "branch pushed, PR never created" path (the case the bot's prior conflict comment flagged as still uncovered), they would need to be rewritten against #2600's fall-through behavior in a fresh PR. That work is not on this branch.

Recommendation

No action on this PR. Closed status is correct. Any follow-up regression coverage should land on main in a new PR.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

20 previous review(s) hidden.