Skip to content

Insert feature → main promotion before close_mark_satisfied (AB#3168) - #375

Merged
PolyphonyRequiem merged 1 commit into
mainfrom
fix/ab-3168-feature-promotion-routing
May 14, 2026
Merged

Insert feature → main promotion before close_mark_satisfied (AB#3168)#375
PolyphonyRequiem merged 1 commit into
mainfrom
fix/ab-3168-feature-promotion-routing

Conversation

@PolyphonyRequiem

Copy link
Copy Markdown
Owner

Closes AB#3168 — apex-driver never opens feature → main PR for indivisible apex roots

Root cause

Indivisible apex roots — those whose lifecycle_router only ever classifies their own work without decomposing into ADO children — never had feature-pr invoked under their item-dispatch lifecycle path. Instead the classifier flips through implement-merge-group for the MG → impl-PR cascade, and ObserveImplementationMergedAsync marks implementation_merged Satisfied as soon as the impl PR merges into mg/{root} (which itself merges into feature/{root}). The work is on feature/{root} but never reaches main, and close_mark_satisfied fires while the apex's own feature branch is still ahead of main — leaving a satisfied work item with un-promoted commits.

AB#3166's post-mortem reproduced this exactly: only plan-level and implement-merge-group lifecycles fired (never feature-pr); feature/3166 had to be manually merged via PR #374.

Fix (Option C — workflow-only)

Two new nodes in apex-driver.yaml's SECTION 4 (Close), funnelling both pre-existing paths to close_mark_satisfied:

  • promote_feature_to_main — script that fetches origin/main and origin/feature/{apex_id}, runs git rev-list --count origin/main..origin/feature/{apex_id}, and routes:
    • needs_promotion=truepromote_feature_pr_dispatch
    • needs_promotion=falseclose_mark_satisfied (catch-all; covers fast-pathed apexes and the case where the feature branch doesn't exist in origin)
  • promote_feature_pr_dispatchtype: workflow invocation of feature-pr.yaml, mirroring the input shape from apex-item-dispatch.yaml's existing feature_pr_dispatch. Routes unconditionally to close_mark_satisfied after completion; feature-pr.yaml has its own escalation gates for non-merge outcomes.

Both upstream paths to close_mark_satisfied are rewired:

  • apex_completion_gate_policy_router (auto branch) → promote_feature_to_main
  • apex_completion_gate (confirm option) → promote_feature_to_main

Why Option C and not Option A (new requirement kind)

Considered adding a new feature_promoted RequirementKind + ObserveFeaturePromotedAsync observer + derivation rules + e2e tests. That is the highest-fidelity fix and may still be appropriate for AB#3169 (premature ADO Done cascade), but it carries engine churn that Option C avoids entirely. Option C is purely YAML, mirrors an existing dispatch shape, and is easy to reason about — and feature-pr.yaml is already battle-tested in the decomposed-apex path. If AB#3169's fix needs to introduce the feature_promoted kind anyway, this PR doesn't block that work — the workflow-level routing layer would simply remain in place as defense-in-depth.

Tests

  • e2e-apex-driver.Tests.ps1 — 5 new structural assertions:
    • promote_feature_to_main is a script with the expected pwsh args + git rev-list --count shape
    • Routes to promote_feature_pr_dispatch when needs_promotion, else close_mark_satisfied
    • promote_feature_pr_dispatch invokes ./feature-pr.yaml with the apex root inputs
    • Funnel invariant: NO direct path from apex_completion_gate_policy_router or apex_completion_gate.confirm to close_mark_satisfied
  • Updated existing apex_completion_gate exposes confirm/abandon test to assert confirm now routes via promote_feature_to_main
  • Updated documented happy-path waypoint list to include both new nodes

Verification

  • conductor validate apex-driver.yaml — Validation Successful
  • Invoke-Pester e2e-apex-driver.Tests.ps1105/105 pass (including the 5 new tests)
  • Invoke-Pester lint-apex-driver.Tests.ps129/29 pass
  • All registry lint scripts pass: lint-apex-driver, lint-strict-undefined, lint-bare-cross-branch-refs, lint-pwsh-jinja-bareword, lint-prose-children, lint-sync-after-mutation

The bare-cross-branch lint required a # bare-ref-ok: annotation on the line preceding the git rev-list call, since the lint tokenizer cannot see through PowerShell variable interpolation (origin/$targetBranch..origin/$featureBranch). The actual git command has explicit origin/ prefixes.

Deferred / known limitations

  • target_branch = main is hard-coded, matching the existing feature_pr_dispatch invocation in apex-item-dispatch.yaml. If per-apex target branches are ever supported, this is the second of two sites to update.
  • Pre-flight refusal when gh/az auth is missing is not added here — feature-pr.yaml already has its own auth-required gates that surface to a human.
  • Two pre-existing failures in lint-plan-level.Tests.ps1 are unrelated (verified by stashing this PR's changes; same 2/25 failures on bare main).

Related

  • Sibling fix: AB#3169 (premature ADO Done cascade)
  • Sibling fix: AB#3170 (polyphony validate idempotency + cache coherence)
  • Reproduced by: AB#3166 dogfood post-mortem

Root cause
----------
Indivisible apex roots — those whose lifecycle_router only ever
classifies their own work without decomposing into ADO children — never
had `feature-pr` invoked under their item-dispatch lifecycle path.
Instead the classifier flips through `implement-merge-group` for the
MG → impl-PR cascade, and `ObserveImplementationMergedAsync` marks
`implementation_merged` Satisfied as soon as the impl PR merges into
`mg/{root}` (which itself merges into `feature/{root}`). The work is
on `feature/{root}` but never reaches `main`, and `close_mark_satisfied`
fires while the apex's own feature branch is still ahead of main —
leaving a satisfied work item with un-promoted commits. AB#3166's
post-mortem reproduced this exactly.

Fix (Option C — workflow-only)
------------------------------
Two new nodes inserted in apex-driver.yaml's SECTION 4 (Close),
funnelling both pre-existing paths to `close_mark_satisfied`:

* `promote_feature_to_main` — script that fetches `origin/main` and
  `origin/feature/{apex_id}`, runs `git rev-list --count
  origin/main..origin/feature/{apex_id}`, and routes:
    * needs_promotion=true  → promote_feature_pr_dispatch
    * needs_promotion=false → close_mark_satisfied (catch-all,
      covers fast-pathed apexes and the case where the feature
      branch doesn't exist in origin)
* `promote_feature_pr_dispatch` — `type: workflow` invocation of
  `feature-pr.yaml`, mirroring the input shape from
  apex-item-dispatch.yaml's existing `feature_pr_dispatch`. Routes
  unconditionally to `close_mark_satisfied` after completion;
  feature-pr.yaml has its own escalation gates for non-merge outcomes.

Both upstream paths to `close_mark_satisfied` are rewired:
* apex_completion_gate_policy_router (auto branch) → promote_feature_to_main
* apex_completion_gate (confirm option)            → promote_feature_to_main

Tests
-----
* e2e-apex-driver.Tests.ps1: 5 new structural assertions (script
  command/args/routing on promote_feature_to_main, workflow +
  input_mapping on promote_feature_pr_dispatch, and a funnel-invariant
  test asserting NO direct path from apex_completion_gate_policy_router
  or apex_completion_gate's confirm option to close_mark_satisfied).
* Updated the documented happy-path waypoint list to include both
  new nodes.
* Updated the existing `apex_completion_gate exposes confirm/abandon`
  test to assert confirm now routes via promote_feature_to_main.

Verification
------------
* `conductor validate apex-driver.yaml` — Validation Successful.
* `Invoke-Pester e2e-apex-driver.Tests.ps1` — 105/105 pass (including
  the 5 new tests).
* `Invoke-Pester lint-apex-driver.Tests.ps1` — 29/29 pass.
* All registry lint scripts pass: lint-apex-driver,
  lint-strict-undefined, lint-bare-cross-branch-refs,
  lint-pwsh-jinja-bareword, lint-prose-children, lint-sync-after-mutation.

The bare-cross-branch lint required a `# bare-ref-ok:` annotation on
the line preceding the `git rev-list` call, since the lint tokenizer
cannot see through PowerShell variable interpolation
(`origin/$targetBranch..origin/$featureBranch`).

Hard-coded `target_branch = main` matches the existing
feature_pr_dispatch invocation in apex-item-dispatch.yaml; if per-apex
target branches are ever supported, this is the second of two sites
to update.

Closes AB#3168.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PolyphonyRequiem
PolyphonyRequiem merged commit 4c2c1c5 into main May 14, 2026
1 check passed
@PolyphonyRequiem
PolyphonyRequiem deleted the fix/ab-3168-feature-promotion-routing branch May 14, 2026 05:22
PolyphonyRequiem pushed a commit that referenced this pull request May 14, 2026
…B#3169)

`primary_completer` in `implement-merge-group.yaml` previously fired
`polyphony validate --event implementation_complete` + `twig state Done`
unconditionally for `primary_router.output.primary_id`. For an
**indivisible apex** (where primary_id == apex_id), that transitioned
the apex Issue from `Doing → Done` at the MG → feature merge moment,
**before** feature → main was merged. ADO then cascaded the apex's
parent items (User Story / Feature) to Done while the work was still
sitting on the feature branch.

Companion to PR #375 (AB#3168), which inserted `promote_feature_to_main`
+ `promote_feature_pr_dispatch` into apex-driver.yaml's close path so
the feature → main merge happens before `close_mark_satisfied`. With
that promotion path in place, the apex's terminal state can safely
defer to `close_mark_satisfied` (which fires `item_satisfied`).

## What this PR does

- `primary_completer`: capture `$rootId` from `workflow.input.root_id`,
  compute `$isApexRoot = ($taskId -eq $rootId)`, then split the body:
  - **Apex root branch**: skip `polyphony validate --event implementation_complete`
    and `twig state Done`. Only add an explanatory note + sync. Output
    includes `deferred_apex_root: true` so observability is preserved.
  - **Child task branch**: unchanged — `validate` then `twig state Done`,
    matching the pre-existing rubber-duck #7 acknowledgement that child
    Tasks transition at MG→feature merge time.
- Updated `description:` on the agent (with quoting to keep the colon
  inside the parenthetical out of YAML mapping scope).
- 5 new structural Pester tests in `lint-implement-merge-group.Tests.ps1`
  asserting: rootId capture, isApexRoot discriminator, if/else partition
  of the apex-root vs child-task code paths, `deferred_apex_root` flag in
  the JSON output, and AB#3169-referencing note text in the apex-root
  branch.

## Tests

- `lint-implement-merge-group.Tests.ps1`: **38/38 pass** (was 33/33 +
  5 new).
- Full `.conductor/registry/tests` suite: **391/393 pass**. Two failures
  in `lint-plan-level.Tests.ps1` "Policy wiring checks" are pre-existing
  on `main` (stash-confirmed unrelated; out of scope for this PR).
- `conductor validate implement-merge-group.yaml`: Successful.
- `lint-strict-undefined`, `lint-pwsh-jinja-bareword`,
  `lint-sync-after-mutation`: green.

## Why apex-root only

For child Tasks inside a multi-task MG, the existing rubber-duck #7
acknowledgement at the top of `primary_completer` explicitly accepts
the MG→feature timing for the implementation_complete event. That
behavior is unchanged. Only the indivisible-apex case (the symptom
filed in AB#3169) is carved out, because:

1. Apex Issues have a different ADO state-machine semantics
   (`item_satisfied` is the conventional terminal trigger, fired by
   `close_mark_satisfied`).
2. The apex's terminal transition cascades to its parent tree (User
   Story / Feature in the AB#3169 trace), so a premature transition
   has blast radius beyond the apex itself.

For child Tasks, no such cascade happens — they're leaves.

## How AB#3168 + AB#3169 interact

Together they close the gap that AB#3169 surfaced:

- AB#3168 (PR #375, merged): apex-driver.yaml now routes through
  `promote_feature_to_main` → `promote_feature_pr_dispatch` →
  `close_mark_satisfied`. Feature → main is merged BEFORE
  `close_mark_satisfied` fires `item_satisfied`.
- AB#3169 (this PR): primary_completer no longer prematurely transitions
  the apex Issue at MG→feature merge. The Issue stays in `Doing` until
  `close_mark_satisfied` runs `item_satisfied` after promotion.

Result: apex Issue (and its parent cascade) only goes Done after the
work is actually on `main`.

Closes AB#3169.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 14, 2026
…B#3169) (#376)

`primary_completer` in `implement-merge-group.yaml` previously fired
`polyphony validate --event implementation_complete` + `twig state Done`
unconditionally for `primary_router.output.primary_id`. For an
**indivisible apex** (where primary_id == apex_id), that transitioned
the apex Issue from `Doing → Done` at the MG → feature merge moment,
**before** feature → main was merged. ADO then cascaded the apex's
parent items (User Story / Feature) to Done while the work was still
sitting on the feature branch.

Companion to PR #375 (AB#3168), which inserted `promote_feature_to_main`
+ `promote_feature_pr_dispatch` into apex-driver.yaml's close path so
the feature → main merge happens before `close_mark_satisfied`. With
that promotion path in place, the apex's terminal state can safely
defer to `close_mark_satisfied` (which fires `item_satisfied`).

## What this PR does

- `primary_completer`: capture `$rootId` from `workflow.input.root_id`,
  compute `$isApexRoot = ($taskId -eq $rootId)`, then split the body:
  - **Apex root branch**: skip `polyphony validate --event implementation_complete`
    and `twig state Done`. Only add an explanatory note + sync. Output
    includes `deferred_apex_root: true` so observability is preserved.
  - **Child task branch**: unchanged — `validate` then `twig state Done`,
    matching the pre-existing rubber-duck #7 acknowledgement that child
    Tasks transition at MG→feature merge time.
- Updated `description:` on the agent (with quoting to keep the colon
  inside the parenthetical out of YAML mapping scope).
- 5 new structural Pester tests in `lint-implement-merge-group.Tests.ps1`
  asserting: rootId capture, isApexRoot discriminator, if/else partition
  of the apex-root vs child-task code paths, `deferred_apex_root` flag in
  the JSON output, and AB#3169-referencing note text in the apex-root
  branch.

## Tests

- `lint-implement-merge-group.Tests.ps1`: **38/38 pass** (was 33/33 +
  5 new).
- Full `.conductor/registry/tests` suite: **391/393 pass**. Two failures
  in `lint-plan-level.Tests.ps1` "Policy wiring checks" are pre-existing
  on `main` (stash-confirmed unrelated; out of scope for this PR).
- `conductor validate implement-merge-group.yaml`: Successful.
- `lint-strict-undefined`, `lint-pwsh-jinja-bareword`,
  `lint-sync-after-mutation`: green.

## Why apex-root only

For child Tasks inside a multi-task MG, the existing rubber-duck #7
acknowledgement at the top of `primary_completer` explicitly accepts
the MG→feature timing for the implementation_complete event. That
behavior is unchanged. Only the indivisible-apex case (the symptom
filed in AB#3169) is carved out, because:

1. Apex Issues have a different ADO state-machine semantics
   (`item_satisfied` is the conventional terminal trigger, fired by
   `close_mark_satisfied`).
2. The apex's terminal transition cascades to its parent tree (User
   Story / Feature in the AB#3169 trace), so a premature transition
   has blast radius beyond the apex itself.

For child Tasks, no such cascade happens — they're leaves.

## How AB#3168 + AB#3169 interact

Together they close the gap that AB#3169 surfaced:

- AB#3168 (PR #375, merged): apex-driver.yaml now routes through
  `promote_feature_to_main` → `promote_feature_pr_dispatch` →
  `close_mark_satisfied`. Feature → main is merged BEFORE
  `close_mark_satisfied` fires `item_satisfied`.
- AB#3169 (this PR): primary_completer no longer prematurely transitions
  the apex Issue at MG→feature merge. The Issue stays in `Doing` until
  `close_mark_satisfied` runs `item_satisfied` after promotion.

Result: apex Issue (and its parent cascade) only goes Done after the
work is actually on `main`.

Closes AB#3169.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant