Skip to content

fix(plan): flush staged seed-children parent-tag patch to ADO (AB#3128) - #340

Merged
PolyphonyRequiem merged 1 commit into
mainfrom
feature/3128
May 12, 2026
Merged

fix(plan): flush staged seed-children parent-tag patch to ADO (AB#3128)#340
PolyphonyRequiem merged 1 commit into
mainfrom
feature/3128

Conversation

@PolyphonyRequiem

Copy link
Copy Markdown
Owner

Summary

Sister-bug to AB#3126 (PR #339, commit f5f3aed). PlanCommands.SeedChildren mutates the parent's tags via ITwigClient.PatchFieldsAsync (stamping polyphony:planned and any polyphony:facets=... override) but never calls SyncAsync. The change is staged in twig's local cache + pending queue and never reaches ADO. The next workflow step that reads the parent's tag (e.g. state_detector in apex-driver.yaml) sees stale tag state and the children-seeded heuristic fails to fire.

What changed

Verb-level fix (src/Polyphony/Commands/PlanCommands.SeedChildren.cs):

  • After twig.PatchFieldsAsync(...) for the parent System.Tags patch, call twig.SyncAsync(ct) to flush the staged mutation to ADO.
  • Guarded inside the existing if (addPlanned || facetsTagSet) block so the no-op idempotent path (tag already present and no facets-override) still skips the round-trip.
  • Mirrors the AB#3126 pattern in BranchCommands.NextImpl exactly.

Tests (tests/Polyphony.Tests/Commands/PlanCommandsSeedChildrenTests.cs):

  • SeedChildren_AfterParentTagPatch_FlushesViaSync -- finds the index of twig patch in runner.Invocations and asserts a subsequent twig sync invocation exists. Models BranchCommandsNextImplTests.NextImpl_HappyPath_FlushesStagedTransitionAfterSetState.
  • SeedChildren_NoParentTagPatch_DoesNotFlush -- belt-and-suspenders; when the tag is already present and no patch happens, asserts neither twig patch nor twig sync is called. Models BranchCommandsCloseScopeTests.CloseScope_NoTransitions_DoesNotFlush.
  • StubPatchOk extended to also stub the follow-up twig sync --output json; new StubSync helper mirrors the BranchCommands pattern.

Repro evidence

Test results

  • dotnet build -c Release -- clean (0 warnings, 0 errors)
  • dotnet test -c Release -- 3392/3396 pass + 4 pre-existing skips (PR fix(branch): flush staged twig state transitions so workflow reads see fresh ADO (AB#3126) #339 baseline was 3390/3394; +2 new tests here = 3396 total)
  • Two environmental flakes intermittently fail under full-suite contention but pass in isolation:
    • ProcessRunnerTests.RunAsync_TimeoutAfterEmittedOutput_ThrowsProcessCanceledWithBufferedOutput (timing)
    • LifecycleRouterScriptTests.IsRoot_FalseWhenWorkItemDiffersFromApex (pwsh subprocess contention)
  • All 8 lint scripts green: conductor-validate, jinja-resolver, lifecycle-router-coverage, no-tracked-polyphony-state, prose-children, psscriptroot-paths, type-agnostic, version-drift.

Sibling concerns NOT addressed

  • AB#3127 (ScopeCommands sibling site) -- intentionally out of scope; separate PR.
  • AB#3129 (close_mark_satisfied + terminal_satisfied workflow scripts) -- intentionally out of scope; separate PR.

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

Sister-bug to AB#3126. `PlanCommands.SeedChildren` mutates the parent's
tags via `ITwigClient.PatchFieldsAsync` (stamping `polyphony:planned`
and any `polyphony:facets=...` override) but does NOT call
`SyncAsync` afterward. The change is staged in twig's local cache +
pending queue but never pushed to ADO. The next workflow step that
reads the parent's tag (e.g. `state_detector` in `apex-driver.yaml`
checking `polyphony:planned`) sees stale tag state. This interacts
with the AB#3106 children-seeded heuristic -- the architect emits
`polyphony:planned` but `state_detector` re-entry doesn't always
see it without a sync.

Verb-level fix:
* `plan seed-children` -- `SyncAsync` after `PatchFieldsAsync` so
  the staged parent-tag mutation is durable before the verb returns.
  Guarded inside the existing `if (addPlanned || facetsTagSet)` block
  so the no-op idempotent path still skips the round-trip.

Tests: 2 new regression tests using `runner.Invocations` ordering,
mirroring the AB#3126 pattern (`BranchCommandsNextImplTests` /
`BranchCommandsCloseScopeTests`):
* `SeedChildren_AfterParentTagPatch_FlushesViaSync` -- asserts a
  `twig sync` invocation occurs after the `twig patch` for the
  parent tag.
* `SeedChildren_NoParentTagPatch_DoesNotFlush` -- belt-and-suspenders;
  when no patch was needed (tag already present), no flush either.

`StubPatchOk` extended to also stub the follow-up sync, and a new
`StubSync` helper mirrors the BranchCommands pattern.

Convention: follows the `State-mutation durability` section in
`.github/skills/polyphony-cli-developer/SKILL.md` (added by PR #339).

Tests: full Polyphony.Tests suite 3392/3396 pass (4 pre-existing skips);
2 flaky environmental tests (`ProcessRunnerTests.RunAsync_TimeoutAfter
EmittedOutput_ThrowsProcessCanceledWithBufferedOutput` and
`LifecycleRouterScriptTests.IsRoot_FalseWhenWorkItemDiffersFromApex`)
intermittently fail under suite contention but pass in isolation. All
8 lint scripts green.

Related:
* AB#3126 -- original `BranchCommands.NextImpl` + `close-scope` fix
  (PR #339, commit f5f3aed).
* AB#3127 -- `ScopeCommands` sibling site (separate PR).
* AB#3129 -- `close_mark_satisfied` + `terminal_satisfied` workflow
  scripts (separate PR).
* AB#3104 -- Bucket-C policy controllability umbrella.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@PolyphonyRequiem
PolyphonyRequiem merged commit ab8cc7f into main May 12, 2026
1 check passed
@PolyphonyRequiem
PolyphonyRequiem deleted the feature/3128 branch May 12, 2026 22:34
PolyphonyRequiem added a commit that referenced this pull request May 12, 2026
…+ terminal_satisfied (AB#3129) (#341)

Sister-fix to AB#3126. Two workflow scripts in
`.conductor/registry/workflows/` invoked `twig state` to transition
work items but did NOT follow up with `twig sync`, leaving the
transition staged in twig's local pending queue rather than pushed to
ADO. Subsequent processes reading ADO without first calling sync saw
stale state.

Fixed sites
-----------
* `apex-driver.yaml` > `close_mark_satisfied` (apex-level close-out):
  added `twig sync` after the `twig state` invocation so the apex's
  terminal-state transition is durable in ADO before the workflow
  terminates. Also added the fail-fast prologue (`ErrorActionPreference`
  + `PSNativeCommandUseErrorActionPreference`) -- reinforces the
  AUDIT AB#3066 fail-loud design for this script.

* `apex-item-dispatch.yaml` > `terminal_satisfied` (per-item wave
  corridor): added `twig sync` after the `twig state` invocation
  inside the existing try/catch, plus the fail-fast prologue. The
  AB#3066 observable-error envelope still applies -- a failed sync
  now surfaces via stderr + the `error` / `error_code` output fields,
  allowing the apex driver outer loop to re-evaluate next-ready on
  the following pass.

No additional unflushed sites found in the other 11 workflow YAMLs;
`implement-merge-group.yaml` > `primary_completer` was fixed by
PR #339, the only other `twig state` call site in workflow scripts.

New lint check
--------------
Added Check 9 to `lint-apex-driver.ps1` to pin this convention going
forward: every PowerShell `script` node in the apex-driver suite that
calls `twig state` MUST declare the fail-fast prologue and follow the
`twig state` with a `twig sync` later in the body. Five new Pester
assertions cover the rule (two real-YAML pins on the two fixed sites,
three synthetic mutation tests for missing-sync / missing-prologue /
twig-note-only-no-trigger).

Tests
-----
* dotnet test Polyphony.slnx -c Release: 3392 pass / 4 pre-existing skip
* Invoke-Pester .conductor/registry/tests: 361 pass / 2 fail (both
  pre-existing on main -- `lint-plan-level` open_questions policy
  check, unrelated to AB#3129)
* Invoke-Pester tests/: 143 pass / 0 fail
* conductor validate apex-driver.yaml: exit 0
* conductor validate apex-item-dispatch.yaml: exit 0

Cross-references: AB#3126 (parent class-of-bug, fixed by PR #339),
AB#3127 + AB#3128 (sibling instances filed under AB#3104; AB#3128
landed in PR #340 just before this PR).

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.

1 participant