Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions .conductor/registry/tests/lint-plan-level.Tests.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -856,4 +856,42 @@ agents:
($output -join "`n") | Should -Match 'missing-warning-mode-route'
}
}

Context 'parent_item_id default-filter form (regression: dogfood apex #3043, 2026-05-08)' {

BeforeEach {
$script:TempRoot = Join-Path ([System.IO.Path]::GetTempPath()) "lint-plan-level-pid-$([guid]::NewGuid().ToString('N').Substring(0,8))"
$script:WorkflowsDir = Join-Path $script:TempRoot 'workflows'
$script:TestsDir = Join-Path $script:TempRoot 'tests'
$script:RealYaml = Join-Path $PSScriptRoot '..' 'workflows' 'plan-level.yaml'
New-Item $script:WorkflowsDir -ItemType Directory -Force | Out-Null
New-Item $script:TestsDir -ItemType Directory -Force | Out-Null
Copy-Item $script:LintScript (Join-Path $script:TestsDir 'lint-plan-level.ps1')
}

AfterEach {
Remove-Item $script:TempRoot -Recurse -Force -ErrorAction SilentlyContinue
}

It 'Real plan-level.yaml uses the two-arg default(0, true) form for every parent_item_id reference' {
$content = Get-Content $script:RealYaml -Raw
# No bare default(0) form should remain — every reference uses default(0, true).
$bareMatches = [regex]::Matches($content, 'parent_item_id\s*\|\s*default\(\s*0\s*\)')
$bareMatches.Count | Should -Be 0
# And the two-arg form IS present (sanity check that we have references at all).
$twoArgMatches = [regex]::Matches($content, 'parent_item_id\s*\|\s*default\(\s*0\s*,\s*true\s*\)')
$twoArgMatches.Count | Should -BeGreaterThan 0
}

It 'Lint fails when plan-level.yaml regresses to bare default(0)' {
$content = Get-Content $script:RealYaml -Raw
# Mutate every two-arg form back to the bare form.
$mutated = $content -replace 'parent_item_id\s*\|\s*default\(\s*0\s*,\s*true\s*\)', 'parent_item_id | default(0)'
Set-Content (Join-Path $script:WorkflowsDir 'plan-level.yaml') $mutated
$lintScript = Join-Path $script:TestsDir 'lint-plan-level.ps1'
$output = pwsh -NoProfile -File $lintScript 2>&1
$LASTEXITCODE | Should -Be 1
($output -join "`n") | Should -Match 'parent-item-id-bare-default'
}
}
}
20 changes: 20 additions & 0 deletions .conductor/registry/tests/lint-plan-level.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,26 @@ if ($policyBlock -match 'type_loader\.output\.type_name\b') {
Rule = 'open-questions-policy-bad-type-field'
Detail = "open_questions_policy --scope references type_loader.output.type_name; the verb emits 'type', not 'type_name' (caused dogfood failure on apex #3043, 2026-05-07)"
}
}# ── Check 25: ancestor_chain.output.parent_item_id uses default(0, true) ─
# Bug #8 (dogfood apex #3043, 2026-05-08). The verb's wire shape carries
# parent_item_id as JSON null on the root-plan path; Jinja's bare
# `default(0)` filter substitutes only on Undefined, NOT on a present
# null value. The two-arg `default(0, true)` form substitutes on both
# Undefined and falsy (including None). Every reference to
# `ancestor_chain.output.parent_item_id` that wraps in `default()` MUST
# use the two-arg form, or the recursive `for_each` rebuilds with
# parent_item_id=None and the polyphony CLI rejects it.
#
# This regex finds bare `parent_item_id | default(0)` (no second arg).
# The two-arg form `default(0, true)` is allowed.
$badDefaultMatches = [regex]::Matches(
$content,
'parent_item_id\s*\|\s*default\(\s*0\s*\)')
if ($badDefaultMatches.Count -gt 0) {
$violations += [PSCustomObject]@{
Rule = 'parent-item-id-bare-default'
Detail = "Found $($badDefaultMatches.Count) reference(s) to ancestor_chain.output.parent_item_id with bare 'default(0)'; must use two-arg form 'default(0, true)' so JSON null is coerced (caused dogfood failure on apex #3043, 2026-05-08)"
}
}

