Skip to content

Feature/2582 pg 4 - #7

Merged
PolyphonyRequiem merged 6 commits into
mainfrom
feature/2582-pg-4
Apr 30, 2026
Merged

Feature/2582 pg 4#7
PolyphonyRequiem merged 6 commits into
mainfrom
feature/2582-pg-4

Conversation

@PolyphonyRequiem

Copy link
Copy Markdown
Owner

No description provided.

Daniel Green and others added 5 commits April 30, 2026 09:00
Add CI lint script that scans scripts/ for hardcoded ADO work-item
type-name literals (Epic, Issue, Task, User Story, Bug, Feature)
to enforce P5 type-agnostic compliance.

- Case-sensitive Select-String with word-boundary matching
- Excludes .Tests.ps1 fixtures and comment-only lines
- Exits 1 with actionable output when violations found
- 13 Pester tests covering clean/violation/edge cases

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Create scripts/lib/ado-helpers.ps1 with Get-AdoOrg, Get-AdoProject,
and Get-AdoWorkspace functions that read values from twig config at
runtime. All five production scripts now dot-source the helper and
emit ado_workspace in their JSON output, ensuring no hardcoded ADO
org/project references exist.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ct-state.ps1 AB#2653

Final type literal audit across all scripts/ found 1 remaining violation:
detect-state.ps1:84 used 'Issue' in a legacy table metadata regex pattern.

Replaced type-specific '\|\s*\*{0,2}Issue\*{0,2}\s*\|\s*#(\d+)' with
type-agnostic '\|\s*\*{0,2}[^|*]+\*{0,2}\s*\|\s*#(\d+)' to match any
work item type label in legacy plan file metadata tables.

Audit results: 0 violations across 9 production scripts (lint passes).
Test files contain only legitimate lint guard assertions and test data.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… AB#2650

