Files
open-design/apps/daemon/tests/critique-orchestrator.test.ts
76e6c7a9f6 feat: Critique Theater Phase 4 (persistence + transcript + orchestrator) (#481)
* docs(specs): add Critique Theater design spec for panel-tempered artifacts

* docs(specs): add Critique Theater implementation plan

* docs(specs): rename UI to Design Jury, add lane-density modes, ship-rule explainer, label sizing

* feat(contracts): add CritiqueConfig schema and defaults

* fix(contracts): apply Task 1.1 review (CRITIQUE_PROTOCOL_VERSION rename, descriptions, RoleWeights export)

* feat(contracts): add PanelEvent discriminated union and isPanelEvent guard

* fix(contracts): apply Task 1.2 review (exhaustive event-type list, runId guard, import order)

* feat(contracts): add CritiqueSseEvent variants and panelEventToSse mapper

* test(daemon): add v1 wire-protocol golden fixtures for Critique Theater parser

* feat(daemon): add v1 streaming parser for Critique Theater wire protocol

* chore(contracts): add .js extensions to relative imports for NodeNext consumers

* fix(daemon): satisfy noUncheckedIndexedAccess in v1 parser regex match access

* test(daemon): cover parser failure modes; fix unclosed-PANELIST swallow bug

* fix(daemon,contracts): address PR #387 review

- parser now clamps panelist + DIM scores against the run-declared scale
  captured from <CRITIQUE_RUN scale=...>, not a hardcoded 100
- PANELIST appearing before any <ROUND n=...> opens now throws
  MalformedBlockError rather than emitting events with NaN round
- DIM_RE and MUST_FIX_RE hoisted to module scope and lastIndex reset per
  call so the parser hot path stops recompiling regex per artifact
- overflow check after drain simplified to a plain buf.length > cap test
  (the prior compound condition was always true on the right side and
  obscured intent)
- scoreThreshold <= scoreScale refine gains a 1e-9 epsilon so floating
  slack does not reject semantically valid configs
- round-1 designer ARTIFACT guard gains a comment naming the spec
  invariant and the v2 relaxation path
- 3 new regression tests cover the panelist-without-round, scale=10
  clamp, and scale=20 plumbing cases

* docs(specs): rationale for non-goals, failure-mode rate targets, Phase 10 matrix, Phase 14 doc layout

* Merge branch 'main' into feat/critique-theater

Resolves the contracts/index.ts conflict by keeping the .js extensions added
by chore(contracts) 2d6e8d6 and slotting in the new export for ./api/app-config
introduced upstream by #255 (9d700ec). Critique Theater additions
(./sse/critique, ./critique) preserved in their original positions.

Verified after merge:
  pnpm --filter @open-design/contracts test    -> 10/10 pass
  pnpm --filter @open-design/contracts typecheck -> exit 0
  pnpm --filter @open-design/daemon typecheck  -> exit 0
  pnpm --filter @open-design/web typecheck     -> exit 0

Two daemon tests in tests/media-config.test.ts fail both before and after the
merge because they read real OAuth credentials from the developer machine
instead of using mock fixtures. That's an upstream isolation issue on
origin/main, not something this branch introduces.

* fix: unblock web build and address mrcfps PANELIST oversize bypass

The chore commit that added .js extensions to satisfy daemon's nodenext
typecheck broke apps/web's Next.js build, because webpack tried to resolve
the literal ./common.js when only common.ts exists on disk. Replaced with
a subpath approach: contracts/exports gains a './critique' entry pointing
straight at src/critique.ts (which has no relative imports), and daemon
imports route through @open-design/contracts/critique instead of the
barrel. Web keeps the bundler-friendly barrel; daemon's nodenext walks
only the leaf module. All 13 contracts source files reverted to no-.js.

Separately, mrcfps flagged that parserMaxBlockBytes was only enforced on
the leftover buffer after drain returned, so a complete oversized block
arriving in one chunk slipped past the cap. Added an explicit per-block
size check inside drain for every buffered block type (PANELIST,
ROUND_END, SHIP). Three regression tests yield the whole stream as a
single chunk and assert OversizeBlockError fires before any events emit.

* fix(daemon): close three v1 parser invariant gaps from mrcfps review

Three independent gaps that all let malformed or oversized protocol
output pass the v1 envelope contract:

(1) Envelope guard. ROUND, PANELIST, ROUND_END, and SHIP now throw
MalformedBlockError when state.inRun is false. Without this, a stream
that omits <CRITIQUE_RUN> could still emit panelist_* events without
the run_started handshake, leaving downstream reducers with no run-level
config.

(2) UTF-8 byte length. Both the per-block size check and the post-drain
buf-size check now compare Buffer.byteLength(text, 'utf8') against
parserMaxBlockBytes. The previous string-length comparison let multibyte
content (CJK, emoji) inside <NOTES>/<SUMMARY> exceed the configured
byte cap while staying under the JS string length cap, bypassing the
daemon's resource guard.

(3) Header-end ordering. PANELIST, ROUND_END, and SHIP now require the
opener's > to appear before the matched closing tag. A malformed opener
like <PANELIST role="x" score="8"</PANELIST> previously fell through
to the closing tag's > and emitted events for an invalid block.

Four regression tests cover each gap (ROUND-without-run,
SHIP-without-run, multibyte-byte-cap, malformed-opener).

* feat(daemon): add critique_runs persistence (Task 4.1)

Introduces a new SQLite table critique_runs to back the orchestrator's
run lifecycle. Plan called for ALTER TABLE artifacts ADD COLUMN ..., but
artifacts is not a DB concept in this repo; runs get their own table.

- migrateCritique(db) creates the table + two indexes idempotently and
  is wired into the existing migrate(db) flow on daemon boot.
- CRUD helpers (insertCritiqueRun, getCritiqueRun, updateCritiqueRun,
  listCritiqueRunsByProject, deleteCritiqueRun) round-trip rounds_json
  through helpers so callers see typed CritiqueRunRow.
- reconcileStaleRuns flips stale 'running' rows to 'interrupted' with
  a recoveryReason='daemon_restart' marker, supporting the spec's
  daemon-restart-mid-run failure mode.
- Public CritiqueRunStatus union excludes the in-flight 'running' value
  but the runtime CHECK accepts it, matching the spec's lifecycle.
- 11 vitest cases cover migration idempotence, round-trip, default
  rounds, status validation, update + list ordering, deletion, and
  reconciliation, plus FK CASCADE on project deletion.

* feat(daemon): add Critique Theater transcript writer (Task 4.2)

Streams PanelEvent sequences to .ndjson on disk under the artifact dir,
gzipping to .ndjson.gz when the cumulative UTF-8 byte size crosses
gzipThresholdBytes (default 256 KiB). Uses Node fs streams plus
zlib.createGzip so the writer never holds the full transcript in memory.
readTranscript inverts the path and streams events back, picking the
right pipeline by file extension. Covers happy path, large multibyte,
empty input, mid-stream failure cleanup, and unknown-extension reject.

* feat(daemon): add Critique Theater orchestrator (Task 4.3)

Drives one run end-to-end: parses stdout via parseCritiqueStream, scores
each round through scoreboard helpers, persists lifecycle to critique_runs,
and emits CritiqueSseEvent variants on the existing project event bus.
Honors per-round and total timeouts, applies fallbackPolicy when no
<SHIP> arrives, and tees events into writeTranscript so transcripts
stream to disk without buffering the whole run in memory. Defensive entry
validation throws RangeError on invalid CritiqueConfig before any side
effect.

Also adds scoreboard.ts (computeComposite, decideRound, selectFallbackRound)
and re-exports panelEventToSse/CritiqueSseEvent from the critique subpath
so daemon imports never touch the barrel. Fixes missing .js extensions in
sse/critique.ts that caused NodeNext module resolution errors.

* feat(daemon): wire Critique Theater orchestrator into spawn path (Task 4.4)

Adds loadCritiqueConfigFromEnv to read OD_CRITIQUE_* keys with strict
validation at boot. Branches the existing CLI spawn flow on cfg.enabled:
when false (the M0 default) the legacy single-pass generation runs
unchanged; when true the orchestrator owns the run end-to-end. Same SSE
bus, same artifact dir, no behavior change for users until they flip the
flag.

* fix(lockfile): regenerate to include contracts zod + vitest entries

The earlier conflict resolution took main's lockfile and ran pnpm
install, but the install pass on Windows didn't write the contracts
package's zod and vitest entries back into the lockfile. CI's
--frozen-lockfile install rejected the resulting state. Re-running
pnpm install with --no-frozen-lockfile rewrites the lockfile so it
now matches every package.json across the workspace, including
contracts/zod ^3.23.8 and contracts/vitest ^2.1.8. Verified locally:
pnpm install --frozen-lockfile passes.

* fix(daemon): parser ship envelope, SHIP-before-round guard, real artifactRef (Defects 3 + 5)

- ParserOptions gains projectId + artifactId; the parser threads them into
  every emitted ship event's artifactRef so downstream consumers see the
  real run identity instead of empty placeholders.
- <SHIP> now requires at least one closed <ROUND_END> in the same run;
  malformed streams that emit SHIP before any round complete now throw
  MalformedBlockError instead of bypassing the round-1 artifact invariant.
- The SHIP handler validates the inner <ARTIFACT> block is present and
  non-empty; missing artifact raises MissingArtifactError.
- Three new regressions: SHIP-before-round, SHIP-without-artifact,
  artifactRef populated from parser options.
- Orchestrator threads projectId + artifactId into parserOpts.
- Test fixtures updated to include <ARTIFACT> inside <SHIP> blocks.

* fix(daemon): orchestrator owns lifecycle, gzip atomicity, fallback on timeout (Defects 2,4,7,8)

- Orchestrator now accepts child + childExitPromise, races parser /
  child-exit / abort / timeout in one awaited flow, and SIGTERMs the
  child on every non-clean termination. Server awaits the result so
  the run lifecycle has a single owner.
- ChildExitError surfaces when child exits non-zero mid-stream; the
  run is classified as failed with cause cli_exit_nonzero.
- Timeout / abort with at least one completed round elects a fallback
  via selectFallbackRound and emits a synthetic ship event with
  status=timed_out or interrupted; the score persists to
  critique_runs instead of staying null.
- applyTimeouts includes childExitRace in every Promise.race so early
  child exits are classified without waiting for the total timeout.
  iter.return() cleanup is capped at 200ms to prevent hang on
  stalling generators.
- writeTranscript writes gzip output to transcript.ndjson.gz.tmp,
  fsyncs, then atomic-renames. Crashes mid-write leave no partial
  .gz or .gz.tmp on disk.

* fix(daemon): plain-stream gating, per-run artifact dir, boot reconcile (Defects 1, 2, 6)

- Spawn-path branch now inspects def.streamFormat and only routes through
  runOrchestrator when format === 'plain'. Adapters emitting wrapper
  formats (claude-stream-json, copilot-stream-json, json-event-stream,
  acp-json-rpc, pi-rpc) fall through to legacy single-pass with a
  one-time stderr warning per format. Per-format decoding into the
  orchestrator is reserved for v2.
- critiqueArtifactDir is now path.join(ARTIFACTS_DIR, projectId, runId)
  so concurrent or sequential runs in the same project never overwrite
  each other's transcript or final HTML. Persistence stores the relative
  per-run path.
- reconcileStaleRuns is now invoked after openDatabase on every daemon
  boot with staleAfterMs = critiqueCfg.totalTimeoutMs. Stale running
  rows from a prior crash flip to interrupted with rounds_json.
  recoveryReason='daemon_restart'. Logs a one-line warning naming the
  flipped count when greater than zero.
- Spawn now passes child + childExitPromise to runOrchestrator so the
  orchestrator can race child exit against the parser, abort signal,
  and timeouts in one awaited flow. Server awaits the orchestrator's
  result and surfaces failures through the existing run lifecycle.

* fix(daemon): daemon-authoritative scoring, lifecycle status, stderr ordering, insert type

Round 2 review feedback on PR #481.

1. CritiqueRunInsert.status now accepts 'running' so the boot-reconcile
   tests (and any caller seeding an in-flight row) typecheck without
   casting. The runtime check in insertCritiqueRun already accepted
   'running' against the DB constraint set, only the public type was
   stricter than the DB.
2. round_end keeps the daemon-computed composite authoritative. The
   agent's <ROUND_END composite=...> attribute is advisory: a divergence
   beyond COMPOSITE_TOLERANCE emits a composite_mismatch parser_warning
   so the discrepancy is observable, but the daemon value is what scores
   and persists. Same policy for must_fix.
3. SHIP-handling derives the final status from decideRound(...) using the
   daemon's scored round rather than trusting <SHIP composite=... status=...>.
   A run that the agent claims as shipped but whose daemon composite is
   below threshold now finalizes as below_threshold, so a malformed or
   adversarial stream cannot force a ship.
4. server.ts captures the orchestrator's result and maps the critique
   terminal status to the chat run lifecycle. shipped/below_threshold
   finalize as 'succeeded'; timed_out/interrupted/degraded/failed
   finalize as 'failed'. cancelRequested is honored.
5. stderr forwarding and child.on('error') registrations moved BEFORE
   the orchestrator await so a CLI that floods stderr cannot fill the
   OS pipe and deadlock until the total timeout, and so an early
   child error fired during the run is observed by the same listener
   used after.

Tests:
- tests/critique-authority.test.ts: 3 new regressions (lying ship
  downgraded to below_threshold, mismatch warning emitted, aligned
  composites stay quiet).
- All four affected suites green: 14 orchestrator + 10 spawn-wiring +
  3 boot-reconcile + 3 authority = 30/30.

Workspace typechecks: contracts, daemon, web all exit 0.

* fix(daemon,contracts): inline critique SSE, signal-terminated child, null shipped artifactPath

Round 3 review feedback on PR #481.

1. packages/contracts/src/critique.ts inlines CritiqueSseEvent +
   panelEventToSse + CRITIQUE_SSE_EVENT_NAMES + a local mirror of
   SseTransportEvent. The previous re-export from './sse/critique.js'
   broke the workspace web build (Turbopack cannot rewrite .js to .ts
   on a relative source import) while removing the .js extension broke
   daemon's NodeNext typecheck (it walks this leaf via the './critique'
   subpath export which requires explicit .js extensions). Inlining
   removes the cross-file relative import entirely so both consumers
   walk one self-contained file. packages/contracts/src/sse/critique.ts
   is removed and its co-located test moves up to
   packages/contracts/src/critique.test.ts. The barrel
   packages/contracts/src/index.ts drops the redundant
   './sse/critique' re-export since './critique' already exports the
   same symbols.

