refactor(workflows): compress pending_review_gate to emit + script-poll pattern#547
Conversation
…ll pattern Implements the gate-compression-pattern ADR (docs/decisions/gate-compression-pattern.md) for the two pending_review_gate nodes across github-pr.yaml and ado-pr.yaml. ## Gates compressed (2) - github-pr.yaml: pending_review_gate (line 1025 on main) - ado-pr.yaml: pending_review_gate (line 1146 on main) Both gates were category-1 (deterministic-outcome): the workflow was pausing to wait for a human to take an action on the PR (merge, approve, comment, or close) that Poll-PrStateDelta.ps1 can detect directly. ## Vestigial cleanup (Q5 Option A) - Removed pending_poll_counter per-file (counter logic superseded by TimeoutSeconds) - Removed pending_review_gate_policy_router per-file (skip/wait/auto modes superseded) - Rewired stuck_review_reset -> poll_pr_state_delta (router gone) - Rewired pr_pre_merge_gate.defer -> poll_pr_state_delta (gate gone) - Updated stuck_review_gate prompts to remove pending_poll_counter template refs ## Per-file changes ### github-pr.yaml - Added poll_timeout_seconds / poll_interval_seconds optional inputs (default 86400 / 30) - Added notifications: block (polyphony.github_pr namespace) - Added types: pr_review_required, pr_ci_attention_required - pr_feedback_analyzer catch-all: pending_poll_counter -> notify_pr_pending - New nodes: notify_pr_pending, poll_pr_state_delta, notify_pr_ci_attention - New node: notify_pr_review_resolved (inserted before already_merged_emitter) - Version bump: 2.4.8 -> 2.5.0 ### ado-pr.yaml - Same structure as github-pr.yaml with ADO-specific args and payload fields - poll_pr_state_delta uses -Platform ado (coords parsed from PrUrl by script) - ci_status_changed FAILURE threshold uses 'failed' (ADO build policy status) - Version bump: 2.4.8 -> 2.5.0 ## Script: Poll-PrStateDelta.ps1 - Added Liszt's implementation (scripts/Poll-PrStateDelta.ps1 + Tests) - Contract gap patched: WatermarkPath is now optional; auto-derived from PR coords (platform+owner+repo+number) when not supplied by caller. This aligns with Wagner's plan §3.2 and allows the YAML to omit -WatermarkPath. - Tests (Poll-PrStateDelta.Tests.ps1) included as-is; AST-clean per Liszt. Full test execution is follow-up work. ## Script contract gaps noted (for follow-up) - reaction_kind enum in script differs from Wagner's plan: pr_merged/pr_closed (not merged/closed), ci_status_changed (not ci_changed), new_review_approved/changes_requested/commented (not new_review). YAML routing uses Liszt's actual values throughout. - initial_observation is a new kind (first call, no watermark); YAML routes it back to poll_pr_state_delta to re-poll immediately. - on_error: -> poll_error_gate deferred to AB#3257 (conductor RFC Phase 2). ## ADRs added - docs/decisions/gate-compression-pattern.md (Bach) - docs/decisions/domain-signal-envelope.md (Bach, dependency of above) - docs/decisions/polyphony-verb-error-boundary.md (Bach, dependency of above) - docs/north-star.md (Bach) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…stub
Five pre-existing bugs surfaced when running Pester locally:
1. Script: Invoke-AssertCli referenced `.Exe`, a property that
doesn't exist on the returned object. Under Set-StrictMode -Version
Latest this throws PropertyNotFoundException before the auth / 404 /
rate-limit catch chain runs, collapsing 401/404/429 all to exit
code 5. Fixed by removing the redundant `-not `.Exe -and`
guard (ExitCode -eq -1 plus the 'not found on PATH' regex already
identify this case unambiguously).
2. Test: Build-GhStub's PR-core pattern 'api repos/*/pulls/* --jq *'
also matched '/pulls/{n}/reviews' because '*' greedily eats the URL
tail. Tightened to '--jq {*' so the brace discriminates against
the reviews/comments/CI paths (whose jq filters start with '[').
3. Script: Find-GitHubDeltas / Find-AdoDeltas return a typed
List[hashtable] via the PowerShell pipeline. An empty list yields
``; a single-item list yields that one hashtable — neither has
a reliable .Count. Wrapped the call-site assignment in `@()` so
`` is always a plain array.
4. Script: Select-HighestPrecedenceDelta declared its parameter as
[System.Collections.Generic.List[hashtable]]. Under StrictMode,
passing a plain hashtable (single-delta case) triggered a
PropertyNotFoundException during argument binding. Removed the
type constraint — the function body only iterates and accesses
hashtable keys, so no type hint is needed.
5. Test: ConvertFrom-Json auto-promotes ISO-8601 date strings to
System.DateTime objects; the Should -Match regex then ran against
DateTime.ToString() which does not produce 'Zulu' notation.
Fixed by normalising to string with the expected format before
asserting in 'Always emits observed_at_utc in ISO 8601 format'.
Also bumped TimeoutSeconds to 30 for the 'pr_merged + new_commit'
precedence test whose CI-stub subprocess calls made one full poll
exceed the prior 10-second ceiling under load.
Pester now 30/30 green (1 slow-test skip intentional) on the
refactor/pr-gate-compression-v2 branch.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
📊 Pester verification — 5 pre-existing bugs caught + patched Ran
Fixed in commit b37d85f. Pester is now 30/30 green (1 intentional skip for the 60s+ hang test). — Liszt |
- Merged 2 inbox files into decisions.md (user directive on Wagner's PR-gate defaults, Wagner's post-ship notes). - Updated liszt/history.md: append note on Wagner's WatermarkPath optional patch (PR #547, poll-pr-state-delta.ps1). - Orchestration and session logs written to gitignored directories (orchestration-log/, log/). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… bugs for Daniel) - Merged .squad/decisions/inbox/ entries (Sibelius collision check + Liszt Pester verification) - Updated Wagner & Liszt history files with cross-agent verification notes - No archiving needed (all decisions <= 7 days old) - Sibelius verdict: AB#3257 covers both on_error retrofit + abandoned_on_error Phase 2 option - Liszt verdict: PR #547 Pester FAIL (22/31) — two bugs block merge until fixed Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…30 green) - Merged inbox decision: Liszt-poll-pr-state-delta-fix-shipped.md - Updated Wagner history.md with fix summary - Archived Wagner history.md to history-archive.md (15KB threshold exceeded) - No entries older than 7 days to archive from decisions.md
Summary
Compresses the two
pending_review_gatenodes ingithub-pr.yamlandado-pr.yamlfrom blockinghuman_gatenodes into an emit + script-poll pattern, following the gate-compression-pattern ADR (docs/decisions/gate-compression-pattern.md).Gates compressed: 2
Both
pending_review_gatenodes were category-1 (deterministic-outcome): the workflow was pausing to wait for a human to take an action on the PR (merge, approve, comment, close) thatPoll-PrStateDelta.ps1can detect directly. They stay compressed.Gates staying as human gates: 17
poll_error_gate,revise_cap_gate,remediation_cap_gate,stuck_review_gate,pr_pre_merge_gate,merge_failed_gateand others — all genuine judgment calls, config errors, or infrastructure failures. They are unchanged.Per-file changes
github-pr.yaml(v2.4.8 → v2.5.0)poll_timeout_seconds(default 86400 s / 24 h) andpoll_interval_seconds(default 30 s) as optional workflow inputsnotifications:block (polyphony.github_prnamespace) with typespr_review_requiredandpr_ci_attention_requiredpending_poll_counter+pending_review_gate_policy_router+pending_review_gate) with:notify_pr_pending— emitspr_review_requireddomain signal once when gate openspoll_pr_state_delta— callsPoll-PrStateDelta.ps1; blocks until reaction or timeoutnotify_pr_ci_attention— emitspr_ci_attention_requiredon CI FAILURE (Q3 Option C)notify_pr_review_resolvedbeforealready_merged_emitter(emitsdisposition:resolvedto close the pending CTA)stuck_review_gateprompt (removedpending_poll_countertemplate refs; timeout-based description)stuck_review_reset→poll_pr_state_deltapr_pre_merge_gate.defer→poll_pr_state_deltaado-pr.yaml(v2.4.8 → v2.5.0)Same pattern as GitHub with ADO-specific details:
poll_pr_state_deltauses-Platform ado; coords parsed fromPrUrlby the scriptnotify_pr_pendingpayload includesorganizationandproject'failed'(ADO build policy status string)Script:
Poll-PrStateDelta.ps1Added Liszt's implementation (was untracked). One small contract gap patched:
Reaction-kind contract note
Liszt's actual
reaction_kindenum differs slightly from Wagner's plan. The YAML routing uses Liszt's actual values:initial_observationpr_merged/pr_closednew_review_approved/new_review_changes_requested/new_review_commentednew_commit/new_commentci_status_changeddelta_details.newfor green/red routing)timeoutstuck_review_gate_policy_routeron_error: → poll_error_gateis deferred to AB#3257 (conductor RFC Phase 2 —on_error:in routes not yet shipped in v0.1.18).Pester tests for the script (
Poll-PrStateDelta.Tests.ps1) are included and were AST-clean per Liszt. Full test execution is follow-up work.ADRs added
docs/decisions/gate-compression-pattern.md— the pattern this PR implements (Bach)docs/decisions/domain-signal-envelope.md— dependency (Bach)docs/decisions/polyphony-verb-error-boundary.md— dependency (Bach)docs/north-star.md— high-level vision doc (Bach)Open follow-ups
on_error: to: poll_error_gatetopoll_pr_state_deltaonce conductor RFC Phase 2 ships27006af): bulk-renametype: notification→type: emitandnotification:→emit:in both filesPoll-PrStateDelta.Tests.ps1in CIpoll_timeout_seconds/poll_interval_secondsthroughfeature-pr.yamlandimplement-merge-group.yamlcallersCo-authored-by: Copilot 223556219+Copilot@users.noreply.github.com