# ── Report ───────────────────────────────────────────────────────────────
Expand Down
10 changes: 5 additions & 5 deletions .conductor/registry/workflows/plan-level.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ agents:
- "--item-id"
- "{{ workflow.input.work_item_id }}"
- "--parent-item-id"
- "{{ ancestor_chain.output.parent_item_id | default(0) }}"
- "{{ ancestor_chain.output.parent_item_id | default(0, true) }}"
routes:
- to: ensure_plan_branch_error_gate
when: "{{ ensure_plan_branch.output.error is defined and ensure_plan_branch.output.error }}"
Expand Down Expand Up @@ -856,7 +856,7 @@ agents:
- "--item-id"
- "{{ workflow.input.work_item_id }}"
- "--parent-item-id"
- "{{ ancestor_chain.output.parent_item_id | default(0) }}"
- "{{ ancestor_chain.output.parent_item_id | default(0, true) }}"
- "--ancestor-ids"
- "{{ ancestor_chain.output.ancestor_ids }}"
routes:
Expand Down Expand Up @@ -1527,7 +1527,7 @@ agents:
- "--item-id"
- "{{ workflow.input.work_item_id }}"
- "--parent-item-id"
- "{{ ancestor_chain.output.parent_item_id | default(0) }}"
- "{{ ancestor_chain.output.parent_item_id | default(0, true) }}"
routes:
- to: stale_generation_gate
when: "{{ merge_plan_pr.output.error_code is defined and merge_plan_pr.output.error_code == 'stale_generation' }}"
Expand Down Expand Up @@ -1834,7 +1834,7 @@ agents:
- "--item-id"
- "{{ workflow.input.work_item_id }}"
- "--parent-item-id"
- "{{ ancestor_chain.output.parent_item_id | default(0) }}"
- "{{ ancestor_chain.output.parent_item_id | default(0, true) }}"
- "--ancestor-ids"
- "{{ ancestor_chain.output.ancestor_ids }}"
routes:
Expand Down Expand Up @@ -1961,7 +1961,7 @@ agents:
- "--item-id"
- "{{ workflow.input.work_item_id }}"
- "--parent-item-id"
- "{{ ancestor_chain.output.parent_item_id | default(0) }}"
- "{{ ancestor_chain.output.parent_item_id | default(0, true) }}"
routes:
# NOTE: stale_generation reuses stale_generation_gate. The gate's
# prompt references merge_plan_pr.output.* fields — those are
Expand Down
43 changes: 43 additions & 0 deletions docs/polyphony-state-effects-catalog.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,31 @@ introduce new dependencies.
runtime. Pinned by lint check `open-questions-policy-bad-type-field`
in `lint-plan-level.ps1` as of this PR.

### `polyphony plan derive-ancestor-chain --root-id R --item-id I`
- **Purpose**: derive the parent-chain (ancestors from root → leaf) for
an item under a given apex root. Workflows feed the result into
recursive planning to know who the parent plan branch is.
- **Pre**: ADO reachable; `R > 0`, `I > 0`.
- **Post**: emits `{root_id, item_id, is_root_plan, parent_item_id,
ancestor_ids, ancestor_chain, depth}`. On error, `{error}` is added.
- **Side effects**: none (read-only — walks the work-item tree).
- **Idempotent**: yes.
- **Wire-shape gotcha**: `parent_item_id` is `int?` and is **always
emitted** (per-property `[JsonIgnore(Condition = Never)]` overrides the
`PolyphonyJsonContext` default of `WhenWritingNull`). The field is
`null` for the root-plan case (`item_id == root_id`) and for direct
children of root, and an integer for deeper descendants.

