fix(workflow): terminal_satisfied observable-error fix (AB#3066)

.conductor/registry/workflows/apex-driver.yaml

@@ -921,7 +921,7 @@
 
       ```
       {%- if build_worklist.output.error is defined -%}
       {{ build_worklist.output.error }}
       {%- else -%}
       (no error message captured — check polyphony logs)
       {%- endif -%}
@@ -957,7 +957,7 @@
       ### `polyphony state next-ready` error
       ```
       {%- if preflight_apex_state is defined and preflight_apex_state.output is defined and preflight_apex_state.output.error is defined and preflight_apex_state.output.error != '' -%}
       {{ preflight_apex_state.output.error }}
       {%- else -%}
       (no error — this step succeeded or did not run)
       {%- endif -%}
@@ -966,7 +966,7 @@
       ### `polyphony branch ensure-feature` error
       ```
       {%- if preflight_ensure_branch is defined and preflight_ensure_branch.output is defined and preflight_ensure_branch.output.error is defined and preflight_ensure_branch.output.error != '' -%}
       {{ preflight_ensure_branch.output.error }}
       {%- else -%}
       (no error — this step succeeded or did not run)
       {%- endif -%}
@@ -984,7 +984,7 @@
       ### `commit_and_push_manifest` error
       ```
       {%- if commit_and_push_manifest is defined and commit_and_push_manifest.output is defined and commit_and_push_manifest.output.error is defined and commit_and_push_manifest.output.error != '' -%}
       [{{ commit_and_push_manifest.output.error_code | default('?') }}] {{ commit_and_push_manifest.output.error }}
       {%- else -%}
       (no error — this step succeeded or did not run)
       {%- endif -%}
@@ -993,7 +993,7 @@
       ### `declare_root` error
       ```
       {%- if declare_root is defined and declare_root.output is defined and declare_root.output.error is defined and declare_root.output.error != '' -%}
       {{ declare_root.output.error }}
       {%- else -%}
       (no error — this step succeeded or did not run)
       {%- endif -%}
@@ -1073,6 +1073,14 @@
   # Sync ADO once more (so the validate event sees a fresh cache),
   # validate the satisfaction event against the apex's type config,
   # then transition state via twig.
+  # AUDIT (AB#3066): No try/catch wrapper here — intentional. Unlike
+  # `terminal_satisfied` in apex-item-dispatch.yaml (the per-item wave
+  # corridor, where a swallow lets the wave complete and the outer loop
+  # re-evaluate next-ready), this is the apex-level close-out. A failure
+  # here SHOULD fail the workflow loudly so the operator sees that the
+  # apex didn't transition. If a try/catch is later added here, mirror
+  # the apex-item-dispatch.yaml > terminal_satisfied shape — log to
+  # stderr AND surface in structured output.
   - name: close_mark_satisfied
     type: script
     description: Sync, validate, and transition apex to its satisfied state

.conductor/registry/workflows/apex-item-dispatch.yaml

@@ -465,18 +465,25 @@ agents:
       - "-NoProfile"
       - "-Command"
       - |
+        $errMsg = '';
         try {
           twig sync;
           $validate = polyphony validate --work-item {{ workflow.input.work_item_id }} --event item_satisfied | ConvertFrom-Json;
           if ($validate.target_state) {
             twig state $validate.target_state --id {{ workflow.input.work_item_id }};
           }
         } catch {
-          # Swallow: the apex driver's outer loop (PR #9) re-evaluates
-          # next-ready on the next pass; transient ADO/twig failures
-          # surface there rather than failing the whole wave here.
+          # Capture, don't swallow: the apex driver's outer loop (PR #9)
+          # re-evaluates next-ready on the next pass — transient ADO/twig
+          # failures still surface there rather than failing the whole
+          # wave here. But we now log the message to stderr (visible in
+          # the conductor event log) AND surface it in the structured
+          # output so wave-aggregator consumers can observe failures
+          # instead of silently re-looping. AB#3066 fix.
+          $errMsg = $_.Exception.Message;
+          [Console]::Error.WriteLine("terminal_satisfied caught: $errMsg");
         }
-        @{
+        $out = @{
           work_item_id = {{ workflow.input.work_item_id }};
           apex_id = '{{ workflow.input.apex_id }}';
           lifecycle_workflow = 'terminal-satisfied';
@@ -490,7 +497,12 @@ agents:
           actionable_satisfied = $false;
           implement_pg_merged = $false;
           feature_pr_merged = $false;
-        } | ConvertTo-Json -Compress
+        };
+        if ($errMsg -ne '') {
+          $out.error = $errMsg;
+          $out.error_code = 'terminal_satisfied_caught';
+        }
+        $out | ConvertTo-Json -Compress
     routes:
       - to: $end
 

.polyphony/run.yaml

@@ -1,7 +1,7 @@
 schema: 1
-root_id: 3065
+root_id: 3066
 platform_project: dev.azure.com/dangreen-msft/Polyphony
-created_at: 2026-05-09T19:54:35.4584004Z
+created_at: 2026-05-09T21:41:13.2745090Z
 created_by: dangreen
 branch_model_version: 1
 plan_generations: {}

plans/plan-3066.md

@@ -0,0 +1,46 @@
+---
+apex_facets: [implementable]
+---
+
+# Replace terminal_satisfied silent-swallow with structured error reporting
+
+## Strategic Objective
+
+Make permanent failures in the `terminal_satisfied` step of the apex item dispatch workflow observable instead of silently swallowed, so that wrong-state-name, blocked-transition, expired-token, and DNS-class failures surface to the user (via conductor event log and/or structured wave-aggregator output) rather than being hidden inside an outer-loop re-run cycle bounded only by the iteration ceiling.
+
+## Success Criteria
+
+- The `terminal_satisfied` catch block in [.conductor/registry/workflows/apex-item-dispatch.yaml](.conductor/registry/workflows/apex-item-dispatch.yaml) (currently ~lines 474–478) no longer silently discards exceptions from `twig sync` / `polyphony validate` / `twig state`. The failure is logged to script stdout (so it appears in the conductor event log) and/or surfaced in the structured output to the wave aggregator (e.g. an `error_message` field).
+- The non-fail-the-wave behavior is preserved: a permanent twig state failure still lets the wave complete and the outer loop re-evaluate next-ready on the next pass. The change is observability-only, not retry-policy.
+- `close_mark_satisfied` in [.conductor/registry/workflows/apex-driver.yaml](.conductor/registry/workflows/apex-driver.yaml) is audited for the same silent-swallow pattern. If present, fixed the same way; if absent, the absence (and reasoning) is documented in the PR description or a brief comment near the audited region.
+- Existing e2e tests still pass.
+
+## Scope
+
+### In Scope
+
+- Edit `terminal_satisfied` in [.conductor/registry/workflows/apex-item-dispatch.yaml](.conductor/registry/workflows/apex-item-dispatch.yaml) to log/surface caught exceptions.
+- Audit (and, if needed, edit) `close_mark_satisfied` in [.conductor/registry/workflows/apex-driver.yaml](.conductor/registry/workflows/apex-driver.yaml) for the same pattern.
+- Verify existing e2e tests still pass.
+
+### Out of Scope
+
+- Changing retry / no-retry semantics — "transient failures re-evaluate on next pass" stays.
+- Wiring an alerting layer — event-log + structured-output observability is sufficient.
+- Touching unrelated silent-swallow blocks elsewhere in the workflow registry.
+
+## Child Issues
+
+None — this Epic is indivisible per the work item's planner note. The work is a single-file YAML edit (with a possible second-file edit after audit) and is the unit of work itself. The plan front matter declares `apex_facets: [implementable]` to signal deliberate atomicity to the seeder.
+
+## Risks
+
+- **Audit risk:** the `close_mark_satisfied` audit may surface a more pervasive pattern across the workflow registry. If so, the broader cleanup is explicitly out of scope for this Epic — file a follow-up rather than expanding scope here.
+- **Output-shape risk:** adding an `error_message` field to the structured wave-aggregator output may need to match an existing schema the aggregator consumes. Implementer should verify the aggregator side accepts the added field (or treats unknown fields as harmless) before shipping.
+
+## References
+
+- [.conductor/registry/workflows/apex-item-dispatch.yaml](.conductor/registry/workflows/apex-item-dispatch.yaml) — `terminal_satisfied` step
+- [.conductor/registry/workflows/apex-driver.yaml](.conductor/registry/workflows/apex-driver.yaml) — `close_mark_satisfied` audit target
+- F5 audit PR #245
+- [plans/plan-3064.md](plans/plan-3064.md) — F5 dogfood plan, criterion 5 ("dogfood-verified") deferral context

tests/workflow-terminal-satisfied-observable.Tests.ps1

@@ -0,0 +1,82 @@
+#requires -Version 7.0
+
+<#
+.SYNOPSIS
+    Pins the AB#3066 fix to apex-item-dispatch.yaml's `terminal_satisfied` step.
+
+.DESCRIPTION
+    `terminal_satisfied` wraps a twig+polyphony block in try/catch so that
+    transient ADO/twig failures don't fail the entire wave (the apex-driver
+    outer loop re-evaluates next-ready on the next pass). Before AB#3066 the
+    catch was silent: no stderr write, no field surfaced in the structured
+    output. The fix preserves the don't-fail-the-wave behavior while making
+    every caught exception observable in two channels:
+
+      1. stderr  — `[Console]::Error.WriteLine(...)` so the conductor event
+                   log shows the failure.
+      2. output  — conditional `error` + `error_code` fields so wave
+                   aggregators can act on it.
+
+    These tests are structural assertions against the YAML text. They prevent
+    regression to the silent-swallow shape.
+#>
+
+BeforeAll {
+    $script:WorkflowPath = Join-Path $PSScriptRoot '..' '.conductor' 'registry' 'workflows' 'apex-item-dispatch.yaml'
+    $script:WorkflowYaml = Get-Content -Raw -LiteralPath $script:WorkflowPath
+
+    # Slice out just the terminal_satisfied block (everything from the step
+    # name to the next top-level step).
+    $pattern = '(?ms)^  - name: terminal_satisfied\b.*?(?=^  - name: |\Z)'
+    $match = [regex]::Match($script:WorkflowYaml, $pattern)
+    if (-not $match.Success) {
+        throw "Could not locate terminal_satisfied block in $script:WorkflowPath"
+    }
+    $script:Block = $match.Value
+}
+
+Describe 'apex-item-dispatch.yaml > terminal_satisfied — AB#3066 observable-error fix' {
+
+    It 'has a try/catch wrapper (preserves "do not fail the wave" semantics)' {
+        $script:Block | Should -Match 'try\s*\{'
+        $script:Block | Should -Match '\}\s*catch\s*\{'
+    }
+
+    It 'captures the caught exception message into a variable' {
+        # Must be `$_.Exception.Message`, not `$_` (which is the ErrorRecord
+        # object) — the latter would serialize poorly into the output payload.
+        $script:Block | Should -Match '\$errMsg\s*=\s*\$_\.Exception\.Message'
+    }
+
+    It 'writes the caught exception to stderr (visible in conductor event log)' {
+        # Must be `[Console]::Error.WriteLine` — `Write-Error` would terminate
+        # the script (defeating the don't-fail-the-wave intent), and a plain
+        # `Write-Host` would not land on stderr.
+        $script:Block | Should -Match '\[Console\]::Error\.WriteLine'
+        $script:Block | Should -Not -Match 'Write-Error\b'
+    }
+
+    It 'surfaces the error in structured output via `error` and `error_code` fields' {
+        # Match the existing terminal-error convention (e.g. terminal_classify_error,
+        # terminal_spawn_error): a string `error` + a string `error_code`.
+        $script:Block | Should -Match '\$out\.error\s*='
+        $script:Block | Should -Match "\`$out\.error_code\s*=\s*'terminal_satisfied_caught'"
+    }
+
+    It 'only adds the error fields when an exception was actually caught' {
+        # Conditional on `$errMsg -ne ''` so the happy path output stays clean
+        # (no spurious empty error fields when nothing went wrong).
+        $script:Block | Should -Match "if\s*\(\s*\`$errMsg\s+-ne\s+''\s*\)"
+    }
+
+    It 'does NOT rethrow inside the catch block (would fail the wave)' {
+        # If a future edit adds `throw` inside the catch, the wave fails and
+        # the outer loop re-evaluation is bypassed — that would be a regression
+        # of the AB#3066 design, not just the observability fix.
+        $catchPattern = '(?ms)\}\s*catch\s*\{(.*?)\}\s*\$out\s*='
+        $catchMatch = [regex]::Match($script:Block, $catchPattern)
+        $catchMatch.Success | Should -BeTrue
+        $catchBody = $catchMatch.Groups[1].Value
+        $catchBody | Should -Not -Match '^\s*throw\b'
+    }
+}