feat(branch): assert HEAD is on impl branch around coder turn (AB#3210) by PolyphonyRequiem · Pull Request #401 · PolyphonyRequiem/polyphony

PolyphonyRequiem · 2026-05-14T23:07:01Z

AB#3210: assert HEAD is on impl branch around coder turn

Closes AB#3210.

Background — what bit us in apex run 3165

The impl agent for AB#3175 wrote its commit (3f45269) onto
impl/3165-3176 — a SIBLING task's impl branch — instead of
impl/3165-3175. This produced a perfectly silent failure:

Surface	What it showed
`impl/3165-3175`	empty (0 commits ahead of MG base)
`mg/3165_pg-3175`	empty (squash of empty impl)
AB#3175 work item	marked Done
`feature/3165`	missing the verb the AB was filed to deliver

The misroute was structurally invisible until the empty-MG check
caught it three review cycles later. There was no fast-fail at the
boundary where the impl agent acquired the worktree — by the time
the reviewer saw the no-op MG, the misrouted commit was already
stranded on the wrong impl branch and AB#3175 had been state-
transitioned to Done.

Recovery required hand-shipping PR #398 (cherry-picking the verb
onto impl/3165-3175) and aborting the scope-review cap gate.

Fix — verb + workflow gates

New verb: polyphony branch assert-on-impl --root-id N --item-id M