Bug #8 (dogfood apex #3043, 2026-05-08) had the field elided on the
root path; conductor's `strict_undefined` then raised
`'dict object' has no attribute 'parent_item_id'` when the workflow
tried to thread it into the recursive `for_each` step. Even after the
field is always emitted, Jinja's bare `default(0)` filter does NOT
substitute on `None` — only on `Undefined`. Workflow MUST use
`default(0, true)` (two-arg, `boolean=True`) to coerce both. Pinned
by lint check `parent-item-id-bare-default` in `lint-plan-level.ps1`
as of this PR.

### `polyphony policy resolve --domain <d> --scope <s> [--path P]`
- **Purpose**: resolve effective policy for a `(scope, domain)` pair by
layering most-specific-wins (`type:Name` > `root` > `defaults`).
Expand Down Expand Up @@ -267,6 +292,24 @@ introduce new dependencies.
agree. A general remedy would be a registry of conductor-honored
Jinja functions/filters that lints can validate against — also
rolls into [#163](https://github.com/PolyphonyRequiem/polyphony/issues/163).
- **`DefaultIgnoreCondition.WhenWritingNull` breaks `strict_undefined`
Jinja consumers**: `PolyphonyJsonContext` defaults to
`JsonIgnoreCondition.WhenWritingNull`, so any nullable verb-output
field (`int?`, `string?`, etc.) is silently elided when its value is
null. Conductor renders agent prompts with `strict_undefined`, which
raises on attribute access for missing dict keys *before* any
`default()` filter can apply. Bug #8 (dogfood apex #3043,
2026-05-08) was the first instance: `plan derive-ancestor-chain`'s
`parent_item_id` was elided on the root path, blowing up the
recursive `for_each` consumer. Compounding gotcha: even when the
field IS present-as-null, Jinja's bare `default(0)` only substitutes
on Undefined, NOT on `None` — workflow must use the two-arg form
`default(0, true)` to coerce `None → 0`. Per-property
`[JsonIgnore(Condition = Never)]` is the surgical Option-A fix; the
blanket alternative (flip the context default to `Never` globally)
would tax every error-envelope shape and is deferred as a systematic
decision. **This pattern affects every nullable verb-output field**
reachable from a `strict_undefined` Jinja consumer — audit needed.
- **Wave integration idempotency**: not yet exercised; document after
first wave-integration smoke.
- **`apex-wave-dispatch.yaml` and per-lifecycle sub-workflows**: not
Expand Down
23 changes: 21 additions & 2 deletions src/Polyphony/Models/PlanDeriveAncestorChainResult.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
namespace Polyphony;

using System.Text.Json.Serialization;

