fix(plan-level): thread ADO inputs into open_plan_pr_ado - #190
Merged
Conversation
The open_plan_pr_ado script step in plan-level.yaml was the only ADO verb invocation not threading --organization/--project/--repository. This caused polyphony pr open-plan-ado to fail with the ConsoleAppFramework "Required argument 'organization' was not specified" text-not-JSON error, which then cascaded to plan_reviewer crashing on undefined pr_url. Surfaced live by iter 11 of apex-driver dogfood against #3043. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 8, 2026
PolyphonyRequiem
added a commit
that referenced
this pull request
May 8, 2026
…t 2) (#197) Extends the Jinja resolver lint with cross-reference checks against the `inputs[]` registry block landed in PR #196 (CR+CRL Part 1). Closes the silent-fail signature-drift class that motivated PRs #157/#158/#159/#190 and issue #191 — polyphony exits 0 on unrecognized args (CAF behavior), conductor sees ✓, and the workflow proceeds with garbage. The lint now catches the drift at PR time. Three new diagnostic codes: - VERB001 — unknown verb (e.g. `polyphony plan does-not-exist`) - VERB002 — unknown flag (`--flag` not in the verb's `inputs[]`) - VERB003 — required input not threaded by either flag or positional The implementation models ConsoleAppFramework's mixed flag+positional binding via a two-pass algorithm (named flags consumed first since they can refer to any slot; bare positionals then bind to remaining un-threaded slots in declaration order). The first naive single-pass implementation flagged 10 false positives on the live corpus; the two-pass form reduced that to **2 real findings on first run**. The 2 findings — both production drifts — are fixed in the same PR: - `merge_plan_pr` step (plan-level.yaml line 1518) — required `int prNumber` was not threaded. - `merge_plan_pr_ado` step (plan-level.yaml line 1952) — same drift on the ADO leg. Both would have failed at runtime with CAF's "missing required argument" error. Each now threads `--pr-number` with the canonical `{{ poll_status.output.pr_number if … is defined else open_plan_pr.output.pr_number }}` pattern used elsewhere in the file. Tests: - 4 new Pester fixtures + 4 new tests (VERB001 / VERB002 / VERB003-missing / VERB003-positional-ok). Existing JINJA fixtures threaded the second positional arg required by `plan derive-ancestor-chain` to keep them realistic invocations. - 30/30 lint-jinja-resolver.Tests.ps1 pass (was 26 + 4 new). - 101/101 full Pester suite pass. - Live lint: 0 errors, 32 JINJA002 warnings (unchanged baseline), exit 0. ADR `docs/decisions/jinja-resolver-lint.md` amended with the new amendment section covering the diagnostic codes, the two-pass positional-binding algorithm, the silent-fail motivation, and the suppression key (`<step_name>::<code>`). Co-authored-by: Daniel Green <dangreen@microsoft.com> 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
1 participant
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.
What
Thread --organization, --project, and --repository flags into the
polyphony pr open-plan-adoinvocation inplan-level.yaml'sopen_plan_pr_adoscript step.Why
Surfaced live by iter 11 of the apex-driver dogfood against #3043:
Tracing back from
plan_reviewer:open_plan_pr_ado.output.pr_urlopen_plan_pr_ado'sscript_completedevent:stdout: "Required argument 'organization' was not specified."exit_code: 1Looking at the workflow YAML:
open_plan_pr_adowas the only ADOverb invocation in
plan-level.yamlnot threading the standard--organization/--project/--repositoryflags. The other three(
post-comment-ado,poll-status-ado,merge-plan-ado) werecorrect. Localized miss, not a systemic gap.
Verification
conductor validate plan-level.yaml— cleantests/lint-conductor-validate.ps1— 14/14 PASStests/lint-jinja-resolver.ps1— 0 errorsFollowups (not in this PR)
missed required flag on a CLI invocation. Worth adding a check that
every
polyphony pr *-adoscript step passes the ADO triple.envelope. When a required arg is missing, ConsoleAppFramework prints
plain text to stdout and exits 1 — the verb body never runs, so the
EmitOpenPlanAdoErrorenvelope never fires. Conductor sees textnot JSON, no
errorkey in output, route to error gate doesn't fire,and the next step crashes on undefined
pr_url. Same shape asthe conductor required-input gap (conductor: workflow runs with missing required inputs (no door enforcement) #188), but at the verb layer.
open_plan_pr_ado.output.pr_urlreference at line 988 wouldbenefit from an
is definedguard, but that just delays the crash— the real fix is making the verb's failure observable via the error
route. Better to land feat: Phase Detection, Routing Engine, and Command Implementations (PG-3) AB#2593 AB#2594 #2 first.
How this slipped past prior shippers
The bug existed since ADO platform support landed on plan-level.yaml.
Prior dogfood runs never reached
plan_level_dispatch; iter 11 isthe first to walk that deep. Validates the dogfood-as-test approach.