v2.4.1: Permissive ADO approval aggregation (allow_any_approval_vote) - #450
Merged
Conversation
Fix the dogfood blocker where ADO PRs polyphony opens have no required
reviewers configured, so AdoClient.AggregateReviewDecision falls through
to REVIEW_REQUIRED on any reviewer vote — the workflow then loops
pending_review_gate forever despite the gate prompt promising "Approve
(+5 or +10) — workflow merges next poll."
Add an opt-in permissive aggregation mode:
if (anyRejection) return "REJECTED"; // unchanged
if (requiredCount > 0 && requiredApproved == requiredCount) return "APPROVED";
if (allowAnyApprovalVote && anyApprovalVote) return "APPROVED"; // NEW
return "REVIEW_REQUIRED";
Rejection (-10) still shortcircuits — operators retain a hard veto.
Default is false, preserving strict ADO branch-policy semantics for
all consumers that have not opted in. The cloudvault dogfood writes
pr.defaults.allow_any_approval_vote: true to its policy.yaml.
Threaded:
- IAdoClient: new GetPullRequestPollDataAsync overload with bool flag
(default-interface-method delegates to strict overload — zero
blast radius on the 19 FakeAdoClients across the suite).
- polyphony pr poll-status-ado: new --allow-any-approval-vote CLI flag.
- Pr.Defaults.AllowAnyApprovalVote: new policy field with extensive
XML doc covering the stale-approval caveat (ADO does NOT invalidate
votes on new commits, unlike GitHub dismiss_stale_reviews).
- plan-level.yaml: pr_approval_policy_ado resolver step before
poll_status_ado; pending_review_gate prompt rewritten with
conditional permissive/strict messaging.
Tests: 7 new AggregateReviewDecision cases (strict + permissive),
2 PollStatusAdo flag-threading cases, 2 PolicyResolver
allow_any_approval_vote cases, golden-test update for the new verb
input. Lint fixture surgically extended with the new input + the new
ResolvedRule field (no blind regeneration — preserves Move #2
hand-curation per fixture convention).
Version bump 2.4.0 -> 2.4.1 across all 14 workflow YAMLs +
.conductor/registry/index.yaml versions arrays.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three Pester assertions hard-coded min_polyphony_version='2.4.0' for apex-driver/apex-wave-dispatch/apex-item-dispatch — broke when v2.4.1 landed in the YAMLs. Bump to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ADO PRs that polyphony opens (plan PR, feature PR) have no required-reviewer branch policy configured.
AdoClient.AggregateReviewDecisiononly returnsAPPROVEDwhenrequiredCount > 0 && requiredApproved == requiredCount— so even a+10vote from the operator falls through toREVIEW_REQUIRED, the workflow loops back topending_review_gate, and the apex item is un-mergeable through the normal flow.Observed live on apex 62286666 — operator clicks Approve (+10) in the PR UI, clicks Continue on the gate, the next poll returns
state: pending,reasons: ["state is pending"], the gate fires again. The advertised escape ("Approve (+5 or +10) — workflow merges next poll") is a promise the logic cannot keep.Fix
Add an opt-in permissive aggregation mode gated by a new CLI flag, fed from a new policy field.
\\csharp
if (anyRejection) return "REJECTED"; // unchanged
if (requiredCount > 0 && requiredApproved == requiredCount) return "APPROVED";
if (allowAnyApprovalVote && anyApprovalVote) return "APPROVED"; // NEW
return "REVIEW_REQUIRED";
\\
-10) still shortcircuits — operators retain a hard veto.false— strict ADO branch-policy semantics preserved for all consumers that have not opted in.pr.defaults.allow_any_approval_vote: trueto its.polyphony-config/policy.yaml.Scope
In scope:
plan-level.yamlpoll_status_ado. Other PR kinds (impl, feature, evidence viaado-pr.yaml) continue using strict aggregation regardless of the flag — PR-kind-aware schema (pr.allow_any_approval_vote.by_kind) is the proper long-term fix and is deferred.Stale-approval caveat
ADO does NOT auto-invalidate reviewer votes on new commits (unlike GitHub
dismiss_stale_reviews). With this flag enabled, an approval cast before a force push will still count after the push. Documented in the policy field XML doc and the gate prompt. SHA-bound approval semantics are tracked under AB#3104 PR2.Wiring
IAdoClient— newGetPullRequestPollDataAsyncoverload with the bool flag; default-interface-method delegates to the strict overload so the 19FakeAdoClients across the suite don't break.polyphony pr poll-status-ado— new--allow-any-approval-voteCLI flag.Pr.Defaults.AllowAnyApprovalVote— new policy field with extensive XML doc covering the stale-approval caveat.plan-level.yaml— newpr_approval_policy_adoresolver step beforepoll_status_ado;pending_review_gateprompt rewritten with conditional permissive/strict messaging.Tests
AggregateReviewDecisioncases (strict-mode regression + permissive-mode behaviour).PollStatusAdoflag-threading cases (default false + opt-in true).PolicyResolverallow_any_approval_votecases (default false / dogfood override).tests/lint/fixtures/verb-output-schemas.json) surgically extended with the new input + the newResolvedRulefield — no blind regeneration; preserves Move feat: Phase Detection, Routing Engine, and Command Implementations (PG-3) AB#2593 AB#2594 #2 hand-curation per the fixture convention.Version
Patch bump 2.4.0 → 2.4.1 across all 14 workflow YAMLs + the
index.yamlversions arrays.min_polyphony_version: 2.4.1inplan-level.yamlso older binaries fail preflight loudly if they encounter the new flag.Verification
Local:
dotnet test3697 passed (4 flakes in cross-process tests passed on isolated re-run), lint-jinja-resolver + lint-version-drift Pester suites 46/46 green.Dogfood: tag v2.4.1, install via
~/.polyphony/bin/install.ps1, writepr.defaults.allow_any_approval_vote: trueto cloudvault, click Continue on the still-open gate of apex 62286666 — expectstate=approved→route=merge_now→ merges.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com