/// <summary>
/// Output of <c>polyphony plan derive-ancestor-chain</c>. Walks the work-item
/// parent chain from <see cref="ItemId"/> up to (but not past) <see cref="RootId"/>
Expand All @@ -8,8 +10,8 @@ namespace Polyphony;
/// <item><c>--root-id</c> for every verb (already known by the workflow).</item>
/// <item><c>--parent-item-id</c> for <c>branch ensure-plan</c>,
/// <c>pr open-plan-pr</c>, and <c>pr merge-plan-pr</c> — required for
/// descendants of descendants; omitted (null) for the root plan and
/// direct children of root.</item>
/// descendants of descendants; null for the root plan and direct
/// children of root.</item>
/// <item><c>--ancestor-ids</c> for <c>pr open-plan-pr</c> — comma-separated
/// chain (immediate parent first), with the literal <c>"root"</c>
/// token used in place of the root work-item id, e.g. <c>"5678,root"</c>
Expand All @@ -32,7 +34,24 @@ public sealed record PlanDeriveAncestorChainResult
/// Immediate plan-tree parent's work-item id, or null when the parent is
/// implicit (root plan, or direct child of root). Maps directly to the
/// <c>--parent-item-id</c> flag of the plan-PR verbs (omit when null).
///
/// <para>
/// Always serialized to JSON (overrides the per-context
/// <c>WhenWritingNull</c> default) so workflow Jinja consumers under
/// <c>strict_undefined</c> can reference <c>output.parent_item_id</c>
/// unconditionally without raising on a missing attribute. The wire shape
/// is uniform across root-plan and descendant-plan invocations.
/// </para>
/// <para>
/// Bug #8 (dogfood apex #3043, 2026-05-08) surfaced the original wire
/// shape: <c>WhenWritingNull</c> elided the field for the root case,
/// then conductor's <c>strict_undefined</c> raised on
/// <c>ancestor_chain.output.parent_item_id | default(0)</c> because
/// Jinja's <c>default()</c> filter triggers on Undefined values, not on
/// missing attributes of a defined dict.
/// </para>
/// </summary>
[JsonIgnore(Condition = JsonIgnoreCondition.Never)]
public int? ParentItemId { get; init; }

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,67 @@ await SeedAsync(
result.Error.ShouldNotBeNull();
result.Error.ShouldContain("Cycle detected");
}

// ─────────────────────────────────────────────────────────────────────────
// Wire-shape regression: parent_item_id is always emitted (bug #8 — dogfood
// apex #3043, 2026-05-08). The PolyphonyJsonContext default is
// WhenWritingNull, but ParentItemId is per-property pinned to Never so
// workflow Jinja under strict_undefined can reference
// `output.parent_item_id` unconditionally without raising on a missing
// attribute. The wire shape is uniform across root-plan and descendant
// invocations.
// ─────────────────────────────────────────────────────────────────────────

[Fact]
public async Task RootPlan_WireShape_ParentItemIdAlwaysPresentAsNull()
{
var cmd = CreateCommand();
var (_, output) = await CaptureConsoleAsync(() => cmd.DeriveAncestorChain(100, 100));

// Field must be present (not elided by WhenWritingNull) with explicit null value.
output.ShouldContain("\"parent_item_id\":null");
}

[Fact]
public async Task DirectChildOfRoot_WireShape_ParentItemIdAlwaysPresentAsNull()
{
await SeedAsync(
new WorkItemBuilder().WithId(100).WithType("Epic").Build(),
new WorkItemBuilder().WithId(200).WithType("Issue").WithParentId(100).Build());

var cmd = CreateCommand();
var (_, output) = await CaptureConsoleAsync(() => cmd.DeriveAncestorChain(100, 200));

// Direct children of root also have null parent_item_id (the parent IS root,
// which is implicit). Field must still be emitted explicitly.
output.ShouldContain("\"parent_item_id\":null");
}

[Fact]
public async Task DescendantPlan_WireShape_ParentItemIdHasIntegerValue()
{
await SeedAsync(
new WorkItemBuilder().WithId(100).WithType("Epic").Build(),
new WorkItemBuilder().WithId(200).WithType("Issue").WithParentId(100).Build(),
new WorkItemBuilder().WithId(250).WithType("Task").WithParentId(200).Build());

var cmd = CreateCommand();
var (_, output) = await CaptureConsoleAsync(() => cmd.DeriveAncestorChain(100, 250));

// Descendant: parent_item_id must be the integer parent id (here, 200).
output.ShouldContain("\"parent_item_id\":200");
}

[Fact]
public async Task ErrorPath_WireShape_ParentItemIdAlwaysPresent()
{
// Even on the error path, the schema must include parent_item_id so
// workflow consumers don't have to branch the access pattern by success
// vs. failure envelope.
var cmd = CreateCommand();
var (_, output) = await CaptureConsoleAsync(() => cmd.DeriveAncestorChain(0, 0));

output.ShouldContain("\"parent_item_id\":null");
output.ShouldContain("\"error\":");
}
}
Loading