test(workflows): guard against workflow-level schema fields being silently dropped at parse (#2459)

* test(workflows): guard against workflow-level schema fields being silently dropped at parse

parseWorkflow hand-assembles its result field by field, so a field added to
workflowDefinitionSchema but not to that object literal is silently discarded:
the YAML parses, the workflow loads, and the feature is inert.

That already happened. `requires:` landed in workflowBaseSchema in ab81248
(2026-06-01) without touching the loader, and the assembly block only arrived in
2d7bf58 (2026-07-16) — six weeks in which the GitHub capability gate could never
fire for a discovered workflow, fixed incidentally inside an unrelated PR.

This is the third instance of one pattern: parallel enumerations that must agree
with nothing enforcing agreement. The ref-surface enumerations carry a KEEP IN
SYNC comment and were found broken anyway (#2450); the nested key sets are
derived from each schema's .shape and cannot drift (#2455). This applies the
derived form to the second case.

The field list comes from workflowDefinitionSchema.shape, so a new schema field
fails the test until it is given a fixture. Deliberately not solved by deriving
the assembly itself — the hand assembly exists because of warn-and-drop, and
schema.parse() would reject a bad field instead of logging and dropping it.

The per-field assertion clears the mock logger first so it can tell the two
failure causes apart: a warning means the fixture value is invalid (warn-and-drop
working as designed), silence means a valid field was dropped (the actual bug).

Verified by breaking it both ways: removing `requires` from the object literal
reproduces the historical bug and fails with the right diagnosis, and adding a
new schema key fails the ratchet until a fixture exists.

* test(workflows): tighten the parity guard after review

Addresses I1, I2, I3 and S1, S3, S4 from the review on #2459. No change to what
the guard catches; all six make a precision tool more precise.

I1 — the docblock claimed warn-and-drop universally. Re-verified the field audit
against loader.ts rather than taking it on faith: 4 of the 20 hard-reject
(name, description, nodes, evidence_policy at :619-629), 13 warn-and-drop, and 3
coerce silently with no log at all (provider :423, model :425, persist_sessions
:473 — there is no invalid_provider/invalid_model/invalid_persist_sessions warn
event anywhere in the file). Rewritten to say most rather than all, and to point
at loader.ts as the authority instead of restating a per-field table that would
rot the moment a field changes category.

I3 — the two-branch failure message was backwards for exactly those 3 silent
fields: a bad `provider: 123` fixture is discarded with no warning, so the
message confidently blamed the loader and sent the reader into parseWorkflow when
the fixture was at fault. That is the same failure the message exists to prevent,
and the one I hit during development with a bad `thinking: true` fixture. Fixed
by ranking rather than verdict: a warning is still strong evidence the fixture is
wrong, but silence now names both causes and points at the fixture first. Chosen
over listing the three exceptions in a comment, which would duplicate loader.ts
and rot. This subsumes S2's unstated-invariant concern.

I2 — effort, thinking and sandbox used presence checks where the other 17
fixtures check values, and their schemas transform deterministically, so exact
checks are available. Verified by mutation: returning effort:'low' and
thinking:{type:'disabled'} from the loader now fails both round-trips, where
before it left them green.

S1 — the hand-assembly literal predates 2d7bf58; only the requires entry landed
there. Reworded so it cannot be skimmed as "the mechanism didn't exist until then".

S3 — the two diagnostic strings moved out of the assertion into a named message.
S4 — nodes?.length, so a dropped nodes yields a clean false instead of a TypeError.

Verified: full validate green (132 batches, 0 fail); the I3 message re-checked by
running a deliberately invalid provider fixture; I2 re-checked by mutation.

Author: Wirasm

packages/workflows/src/loader.test.ts

@@ -35,6 +35,9 @@ registerBuiltinProviders();
 
 import { discoverWorkflows, discoverWorkflowsWithConfig } from './workflow-discovery';
 import { isBashNode, isCancelNode, isLoopNode } from './schemas';
+import { parseWorkflow } from './loader';
+import { workflowDefinitionSchema } from './schemas/workflow';
+import type { WorkflowDefinition } from './schemas/workflow';
 import * as bundledDefaults from './defaults/bundled-defaults';
 
 describe('Workflow Loader', () => {
@@ -4074,3 +4077,149 @@ nodes:
     });
   });
 });
