fix: always dump case ledgers, even when a phase dies

notes/demo-ci-invariants.md

@@ -39,6 +39,66 @@ invariant-harness (matrix: fv) → downloads artifact → runs pytest
 
 ---
 
+## Artifact Availability on Failure (DEMOCI-10)
+
+**Problem**: The separate-job pattern above only pays off if the artifact
+*exists* when the demo fails. It did not. Each `run_<name>_demo()` called
+`_phase_dump_case_ledgers()` as its last statement, so an assertion escaping a
+`demo_check`/`demo_gate` block skipped the dump; `main()`'s
+`finally: assert_demo_success()` still raised, so CI reported the demo failure
+but `devlogs/` was empty. The `invariant-harness` job then died on **artifact
+download** — the run that most needed forensics produced none, and the harness
+reported a plumbing error instead of an invariant result (issue #2239).
+
+**Design**: three pieces, spec'd as DEMOCI-10-001 through DEMOCI-10-004.
+
+1. **A shared harness owns the ordering.**
+   `vultron/demo/helpers/harness.py` provides `scenario_harness(demo_name)`.
+   Every `run_<name>_demo()` body runs inside it: it resets the failure
+   accumulator, always dumps the case ledgers on the way out (normal return or
+   any `BaseException`), then calls `assert_demo_success()` last. Scenarios
+   register their dump with `harness.dump_with(...)` as soon as a case exists,
+   so every later phase can fail without costing the ledgers. `main()` no longer
+   wraps the run in `try/finally: assert_demo_success()` — a second owner of the
+   accumulator would assert before the dump had run. See DEMOMA-23.
+
+2. **The dump always leaves a manifest.**
+   `vultron/demo/helpers/ledger_dump.py::dump_case_ledgers()` writes
+   `devlogs/<demo>/dump-manifest.json` from a `finally`, recording `demoName`,
+   `caseId`, `ledgerFileCount`, `targetCount`, an optional top-level `reason`,
+   and a per-actor list naming each missing actor with the reason it was
+   missing. So the artifact is non-empty even when there were no ledgers at all
+   to capture — including the "died before any case existed" case, where the
+   manifest records `ledgerFileCount: 0` and the reason why.
+
+3. **`load_devlogs()` fails instead of skipping when a dump happened.**
+   Previously a missing/empty `devlogs/` meant "no test data" → `pytest.skip`,
+   which reads green. Now `load_devlogs()` distinguishes the two cases:
+
+   | State of the downloaded artifact | Outcome |
+   |---|---|
+   | no `devlogs/`, or no `devlogs/<demo>/` | `skip` — the demo genuinely did not run |
+   | no ledger files **and** no `dump-manifest.json` | `skip` — same |
+   | no ledger files **but** a manifest exists | **`fail`** — real invariant failure |
+   | manifest present but unparseable | **`fail`** |
+   | ledger files present | load and check normally |
+
+   The failure message reproduces the manifest's own account — case ID, captured
+   *X* of *Y* targets, and one line per missing actor with its route key and
+   reason — so the harness output explains *why* there are no ledgers rather
+   than leaving a reviewer to guess.
+
+**A dump failure must not mask the scenario failure.** Errors raised inside the
+dump are recorded in the manifest's `reason` field and swallowed; the harness
+re-raises the original exception with the accumulated `demo_check` failures
+attached as exception notes (DEMOCI-10-004).
+
+Regression coverage: `test/demo/test_issue_2239_ledger_dump_in_finally.py`
+(all nine scenarios), `test/demo/test_scenario_harness.py`, and
+`test/ci/invariants/test_common.py::TestLoadDevlogsManifestHandling`.
+
+---
+
 ## Harness File Conventions
 
 Each scenario gets **one self-contained harness file**,
@@ -48,7 +108,9 @@ existing nine:
 
 1. Declare `_DEMO_NAME = "<scenario>"` at module scope.
 2. Load replicas with `load_devlogs(demo_name=_DEMO_NAME)`, imported from
-   `test/ci/invariants/common.py`.
+   `test/ci/invariants/common.py`. It skips when the demo did not run and
+   **fails** when the demo ran, dumped, and still produced no ledgers — see
+   "Artifact Availability on Failure" above.
 3. Declare `_CHAIN_ACTORS` (scenario-role names, not docker service names) and
    `_<SCENARIO>_EXPECTED_EVENT_TYPES`.
 4. Call the shared check functions from `common.py`; keep scenario-specific

plan/incoming/learnings/20260812-artifact-dump-belongs-in-the-failure-path.md

@@ -0,0 +1,36 @@
+---
+title: "No spec required forensic artifacts to survive the failure they document"
+type: learning
+timestamp: "2026-08-12T00:00:00Z"
+source: ISSUE-2239
+signal: spec-gap
+---
+
+DEMOCI-04 requires the demo job to upload `devlogs/` as an artifact and the
+invariant harness to run as a separate `if: always()` job. Both were implemented
+correctly. Nothing anywhere required the *producer* of that artifact to run on
+the failing path, so all nine scenarios ended `run_<name>_demo()` with a plain
+call to `_phase_dump_case_ledgers()` — code that by construction runs only when
+nothing went wrong. The workflow-side requirement (`if: always()`) and the
+application-side placement (after the last phase) were each locally reasonable
+and jointly guaranteed that the run most in need of forensics produced none.
+
+The gap is a shape, not a line: **an artifact whose only purpose is diagnosing
+failures must be produced from a path that failure cannot skip.** Filled as
+DEMOCI-10-001/-002 (dump from a path that runs on failure; always write a
+manifest) and DEMOMA-23-001/-003 (`scenario_harness()` owns the ordering;
+register the dump as soon as a case exists).
+
+The related trap, worth stating separately because it survived the first fix
+attempt: the invariant harness treated a missing/empty `devlogs/` as "no test
+data" and called `pytest.skip`. A skip that means *"the thing I was going to
+assert on is absent"* is a false green whenever the absence **is** the failure.
+Distinguishing the two needs evidence from the producer, which is why the dump
+now always writes `dump-manifest.json` — the manifest is what turns "nothing to
+check" into "the demo ran, dumped, and had no ledgers, and here is why per
+actor" (DEMOCI-10-003).
+
+Generalisable check for any future skip-on-missing-input fixture: can the input
+be missing *because* the system under test broke? If yes, the fixture needs a
+positive signal that the producer ran, and it must fail — not skip — when that
+signal is present and the data is not.

plan/incoming/learnings/20260812-harness-propagates-the-cause-not-a-summary.md

@@ -0,0 +1,44 @@
+---
+title: "The harness propagates the original exception and attaches accumulated failures as notes"
+type: learning
+timestamp: "2026-08-12T00:00:00Z"
+source: ISSUE-2239
+signal: design-question
+---
+
+ISSUE-2239 specified the shape — "run phases, always dump ledgers, then assert.
+The dump belongs in the `finally`" — but not what the failing path should raise.
+Two behaviours are defensible and they differ in what a CI reader sees:
+
+- Call `assert_demo_success()` unconditionally on the way out. Consistent, but
+  on the failing path it replaces the exception that actually ended the run with
+  a generic `DemoFailureError("N demo failure(s)")` — exactly the unusable
+  reporting #2240 is about.
+- Let the original exception propagate untouched and attach the accumulated
+  `demo_check` failures via `exc.add_note()`.
+
+`scenario_harness()` does the second. The traceback still points at the phase
+that died; the soft failures that `demo_check` had recorded and would otherwise
+be dropped ride along as `__notes__` (DEMOMA-23-004, SHOULD-level). Only the
+succeeding path calls `assert_demo_success()`.
+
+Two consequences worth knowing before touching this:
+
+- **A dump error must not become the reported failure.** The dump runs inside
+  `demo_step`, which records and continues, so a dump that dies still leaves the
+  scenario's own exception in charge — and a scenario that otherwise succeeded
+  but failed to dump does fail, as an accumulated failure rather than a raise.
+- **The backstop manifest is written only when the dump raised.** Writing it
+  from an unconditional `finally` looked equivalent and was not: `dump_case_ledgers`
+  writes its own manifest, so the backstop only ever fires when the dump never
+  got that far. Unconditionally, it stamped the "dump crashed" reason onto runs
+  whose dump was fine — including unit tests that stub the dump out, which then
+  wrote a spurious manifest into the repo-root `devlogs/` and turned the local
+  invariant harness's skips into failures. Pinned by
+  `test_no_crash_manifest_when_the_dump_succeeds`.
+
+Unresolved and already tracked, noted here so it is not mistaken for settled:
+AGENTS.md, DEMOCI-01-007 and ADR-0058 all describe `demo_gate` as the primitive
+that stops dependent steps, and it still does not exist in code (#2201, #2203).
+The harness makes an escaping assertion survivable; it does not make the ~20
+unguarded `wait_for_case_participants` call sites causal.

plan/incoming/learnings/20260812-pytest-timeout-stack-names-the-victim-not-the-culprit.md

@@ -0,0 +1,46 @@
+---
+title: "A pytest-timeout stack names the victim, not the culprit — and the spec registry was the cost"
+type: learning
+timestamp: "2026-08-12T00:00:00Z"
+source: ISSUE-2239
+signal: tooling-issue
+---
+
+Validating the ISSUE-2239 branch, the full suite aborted at 82% with a
+`+++ Timeout +++` stack dump, no `N passed / M failed` summary, and exit 1. Two
+things about that were misleading enough to record:
+
+1. **The reported test moves.** `timeout = 5` with `timeout_method = "thread"`
+   in `pyproject.toml` kills the whole process; the stack is dumped wherever the
+   alarm happened to fire, so the named test is whichever one was running, not
+   necessarily the expensive one. Across runs it landed in
+   `test_docs_render.py`, `test_decision_audit_inventory.py`, and
+   `test_pcr_late_joiner.py` (#2270). Diagnosing from the test name leads
+   nowhere; timing the shared call in the stack does.
+
+2. **A 5-run A/B beats one run.** With `specs/` stashed the flaky test passed
+   0/5 failures; with the branch's two spec-file additions it failed 2/5. That
+   looks causal and is not — the same test was already 3.5–4.3 s against a 5 s
+   ceiling on clean `main`. Eight new spec entries out of 2355 (0.3%) only moved
+   it across a line it was already sitting on.
+
+Where the cost actually was, measured per stage:
+
+| Stage | Time |
+|---|---|
+| `yaml.safe_load` of all `specs/*.yaml` (1.18 MB) | 2.73 s |
+| `SpecFile.model_validate` for every file | 0.024 s |
+| `load_registry()` total, uncached, per call | 3.44 s |
+
+`pyyaml` silently falls back to the pure-Python `SafeLoader`; this environment
+ships `libyaml`. Selecting `yaml.CSafeLoader` when present (identical
+SafeLoader semantics) took `load_registry()` from 3.44 s to 0.35 s and the flaky
+test from 2/5 to 0/5. Two lines, in
+`vultron/metadata/specs/registry.py`.
+
+Two durable takeaways: **when a validation step is 100× cheaper than the parse
+step in front of it, check which loader the parser picked** — the 10× is free
+and applies to every `yaml.safe_load` of a large file in this repo. And the
+underlying fragility is untouched: any test doing real work is one slow
+container away from aborting the session while `timeout_method = "thread"` is
+global, which is #2270's actual fix (parented to #2089).

specs/demo-ci.yaml

@@ -6,7 +6,7 @@ description: >-
   when the demo fails — closing the gap where tests pass while the demo produces
   silent failures. Includes requirements for the invariant harness to run as an
   independent CI gate so failures are visible even when the demo itself also fails.
-version: 1.8.0
+version: 1.9.0
 scope:
   - prototype
   - production
@@ -848,3 +848,86 @@ groups:
         relationships:
           - rel_type: refines
             spec_id: DEMOCI-05-001
+
+  - id: DEMOCI-10
+    title: Case-Ledger Artifact Availability on Failure
+    specs:
+      - id: DEMOCI-10-001
+        priority: MUST
+        kind: project
+        statement: >-
+          Every demo scenario MUST export its case ledgers from a path that runs
+          whether or not the scenario's phases succeeded — in practice, from the
+          shared scenario harness's exception path as well as its success path
+          (see DEMOMA-23-001). A scenario MUST NOT place the ledger dump only
+          after its last phase, where any escaping exception skips it.
+        rationale: >-
+          `demo_check` and `demo_step` accumulate rather than raise, so the
+          exceptions that do escape a scenario are the ones that most need
+          forensics — and those were exactly the runs that uploaded nothing,
+          because the dump sat inline after the last phase. Losing the ledgers on
+          failure inverts the value of the artifact.
+        relationships:
+          - rel_type: refines
+            spec_id: DEMOCI-04-001
+
+      - id: DEMOCI-10-002
+        priority: MUST
+        kind: project
+        statement: >-
+          The case-ledger dump MUST always write a `dump-manifest.json` under
+          `devlogs/<demo-name>/`, even when it captured no ledgers at all. The
+          manifest MUST record `demoName`, `caseId`, `ledgerFileCount`,
+          `targetCount`, an optional top-level `reason`, and one `actors[]` entry
+          per dump target carrying `actorName`, `routeKey`, `captured`,
+          `entryCount`, `ledgerFile`, and `reason`.
+        rationale: >-
+          The `upload-artifact` step and the `invariant-harness` job's download
+          both need the artifact to exist. An always-present manifest makes the
+          download succeed unconditionally and turns "nothing was uploaded" into
+          a machine-readable statement of which actors were missing and why, so
+          the invariant harness can report a real invariant result instead of a
+          download error.
+        relationships:
+          - rel_type: refines
+            spec_id: DEMOCI-10-001
+
+      - id: DEMOCI-10-003
+        priority: MUST
+        kind: project
+        statement: >-
+          `test/ci/invariants/common.py::load_devlogs()` MUST fail — not skip —
+          when a `dump-manifest.json` is present under the scenario's devlogs
+          directory but no `*-case-ledger.jsonl` file is, and MUST fail when a
+          manifest is present but unparseable. It MUST skip only when there is no
+          evidence the demo ran: no `devlogs/`, no scenario sub-directory, or
+          neither ledger files nor a manifest.
+        rationale: >-
+          Skipping on absent data is right for a developer who has not run the
+          demo and wrong for CI, where absent data means the scenario died. The
+          manifest is the evidence that distinguishes the two cases; without it
+          the harness reported green for scenarios that produced no ledger at
+          all.
+        relationships:
+          - rel_type: refines
+            spec_id: DEMOCI-04-001
+          - rel_type: implements
+            spec_id: DEMOCI-10-002
+
+      - id: DEMOCI-10-004
+        priority: MUST
+        kind: project
+        statement: >-
+          A failure inside the case-ledger dump MUST NOT replace the exception
+          that ended the scenario. The dump MUST record its own failure through
+          the accumulator and the manifest, and the original exception MUST
+          propagate unchanged.
+        rationale: >-
+          A dump that runs on the failure path runs precisely when the system is
+          already unhealthy, so it is the most likely thing to fail second. If it
+          raises over the first exception, the fix that preserves the artifacts
+          destroys the diagnosis instead — the same class of masking as the
+          `try/finally: assert_demo_success()` shape it replaces.
+        relationships:
+          - rel_type: refines
+            spec_id: DEMOCI-10-001

specs/multi-actor-demo.yaml

@@ -4,7 +4,7 @@ description: >-
   Requirements for multi-actor demo scenarios, Docker Compose orchestration, actor isolation,
   acceptance testing, and reproducibility. Includes per-scenario required
   event-type lists for the case-ledger invariant harness.
-version: 1.3.0
+version: 1.4.0
 scope:
 
 - prototype
@@ -2947,3 +2947,82 @@ groups:
     tags:
     - demo
     - documentation
+- id: DEMOMA-23
+  title: Shared Scenario Harness
+  specs:
+  - id: DEMOMA-23-001
+    priority: MUST
+    kind: project
+    statement: >-
+      Every `run_<name>_demo()` orchestrator MUST run its phase body inside the
+      shared `scenario_harness("<demo-name>")` context manager from
+      `vultron.demo.helpers.harness`, which resets the failure accumulator, always
+      dumps the case ledgers, and only then asserts demo success. The scenario's
+      `main()` MUST NOT wrap the run in `try: ... finally: assert_demo_success()`.
+    rationale: >-
+      Each scenario previously repeated the same three responsibilities in a
+      slightly different order, and the `finally: assert_demo_success()` shape in
+      `main()` substituted a generic `DemoFailureError` for whatever actually went
+      wrong. One harness fixes the order once for all scenarios: run phases,
+      always dump, then assert.
+    relationships:
+    - rel_type: implements
+      spec_id: DEMOCI-10-001
+    - rel_type: refines
+      spec_id: DEMOMA-17-001
+    tags:
+    - demo
+    - testing
+  - id: DEMOMA-23-002
+    priority: MUST
+    kind: project
+    statement: >-
+      A scenario's `_phase_dump_case_ledgers()` MUST be a thin wrapper over
+      `vultron.demo.helpers.ledger_dump.dump_case_ledgers()`, declaring only that
+      scenario's dump targets (actor name, client, route key, and any fallback).
+      Per-actor fetching, 404 handling, JSONL writing, and manifest writing MUST
+      live in the shared helper, not in the scenario module.
+    rationale: >-
+      Nine copies of the same export loop is exactly the copy-paste that
+      DEMOMA-17-001 forbids, and it is why a defect in the dump had to be fixed
+      nine times. Keeping the scenario-specific participant map in the scenario
+      and the mechanics in the helper puts each fact in one place.
+    relationships:
+    - rel_type: refines
+      spec_id: DEMOMA-17-001
+    tags:
+    - demo
+  - id: DEMOMA-23-003
+    priority: MUST
+    kind: project
+    statement: >-
+      A scenario MUST register its dump with `harness.dump_with(...)` as soon as
+      a case exists — immediately after the report-submission phase returns —
+      rather than at the end of the run.
+    rationale: >-
+      The harness can only dump what it has been given. Registering the dump at
+      the first point where a case exists is what makes every later phase failure
+      survivable: the ledgers that existed at the moment of failure are still
+      exported.
+    relationships:
+    - rel_type: implements
+      spec_id: DEMOCI-10-001
+    tags:
+    - demo
+  - id: DEMOMA-23-004
+    priority: SHOULD
+    kind: project
+    statement: >-
+      When a scenario body raises, the harness SHOULD attach the accumulated
+      `demo_step`/`demo_check` failures to the propagating exception as notes
+      rather than raising `DemoFailureError` in their place.
+    rationale: >-
+      The accumulated soft failures are usually downstream consequences of the
+      exception, so they are context, not the cause. Attaching them as notes keeps
+      both: the reader sees the failure that stopped the run and every check that
+      had already failed.
+    relationships:
+    - rel_type: refines
+      spec_id: DEMOCI-10-004
+    tags:
+    - demo