2. apps/daemon/src/critique/orchestrator.ts treats a signal-terminated
   child as a terminal race rejection. Previously the race only caught
   non-zero numeric exit codes and treated code === null as
   indefinitely pending, so a SIGTERM from /api/runs/:id/cancel
   resolved childExitPromise as { code: null, signal: 'SIGTERM' } and
   the orchestrator fell through to the no-SHIP fallback path,
   persisting below_threshold instead of interrupted. The race now
   rejects with a new ChildSignaledError when signal !== null, and a
   new catch branch classifies the run as 'interrupted' and (if at
   least one round closed) emits a synthetic ship event with
   status='interrupted' so the persisted row and the SSE transcript
   reflect the actual cause.

3. Same file, ship-handling: artifactPath is now persisted as null on
   shipped runs until a future phase actually extracts the
   <SHIP><ARTIFACT> body to disk. Previously the orchestrator wrote
   ${artifactDir}/${artifactId} even though no file existed at that
   path, so any later replay/export/UI code that trusted
   critique_runs.artifact_path would dereference a missing file. The
   transcript still records the ship event with the artifact reference
   so consumers can find the run.

Tests:
- apps/daemon/tests/critique-lifecycle.test.ts: 2 new regressions
  (SIGTERM-terminated child after one closed round persists
  'interrupted' with a synthetic ship event of the same status; shipped
  run leaves artifactPath null in result and DB row).
