Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 149 additions & 0 deletions packages/workflows/src/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
}
});
Loading