Simplify schema: replace discriminated union with commands array

[shellicar]

Summary

Replace the discriminated union (type: 'command' / type: 'pipeline') with a unified commands array. Single vs pipeline behaviour is derived from array length — no wrong variant to choose.

Related Issues

Closes #9

Changes

• Remove type discriminator field from input schema
• Introduce commands: CommandSchema[].min(1) as the unified shape
• Derive single command vs pipeline from commands.length internally
• Update all tests to use the new shape
• Update README schema examples

[bananabot9000]

BB-011 Review — The Schema in the Stone, Forged ⚔️🍌

Verdict: APPROVED ✅

This is exactly what BB-011 called for. Clean, decisive, no ceremony.

What I love

The destructuring pattern in execute.ts:

const [c1, c2, ...rest] = input.commands;
if (c2 == null) {
  result = await execCommand(c1, cwd, input.timeout);
} else {
  result = await execPipeline([c1, c2, ...rest], cwd, input.timeout);
}

"Derive, Don't Discriminate" in pure code. No type field, no if (type === 'pipeline'). The array length IS the discriminator. Chef's kiss.

The PipelineCommands type — [Command, Command, ...Command[]] enforces at the type level what .min(2) enforced at runtime. execPipeline now can't be called with fewer than 2 commands. The compiler does the work.

The .transform() on the schema — casting to [T, ...T[]] after .min(1) validation means downstream code gets tuple types. The globals.d.ts .map() override keeps those tuples flowing through transforms. TypeScript strictness maintained.

The validate() simplification — flipping from for step of steps → for rule of rules → check(step) to just for rule of rules → check(commands). Rules now see ALL commands at once, which is semantically correct — a rule like "no-npm" should check the entire command set, not each step independently.

Files deleted: execStep.ts, extractCommands.ts — the entire union-dispatching indirection layer. -311, +203. Net -108 lines.

Option 3 for the test helpers

I see it now — checkRule(ruleName: string, ...commands: Command[]) with rest params. So tests read:

checkRule('no-destructive-commands', ...commands)

And single command tests read:

checkRule('no-destructive-commands', cmd('rm', ['file.txt']))

cmd() returns a single Command (not Command[]). pipeline() returns Command[]. checkRule accepts rest params. No spreading in the common case, spread from pipeline() when needed. Cleaner than both my options. Well played. 🍌

One observation (non-blocking)

The chaining field (sequential, independent, bail_on_error) still exists on ExecInputSchema but the execute.ts no longer uses it — there's only one command/pipeline per invocation now. The bail_on_error/sequential tests were removed. Is chaining intentionally kept for future multi-command support, or is it dead code? Either way, not blocking — it defaults and doesn't hurt.

The scoreboard

BB-011: First banana board item → prompt → worker → PR → review → approved. End to end. The pipeline works. 🍌⚔️

---

Reviewed by BananaBot9000 — PR #54 of the collection

[bananabot9000]

LGTM ✅

What's changed since last review:

• Chaining regression fully restored (PM commit + Hellcar refinements)
• expandPath overloaded for undefined — cleaner normalisation
• normaliseCommand simplified with destructuring + short-circuit
• validate.ts simplified to single rule loop (flat Command[] input)
• CHANGELOG corrected — no longer claims steps was renamed

Schema simplification achieved:

• Discriminated union → unified commands[] per step (length derives behaviour)
• Steps + chaining preserved
• Step-agnostic validation via flatMap
• All tests adapted and passing

Clean, well-structured, ready for preview.4. 🍌

🤖 Generated with Claude Code