diff --git a/assets/skills/spok-flow/SKILL.md b/assets/skills/spok-flow/SKILL.md index 0a94acb..8c9eaee 100644 --- a/assets/skills/spok-flow/SKILL.md +++ b/assets/skills/spok-flow/SKILL.md @@ -1,171 +1,43 @@ --- name: spok-flow -description: end-to-end problem validation → research → design → plan → implement → review → commit workflow for a single chunk, with native or hybrid model routing and an optional post-commit self-learn gate. Driven by spok-apply. +description: end-to-end problem validation → research → design → plan → implement → review → commit workflow for a single chunk, executed by delegating the whole loop to the `spok run` CLI command, with native or hybrid model routing. Driven by spok-apply. argument-hint: [hybrid] (absolute path to a pre-staged chunk directory containing ticket.md) -version: 0.9.0 +version: 1.0.0 --- # Flow Instructions -## 0. Receive Pre-Staged Task Directory +## 1. Receive the task directory -`spok-apply` has already created the task directory and written `ticket.md` for the chunk to execute. The argument to this skill is the **absolute path** to that directory (e.g. `spok/changes//.flow//`). A leading `hybrid` token selects the built-in Claude + Codex execution profile; remove that token before resolving the task directory. +`spok-apply` has already created the task directory and written `ticket.md` for the chunk to execute. The argument to this skill is the **absolute path** to that directory (e.g. `spok/changes//.flow//`). A leading `hybrid` token selects the built-in Claude + Codex execution profile; remove that token before resolving the path. -> Verify the directory exists and contains `ticket.md` using the **Read** tool. Do NOT recreate the directory or overwrite `ticket.md`. +Verify the directory exists and contains `ticket.md` using the **Read** tool. If `ticket.md` is missing, halt and report back — `spok-apply` is responsible for staging it. Do NOT recreate the directory or overwrite `ticket.md`. -If `ticket.md` is missing, halt and report back — `spok-apply` is responsible for staging it. +## 2. Run the flow -## 1. Deterministic Control Loop - -The `spok` CLI owns the inner flow sequence and resume state. Do not choose, skip, reorder, or rename steps yourself. - -For a hybrid invocation, prefix every `spok flow status`, `spok flow next`, and -`spok flow complete` command with `SPOK_FLOW_PROFILE=hybrid`. For a default -invocation, run the commands without that environment variable. An existing -workflow state owns its persisted profile; if the requested profile conflicts, -surface the CLI blocker exactly. - -Run: +Execute the flow in the foreground with the host's shell tool: ```bash -spok flow status "" --json +spok run "" --json ``` -Hybrid equivalent: +When the `hybrid` token was present, append `--profile hybrid`: ```bash -SPOK_FLOW_PROFILE=hybrid spok flow status "" --json +spok run "" --json --profile hybrid ``` -If it returns `state: "blocked"`, halt and report the `reason` exactly, applying the design-review clause in step 6 when it matches. - -Then repeat this loop until the CLI returns `state: "complete"`: - -1. Run: - - ```bash - spok flow next "" --json - ``` - - For hybrid execution, use: - - ```bash - SPOK_FLOW_PROFILE=hybrid spok flow next "" --json - ``` - -2. If `next` returns `state: "blocked"`, halt and report the `reason` exactly, applying the design-review clause in step 6 when it matches. If it returns `state: "complete"`, return success to `spok-apply`. - -3. Read the returned `step` object: - - `id` is the workflow step id. - - `skill` is the exact skill to invoke. - - `runner` is the exact tool that must execute the step: `claude` or `codex`. - - `model` is the exact model to pass to that runner. - - `effort` is present when the step carries a reasoning-effort hint; relay it to the selected runner when present. - - `argument` is the exact argument to pass to that skill. - - `expectedOutput` is present for file-producing steps. - - `prompt` is the **complete subagent prompt**, composed by the CLI. It already - carries the skill invocation, the return contract, any step-specific clause, - and the repository rules from `spok/MEMORY.md`. Do not rewrite, summarize, or - add to it. - - If the response carries `memoryWarning` or `workRootWarning`, surface it to the user once and continue. - -4. Dispatch the step through `step.runner`. - - Detect the active harness once: a non-empty `CODEX_HOME` means `codex`; - otherwise it is `claude`. - - - When `step.runner` matches the active harness, delegate in the foreground - through the current host's native subagent mechanism to the - host-owned `general-purpose` agent. Pass `model: `, (when - present) `effort: `, and `` **verbatim** as the - prompt. - - When `step.runner` is `codex` from another harness, first verify `codex` is - on `PATH`, then run `codex exec` sequentially in the foreground. Use - `--ephemeral`, `--dangerously-bypass-hook-trust`, `--cd `, - `--model `, `--sandbox workspace-write`, and, when - `step.effort` is present, `-c model_reasoning_effort=""`. - Pass `` **verbatim** on stdin with `-`; do not interpolate it - into a shell command. - - When `step.runner` is `claude` from another harness, first verify `claude` - is on `PATH`, then run `claude -p` sequentially in the foreground. Use - `--no-session-persistence`, `--model `, - `--permission-mode auto`, text output, and, when `step.effort` is present, - `--effort `. Pass `` **verbatim** on stdin; do not - interpolate it into a shell command. - - Resolve `` with `git -C "" rev-parse --show-toplevel`. - Use `--dangerously-bypass-hook-trust` to run enabled hooks without an - interactive trust prompt; it does not enable disabled hooks or relax the - sandbox. Do not use `--dangerously-bypass-approvals-and-sandbox`. If the - executable is missing, authentication fails, or the child exits nonzero, - report the error and halt. Do not call `spok flow complete`; leaving the - current step ready makes the run safely resumable after the tool is fixed. - - Run every path **sequentially in the foreground** because each step depends - on the previous step's validated artifact or recorded result. Do not invoke - the step skill inline: process isolation keeps each step's context bounded. - -5. Record completion with the CLI. - - A `--summary` is recorded permanently in `workflow-state.json`. Do not relay a verification claim that cannot be attributed to a command that ran during the step. If the subagent reports "lint clean" or "tests pass" without naming the command it ran, drop the claim from the summary rather than passing it through — the CLI checks only that the summary is non-empty, so you are the last check on it. - - File-producing steps (the CLI verifies `expectedOutput` exists and is non-empty): - - ```bash - spok flow complete "" --step "" --json - ``` - - Prefix this command with `SPOK_FLOW_PROFILE=hybrid` for a hybrid run, as - described above. - - - `validate` additionally has its recorded verdict read from `validation.md` by the CLI: `PASS` completes the step. A `FAIL` with repair attempts remaining is a *successful* completion that routes to a `repair` step and then back to `validate` — the CLI may return `validate` more than once; complete every occurrence with the same bare `--step validate`. When repair attempts are exhausted and the verdict is still `FAIL`, `complete` (and subsequent `next`/`status`) return `state: "blocked"` with an exhausted-repair reason: report it exactly as returned — do not retry the step and do not edit `validation.md` to unblock it. An unreadable verdict blocks as before. - - - `implement`, `simplify`, and `repair` (dispatched like any other step when the CLI returns it): - - ```bash - spok flow complete "" --step "" --summary "" --json - ``` - - The `implement` prompt requires the subagent to end its reply with a - `Work root: ` line naming the repository it edited. Pass that - path through so the commit step is told where the changes live: - - ```bash - spok flow complete "" --step "implement" --summary "" --work-root "" --json - ``` - - Pass `--work-root` on `simplify` or `repair` too when that subagent reports a - different repository. If the subagent reported no work root, omit the flag — - the CLI degrades to unsteered commit discovery and warns. Never invent the - path. A path that is relative or does not exist blocks the completion. - - - `commit`: - - ```bash - spok flow complete "" --step "commit" --commit "" --summary "" --json - ``` - - When a work root was recorded, the CLI resolves the SHA in that repository - and blocks when it is not a commit reachable from `HEAD` there. Report such - a block exactly; do not retry with a different SHA. - - - `self-learn` is an optional file-producing advisory gate returned only when - project config enables `flow.self_learn: true`. Complete it like any other - file-producing step. Its findings do not fail or amend the commit. +Do not background the command and do not run anything in parallel with it. Harness progress streams on stderr; stdout carries one JSONL event per line, each with `schemaVersion: 1`. -6. If `complete` returns `state: "blocked"`, halt and report the `reason` exactly. When the `reason` starts with `Step design-review recorded a FAIL verdict`, read `/design-review.md` and present its `## Human Decisions Required` section verbatim alongside the reason, then halt — do not retry the step and do not edit `design-review.md` to unblock it. When the `reason` starts with `Step design-review has no readable verdict`, read `/design-review.md` if it exists and present its `## Human Decisions Required` section verbatim alongside the reason if that section is present, then halt and report as a generic blocked state. +## 3. Relay the outcome -Do not restate or assume the step order — `spok flow next` is the only source of truth. -Do not derive or override runner or model routing inside this skill — `spok flow next --json` is the source of truth, including `step.runner`, `step.model`, and `step.effort`. -In plain terms: spok flow next --json is the source of truth for model routing. -Step-specific instructions — including the `implement` no-commit rule — are composed -into `step.prompt` by the CLI. Do not restate them. +Report the run's result from its exit code and final JSONL event. Relay what the CLI reported; never invent or soften an outcome. - -## Important guidelines +- **Exit 0** (`complete` event): report success to `spok-apply`, including the recorded commit when the event carries a `commit`. +- **Exit 2** (`blocked` event): report the event's `reason` verbatim. When the event carries `humanDecisions`, present that `## Human Decisions Required` content verbatim alongside the reason. Do not retry, and do not edit artifacts to unblock the flow. +- **Exit 3** (`step_failed` event): report the failure. Re-invoking the same command resumes at the same step. +- **Exit 1 or a signal code (130/143)**: report the error or cancellation verbatim. +- **A `warning` event**, at any point in the stream: surface its `message` to the user once and continue; a warning does not stop the run. -- Raise questions or concerns about objectives, design, or plan to the user at any time using the **AskUserQuestion** tool. -- Run step subagents **sequentially in the foreground** because each step depends on the previous step's validated artifact or recorded result. -- Let `spok flow next` choose the next step. Let `spok flow complete` validate step completion. -- Use a **TaskList** to track the steps and their status. +## Guardrails - +`spok run` owns all execution semantics: step order, routing, models, prompt composition, harness dispatch, and completion recording. Do not choose, skip, reorder, or re-route steps; do not drive the flow state machine directly; do not construct harness invocations; do not restate routing, models, or step prompts. diff --git a/docs/cli.md b/docs/cli.md index e4fc8a3..ca5dca0 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -12,7 +12,7 @@ For slash commands, see [Commands](commands.md). For workflow patterns, see [Wor | Surface | Verbs | Purpose | |---------|-------|---------| -| **User-facing** | `version`, `init`, `doctor`, `update`, `skills install`, `archive`, `list` | Inspect, setup, diagnose, refresh, finalize, browse | +| **User-facing** | `version`, `init`, `doctor`, `update`, `skills install`, `archive`, `list`, `run` | Inspect, setup, diagnose, refresh, finalize, browse, drive | | **Agent discovery** | `capabilities` | Machine-readable CLI manifest for agents and scripts | | **Internal plumbing** | `new`, `status`, `instructions` | Called by skills; safe to inspect, not meant for daily human use | @@ -274,6 +274,43 @@ spok archive update-ci-config --skip-specs --- +### `spok run` + +Drive the deterministic flow of a staged task to a terminal outcome, dispatching each step to its routed harness (`claude` or `codex`) as a foreground subprocess. Progress is reported as human-readable lines, or as a machine-readable JSONL event stream with `--json` for orchestrators and CI. + +``` +spok run [options] +``` + +**Arguments:** + +| Argument | Required | Description | +|----------|----------|-------------| +| `task-dir` | Yes | Flow task directory (e.g. `spok/changes//.flow/`) | + +**Options:** + +| Option | Description | +|--------|-------------| +| `--profile ` | Flow profile: `claude`, `codex`, or `hybrid` | +| `--json` | Emit JSONL supervision events on stdout | + +**Examples:** + +```bash +# Drive a flow to completion with human-readable progress +spok run spok/changes/add-dark-mode/.flow/chunk-one + +# Supervise from an orchestrator via JSONL events +spok run spok/changes/add-dark-mode/.flow/chunk-one --json +``` + +**JSONL protocol (`--json`):** + +With `--json`, stdout is pure protocol — one JSON object per line, each carrying `schemaVersion: 1` — while harness output stays on stderr. Events: `run_started` (taskDir, profile), `step_started` (step, runner, model, effort?, attempt?), `step_completed` (step), `warning` (message — a relayed memory or work-root warning, emitted at most once per distinct message per run), `blocked` (code, reason, and `humanDecisions` when a design-review FAIL records a `## Human Decisions Required` section), `step_failed` (step, reason), and terminal `complete` (commit? from the recorded commit step). + +--- + ## Agent Discovery ### `spok capabilities` @@ -525,6 +562,17 @@ spok instructions design --change add-dark-mode --json | `0` | Success | | `1` | Error (validation failure, missing files, etc.) | +`spok run` reports the flow's terminal outcome through its exit code: + +| Code | Meaning | +|------|---------| +| `0` | Flow complete | +| `1` | Usage error (e.g. invalid `--profile` value) | +| `2` | Blocked (the state machine reported a blocker) | +| `3` | Execution failure (no dispatchable prompt, missing runner, harness failure, or completion-contract violation) | +| `130` | Interrupted (SIGINT) | +| `143` | Terminated (SIGTERM) | + --- ## Environment Variables diff --git a/src/cli/index.ts b/src/cli/index.ts index 2b6208c..fdfa7fd 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -18,12 +18,15 @@ import { flowCompleteCommand, flowNextCommand, flowStatusCommand, + isFlowProfile, + runCommand, DEFAULT_SCHEMA, type StatusOptions, type InstructionsOptions, type NewChangeOptions, type FlowCommandOptions, type FlowCompleteCommandOptions, + type RunCommandOptions, } from '../commands/workflow/index.js'; import { maybeShowTelemetryNotice, trackCommand, trackTelemetryEvent, shutdown } from '../telemetry/index.js'; import { collectCliSignals } from './signals.js'; @@ -97,6 +100,9 @@ const COMMAND_VISIBILITY: Record = { instructions: 'skill', new: 'skill', 'new change': 'skill', + // Documented under User-Facing Commands in docs/cli.md; humans and external + // orchestrators invoke it directly, not only skills. + run: 'user', flow: 'internal', 'flow status': 'internal', 'flow next': 'internal', @@ -592,6 +598,31 @@ flowCmd } }); +program + .command('run ') + .description('Drive the deterministic flow to completion by dispatching each step to its harness') + .option('--profile ', 'Flow profile: claude, codex, or hybrid') + .option('--json', 'Output as JSON') + .action(async (taskDir: string, options: RunCommandOptions) => { + // An unrecognized profile is a usage error, caught before the state machine + // could report it as a blocker and exit 2. + if (options.profile && !isFlowProfile(options.profile)) { + if (!options.json) console.log(); + ora().fail( + `Error: Invalid --profile value: ${options.profile}. Expected claude, codex, or hybrid.` + ); + process.exit(1); + } + + try { + process.exitCode = await runCommand(taskDir, options); + } catch (error) { + if (!options.json) console.log(); + ora().fail(`Error: ${(error as Error).message}`); + process.exit(1); + } + }); + const newCmd = program.command('new').description('Create new items'); newCmd diff --git a/src/commands/workflow/flow.ts b/src/commands/workflow/flow.ts index fb52a8f..4dad21d 100644 --- a/src/commands/workflow/flow.ts +++ b/src/commands/workflow/flow.ts @@ -30,7 +30,9 @@ const VALIDATE_STEP_ID = 'validate'; const REPAIR_STEP_ID = 'repair'; const SELF_LEARN_STEP_ID = 'self-learn'; const FLOW_EVENT_DIR = '.spok'; -const FLOW_PROFILE_ENV = 'SPOK_FLOW_PROFILE'; +export const FLOW_PROFILE_ENV = 'SPOK_FLOW_PROFILE'; +/** Summaries are recorded permanently in workflow-state.json; every intake caps them. */ +export const SUMMARY_MAX_LENGTH = 4000; /** Bounded: a FAIL splices at most this many [repair, validate] pairs per flow. */ const MAX_REPAIR_ATTEMPTS = 2; @@ -108,7 +110,7 @@ function detectTool(): FlowRunner { return process.env.CODEX_HOME?.trim() ? 'codex' : 'claude'; } -function isFlowProfile(value: unknown): value is FlowProfile { +export function isFlowProfile(value: unknown): value is FlowProfile { return value === 'claude' || value === 'codex' || value === 'hybrid'; } @@ -138,6 +140,8 @@ export interface FlowStep { effort?: FlowEffort; argument: string; expectedOutput?: string; + /** How the step records completion; drives how a driver reads the harness reply. */ + completionKind: FlowCompletionKind; status: FlowStepStatus; result?: FlowStepResult; /** 1-based repair-cycle attempt for spliced repair/validate steps; absent on the base graph. */ @@ -448,6 +452,10 @@ const STEP_PROMPT_CLAUSES: Partial> = { 'absolute path of the repository working tree you edited (the git worktree root that ' + 'holds the changed files, which may differ from the task directory). Report the path ' + '`git -C rev-parse --show-toplevel` prints, not a guess.', + commit: + 'End your reply with a final line reading `Commit: `, naming the SHA of the ' + + 'commit you created. Report the SHA `git rev-parse HEAD` prints after committing, ' + + 'not a guess.', [SELF_LEARN_STEP_ID]: 'This gate is advisory. Do not fail, amend, or rewrite the commit.', }; @@ -605,6 +613,7 @@ function stepFromDefinition( effort: definition.effort, argument: definition.argument, expectedOutput: definition.expectedOutput, + completionKind: definition.completionKind, status, result, attempt: definition.attempt, @@ -820,7 +829,13 @@ async function pathIsDirectory(targetPath: string): Promise { } } -function flowBlockCode(reason: string): string { +/** The two block codes whose blocked events carry design-review human decisions. */ +export const DESIGN_REVIEW_BLOCK_CODES = [ + 'design_review_verdict_fail', + 'design_review_verdict_unreadable', +] as const; + +export function flowBlockCode(reason: string): string { if (reason.startsWith('Task directory does not exist:')) return 'missing_task_dir'; if (reason.startsWith('Missing required ticket file:')) return 'missing_ticket'; if (reason.startsWith('Invalid workflow state JSON:')) return 'invalid_state_json'; @@ -835,10 +850,10 @@ function flowBlockCode(reason: string): string { if (reason.includes('must set Flow Decision to proceed')) return 'flow_decision_not_proceed'; if (reason.includes('repair attempts')) return 'repair_attempts_exhausted'; if (reason.startsWith(`Step ${DESIGN_REVIEW_STEP_ID} recorded a FAIL verdict`)) { - return 'design_review_verdict_fail'; + return DESIGN_REVIEW_BLOCK_CODES[0]; } if (reason.startsWith(`Step ${DESIGN_REVIEW_STEP_ID} has no readable verdict`)) { - return 'design_review_verdict_unreadable'; + return DESIGN_REVIEW_BLOCK_CODES[1]; } if (reason.includes('recorded a FAIL verdict')) return 'validation_verdict_fail'; if (reason.includes('has no readable verdict')) return 'validation_verdict_unreadable'; @@ -886,9 +901,13 @@ async function recordFlowResponse( await appendFlowEvent(response.taskDir, event); } +/** Write-then-rename: an interrupt mid-write must never leave truncated JSON behind. */ async function writeState(state: WorkflowState): Promise { state.updatedAt = nowIso(); - await fs.writeFile(getStatePath(state.taskDir), `${JSON.stringify(state, null, 2)}\n`, 'utf-8'); + const statePath = getStatePath(state.taskDir); + const tempPath = `${statePath}.tmp`; + await fs.writeFile(tempPath, `${JSON.stringify(state, null, 2)}\n`, 'utf-8'); + await fs.rename(tempPath, statePath); } async function loadOrCreateState(taskDirInput: string): Promise { @@ -1118,7 +1137,8 @@ function validateFileCompletion( } } -function extractMarkdownSection(content: string, heading: string): string { +/** Body of the `## ` section, verbatim to the next `##` or EOF; '' when absent. */ +export function extractMarkdownSection(content: string, heading: string): string { const escapedHeading = heading.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const match = content.match( new RegExp(`(?:^|\\n)##\\s+${escapedHeading}\\s*\\n([\\s\\S]*?)(?=\\n##\\s+|$)`, 'i') @@ -1141,7 +1161,7 @@ function flowDecisionAllowsProceed(content: string): boolean { } /** Undefined when the artifact cannot be read: the callers treat that as a failed gate. */ -async function readArtifact(targetPath: string): Promise { +export async function readArtifact(targetPath: string): Promise { try { return await fs.readFile(targetPath, 'utf-8'); } catch { @@ -1222,7 +1242,7 @@ async function validateValidationVerdict(definition: StepDefinition): Promise { +export async function runGit(workRoot: string, args: string[]): Promise { try { const { stdout } = await execFileAsync('git', ['-C', workRoot, ...args]); return stdout; @@ -1304,7 +1324,7 @@ async function completeCommitResult( return { commit: resolvedCommit, - summary: input.summary?.trim() || undefined, + summary: input.summary?.trim().slice(0, SUMMARY_MAX_LENGTH) || undefined, workRoot, completedAt: nowIso(), }; @@ -1354,7 +1374,7 @@ async function completeStepResult( } if (definition.completionKind === 'summary') { - const summary = input.summary?.trim(); + const summary = input.summary?.trim().slice(0, SUMMARY_MAX_LENGTH); if (!summary) { return `Step ${definition.id} must provide a non-empty --summary.`; } diff --git a/src/commands/workflow/harness-runners.ts b/src/commands/workflow/harness-runners.ts new file mode 100644 index 0000000..99b013f --- /dev/null +++ b/src/commands/workflow/harness-runners.ts @@ -0,0 +1,193 @@ +/** + * The harness boundary the run loop dispatches through: one runner per harness + * (claude, codex). The loop never constructs a command line itself. The + * invocations mirror the recipes the spok-flow skill has always specified. + */ +import { spawn, type ChildProcess } from 'node:child_process'; +import { promises as fs } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { FLOW_PROFILE_ENV, runGit } from './flow.js'; +import type { FlowRunner } from './flow.js'; + +export interface HarnessRunRequest { + model: string; + effort?: string; + /** The state machine's composed prompt, dispatched verbatim. */ + prompt: string; + projectRoot: string; +} + +export type HarnessRunResult = + | { ok: true; finalMessage: string } + | { ok: false; reason: string }; + +export interface HarnessRunner { + run(request: HarnessRunRequest): Promise; +} + +export type RunnerLookup = (runner: FlowRunner) => HarnessRunner | undefined; + +/** Exposed for tests: the exact argv, prompt never included. */ +export function buildClaudeArgv(request: HarnessRunRequest): string[] { + const argv = [ + '-p', + '--no-session-persistence', + '--permission-mode', + 'auto', + '--model', + request.model, + ]; + if (request.effort) argv.push('--effort', request.effort); + return argv; +} + +/** Exposed for tests: the exact argv, prompt never included; `-` reads it from stdin. */ +export function buildCodexArgv(request: HarnessRunRequest, outputFile: string): string[] { + const argv = [ + 'exec', + '--ephemeral', + '--dangerously-bypass-hook-trust', + '--cd', + request.projectRoot, + '--sandbox', + 'workspace-write', + '--model', + request.model, + ]; + if (request.effort) argv.push('-c', `model_reasoning_effort="${request.effort}"`); + argv.push('-o', outputFile, '-'); + return argv; +} + +/** `git -C rev-parse --show-toplevel`, falling back to the task dir. */ +export async function resolveProjectRoot(taskDir: string): Promise { + const toplevel = (await runGit(taskDir, ['rev-parse', '--show-toplevel']))?.trim(); + return toplevel || path.resolve(taskDir); +} + +let activeChild: ChildProcess | undefined; + +/** + * Signals the running harness's whole process group, if any. Cancellation + * records no completion, so the current step stays ready and the run resumes + * where it stopped. + */ +export function killActiveHarnessGroup(signal: 'SIGINT' | 'SIGTERM'): void { + const pid = activeChild?.pid; + if (!pid) return; + try { + process.kill(-pid, signal); // Negative pid: the child's whole process group. + } catch { + // Child already exited between the check and the kill. + } +} + +type SpawnOutcome = { ok: true; stdout: string } | { ok: false; reason: string }; + +/** + * Spawns the harness detached so it leads its own process group (the signal + * handlers kill the group), pipes the prompt to stdin, and resolves on close. + * `forwardStdoutToStderr` streams codex's progress output through; claude's + * stdout is captured as the final message instead. stderr is inherited. + */ +function spawnHarness( + command: string, + argv: string[], + prompt: string, + cwd: string, + forwardStdoutToStderr: boolean +): Promise { + return new Promise((resolve) => { + // The run's profile override must not leak into the harness: a nested spok + // invocation from a subagent would inherit a profile it was never given. + const env = { ...process.env }; + delete env[FLOW_PROFILE_ENV]; + const child = spawn(command, argv, { + cwd, + env, + detached: true, + stdio: ['pipe', 'pipe', 'inherit'], + }); + activeChild = child; + + let stdout = ''; + child.stdout!.setEncoding('utf-8'); + child.stdout!.on('data', (chunk: string) => { + if (forwardStdoutToStderr) { + process.stderr.write(chunk); + } else { + stdout += chunk; + } + }); + + child.on('error', (error: NodeJS.ErrnoException) => { + activeChild = undefined; + resolve({ + ok: false, + reason: + error.code === 'ENOENT' + ? `${command} executable not found on PATH` + : `${command} failed to start: ${error.message}`, + }); + }); + child.on('close', (code) => { + activeChild = undefined; + resolve( + code === 0 + ? { ok: true, stdout } + : { ok: false, reason: `${command} exited with code ${code ?? 'null (signal)'}` } + ); + }); + + child.stdin!.on('error', () => { + // A child that dies before reading its stdin surfaces via 'error'/'close'. + }); + child.stdin!.end(prompt); + }); +} + +export const claudeRunner: HarnessRunner = { + async run(request) { + const outcome = await spawnHarness( + 'claude', + buildClaudeArgv(request), + request.prompt, + request.projectRoot, + false + ); + return outcome.ok ? { ok: true, finalMessage: outcome.stdout } : outcome; + }, +}; + +export const codexRunner: HarnessRunner = { + async run(request) { + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'spok-run-codex-')); + const outputFile = path.join(tempDir, 'last-message.txt'); + try { + const outcome = await spawnHarness( + 'codex', + buildCodexArgv(request, outputFile), + request.prompt, + request.projectRoot, + true // codex streams progress on stdout; forward it to parent stderr. + ); + if (!outcome.ok) return outcome; + + try { + return { ok: true, finalMessage: await fs.readFile(outputFile, 'utf-8') }; + } catch { + return { ok: false, reason: `codex did not write its final message to ${outputFile}` }; + } + } finally { + await fs.rm(tempDir, { recursive: true, force: true }).catch(() => {}); + } + }, +}; + +const RUNNERS: Record = { + claude: claudeRunner, + codex: codexRunner, +}; + +export const defaultRunnerLookup: RunnerLookup = (runner: FlowRunner) => RUNNERS[runner]; diff --git a/src/commands/workflow/index.ts b/src/commands/workflow/index.ts index b25fbfa..b80dde8 100644 --- a/src/commands/workflow/index.ts +++ b/src/commands/workflow/index.ts @@ -10,9 +10,12 @@ export type { StatusOptions } from './status.js'; export { instructionsCommand, applyInstructionsCommand } from './instructions.js'; export type { InstructionsOptions } from './instructions.js'; -export { flowCompleteCommand, flowNextCommand, flowStatusCommand } from './flow.js'; +export { flowCompleteCommand, flowNextCommand, flowStatusCommand, isFlowProfile } from './flow.js'; export type { FlowCommandOptions, FlowCompleteCommandOptions } from './flow.js'; +export { runCommand } from './run.js'; +export type { RunCommandOptions } from './run.js'; + export { newChangeCommand } from './new-change.js'; export type { NewChangeOptions } from './new-change.js'; diff --git a/src/commands/workflow/run.ts b/src/commands/workflow/run.ts new file mode 100644 index 0000000..5fddb7e --- /dev/null +++ b/src/commands/workflow/run.ts @@ -0,0 +1,317 @@ +/** + * Drives the deterministic flow state machine to a terminal outcome without an + * interpreting agent: each ready step is dispatched to its routed harness as a + * foreground subprocess, and completion is recorded from artifacts and a + * machine-readable output contract — never from interpretation. + */ +import path from 'node:path'; +import { + DESIGN_REVIEW_BLOCK_CODES, + FLOW_PROFILE_ENV, + SUMMARY_MAX_LENGTH, + completeFlowStep, + extractMarkdownSection, + flowBlockCode, + getFlowNext, + readArtifact, +} from './flow.js'; +import type { FlowCompleteInput, FlowStep } from './flow.js'; +import { + defaultRunnerLookup, + killActiveHarnessGroup, + resolveProjectRoot, +} from './harness-runners.js'; +import type { RunnerLookup } from './harness-runners.js'; + +const SIGNAL_EXIT_CODES = { SIGINT: 130, SIGTERM: 143 } as const; + +/** + * Blocked codes that mean the just-dispatched harness broke its completion + * contract (no artifact, unreadable verdict, unusable work root). The blocked + * state is never persisted, so a plain re-run resumes the same step — these + * report as resumable step failures (exit 3), not do-not-retry blockers. + */ +const STEP_CONTRACT_FAILURE_CODES = new Set([ + 'missing_output', + 'validation_verdict_unreadable', + 'invalid_work_root', + 'missing_work_root', +]); + +export interface RunCommandOptions { + profile?: string; + json?: boolean; +} + +function capSummary(text: string): string { + return text.slice(0, SUMMARY_MAX_LENGTH); +} + +/** Splits the already-trimmed message into [body, lastNonEmptyLine]. */ +function splitTrailingLine(trimmed: string): { body: string; lastLine: string } { + const newlineIndex = trimmed.lastIndexOf('\n'); + if (newlineIndex === -1) return { body: '', lastLine: trimmed }; + return { + body: trimmed.slice(0, newlineIndex).trim(), + lastLine: trimmed.slice(newlineIndex + 1).trim(), + }; +} + +/** Undefined means the contract was not met: the step must not be completed. */ +export function parseSummaryCompletion( + finalMessage: string +): { summary: string; workRoot?: string } | undefined { + const trimmed = finalMessage.trim(); + if (!trimmed) return undefined; + + const { body, lastLine } = splitTrailingLine(trimmed); + const workRootMatch = lastLine.match(/^[*_]*Work root[*_]*:[*_]*\s*(.+)$/i); + if (!workRootMatch) return { summary: capSummary(trimmed) }; + + if (!body) return undefined; // Only a Work root line: no summary to record. + const workRoot = stripTokenDecoration(workRootMatch[1]!.trim()); + return { summary: capSummary(body), workRoot }; +} + +/** Harness replies wrap tokens in backticks/quotes or end them with punctuation. */ +function stripTokenDecoration(token: string): string { + return token.replace(/^[`'"]+|[`'".,;:]+$/g, ''); +} + +/** Undefined means no trailing `Commit: ` line: the step must not be completed. */ +export function parseCommitCompletion( + finalMessage: string +): { commit: string; summary?: string } | undefined { + const trimmed = finalMessage.trim(); + if (!trimmed) return undefined; + + const { body, lastLine } = splitTrailingLine(trimmed); + const commitMatch = lastLine.match(/^[*_]*Commit[*_]*:[*_]*\s*(\S+)$/i); + if (!commitMatch) return undefined; + + const commit = stripTokenDecoration(commitMatch[1]!); + if (!commit) return undefined; + return { commit, summary: body ? capSummary(body) : undefined }; +} + +/** FlowCompleteInput for the step, or a failure reason string. */ +function buildCompleteInput(step: FlowStep, finalMessage: string): FlowCompleteInput | string { + if (step.completionKind === 'file') { + // The artifact gate in completeFlowStep validates the step's own expected + // output path; the harness message is ignored. + return { step: step.id }; + } + + if (step.completionKind === 'commit') { + const parsed = parseCommitCompletion(finalMessage); + if (!parsed) { + return 'harness reply did not end with a `Commit: ` line'; + } + return { step: step.id, commit: parsed.commit, summary: parsed.summary }; + } + + const parsed = parseSummaryCompletion(finalMessage); + if (!parsed) { + return 'harness reply contained no summary text'; + } + return { step: step.id, summary: parsed.summary, workRoot: parsed.workRoot }; +} + +/** + * `steps` holds the persisted list; the dispatched step is a prompt-carrying + * copy, so the position comes from the ready status rather than identity. + */ +function describeStep(step: FlowStep, steps: FlowStep[]): string { + const position = steps.findIndex((candidate) => candidate.status === 'ready') + 1; + const effort = step.effort ? ` (effort ${step.effort})` : ''; + return `[${position}/${steps.length}] ${step.id} → ${step.runner} ${step.model}${effort}`; +} + +/** The recorded commit step's SHA on the terminal complete response, if any. */ +function findCommitSha(steps: FlowStep[]): string | undefined { + return steps.find((step) => step.completionKind === 'commit')?.result?.commit; +} + +/** One semantic progress event per report site; `--json` renders these as JSONL. */ +type RunEvent = + | { event: 'run_started'; taskDir: string; profile: string | null } + | { + event: 'step_started'; + step: string; + runner: string; + model: string; + effort?: string; + attempt?: number; + } + | { event: 'step_completed'; step: string } + | { event: 'warning'; message: string } + | { event: 'blocked'; code: string; reason: string; humanDecisions?: string } + | { event: 'step_failed'; step: string | null; reason: string } + | { event: 'complete'; commit?: string }; + +/** JSONL under `--json`; otherwise the human line, at the sites that have one. */ +function report(json: boolean, event: RunEvent, humanLine?: string): void { + if (json) { + // JSON.stringify drops undefined-valued keys, so optional fields stay absent. + console.log(JSON.stringify({ schemaVersion: 1, ...event })); + } else if (humanLine !== undefined) { + console.log(humanLine); + } +} + +const HUMAN_DECISION_CODES = new Set(DESIGN_REVIEW_BLOCK_CODES); + +/** + * Attachment for a blocked event: only the two design-review codes qualify, and + * only when the task's design review carries a `## Human Decisions Required` + * section. Best-effort — an unreadable file or an empty section attaches nothing. + */ +async function humanDecisionsFor(taskDir: string, code: string): Promise { + if (!HUMAN_DECISION_CODES.has(code)) return undefined; + const content = await readArtifact(path.join(taskDir, 'design-review.md')); + if (content === undefined) return undefined; + return extractMarkdownSection(content, 'Human Decisions Required') || undefined; +} + +/** Reports the blocker, then yields the run's exit code. */ +async function reportBlocked(json: boolean, taskDir: string, reason?: string): Promise { + const blockReason = reason ?? 'Unknown workflow blocker'; + const code = flowBlockCode(blockReason); + report( + json, + { + event: 'blocked', + code, + reason: blockReason, + humanDecisions: await humanDecisionsFor(taskDir, code), + }, + `Blocked: ${blockReason}` + ); + return 2; +} + +/** Reports the failure, then yields the run's exit code. */ +function reportStepFailed(json: boolean, stepId: string | null, detail: string): number { + const reason = stepId === null ? detail : `Step ${stepId} failed: ${detail}`; + report(json, { event: 'step_failed', step: stepId, reason }, reason); + return 3; +} + +/** Overrides the flow profile for this run; returns the restorer. */ +function overrideFlowProfile(profile: string | undefined): () => void { + const original = process.env[FLOW_PROFILE_ENV]; + if (profile) process.env[FLOW_PROFILE_ENV] = profile; + return () => { + if (original === undefined) delete process.env[FLOW_PROFILE_ENV]; + else process.env[FLOW_PROFILE_ENV] = original; + }; +} + +/** Routes SIGINT/SIGTERM to the running harness's process group; returns the disposer. */ +function forwardSignalsToHarness(): () => void { + const removers = (['SIGINT', 'SIGTERM'] as const).map((signal) => { + const handler = () => { + killActiveHarnessGroup(signal); + process.exit(SIGNAL_EXIT_CODES[signal]); + }; + process.on(signal, handler); + return () => void process.removeListener(signal, handler); + }); + return () => removers.forEach((remove) => remove()); +} + +export async function runCommand( + taskDir: string, + options: RunCommandOptions = {}, + runners: RunnerLookup = defaultRunnerLookup +): Promise { + const json = options.json === true; + const restoreFlowProfile = overrideFlowProfile(options.profile); + const stopForwardingSignals = forwardSignalsToHarness(); + + try { + const projectRoot = await resolveProjectRoot(taskDir); + let started = false; + // Each distinct warning text is relayed at most once per run: + // memoryWarning recurs on every response. + const relayedWarnings = new Set(); + + while (true) { + const next = await getFlowNext(taskDir); + if (!started) { + started = true; + // Emitted before the event derived from this same response. + report(json, { + event: 'run_started', + taskDir: next.taskDir, + profile: next.profile ?? null, + }); + } + for (const warning of [next.memoryWarning, next.workRootWarning]) { + if (!warning || relayedWarnings.has(warning)) continue; + relayedWarnings.add(warning); + report(json, { event: 'warning', message: warning }, `Warning: ${warning}`); + } + if (next.state === 'blocked') return reportBlocked(json, next.taskDir, next.reason); + if (next.state === 'complete') { + report( + json, + { event: 'complete', commit: findCommitSha(next.steps) }, + `Flow complete: ${next.taskDir}` + ); + return 0; + } + + const step = next.nextStep; + if (!step?.prompt) { + return reportStepFailed( + json, + null, + 'No dispatchable step: the flow is ready but carries no prompt.' + ); + } + + report( + json, + { + event: 'step_started', + step: step.id, + runner: step.runner, + model: step.model, + effort: step.effort, + attempt: step.attempt, + }, + describeStep(step, next.steps) + ); + + const runner = runners(step.runner); + if (!runner) return reportStepFailed(json, step.id, `no runner available for ${step.runner}.`); + + const result = await runner.run({ + model: step.model, + effort: step.effort, + prompt: step.prompt, + projectRoot, + }); + if (!result.ok) return reportStepFailed(json, step.id, result.reason); + + const input = buildCompleteInput(step, result.finalMessage); + if (typeof input === 'string') return reportStepFailed(json, step.id, input); + + const completed = await completeFlowStep(taskDir, input); + if (completed.state === 'blocked') { + // A contract break by the harness we just ran resumes on re-invocation; + // everything else is a genuine workflow blocker. + const reason = completed.reason ?? 'Unknown workflow blocker'; + if (STEP_CONTRACT_FAILURE_CODES.has(flowBlockCode(reason))) { + return reportStepFailed(json, step.id, reason); + } + return reportBlocked(json, completed.taskDir, completed.reason); + } + report(json, { event: 'step_completed', step: step.id }, `${step.id} completed`); + } + } finally { + stopForwardingSignals(); + restoreFlowProfile(); + } +} diff --git a/test/commands/workflow/harness-runners.test.ts b/test/commands/workflow/harness-runners.test.ts new file mode 100644 index 0000000..af04ba7 --- /dev/null +++ b/test/commands/workflow/harness-runners.test.ts @@ -0,0 +1,189 @@ +import { execFileSync } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; +import { promises as fs, realpathSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { + buildClaudeArgv, + buildCodexArgv, + claudeRunner, + codexRunner, + resolveProjectRoot, +} from '../../../src/commands/workflow/harness-runners.js'; +import type { HarnessRunRequest } from '../../../src/commands/workflow/harness-runners.js'; + +const PROMPT = 'Call the `spok-create-plan` skill with `/tmp/task` as the argument.'; + +// The stubs are POSIX shell scripts; CI runs the process-boundary cases on POSIX. +const posixOnly = process.platform === 'win32' ? it.skip : it; + +function request(overrides: Partial = {}): HarnessRunRequest { + return { model: 'opus', prompt: PROMPT, projectRoot: os.tmpdir(), ...overrides }; +} + +describe('harness argv construction', () => { + it('builds the claude recipe without effort', () => { + expect(buildClaudeArgv(request())).toEqual([ + '-p', + '--no-session-persistence', + '--permission-mode', + 'auto', + '--model', + 'opus', + ]); + }); + + it('appends the effort flag when the step carries one', () => { + expect(buildClaudeArgv(request({ effort: 'xhigh' }))).toEqual([ + '-p', + '--no-session-persistence', + '--permission-mode', + 'auto', + '--model', + 'opus', + '--effort', + 'xhigh', + ]); + }); + + it('builds the codex recipe with the project root and output file', () => { + expect( + buildCodexArgv( + request({ model: 'gpt-5.6-sol', projectRoot: '/repo' }), + '/tmp/out/last-message.txt' + ) + ).toEqual([ + 'exec', + '--ephemeral', + '--dangerously-bypass-hook-trust', + '--cd', + '/repo', + '--sandbox', + 'workspace-write', + '--model', + 'gpt-5.6-sol', + '-o', + '/tmp/out/last-message.txt', + '-', + ]); + }); + + it('passes codex reasoning effort as a config override only when present', () => { + expect(buildCodexArgv(request({ model: 'gpt-5.6-sol', effort: 'max' }), '/tmp/out')).toContain( + 'model_reasoning_effort="max"' + ); + }); + + it('never places the prompt in the argument list', () => { + expect(buildClaudeArgv(request({ effort: 'high' }))).not.toContain(PROMPT); + expect(buildCodexArgv(request({ effort: 'high' }), '/tmp/out')).not.toContain(PROMPT); + }); +}); + +describe('project root resolution', () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = path.join(os.tmpdir(), `spok-run-root-${randomUUID()}`); + await fs.mkdir(tempDir, { recursive: true }); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + // git prints a forward-slash (and possibly short-name) toplevel on Windows, + // which no path normalization reliably reconciles with fs.realpath there. + posixOnly('returns the git toplevel for a task directory inside a repository', async () => { + execFileSync('git', ['init', '-b', 'main', tempDir], { encoding: 'utf-8' }); + const taskDir = path.join(tempDir, 'spok', 'changes', 'demo'); + await fs.mkdir(taskDir, { recursive: true }); + + expect(realpathSync.native(await resolveProjectRoot(taskDir))).toBe( + realpathSync.native(tempDir) + ); + }); + + posixOnly('falls back to the resolved task directory outside a repository', async () => { + expect(await resolveProjectRoot(tempDir)).toBe(path.resolve(tempDir)); + }); +}); + +describe('harness subprocess failure mapping', () => { + let binDir: string; + let originalPath: string | undefined; + + async function writeStub(name: string, script: string): Promise { + const stubPath = path.join(binDir, name); + await fs.writeFile(stubPath, script, 'utf-8'); + await fs.chmod(stubPath, 0o755); + } + + beforeEach(async () => { + binDir = path.join(os.tmpdir(), `spok-run-bin-${randomUUID()}`); + await fs.mkdir(binDir, { recursive: true }); + originalPath = process.env.PATH; + // Stubs shadow the real harnesses while still reaching system utilities. + process.env.PATH = `${binDir}${path.delimiter}${originalPath ?? ''}`; + }); + + afterEach(async () => { + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + await fs.rm(binDir, { recursive: true, force: true }); + }); + + posixOnly('reports a missing executable instead of throwing', async () => { + process.env.PATH = binDir; // Nothing on PATH but the empty stub directory. + + expect(await claudeRunner.run(request())).toEqual({ + ok: false, + reason: 'claude executable not found on PATH', + }); + }); + + posixOnly('reports the exit code of a failing harness', async () => { + await writeStub('claude', '#!/bin/sh\ncat > /dev/null\nexit 7\n'); + + expect(await claudeRunner.run(request())).toEqual({ + ok: false, + reason: 'claude exited with code 7', + }); + }); + + posixOnly('returns the claude stdout as the final message', async () => { + await writeStub('claude', '#!/bin/sh\ncat > /dev/null\nprintf "done\\nWork root: /repo\\n"\n'); + + expect(await claudeRunner.run(request())).toEqual({ + ok: true, + finalMessage: 'done\nWork root: /repo\n', + }); + }); + + posixOnly('reads the codex final message from the file it was told to write', async () => { + await writeStub( + 'codex', + '#!/bin/sh\nout=""\nwhile [ $# -gt 0 ]; do\n if [ "$1" = "-o" ]; then out="$2"; fi\n shift\ndone\ncat > /dev/null\nprintf "codex reply" > "$out"\n' + ); + + expect(await codexRunner.run(request({ model: 'gpt-5.6-sol' }))).toEqual({ + ok: true, + finalMessage: 'codex reply', + }); + }); + + posixOnly('reports a codex run that wrote no final message', async () => { + await writeStub('codex', '#!/bin/sh\ncat > /dev/null\nexit 0\n'); + + const result = await codexRunner.run(request({ model: 'gpt-5.6-sol' })); + + expect(result.ok).toBe(false); + expect(result.ok === false && result.reason).toMatch(/did not write its final message/); + }); +}); diff --git a/test/commands/workflow/run.test.ts b/test/commands/workflow/run.test.ts new file mode 100644 index 0000000..c00a2f1 --- /dev/null +++ b/test/commands/workflow/run.test.ts @@ -0,0 +1,623 @@ +import { execFileSync } from 'node:child_process'; +import { randomUUID } from 'node:crypto'; +import { promises as fs, realpathSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { getFlowNext, WORKFLOW_STATE_FILE } from '../../../src/commands/workflow/flow.js'; +import { + parseCommitCompletion, + parseSummaryCompletion, + runCommand, +} from '../../../src/commands/workflow/run.js'; +import type { + HarnessRunRequest, + HarnessRunResult, + RunnerLookup, +} from '../../../src/commands/workflow/harness-runners.js'; + +const PASS_DESIGN_REVIEW = '---\ntype: design-review\nverdict: PASS\n---\n\n# Design Review\n'; +const FAIL_DESIGN_REVIEW = + '---\ntype: design-review\nverdict: FAIL\n---\n\n# Design Review\n\n- revise the design\n'; +const FAIL_DESIGN_REVIEW_WITH_DECISIONS = + '---\ntype: design-review\nverdict: FAIL\n---\n\n# Design Review\n\n' + + '## Human Decisions Required\n\n- Choose between approach A and approach B.\n'; +const PASS_VALIDATION = '---\nverdict: PASS\n---\n\n# Validation\n'; +const FAIL_VALIDATION = + '---\nverdict: FAIL\n---\n\n# Validation\n\n## Blocking Findings\n\n- something broke\n'; + +const BASE_SKILL_SEQUENCE = [ + 'spok-validate-problem', + 'spok-create-research-questions', + 'spok-create-research', + 'spok-create-design-discussion', + 'spok-create-structure-outline', + 'spok-review-design', + 'spok-create-plan', + 'spok-implement-plan', + 'spok-simplify', + 'spok-validate-implementation', + 'spok-ci-commit', +]; + +const HYBRID_RUNNER_SEQUENCE = [ + 'codex', + 'codex', + 'codex', + 'claude', + 'claude', + 'codex', + 'claude', + 'codex', + 'claude', + 'claude', + 'codex', +]; + +const FILE_OUTPUT_BY_SKILL: Record = { + 'spok-validate-problem': 'problem-validation.md', + 'spok-create-research-questions': 'research-questions.md', + 'spok-create-research': 'research.md', + 'spok-create-design-discussion': 'design-discussion.md', + 'spok-create-structure-outline': 'structure-outline.md', + 'spok-review-design': 'design-review.md', + 'spok-create-plan': 'plan.md', + 'spok-validate-implementation': 'validation.md', +}; + +interface Dispatch { + skill: string; + runner: 'claude' | 'codex'; + request: HarnessRunRequest; +} + +/** Skill name from the prompt's `Call the \`\` skill ...` sentence. */ +function skillFromPrompt(prompt: string): string { + const match = prompt.match(/Call the `([a-z-]+)` skill/); + if (!match) throw new Error(`No skill found in prompt: ${prompt.slice(0, 120)}`); + return match[1]!; +} + +function defaultArtifact(skill: string): string { + if (skill === 'spok-validate-problem') { + return '# Problem Validation\n\n## Flow Decision\n\nproceed\n'; + } + if (skill === 'spok-review-design') return PASS_DESIGN_REVIEW; + if (skill === 'spok-validate-implementation') return PASS_VALIDATION; + return `# ${skill}\n`; +} + +interface FakeHarnessOptions { + taskDir: string; + workRoot: string; + /** Per-skill overrides: return the final message, or a failure. */ + overrides?: Record Promise>; + /** Artifact content overrides keyed by skill (e.g. a FAIL design review). */ + artifactBySkill?: Record; +} + +/** + * A fake harness that performs each step's side effects the way a real + * subagent would: writes the expected artifact for file steps, replies with + * summary/Work root/Commit lines for the others. Records every dispatch. + */ +function createFakeHarness(options: FakeHarnessOptions) { + const dispatches: Dispatch[] = []; + const artifactUse = new Map(); + + const run = + (runner: 'claude' | 'codex') => + async (request: HarnessRunRequest): Promise => { + const skill = skillFromPrompt(request.prompt); + dispatches.push({ skill, runner, request }); + + const override = options.overrides?.[skill]; + if (override) return override(request); + + const fileName = FILE_OUTPUT_BY_SKILL[skill]; + if (fileName) { + const configured = options.artifactBySkill?.[skill]; + const useIndex = artifactUse.get(skill) ?? 0; + artifactUse.set(skill, useIndex + 1); + const content = Array.isArray(configured) + ? configured[Math.min(useIndex, configured.length - 1)]! + : (configured ?? defaultArtifact(skill)); + await fs.writeFile(path.join(options.taskDir, fileName), content, 'utf-8'); + return { ok: true, finalMessage: path.join(options.taskDir, fileName) }; + } + + if (skill === 'spok-implement-plan') { + return { ok: true, finalMessage: `Implemented the plan.\nWork root: ${options.workRoot}` }; + } + if (skill === 'spok-simplify') { + return { ok: true, finalMessage: 'Simplified the implementation.' }; + } + if (skill === 'spok-repair') { + return { ok: true, finalMessage: 'Fixed the blocking findings.' }; + } + if (skill === 'spok-ci-commit') { + const sha = execFileSync('git', ['-C', options.workRoot, 'rev-parse', 'HEAD'], { + encoding: 'utf-8', + }).trim(); + return { ok: true, finalMessage: `Committed the chunk.\nCommit: ${sha}` }; + } + throw new Error(`Fake harness has no behavior for skill ${skill}`); + }; + + const lookup: RunnerLookup = (runner) => ({ run: run(runner) }); + return { lookup, dispatches }; +} + +/** Captures console.log lines; JSONL assertions parse them per line. */ +function captureStdout(): { lines: string[]; restore: () => void } { + const lines: string[] = []; + const spy = vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + lines.push(args.map(String).join(' ')); + }); + return { lines, restore: () => spy.mockRestore() }; +} + +function parseEvents(lines: string[]): Array> { + return lines.map((line) => JSON.parse(line) as Record); +} + +describe('spok run loop', () => { + let tempDir: string; + let taskDir: string; + let workRoot: string; + let headSha: string; + let originalCodexHome: string | undefined; + let originalFlowProfile: string | undefined; + + function git(args: string[]): string { + return execFileSync('git', ['-C', workRoot, ...args], { encoding: 'utf-8' }).trim(); + } + + function fakeHarness(options: Omit = {}) { + return createFakeHarness({ taskDir, workRoot, ...options }); + } + + async function readState(): Promise<{ + steps: Array<{ id: string; status: string; result?: Record }>; + }> { + return JSON.parse(await fs.readFile(path.join(taskDir, WORKFLOW_STATE_FILE), 'utf-8')); + } + + const MEMORY_WITH_MALFORMED_BULLET = + '# Memory\n\n- `good` — A conforming rule.\n- `broken` missing the dash entirely.\n'; + + /** Enables memory reading: the project root is found via spok/config.yaml. */ + async function writeMemoryFixture(text: string): Promise { + const configDir = path.join(tempDir, 'spok'); + await fs.mkdir(configDir, { recursive: true }); + await fs.writeFile(path.join(configDir, 'config.yaml'), 'schema: spec-driven\n', 'utf-8'); + await fs.writeFile(path.join(configDir, 'MEMORY.md'), text, 'utf-8'); + } + + beforeEach(async () => { + originalCodexHome = process.env.CODEX_HOME; + originalFlowProfile = process.env.SPOK_FLOW_PROFILE; + delete process.env.CODEX_HOME; + delete process.env.SPOK_FLOW_PROFILE; + + tempDir = path.join(os.tmpdir(), `spok-run-${randomUUID()}`); + taskDir = path.join(tempDir, 'spok', 'changes', 'demo', '.flow', 'chunk-one'); + await fs.mkdir(taskDir, { recursive: true }); + await fs.writeFile(path.join(taskDir, 'ticket.md'), '# Chunk One\n', 'utf-8'); + + workRoot = path.join(tempDir, 'work-repo'); + await fs.mkdir(workRoot, { recursive: true }); + execFileSync('git', ['init', '-b', 'main', workRoot], { encoding: 'utf-8' }); + git(['config', 'user.email', 'flow@example.com']); + git(['config', 'user.name', 'Flow Test']); + await fs.writeFile(path.join(workRoot, 'work.txt'), 'one\n', 'utf-8'); + git(['add', 'work.txt']); + git(['commit', '--no-gpg-sign', '-m', 'first']); + headSha = git(['rev-parse', 'HEAD']); + }); + + afterEach(async () => { + if (originalCodexHome === undefined) { + delete process.env.CODEX_HOME; + } else { + process.env.CODEX_HOME = originalCodexHome; + } + if (originalFlowProfile === undefined) { + delete process.env.SPOK_FLOW_PROFILE; + } else { + process.env.SPOK_FLOW_PROFILE = originalFlowProfile; + } + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('drives every step in order and exits 0', async () => { + const fake = fakeHarness(); + + const code = await runCommand(taskDir, {}, fake.lookup); + + expect(code).toBe(0); + expect(fake.dispatches.map((dispatch) => dispatch.skill)).toEqual(BASE_SKILL_SEQUENCE); + + const state = await readState(); + expect(state.steps.every((step) => step.status === 'completed')).toBe(true); + const recordedWorkRoot = state.steps.find((step) => step.id === 'implement')?.result?.workRoot; + expect(recordedWorkRoot && realpathSync.native(recordedWorkRoot)).toBe( + realpathSync.native(workRoot) + ); + expect(state.steps.find((step) => step.id === 'commit')?.result?.commit).toBe(headSha); + }); + + it('dispatches each step to the runner its profile routing selects', async () => { + const fake = fakeHarness(); + + const code = await runCommand(taskDir, { profile: 'hybrid' }, fake.lookup); + + expect(code).toBe(0); + expect(fake.dispatches.map((dispatch) => dispatch.runner)).toEqual(HYBRID_RUNNER_SEQUENCE); + }); + + it('asks the commit step for a parseable commit line', async () => { + const fake = fakeHarness(); + + await runCommand(taskDir, {}, fake.lookup); + + const commit = fake.dispatches.find((dispatch) => dispatch.skill === 'spok-ci-commit'); + expect(commit?.request.prompt).toContain('`Commit: `'); + }); + + it('restores the flow profile environment variable after a run', async () => { + const fake = fakeHarness(); + + await runCommand(taskDir, { profile: 'hybrid' }, fake.lookup); + + expect(process.env.SPOK_FLOW_PROFILE).toBeUndefined(); + }); + + it('exits 2 without dispatching when the state machine blocks the first query', async () => { + await getFlowNext(taskDir); // Persists state under the detected claude profile. + const fake = fakeHarness(); + + const code = await runCommand(taskDir, { profile: 'codex' }, fake.lookup); + + expect(code).toBe(2); + expect(fake.dispatches).toEqual([]); + }); + + it('exits 2 and stops when recording a completion blocks the flow', async () => { + const fake = fakeHarness({ + artifactBySkill: { 'spok-review-design': FAIL_DESIGN_REVIEW }, + }); + + const code = await runCommand(taskDir, {}, fake.lookup); + + expect(code).toBe(2); + expect(fake.dispatches.filter((dispatch) => dispatch.skill === 'spok-review-design')).toHaveLength(1); + expect(fake.dispatches.at(-1)?.skill).toBe('spok-review-design'); + }); + + it('exits 3 and resumes when a file step exits 0 without writing its artifact', async () => { + const fake = fakeHarness({ + overrides: { + 'spok-create-research': async () => ({ ok: true, finalMessage: 'claimed done' }), + }, + }); + + expect(await runCommand(taskDir, {}, fake.lookup)).toBe(3); + expect((await getFlowNext(taskDir)).nextStep?.id).toBe('research'); + + const resumed = fakeHarness(); + expect(await runCommand(taskDir, {}, resumed.lookup)).toBe(0); + expect(resumed.dispatches[0]?.skill).toBe('spok-create-research'); + }); + + it('exits 3 on a harness failure and resumes at the same step', async () => { + const failing = fakeHarness({ + overrides: { + 'spok-create-research': async () => ({ ok: false, reason: 'boom' }), + }, + }); + + expect(await runCommand(taskDir, {}, failing.lookup)).toBe(3); + expect((await getFlowNext(taskDir)).nextStep?.id).toBe('research'); + + const resumed = fakeHarness(); + expect(await runCommand(taskDir, {}, resumed.lookup)).toBe(0); + expect(resumed.dispatches[0]?.skill).toBe('spok-create-research'); + }); + + it('exits 3 when a summary step returns no summary text', async () => { + const fake = fakeHarness({ + overrides: { + 'spok-implement-plan': async () => ({ ok: true, finalMessage: ' ' }), + }, + }); + + expect(await runCommand(taskDir, {}, fake.lookup)).toBe(3); + expect((await getFlowNext(taskDir)).nextStep?.id).toBe('implement'); + }); + + it('exits 3 when the commit step returns no commit line', async () => { + const fake = fakeHarness({ + overrides: { + 'spok-ci-commit': async () => ({ ok: true, finalMessage: 'Committed the chunk.' }), + }, + }); + + expect(await runCommand(taskDir, {}, fake.lookup)).toBe(3); + expect((await getFlowNext(taskDir)).nextStep?.id).toBe('commit'); + }); + + it('emits a parseable JSONL event stream for a full --json run', async () => { + const fake = fakeHarness(); + const capture = captureStdout(); + try { + const code = await runCommand(taskDir, { profile: 'hybrid', json: true }, fake.lookup); + expect(code).toBe(0); + + const events = parseEvents(capture.lines); // Throws if any line is not JSON. + expect(events.every((event) => event.schemaVersion === 1)).toBe(true); + expect(events[0]).toMatchObject({ event: 'run_started', taskDir, profile: 'hybrid' }); + const names = events.map((event) => event.event); + expect(names.filter((name) => name === 'step_started')).toHaveLength(11); + expect(names.filter((name) => name === 'step_completed')).toHaveLength(11); + expect(events.at(-1)).toMatchObject({ event: 'complete', commit: headSha }); + } finally { + capture.restore(); + } + }); + + it('emits step_failed with the failing step and reason on a harness failure', async () => { + const fake = fakeHarness({ + overrides: { 'spok-create-research': async () => ({ ok: false, reason: 'boom' }) }, + }); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(3); + expect(parseEvents(capture.lines).at(-1)).toMatchObject({ + event: 'step_failed', + step: 'research', + reason: 'Step research failed: boom', + }); + } finally { + capture.restore(); + } + }); + + it('emits step_failed on a completion-contract violation', async () => { + const fake = fakeHarness({ + overrides: { + 'spok-ci-commit': async () => ({ ok: true, finalMessage: 'Committed the chunk.' }), + }, + }); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(3); + const last = parseEvents(capture.lines).at(-1); + expect(last).toMatchObject({ event: 'step_failed', step: 'commit' }); + expect(last?.reason).toContain('Commit: '); + } finally { + capture.restore(); + } + }); + + it('emits run_started then blocked with a code when the first query blocks', async () => { + await getFlowNext(taskDir); // Persists state under the detected claude profile. + const fake = fakeHarness(); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { profile: 'codex', json: true }, fake.lookup)).toBe(2); + const events = parseEvents(capture.lines); + expect(events[0]?.event).toBe('run_started'); + expect(events[1]).toMatchObject({ event: 'blocked', code: 'flow_profile_mismatch' }); + } finally { + capture.restore(); + } + }); + + it('keeps the human-readable output free of JSON without --json', async () => { + const fake = fakeHarness(); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, {}, fake.lookup)).toBe(0); + expect(capture.lines[0]).toMatch(/^\[1\/11\] validate-problem /); + expect(capture.lines.at(-1)).toBe(`Flow complete: ${taskDir}`); + expect(capture.lines.some((line) => line.startsWith('{'))).toBe(false); + } finally { + capture.restore(); + } + }); + + it('attaches humanDecisions to a design-review FAIL blocked event', async () => { + const fake = fakeHarness({ + artifactBySkill: { 'spok-review-design': FAIL_DESIGN_REVIEW_WITH_DECISIONS }, + }); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(2); + expect(parseEvents(capture.lines).at(-1)).toMatchObject({ + event: 'blocked', + code: 'design_review_verdict_fail', + humanDecisions: '- Choose between approach A and approach B.', + }); + } finally { + capture.restore(); + } + }); + + it('omits humanDecisions when the design review has no such section', async () => { + const fake = fakeHarness({ + artifactBySkill: { 'spok-review-design': FAIL_DESIGN_REVIEW }, + }); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(2); + const last = parseEvents(capture.lines).at(-1); + expect(last).toMatchObject({ event: 'blocked', code: 'design_review_verdict_fail' }); + expect(last).not.toHaveProperty('humanDecisions'); + } finally { + capture.restore(); + } + }); + + it('omits humanDecisions for non-design-review block codes', async () => { + await fs.writeFile( + path.join(taskDir, 'design-review.md'), + FAIL_DESIGN_REVIEW_WITH_DECISIONS, + 'utf-8' + ); + await getFlowNext(taskDir); // Persists claude-profile state; next run mismatches. + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { profile: 'codex', json: true }, fakeHarness().lookup)).toBe( + 2 + ); + const last = parseEvents(capture.lines).at(-1); + expect(last).toMatchObject({ event: 'blocked', code: 'flow_profile_mismatch' }); + expect(last).not.toHaveProperty('humanDecisions'); + } finally { + capture.restore(); + } + }); + + describe('warning relay', () => { + it('emits one warning JSONL event per distinct warning text across the run', async () => { + await writeMemoryFixture(MEMORY_WITH_MALFORMED_BULLET); + const fake = fakeHarness(); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(0); + const events = parseEvents(capture.lines); + expect(events[0]).toMatchObject({ event: 'run_started' }); + const warnings = events.filter((event) => event.event === 'warning'); + expect(warnings).toHaveLength(1); // Deduped across every loop iteration. + expect(warnings[0]).toMatchObject({ schemaVersion: 1, event: 'warning' }); + expect(String(warnings[0]?.message)).toContain( + '1 bullet(s) dropped for not matching the rule grammar' + ); + } finally { + capture.restore(); + } + }); + + it('prints one Warning line in human mode', async () => { + await writeMemoryFixture(MEMORY_WITH_MALFORMED_BULLET); + const fake = fakeHarness(); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, {}, fake.lookup)).toBe(0); + const warningLines = capture.lines.filter((line) => line.startsWith('Warning: ')); + expect(warningLines).toHaveLength(1); + expect(warningLines[0]).toContain('bullet(s) dropped'); + expect(capture.lines.some((line) => line.startsWith('{'))).toBe(false); + } finally { + capture.restore(); + } + }); + + it('relays the work-root warning when no work root reaches the commit step', async () => { + const fake = fakeHarness({ + overrides: { + // No trailing `Work root:` line: the commit step arrives unsteered. + 'spok-implement-plan': async () => ({ ok: true, finalMessage: 'Implemented the plan.' }), + }, + }); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(0); + const warnings = parseEvents(capture.lines).filter((event) => event.event === 'warning'); + expect(warnings).toHaveLength(1); + expect(String(warnings[0]?.message)).toContain('No work root was recorded'); + } finally { + capture.restore(); + } + }); + + it('emits no warning events when responses carry no warnings', async () => { + const fake = fakeHarness(); + const capture = captureStdout(); + try { + expect(await runCommand(taskDir, { json: true }, fake.lookup)).toBe(0); + expect(parseEvents(capture.lines).some((event) => event.event === 'warning')).toBe(false); + } finally { + capture.restore(); + } + }); + }); + + it('continues into the repair cycle when validation records a FAIL with budget left', async () => { + const fake = fakeHarness({ + artifactBySkill: { + 'spok-validate-implementation': [FAIL_VALIDATION, PASS_VALIDATION], + }, + }); + + const code = await runCommand(taskDir, {}, fake.lookup); + + expect(code).toBe(0); + expect(fake.dispatches.map((dispatch) => dispatch.skill)).toEqual([ + ...BASE_SKILL_SEQUENCE.slice(0, 10), + 'spok-repair', + 'spok-validate-implementation', + 'spok-ci-commit', + ]); + }); +}); + +describe('harness completion contract parsing', () => { + it('records a plain reply as the summary', () => { + expect(parseSummaryCompletion('Did the work.')).toEqual({ summary: 'Did the work.' }); + }); + + it('splits a trailing work root line from the summary', () => { + expect(parseSummaryCompletion('Did the work.\n\nWork root: /tmp/repo')).toEqual({ + summary: 'Did the work.', + workRoot: '/tmp/repo', + }); + }); + + it('rejects a reply that is only a work root line', () => { + expect(parseSummaryCompletion('Work root: /tmp/repo')).toBeUndefined(); + }); + + it('rejects an empty reply', () => { + expect(parseSummaryCompletion(' \n ')).toBeUndefined(); + }); + + it('caps an oversized summary at 4000 characters', () => { + expect(parseSummaryCompletion('x'.repeat(5000))?.summary).toHaveLength(4000); + }); + + it('reads a trailing commit line with the body as its summary', () => { + expect(parseCommitCompletion('Committed the chunk.\nCommit: abc123')).toEqual({ + commit: 'abc123', + summary: 'Committed the chunk.', + }); + }); + + it('reads a bare commit line without a summary', () => { + expect(parseCommitCompletion('Commit: abc123')).toEqual({ + commit: 'abc123', + summary: undefined, + }); + }); + + it('rejects a reply with no commit line', () => { + expect(parseCommitCompletion('Committed the chunk.')).toBeUndefined(); + }); + + it('tolerates markdown decoration around the commit token', () => { + expect(parseCommitCompletion('Done.\n**Commit:** `abc123`.')).toEqual({ + commit: 'abc123', + summary: 'Done.', + }); + }); + + it('tolerates backticks around the work root path', () => { + expect(parseSummaryCompletion('Did the work.\nWork root: `/repo/path`')).toEqual({ + summary: 'Did the work.', + workRoot: '/repo/path', + }); + }); +}); diff --git a/test/docs/workflow-surface.test.ts b/test/docs/workflow-surface.test.ts index 2a8e2e6..20e6667 100644 --- a/test/docs/workflow-surface.test.ts +++ b/test/docs/workflow-surface.test.ts @@ -25,6 +25,17 @@ describe('workflow surface docs', () => { expect(commands).toContain('self-discovery escape hatch'); }); + it('documents spok run with its JSONL protocol and exit codes', async () => { + const cli = await readDoc('docs/cli.md'); + + expect(cli).toContain('### `spok run`'); + expect(cli).toContain('spok run [options]'); + expect(cli).toContain('| `--json` | Emit JSONL supervision events on stdout |'); + expect(cli).toContain('`schemaVersion: 1`'); + expect(cli).toContain('| `2` | Blocked (the state machine reported a blocker) |'); + expect(cli).toContain('| `130` | Interrupted (SIGINT) |'); + }); + it('documents MEMORY.md as presence-based, capped, and human-promoted', async () => { const workflows = await readDoc('docs/workflows.md'); const commands = await readDoc('docs/commands.md'); diff --git a/test/features/spok-run.feature b/test/features/spok-run.feature new file mode 100644 index 0000000..026302c --- /dev/null +++ b/test/features/spok-run.feature @@ -0,0 +1,53 @@ +Feature: spok run drives a flow to a terminal outcome + + Background: + Given a new project + And a separate flow work repository + And a staged flow task + And a stub harness on PATH + + Scenario: A full run completes with exit code 0 + When I run spok run on the staged flow task + Then the spok run exits with code 0 + And the spok run reports each dispatched step in order + And the workflow state records the commit step as completed + + Scenario: A design-review FAIL blocks the run with exit code 2 + Given the stub harness records a FAIL design review + When I run spok run on the staged flow task + Then the spok run exits with code 2 + And the spok run reports a blocked reason containing "recorded a FAIL verdict" + + Scenario: A failing harness exits 3 and the run resumes at the same step + Given the stub harness fails on the research step + When I run spok run on the staged flow task + Then the spok run exits with code 3 + Given the stub harness is repaired + When I run spok run on the staged flow task + Then the spok run exits with code 0 + And the spok run resumed at the research step + + Scenario: A profile mismatch with persisted state exits 2 + Given the staged flow task has persisted workflow state under the claude profile + When I run spok run on the staged flow task with profile "codex" + Then the spok run exits with code 2 + And the spok run reports a blocked reason containing "Flow profile mismatch" + + Scenario: An unrecognized profile value is a usage error + When I run spok run on the staged flow task with profile "turbo" + Then the spok run exits with code 1 + And the staged flow task has no workflow state file + + Scenario: A full run with --json emits a parseable JSONL event stream + When I run spok run on the staged flow task with the --json flag + Then the spok run exits with code 0 + And every spok run stdout line is JSON with schema version 1 + And the spok run event stream reports the full flow lifecycle + + Scenario: A design-review FAIL surfaces its human decisions in the event stream + Given the stub harness records a FAIL design review + When I run spok run on the staged flow task with the --json flag + Then the spok run exits with code 2 + And every spok run stdout line is JSON with schema version 1 + And the spok run event stream ends with a blocked event carrying code "design_review_verdict_fail" + And the blocked event carries the human decisions from the design review diff --git a/test/features/step-definitions/spok-run.steps.ts b/test/features/step-definitions/spok-run.steps.ts new file mode 100644 index 0000000..1fade10 --- /dev/null +++ b/test/features/step-definitions/spok-run.steps.ts @@ -0,0 +1,282 @@ +import { Given, Then, When } from '@cucumber/cucumber'; +import assert from 'node:assert/strict'; +import { promises as fs } from 'node:fs'; +import path from 'node:path'; +import { runCLI, type RunCLIResult } from '../../helpers/run-cli.js'; + +interface SpokRunWorld { + projectDir?: string; + flowTaskDir?: string; + flowWorkRoot?: string; + flowHeadCommit?: string; + runBinDir?: string; + runDesignReview?: string; + runFailMarker?: string; + cliResult?: RunCLIResult; +} + +const EXPECTED_STEP_ORDER = [ + 'validate-problem', + 'research-questions', + 'research', + 'design-discussion', + 'structure-outline', + 'design-review', + 'plan', + 'implement', + 'simplify', + 'validate', + 'commit', +]; + +/** + * Stands in for a real harness: reads the composed prompt on stdin, writes the + * artifact the step's gate requires, and replies with the completion contract. + * The research-questions arm must precede the research arm — the shorter skill + * name is a substring of the longer one. + */ +const STUB_HARNESS = String.raw`#!/bin/sh +prompt=$(cat) + +if [ -n "$STUB_FAIL_MARKER" ]; then + case "$prompt" in + *"$STUB_FAIL_MARKER"*) + echo "stub harness failure" >&2 + exit 1 + ;; + esac +fi + +case "$prompt" in + *spok-validate-problem*) + printf '# Problem Validation\n\n## Flow Decision\n\nproceed\n' > "$STUB_TASK_DIR/problem-validation.md" + echo "$STUB_TASK_DIR/problem-validation.md" + ;; + *spok-create-research-questions*) + printf '# Research Questions\n' > "$STUB_TASK_DIR/research-questions.md" + echo "$STUB_TASK_DIR/research-questions.md" + ;; + *spok-create-research*) + printf '# Research\n' > "$STUB_TASK_DIR/research.md" + echo "$STUB_TASK_DIR/research.md" + ;; + *spok-create-design-discussion*) + printf '# Design Discussion\n' > "$STUB_TASK_DIR/design-discussion.md" + echo "$STUB_TASK_DIR/design-discussion.md" + ;; + *spok-create-structure-outline*) + printf '# Structure Outline\n' > "$STUB_TASK_DIR/structure-outline.md" + echo "$STUB_TASK_DIR/structure-outline.md" + ;; + *spok-review-design*) + if [ "$STUB_DESIGN_REVIEW" = "FAIL" ]; then + printf -- '---\ntype: design-review\nverdict: FAIL\n---\n\n# Design Review\n\n## Human Decisions Required\n\n- Pick a direction for the API.\n' > "$STUB_TASK_DIR/design-review.md" + else + printf -- '---\ntype: design-review\nverdict: PASS\n---\n\n# Design Review\n' > "$STUB_TASK_DIR/design-review.md" + fi + echo "$STUB_TASK_DIR/design-review.md" + ;; + *spok-create-plan*) + printf '# Plan\n' > "$STUB_TASK_DIR/plan.md" + echo "$STUB_TASK_DIR/plan.md" + ;; + *spok-implement-plan*) + echo "Implemented the plan." + echo "Work root: $STUB_WORK_ROOT" + ;; + *spok-simplify*) + echo "Simplified the implementation." + ;; + *spok-validate-implementation*) + printf -- '---\nverdict: PASS\n---\n\n# Validation\n' > "$STUB_TASK_DIR/validation.md" + echo "$STUB_TASK_DIR/validation.md" + ;; + *spok-ci-commit*) + echo "Committed the chunk." + echo "Commit: $(git -C "$STUB_WORK_ROOT" rev-parse HEAD)" + ;; + *) + echo "stub harness cannot handle this prompt" >&2 + exit 1 + ;; +esac +`; + +function stubEnv(world: SpokRunWorld): NodeJS.ProcessEnv { + return { + PATH: `${world.runBinDir}${path.delimiter}${process.env.PATH ?? ''}`, + SPOK_FLOW_PROFILE: 'claude', + STUB_TASK_DIR: world.flowTaskDir, + STUB_WORK_ROOT: world.flowWorkRoot, + STUB_DESIGN_REVIEW: world.runDesignReview ?? 'PASS', + STUB_FAIL_MARKER: world.runFailMarker ?? '', + }; +} + +async function invokeRun(world: SpokRunWorld, args: string[]): Promise { + assert.ok(world.projectDir, 'projectDir must be set by Given a new project'); + assert.ok(world.flowTaskDir, 'flowTaskDir must be set by Given a staged flow task'); + world.cliResult = await runCLI(['run', world.flowTaskDir, ...args], { + cwd: world.projectDir, + env: stubEnv(world), + timeoutMs: 60_000, + }); +} + +/** Step ids in the order the run reported dispatching them. */ +function dispatchedSteps(world: SpokRunWorld): string[] { + assert.ok(world.cliResult, 'cliResult must be set by a spok run step'); + return world.cliResult.stdout + .split('\n') + .map((line) => line.match(/^\[\d+\/\d+\] (\S+) /)) + .filter((match): match is RegExpMatchArray => match !== null) + .map((match) => match[1]!); +} + +/** Parsed JSONL events from the run's stdout; throws on a non-JSON line. */ +function stdoutEvents(world: SpokRunWorld): Array> { + assert.ok(world.cliResult, 'cliResult must be set by a spok run step'); + return world.cliResult.stdout + .split('\n') + .filter((line) => line.trim() !== '') + .map((line) => JSON.parse(line) as Record); +} + +Given('a stub harness on PATH', async function (this: SpokRunWorld) { + assert.ok(this.projectDir, 'projectDir must be set by Given a new project'); + this.runBinDir = path.join(this.projectDir, 'stub-bin'); + await fs.mkdir(this.runBinDir, { recursive: true }); + const stubPath = path.join(this.runBinDir, 'claude'); + await fs.writeFile(stubPath, STUB_HARNESS, 'utf-8'); + await fs.chmod(stubPath, 0o755); +}); + +Given('the stub harness records a FAIL design review', function (this: SpokRunWorld) { + this.runDesignReview = 'FAIL'; +}); + +Given('the stub harness fails on the research step', function (this: SpokRunWorld) { + this.runFailMarker = 'Call the `spok-create-research` skill'; +}); + +Given('the stub harness is repaired', function (this: SpokRunWorld) { + this.runFailMarker = undefined; +}); + +Given( + 'the staged flow task has persisted workflow state under the claude profile', + async function (this: SpokRunWorld) { + assert.ok(this.projectDir, 'projectDir must be set by Given a new project'); + assert.ok(this.flowTaskDir, 'flowTaskDir must be set by Given a staged flow task'); + const result = await runCLI(['flow', 'next', this.flowTaskDir, '--json'], { + cwd: this.projectDir, + env: stubEnv(this), + }); + assert.equal(result.exitCode, 0, result.stderr); + } +); + +When( + 'I run spok run on the staged flow task', + { timeout: 90_000 }, + async function (this: SpokRunWorld) { + await invokeRun(this, []); + } +); + +When( + 'I run spok run on the staged flow task with profile {string}', + { timeout: 90_000 }, + async function (this: SpokRunWorld, profile: string) { + await invokeRun(this, ['--profile', profile]); + } +); + +When( + 'I run spok run on the staged flow task with the --json flag', + { timeout: 90_000 }, + async function (this: SpokRunWorld) { + await invokeRun(this, ['--json']); + } +); + +Then('the spok run exits with code {int}', function (this: SpokRunWorld, expected: number) { + assert.ok(this.cliResult, 'cliResult must be set by a spok run step'); + assert.equal( + this.cliResult.exitCode, + expected, + `stdout:\n${this.cliResult.stdout}\nstderr:\n${this.cliResult.stderr}` + ); +}); + +Then('the spok run reports each dispatched step in order', function (this: SpokRunWorld) { + assert.deepEqual(dispatchedSteps(this), EXPECTED_STEP_ORDER); +}); + +Then('the spok run resumed at the research step', function (this: SpokRunWorld) { + assert.equal(dispatchedSteps(this)[0], 'research'); +}); + +Then('the spok run reports a blocked reason containing {string}', function ( + this: SpokRunWorld, + expected: string +) { + assert.ok(this.cliResult, 'cliResult must be set by a spok run step'); + const blocked = this.cliResult.stdout + .split('\n') + .find((line) => line.startsWith('Blocked: ')); + assert.ok(blocked, `stdout should carry a blocked line:\n${this.cliResult.stdout}`); + assert.ok(blocked.includes(expected), `blocked reason should contain: ${expected}\n${blocked}`); +}); + +Then('every spok run stdout line is JSON with schema version 1', function (this: SpokRunWorld) { + const events = stdoutEvents(this); + assert.ok(events.length > 0, 'stdout should carry at least one event line'); + for (const event of events) { + assert.equal(event.schemaVersion, 1); + } +}); + +Then('the spok run event stream reports the full flow lifecycle', function (this: SpokRunWorld) { + const events = stdoutEvents(this); + assert.equal(events[0]?.event, 'run_started'); + const started = events.filter((e) => e.event === 'step_started').map((e) => e.step); + const completed = events.filter((e) => e.event === 'step_completed').map((e) => e.step); + assert.deepEqual(started, EXPECTED_STEP_ORDER); + assert.deepEqual(completed, EXPECTED_STEP_ORDER); + const last = events.at(-1); + assert.equal(last?.event, 'complete'); + assert.equal(last?.commit, this.flowHeadCommit); +}); + +Then( + 'the spok run event stream ends with a blocked event carrying code {string}', + function (this: SpokRunWorld, code: string) { + const last = stdoutEvents(this).at(-1); + assert.equal(last?.event, 'blocked'); + assert.equal(last?.code, code); + assert.ok(typeof last?.reason === 'string' && last.reason.includes('recorded a FAIL verdict')); + } +); + +Then('the blocked event carries the human decisions from the design review', function ( + this: SpokRunWorld +) { + const last = stdoutEvents(this).at(-1); + assert.equal(last?.humanDecisions, '- Pick a direction for the API.'); +}); + +Then('the workflow state records the commit step as completed', async function (this: SpokRunWorld) { + assert.ok(this.flowTaskDir, 'flowTaskDir must be set by Given a staged flow task'); + const state = JSON.parse( + await fs.readFile(path.join(this.flowTaskDir, 'workflow-state.json'), 'utf-8') + ) as { steps: Array<{ id: string; status: string; result?: { commit?: string } }> }; + const commit = state.steps.find((step) => step.id === 'commit'); + assert.equal(commit?.status, 'completed'); + assert.equal(commit?.result?.commit, this.flowHeadCommit); +}); + +Then('the staged flow task has no workflow state file', async function (this: SpokRunWorld) { + assert.ok(this.flowTaskDir, 'flowTaskDir must be set by Given a staged flow task'); + await assert.rejects(fs.access(path.join(this.flowTaskDir, 'workflow-state.json'))); +}); diff --git a/test/features/tool-skill-artifacts.feature b/test/features/tool-skill-artifacts.feature index ecb5446..d6ee470 100644 --- a/test/features/tool-skill-artifacts.feature +++ b/test/features/tool-skill-artifacts.feature @@ -126,24 +126,21 @@ Feature: Tool skill artifacts Then the Spok CLI exits with code 1 And the Spok CLI output contains "schema is required" - Scenario: Apply delegates inner flow sequencing to deterministic flow commands + Scenario: Apply delegates inner flow sequencing to the spok run command Given a new project When I initialize Spok for the tools "claude" - Then the workflow skill "spok-flow" under ".claude/skills" mentions "spok flow next" - And the workflow skill "spok-flow" under ".claude/skills" mentions "spok flow complete" - And the workflow skill "spok-flow" under ".claude/skills" mentions "host-owned `general-purpose`" - And the workflow skill "spok-flow" under ".claude/skills" mentions "model: " - And the workflow skill "spok-flow" under ".claude/skills" mentions "effort: " - And the workflow skill "spok-flow" under ".claude/skills" mentions "spok flow next --json is the source of truth" + Then the workflow skill "spok-flow" under ".claude/skills" mentions "spok run" + And the workflow skill "spok-flow" under ".claude/skills" does not mention "spok flow next" + And the workflow skill "spok-flow" under ".claude/skills" does not mention "spok flow complete" And the workflow skill "spok-apply" under ".claude/skills" mentions "Spok settings live in spok/config.toml. To enable it, add:" And the workflow skill "spok-apply" under ".claude/skills" mentions "See available settings with: spok capabilities --json" Scenario: Flow relays design-review human decisions on FAIL and unreadable verdicts Given a new project When I initialize Spok for the tools "claude" - Then the workflow skill "spok-flow" under ".claude/skills" mentions "Step design-review recorded a FAIL verdict" - And the workflow skill "spok-flow" under ".claude/skills" mentions "Step design-review has no readable verdict" + Then the workflow skill "spok-flow" under ".claude/skills" mentions "humanDecisions" And the workflow skill "spok-flow" under ".claude/skills" mentions "## Human Decisions Required" + And the workflow skill "spok-flow" under ".claude/skills" mentions "content verbatim" Scenario: Design review makes the discussion the final design authority Given a new project @@ -163,10 +160,7 @@ Feature: Tool skill artifacts When I initialize Spok for the tools "claude,codex" Then the workflow skill "spok-apply" under ".claude/skills" mentions "/spok-apply hybrid" And the workflow skill "spok-apply" under ".claude/skills" mentions "hybrid " - And the workflow skill "spok-flow" under ".claude/skills" mentions "SPOK_FLOW_PROFILE=hybrid" - And the workflow skill "spok-flow" under ".claude/skills" mentions "codex exec" - And the workflow skill "spok-flow" under ".claude/skills" mentions "--dangerously-bypass-hook-trust" - And the workflow skill "spok-flow" under ".claude/skills" mentions "claude -p" + And the workflow skill "spok-flow" under ".claude/skills" mentions "--profile hybrid" Scenario: Hybrid apply preflights both harness skill closures Given a new project @@ -590,8 +584,6 @@ Feature: Tool skill artifacts And the workflow skill "spok-validate-implementation" under ".claude/skills" mentions "spok-codebase-analyzer" And the workflow skill "spok-validate-implementation" under ".claude/skills" does not mention "spok-qa" And the workflow skill "spok-implement-plan" under ".claude/skills" mentions "spok-implementer-agent" - And the workflow skill "spok-flow" under ".claude/skills" mentions "the current host's native subagent mechanism" - And the workflow skill "spok-flow" under ".claude/skills" mentions "host-owned `general-purpose`" Scenario: Interactive init creates missing agents for selected harnesses Given a new project diff --git a/test/skills/artifact-grounding.test.ts b/test/skills/artifact-grounding.test.ts index 4d67a62..c22ace8 100644 --- a/test/skills/artifact-grounding.test.ts +++ b/test/skills/artifact-grounding.test.ts @@ -76,13 +76,12 @@ describe('artifact grounding rules in skill assets', () => { }); describe('spok-flow', () => { - it('forbids relaying unattributed verification claims into the summary', async () => { + it('relays the CLI-reported outcome without inventing or softening it', async () => { const body = await readSkill('spok-flow'); - expect(body).toContain( - 'Do not relay a verification claim that cannot be attributed to a command that ran', - ); - expect(body).toContain('drop the claim from the summary'); + expect(body).toContain("report the event's `reason` verbatim"); + expect(body).toContain('`## Human Decisions Required` content verbatim'); + expect(body).toContain('never invent or soften an outcome'); }); }); diff --git a/test/skills/routing.test.ts b/test/skills/routing.test.ts index 73723cd..a9cf422 100644 --- a/test/skills/routing.test.ts +++ b/test/skills/routing.test.ts @@ -60,46 +60,43 @@ describe('spok fork-skill artifact routing', () => { } }); -describe('spok-flow prompt dispatch contract', () => { +describe('spok-flow wrapper contract', () => { async function readFlowSkill(): Promise { return fs.readFile(path.join(SKILLS_DIR, 'spok-flow', 'SKILL.md'), 'utf-8'); } - it('dispatches the CLI-composed prompt verbatim', async () => { + it('delegates the whole loop to spok run in the foreground', async () => { const body = await readFlowSkill(); - expect(body).toContain('`` **verbatim**'); - expect(body).toContain('spok/MEMORY.md'); - expect(body).toContain('memoryWarning'); + expect(body).toContain('spok run'); + expect(body).toContain('--json'); + expect(body).toContain('--profile hybrid'); + expect(body).toContain('ticket.md'); }); - it('leaves step-specific clauses to the CLI', async () => { + it('relays the CLI-reported outcome verbatim', async () => { const body = await readFlowSkill(); - expect(body).not.toContain('must not create commits'); - expect(body).not.toContain('Invoke `spok-self-learn`'); + expect(body).toContain("report the event's `reason` verbatim"); + expect(body).toContain('never invent or soften an outcome'); }); - it('dispatches hybrid steps through their declared runner', async () => { + it('surfaces warning events once and continues', async () => { const body = await readFlowSkill(); - expect(body).toContain('SPOK_FLOW_PROFILE=hybrid'); - expect(body).toContain('`step.runner`'); - expect(body).toContain('codex exec'); - expect(body).toContain('--dangerously-bypass-hook-trust'); - expect(body).toMatch(/run enabled hooks without an\s+interactive trust prompt/); - expect(body).toContain('claude -p'); - expect(body).toContain('Do not call `spok flow complete`'); - expect(body).toContain('Do not use `--dangerously-bypass-approvals-and-sandbox`'); + expect(body).toContain('surface its `message` to the user once and continue'); }); - it('uses host-neutral native delegation for the host-owned fallback agent', async () => { + it('contains no per-step dispatch or execution-semantics prose', async () => { const body = await readFlowSkill(); - expect(body).toContain("the current host's native subagent mechanism"); - expect(body).toContain('host-owned `general-purpose`'); - expect(body).not.toContain('subagent_type'); - expect(body).not.toContain('**Agent** tool'); + expect(body).not.toContain('claude -p'); + expect(body).not.toContain('codex exec'); + expect(body).not.toContain('spok flow next'); + expect(body).not.toContain('spok flow complete'); + expect(body).not.toContain('SPOK_FLOW_PROFILE'); + expect(body).not.toContain('subagent'); + expect(body).not.toContain('general-purpose'); }); }); diff --git a/vitest.setup.ts b/vitest.setup.ts index 1eea108..ef1b6d2 100644 --- a/vitest.setup.ts +++ b/vitest.setup.ts @@ -1,5 +1,13 @@ import { ensureCliBuilt } from './test/helpers/run-cli.js'; +// The pre-commit hook runs this suite from inside `git commit`, where git +// exports repo-pinning variables. Inherited by the temp git repos the tests +// (and the code under test) create, they retarget every git call at the real +// repository, so they must not survive into the workers. +for (const name of ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_OBJECT_DIRECTORY', 'GIT_PREFIX']) { + delete process.env[name]; +} + // Ensure the CLI bundle exists before tests execute export async function setup() { await ensureCliBuilt();