- 43 critique-suite tests pass: 14 orchestrator + 11 transcript +
  10 spawn-wiring + 3 boot-reconcile + 3 authority + 2 lifecycle.

Workspace typechecks: contracts, daemon, web all exit 0.

* fix(daemon): buffer raw SHIP, emit only normalized; reject SHIP for unclosed round

Round 4 review feedback on PR #481.

The parser-event loop used to unconditionally collectedEvents.push(event)
and bus.emit(panelEventToSse(event)) for every event, including raw
<SHIP>. SSE clients and the transcript could see the agent's forged
status="shipped" / composite="9.5" before decideRound(...) ran, even
when the daemon later corrected the persisted DB row to below_threshold.
The loop now skips ship events entirely; the orchestrator buffers the
raw shipEvent, runs daemon-authoritative scoring, and emits a single
normalized ship payload built from the daemon's computed composite,
selectFallbackRound's mustFix, and decideRound's status. The transcript
and SSE bus now only ever see the daemon-scored ship.

The unknown-round fallback used to make agent-claimed status/composite
authoritative when SHIP referenced a round that was never closed: a
malformed stream could close low round 1, then send <SHIP round="2"
status="shipped" composite="10">, completedRounds.find(r => r.n === 2)
was undefined, and the orchestrator persisted the agent's value. That
re-opened the scoring-integrity hole the previous round was meant to
close. The orchestrator now drops a SHIP whose round isn't in
completedRounds, emits a parser_warning, and falls through to the
no-SHIP fallback policy. The synthetic ship from selectFallbackRound
gets emitted instead, with daemon-authoritative round/composite/status.