Extract repo slug derivation to scripts/lib/gh-helpers.ps1 with a
Get-RepoSlug function using consistent regex capture pattern. Update
detect-state.ps1, load-work-tree.ps1, and pg-router.ps1 to use the
shared helper instead of inline derivation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Audit all scripts/*.ps1 for -relative dot-source paths and
absence of hardcoded absolute paths. All 10 production scripts are
already compliant. Add tests/lint-psscriptroot-paths.ps1 CI lint with
13 Pester tests to enforce the invariant going forward.

Checks:
- Dot-source statements without \
- Hardcoded Windows drive paths (C:\...)
- UNC paths (\\\\server\\...)
- Unix absolute paths (/home/, /Users/, etc.)
- Excludes .Tests.ps1 fixtures, comment lines, regex escapes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PolyphonyRequiem

Copy link
Copy Markdown
Owner Author

@copilot resolve the merge conflicts in this pull request

Co-authored-by: PolyphonyRequiem <3107779+PolyphonyRequiem@users.noreply.github.com>

Copilot AI commented Apr 30, 2026

Copy link
Copy Markdown

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in commit 4748dce. The conflicts were modify/delete conflicts on .twig/ files — resolved by accepting the deletions, consistent with the .twig/ entry in .gitignore that was added on main.

@PolyphonyRequiem
PolyphonyRequiem merged commit a0568e0 into main Apr 30, 2026
@PolyphonyRequiem
PolyphonyRequiem deleted the feature/2582-pg-4 branch April 30, 2026 17:00
Copilot stopped work on behalf of PolyphonyRequiem due to an error April 30, 2026 17:00
PolyphonyRequiem added a commit that referenced this pull request May 7, 2026
GitHub-only verb that opens (or reuses) the PR promoting an evidence
branch into its parent feature branch (or main for the orphan-evidence
case where no apex is supplied). Mirrors pr create-feature-pr structure
(twig-driven title) and pr open-mg-pr (head/base validation, idempotent
reuse, GhClientPolicy-inherited retry/timeout/reconcile).

Verb signature:
  polyphony pr open-evidence-pr <work-item-id>
    [--apex-id N] [--head ref] [--base-branch ref]
    [--title text] [--body text]

Branch naming defaults:
  apex provided  -> head=evidence/<apex>-<wi>, base=feature/<apex>
  orphan/collapsed -> head=evidence/<wi>, base=main

ADO sibling deferred per session policy (GitHub-only PR-openers in
Phase 6). Workflow consumption (PR #4) and evidence floor check
(PR #7) land separately.

Tests: 15 new (2583 total). Covers argument validation, naming
defaults, override knobs, twig fallback, idempotent reuse, error
envelopes (missing branches, no slug, gh non-zero), inherited
gh-create timeout-with-empty-reconcile path, and snake_case JSON
contract.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 7, 2026
Replace BFS-by-depth wave logic in 'polyphony worklist build' with the EdgeGraph.ToWaves + ExecutionModeInjector composition, so worklist generation goes through the same wave-derivation engine as 'edges check' and the apex driver. Hard cutover, no opt-in flag.

WorklistResult gains required fields items_walked, has_conflicts, conflicts (reusing EdgesCheckConflict shape). Always present, including on error envelopes. waves is empty when has_conflicts is true. Verb signature unchanged.

Missing root now errors with root_not_found instead of emitting plan_status: unknown and walking — required by EdgeGraph composition needing the type from process config. Consistent with 'edges check'.

Tests: 2707/2707 pass.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem pushed a commit that referenced this pull request May 7, 2026
#4)

Wires the Phase 6 evidence verbs (branch ensure-evidence-branch from
PR #132 / e63d84f, pr open-evidence-pr from PR #130 / 97bd373) into a
runnable actionable.yaml conductor workflow. Routes between a polyphony
executor (full evidence chain: branch -> agent -> PR -> review -> merge)
and a human executor (satisfaction gate only) via an in-workflow
executor_router script.

Includes:
- .conductor/registry/workflows/actionable.yaml (~700 lines)
- .conductor/registry/scripts/route-actionable-executor.ps1 + 6 xUnit tests
- .conductor/registry/tests/lint-actionable.{ps1,Tests.ps1} (12 checks, 21 tests)
- .conductor/registry/index.yaml entry at 1.0.0
- docs/decisions/actionable-executor-split.md (ADR)
- docs/glossary.md "Workflows" section (3 new entries)
- .github/skills/polyphony-actionable/SKILL.md

Three deferred-wiring slots are marked with TODO(p6-pr5/7/8) markers
and pinned by lint-actionable.ps1: facet-profile composition (PR #5),
evidence floor check (PR #7), full evidence_reviewer rubric (PR #8).

Validation:
- dotnet build clean
- dotnet test 2705/2705 passing (baseline 2699 + 6 new)
- Pester registry 122/122 across 9 lint suites
- Pester root 71/71 across 4 lint suites

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 7, 2026
#4) (#136)

Wires the Phase 6 evidence verbs (branch ensure-evidence-branch from
PR #132 / e63d84f, pr open-evidence-pr from PR #130 / 97bd373) into a
runnable actionable.yaml conductor workflow. Routes between a polyphony
executor (full evidence chain: branch -> agent -> PR -> review -> merge)
and a human executor (satisfaction gate only) via an in-workflow
executor_router script.

Includes:
- .conductor/registry/workflows/actionable.yaml (~700 lines)
- .conductor/registry/scripts/route-actionable-executor.ps1 + 6 xUnit tests
- .conductor/registry/tests/lint-actionable.{ps1,Tests.ps1} (12 checks, 21 tests)
- .conductor/registry/index.yaml entry at 1.0.0
- docs/decisions/actionable-executor-split.md (ADR)
- docs/glossary.md "Workflows" section (3 new entries)
- .github/skills/polyphony-actionable/SKILL.md

Three deferred-wiring slots are marked with TODO(p6-pr5/7/8) markers
and pinned by lint-actionable.ps1: facet-profile composition (PR #5),
evidence floor check (PR #7), full evidence_reviewer rubric (PR #8).

Validation:
- dotnet build clean
- dotnet test 2705/2705 passing (baseline 2699 + 6 new)
- Pester registry 122/122 across 9 lint suites
- Pester root 71/71 across 4 lint suites

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 7, 2026
Add a strict mechanical pre-reviewer gate for actionable evidence PRs.
The new `polyphony pr check-evidence-floor <pr-number>` verb verifies
the PR has at least one commit on its head branch beyond base AND a
non-empty body (after whitespace trim). The actionable workflow gains
two new nodes - `evidence_floor_check` (script) and `floor_failed_gate`
(human gate) - wired between `open_evidence_pr` and `evidence_reviewer`.

Per the Phase 6 design sketch (PICK #6), the floor is mechanical only:
content quality remains the LLM reviewer's exclusive judgment. The
verb is routing-style (always exits 0); outcomes are conveyed via
`passes_floor`/`violations` (pass/violation) or `error_code`
(`pr_not_found`/`gh_failed`).

Updates `lint-actionable.ps1` to drop `TODO(p6-pr7)` from the
deferred-wiring marker set, add an inverse "shipped TODO must be
absent" check, register `evidence_floor_check` + `floor_failed_gate`
as required nodes, and add `pr check-evidence-floor` to the
evidence-verb invocation set.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 7, 2026
…d) (#169)

Surfaced live by the apex #3043 dogfood at 16:22Z (post-#168 relaunch):

  Agent open_questions_counter failed: 'severities_at_or_above' is undefined

`plan-level.yaml` referenced `severities_at_or_above(...)` as a Jinja
function call (lines 625 + 641); conductor 0.1.14 has no such Jinja
extension registered. Two-vector failure mode — both the warning-mode
route and the gate-prompt template detonated.

Worse: `lint-plan-level.ps1` Check 7 *required* the function-call form,
so the lint and the workflow agreed on a contract conductor never
honored. Same root-cause family as bug #6 (Jinja field references go
un-checked at lint time), different angle: lint contracts that aren't
grounded in conductor's actual Jinja environment are equally vulnerable.

Fix (Option A — recommended in catalog post-#168): make
`polyphony policy resolve --domain open_questions` precompute the
ascending severity list from `min_severity` and emit it as a wire-level
`severities_at_or_above` field. Workflow and lint then agree with
conductor (field reference, not function call).

Changes:

- src/Polyphony/Policy/PolicyResolver.cs:
  - `ResolvedRule.SeveritiesAtOrAbove: List<string>?` with
    `[JsonPropertyName("severities_at_or_above")]`.
  - Computed from `MinSeverity` via `Enum.GetValues<Severity>()` filtered
    `>= threshold`, ToString().ToLowerInvariant(). Null when MinSeverity
    is null (non-OpenQuestions domains stay shape-clean).
- .conductor/registry/workflows/plan-level.yaml:
  - Lines 625, 641: `severities_at_or_above(open_questions_policy.output.min_severity)`
    → `open_questions_policy.output.severities_at_or_above`.
- .conductor/registry/tests/lint-plan-level.ps1:
  - Check 7 inverted: now requires the field reference, rejects the
    function-call form with rule `severities-at-or-above-as-function`.
  - Check 8 detail message updated to reference the new field.
- .conductor/registry/tests/lint-plan-level.Tests.ps1:
  - 7 synthetic-fixture occurrences of the legacy form swept to the
    new field reference.
  - New "severities_at_or_above field reference" Context with 4 tests
    (production yaml uses field, production yaml has no function call,
    lint fails on legacy form, lint fails on missing reference).
- tests/Polyphony.Tests/Commands/PolicyCommandsTests.cs:
  - Existing OpenQuestions resolve tests assert SeveritiesAtOrAbove
    contents (default scope: moderate/major/critical; root override
    critical: just critical; by-type fallback low: all four).
  - New Resolve_OpenQuestions_SeveritiesAtOrAbove_IsAscending pinning
    `["major","critical"]` for `min_severity: major` plus wire-format
    assertion.
  - Approvals/PR shape regression tests extended: the new field is
    null on non-OpenQuestions domains, omitted from JSON.
- docs/polyphony-state-effects-catalog.md:
  - New `polyphony policy resolve` verb entry with
    severity-list gotcha + bug #7 history.
  - New "Lint can enforce a Jinja contract conductor doesn't honor"
    open-question entry pointing at #163.

Verification:

- Build clean (0 warnings, 0 errors).
- xUnit: 2431/2431 (was 2431; +1 new test, modified 5 existing
  assertions in-place — net +1 net new test count expected when
  CI runs with the fresh assemblies).
- Pester (lint-plan-level.Tests.ps1): 23/23 (was 19; +4 new).
- All 14 conductor lints pass; conductor validate clean for all 14
  registered workflows.
- Live smoke: `polyphony policy resolve --domain open_questions
  --scope default` now emits
  `"severities_at_or_above":["moderate","major","critical"]`.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 8, 2026
…r) (#171)

PR #170 introduced `default(0, true)` (the standard Jinja two-arg form)
on the assumption that bare `default(0)` would not coerce JSON null →
0. Iter 5 of the apex #3043 dogfood crashed in the same step
(ensure_plan_branch) with:

  TemplateError: TemplateRenderer._default_filter() takes from 1 to 2
  positional arguments but 3 were given

Investigation: conductor registers a CUSTOM `default` filter at
`conductor/executor/template.py:67` whose signature is
`_default_filter(value: Any, default: Any = "") -> Any` and whose body
returns `default` when `value is None`. This is BETTER than standard
Jinja — it handles BOTH Undefined and None without needing the
`boolean=True` second arg. But it accepts only TWO positional args, so
the standard 3-arg form crashes it.

So bare `default(0)` was always correct for conductor; the wire-shape
fix from PR #170 (always-emit `parent_item_id`) was the only thing we
needed. The two-arg "extra safety" change broke a contract conductor
defines differently than standard Jinja — same shape of bug as #7.

Reverts and pivots:

- .conductor/registry/workflows/plan-level.yaml: 5 references to
  `parent_item_id | default(0, true)` reverted to bare
  `parent_item_id | default(0)`.
- .conductor/registry/tests/lint-plan-level.ps1 Check 25: inverted —
  now refuses the multi-arg `default(...)` form (rule
  `parent-item-id-multi-arg-default`) instead of requiring it. Comment
  block expanded to capture the conductor-filter intel.
- .conductor/registry/tests/lint-plan-level.Tests.ps1: regression
  Context inverted — asserts the production yaml uses bare `default(0)`
  for every `parent_item_id` reference (none use the multi-arg form),
  and asserts the lint fires when mutated to the multi-arg form.
- docs/polyphony-state-effects-catalog.md: `polyphony plan
  derive-ancestor-chain` verb entry updated. Wire-shape gotcha
  expanded with a "Filter form" subsection capturing conductor's
  custom `_default_filter` semantics (handles Undefined AND None;
  crashes on 3 positional args). Bug-#8 history records both
  iterations.

Verification:

- xUnit wire-shape tests from PR #170 still pass (no model changes).
- Pester lint-plan-level: 25/25 (regression Context inverted but still
  +2 over pre-PR-170 baseline).
- Production lint-plan-level.ps1: PASS.
- conductor validate (all 14 workflows): PASS.
- Type-agnostic lint: PASS (48 files, surface=all).
- The dogfood iter-5 crash mode is now the lint regression target.

Future remediation (deferred, not blocking):
- Conductor's custom `_default_filter` could be extended to accept the
  3-arg standard Jinja signature for compatibility — but its current
  semantics already exceed standard Jinja's, so the conductor-side
  filter behavior should be the contract polyphony plans against.
- A "registry of conductor-honored Jinja filters/functions"
  (mentioned in the catalog gap entry from PR #169) would catch this
  class of bug at lint time across all workflows, not just
  plan-level.yaml.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 8, 2026
#216)

Replaces the broken �ny non-Done child' heuristic for children_seeded' with the canonical polyphony:planned' tag-presence check, via PlanObserver.IsParentSeededAsync (the PR #1 primitive). The new semantics correctly report:

- Tag present + children -> Satisfied

- Tag present + zero children (decomposable but indivisible' from PR #7 / closed-loop-state-plan.md section 3.4) -> Satisfied

- Tag absent (regardless of child count) -> Needed

- Twig read failure -> Needed with a non-empty diagnostic reason; no exception escapes the verb

Implementation follows the 4-step plug-in recipe documented on NextReadyObservationScope (added in PR #2):

1. Two new fields on the scope (PlannedTagPresent, PlannedTagFetchError).

2. FetchPlannedTagAsync called from BuildObservationScopeAsync up-front so children_seeded is observed even on plan-branch / slug short-circuit paths.

3. Static composer ComposeChildrenSeeded mirrors ComposePlanAuthored / ComposePlanReviewed / ComposePlanPromoted.

4. Wired into ComputeObservedAsync alongside the plan-kind composers.

Per the recipe, no new Observer class file was added (PR #1 already lifted MapChildrenSeeded onto PlanObserver). This is the deliberate deviation from the original plan section 4 row 3 wording (ChildrenSeededObserver.cs') in favor of the actual scaffolding pattern PR #2 produced.

Tests: new tests/Polyphony.Tests/Commands/StateNextReadyChildrenSeededTests.cs covers the four scenarios from the spec (tag absent regardless of child count, tag present with children, tag present with zero children, twig error and twig throw paths). Test counts: 3073 -> 3079 (+6, all green).

PR #4 (ImplementationObserver) follows next.

Refs files/closed-loop-state-plan.md section 3.1 row 4 + section 4 PR #3.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 8, 2026
…ing (closes #215) (#218)

Builds the cross-item rollup into `state next-ready` so per-item
dispositions reflect children's terminal state, and threads the
`polyphony:facets=...` tag (PR #7) through `RequirementInputResolver`
so non-decomposed apexes are observed against their declared facets.
Together these close the loop on closed-loop §3.1 rows 5-6 and §4 PR #5.

Cross-item rollup (rows 5-6):
- New `BuildChildRollupSnapshotsAsync` walks an item's children,
  composing each child's reduced `implementation_merged` and
  `item_satisfied` dispositions into the parent's observation scope.
- `ComposeImplementationMerged` takes the worst-of (parent self,
  every implementable child) so an MG with a merged self-PR is NOT
  Satisfied while any child PR is unmerged or absent. The reason
  string surfaces the offending child's id so callers can drill down
  without re-deriving the rollup.
- `ApplyChildItemSatisfiedRollup` mirrors the same posture for
  `item_satisfied`; cycles in the child cache degrade gracefully via
  `snap.Error` rather than throwing.
- Recursion is depth-capped (default 5, override via
  `POLYPHONY_NEXTREADY_ROLLUP_DEPTH`) — the budget is per-frame and
  the truncated frame demotes its own composer to Needed so the cap
  is observable upstream via the worst-of cascade.

apex_facets threading:
- `ExtractFacetOverride` parses the `polyphony:facets=<csv>` tag into
  a facet list via `FacetTagParser.TryExtract`, then passes it as
  `overrideFacets` to `RequirementInputResolver.Resolve`. Non-
  decomposed apexes derive against the declared subset; the facet
  set in the result envelope reflects the override.
- Malformed tag (unknown facet) routes through `EmitNextReadyError`
  → `ExitCodes.ConfigError` with the offending token in the error
  string, matching the routing-style verb contract.

Tests (13 new, all passing on top of the 3085 baseline):
- StateNextReadyCrossItemRollupTests (8): both-merged Satisfied rollup,
  one-child-no-PR demotion, three-mixed-children worst-of, three-level
  chain, default-depth truncation via demoted disposition, env-var
  shrinks cap, env-var raises cap (no truncation pair-test), cycle.
- StateNextReadyImplementationTests (2 new on top of 6): MG + 2
  children all merged → impl_merged Satisfied; MG + 2 children one
  open → not Satisfied + open child id in reason.
- StateNextReadyApexFacetsTests (3 new): tag narrows requirement set
  to declared facets, no-tag baseline falls back to type-config
  default, malformed-tag emits error envelope + ConfigError exit.

Hand-off from PR #4 (#217) — implementation_merged is now the
joint-responsibility kind across parent + descendants. Closes #215.

Co-authored-by: Daniel Green <dangreen@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PolyphonyRequiem added a commit that referenced this pull request May 12, 2026
)

Adds the canonical revise-counter / cap-gate / reset trio around the
scope_reviewer -> primary_router edge so that a structurally broken
MG (empty branch, zero implementable items, reviewer correctly emits
'changes_requested' per its empty-MG check) escalates to a human gate
after MAX_REVISIONS=3 cycles instead of looping forever.

Mirrors the pattern from github-pr.yaml (review_counter +
pr_fix_exhausted_gate + review_counter_reset) and plan-level.yaml
(revise_counter + revise_cap_gate). The MG-scoped counter file is
keyed on work_item_id + mg_path so parallel sibling MGs under the
same parent do not clobber each other's iteration state.

Repro evidence:
  apex AB#3107 (dogfood Run #7), 2026-05-12 14:12-14:20 UTC. Two
  parallel MGs (dispatch_items[1] and [2]) entered the loop. JSONL
  events at:
  C:\Users\dangreen\AppData\Local\Temp\conductor\conductor-apex-driver-20260512-125014-1c5bcde7.events.jsonl
  Each cycle: scope_reviewer (~30s, Opus-4.7) -> primary_router ->
  scope_reviewer. Manual kill after ~10 minutes burn rate.

Routing changes:
  scope_reviewer.routes:
    'changes_requested' route now targets scope_revise_counter (was
    primary_router).
    Catch-all (unknown verdict) now targets scope_revise_counter (was
    primary_router) so malformed verdicts cannot bypass the cap.

  scope_revise_counter (new, script):
    Persists iteration in TempPath. Routes to scope_revise_cap_gate
    when cap_reached, else primary_router. Catch-all targets the gate
    defensively.

  scope_revise_cap_gate (new, human_gate):
    Three options matching pr_fix_exhausted_gate shape:
      re_loop      -> scope_revise_reset (resume with fresh budget)
      force_accept -> pr_platform_router (bypass scope review,
                       proceed to MG PR open)
      abort        -> \

  scope_revise_reset (new, script):
    Resets the counter file to 0; routes to primary_router.

Lint additions (Check 18):
  - Asserts the three new agents exist.
  - Asserts cap_gate has options re_loop / force_accept / abort.
  - Asserts scope_revise_counter routes on output.cap_reached (M2).
  - Asserts scope_reviewer routes through scope_revise_counter, not
    directly to primary_router.

Test fixture in lint-implement-merge-group.Tests.ps1 updated to
include the new structure. Five new mutation tests cover each Check 18
violation. 33 of 33 implement-merge-group lint Pester tests pass.

Sibling concerns NOT addressed in this PR (recommend separate Issues):

  1. The per-task primary_reviewer -> coder loop has the same shape
     (no cap-gate) and the same potential for runaway. Fix would
     mirror this PR but with a primary_revise_counter / cap_gate /
     reset trio inserted at scope_reviewer's per-task analogue.
     Less urgent because primary_reviewer runs Opus-4.6 (cheaper)
     and is bounded in practice by the task list, but still a
     theoretical infinite loop on malformed primary_reviewer output.

  2. Counter file staleness across runs: the counter file at
     [System.IO.Path]::GetTempPath() persists between conductor
     runs. A successful run that took 2 revisions leaves stale
     state '2' that can cause the next run to hit the cap early.
     Same gap exists in the canonical github-pr.yaml review_counter
     and plan-level.yaml revise_counter. Class-of-bug fix would key
     the file on conductor run id (not currently exposed in the
     template surface).

  3. Verdict taxonomy ('structurally_broken' vs 'changes_requested')
     deferred. The reviewer prompt could distinguish 'salvageable
     changes requested' from 'structurally broken, do not loop',
     and the latter could route directly to the cap gate. That is
     a prompt + schema change, not a routing change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
PolyphonyRequiem added a commit that referenced this pull request May 20, 2026
…494)

First pass of the workflow-comments cleanup sweep. Targets two narrow,
verifiable categories of staleness — duplicate-paste and "deferred /
shipped in this PR" framing that no longer reflects what is on main.

implement-merge-group.yaml:

  1. Header lines 1-10 contained a 5-line copy-paste duplicate of the
     opening summary. Removed lines 6-10.

  2. Item 4 of "Transitional notes (TODO follow-up PRs)" claimed the
     ADO leg uses a human-gate stub pending the ADO MG verbs. Verified
     stale: PR #427 (ADO support across polyphony, commit f9d4aef)
     wired the real pr open-mg-ado and pr merge-mg-ado verbs into
     this same workflow at lines ~2191/2209/2253. Removed item 4.

  3. Item 3 (lock + manifest delegation to apex) is the permanent
     contract that lets parallel-MG execution work — not transitional
     scaffolding. Promoted it out of "Transitional notes" into a
     "Permanent design contract" section so future readers don't try
     to "complete" it.

  Items 1-2 (PG-N legacy tag dependency in branch next-impl / route /
  close-scope) are kept — verified still valid against
  src/Polyphony/Commands/BranchCommands.NextImpl.cs (operator-facing
  --pg-name / --pg-number flags still required) and BranchCommands.cs
  (close-scope verb still in place).

actionable.yaml:

  Header read as if the workflow YAML itself were a Phase 6 PR scaffold
  — "this PR", "Wiring landed in this PR (Phase 6 PR #5)", "PR #5 will
  wire in facet-profile context...". From main, "this PR" is ambiguous
  and the forward references are a snapshot of an in-flight stack.

  Reframed the header as a "History" log + a current-state description:
    - Phase 6 PR #4 (#136) scaffolded the executor router.
    - Phase 6 PR #5 (#142) wired facet-profile composition.
    - Phase 6 PR #7 (#139) added the evidence floor check.
    - Phase 6 PR #8 (deferred): full evidence_reviewer rubric.

  Verified each shipped commit via git log on the actionable.yaml path.
  PR #8 is the only one still pending; the placeholder reviewer rubric
  + TODO(p6-pr8) marker at �vidence_reviewer are unchanged.

  Workflow description: updated to drop the "Phase 6 PR #5 wires"
  framing in favour of the present-tense state.

Verification:
  - All 12 .conductor/registry/tests/lint-*.ps1 PASS
    (lint-actionable.ps1 explicitly checks "deferred-wiring TODOs
    present, shipped TODOs removed" — its rule #11/#11b for PR #5/#7
    "TODO MUST be absent" still holds; its rule #10 for PR #8 deferred
    marker still holds because we kept the in-line TODO(p6-pr8))
  - 513/513 .conductor/registry/tests Pester cases PASS

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.

2 participants