[issue-3393][slice-7/7] Secondary-repo worktree + branch... - #3442
Conversation
Two-layer coverage for task-7-1 (secondary-repo worktree + branch materialization), matching the slices 2/4/5/6 convention: * Always-green model-level invariants pinning the materialization repo-set == participating set (repos owning >=1 slice, primary-first, deduped; slice-less excluded), repo-less-slice->primary, per-repo dedup, N=1->one repo, and the uniform per-repo egg/<id>/work branch naming (distinct by remote for same-short-name/diff-owner repos). * End-to-end opener tests driving _open_secondary_context_prs / _maybe_open_secondary_context_prs with the gateway mocked (branches materialized): a context PR opens in every participating secondary (no missing-head-branch soft-fail), routed to the right repo/base/head, idempotently adopting an already-open PR; slice-less secondary skipped; N=1 guard is a no-op (no contract load, no gateway calls). The worktree-list-threading + per-repo branch-push seam is handed to the coder via task-7-1 gap-1; the invariants stay green independent of it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-7-1) + task-5-1 gap fixes
task-7-1 (cq-4): thread the full participating-repo list into worktree
creation so every repo owning >=1 slice has egg/<pipeline_id>/work pushed
on its OWN remote before any PR-opening call runs against it — closing the
slice-4 secondary-context / slice-PR missing-head-branch soft-fail.
- gateway/worktree_manager/_create.py: create_worktree gains push_branch;
new _materialize_work_branch_on_remote pushes the worktree HEAD to
refs/heads/{assigned_branch or work-branch}. Non-forced + idempotent
(an already-present branch / non-ff is treated as materialized), so it
never clobbers the primary's contract-init commit. Bound on the class
in the worktree_manager barrel.
- gateway/gateway.py: worktree_create reads push_branch and forwards it.
- orchestrator/gateway_client/_worktree.py: create_worktrees gains
push_branches, forwarded as request_data["push_branch"].
- orchestrator/kubernetes_spawner/_spawn.py: the per-agent create_worktrees
call passes push_branches=(len(repos) > 1). slice-6 already threads the
full participating repo set (slice-first) into repos, so every
participating remote is materialized on the first multi-repo spawn.
N=1: len(repos)==1 => push_branches=False => byte-identical to pre-#3393.
Operator decision-8 gap closures (task-5-1):
- gap-2: _cross_repo_hold_resolution now releases ONLY on an EXACT match of
the release option id/label; the fail-open `"release" in text` substring
(which readied on a negated "do NOT release yet" freeform) is removed.
Ambiguous/negated/keep values fall through to the KEEP fail-safe,
honouring the operator's cq-1 "human owns the release" ruling.
- gap-1/gap-2 coverage: new orchestrator/tests/test_cross_repo_merge_gate.py
drives poll_once (Tier-A happy path, closed-unmerged terminal, attempt-bound
timeout terminal, multi-upstream AND-gate, Tier-B skip-poll, GateProgress
idempotency) and pins the tightened hold-resolution mapping.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
No agent-mode design concerns.
This slice is pure git infrastructure — threading a push_branch flag through orchestrator → gateway-client → gateway → worktree-manager to materialize each participating repo's egg/<pipeline_id>/work branch on its own remote. No agent-facing surface is touched: no prompt construction or pre-fetching, no structured-output-for-humans, no post-processing of agent output, no direct Anthropic API calls, and no pinned model identifiers.
Branch materialization is performed gateway-side (credentials held by the gateway, the push enforced there), which is consistent with the "the sandbox/gateway is the constraint" core principle rather than pushing this into prompt-level behavior. The N=1 byte-identical guarantee and the non-forced/idempotent push also keep the design change tightly scoped.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Contract Verification — PR #3442 (issue-3393, slice-7/7)
Verdict: Approve. Both slice-7 tasks are implemented, correct, and their acceptance criteria are met. No contract violations found.
Scope reviewed
Diffed the branch against origin/main (three-dot) to isolate the contract-relevant changes from the large main-merge / CI-fix reflow that PR #3442 also carries. The BRC-history JSON/MD blobs and the ruff-format line-reflow across pipelines.py / test_kubernetes_spawner.py are mechanical and out of scope.
task-7-1 — Secondary-repo worktree + branch materialization (commit 3d85c92cc) ✅
Traced the full push_branch seam end-to-end; it matches the task description and its four pinned files + the barrel binding:
orchestrator/kubernetes_spawner/_spawn.py:push_branches=bool(repos) and len(repos) > 1— strictly gated on N>1.orchestrator/gateway_client/_worktree.py:push_branches→request_data["push_branch"](only when true).gateway/gateway.py:worktree_createreadspush_branchand forwards it tocreate_worktree.gateway/worktree_manager/_create.py: new_materialize_work_branch_on_remotepushes worktreeHEAD:refs/heads/{assigned_branch or branch_name}, invoked on both the reused-worktree and freshly-created paths. Bound inworktree_manager/__init__.py.
Acceptance criteria:
- N>1: every ≥1-slice repo has
egg/<id>/workpushed on its own remote before any PR-opening call — met. The push runs at worktree-create (spawn) time, strictly before the slice-4 PR openers; the full participating repo set is threaded via slice-6 (merged dependency). - Secondary context / N>1 slice PRs open with no missing-head-branch soft-fail — met architecturally; opener tests confirm the behavior once heads are materialized.
- N=1: no behavioral change — met.
push_branchdefaultsFalsethrough the entire chain and the gate islen(repos) > 1, so N=1 spawns are byte-identical to pre-#3393.
Safety: the push is non-forced, best-effort (all exceptions swallowed, 30s timeout, worktree creation never fails on it), and treats up-to-date / non-fast-forward / rejected as "already materialized" — so it correctly never clobbers the primary repo's contract-init commit. Verified against the reused-worktree path as well.
task-7-2 — Tests (commit 7076c6db) ✅ (with one operator-accepted limitation)
orchestrator/tests/test_secondary_repo_materialization.py covers the materialization repo-set invariants (participating set, primary-first, dedup, slice-less exclusion, N=1→one repo), per-repo egg/<id>/work naming, end-to-end secondary context-PR opening (gateway mocked), PR adoption/idempotency, slice-less skip, and the N=1 no-op. Both new test files compile cleanly.
Noted (non-blocking): push_branch has no direct executable-seam coverage — grep push_branch across the test tree returns nothing, so the real threading path (_spawn.py gate → gateway_client → gateway → create_worktree(push_branch=…) → _materialize_work_branch_on_remote) and the idempotent-push behavior are exercised only at the invariant / mocked-opener level. This is exactly the gap recorded in task-7-1 gap-1, which is marked resolved and explicitly accepted by the operator as a follow-up (2026-07-02T20:25Z). Per that ruling I am not treating it as blocking; the follow-up integration test should be filed.
Bundled work outside slice-7 (informational)
Commit 3d85c92cc also lands task-5-1 / gap-2 — _cross_repo_hold_resolution now releases only on an exact match of the release option id/label, removing the fail-open "release" in text substring that would have readied a held PR on a negated freeform ("do NOT release yet"). This is correct and well-tested: test_cross_repo_merge_gate.py::test_negated_release_freeform_keeps pins the exact regression, alongside exact-id/label/envelope release tests and keep/ambiguous fail-safe tests. Flagging only because task-5-1 belongs to slice-5, not slice-7; the coder's notes reference operator acceptance.
False alarm investigated
except ValueError, TypeError: (in gateway.py and pipelines.py) is not a bug: the project pins requires-python = ">=3.14" and CI runs 3.14, where PEP 758 makes the unparenthesized form valid and it catches both types. main already uses this form elsewhere in the same file; this is benign ruff-format normalization from the CI-fix commit.
Phase / commit consistency
Slice-7 status=complete; both tasks complete with linked commits that match their content (3d85c92cc = impl, 7076c6db = tests). Consistent.
Process note
egg-contract verify-criterion could not be run — the orchestrator is unreachable (Error: Orchestrator unreachable), so criterion status could not be persisted. Additionally, the contract's top-level acceptance_criteria array is empty (criteria are stored per-task as strings), so there are no ac-N ids to mark; I verified the per-task acceptance criteria directly against the diff instead.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Address contract-verification review on PR #3442: the push_branch threading path (gateway_client -> gateway route -> create_worktree -> _materialize_work_branch_on_remote) and the idempotent-push behavior were only exercised at the invariant / mocked-opener level (grep push_branch over the test tree returned nothing). - orchestrator/tests/test_gateway_client.py: assert push_branches=True forwards push_branch to the wire payload, and the N=1 default omits it. - gateway/tests/test_gateway_integration.py: assert the worktree/create route forwards push_branch to create_worktree (and defaults False). - gateway/tests/test_work_branch_materialization.py: pin the _materialize_work_branch_on_remote behavior — non-forced HEAD:refs/heads/<target> refspec, assigned/work-branch target resolution with origin/ stripped, and the idempotent-push contract (up-to-date / non-fast-forward rejection and push failures/exceptions are all swallowed so worktree creation never fails).
Review feedback addressedBoth reviews approved; the one actionable item was the non-blocking coverage gap the contract-verification review flagged.
All 13 new gateway-side tests and 2 new orchestrator-side tests pass locally; — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
No agent-mode design concerns (re-review).
The delta since my prior approval (commit 952ffaa) is the feedback-addressing commit 8a00d35, which adds only test coverage for the push_branch threading path — no product code changed:
gateway/tests/test_work_branch_materialization.py— pins_materialize_work_branch_on_remote: non-forcedHEAD:refs/heads/<target>refspec,origin/-prefix stripping, and the idempotent/best-effort push contract (up-to-date, non-fast-forward, and exception cases all swallowed; no force push).orchestrator/tests/test_gateway_client.py—push_branches=Trueforwardspush_branchto the wire payload; N=1 default omits the key.gateway/tests/test_gateway_integration.py— theworktree/createroute forwardspush_branchtocreate_worktreeand defaultsFalse.
These close the executable-seam coverage gap the contract-verification review flagged. As before, this slice remains pure git infrastructure with no agent-facing surface: no prompt construction or pre-fetching, no structured-output-for-humans, no post-processing of agent output, no direct Anthropic API calls, and no pinned model identifiers. My prior approval stands.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Contract Verification — Re-review (incremental)
Verdict: Contract-compliant, no blocking issues. (Posting as --comment — self-authored PR.)
Scope note
The branch was squashed/rebased since my last review, so the prior review commit 952ffaaf is no longer reachable in this (shallow) worktree and a 952ffaaf..HEAD delta diff isn't computable. The clone also has no merge-base with origin/main (three-dot diff unavailable). I therefore re-verified against the two-dot net diff (origin/main..HEAD) with focus on the one new PR commit, 8a00d35 — "Add executable-seam coverage for push_branch threading" — which is the substantive delta and directly addresses my prior review.
Delta verified — closes gap-1 follow-up
8a00d35 adds executable-seam coverage for the push_branch threading path that my prior review flagged as only invariant/mock-level (grep push_branch over the test tree previously returned nothing). This matches the gap-1 resolution ("executable integration test of the real worktree-threading/branch-push seam"):
gateway/tests/test_work_branch_materialization.py(new) — pins_materialize_work_branch_on_remote: non-forcedHEAD:refs/heads/<target>refspec, target resolution (assigned branch → else branch_name,origin/stripped), and the idempotent contract (up-to-date / non-fast-forward / fetch-first rejections and generic push failures /TimeoutExpiredall swallowed so worktree creation never fails; no--force, no+refspec).orchestrator/tests/test_gateway_client.py— assertspush_branches=Trueforwardspush_branchto the wire payload; N=1 default omits the key.gateway/tests/test_gateway_integration.py— asserts the/api/v1/worktree/createroute forwardspush_branchtocreate_worktree, defaulting False.
All 12 new tests pass locally (8 + 2 + 2). I did not run the full suite (per review policy — CI owns that).
Task-7-1 acceptance criteria (re-confirmed, no regression)
The core threading is intact end-to-end: _spawn.py sets push_branches=bool(repos) and len(repos) > 1 → gateway_client._worktree.create_worktrees(push_branches=…) emits push_branch → gateway.worktree_create reads it → create_worktree(push_branch=…) → _materialize_work_branch_on_remote (invoked on both the reused-worktree and fresh-create paths, before any PR-opening call).
- ✅ N>1: every participating repo's
egg/<id>/workis pushed (non-forced, idempotent) to its own remote before PR opening. - ✅ N=1:
push_branches=False,push_branchkey omitted,create_worktreedefault False → path byte-identical to pre-#3393. - ✅ Non-forced/idempotent push never clobbers the primary's contract-init commit.
All five modified implementation files compile cleanly under Python 3.14.
Non-blocking advisory
gateway/gateway.py:4613(ingh_pr_merge_state) changesexcept (ValueError, TypeError):→except ValueError, TypeError:. This is valid and semantically identical on Python 3.14 (PEP 758; the project targetspy314and the gateway image ispython:3.14-slim), so it is not a functional bug. However it's outside slice-7's scope and inconsistent with the parenthesized style used elsewhere in the file — suggest restoring the parentheses for readability/consistency and to avoid a latent hazard if any tooling ever runs on <3.14.
Criterion marking
This contract carries no top-level ac-N acceptance criteria (they live as per-task prose), so verify-criterion has nothing to mark. The orchestrator was also unreachable during this run (egg-contract show returned "Orchestrator unreachable"); I read contract state from .egg-state/contracts/issue-3393.json directly.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
Review feedback addressedBoth design reviews approved with no agent-mode concerns, and both contract-verification passes confirm the slice is contract-compliant. Two items surfaced across the rounds:
No new commit is needed: the coverage item is already in HEAD (8a00d35) and the parens advisory is a formatter-enforced no-op that would regress CI if applied. — Authored by egg |
|
egg feedback addressed. View run logs 6 previous review(s) hidden. |
Materialize the operator's cq-4 resolution: thread the full repo list into worktree creation and push each participating repo's work/integration branches to its own remote, making coordinated cross-repo PRs (slice-4's routing and per-repo context PRs) function end-to-end. Without this slice the multi-repo feature is structural-only.
Base PR: #3418
What's in this PR
Commits (3):
This slice
Secondary-repo worktree + branch materialization (cq-4)
Files affected:
orchestrator/gateway_client/_worktree.pyorchestrator/kubernetes_spawner/_spawn.pygateway/gateway.pygateway/worktree_manager/_create.pyorchestrator/tests/gateway/tests/tests/Tasks (2) + acceptance criteria
Stack
issue-3393egg/issue-3393/slice-6