+
+// ---------------------------------------------------------------------------
+// Workflow-level field parity (#2457)
+// ---------------------------------------------------------------------------
+
+/**
+ * `parseWorkflow` does not derive its result from `workflowDefinitionSchema` — it
+ * hand-assembles a WorkflowDefinition field by field into an object literal. A field
+ * added to the schema but not added to that literal is SILENTLY DISCARDED: the YAML
+ * parses, the workflow loads, and the feature is simply inert.
+ *
+ * That is not hypothetical. `requires:` was added to `workflowBaseSchema` in ab81248d
+ * (2026-06-01) without touching the loader, and was not added to that literal until
+ * 2d7bf587 (2026-07-16) — six weeks in which the GitHub capability gate could never
+ * fire for any discovered workflow, fixed incidentally inside an unrelated PR.
+ *
+ * This is the guard. The field list is DERIVED from `workflowDefinitionSchema.shape`,
+ * so a new schema field fails the test until it is given a fixture here — the same
+ * "the derived check fails until the new thing is registered" ratchet used by
+ * `check:capability-matrix` and the schema-parity test in `sqlite.test.ts`.
+ *
+ * Deliberately NOT solved by deriving the assembly itself (`schema.parse(raw)`): most
+ * fields warn-and-drop, logging a present-but-invalid value and continuing rather than
+ * aborting the whole discovery pass, and `.parse()` would reject the workflow instead.
+ * That is not universal — a few fields deliberately hard-reject and a few coerce
+ * silently — but one warn-and-drop field is enough to make a blanket `.parse()` wrong.
+ * `loader.ts` is the authority on which field does what; do not restate it here.
+ * See #2457.
+ */
+describe('workflow-level field parity (#2457)', () => {
+  /**
+   * One fixture per workflow-level schema key: a YAML fragment setting the field, and a
+   * predicate proving it survived `parseWorkflow`. `present` is deliberately a survival
+   * check rather than deep equality — several fields are normalised on the way through
+   * (tags deduped, betas trimmed, thinking preprocessed), and this guard is about the
+   * field reaching the result at all, not about how it is parsed.
+   */
+  const FIELD_FIXTURES: Record<
+    string,
+    { yaml: string; present: (w: WorkflowDefinition) => boolean }
+  > = {
+    name: { yaml: '', present: w => w.name === 'parity' },
+    description: { yaml: '', present: w => w.description === 'parity fixture' },
+    nodes: { yaml: '', present: w => w.nodes?.length === 1 },
+    provider: { yaml: 'provider: claude', present: w => w.provider === 'claude' },
+    model: { yaml: 'model: sonnet', present: w => w.model === 'sonnet' },
+    modelReasoningEffort: {
+      yaml: 'modelReasoningEffort: high',
+      present: w => w.modelReasoningEffort === 'high',
+    },
+    webSearchMode: { yaml: 'webSearchMode: live', present: w => w.webSearchMode === 'live' },
+    interactive: { yaml: 'interactive: true', present: w => w.interactive === true },
+    effort: { yaml: 'effort: high', present: w => w.effort === 'high' },
+    thinking: { yaml: 'thinking: adaptive', present: w => w.thinking?.type === 'adaptive' },
+    fallbackModel: {
+      yaml: 'fallbackModel: haiku',
+      present: w => w.fallbackModel === 'haiku',
+    },
+    betas: { yaml: 'betas:\n  - some-beta', present: w => w.betas?.includes('some-beta') === true },
+    sandbox: { yaml: 'sandbox:\n  enabled: true', present: w => w.sandbox?.enabled === true },
+    worktree: { yaml: 'worktree:\n  enabled: false', present: w => w.worktree?.enabled === false },
+    container: {
+      yaml: 'container:\n  enabled: true',
+      present: w => w.container?.enabled === true,
+    },
+    evidence_policy: {
+      yaml: 'evidence_policy:\n  required: true',
+      present: w => w.evidence_policy?.required === true,
+    },
+    mutates_checkout: {
+      yaml: 'mutates_checkout: false',
+      present: w => w.mutates_checkout === false,
+    },
+    persist_sessions: {
+      yaml: 'persist_sessions: true',
+      present: w => w.persist_sessions === true,
+    },
+    tags: { yaml: 'tags:\n  - alpha', present: w => w.tags?.includes('alpha') === true },
+    requires: {
+      yaml: 'requires:\n  - github',
+      present: w => w.requires?.includes('github') === true,
+    },
+  };
+
+  const schemaKeys = Object.keys(workflowDefinitionSchema.shape);
+
+  it('has a fixture for every workflow-level schema key (the ratchet)', () => {
+    const missing = schemaKeys.filter(k => !(k in FIELD_FIXTURES));
+    expect(
+      missing,
+      `Workflow-level schema keys with no parity fixture: ${missing.join(', ')}. ` +
+        'Add a fixture in FIELD_FIXTURES AND make sure parseWorkflow actually carries the ' +
+        'field into its returned object literal — a schema field missing from that literal ' +
+        'is silently discarded at parse (see #2457).'
+    ).toEqual([]);
+  });
+
+  it('has no fixture for a key that is not in the schema', () => {
+    const stale = Object.keys(FIELD_FIXTURES).filter(k => !schemaKeys.includes(k));
+    expect(stale, `Parity fixtures for keys no longer in the schema: ${stale.join(', ')}`).toEqual(
+      []
+    );
+  });
+
+  for (const key of Object.keys(FIELD_FIXTURES)) {
+    it(`round-trips '${key}' through parseWorkflow`, () => {
+      const fixture = FIELD_FIXTURES[key];
+      const yaml = [
+        'name: parity',
+        'description: parity fixture',
+        fixture.yaml,
+        'nodes:',
+        '  - id: only',
+        '    prompt: hello',
+      ]
+        .filter(line => line !== '')
+        .join('\n');
+
+      // An INVALID fixture value is dropped by design, which looks identical to the bug
+      // this test hunts. Clearing the logger first lets the failure message rank the two
+      // causes: a warning is strong evidence the fixture is at fault. Silence is NOT
+      // proof of the opposite — a few fields coerce an invalid value away with no log at
+      // all — so the silent branch names both causes rather than rendering a verdict.
+      mockLogger.warn.mockClear();
+
+      const result = parseWorkflow(yaml, `parity-${key}.yaml`);
+      expect(
+        result.error,
+        `parseWorkflow rejected the '${key}' fixture: ${result.error?.error}`
+      ).toBeNull();
+
+      const warned = mockLogger.warn.mock.calls.length > 0;
+      const message = warned
+        ? `Field '${key}' did not survive parseWorkflow, and a warning fired — the FIXTURE ` +
+          'value above is almost certainly invalid for this field, which warn-and-drop ' +
+          'discards by design. Fix the fixture, not the loader.'
+        : `Field '${key}' is declared on workflowDefinitionSchema and did NOT survive ` +
+          'parseWorkflow, with no warning logged. Two possible causes, likeliest first: ' +
+          "(1) the field is missing from the object literal parseWorkflow returns — that's " +
+          'the #2457 bug, add it there; or (2) the fixture value is invalid for a field ' +
+          'that coerces silently without logging, in which case fix the fixture. Check the ' +
+          'fixture value against the schema first — it is the cheaper of the two to rule out.';
+      expect(fixture.present(result.workflow as WorkflowDefinition), message).toBe(true);
+    });
+  }
+});