Reads git symbolic-ref HEAD via IGitClient.
Compares against BranchNameBuilder.Impl(root, item).Value
(impl/{root}-{item}).
Emits routing-style envelope with action discriminator:
- ok — current branch matches expected
- mismatch — current ≠ expected, OR detached HEAD, OR git failure
  (all surface uniformly so the gate prompt covers every "not on the
  right branch" case)
- error — input validation failure (caller-side bug)

Workflow integration — implement-merge-group.yaml:

The assertion runs at TWO points around every coder turn:

Pre-coder (between guidance_loader and coder).
If the worktree is on the wrong branch BEFORE the coder runs,
halt to a human gate so the operator can git checkout the right
branch before any code is written. Prevents the bug.
Post-coder (between coder and fetch_for_primary_review).
If the coder somehow ended up off-branch, halt before the
misrouted commit can be promoted into the MG. Catches any path
that bypassed the pre-check.

Each gate offers Retry (re-run the assertion after operator fixes
the worktree) or Abort. Pre-coder gate prevents the data loss;
post-coder gate is defense-in-depth.

Test coverage

10 unit tests in BranchCommandsAssertOnImplTests covering
missing args, ok / mismatch / detached-HEAD / git-failure paths,
and the snake_case JSON output contract.
AB#3175 reproducer: HeadOnSiblingImplBranch_EmitsMismatch
asserts that root_id=3165, item_id=3175, actual=impl/3165-3176
returns action=mismatch (the exact 3175 incident).
tests/lint-jinja-resolver.ps1 over the live workflows: no new
errors, no new warnings introduced by this change.
conductor validate .conductor/registry/workflows/implement-merge-group.yaml
passes.
Full Polyphony.Tests suite green (3511 pass, 4 skip, 0 fail).

Notes

Hand-curated fixture: per existing convention,
tests/lint/fixtures/verb-output-schemas.json is hand-curated to
preserve VERB003 lint coverage (Move feat: Phase Detection, Routing Engine, and Command Implementations (PG-3) AB#2593 AB#2594 #2 sentinels would otherwise
flatten required: true in the live schema). I added the new verb
- result-type entries manually with required: true for inputs to
  match the existing pattern.
Harness scenario deferred: the workflow harness driver does not
yet support scripted human-gate responses. A scenario covering
these two new gates would need driver-side work first. Worth filing
as a follow-up.
Defense, not root-cause fix: the misroute most likely
originates upstream in the per-task branch checkout in the impl
executor. Tracking that down is a separate investigation. This PR
ensures that when the bug fires again, it halts loudly at the
dispatch boundary instead of green-washing into the MG.

Files

src/Polyphony/Models/BranchAssertOnImplResult.cs (new)
src/Polyphony/Commands/BranchCommands.AssertOnImpl.cs (new)
src/Polyphony/PolyphonyJsonContext.cs (+1 line registration)
tests/Polyphony.Tests/Commands/BranchCommandsAssertOnImplTests.cs (new, 10 tests)
.conductor/registry/workflows/implement-merge-group.yaml
(+4 nodes: 2 assertions + 2 gates)
tests/lint/fixtures/verb-output-schemas.json (hand-curated entry)

Closes AB#3210. ## Problem In apex run 3165, the impl agent for AB#3175 wrote its commit (`3f45269`) onto `impl/3165-3176` — a SIBLING task's impl branch — rather than `impl/3165-3175`. Result: - `impl/3165-3175` stayed empty. - `mg/3165_pg-3175` squash-merged a no-op. - AB#3175 was marked Done with zero code on `feature/3165`. - Recovery required a hand-shipped PR (#398) and a scope-review abort. The misroute was structurally invisible until the empty-MG check fired three reviews later. There was no fast-fail at the boundary where the impl agent acquired a worktree. ## Fix New verb `polyphony branch assert-on-impl --root-id N --item-id M`: - Reads `git symbolic-ref HEAD` (via existing `IGitClient`). - Compares against `BranchNameBuilder.Impl(root, item).Value` (`impl/{root}-{item}`). - Emits a routing-style envelope with discriminator `action`: - `ok` when current == expected - `mismatch` when current != expected (or detached HEAD, or git failure — both surface as mismatch with empty actual_branch) - `error` reserved for input validation failures (caller-side bug) `implement-merge-group.yaml` now invokes the assertion at TWO points: 1. Pre-coder — between guidance loader and coder dispatch. If the worktree is on the wrong branch BEFORE the coder runs, halt to a human gate so the operator can `git checkout` the right branch before any code is written. 2. Post-coder — between coder return and primary review. If the coder somehow ended up off-branch (or the worktree drifted mid-turn), halt before the misrouted commit can be promoted into the MG. Both gates offer Retry (re-run the assertion after operator intervention) or Abort. ## Test coverage - 10 unit tests in `BranchCommandsAssertOnImplTests` covering missing args, ok / mismatch / detached-HEAD / git-failure paths, and the snake_case JSON output contract. - The exact AB#3175 reproducer (`HeadOnSiblingImplBranch_EmitsMismatch`) asserts that `root_id=3165, item_id=3175, actual=impl/3165-3176` returns `action=mismatch`. - `tests/lint-jinja-resolver.ps1` over the live workflows reports no new errors and no new warnings. - Workflow YAML validates with `conductor validate`. ## Notes - Verb fixture (`tests/lint/fixtures/verb-output-schemas.json`) is hand-curated per the existing convention — manually added the new verb + result-type entries with `required: true` for inputs to preserve VERB003 lint coverage. - A harness scenario for these gates was deferred: the harness driver does not yet support scripted human-gate responses. Filing a follow-up to extend it would unblock E2E coverage. - The verb is a defense, not a root-cause fix. The misroute itself most likely originates in the per-task branch checkout in the upstream impl executor; tracking that down is a separate investigation. The assertion ensures that when the bug fires again, it halts loudly at the dispatch boundary instead of green-washing into the MG. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…3216 (#404) Three lint failures cascaded since PR #401/#402 merged: 1. lint-type-agnostic: implement-merge-group.yaml had `| Task | AB#... |` markdown table rows in failure-recovery prompts. The row label `Task` was generic prose meaning "work item being implemented", not the type. Renamed to `| Work item | AB#... |` (3 sites). 2. lint-bare-cross-branch-refs: squash_coverage_mismatch_gate prompt inlined `git diff/log` commands with `{{ assert_impl_pr_coverage.output.{comparison_base,impl_ref,mg_ref} }}` jinja templates. The verb returns origin/-prefixed values at runtime (PrCommands.AssertImplPrCoverage.cs lines 69-75) but the lint can only see the unresolved templates. Added `# bare-ref-ok: <reason>` comments above each line, and reworded the line 872 comment to drop the literal `git diff` verb. 3. lint-pwsh-jinja-bareword: this lint has `if: always()` so it runs even after step (1)/(2) fail. But `Install powershell-yaml` was sequenced AFTER the lint block; once a non-`if: always()` lint failed, the install skipped and the bareword lint errored with "FATAL: the powershell-yaml module is required". Reordering ci.yml hardens against this whole class: - Move `Install Pester` and `Install powershell-yaml` BEFORE the lint block so they always run. - Add `if: always()` to every lint step so a single failure surfaces ALL violations in one CI run instead of cascading via skip. Why both PRs slipped through: PR #401 and #402 squash-merged within 30s of each other (23:26:48Z and 23:27:19Z). Each PR's CI ran against its own merge commit on a clean base — neither saw the OTHER PR's regression that would land alongside on main. Classic merge-skew false-green. Verified: all three lints pass locally (lint-type-agnostic, lint-bare-cross-branch-refs, lint-pwsh-jinja-bareword). conductor validate implement-merge-group.yaml is clean. Co-authored-by: Daniel Green <dangreen@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ses AB#3214 (#406) AB#3175 incident (apex run 3165, 2026-05-14): a child task reached state=Done in ADO via `primary_completer` while the implementing commit was stranded on a sibling task's impl branch and the squash-merge to the MG carried zero diff. Net result: ADO reported the work as Done while `feature/{root}` lacked any of its commits. PRs #401 (AB#3210, impl-branch routing assertion) and #402 (AB#3211, `pr assert-impl-pr-coverage`) plug the upstream root causes. After both ship, the failure mode is structurally impossible — PROVIDED `primary_completer` is reachable ONLY via the coverage-asserted path. This PR encodes that contract as a structural lint (`tests/lint-primary-completer-trust-chain.ps1`) so future routing edits cannot reintroduce a bypass path: I1. The ONLY route INTO `primary_completer` is from `delete_impl_branch`. I2. The ONLY routes INTO `delete_impl_branch` are from `assert_impl_pr_coverage` (action='ok') or `squash_coverage_mismatch_gate` (option='force_accept'). Together these guarantee that `primary_completer` fires only after diff-equivalence was asserted or the operator explicitly acknowledged a known mismatch. Approach. Added a "Trust chain (AB#3214)" comment block to `primary_completer` documenting the upstream defenses, the routing invariant, the historical incident, and the lint that enforces it. Did NOT add an inline coverage re-assertion: the upstream defense runs immediately before this step (via `delete_impl_branch`), so an inline call would be redundant duplicate work. Per AB#3214 AC, the "document trust chain explicitly" branch is the right answer, with the structural lint as the regression test. Validation: - `conductor validate` clean on implement-merge-group.yaml. - New lint PASSes against the real workflow (trust chain intact). - 5/5 Pester tests for the new lint: * 2 PASS scenarios (synthetic correct chain + real workflow) * 3 FAIL scenarios (rogue predecessor on primary_completer, rogue predecessor on delete_impl_branch, missing assert_impl_pr_coverage) - 74/74 existing trunk lints (type-agnostic, bare-cross-branch-refs, pwsh-jinja-bareword) still pass. References: AB#3214, AB#3210 (PR #401), AB#3211 (PR #402), AB#3175 (incident) Co-authored-by: Daniel Green <dangreen@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…3210) (#484) Adds pre_coder_branch_mismatch_aborts_via_scripted_gate scenario to implement-merge-group.yaml. Stubs the 5-step entry chain (branch_ensure_mg → primary_router → impl_branch_ensure → guidance_loader → branch_assert_on_impl_pre_coder returning action='mismatch') and uses the AB#3212 scripted-gates infrastructure to drive the gate's 'abort' option — the recovery path proven by AB#3210 (PR #401) but until now uncovered by automated tests because --skip-gates would auto-pick the first option (retry) and loop forever against the stubbed verb. Uses the gates_resolved trace assertion shipped in PR #483: gates_resolved: pre_coder_branch_mismatch_gate: abort No direct git, no agents — pure polyphony-verb entry chain, all shim-interceptable. Co-authored-by: Daniel Green <dangreen@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

PolyphonyRequiem mentioned this pull request May 14, 2026

fix: detect silent commit drop on squash merge (AB#3211) #402

Merged

PolyphonyRequiem merged commit c80548b into main May 14, 2026
1 check failed

PolyphonyRequiem deleted the fix/3210-impl-branch-assertion branch May 14, 2026 23:26

PolyphonyRequiem mentioned this pull request May 15, 2026

fix(ci): unblock trunk after PR #401/#402 — type-agnostic + bare-ref + install order AB#3216 #404

Merged

PolyphonyRequiem mentioned this pull request May 15, 2026

fix: lint primary_completer trust chain to enforce AB#3210/3211 defenses AB#3214 #406

Merged

PolyphonyRequiem mentioned this pull request May 19, 2026

test(harness): pre_coder_branch_mismatch_gate abort scenario (AB#3210) #484

Merged

Navigation Menu

feat(branch): assert HEAD is on impl branch around coder turn (AB#3210) - #401

feat(branch): assert HEAD is on impl branch around coder turn (AB#3210)#401
PolyphonyRequiem merged 1 commit into
mainfrom
fix/3210-impl-branch-assertion

PolyphonyRequiem commented May 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

PolyphonyRequiem commented May 14, 2026

AB#3210: assert HEAD is on impl branch around coder turn

Background — what bit us in apex run 3165

Fix — verb + workflow gates

Test coverage

Notes

Files

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant