CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — The three new flags (--skip-checks, --skip-check <name>, --skip-check-label <label>) added to workflow run are identical in name, description, and behavior to the existing flags on model method run, which is exactly what you want for parity. Help text is clear, flags use collect correctly, JSON output is unaffected (these are execution control flags, not output fields), and both log and JSON output modes are already handled by the existing renderer. No UX regressions visible.

Adversarial Review

Critical / High

None found.

Medium

1. Degraded zodToJsonSchema fallback in src/domain/models/output_spec_builder.ts:28-33

   The new zodToJsonSchema function catches conversion failures and falls back to { type: "object" }. The original in src/libswamp/types/schema_helpers.ts:175-181 falls back to a comprehensive manualZodToJsonSchema that handles strings, numbers, booleans, arrays, records, enums, literals, and unknowns. If a model uses z.record(z.unknown()) or z.array(z.string()) as a resource schema — types known to crash z.toJSONSchema() per the comment on line 102 of schema_helpers.ts — the new code will silently produce { type: "object" } instead of an accurate schema representation. This affects report context data integrity for consumers that rely on outputSpecs[].schema.

   Breaking example: A model with resources: { data: { schema: z.record(z.unknown()), ... } } will produce { type: "object" } from the workflow path but { type: "object", additionalProperties: {} } from the CLI path (which still uses the old schema_helpers.ts zodToJsonSchema). Report consumers comparing CLI vs workflow output specs will see different schemas.

   Suggested fix: Import and reuse zodToJsonSchema from ../../libswamp/types/schema_helpers.ts instead of duplicating it with a weaker fallback — or move the comprehensive version to the domain layer alongside buildOutputSpecs.

2. Ratchet bump to KNOWN_MUTUAL_DEPENDENCIES = 11 in integration/architecture_boundary_test.ts:134

   The PR description states it fixes a layer violation by moving buildOutputSpecs from libswamp to domain, but it introduces a new mutual dependency between bounded contexts in the process (bumping the ratchet from 10 to 11). The PR description mentions "Layer violation fix" but the net effect is trading one cross-boundary dependency for another. Worth confirming the new cycle is the intended domain/models → domain/reports (for OutputSpecInfo type import) and not something unintentional.

Low

1. reportRegistry.getAll().length > 0 called twice in the error path (src/domain/workflows/execution_service.ts:756) — getAll() builds a fresh array on each call. Not a correctness issue, but on the success path (line 598) it's also called this way. Minor inefficiency if the registry is large.

2. No model-scope reports on failure (src/domain/workflows/execution_service.ts:826) — The success path runs both method-scope and model-scope reports (lines 700-725). The failure path only runs method-scope reports. This matches the CLI failure path in run.ts:488 which also only runs method-scope, so it's consistent — but worth noting that model-scope reports never see failures in either path.

Verdict

PASS — The changes are structurally sound, properly thread context through the full workflow chain, and maintain consistency with the existing CLI path. The zodToJsonSchema fallback divergence (Medium #1) is the most actionable finding — it creates a subtle behavioral difference between the two paths that this PR is specifically trying to unify. The architecture ratchet bump deserves a quick confirmation. No blocking issues found.

Code Review

Blocking Issues

None — this PR is well-structured and addresses a real parity gap between the CLI and workflow execution paths.

Suggestions

1. zodToJsonSchema fallback divergence (src/domain/models/output_spec_builder.ts:28-33): The new zodToJsonSchema falls back to { type: "object" } on failure, while the original in src/libswamp/types/schema_helpers.ts:175-181 falls back to a comprehensive manualZodToJsonSchema that handles records, arrays, enums, literals, etc. This means workflow-path reports will get degraded schema info when z.toJSONSchema() fails (e.g., z.record(z.unknown())). Consider importing and reusing the existing fallback, or extracting the manual converter to the domain layer alongside buildOutputSpecs.

2. New models ↔ reports mutual dependency (ratchet 10→11): The extraction of buildOutputSpecs into src/domain/models/ creates a new circular dependency because it imports OutputSpecInfo from src/domain/reports/report_context.ts. One option to avoid this would be to define OutputSpecInfo in a shared types module or in the models bounded context (since it describes model output structure). Not blocking since the ratchet mechanism explicitly tracks this, but worth considering as a follow-up to keep the dependency count from growing.

3. Large duplicated report execution block (src/domain/workflows/execution_service.ts:754-848): The ~95-line block for running reports on failed executions closely mirrors the success-path report block (~30 lines above). Consider extracting a shared helper to reduce duplication and ensure both paths stay in sync. Not blocking since the code is correct and tested.