Tests:
- tests/critique-authority.test.ts: extended the lying-ship regression
  to also assert the emitted critique.ship payload is downgraded
  (status='below_threshold', composite < threshold), so the SSE bus
  cannot see the agent's claim. Added a new regression where SHIP
  references an unclosed round 2: the agent ship is dropped, a
  parser_warning fires, the fallback selects round 1, and the only
  emitted critique.ship has round=1 and status=below_threshold.
- 44 critique-suite tests pass: 14 orchestrator + 11 transcript + 10
  spawn-wiring + 3 boot-reconcile + 4 authority + 2 lifecycle.

Workspace daemon typecheck exits 0.

---------

Co-authored-by: Nagendhra <nagendhra405@gmail.com>
Co-authored-by: mrcfps <mrc@powerformer.com>
2026-05-05 15:50:35 +08:00

778 lines
28 KiB
TypeScript

import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { mkdtempSync, existsSync, readFileSync } from 'node:fs';
import { rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import Database from 'better-sqlite3';
import { migrateCritique, getCritiqueRun } from '../src/critique/persistence.js';
import { runOrchestrator, type CritiqueSseBus, type OrchestratorParams } from '../src/critique/orchestrator.js';
import type { CritiqueSseEvent } from '@open-design/contracts/critique';
import { defaultCritiqueConfig, type CritiqueConfig } from '@open-design/contracts/critique';
// ---------------------------------------------------------------------------
// DB fixture
// ---------------------------------------------------------------------------
function freshDb(): Database.Database {
const db = new Database(':memory:');
db.pragma('journal_mode = WAL');
db.pragma('foreign_keys = ON');
db.exec(`
CREATE TABLE projects (
id TEXT PRIMARY KEY,
name TEXT NOT NULL,
created_at INTEGER NOT NULL,
updated_at INTEGER NOT NULL
);
CREATE TABLE conversations (
id TEXT PRIMARY KEY,
project_id TEXT NOT NULL,
created_at INTEGER NOT NULL,
updated_at INTEGER NOT NULL,
FOREIGN KEY(project_id) REFERENCES projects(id) ON DELETE CASCADE
);
INSERT INTO projects (id, name, created_at, updated_at) VALUES ('p1', 'p1', 0, 0);
INSERT INTO conversations (id, project_id, created_at, updated_at) VALUES ('c1', 'p1', 0, 0);
`);
migrateCritique(db);
return db;
}
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
function makeBus(): { bus: CritiqueSseBus; events: CritiqueSseEvent[] } {
const events: CritiqueSseEvent[] = [];
const bus: CritiqueSseBus = { emit: (e) => { events.push(e); } };
return { bus, events };
}
/**
* Builds a minimal 3-round happy-path wire protocol stream. Uses a threshold
* low enough (1.0) so every round passes, meaning SHIP with status=shipped.
*/
function happyStream3Rounds(): string {
return `<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">
<ROUND n="1">
<PANELIST role="designer">
<NOTES>Design intent v1.</NOTES>
<ARTIFACT mime="text/html"><![CDATA[<html></html>]]></ARTIFACT>
</PANELIST>
<PANELIST role="critic" score="9.0">
<DIM name="hierarchy" score="9">Good layout.</DIM>
</PANELIST>
<PANELIST role="brand" score="9.0">
<DIM name="voice" score="9">Strong brand.</DIM>
</PANELIST>
<PANELIST role="a11y" score="9.0">
<DIM name="contrast" score="9">Passes AA.</DIM>
</PANELIST>
<PANELIST role="copy" score="9.0">
<DIM name="clarity" score="9">Clear copy.</DIM>
</PANELIST>
<ROUND_END n="1" composite="9.0" must_fix="0" decision="continue">
<REASON>Composite 9.0 but continuing per test.</REASON>
</ROUND_END>
</ROUND>
<ROUND n="2">
<PANELIST role="designer">
<NOTES>Design intent v2.</NOTES>
</PANELIST>
<PANELIST role="critic" score="9.2">
<DIM name="hierarchy" score="9">Better.</DIM>
</PANELIST>
<PANELIST role="brand" score="9.1">
<DIM name="voice" score="9">Consistent.</DIM>
</PANELIST>
<PANELIST role="a11y" score="9.3">
<DIM name="contrast" score="9">Still passes.</DIM>
</PANELIST>
<PANELIST role="copy" score="9.0">
<DIM name="clarity" score="9">Still clear.</DIM>
</PANELIST>
<ROUND_END n="2" composite="9.15" must_fix="0" decision="continue">
<REASON>Continuing to round 3.</REASON>
</ROUND_END>
</ROUND>
<ROUND n="3">
<PANELIST role="designer">
<NOTES>Design intent v3.</NOTES>
</PANELIST>
<PANELIST role="critic" score="9.5">
<DIM name="hierarchy" score="9">Excellent.</DIM>
</PANELIST>
<PANELIST role="brand" score="9.4">
<DIM name="voice" score="9">Perfect.</DIM>
</PANELIST>
<PANELIST role="a11y" score="9.6">
<DIM name="contrast" score="9">Excellent.</DIM>
</PANELIST>
<PANELIST role="copy" score="9.3">
<DIM name="clarity" score="9">Great.</DIM>
</PANELIST>
<ROUND_END n="3" composite="9.45" must_fix="0" decision="ship">
<REASON>Threshold met.</REASON>
</ROUND_END>
</ROUND>
<SHIP round="3" composite="9.45" status="shipped">
<ARTIFACT mime="text/html"><![CDATA[<html><body>final</body></html>]]></ARTIFACT>
<SUMMARY>Design converged in 3 rounds.</SUMMARY>
</SHIP>
</CRITIQUE_RUN>`;
}
async function* streamOf(text: string, chunkSize = 64): AsyncIterable<string> {
for (let i = 0; i < text.length; i += chunkSize) {
yield text.slice(i, i + chunkSize);
}
}
// ---------------------------------------------------------------------------
// Setup / Teardown
// ---------------------------------------------------------------------------
let tmpDir: string;
let db: Database.Database;
beforeEach(() => {
tmpDir = mkdtempSync(join(tmpdir(), 'od-orch-test-'));
db = freshDb();
});
afterEach(async () => {
db.close();
await rm(tmpDir, { recursive: true, force: true });
});
// ---------------------------------------------------------------------------
// Happy path
// ---------------------------------------------------------------------------
describe('runOrchestrator - happy path', () => {
it('3-round shipped run: row reflects shipped + composite + rounds + transcript path', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run1');
const cfg = defaultCritiqueConfig();
const result = await runOrchestrator({
runId: 'r1',
projectId: 'p1',
conversationId: 'c1',
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: streamOf(happyStream3Rounds()),
});
expect(result.status).toBe('shipped');
expect(result.composite).toBeCloseTo(9.45, 1);
expect(result.rounds).toHaveLength(3);
expect(result.transcriptPath).toBeTruthy();
const row = getCritiqueRun(db, 'r1');
expect(row?.status).toBe('shipped');
expect(row?.rounds).toHaveLength(3);
expect(row?.transcriptPath).toBeTruthy();
// Transcript file exists on disk.
const transcriptFile = join(artifactDir, result.transcriptPath!);
expect(existsSync(transcriptFile)).toBe(true);
// SSE events emitted: should include run_started and ship.
const eventNames = events.map((e) => e.event);
expect(eventNames).toContain('critique.run_started');
expect(eventNames).toContain('critique.ship');
});
it('SSE events are emitted in source order', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-order');
await runOrchestrator({
runId: 'r-order',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg: defaultCritiqueConfig(),
db,
bus,
stdout: streamOf(happyStream3Rounds()),
});
const names = events.map((e) => e.event);
const runStartedIdx = names.indexOf('critique.run_started');
const shipIdx = names.lastIndexOf('critique.ship');
expect(runStartedIdx).toBe(0);
expect(shipIdx).toBeGreaterThan(runStartedIdx);
});
});
// ---------------------------------------------------------------------------
// Malformed / degraded
// ---------------------------------------------------------------------------
describe('runOrchestrator - degraded', () => {
it('malformed input: row is degraded, critique.degraded emitted, no transcript path in row (transcript may still be written)', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-malformed');
// Malformed: ROUND before CRITIQUE_RUN.
const malformedText = `<ROUND n="1"><PANELIST role="critic" score="9"></PANELIST></ROUND>`;
const result = await runOrchestrator({
runId: 'r-malformed',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg: defaultCritiqueConfig(),
db,
bus,
stdout: streamOf(malformedText),
});
expect(result.status).toBe('degraded');
const row = getCritiqueRun(db, 'r-malformed');
expect(row?.status).toBe('degraded');
const degradedEvents = events.filter((e) => e.event === 'critique.degraded');
expect(degradedEvents).toHaveLength(1);
});
});
// ---------------------------------------------------------------------------
// Fallback policy
// ---------------------------------------------------------------------------
describe('runOrchestrator - fallback policy', () => {
it('below threshold: stream ends without SHIP, ship_best selects highest composite', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-below');
// 2 rounds but no SHIP - scores below threshold (default 8.0).
const noShipText = `<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">
<ROUND n="1">
<PANELIST role="designer">
<NOTES>v1</NOTES>
<ARTIFACT mime="text/html"><![CDATA[<html></html>]]></ARTIFACT>
</PANELIST>
<PANELIST role="critic" score="6.0">
<DIM name="h" score="6">needs work</DIM>
<MUST_FIX>Fix hierarchy</MUST_FIX>
</PANELIST>
<PANELIST role="brand" score="6.0"><DIM name="v" score="6">ok</DIM></PANELIST>
<PANELIST role="a11y" score="6.0"><DIM name="c" score="6">ok</DIM></PANELIST>
<PANELIST role="copy" score="6.0"><DIM name="cl" score="6">ok</DIM></PANELIST>
<ROUND_END n="1" composite="6.0" must_fix="1" decision="continue">
<REASON>Below threshold.</REASON>
</ROUND_END>
</ROUND>
<ROUND n="2">
<PANELIST role="designer"><NOTES>v2</NOTES></PANELIST>
<PANELIST role="critic" score="7.0"><DIM name="h" score="7">better</DIM></PANELIST>
<PANELIST role="brand" score="7.0"><DIM name="v" score="7">ok</DIM></PANELIST>
<PANELIST role="a11y" score="7.0"><DIM name="c" score="7">ok</DIM></PANELIST>
<PANELIST role="copy" score="7.0"><DIM name="cl" score="7">ok</DIM></PANELIST>
<ROUND_END n="2" composite="7.0" must_fix="0" decision="continue">
<REASON>Still below threshold.</REASON>
</ROUND_END>
</ROUND>
</CRITIQUE_RUN>`;
const cfg: CritiqueConfig = { ...defaultCritiqueConfig(), fallbackPolicy: 'ship_best' };
const result = await runOrchestrator({
runId: 'r-below',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: streamOf(noShipText),
});
expect(result.status).toBe('below_threshold');
// ship_best should select round 2 (composite 7.0 > 6.0).
expect(result.composite).toBeGreaterThan(6.0);
const row = getCritiqueRun(db, 'r-below');
expect(row?.status).toBe('below_threshold');
const shipEvents = events.filter((e) => e.event === 'critique.ship');
expect(shipEvents).toHaveLength(1);
});
it('fallback policy fail: row is failed, no synthetic ship event', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-failpolicy');
const noShipText = `<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">
<ROUND n="1">
<PANELIST role="designer">
<NOTES>v1</NOTES>
<ARTIFACT mime="text/html"><![CDATA[<html></html>]]></ARTIFACT>
</PANELIST>
<PANELIST role="critic" score="6.0"><DIM name="h" score="6">ok</DIM></PANELIST>
<PANELIST role="brand" score="6.0"><DIM name="v" score="6">ok</DIM></PANELIST>
<PANELIST role="a11y" score="6.0"><DIM name="c" score="6">ok</DIM></PANELIST>
<PANELIST role="copy" score="6.0"><DIM name="cl" score="6">ok</DIM></PANELIST>
<ROUND_END n="1" composite="6.0" must_fix="0" decision="continue">
<REASON>Below threshold.</REASON>
</ROUND_END>
</ROUND>
</CRITIQUE_RUN>`;
const cfg: CritiqueConfig = { ...defaultCritiqueConfig(), fallbackPolicy: 'fail' };
const result = await runOrchestrator({
runId: 'r-failpolicy',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: streamOf(noShipText),
});
expect(result.status).toBe('failed');
const row = getCritiqueRun(db, 'r-failpolicy');
expect(row?.status).toBe('failed');
const shipEvents = events.filter((e) => e.event === 'critique.ship');
expect(shipEvents).toHaveLength(0);
});
});
// ---------------------------------------------------------------------------
// Timeout
// ---------------------------------------------------------------------------
describe('runOrchestrator - timeouts', () => {
it('per-round timeout: stalled stream causes timed_out row', async () => {
const { bus } = makeBus();
const artifactDir = join(tmpDir, 'run-round-timeout');
// Source that yields initial data then stalls past the per-round timeout.
async function* stallingSource(): AsyncIterable<string> {
yield '<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">\n';
yield ' <ROUND n="1">\n';
yield ' <PANELIST role="designer">\n';
yield ' <NOTES>v1</NOTES>\n';
yield ' <ARTIFACT mime="text/html"><![CDATA[<html></html>]]></ARTIFACT>\n';
yield ' </PANELIST>\n';
// Stall: never send ROUND_END, timeout will fire.
await new Promise<void>((_, reject) => setTimeout(() => reject(new Error('stall')), 200));
}
const cfg: CritiqueConfig = {
...defaultCritiqueConfig(),
perRoundTimeoutMs: 50,
totalTimeoutMs: 60_000,
};
const result = await runOrchestrator({
runId: 'r-round-timeout',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: stallingSource(),
});
expect(result.status).toBe('timed_out');
const row = getCritiqueRun(db, 'r-round-timeout');
expect(row?.status).toBe('timed_out');
}, 5000);
it('total timeout: wall-clock deadline exceeded causes timed_out row', async () => {
const { bus } = makeBus();
const artifactDir = join(tmpDir, 'run-total-timeout');
async function* slowSource(): AsyncIterable<string> {
yield '<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">\n';
await new Promise<void>((_, reject) => setTimeout(() => reject(new Error('total stall')), 200));
}
const cfg: CritiqueConfig = {
...defaultCritiqueConfig(),
perRoundTimeoutMs: 60_000,
totalTimeoutMs: 50,
};
const result = await runOrchestrator({
runId: 'r-total-timeout',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: slowSource(),
});
expect(result.status).toBe('timed_out');
const row = getCritiqueRun(db, 'r-total-timeout');
expect(row?.status).toBe('timed_out');
}, 5000);
});
// ---------------------------------------------------------------------------
// Abort signal
// ---------------------------------------------------------------------------
describe('runOrchestrator - abort signal', () => {
it('abort mid-run: row is interrupted, transcript captures events seen so far', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-abort');
const controller = new AbortController();
async function* abortingSource(): AsyncIterable<string> {
yield '<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">\n';
yield ' <ROUND n="1">\n';
yield ' <PANELIST role="designer">\n';
yield ' <NOTES>v1</NOTES>\n';
yield ' <ARTIFACT mime="text/html"><![CDATA[<html></html>]]></ARTIFACT>\n';
yield ' </PANELIST>\n';
// Abort mid-stream.
controller.abort();
yield ' <PANELIST role="critic" score="9">\n';
}
const result = await runOrchestrator({
runId: 'r-abort',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg: defaultCritiqueConfig(),
db,
bus,
stdout: abortingSource(),
signal: controller.signal,
});
expect(result.status).toBe('interrupted');
const row = getCritiqueRun(db, 'r-abort');
expect(row?.status).toBe('interrupted');
// Transcript should exist with partial events.
if (result.transcriptPath) {
expect(existsSync(join(artifactDir, result.transcriptPath))).toBe(true);
}
const interruptedEvents = events.filter((e) => e.event === 'critique.interrupted');
expect(interruptedEvents).toHaveLength(1);
});
});
// ---------------------------------------------------------------------------
// Defensive entry validation
// ---------------------------------------------------------------------------
describe('runOrchestrator - defensive entry', () => {
it('throws RangeError on invalid cfg (negative scoreThreshold) before any side effects', async () => {
const { bus } = makeBus();
const cfg: CritiqueConfig = { ...defaultCritiqueConfig(), scoreThreshold: -1 };
await expect(
runOrchestrator({
runId: 'r-invalid',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir: join(tmpDir, 'run-invalid'),
adapter: 'claude',
cfg,
db,
bus,
stdout: streamOf(''),
}),
).rejects.toThrow(RangeError);
// No row should have been inserted.
expect(getCritiqueRun(db, 'r-invalid')).toBeNull();
});
it('throws RangeError on invalid cfg (zero perRoundTimeoutMs)', async () => {
const { bus } = makeBus();
const cfg: CritiqueConfig = { ...defaultCritiqueConfig(), perRoundTimeoutMs: 0 };
await expect(
runOrchestrator({
runId: 'r-invalid2',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir: join(tmpDir, 'run-invalid2'),
adapter: 'claude',
cfg,
db,
bus,
stdout: streamOf(''),
}),
).rejects.toThrow(RangeError);
expect(getCritiqueRun(db, 'r-invalid2')).toBeNull();
});
});
// ---------------------------------------------------------------------------
// Child exit races (Defect 4)
// ---------------------------------------------------------------------------
describe('runOrchestrator - child exit race (Defect 4)', () => {
it('child exits non-zero mid-stream: result is failed with cli_exit_nonzero', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-child-exit');
// Stub child that exits with code 1 immediately.
let killCalled = false;
const stubChild = { kill: (_sig?: number | NodeJS.Signals) => { killCalled = true; return true as boolean; } };
// childExitPromise resolves with code=1 after a short delay.
const childExitPromise = new Promise<{ code: number | null; signal: string | null }>(
(resolve) => setTimeout(() => resolve({ code: 1, signal: null }), 20),
);
// Stdout that emits the run header then stalls.
// Uses a short delay (longer than childExitPromise's 20ms) so the child
// exit race wins before the stall promise resolves, but the generator
// itself does eventually resolve so iter.return() cleanup doesn't hang.
async function* stallingStdout(): AsyncIterable<string> {
yield '<CRITIQUE_RUN version="1" maxRounds="3" threshold="8.0" scale="10">\n';
yield ' <ROUND n="1">\n';
// Stall for longer than the child exit delay (20ms) but eventually resolve
// so the generator can be cleaned up by iter.return() in applyTimeouts.
await new Promise<void>((resolve) => setTimeout(resolve, 5000));
}
const cfg: CritiqueConfig = {
...defaultCritiqueConfig(),
// Long timeouts so only the child exit race wins; we don't want the
// per-round or total timer to fire before childExitPromise resolves.
perRoundTimeoutMs: 30_000,
totalTimeoutMs: 30_000,
};
const result = await runOrchestrator({
runId: 'r-child-exit',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: stallingStdout(),
child: stubChild,
childExitPromise,
});
expect(result.status).toBe('failed');
const row = getCritiqueRun(db, 'r-child-exit');
expect(row?.status).toBe('failed');
expect(killCalled).toBe(true);
const failedEvents = events.filter((e) => e.event === 'critique.failed');
expect(failedEvents).toHaveLength(1);
}, 10000);
it('child exits zero before parser completes: parser continues until stream ends', async () => {
const { bus } = makeBus();
const artifactDir = join(tmpDir, 'run-child-exit-zero');
// Child exits with code 0 (zero is not an error).
const childExitPromise = new Promise<{ code: number | null; signal: string | null }>(
(resolve) => setTimeout(() => resolve({ code: 0, signal: null }), 10),
);
// A complete valid 1-round stream that finishes after the child exit.
async function* delayedStream(): AsyncIterable<string> {
await new Promise<void>((r) => setTimeout(r, 30));
yield '<CRITIQUE_RUN version="1" maxRounds="1" threshold="8.0" scale="10">\n';
yield ' <ROUND n="1">\n';
yield ' <PANELIST role="designer"><NOTES>v1</NOTES><ARTIFACT mime="text/html"><![CDATA[<p>v1</p>]]></ARTIFACT></PANELIST>\n';
yield ' <PANELIST role="critic" score="9.0"><DIM name="h" score="9">ok</DIM></PANELIST>\n';
yield ' <PANELIST role="brand" score="9.0"><DIM name="v" score="9">ok</DIM></PANELIST>\n';
yield ' <PANELIST role="a11y" score="9.0"><DIM name="c" score="9">ok</DIM></PANELIST>\n';
yield ' <PANELIST role="copy" score="9.0"><DIM name="cl" score="9">ok</DIM></PANELIST>\n';
yield ' <ROUND_END n="1" composite="9.0" must_fix="0" decision="ship"><REASON>ok</REASON></ROUND_END>\n';
yield ' </ROUND>\n';
yield ' <SHIP round="1" composite="9.0" status="shipped">\n';
yield ' <ARTIFACT mime="text/html"><![CDATA[<p>final</p>]]></ARTIFACT>\n';
yield ' <SUMMARY>done</SUMMARY>\n';
yield ' </SHIP>\n';
yield '</CRITIQUE_RUN>\n';
}
const result = await runOrchestrator({
runId: 'r-child-exit-zero',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg: defaultCritiqueConfig(),
db,
bus,
stdout: delayedStream(),
childExitPromise,
});
// Zero exit does not disrupt the parser; it should complete as shipped.
expect(result.status).toBe('shipped');
}, 10000);
});
// ---------------------------------------------------------------------------
// Timeout / abort best-so-far fallback (Defect 7)
// ---------------------------------------------------------------------------
describe('runOrchestrator - fallback on timeout/abort (Defect 7)', () => {
it('timeout after 2 completed rounds: status=timed_out, score=max(composite)', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-timeout-fallback');
// 2 complete rounds then stall.
// Two complete rounds. After emitting both ROUND_END events, stall so the
// total-timeout fires and the orchestrator elects a fallback round.
const twoRounds = `<CRITIQUE_RUN version="1" maxRounds="3" threshold="9.0" scale="10">
<ROUND n="1">
<PANELIST role="designer"><NOTES>v1</NOTES><ARTIFACT mime="text/html"><![CDATA[<p>v1</p>]]></ARTIFACT></PANELIST>
<PANELIST role="critic" score="6.0"><DIM name="h" score="6">ok</DIM></PANELIST>
<PANELIST role="brand" score="6.0"><DIM name="v" score="6">ok</DIM></PANELIST>
<PANELIST role="a11y" score="6.0"><DIM name="c" score="6">ok</DIM></PANELIST>
<PANELIST role="copy" score="6.0"><DIM name="cl" score="6">ok</DIM></PANELIST>
<ROUND_END n="1" composite="6.0" must_fix="0" decision="continue"><REASON>continue</REASON></ROUND_END>
</ROUND>
<ROUND n="2">
<PANELIST role="designer"><NOTES>v2</NOTES></PANELIST>
<PANELIST role="critic" score="7.5"><DIM name="h" score="7">better</DIM></PANELIST>
<PANELIST role="brand" score="7.5"><DIM name="v" score="7">ok</DIM></PANELIST>
<PANELIST role="a11y" score="7.5"><DIM name="c" score="7">ok</DIM></PANELIST>
<PANELIST role="copy" score="7.5"><DIM name="cl" score="7">ok</DIM></PANELIST>
<ROUND_END n="2" composite="7.5" must_fix="0" decision="continue"><REASON>continue</REASON></ROUND_END>
</ROUND>`;
async function* stallingAfterTwoRounds(): AsyncIterable<string> {
yield* streamOf(twoRounds, 64);
// After both rounds land, stall so the total-timeout fires.
await new Promise<void>((resolve) => setTimeout(resolve, 10_000));
}
const cfg: CritiqueConfig = {
...defaultCritiqueConfig(),
// Per-round timeout starts when the first panelist_open of a new round
// fires. Set it to 200ms so it fires quickly once the stall begins.
// Total timeout is long so only the per-round timer fires.
// But we yield the stall after ROUND_END so no panelist_open is active.
// Use total timeout of 300ms which fires after the stall begins.
perRoundTimeoutMs: 60_000,
totalTimeoutMs: 300,
fallbackPolicy: 'ship_best',
};
const result = await runOrchestrator({
runId: 'r-timeout-fallback',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: stallingAfterTwoRounds(),
});
expect(result.status).toBe('timed_out');
// Best round is 2 with composite 7.5.
expect(result.composite).toBeCloseTo(7.5, 1);
const row = getCritiqueRun(db, 'r-timeout-fallback');
expect(row?.status).toBe('timed_out');
expect(row?.score).toBeCloseTo(7.5, 1);
// A synthetic ship event should have been emitted.
const shipEvents = events.filter((e) => e.event === 'critique.ship');
expect(shipEvents).toHaveLength(1);
}, 15000);
it('abort after 1 completed round: status=interrupted, score matches that round', async () => {
const { bus, events } = makeBus();
const artifactDir = join(tmpDir, 'run-abort-fallback');
const controller = new AbortController();
const oneRound = `<CRITIQUE_RUN version="1" maxRounds="3" threshold="9.0" scale="10">
<ROUND n="1">
<PANELIST role="designer"><NOTES>v1</NOTES><ARTIFACT mime="text/html"><![CDATA[<p>v1</p>]]></ARTIFACT></PANELIST>
<PANELIST role="critic" score="8.0"><DIM name="h" score="8">ok</DIM></PANELIST>
<PANELIST role="brand" score="8.0"><DIM name="v" score="8">ok</DIM></PANELIST>
<PANELIST role="a11y" score="8.0"><DIM name="c" score="8">ok</DIM></PANELIST>
<PANELIST role="copy" score="8.0"><DIM name="cl" score="8">ok</DIM></PANELIST>
<ROUND_END n="1" composite="8.0" must_fix="0" decision="continue"><REASON>continue</REASON></ROUND_END>
</ROUND>`;
async function* abortAfterRound(): AsyncIterable<string> {
yield* streamOf(oneRound, 64);
controller.abort();
// One more yield after abort to ensure the abort is caught.
yield ' <ROUND n="2">\n';
}
const cfg: CritiqueConfig = {
...defaultCritiqueConfig(),
fallbackPolicy: 'ship_best',
};
const result = await runOrchestrator({
runId: 'r-abort-fallback',
projectId: 'p1',
conversationId: null,
artifactId: 'a1',
artifactDir,
adapter: 'claude',
cfg,
db,
bus,
stdout: abortAfterRound(),
signal: controller.signal,
});
expect(result.status).toBe('interrupted');
expect(result.composite).toBeCloseTo(8.0, 1);
const row = getCritiqueRun(db, 'r-abort-fallback');
expect(row?.status).toBe('interrupted');
expect(row?.score).toBeCloseTo(8.0, 1);
const shipEvents = events.filter((e) => e.event === 'critique.ship');
expect(shipEvents).toHaveLength(1);
});
});