From b02a0a6f2c4d578016f6686a64501245eb432f0b Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 22 Mar 2026 14:18:32 +0200 Subject: [PATCH 1/6] refactor: extract parseScheduleCreateArgs pure function (#103) Extract parsing and validation from the 188-line scheduleCreate handler into a pure function returning Result, matching the parseLoopCreateArgs pattern in loop.ts. - Add ParsedScheduleCreateArgs interface (private) - Export parseScheduleCreateArgs (no side effects, no process.exit) - Simplify scheduleCreate handler to ~45 lines - Replace hand-rolled test helpers with real parser calls - Add 22 parser unit tests (happy paths + error cases) - Fix workingDirectory test to use process.cwd() (was bypassing validation) Closes #103 --- src/cli/commands/schedule.ts | 277 ++++++++++++++----------- tests/unit/cli.test.ts | 385 +++++++++++++++++++++++++++-------- 2 files changed, 454 insertions(+), 208 deletions(-) diff --git a/src/cli/commands/schedule.ts b/src/cli/commands/schedule.ts index 081d9fa..5d3031f 100644 --- a/src/cli/commands/schedule.ts +++ b/src/cli/commands/schedule.ts @@ -1,64 +1,37 @@ import { AGENT_PROVIDERS, type AgentProvider, isAgentProvider } from '../../core/agents.js'; import { Priority, ScheduleId, ScheduleStatus, ScheduleType } from '../../core/domain.js'; import type { ScheduleExecution, ScheduleRepository, ScheduleService } from '../../core/interfaces.js'; +import { err, ok, type Result } from '../../core/result.js'; import { toMissedRunPolicy, truncatePrompt } from '../../utils/format.js'; import { validatePath } from '../../utils/validation.js'; import { exitOnError, exitOnNull, withReadOnlyContext, withServices } from '../services.js'; import * as ui from '../ui.js'; -export async function handleScheduleCommand(subCmd: string | undefined, scheduleArgs: string[]): Promise { - if (!subCmd) { - ui.error('Usage: beat schedule '); - process.exit(1); - } - - // Read-only subcommands: lightweight context, no full bootstrap - if (subCmd === 'list' || subCmd === 'get') { - const s = ui.createSpinner(); - s.start(subCmd === 'list' ? 'Fetching schedules...' : 'Fetching schedule...'); - const ctx = withReadOnlyContext(s); - s.stop('Ready'); - - try { - if (subCmd === 'list') { - await scheduleList(ctx.scheduleRepository, scheduleArgs); - } else { - await scheduleGet(ctx.scheduleRepository, scheduleArgs); - } - } finally { - ctx.close(); - } - process.exit(0); - } - - // Mutation subcommands: full bootstrap - const s = ui.createSpinner(); - s.start('Initializing...'); - const { scheduleService } = await withServices(s); - s.stop('Ready'); - - switch (subCmd) { - case 'create': - await scheduleCreate(scheduleService, scheduleArgs); - break; - case 'cancel': - await scheduleCancel(scheduleService, scheduleArgs); - break; - case 'pause': - await schedulePause(scheduleService, scheduleArgs); - break; - case 'resume': - await scheduleResume(scheduleService, scheduleArgs); - break; - default: - ui.error(`Unknown schedule subcommand: ${subCmd}`); - process.stderr.write('Valid subcommands: create, list, get, cancel, pause, resume\n'); - process.exit(1); - } - process.exit(0); +/** + * Parsed arguments from CLI schedule create command + */ +interface ParsedScheduleCreateArgs { + readonly prompt?: string; + readonly scheduleType: 'cron' | 'one_time'; + readonly cronExpression?: string; + readonly scheduledAt?: string; + readonly timezone?: string; + readonly missedRunPolicy?: 'skip' | 'catchup' | 'fail'; + readonly priority?: 'P0' | 'P1' | 'P2'; + readonly workingDirectory?: string; + readonly maxRuns?: number; + readonly expiresAt?: string; + readonly afterScheduleId?: string; + readonly agent?: AgentProvider; + readonly isPipeline: boolean; + readonly pipelineSteps?: readonly string[]; } -async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): Promise { +/** + * Parse and validate schedule create arguments + * ARCHITECTURE: Pure function — no side effects, returns Result for testability + */ +export function parseScheduleCreateArgs(scheduleArgs: string[]): Result { const promptWords: string[] = []; let scheduleType: 'cron' | 'one_time' | undefined; let cronExpression: string | undefined; @@ -80,8 +53,7 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): if (arg === '--type' && next) { if (next !== 'cron' && next !== 'one_time') { - ui.error('--type must be "cron" or "one_time"'); - process.exit(1); + return err('--type must be "cron" or "one_time"'); } scheduleType = next; i++; @@ -96,31 +68,27 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): i++; } else if (arg === '--missed-run-policy' && next) { if (!['skip', 'catchup', 'fail'].includes(next)) { - ui.error('--missed-run-policy must be "skip", "catchup", or "fail"'); - process.exit(1); + return err('--missed-run-policy must be "skip", "catchup", or "fail"'); } missedRunPolicy = next as 'skip' | 'catchup' | 'fail'; i++; } else if ((arg === '--priority' || arg === '-p') && next) { if (!['P0', 'P1', 'P2'].includes(next)) { - ui.error('Priority must be P0, P1, or P2'); - process.exit(1); + return err('Priority must be P0, P1, or P2'); } priority = next as 'P0' | 'P1' | 'P2'; i++; } else if ((arg === '--working-directory' || arg === '-w') && next) { const pathResult = validatePath(next); if (!pathResult.ok) { - ui.error(`Invalid working directory: ${pathResult.error.message}`); - process.exit(1); + return err(`Invalid working directory: ${pathResult.error.message}`); } workingDirectory = pathResult.value; i++; } else if (arg === '--max-runs' && next) { maxRuns = parseInt(next); if (isNaN(maxRuns) || maxRuns < 1) { - ui.error('--max-runs must be a positive integer'); - process.exit(1); + return err('--max-runs must be a positive integer'); } i++; } else if (arg === '--expires-at' && next) { @@ -131,12 +99,10 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): i++; } else if (arg === '--agent' || arg === '-a') { if (!next || next.startsWith('-')) { - ui.error(`--agent requires an agent name (${AGENT_PROVIDERS.join(', ')})`); - process.exit(1); + return err(`--agent requires an agent name (${AGENT_PROVIDERS.join(', ')})`); } if (!isAgentProvider(next)) { - ui.error(`Unknown agent: "${next}". Available agents: ${AGENT_PROVIDERS.join(', ')}`); - process.exit(1); + return err(`Unknown agent: "${next}". Available agents: ${AGENT_PROVIDERS.join(', ')}`); } agent = next; i++; @@ -146,8 +112,7 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): pipelineSteps.push(next); i++; } else if (arg.startsWith('-')) { - ui.error(`Unknown flag: ${arg}`); - process.exit(1); + return err(`Unknown flag: ${arg}`); } else { promptWords.push(arg); } @@ -155,43 +120,128 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): // Infer type from --cron / --at flags if (cronExpression && scheduledAt) { - ui.error('Cannot specify both --cron and --at'); - process.exit(1); + return err('Cannot specify both --cron and --at'); } const inferredType = cronExpression ? 'cron' : scheduledAt ? 'one_time' : undefined; if (scheduleType && inferredType && scheduleType !== inferredType) { - ui.error(`--type ${scheduleType} conflicts with ${cronExpression ? '--cron' : '--at'}`); - process.exit(1); + return err(`--type ${scheduleType} conflicts with ${cronExpression ? '--cron' : '--at'}`); } scheduleType = scheduleType ?? inferredType; if (!scheduleType) { - ui.error('Provide --cron, --at, or --type'); - process.exit(1); + return err('Provide --cron, --at, or --type'); } - // Pipeline mode: --pipeline with --step flags + // Pipeline mode if (isPipeline) { - if (promptWords.length > 0) { - ui.info(`Ignoring positional prompt text in --pipeline mode: "${promptWords.join(' ')}". Use --step flags only.`); - } if (pipelineSteps.length < 2) { - ui.error('Pipeline requires at least 2 --step flags'); - process.exit(1); + return err('Pipeline requires at least 2 --step flags'); } + } else if (pipelineSteps.length > 0) { + return err('--step requires --pipeline. Did you mean: beat schedule create --pipeline --step "..." --step "..."'); + } + + // Non-pipeline mode: prompt is required + const prompt = promptWords.join(' '); + if (!isPipeline && !prompt) { + return err('Usage: beat schedule create --cron "..." | --at "..." [options]'); + } + + return ok({ + prompt: prompt || undefined, + scheduleType, + cronExpression, + scheduledAt, + timezone, + missedRunPolicy, + priority, + workingDirectory, + maxRuns, + expiresAt, + afterScheduleId, + agent, + isPipeline, + pipelineSteps: isPipeline ? pipelineSteps : undefined, + }); +} +export async function handleScheduleCommand(subCmd: string | undefined, scheduleArgs: string[]): Promise { + if (!subCmd) { + ui.error('Usage: beat schedule '); + process.exit(1); + } + + // Read-only subcommands: lightweight context, no full bootstrap + if (subCmd === 'list' || subCmd === 'get') { + const s = ui.createSpinner(); + s.start(subCmd === 'list' ? 'Fetching schedules...' : 'Fetching schedule...'); + const ctx = withReadOnlyContext(s); + s.stop('Ready'); + + try { + if (subCmd === 'list') { + await scheduleList(ctx.scheduleRepository, scheduleArgs); + } else { + await scheduleGet(ctx.scheduleRepository, scheduleArgs); + } + } finally { + ctx.close(); + } + process.exit(0); + } + + // Mutation subcommands: full bootstrap + const s = ui.createSpinner(); + s.start('Initializing...'); + const { scheduleService } = await withServices(s); + s.stop('Ready'); + + switch (subCmd) { + case 'create': + await scheduleCreate(scheduleService, scheduleArgs); + break; + case 'cancel': + await scheduleCancel(scheduleService, scheduleArgs); + break; + case 'pause': + await schedulePause(scheduleService, scheduleArgs); + break; + case 'resume': + await scheduleResume(scheduleService, scheduleArgs); + break; + default: + ui.error(`Unknown schedule subcommand: ${subCmd}`); + process.stderr.write('Valid subcommands: create, list, get, cancel, pause, resume\n'); + process.exit(1); + } + process.exit(0); +} + +async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): Promise { + const parsed = parseScheduleCreateArgs(scheduleArgs); + if (!parsed.ok) { + ui.error(parsed.error); + process.exit(1); + } + const args = parsed.value; + + if (args.isPipeline && args.prompt) { + ui.info(`Ignoring positional prompt text in --pipeline mode: "${args.prompt}". Use --step flags only.`); + } + + if (args.isPipeline) { const result = await service.createScheduledPipeline({ - steps: pipelineSteps.map((prompt) => ({ prompt })), - scheduleType: scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, - cronExpression, - scheduledAt, - timezone, - missedRunPolicy: missedRunPolicy ? toMissedRunPolicy(missedRunPolicy) : undefined, - priority: priority ? Priority[priority] : undefined, - workingDirectory, - maxRuns, - expiresAt, - afterScheduleId: afterScheduleId ? ScheduleId(afterScheduleId) : undefined, - agent, + steps: args.pipelineSteps!.map((prompt) => ({ prompt })), + scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, + cronExpression: args.cronExpression, + scheduledAt: args.scheduledAt, + timezone: args.timezone, + missedRunPolicy: args.missedRunPolicy ? toMissedRunPolicy(args.missedRunPolicy) : undefined, + priority: args.priority ? Priority[args.priority] : undefined, + workingDirectory: args.workingDirectory, + maxRuns: args.maxRuns, + expiresAt: args.expiresAt, + afterScheduleId: args.afterScheduleId ? ScheduleId(args.afterScheduleId) : undefined, + agent: args.agent, }); const pipeline = exitOnError(result, undefined, 'Failed to create scheduled pipeline'); @@ -203,38 +253,24 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): ]; if (pipeline.nextRunAt) details.push(`Next run: ${new Date(pipeline.nextRunAt).toISOString()}`); if (pipeline.cronExpression) details.push(`Cron: ${pipeline.cronExpression}`); - if (agent) details.push(`Agent: ${agent}`); + if (args.agent) details.push(`Agent: ${args.agent}`); ui.info(details.join(' | ')); - process.exit(0); - } - - // Guard: --step without --pipeline is a user error - if (pipelineSteps.length > 0) { - ui.error('--step requires --pipeline. Did you mean: beat schedule create --pipeline --step "..." --step "..."'); - process.exit(1); - } - - // Single-task mode - const prompt = promptWords.join(' '); - if (!prompt) { - ui.error('Usage: beat schedule create --cron "..." | --at "..." [options]'); - ui.info(' Pipeline: beat schedule create --pipeline --step "lint" --step "test" --cron "0 9 * * *"'); - process.exit(1); + return; } const result = await service.createSchedule({ - prompt, - scheduleType: scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, - cronExpression, - scheduledAt, - timezone, - missedRunPolicy: missedRunPolicy ? toMissedRunPolicy(missedRunPolicy) : undefined, - priority: priority ? Priority[priority] : undefined, - workingDirectory, - maxRuns, - expiresAt, - afterScheduleId: afterScheduleId ? ScheduleId(afterScheduleId) : undefined, - agent, + prompt: args.prompt!, + scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, + cronExpression: args.cronExpression, + scheduledAt: args.scheduledAt, + timezone: args.timezone, + missedRunPolicy: args.missedRunPolicy ? toMissedRunPolicy(args.missedRunPolicy) : undefined, + priority: args.priority ? Priority[args.priority] : undefined, + workingDirectory: args.workingDirectory, + maxRuns: args.maxRuns, + expiresAt: args.expiresAt, + afterScheduleId: args.afterScheduleId ? ScheduleId(args.afterScheduleId) : undefined, + agent: args.agent, }); const created = exitOnError(result, undefined, 'Failed to create schedule'); @@ -243,9 +279,8 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): if (created.nextRunAt) details.push(`Next run: ${new Date(created.nextRunAt).toISOString()}`); if (created.cronExpression) details.push(`Cron: ${created.cronExpression}`); if (created.afterScheduleId) details.push(`After: ${created.afterScheduleId}`); - if (agent) details.push(`Agent: ${agent}`); + if (args.agent) details.push(`Agent: ${args.agent}`); ui.info(details.join(' | ')); - process.exit(0); } async function scheduleList(repo: ScheduleRepository, scheduleArgs: string[]): Promise { diff --git a/tests/unit/cli.test.ts b/tests/unit/cli.test.ts index 0e4df20..cdbcdd3 100644 --- a/tests/unit/cli.test.ts +++ b/tests/unit/cli.test.ts @@ -61,6 +61,7 @@ import type { TaskRepository, } from '../../src/core/interfaces'; import { err, ok, type Result } from '../../src/core/result'; +import { toMissedRunPolicy } from '../../src/utils/format'; import { TaskFactory } from '../fixtures/factories'; // Test constants @@ -1048,6 +1049,12 @@ describe('CLI - Command Parsing and Validation', () => { describe('CLI - Schedule Commands', () => { let mockScheduleService: MockScheduleService; let mockScheduleReadOnlyCtx: MockReadOnlyContext; + let parseScheduleCreateArgs: typeof import('../../src/cli/commands/schedule').parseScheduleCreateArgs; + + beforeAll(async () => { + const mod = await import('../../src/cli/commands/schedule'); + parseScheduleCreateArgs = mod.parseScheduleCreateArgs; + }); beforeEach(() => { mockScheduleService = new MockScheduleService(); @@ -1059,6 +1066,209 @@ describe('CLI - Schedule Commands', () => { mockScheduleReadOnlyCtx.reset(); }); + describe('parseScheduleCreateArgs - pure function', () => { + it('should parse cron schedule', () => { + const result = parseScheduleCreateArgs(['run', 'tests', '--cron', '0 9 * * *']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.scheduleType).toBe('cron'); + expect(result.value.cronExpression).toBe('0 9 * * *'); + expect(result.value.prompt).toBe('run tests'); + }); + + it('should parse one-time schedule with --at', () => { + const result = parseScheduleCreateArgs(['deploy', '--at', '2026-04-01T09:00:00Z']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.scheduleType).toBe('one_time'); + expect(result.value.scheduledAt).toBe('2026-04-01T09:00:00Z'); + expect(result.value.prompt).toBe('deploy'); + }); + + it('should infer type from --cron without explicit --type', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '*/5 * * * *']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.scheduleType).toBe('cron'); + }); + + it('should infer type from --at without explicit --type', () => { + const result = parseScheduleCreateArgs(['task', '--at', '2026-04-01T09:00:00Z']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.scheduleType).toBe('one_time'); + }); + + it('should parse all optional flags', () => { + const cwd = process.cwd(); + const result = parseScheduleCreateArgs([ + 'run', + 'tests', + '--cron', + '0 9 * * 1-5', + '--timezone', + 'America/New_York', + '--missed-run-policy', + 'catchup', + '--priority', + 'P0', + '--working-directory', + cwd, + '--max-runs', + '10', + '--expires-at', + '2026-12-31T23:59:59Z', + '--after', + 'sched-abc', + '--agent', + 'claude', + ]); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.timezone).toBe('America/New_York'); + expect(result.value.missedRunPolicy).toBe('catchup'); + expect(result.value.priority).toBe('P0'); + expect(result.value.workingDirectory).toBe(cwd); + expect(result.value.maxRuns).toBe(10); + expect(result.value.expiresAt).toBe('2026-12-31T23:59:59Z'); + expect(result.value.afterScheduleId).toBe('sched-abc'); + expect(result.value.agent).toBe('claude'); + }); + + it('should parse pipeline with --pipeline and --step flags', () => { + const result = parseScheduleCreateArgs(['--pipeline', '--step', 'lint', '--step', 'test', '--cron', '0 9 * * *']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.isPipeline).toBe(true); + expect(result.value.pipelineSteps).toEqual(['lint', 'test']); + expect(result.value.prompt).toBeUndefined(); + }); + + it('should preserve prompt in pipeline mode for handler warning', () => { + const result = parseScheduleCreateArgs([ + 'extra', + 'words', + '--pipeline', + '--step', + 'lint', + '--step', + 'test', + '--cron', + '0 9 * * *', + ]); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.isPipeline).toBe(true); + expect(result.value.prompt).toBe('extra words'); + }); + + it('should parse --priority with -p shorthand', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '-p', 'P1']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.priority).toBe('P1'); + }); + + it('should parse --agent with -a shorthand', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '-a', 'claude']); + expect(result.ok).toBe(true); + if (!result.ok) return; + expect(result.value.agent).toBe('claude'); + }); + + // Error cases + it('should reject both --cron and --at', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--at', '2026-04-01T09:00:00Z']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('Cannot specify both'); + }); + + it('should reject --type conflict with --cron', () => { + const result = parseScheduleCreateArgs(['task', '--type', 'one_time', '--cron', '0 9 * * *']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('conflicts'); + }); + + it('should reject missing schedule type', () => { + const result = parseScheduleCreateArgs(['task']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('--cron'); + }); + + it('should reject invalid --type value', () => { + const result = parseScheduleCreateArgs(['task', '--type', 'weekly']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('--type'); + }); + + it('should reject invalid --missed-run-policy', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--missed-run-policy', 'ignore']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('--missed-run-policy'); + }); + + it('should reject invalid priority', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--priority', 'P9']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('Priority'); + }); + + it('should reject pipeline with fewer than 2 steps', () => { + const result = parseScheduleCreateArgs(['--pipeline', '--step', 'only-one', '--cron', '0 9 * * *']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('at least 2'); + }); + + it('should reject --step without --pipeline', () => { + const result = parseScheduleCreateArgs(['task', '--step', 'lint', '--step', 'test', '--cron', '0 9 * * *']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('--pipeline'); + }); + + it('should reject unknown flag', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--bogus']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('Unknown flag'); + }); + + it('should reject missing prompt in non-pipeline mode', () => { + const result = parseScheduleCreateArgs(['--cron', '0 9 * * *']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('Usage'); + }); + + it('should reject non-positive --max-runs', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--max-runs', '0']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('--max-runs'); + }); + + it('should reject unknown agent', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--agent', 'skynet']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('Unknown agent'); + }); + + it('should reject --agent without value', () => { + const result = parseScheduleCreateArgs(['task', '--cron', '0 9 * * *', '--agent', '--priority']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('--agent'); + }); + }); + describe('schedule create', () => { it('should create a cron schedule with required fields', async () => { const result = await simulateScheduleCreate(mockScheduleService, { @@ -1095,7 +1305,7 @@ describe('CLI - Schedule Commands', () => { timezone: 'America/New_York', missedRunPolicy: 'catchup', priority: 'P0', - workingDirectory: '/workspace', + workingDirectory: process.cwd(), maxRuns: 10, }); @@ -1119,18 +1329,18 @@ describe('CLI - Schedule Commands', () => { }); it('should reject missing prompt', () => { - const validation = validateScheduleCreateInput('', { type: 'cron', cron: '0 9 * * *' }); - expect(validation.ok).toBe(false); + const result = parseScheduleCreateArgs(['--cron', '0 9 * * *']); + expect(result.ok).toBe(false); }); it('should reject missing schedule type when no --cron or --at given', () => { - const validation = validateScheduleCreateInput('run tests', {}); - expect(validation.ok).toBe(false); + const result = parseScheduleCreateArgs(['run', 'tests']); + expect(result.ok).toBe(false); }); it('should reject invalid schedule type', () => { - const validation = validateScheduleCreateInput('run tests', { type: 'weekly' }); - expect(validation.ok).toBe(false); + const result = parseScheduleCreateArgs(['run', 'tests', '--type', 'weekly']); + expect(result.ok).toBe(false); }); it('should infer type from --cron flag', async () => { @@ -1173,13 +1383,10 @@ describe('CLI - Schedule Commands', () => { }); it('should reject pipeline with fewer than 2 steps', () => { - const validation = validatePipelineCreateInput(['only-one-step']); - - expect(validation.ok).toBe(false); - if (!validation.ok) { - expect(validation.error.code).toBe(ErrorCode.INVALID_INPUT); - expect(validation.error.message).toContain('at least 2'); - } + const result = parseScheduleCreateArgs(['--pipeline', '--step', 'only-one', '--cron', '0 9 * * *']); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error).toContain('at least 2'); }); }); @@ -2264,23 +2471,36 @@ async function simulateRetryCommand(taskManager: MockTaskManager, taskId: string // Schedule, Pipeline, Resume Helpers // ============================================================================ -function validateScheduleCreateInput(prompt: string, options: { type?: string }) { - if (!prompt || prompt.trim().length === 0) { - return err( - new BackbeatError(ErrorCode.INVALID_INPUT, 'Prompt is required for schedule creation', { field: 'prompt' }), - ); - } - - if (!options.type || !['cron', 'one_time'].includes(options.type)) { - return err( - new BackbeatError(ErrorCode.INVALID_INPUT, '--type must be "cron" or "one_time"', { - field: 'type', - value: options.type, - }), - ); - } - - return ok(undefined); +/** + * Build CLI arg array from structured options for schedule create. + */ +function buildScheduleCreateArgs(options: { + prompt: string; + type: string; + cron?: string; + at?: string; + timezone?: string; + missedRunPolicy?: string; + priority?: string; + workingDirectory?: string; + maxRuns?: number; + expiresAt?: string; + afterScheduleId?: string; + agent?: string; +}): string[] { + const args = options.prompt.split(' '); + if (options.cron) args.push('--cron', options.cron); + if (options.at) args.push('--at', options.at); + if (!options.cron && !options.at) args.push('--type', options.type); + if (options.timezone) args.push('--timezone', options.timezone); + if (options.missedRunPolicy) args.push('--missed-run-policy', options.missedRunPolicy); + if (options.priority) args.push('--priority', options.priority); + if (options.workingDirectory) args.push('--working-directory', options.workingDirectory); + if (options.maxRuns) args.push('--max-runs', String(options.maxRuns)); + if (options.expiresAt) args.push('--expires-at', options.expiresAt); + if (options.afterScheduleId) args.push('--after', options.afterScheduleId); + if (options.agent) args.push('--agent', options.agent); + return args; } async function simulateScheduleCreate( @@ -2299,28 +2519,23 @@ async function simulateScheduleCreate( afterScheduleId?: string; }, ) { - const validation = validateScheduleCreateInput(options.prompt, options); - if (!validation.ok) return validation; + const { parseScheduleCreateArgs } = await import('../../src/cli/commands/schedule'); + const parsed = parseScheduleCreateArgs(buildScheduleCreateArgs(options)); + if (!parsed.ok) return err(new BackbeatError(ErrorCode.INVALID_INPUT, parsed.error)); + const args = parsed.value; return service.createSchedule({ - prompt: options.prompt, - scheduleType: options.type === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, - cronExpression: options.cron, - scheduledAt: options.at, - timezone: options.timezone, - missedRunPolicy: - options.missedRunPolicy === 'catchup' - ? MissedRunPolicy.CATCHUP - : options.missedRunPolicy === 'fail' - ? MissedRunPolicy.FAIL - : options.missedRunPolicy - ? MissedRunPolicy.SKIP - : undefined, - priority: options.priority, - workingDirectory: options.workingDirectory, - maxRuns: options.maxRuns, - expiresAt: options.expiresAt, - afterScheduleId: options.afterScheduleId ? ScheduleId(options.afterScheduleId) : undefined, + prompt: args.prompt!, + scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, + cronExpression: args.cronExpression, + scheduledAt: args.scheduledAt, + timezone: args.timezone, + missedRunPolicy: args.missedRunPolicy ? toMissedRunPolicy(args.missedRunPolicy) : undefined, + priority: args.priority, + workingDirectory: args.workingDirectory, + maxRuns: args.maxRuns, + expiresAt: args.expiresAt, + afterScheduleId: args.afterScheduleId ? ScheduleId(args.afterScheduleId) : undefined, }); } @@ -2331,22 +2546,9 @@ function validatePipelineInput(steps: string[]) { return ok(undefined); } -/** - * Validates pipeline create input (--pipeline --step flags). - * Mirrors the validation in scheduleCreate() when isPipeline is true. - */ -function validatePipelineCreateInput(steps: string[]) { - if (steps.length < 2) { - return err( - new BackbeatError(ErrorCode.INVALID_INPUT, 'Pipeline requires at least 2 --step flags', { field: 'steps' }), - ); - } - return ok(undefined); -} - /** * Simulates `beat schedule create --pipeline --step "..." --step "..." --cron "..."` - * Mirrors the pipeline branch in scheduleCreate(). + * Uses real parseScheduleCreateArgs for validation. */ async function simulateScheduleCreatePipeline( service: MockScheduleService, @@ -2364,30 +2566,39 @@ async function simulateScheduleCreatePipeline( agent?: string; }, ) { - const validation = validatePipelineCreateInput(options.steps); - if (!validation.ok) return validation; - - const scheduleType = options.cron ? ScheduleType.CRON : ScheduleType.ONE_TIME; + const { parseScheduleCreateArgs } = await import('../../src/cli/commands/schedule'); + const args: string[] = ['--pipeline']; + for (const step of options.steps) { + args.push('--step', step); + } + if (options.cron) args.push('--cron', options.cron); + if (options.at) args.push('--at', options.at); + if (options.timezone) args.push('--timezone', options.timezone); + if (options.missedRunPolicy) args.push('--missed-run-policy', options.missedRunPolicy); + if (options.priority) args.push('--priority', options.priority); + if (options.workingDirectory) args.push('--working-directory', options.workingDirectory); + if (options.maxRuns) args.push('--max-runs', String(options.maxRuns)); + if (options.expiresAt) args.push('--expires-at', options.expiresAt); + if (options.afterScheduleId) args.push('--after', options.afterScheduleId); + if (options.agent) args.push('--agent', options.agent); + + const parsed = parseScheduleCreateArgs(args); + if (!parsed.ok) return err(new BackbeatError(ErrorCode.INVALID_INPUT, parsed.error)); + const p = parsed.value; return service.createScheduledPipeline({ - steps: options.steps.map((prompt) => ({ prompt })), - scheduleType, - cronExpression: options.cron, - scheduledAt: options.at, - timezone: options.timezone, - missedRunPolicy: - options.missedRunPolicy === 'catchup' - ? MissedRunPolicy.CATCHUP - : options.missedRunPolicy === 'fail' - ? MissedRunPolicy.FAIL - : options.missedRunPolicy - ? MissedRunPolicy.SKIP - : undefined, - priority: options.priority, - workingDirectory: options.workingDirectory, - maxRuns: options.maxRuns, - expiresAt: options.expiresAt, - afterScheduleId: options.afterScheduleId ? ScheduleId(options.afterScheduleId) : undefined, + steps: p.pipelineSteps!.map((prompt) => ({ prompt })), + scheduleType: p.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, + cronExpression: p.cronExpression, + scheduledAt: p.scheduledAt, + timezone: p.timezone, + missedRunPolicy: p.missedRunPolicy ? toMissedRunPolicy(p.missedRunPolicy) : undefined, + priority: p.priority, + workingDirectory: p.workingDirectory, + maxRuns: p.maxRuns, + expiresAt: p.expiresAt, + afterScheduleId: p.afterScheduleId ? ScheduleId(p.afterScheduleId) : undefined, + agent: p.agent, }); } From fe5fe0877f164f384d946c8f53957162440697be Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 22 Mar 2026 14:38:03 +0200 Subject: [PATCH 2/6] fix: address self-review issues - Fix biome formatting (extra blank line in test file) - Simplifier changes: deduplicate baseOptions, replace nested ternary with if/else, remove redundant JSDoc, remove 4 duplicate tests --- src/cli/commands/schedule.ts | 48 +++++++++++++++++------------------- tests/unit/cli.test.ts | 22 ----------------- 2 files changed, 23 insertions(+), 47 deletions(-) diff --git a/src/cli/commands/schedule.ts b/src/cli/commands/schedule.ts index 5d3031f..7ec470b 100644 --- a/src/cli/commands/schedule.ts +++ b/src/cli/commands/schedule.ts @@ -28,8 +28,7 @@ interface ParsedScheduleCreateArgs { } /** - * Parse and validate schedule create arguments - * ARCHITECTURE: Pure function — no side effects, returns Result for testability + * Parse and validate schedule create arguments. */ export function parseScheduleCreateArgs(scheduleArgs: string[]): Result { const promptWords: string[] = []; @@ -122,7 +121,12 @@ export function parseScheduleCreateArgs(scheduleArgs: string[]): Result ({ prompt })), - scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, - cronExpression: args.cronExpression, - scheduledAt: args.scheduledAt, - timezone: args.timezone, - missedRunPolicy: args.missedRunPolicy ? toMissedRunPolicy(args.missedRunPolicy) : undefined, - priority: args.priority ? Priority[args.priority] : undefined, - workingDirectory: args.workingDirectory, - maxRuns: args.maxRuns, - expiresAt: args.expiresAt, - afterScheduleId: args.afterScheduleId ? ScheduleId(args.afterScheduleId) : undefined, - agent: args.agent, }); const pipeline = exitOnError(result, undefined, 'Failed to create scheduled pipeline'); @@ -259,18 +267,8 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): } const result = await service.createSchedule({ + ...baseOptions, prompt: args.prompt!, - scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, - cronExpression: args.cronExpression, - scheduledAt: args.scheduledAt, - timezone: args.timezone, - missedRunPolicy: args.missedRunPolicy ? toMissedRunPolicy(args.missedRunPolicy) : undefined, - priority: args.priority ? Priority[args.priority] : undefined, - workingDirectory: args.workingDirectory, - maxRuns: args.maxRuns, - expiresAt: args.expiresAt, - afterScheduleId: args.afterScheduleId ? ScheduleId(args.afterScheduleId) : undefined, - agent: args.agent, }); const created = exitOnError(result, undefined, 'Failed to create schedule'); diff --git a/tests/unit/cli.test.ts b/tests/unit/cli.test.ts index cdbcdd3..f753ec5 100644 --- a/tests/unit/cli.test.ts +++ b/tests/unit/cli.test.ts @@ -1328,21 +1328,6 @@ describe('CLI - Schedule Commands', () => { expect(mockScheduleService.createCalls[0].afterScheduleId).toBe(ScheduleId('schedule-abc123')); }); - it('should reject missing prompt', () => { - const result = parseScheduleCreateArgs(['--cron', '0 9 * * *']); - expect(result.ok).toBe(false); - }); - - it('should reject missing schedule type when no --cron or --at given', () => { - const result = parseScheduleCreateArgs(['run', 'tests']); - expect(result.ok).toBe(false); - }); - - it('should reject invalid schedule type', () => { - const result = parseScheduleCreateArgs(['run', 'tests', '--type', 'weekly']); - expect(result.ok).toBe(false); - }); - it('should infer type from --cron flag', async () => { const result = await simulateScheduleCreate(mockScheduleService, { prompt: 'run tests', @@ -1381,13 +1366,6 @@ describe('CLI - Schedule Commands', () => { expect(call.scheduleType).toBe(ScheduleType.CRON); expect(call.cronExpression).toBe('0 9 * * *'); }); - - it('should reject pipeline with fewer than 2 steps', () => { - const result = parseScheduleCreateArgs(['--pipeline', '--step', 'only-one', '--cron', '0 9 * * *']); - expect(result.ok).toBe(false); - if (result.ok) return; - expect(result.error).toContain('at least 2'); - }); }); describe('schedule list (ReadOnlyContext)', () => { From 6174581b691f816aca42f12f8a12a44878379aa4 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 22 Mar 2026 21:02:24 +0200 Subject: [PATCH 3/6] chore: add .claudeignore (DevFlow configuration) Include .claudeignore to protect against sensitive files and context pollution when using Claude-based tools. Co-Authored-By: Claude --- .claudeignore | 188 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 .claudeignore diff --git a/.claudeignore b/.claudeignore new file mode 100644 index 0000000..8b8d6eb --- /dev/null +++ b/.claudeignore @@ -0,0 +1,188 @@ +# DevFlow .claudeignore - Protects against sensitive files and context pollution +# Generated by DevFlow - Edit as needed for your project + +# === SECURITY: Sensitive Files === +# Environment and secrets +.env +.env.* +.env.local +.env.*.local +*.env +.envrc + +# Credentials and keys +*.key +*.pem +*.p12 +*.pfx +*.cer +*.crt +*.der +id_rsa +id_dsa +id_ecdsa +id_ed25519 +*.ppk +*_rsa +*_dsa +*secret* +*password* +*credential* +credentials.json +secrets.json +secrets.yaml +secrets.yml + +# Cloud provider credentials +.aws/credentials +.aws/config +.gcp/credentials.json +.azure/credentials + +# Package manager credentials +.npmrc +.pypirc +.gem/credentials +pip.conf + +# Database +*.sql +*.db +*.sqlite +*.sqlite3 + +# === DEPENDENCIES & BUILD === +# Node.js +node_modules/ +npm-debug.log* +yarn-debug.log* +yarn-error.log* +pnpm-debug.log* +.pnpm-store/ + +# Python +__pycache__/ +*.py[cod] +*$py.class +.Python +env/ +venv/ +ENV/ +.venv/ +pip-log.txt +pip-delete-this-directory.txt +.eggs/ +*.egg-info/ +dist/ +build/ +*.whl + +# Ruby +vendor/bundle/ +.bundle/ + +# Go +vendor/ +go.sum + +# Rust +target/ +Cargo.lock + +# Java +target/ +*.class +*.jar +*.war + +# PHP +vendor/ +composer.lock + +# === BUILD ARTIFACTS === +dist/ +build/ +out/ +.next/ +.nuxt/ +.output/ +.vite/ +.cache/ +.parcel-cache/ +.turbo/ +*.tsbuildinfo + +# === LOGS & TEMP FILES === +logs/ +*.log +*.tmp +*.temp +*.swp +*.swo +*~ +.DS_Store +Thumbs.db +*.bak +*.orig +*.rej +.cache + +# === VERSION CONTROL === +.git/ +.svn/ +.hg/ +.gitignore + +# === IDE & EDITORS === +.vscode/ +.idea/ +*.sublime-* +*.code-workspace +.project +.classpath +.settings/ + +# === TEST COVERAGE === +coverage/ +.nyc_output/ +htmlcov/ +.coverage +.pytest_cache/ +.tox/ + +# === OS-SPECIFIC === +.DS_Store +.AppleDouble +.LSOverride +Thumbs.db +ehthumbs.db +Desktop.ini + +# === MEDIA & LARGE FILES === +*.mp4 +*.avi +*.mov +*.wmv +*.flv +*.mp3 +*.wav +*.zip +*.tar.gz +*.rar +*.7z +*.dmg +*.iso + +# === DOCUMENTATION BUILD === +site/ +_site/ +.docusaurus/ +.vuepress/dist/ + +# === LOCK FILES (usually not needed for AI context) === +package-lock.json +yarn.lock +pnpm-lock.yaml +Gemfile.lock +poetry.lock +Pipfile.lock From 5a2842bc2533d4a7c61913e1811983b49687d0cf Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 22 Mar 2026 21:26:39 +0200 Subject: [PATCH 4/6] fix(cli): schedule parser consistency and usability improvements - Suppress prompt in pipeline mode to match loop parser pattern - Remove dead handler warning (prompt now undefined in pipeline mode) - Add pipeline usage hint to missing-prompt error message - Add explicit radix 10 to parseInt for --max-runs Co-Authored-By: Claude --- src/cli/commands/schedule.ts | 12 +++++------- tests/unit/cli.test.ts | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/cli/commands/schedule.ts b/src/cli/commands/schedule.ts index 7ec470b..12d286a 100644 --- a/src/cli/commands/schedule.ts +++ b/src/cli/commands/schedule.ts @@ -85,7 +85,7 @@ export function parseScheduleCreateArgs(scheduleArgs: string[]): Result --cron "..." | --at "..." [options]'); + return err( + 'Usage: beat schedule create --cron "..." | --at "..." [options]\n Pipeline: beat schedule create --pipeline --step "lint" --step "test" --cron "0 9 * * *"', + ); } return ok({ - prompt: prompt || undefined, + prompt: isPipeline ? undefined : prompt || undefined, scheduleType, cronExpression, scheduledAt, @@ -228,10 +230,6 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): } const args = parsed.value; - if (args.isPipeline && args.prompt) { - ui.info(`Ignoring positional prompt text in --pipeline mode: "${args.prompt}". Use --step flags only.`); - } - const baseOptions = { scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, cronExpression: args.cronExpression, diff --git a/tests/unit/cli.test.ts b/tests/unit/cli.test.ts index f753ec5..b8df8da 100644 --- a/tests/unit/cli.test.ts +++ b/tests/unit/cli.test.ts @@ -1144,7 +1144,7 @@ describe('CLI - Schedule Commands', () => { expect(result.value.prompt).toBeUndefined(); }); - it('should preserve prompt in pipeline mode for handler warning', () => { + it('should suppress prompt in pipeline mode (matches loop parser)', () => { const result = parseScheduleCreateArgs([ 'extra', 'words', @@ -1159,7 +1159,7 @@ describe('CLI - Schedule Commands', () => { expect(result.ok).toBe(true); if (!result.ok) return; expect(result.value.isPipeline).toBe(true); - expect(result.value.prompt).toBe('extra words'); + expect(result.value.prompt).toBeUndefined(); }); it('should parse --priority with -p shorthand', () => { From a898634ca46f7d67f9dbf05789f4b7a6a03028f1 Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 22 Mar 2026 22:47:22 +0200 Subject: [PATCH 5/6] refactor(cli): convert parser types to discriminated unions (#114) Replace optional prompt/pipelineSteps with discriminated union on isPipeline, eliminating non-null assertions in handlers. Both parsers now encode mutual exclusivity in the type system. --- src/cli/commands/loop.ts | 43 +++++++++++++++++++++--------------- src/cli/commands/schedule.ts | 37 ++++++++++++++++++------------- tests/unit/cli.test.ts | 28 +++++++++++------------ 3 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/cli/commands/loop.ts b/src/cli/commands/loop.ts index 0a120ae..1b8706c 100644 --- a/src/cli/commands/loop.ts +++ b/src/cli/commands/loop.ts @@ -7,10 +7,11 @@ import { exitOnError, exitOnNull, withReadOnlyContext, withServices } from '../s import * as ui from '../ui.js'; /** - * Parsed arguments from CLI loop create command + * Parsed arguments from CLI loop create command. + * Discriminated union on `isPipeline`: pipeline variant has `pipelineSteps`, + * non-pipeline variant has `prompt`. Matches schedule parser pattern. */ -interface ParsedLoopArgs { - readonly prompt?: string; +interface ParsedLoopBaseArgs { readonly strategy: LoopStrategy; readonly exitCondition: string; readonly evalDirection?: 'minimize' | 'maximize'; @@ -20,11 +21,14 @@ interface ParsedLoopArgs { readonly maxConsecutiveFailures?: number; readonly cooldownMs?: number; readonly freshContext: boolean; - readonly pipelineSteps?: readonly string[]; readonly priority?: 'P0' | 'P1' | 'P2'; readonly agent?: AgentProvider; } +type ParsedLoopArgs = + | (ParsedLoopBaseArgs & { readonly isPipeline: true; readonly pipelineSteps: readonly string[] }) + | (ParsedLoopBaseArgs & { readonly isPipeline: false; readonly prompt: string }); + /** * Parse and validate loop create arguments * ARCHITECTURE: Pure function — no side effects, returns Result for testability @@ -62,25 +66,25 @@ export function parseLoopCreateArgs(loopArgs: string[]): Result= 0 (0 = unlimited)'); } i++; } else if (arg === '--max-failures' && next) { - maxFailures = parseInt(next); + maxFailures = parseInt(next, 10); if (isNaN(maxFailures) || maxFailures < 0) { return err('--max-failures must be >= 0'); } i++; } else if (arg === '--cooldown' && next) { - cooldown = parseInt(next); + cooldown = parseInt(next, 10); if (isNaN(cooldown) || cooldown < 0) { return err('--cooldown must be >= 0 (ms)'); } i++; } else if (arg === '--eval-timeout' && next) { - evalTimeout = parseInt(next); + evalTimeout = parseInt(next, 10); if (isNaN(evalTimeout) || evalTimeout < 1000) { return err('--eval-timeout must be >= 1000 (ms)'); } @@ -105,8 +109,8 @@ export function parseLoopCreateArgs(loopArgs: string[]): Result --until [options]'); } - return ok({ - prompt: isPipeline ? undefined : prompt, + const shared = { strategy: isOptimize ? LoopStrategy.OPTIMIZE : LoopStrategy.RETRY, exitCondition, evalDirection: direction, @@ -170,10 +173,14 @@ export function parseLoopCreateArgs(loopArgs: string[]): Result { @@ -216,7 +223,7 @@ async function handleLoopCreate(loopArgs: string[]): Promise { const { loopService } = await withServices(s); const result = await loopService.createLoop({ - prompt: args.prompt, + prompt: args.isPipeline ? undefined : args.prompt, strategy: args.strategy, exitCondition: args.exitCondition, evalDirection: toOptimizeDirection(args.evalDirection), @@ -226,7 +233,7 @@ async function handleLoopCreate(loopArgs: string[]): Promise { maxConsecutiveFailures: args.maxConsecutiveFailures, cooldownMs: args.cooldownMs, freshContext: args.freshContext, - pipelineSteps: args.pipelineSteps, + pipelineSteps: args.isPipeline ? args.pipelineSteps : undefined, priority: args.priority ? Priority[args.priority] : undefined, agent: args.agent, }); @@ -264,7 +271,7 @@ async function handleLoopList(loopArgs: string[]): Promise { status = next; i++; } else if (arg === '--limit' && next) { - limit = parseInt(next); + limit = parseInt(next, 10); i++; } } @@ -324,7 +331,7 @@ async function handleLoopGet(loopArgs: string[]): Promise { let historyLimit: number | undefined; const hlIdx = loopArgs.indexOf('--history-limit'); if (hlIdx !== -1 && loopArgs[hlIdx + 1]) { - historyLimit = parseInt(loopArgs[hlIdx + 1]); + historyLimit = parseInt(loopArgs[hlIdx + 1], 10); } const s = ui.createSpinner(); diff --git a/src/cli/commands/schedule.ts b/src/cli/commands/schedule.ts index 12d286a..bb12e6d 100644 --- a/src/cli/commands/schedule.ts +++ b/src/cli/commands/schedule.ts @@ -8,10 +8,11 @@ import { exitOnError, exitOnNull, withReadOnlyContext, withServices } from '../s import * as ui from '../ui.js'; /** - * Parsed arguments from CLI schedule create command + * Parsed arguments from CLI schedule create command. + * Discriminated union on `isPipeline`: pipeline variant has `pipelineSteps`, + * non-pipeline variant has `prompt`. Eliminates non-null assertions in handlers. */ -interface ParsedScheduleCreateArgs { - readonly prompt?: string; +interface ParsedScheduleBaseArgs { readonly scheduleType: 'cron' | 'one_time'; readonly cronExpression?: string; readonly scheduledAt?: string; @@ -23,10 +24,12 @@ interface ParsedScheduleCreateArgs { readonly expiresAt?: string; readonly afterScheduleId?: string; readonly agent?: AgentProvider; - readonly isPipeline: boolean; - readonly pipelineSteps?: readonly string[]; } +type ParsedScheduleCreateArgs = + | (ParsedScheduleBaseArgs & { readonly isPipeline: true; readonly pipelineSteps: readonly string[] }) + | (ParsedScheduleBaseArgs & { readonly isPipeline: false; readonly prompt: string }); + /** * Parse and validate schedule create arguments. */ @@ -141,7 +144,9 @@ export function parseScheduleCreateArgs(scheduleArgs: string[]): Result 0) { - return err('--step requires --pipeline. Did you mean: beat schedule create --pipeline --step "..." --step "..."'); + return err( + '--step requires --pipeline. Did you mean: beat schedule create --pipeline --step "..." --step "..." --cron "..."', + ); } // Non-pipeline mode: prompt is required @@ -152,8 +157,7 @@ export function parseScheduleCreateArgs(scheduleArgs: string[]): Result { @@ -247,7 +254,7 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): if (args.isPipeline) { const result = await service.createScheduledPipeline({ ...baseOptions, - steps: args.pipelineSteps!.map((prompt) => ({ prompt })), + steps: args.pipelineSteps.map((prompt) => ({ prompt })), }); const pipeline = exitOnError(result, undefined, 'Failed to create scheduled pipeline'); @@ -266,7 +273,7 @@ async function scheduleCreate(service: ScheduleService, scheduleArgs: string[]): const result = await service.createSchedule({ ...baseOptions, - prompt: args.prompt!, + prompt: args.prompt, }); const created = exitOnError(result, undefined, 'Failed to create schedule'); @@ -291,7 +298,7 @@ async function scheduleList(repo: ScheduleRepository, scheduleArgs: string[]): P status = next; i++; } else if (arg === '--limit' && next) { - limit = parseInt(next); + limit = parseInt(next, 10); i++; } } @@ -335,7 +342,7 @@ async function scheduleGet(repo: ScheduleRepository, scheduleArgs: string[]): Pr let historyLimit: number | undefined; const hlIdx = scheduleArgs.indexOf('--history-limit'); if (hlIdx !== -1 && scheduleArgs[hlIdx + 1]) { - historyLimit = parseInt(scheduleArgs[hlIdx + 1]); + historyLimit = parseInt(scheduleArgs[hlIdx + 1], 10); } const scheduleResult = await repo.findById(ScheduleId(scheduleId)); diff --git a/tests/unit/cli.test.ts b/tests/unit/cli.test.ts index b8df8da..9d2de1c 100644 --- a/tests/unit/cli.test.ts +++ b/tests/unit/cli.test.ts @@ -61,7 +61,7 @@ import type { TaskRepository, } from '../../src/core/interfaces'; import { err, ok, type Result } from '../../src/core/result'; -import { toMissedRunPolicy } from '../../src/utils/format'; +import { toMissedRunPolicy, toOptimizeDirection } from '../../src/utils/format'; import { TaskFactory } from '../fixtures/factories'; // Test constants @@ -1073,6 +1073,7 @@ describe('CLI - Schedule Commands', () => { if (!result.ok) return; expect(result.value.scheduleType).toBe('cron'); expect(result.value.cronExpression).toBe('0 9 * * *'); + if (result.value.isPipeline) return; expect(result.value.prompt).toBe('run tests'); }); @@ -1082,6 +1083,7 @@ describe('CLI - Schedule Commands', () => { if (!result.ok) return; expect(result.value.scheduleType).toBe('one_time'); expect(result.value.scheduledAt).toBe('2026-04-01T09:00:00Z'); + if (result.value.isPipeline) return; expect(result.value.prompt).toBe('deploy'); }); @@ -1140,8 +1142,8 @@ describe('CLI - Schedule Commands', () => { expect(result.ok).toBe(true); if (!result.ok) return; expect(result.value.isPipeline).toBe(true); + if (!result.value.isPipeline) return; expect(result.value.pipelineSteps).toEqual(['lint', 'test']); - expect(result.value.prompt).toBeUndefined(); }); it('should suppress prompt in pipeline mode (matches loop parser)', () => { @@ -1159,7 +1161,6 @@ describe('CLI - Schedule Commands', () => { expect(result.ok).toBe(true); if (!result.ok) return; expect(result.value.isPipeline).toBe(true); - expect(result.value.prompt).toBeUndefined(); }); it('should parse --priority with -p shorthand', () => { @@ -2501,9 +2502,10 @@ async function simulateScheduleCreate( const parsed = parseScheduleCreateArgs(buildScheduleCreateArgs(options)); if (!parsed.ok) return err(new BackbeatError(ErrorCode.INVALID_INPUT, parsed.error)); const args = parsed.value; + if (args.isPipeline) return err(new BackbeatError(ErrorCode.INVALID_INPUT, 'Expected non-pipeline')); return service.createSchedule({ - prompt: args.prompt!, + prompt: args.prompt, scheduleType: args.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, cronExpression: args.cronExpression, scheduledAt: args.scheduledAt, @@ -2563,9 +2565,10 @@ async function simulateScheduleCreatePipeline( const parsed = parseScheduleCreateArgs(args); if (!parsed.ok) return err(new BackbeatError(ErrorCode.INVALID_INPUT, parsed.error)); const p = parsed.value; + if (!p.isPipeline) return err(new BackbeatError(ErrorCode.INVALID_INPUT, 'Expected pipeline')); return service.createScheduledPipeline({ - steps: p.pipelineSteps!.map((prompt) => ({ prompt })), + steps: p.pipelineSteps.map((prompt) => ({ prompt })), scheduleType: p.scheduleType === 'cron' ? ScheduleType.CRON : ScheduleType.ONE_TIME, cronExpression: p.cronExpression, scheduledAt: p.scheduledAt, @@ -2635,22 +2638,17 @@ async function simulateLoopCreate(service: MockLoopService, args: string[]) { if (!parsed.ok) return err(new BackbeatError(ErrorCode.INVALID_INPUT, parsed.error)); const p = parsed.value; return service.createLoop({ - prompt: p.prompt, + prompt: p.isPipeline ? undefined : p.prompt, strategy: p.strategy, exitCondition: p.exitCondition, - evalDirection: - p.evalDirection === 'minimize' - ? OptimizeDirection.MINIMIZE - : p.evalDirection === 'maximize' - ? OptimizeDirection.MAXIMIZE - : undefined, + evalDirection: toOptimizeDirection(p.evalDirection), evalTimeout: p.evalTimeout, workingDirectory: p.workingDirectory, maxIterations: p.maxIterations, maxConsecutiveFailures: p.maxConsecutiveFailures, cooldownMs: p.cooldownMs, freshContext: p.freshContext, - pipelineSteps: p.pipelineSteps, + pipelineSteps: p.isPipeline ? p.pipelineSteps : undefined, priority: p.priority ? Priority[p.priority] : undefined, agent: p.agent, }); @@ -2761,6 +2759,7 @@ describe('CLI - Loop Commands', () => { if (!result.ok) return; expect(result.value.strategy).toBe(LoopStrategy.RETRY); expect(result.value.exitCondition).toBe('npm test'); + if (result.value.isPipeline) return; expect(result.value.prompt).toBe('fix tests'); }); @@ -2777,8 +2776,9 @@ describe('CLI - Loop Commands', () => { const result = parseLoopCreateArgs(['--pipeline', '--step', 'lint', '--step', 'test', '--until', 'true']); expect(result.ok).toBe(true); if (!result.ok) return; + expect(result.value.isPipeline).toBe(true); + if (!result.value.isPipeline) return; expect(result.value.pipelineSteps).toEqual(['lint', 'test']); - expect(result.value.prompt).toBeUndefined(); }); it('should parse --max-iterations', () => { From 704f18bb3bab3bdf697254689b6a07b6b200d16b Mon Sep 17 00:00:00 2001 From: Dean Sharon Date: Sun, 22 Mar 2026 22:48:44 +0200 Subject: [PATCH 6/6] =?UTF-8?q?fix:=20address=20PR=20review=20feedback=20?= =?UTF-8?q?=E2=80=94=20JSDoc=20and=20test=20helper=20parity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ARCHITECTURE JSDoc to parseScheduleCreateArgs (matches loop parser) - Add missing agent option to simulateScheduleCreate test helper --- src/cli/commands/schedule.ts | 1 + tests/unit/cli.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/cli/commands/schedule.ts b/src/cli/commands/schedule.ts index bb12e6d..3947540 100644 --- a/src/cli/commands/schedule.ts +++ b/src/cli/commands/schedule.ts @@ -32,6 +32,7 @@ type ParsedScheduleCreateArgs = /** * Parse and validate schedule create arguments. + * ARCHITECTURE: Pure function — no side effects, returns Result for testability */ export function parseScheduleCreateArgs(scheduleArgs: string[]): Result { const promptWords: string[] = []; diff --git a/tests/unit/cli.test.ts b/tests/unit/cli.test.ts index 9d2de1c..158f39c 100644 --- a/tests/unit/cli.test.ts +++ b/tests/unit/cli.test.ts @@ -2496,6 +2496,7 @@ async function simulateScheduleCreate( maxRuns?: number; expiresAt?: string; afterScheduleId?: string; + agent?: string; }, ) { const { parseScheduleCreateArgs } = await import('../../src/cli/commands/schedule');