From 1bd991e335434ed5dd65e2745c94b793f8cec9bc Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 18:22:21 +0100 Subject: [PATCH 1/9] feat: apply the runner's file commands between steps A hosted runner communicates with a step through files and the step's stdout, so the local runner now does the same around each command the executor port runs. It asks a new scratch port for fresh command files per step and passes their paths as GITHUB_OUTPUT, GITHUB_ENV, GITHUB_PATH and GITHUB_STEP_SUMMARY; after the step it reads what was written and applies outputs to the step's report, environment entries and path entries to the job's later steps, and the summary to the run's new output port. A new required field on CommandResult carries stdout, which the runner reads for the workflow command stream: add-mask hides the value from the echoed output, and the debug, warning, error and notice commands route to the output port with their file, line, endLine and title annotation. Unknown commands, malformed file lines and unterminated heredocs are errors naming the command or the file and line, never silent passes. The runner still touches no filesystem and spawns no process: the host supplies the scratch and output ports, the executor's result carries stdout, and CommandExecution carries the job's accumulated path entries for the host to join onto the PATH it started from. The environment a step writes sits below the env the pipeline declares, masks and accumulated state are scoped to one job run, and path entries follow the provider's precedence (last written first, repeats moved not duplicated), each detail read from the actions/runner and actions/toolkit sources. The CLI pipes step stdout to collect it and echoes it after masking, which means step output appears when the step finishes rather than as it happens. What inner steps write is not forwarded to an outer provider runner's own files; that boundary is documented and tracked separately. --- README.md | 2 +- docs/providers.md | 2 +- packages/cli/src/bin.ts | 67 ++- packages/cli/src/main.ts | 37 ++ packages/cli/test/main.unit.test.ts | 118 ++++- packages/pipeline-contract/src/index.ts | 8 + .../src/runner-conformance.ts | 434 ++++++++++++++++-- packages/pipeline-contract/src/runner.ts | 69 +++ packages/pipeline-local/src/file-commands.ts | 186 ++++++++ packages/pipeline-local/src/runner.ts | 140 +++++- .../pipeline-local/test/runner.unit.test.ts | 340 +++++++++++++- 11 files changed, 1352 insertions(+), 51 deletions(-) create mode 100644 packages/pipeline-local/src/file-commands.ts diff --git a/README.md b/README.md index cf6ccdd..016d0dc 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ A workflow file mixes two things. The orchestration (triggers, job graph, matrix **Emit native files.** `emit` compiles a pipeline to the provider's own syntax. `check` fails when a committed file no longer matches what the tool would write. Use this where the provider must read orchestration from the repository. -**Run directly.** `run` executes the pipeline through an in-process runner that asks an injected executor to run each command. Nothing is generated. A single job, a step range or one matrix combination can be run alone. +**Run directly.** `run` executes the pipeline through an in-process runner that asks an injected executor to run each command. Nothing is generated. A single job, a step range or one matrix combination can be run alone. Around each step the runner applies the provider's file commands, as a hosted runner does: the step writes to `GITHUB_OUTPUT`, `GITHUB_ENV`, `GITHUB_PATH` and `GITHUB_STEP_SUMMARY`, and its stdout is read for `::add-mask::` and the log-level commands, so outputs reach the step's record, environment and paths reach the job's later steps, and masked values are not echoed. **Query the platform.** A separate read-only contract lists runs, checks, logs and the checks a branch requires. It does not depend on the pipeline model. diff --git a/docs/providers.md b/docs/providers.md index a685e3e..21ac65e 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -61,7 +61,7 @@ A job that names no runner label has an empty `runsOn`, which means the provider ## Wrapper form -Every adapter can render a job in the wrapper form: orchestration stays native and each contiguous run of portable steps becomes one call to this tool's runner for exactly those step indexes. A run step with `form: "native"` stays in the provider's syntax, which is how an install step runs before the tool exists. A matrix combination is passed as a quoted `--matrix "axis=value"` argument: as a shell-quoted literal where the adapter expands the matrix itself, and through the provider's own substitution (`${{ matrix.node }}`, `$node`, `<< parameters.node >>`) where the provider does. The run context the runner reads comes from the provider's environment variables; a value that provider does not expose (Bitbucket cannot tell a push from a manual run, CircleCI exposes no workflow name) is absent, and the runner fails only if a step or condition reads it. +Every adapter can render a job in the wrapper form: orchestration stays native and each contiguous run of portable steps becomes one call to this tool's runner for exactly those step indexes. A run step with `form: "native"` stays in the provider's syntax, which is how an install step runs before the tool exists. A matrix combination is passed as a quoted `--matrix "axis=value"` argument: as a shell-quoted literal where the adapter expands the matrix itself, and through the provider's own substitution (`${{ matrix.node }}`, `$node`, `<< parameters.node >>`) where the provider does. The run context the runner reads comes from the provider's environment variables; a value that provider does not expose (Bitbucket cannot tell a push from a manual run, CircleCI exposes no workflow name) is absent, and the runner fails only if a step or condition reads it. The runner's own file commands (`GITHUB_OUTPUT` and its siblings) apply between the steps of one wrapper call and are not forwarded to the provider's runner, so a native step after the call does not see what an inner step wrote. ## Check names diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts index 4cfcb62..434a65b 100644 --- a/packages/cli/src/bin.ts +++ b/packages/cli/src/bin.ts @@ -1,9 +1,12 @@ #!/usr/bin/env node import { spawn } from "node:child_process"; -import { mkdir, readFile, writeFile } from "node:fs/promises"; -import { dirname, resolve } from "node:path"; +import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { delimiter as pathDelimiter, dirname, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; +import type { StepScratch } from "@pipewright/pipeline-contract"; + import { main, type Io } from "./main.ts"; const cwd = process.cwd(); @@ -17,6 +20,46 @@ async function readOptional(path: string): Promise { } } +/** + * The run's scratch area: one directory per run under the operating system's temporary directory, one empty file per command per step, left in place for the process's life as a hosted runner leaves its `_temp`. Reading a file that is gone is an error, not an empty read, so a step that deletes its own command file is heard. + */ +async function stepScratch(): Promise { + const root = await mkdtemp(join(tmpdir(), "pipewright-")); + let stepCount = 0; + + return { + async create() { + const directory = join(root, String(stepCount)); + stepCount += 1; + await mkdir(directory, { recursive: true }); + const files = { + output: join(directory, "output"), + env: join(directory, "env"), + path: join(directory, "path"), + summary: join(directory, "summary"), + }; + await Promise.all([ + writeFile(files.output, ""), + writeFile(files.env, ""), + writeFile(files.path, ""), + writeFile(files.summary, ""), + ]); + + return files; + }, + async read(files) { + const [output, env, path, summary] = await Promise.all([ + readFile(files.output, "utf8"), + readFile(files.env, "utf8"), + readFile(files.path, "utf8"), + readFile(files.summary, "utf8"), + ]); + + return { output, env, path, summary }; + }, + }; +} + const io: Io = { cwd, env: process.env, @@ -33,22 +76,36 @@ const io: Io = { await mkdir(dirname(target), { recursive: true }); await writeFile(target, content); }, + scratch: await stepScratch(), executor: { async execute(execution, signal) { return new Promise((done, fail) => { // A run step stops at its first failing command, as it does under GitHub's `bash -e`, so a multi-line step cannot pass with an early command failed. const child = spawn("sh", ["-e", "-c", execution.command], { - stdio: "inherit", + // stdout is piped, not inherited, because the runner reads it for workflow commands and echoes it after masking; stderr has no commands in it and stays on the terminal as it happens. + stdio: ["inherit", "pipe", "inherit"], cwd: execution.workingDirectory === undefined ? cwd : resolve(cwd, execution.workingDirectory), - env: { ...process.env, ...execution.env }, + env: { + ...process.env, + ...execution.env, + // The job's accumulated path entries are searched before the host's own PATH. + PATH: [...execution.pathEntries, process.env.PATH] + .filter((entry): entry is string => entry !== undefined) + .join(pathDelimiter), + }, ...(signal === undefined ? {} : { signal }), }); + child.stdout.setEncoding("utf8"); + let stdout = ""; + child.stdout.on("data", (chunk: string) => { + stdout += chunk; + }); child.on("error", fail); child.on("close", (code) => { - done({ exitCode: code ?? 1 }); + done({ exitCode: code ?? 1, stdout }); }); }); }, diff --git a/packages/cli/src/main.ts b/packages/cli/src/main.ts index e1051cc..3cb353d 100644 --- a/packages/cli/src/main.ts +++ b/packages/cli/src/main.ts @@ -22,10 +22,13 @@ import { type CommandExecutor, type ContextKey, type FileReader, + type LogEntry, type MatrixCombination, type Pipeline, type PipelineCompiler, type PipelineImporter, + type RunOutput, + type StepScratch, } from "@pipewright/pipeline-contract"; import { createBitbucketPipelinesCompiler, @@ -54,6 +57,8 @@ export interface Io { readonly readOptional: (path: string) => Promise; readonly writeFile: (path: string, content: string) => Promise; readonly executor: CommandExecutor; + /** Allocates and reads each step's command files for `run`; the composition root owns the filesystem. */ + readonly scratch: StepScratch; /** Loads a TypeScript pipeline module and returns its exports. Injected because importing code is a runtime effect the CLI's tests must not perform. */ readonly importModule: (path: string) => Promise; readonly fetch: typeof fetch; @@ -219,6 +224,36 @@ function parseRange(text: string): { from: number; to: number } { return { from: Number(match[1]), to: Number(match[2]) }; } +/** + * The run's output as terminal text: a step's own lines as they came, a routed level with the place it pointed at, and each summary's markdown. + */ +function cliRunOutput(io: Io): RunOutput { + const placed = (entry: LogEntry): string => { + const annotation = entry.annotation; + if (annotation === undefined) return `${entry.level}: ${entry.message}`; + const lines = + annotation.line === undefined + ? "" + : `:${String(annotation.line)}${ + annotation.endLine === undefined || annotation.endLine === annotation.line + ? "" + : `-${String(annotation.endLine)}` + }`; + const title = annotation.title === undefined ? "" : ` (${annotation.title})`; + + return `${entry.level} ${annotation.file ?? ""}${lines}${title}: ${entry.message}`; + }; + + return { + log(entry) { + io.stdout(entry.level === "info" ? `${entry.message}\n` : `${placed(entry)}\n`); + }, + summary(step) { + io.stdout(step.text.endsWith("\n") ? step.text : `${step.text}\n`); + }, + }; +} + function repository(text: string | undefined): { owner: string; repo: string } { const [owner, repo, extra] = required(text, "--repo").split("/"); if ( @@ -338,6 +373,8 @@ export async function main(argv: readonly string[], io: Io): Promise { const matrix: MatrixCombination = Object.fromEntries(keyValues(values.matrix, "--matrix")); const report = await createLocalRunner().run(pipeline, { executor: io.executor, + scratch: io.scratch, + output: cliRunOutput(io), context: resolveContext(io.env, { ...(values.local === true ? localContext(pipeline.name) : {}), ...contextOverrides(values.context), diff --git a/packages/cli/test/main.unit.test.ts b/packages/cli/test/main.unit.test.ts index fd59cf7..13f33b5 100644 --- a/packages/cli/test/main.unit.test.ts +++ b/packages/cli/test/main.unit.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vitest"; +import type { CommandExecution, StepScratch } from "@pipewright/pipeline-contract"; + import { main, resolveContext, type Io } from "../src/index.ts"; const SCHEMA = "https://unpkg.com/@pipewright/pipeline-contract/dist/pipeline.schema.json"; @@ -30,6 +32,13 @@ interface Harness { readonly err: string[]; readonly files: Map; readonly commands: string[]; + readonly executions: CommandExecution[]; +} + +/** What each executed command leaves behind: the stdout it produced and the contents it wrote to the command files its environment names. */ +interface HarnessScript { + readonly stdout: string; + readonly writes: Partial>; } interface HarnessOptions { @@ -37,6 +46,44 @@ interface HarnessOptions { readonly modules?: Record; readonly env?: Readonly>; readonly failing?: (command: string) => boolean; + readonly script?: (execution: CommandExecution) => HarnessScript; +} + +/** The scratch over the harness's own file store, so a scripted executor can write the command files by the paths the runner passed. */ +function storeScratch(store: Map): StepScratch { + let stepCount = 0; + + return { + async create() { + const files = { + output: `.scratch/${String(stepCount)}/output`, + env: `.scratch/${String(stepCount)}/env`, + path: `.scratch/${String(stepCount)}/path`, + summary: `.scratch/${String(stepCount)}/summary`, + }; + stepCount += 1; + for (const path of [files.output, files.env, files.path, files.summary]) { + store.set(path, ""); + } + + return Promise.resolve(files); + }, + async read(files) { + const content = (path: string): string => { + const value = store.get(path); + if (value === undefined) throw new Error(`no such file ${path}`); + + return value; + }; + + return Promise.resolve({ + output: content(files.output), + env: content(files.env), + path: content(files.path), + summary: content(files.summary), + }); + }, + }; } function harness(options: HarnessOptions = {}): Harness { @@ -45,6 +92,7 @@ function harness(options: HarnessOptions = {}): Harness { env = {}, failing = () => false, modules = {}, + script = (): HarnessScript => ({ stdout: "", writes: {} }), } = options; const store = new Map( Object.entries(files).map(([path, value]) => [ @@ -55,6 +103,7 @@ function harness(options: HarnessOptions = {}): Harness { const out: string[] = []; const err: string[] = []; const commands: string[] = []; + const executions: CommandExecution[] = []; const io: Io = { cwd: "/repo", env, @@ -79,11 +128,30 @@ function harness(options: HarnessOptions = {}): Harness { return Promise.resolve(); }, + scratch: storeScratch(store), executor: { async execute(execution) { commands.push(execution.command); - - return Promise.resolve({ exitCode: failing(execution.command) ? 1 : 0 }); + executions.push(execution); + const answer = script(execution); + const variables = { + output: "GITHUB_OUTPUT", + env: "GITHUB_ENV", + path: "GITHUB_PATH", + summary: "GITHUB_STEP_SUMMARY", + } as const; + for (const file of ["output", "env", "path", "summary"] as const) { + const content = answer.writes[file]; + if (content === undefined) continue; + const path = execution.env[variables[file]]; + if (path === undefined) throw new Error(`the runner did not pass ${variables[file]}`); + store.set(path, content); + } + + return Promise.resolve({ + exitCode: failing(execution.command) ? 1 : 0, + stdout: answer.stdout, + }); }, }, async importModule(path) { @@ -95,7 +163,7 @@ function harness(options: HarnessOptions = {}): Harness { fetch: async () => Promise.reject(new Error("network is not available in this test")), }; - return { io, out, err, files: store, commands }; + return { io, out, err, files: store, commands, executions }; } const EMIT = ["--pipeline", "ci/ci.pipeline.json", "--provider", "github-actions"]; @@ -290,6 +358,50 @@ describe("run", () => { expect(h.err.join("")).toContain("refName"); expect(h.commands).toEqual([]); }); + + it("writes a step's environment file entries into later steps' environments", async () => { + const oneJob = { + ...pipeline, + jobs: { + build: { + name: "Build", + runsOn: ["ubuntu-latest"], + steps: [ + { kind: "run", run: "pnpm run lint" }, + { kind: "run", run: "echo gate" }, + ], + }, + }, + }; + const h = harness({ + files: { "ci/ci.pipeline.json": oneJob }, + script: (execution) => + execution.command === "pnpm run lint" + ? { stdout: "", writes: { env: "FROM_STEP=set\n" } } + : { stdout: "", writes: {} }, + }); + expect(await main(["run", "--pipeline", "ci/ci.pipeline.json", ...context], h.io)).toBe(0); + const gate = h.executions.find((execution) => execution.command === "echo gate"); + expect(gate?.env.FROM_STEP).toBe("set"); + }); + + it("echoes a step's stdout with masked values hidden and routed levels placed", async () => { + const h = harness({ + script: (execution) => + execution.command === "pnpm run lint" + ? { + stdout: + "::add-mask::s3cret\nthe token is s3cret\n::warning file=app.ts,line=2,endLine=4,title=Check::mind the gap\n", + writes: { summary: "## Lint\n" }, + } + : { stdout: "", writes: {} }, + }); + expect(await main(["run", "--pipeline", "ci/ci.pipeline.json", ...context], h.io)).toBe(0); + const printed = h.out.join(""); + expect(printed).toContain("the token is ***"); + expect(printed).toContain("warning app.ts:2-4 (Check): mind the gap\n"); + expect(printed).toContain("## Lint\n"); + }); }); describe("resolveContext", () => { diff --git a/packages/pipeline-contract/src/index.ts b/packages/pipeline-contract/src/index.ts index eba56fd..3c0dab0 100644 --- a/packages/pipeline-contract/src/index.ts +++ b/packages/pipeline-contract/src/index.ts @@ -65,16 +65,24 @@ export { jobGraphOf } from "./graph.ts"; export { slugOf, uniqueJobIds } from "./slug.ts"; export { expandMatrix, matrixValues, type MatrixCombination } from "./matrix.ts"; export { + type Annotation, type CommandExecution, type CommandExecutor, type CommandResult, type JobReport, + type LogEntry, + type LogLevel, type Outcome, type PipelineRunner, type RunContext, type RunOptions, + type RunOutput, type RunReport, + type StepCommandFiles, + type StepFileContents, type StepReport, + type StepScratch, + type StepSummary, } from "./runner.ts"; export { describeRunnerConformance } from "./runner-conformance.ts"; export type { PredicateNode } from "trilean"; diff --git a/packages/pipeline-contract/src/runner-conformance.ts b/packages/pipeline-contract/src/runner-conformance.ts index 6a632fb..e393768 100644 --- a/packages/pipeline-contract/src/runner-conformance.ts +++ b/packages/pipeline-contract/src/runner-conformance.ts @@ -1,6 +1,17 @@ import { UnsupportedFeatureError } from "./errors.ts"; -import type { CommandExecution, CommandExecutor, PipelineRunner, RunContext } from "./runner.ts"; -import { pipelineSchema } from "./schema.ts"; +import type { + CommandExecution, + CommandExecutor, + LogEntry, + PipelineRunner, + RunContext, + RunOutput, + StepCommandFiles, + StepFileContents, + StepScratch, + StepSummary, +} from "./runner.ts"; +import { pipelineSchema, type Pipeline } from "./schema.ts"; import { samplePipeline, type TestApi } from "./conformance.ts"; const CONTEXT: RunContext = { @@ -13,23 +24,129 @@ const CONTEXT: RunContext = { runId: "1", }; +/** The environment variable that carries each command file, as the command sees it; the runner must pass exactly these. */ +const FILE_VARIABLES: Readonly> = { + output: "GITHUB_OUTPUT", + env: "GITHUB_ENV", + path: "GITHUB_PATH", + summary: "GITHUB_STEP_SUMMARY", +}; + interface RecordingExecutor extends CommandExecutor { readonly executed: CommandExecution[]; } -function recordingExecutor(failing: (command: string) => boolean = () => false): RecordingExecutor { +/** What the kit's scripted executor answers one step with: the stdout it produced, and what it wrote to the command files its environment names. */ +export interface StepScript { + readonly stdout: string; + readonly writes: Readonly>; +} + +interface ExecutorBehaviour { + /** Names the commands that fail, by the text of the command. */ + readonly failing?: (command: string) => boolean; + /** What each executed command leaves behind, by the execution the runner passed it. */ + readonly script?: (execution: CommandExecution) => StepScript; +} + +function recordingExecutor( + scratch: MemoryScratch, + behaviour: ExecutorBehaviour = {}, +): RecordingExecutor { const executed: CommandExecution[] = []; + const failing = behaviour.failing ?? (() => false); + const script = behaviour.script ?? ((): StepScript => ({ stdout: "", writes: {} })); return { executed, - async execute(command) { - executed.push(command); + async execute(execution) { + executed.push(execution); + const answer = script(execution); + for (const file of ["output", "env", "path", "summary"] as const) { + const content = answer.writes[file]; + if (content === undefined) continue; + const path = execution.env[FILE_VARIABLES[file]]; + if (path === undefined) throw new Error(`the runner did not pass ${FILE_VARIABLES[file]}`); + scratch.written.set(path, content); + } + + return Promise.resolve({ + exitCode: failing(execution.command) ? 1 : 0, + stdout: answer.stdout, + }); + }, + }; +} + +interface MemoryScratch extends StepScratch { + /** The scratch's store, keyed by path, so the scripted executor can stand in for the command that writes its files. */ + readonly written: Map; +} + +function memoryScratch(): MemoryScratch { + const written = new Map(); + let stepCount = 0; + + return { + written, + async create(): Promise { + const files: StepCommandFiles = { + output: `scratch/${String(stepCount)}/output`, + env: `scratch/${String(stepCount)}/env`, + path: `scratch/${String(stepCount)}/path`, + summary: `scratch/${String(stepCount)}/summary`, + }; + stepCount += 1; + for (const path of [files.output, files.env, files.path, files.summary]) { + written.set(path, ""); + } + + return Promise.resolve(files); + }, + async read(files): Promise { + const content = (path: string): string => { + const value = written.get(path); + if (value === undefined) throw new Error(`no scratch file at ${path}`); + + return value; + }; + + return Promise.resolve({ + output: content(files.output), + env: content(files.env), + path: content(files.path), + summary: content(files.summary), + }); + }, + }; +} + +interface RecordingOutput extends RunOutput { + readonly entries: LogEntry[]; + readonly summaries: StepSummary[]; +} + +function recordingOutput(): RecordingOutput { + const entries: LogEntry[] = []; + const summaries: StepSummary[] = []; - return Promise.resolve({ exitCode: failing(command.command) ? 1 : 0 }); + return { + entries, + summaries, + log(entry) { + entries.push(entry); + }, + summary(summary) { + summaries.push(summary); }, }; } +/** The ports a run needs besides its executor, fresh for each case so no state leaks between them. */ +function ports(): { scratch: MemoryScratch; output: RecordingOutput } { + return { scratch: memoryScratch(), output: recordingOutput() }; +} + function check(condition: boolean, message: string): void { if (!condition) throw new Error(message); } @@ -40,8 +157,14 @@ function check(condition: boolean, message: string): void { export function describeRunnerConformance(api: TestApi, makeRunner: () => PipelineRunner): void { api.describe("PipelineRunner conformance", () => { api.it("runs jobs in needs order", async () => { - const executor = recordingExecutor(); - const report = await makeRunner().run(samplePipeline(), { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); + const report = await makeRunner().run(samplePipeline(), { + executor, + scratch, + output, + context: CONTEXT, + }); const commands = executor.executed.map((entry) => entry.command); check(report.ok, "a clean run was reported as failed"); check( @@ -52,8 +175,9 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }); api.it("runs a job once per matrix combination with the axis value in scope", async () => { - const executor = recordingExecutor(); - await makeRunner().run(samplePipeline(), { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); + await makeRunner().run(samplePipeline(), { executor, scratch, output, context: CONTEXT }); const versions = executor.executed .filter((entry) => entry.command === "pnpm test") .map((entry) => entry.env.NODE_VERSION); @@ -64,8 +188,9 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }); api.it("resolves context tokens in step environments", async () => { - const executor = recordingExecutor(); - await makeRunner().run(samplePipeline(), { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); + await makeRunner().run(samplePipeline(), { executor, scratch, output, context: CONTEXT }); const test = executor.executed.find((entry) => entry.command === "pnpm test"); check(test?.env.SHA === "abc123", "the sha token was not resolved"); }); @@ -75,8 +200,16 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli const gate = pipeline.jobs.gate; if (gate === undefined) throw new Error("sample pipeline lost its gate job"); gate.needsPolicy = "always"; - const executor = recordingExecutor((command) => command === "pnpm run lint"); - const report = await makeRunner().run(pipeline, { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + failing: (command) => command === "pnpm run lint", + }); + const report = await makeRunner().run(pipeline, { + executor, + scratch, + output, + context: CONTEXT, + }); const commands = executor.executed.map((entry) => entry.command); check(!report.ok, "a failing run was reported as ok"); check(!commands.includes("pnpm test"), "a dependant of a failed job ran"); @@ -101,8 +234,14 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }, }, }); - const executor = recordingExecutor(); - const report = await makeRunner().run(pipeline, { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); + const report = await makeRunner().run(pipeline, { + executor, + scratch, + output, + context: CONTEXT, + }); check(executor.executed.length === 0, "a job whose condition is false ran"); check(report.jobs[0]?.outcome === "skipped", "the job was not reported as skipped"); }); @@ -114,9 +253,15 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli provider: "github-actions", native: { uses: "example/action@v1" }, }); + const { scratch, output } = ports(); let thrown: unknown; try { - await makeRunner().run(pipeline, { executor: recordingExecutor(), context: CONTEXT }); + await makeRunner().run(pipeline, { + executor: recordingExecutor(scratch), + scratch, + output, + context: CONTEXT, + }); } catch (error) { thrown = error; } @@ -124,9 +269,12 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }); api.it("runs only the matrix combination it is given", async () => { - const executor = recordingExecutor(); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); await makeRunner().run(samplePipeline(), { executor, + scratch, + output, context: CONTEXT, only: ["test"], matrix: { node: "24" }, @@ -154,9 +302,12 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }, }, }); - const executor = recordingExecutor(); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); await makeRunner().run(pipeline, { executor, + scratch, + output, context: CONTEXT, only: ["build"], steps: { from: 1, to: 1 }, @@ -175,9 +326,12 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli provider: "github-actions", native: { uses: "example/action@v1" }, }); - const executor = recordingExecutor(); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); const report = await makeRunner().run(pipeline, { executor, + scratch, + output, context: CONTEXT, skipProviderSteps: true, }); @@ -190,9 +344,16 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }); api.it("reports a gate as failed, without running it, when a job it needs failed", async () => { - const pipeline = samplePipeline(); - const executor = recordingExecutor((command) => command === "pnpm run lint"); - const report = await makeRunner().run(pipeline, { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + failing: (command) => command === "pnpm run lint", + }); + const report = await makeRunner().run(samplePipeline(), { + executor, + scratch, + output, + context: CONTEXT, + }); const reported = report.jobs.find((job) => job.id === "gate"); check( reported?.outcome === "failure", @@ -228,8 +389,9 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }, }, }); - const executor = recordingExecutor((command) => command === "fail"); - const report = await makeRunner().run(pipeline, { executor, context: {} }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { failing: (command) => command === "fail" }); + const report = await makeRunner().run(pipeline, { executor, scratch, output, context: {} }); const second = report.jobs.find((job) => job.id === "second"); check(second?.outcome === "skipped", "the blocked job was not reported as skipped"); }); @@ -244,16 +406,27 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli left: { kind: "reference", key: "refName" }, right: { kind: "textLiteral", value: "release" }, }; - const executor = recordingExecutor((command) => command === "pnpm run lint"); - const report = await makeRunner().run(pipeline, { executor, context: CONTEXT }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + failing: (command) => command === "pnpm run lint", + }); + const report = await makeRunner().run(pipeline, { + executor, + scratch, + output, + context: CONTEXT, + }); const reported = report.jobs.find((job) => job.id === "gate"); check(reported?.outcome === "skipped", `the gate was ${String(reported?.outcome)}`); }); api.it("runs only the named jobs when asked", async () => { - const executor = recordingExecutor(); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch); await makeRunner().run(samplePipeline(), { executor, + scratch, + output, context: CONTEXT, only: ["test"], }); @@ -264,4 +437,209 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli ); }); }); + + api.describe("PipelineRunner file commands", () => { + /** A two-step job, the smallest pipeline over which one step can hand something to a later one. */ + const twoStepPipeline = (): Pipeline => + pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + build: { + name: "Build", + runsOn: ["x"], + steps: [ + { kind: "run", run: "first" }, + { kind: "run", run: "second" }, + ], + }, + }, + }); + + api.it("passes fresh command files to each step and reads what the step wrote", async () => { + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: () => ({ + stdout: "", + writes: { env: "HANDED=over\n" }, + }), + }); + await makeRunner().run(twoStepPipeline(), { executor, scratch, output, context: {} }); + const [first, second] = executor.executed; + check(first?.env.GITHUB_OUTPUT !== second?.env.GITHUB_OUTPUT, "steps shared an output file"); + check(first?.env.GITHUB_ENV !== undefined, "the environment file was not passed"); + check(first?.env.GITHUB_PATH !== undefined, "the path file was not passed"); + check(first?.env.GITHUB_STEP_SUMMARY !== undefined, "the summary file was not passed"); + check(second?.env.HANDED === "over", "a later step did not receive the written entry"); + }); + + api.it("records a step's outputs in its report, heredocs included", async () => { + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: (execution) => ({ + stdout: "", + writes: + execution.command === "first" + ? { output: "plain=1\nmulti< { + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: (execution) => ({ + stdout: "", + writes: execution.command === "first" ? { path: "/one\n/two\n" } : {}, + }), + }); + await makeRunner().run(twoStepPipeline(), { executor, scratch, output, context: {} }); + const second = executor.executed[1]; + check( + second?.pathEntries.length === 2 && second.pathEntries[0] === "/two", + `later step searched ${JSON.stringify(second?.pathEntries)}`, + ); + }); + + api.it( + "routes the command stream: masks hide later lines, levels carry their annotation", + async () => { + const { scratch, output } = ports(); + const FILE = "app.ts"; + const FROM_LINE = 2; + const TO_LINE = 4; + const TITLE = "Check"; + const executor = recordingExecutor(scratch, { + script: () => ({ + stdout: `before\n::add-mask::s3cret\nthe token is s3cret\n::warning file=${FILE},line=${String(FROM_LINE)},endLine=${String(TO_LINE)},title=${TITLE}::mind the gap\n`, + writes: {}, + }), + }); + await makeRunner().run(twoStepPipeline(), { executor, scratch, output, context: {} }); + const messages = output.entries.map((entry) => entry.message); + check(messages.includes("before"), "a line before any mask was dropped"); + check(messages.includes("the token is ***"), `masked line was ${JSON.stringify(messages)}`); + const warning = output.entries.find((entry) => entry.level === "warning"); + check(warning !== undefined, "the warning command produced no entry"); + check( + warning?.annotation?.file === FILE && + warning.annotation.line === FROM_LINE && + warning.annotation.endLine === TO_LINE && + warning.annotation.title === TITLE && + warning.message === "mind the gap", + `the annotation was ${JSON.stringify(warning?.annotation)}`, + ); + }, + ); + + api.it("rejects an unknown workflow command, naming it", async () => { + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: () => ({ + stdout: "::group::tests\n", + writes: {}, + }), + }); + let thrown: unknown; + try { + await makeRunner().run(twoStepPipeline(), { executor, scratch, output, context: {} }); + } catch (error) { + thrown = error; + } + check( + thrown instanceof Error && thrown.message.includes("::group::"), + `the unknown command was rejected with ${String(thrown)}`, + ); + }); + + api.it( + "rejects a malformed line in a step's environment file, naming the file and line", + async () => { + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: () => ({ + stdout: "", + writes: { env: "HEALTHY=yes\nnot an entry\n" }, + }), + }); + let thrown: unknown; + try { + await makeRunner().run(twoStepPipeline(), { executor, scratch, output, context: {} }); + } catch (error) { + thrown = error; + } + const file = executor.executed[0]?.env.GITHUB_ENV; + check( + thrown instanceof Error && + file !== undefined && + thrown.message.includes(file) && + thrown.message.includes("line 2"), + `the malformed line was rejected with ${String(thrown)}`, + ); + }, + ); + + api.it("keeps a step's summary as run output the caller can read", async () => { + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: (execution) => ({ + stdout: "", + writes: execution.command === "first" ? { summary: "## Built\n" } : {}, + }), + }); + await makeRunner().run(twoStepPipeline(), { executor, scratch, output, context: {} }); + check( + output.summaries.length === 1 && output.summaries[0]?.text === "## Built\n", + `summaries were ${JSON.stringify(output.summaries)}`, + ); + }); + + api.it("does not carry masks or environment into another job", async () => { + const pipeline = pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + first: { + name: "First", + runsOn: ["x"], + steps: [{ kind: "run", run: "first" }], + }, + second: { + name: "Second", + runsOn: ["x"], + needs: ["first"], + steps: [{ kind: "run", run: "second" }], + }, + }, + }); + const { scratch, output } = ports(); + const executor = recordingExecutor(scratch, { + script: (execution) => ({ + stdout: execution.command === "first" ? "::add-mask::s3cret\ns3cret\n" : "s3cret\n", + writes: execution.command === "first" ? { env: "HELD=back\n" } : {}, + }), + }); + await makeRunner().run(pipeline, { executor, scratch, output, context: {} }); + const echoed = output.entries.filter((entry) => entry.level === "info").map((e) => e.message); + check( + echoed.includes("***") && echoed.includes("s3cret"), + `the mask did not stay in its own job: ${JSON.stringify(echoed)}`, + ); + const second = executor.executed.find((entry) => entry.command === "second"); + check(second?.env.HELD === undefined, "an environment entry leaked into another job"); + }); + }); } diff --git a/packages/pipeline-contract/src/runner.ts b/packages/pipeline-contract/src/runner.ts index 895b8df..5e1ea33 100644 --- a/packages/pipeline-contract/src/runner.ts +++ b/packages/pipeline-contract/src/runner.ts @@ -9,10 +9,73 @@ export interface CommandExecution { readonly command: string; readonly env: Readonly>; readonly workingDirectory: string | undefined; + /** Directories to search before the host's own `PATH`, most preferred first: the additions the job's earlier steps declared, which only the host can join onto the `PATH` it started from. */ + readonly pathEntries: readonly string[]; } export interface CommandResult { readonly exitCode: number; + /** Everything the command wrote to stdout, which the runner reads for workflow commands and echoes through the run's output after masking. */ + readonly stdout: string; +} + +/** The severity a log entry carries. `info` is a step's ordinary output. */ +export type LogLevel = "info" | "debug" | "warning" | "error" | "notice"; + +/** Where in the repository a log entry points, as the command that raised it stated. */ +export interface Annotation { + readonly file?: string; + readonly line?: number; + readonly endLine?: number; + readonly title?: string; +} + +/** One piece of a run's output: a line of a step's stdout after masking, or a command the step routed to a level. */ +export interface LogEntry { + readonly job: string; + readonly step: string; + readonly level: LogLevel; + readonly message: string; + readonly annotation?: Annotation; +} + +/** A step's summary markdown, as the step wrote it to its summary file. */ +export interface StepSummary { + readonly job: string; + readonly step: string; + readonly text: string; +} + +/** + * Where a run's output goes, supplied by whoever hosts the runner: each step's stdout after masking, the levels workflow commands were routed to, and each step's summary. The runner writes nowhere else, so a host that discards it silences the run. + */ +export interface RunOutput { + readonly log: (entry: LogEntry) => void; + readonly summary: (summary: StepSummary) => void; +} + +/** The paths of the command files one step communicates through, named by the environment variable that carries each. */ +export interface StepCommandFiles { + readonly output: string; + readonly env: string; + readonly path: string; + readonly summary: string; +} + +/** What a step wrote to its command files. */ +export interface StepFileContents { + readonly output: string; + readonly env: string; + readonly path: string; + readonly summary: string; +} + +/** + * Allocates and reads the per-step command files, the runner's filesystem boundary: the runner never touches a filesystem itself, so the host creates each step's files empty before the command runs and reads them back after it exits. Both are per step, so one step can never read what another wrote. + */ +export interface StepScratch { + readonly create: () => Promise; + readonly read: (files: StepCommandFiles) => Promise; } export interface CommandExecutor { @@ -29,6 +92,8 @@ export type Outcome = "success" | "failure" | "skipped"; export interface StepReport { readonly name: string; readonly outcome: Outcome; + /** The outputs the step declared through its output file, so later steps and the report consumer can read them. */ + readonly outputs: Readonly>; } export interface JobReport { @@ -48,6 +113,10 @@ export interface RunReport { export interface RunOptions { readonly executor: CommandExecutor; + /** Allocates and reads each step's command files, the runner's filesystem boundary. */ + readonly scratch: StepScratch; + /** Receives the run's output: step stdout after masking, the levels commands were routed to, and summaries. */ + readonly output: RunOutput; readonly context: RunContext; /** Run only these job ids; every job when omitted. The host has already decided to run them, so their `needs` and job-level `when` are treated as satisfied. */ readonly only?: readonly string[]; diff --git a/packages/pipeline-local/src/file-commands.ts b/packages/pipeline-local/src/file-commands.ts new file mode 100644 index 0000000..3c926d6 --- /dev/null +++ b/packages/pipeline-local/src/file-commands.ts @@ -0,0 +1,186 @@ +import type { Annotation } from "@pipewright/pipeline-contract"; + +/** One `key=value`, or heredoc, entry a step wrote to its output or environment file. */ +export interface FileEntry { + readonly key: string; + readonly value: string; +} + +/** + * The lines of a command file. A trailing newline terminates the last entry rather than starting an empty one; an interior blank line carries no entry, as the provider's own runner skips it. + */ +function fileLines(content: string): string[] { + const lines = content.split("\n"); + if (lines[lines.length - 1] === "") lines.pop(); + + return lines; +} + +/** + * Parses an output or environment file: `key=value` lines, and multi-line values as `key<= 0 && (heredoc < 0 || equals < heredoc)) { + const key = line.slice(0, equals); + if (key === "") throw new Error(`${at} has an empty key: ${line}`); + + entries.push({ key, value: line.slice(equals + 1) }); + continue; + } + if (heredoc >= 0 && (equals < 0 || heredoc < equals)) { + const key = line.slice(0, heredoc); + const delimiter = line.slice(heredoc + 2); + if (key === "" || delimiter === "") { + throw new Error(`${at} opens a heredoc with an empty key or delimiter: ${line}`); + } + const end = lines.indexOf(delimiter, offset + 1); + if (end === -1) { + throw new Error(`${at} opens a heredoc for ${key} that no later line closes`); + } + entries.push({ key, value: lines.slice(offset + 1, end).join("\n") }); + consumed = end + 1; + continue; + } + + throw new Error(`${at} is neither key=value nor a heredoc: ${line}`); + } + + return entries; +} + +/** + * Parses a path file: each non-blank line is one directory. No line is malformed, because any line of text can name a directory; the runner never checks the host's filesystem. + */ +export function parsePathFile(content: string): string[] { + return fileLines(content).filter((line) => line !== ""); +} + +/** + * The path entries a step wrote, most preferred first. Each line the step wrote takes precedence over the lines before it and over every entry an earlier step wrote, and a repeated entry moves rather than duplicates, which is the order the provider's own runner joins onto `PATH`. + */ +export function preferredPaths(written: readonly string[], previous: readonly string[]): string[] { + const ordered = [...new Set(written.toReversed())]; + + return [...ordered, ...previous.filter((entry) => !ordered.includes(entry))]; +} + +/** A workflow command parsed from a step's stdout. */ +export type WorkflowCommand = + | { readonly kind: "mask"; readonly value: string } + | { + readonly kind: "level"; + readonly level: "debug" | "warning" | "error" | "notice"; + readonly message: string; + readonly annotation: Annotation | undefined; + }; + +/** One line of a step's stdout: a workflow command, or text the run echoes. */ +export type StdoutLine = + | { readonly kind: "command"; readonly command: WorkflowCommand } + | { readonly kind: "text"; readonly text: string }; + +/** The codes a command writer escapes to, as actions/toolkit writes them: data is `%`, CR and LF; a property value additionally escapes the `:` and `,` its own syntax uses as separators. */ +const DATA_CODES: Readonly> = { + "%25": "%", + "%0D": "\r", + "%0A": "\n", +}; + +const PROPERTY_CODES: Readonly> = { + ...DATA_CODES, + "%3A": ":", + "%2C": ",", +}; + +/** + * Reverses the toolkit's escaping in one pass, so a `%25` that stands for a literal percent is not itself read as the start of another code. A percent sequence the writer did not escape stays as written. + */ +function unescape(text: string, codes: Readonly>): string { + return text.replace(/%[0-9A-F]{2}/g, (code) => codes[code] ?? code); +} + +const LOG_LEVELS = ["debug", "warning", "error", "notice"] as const; + +/** + * The properties a log command carries: `file`, `line`, `endLine` and `title`, exactly the set the provider documents. A property outside that set, or a line number that is not one, is an error naming the command and the property. + */ +function parseAnnotation(command: string, properties: string): Annotation | undefined { + if (properties === "") return undefined; + + const annotation: { file?: string; line?: number; endLine?: number; title?: string } = {}; + for (const part of properties.split(",")) { + const separator = part.indexOf("="); + if (separator <= 0) { + throw new Error(`::${command}:: carries the property "${part}", which is not name=value`); + } + const name = part.slice(0, separator); + const value = unescape(part.slice(separator + 1), PROPERTY_CODES); + if (name === "file") { + annotation.file = value; + } else if (name === "title") { + annotation.title = value; + } else if (name === "line" || name === "endLine") { + if (!/^\d+$/.test(value)) { + throw new Error(`::${command}:: property ${name} is not a line number: "${value}"`); + } + annotation[name] = Number(value); + } else { + throw new Error(`::${command}:: carries the unknown property ${name}`); + } + } + + return annotation; +} + +/** A command head: a name, optionally followed by a space and the command's `property=value` list, as actions/toolkit writes it. */ +const COMMAND_HEAD = /^([a-z][a-z0-9-]*)(?:[ ](.*))?$/; + +/** + * Parses one line of a step's stdout as a workflow command: `::name::message` or `::name property=value,...::message`. A line that starts with `::` and closes a second `::` is a command, and a name outside the implemented set is an error naming it, never a silent pass; a line that does not parse as one is ordinary text. + */ +export function parseStdoutLine(line: string): StdoutLine { + if (!line.startsWith("::")) return { kind: "text", text: line }; + const closing = line.indexOf("::", 2); + if (closing === -1) return { kind: "text", text: line }; + + const head = COMMAND_HEAD.exec(line.slice(2, closing)); + if (head?.[1] === undefined) return { kind: "text", text: line }; + const name = head[1]; + const properties = head[2] ?? ""; + const message = unescape(line.slice(closing + 2), DATA_CODES); + + if (name === "add-mask") { + if (properties !== "") { + throw new Error( + `::add-mask:: carries the unknown property ${properties.split("=")[0] ?? ""}`, + ); + } + if (message === "") throw new Error("::add-mask:: masks an empty value"); + + return { kind: "command", command: { kind: "mask", value: message } }; + } + const level = LOG_LEVELS.find((candidate) => candidate === name); + if (level === undefined) throw new Error(`unknown workflow command ::${name}::`); + + return { + kind: "command", + command: { kind: "level", level, message, annotation: parseAnnotation(name, properties) }, + }; +} + +/** + * Replaces every occurrence of each masked value with `***`, longer values first so a mask that contains another does not leave fragments of it behind. + */ +export function applyMasks(text: string, masks: readonly string[]): string { + return masks + .toSorted((left, right) => right.length - left.length) + .reduce((result, mask) => result.replaceAll(mask, "***"), text); +} diff --git a/packages/pipeline-local/src/runner.ts b/packages/pipeline-local/src/runner.ts index 93044f5..a6d3f4c 100644 --- a/packages/pipeline-local/src/runner.ts +++ b/packages/pipeline-local/src/runner.ts @@ -19,8 +19,23 @@ import { type StepReport, } from "@pipewright/pipeline-contract"; +import { + applyMasks, + parseKeyValueFile, + parsePathFile, + parseStdoutLine, + preferredPaths, + type FileEntry, +} from "./file-commands.ts"; + const PROVIDER = "local"; +/** The environment variables that carry the command files, as the command itself sees them. */ +const OUTPUT_VARIABLE = "GITHUB_OUTPUT"; +const ENV_VARIABLE = "GITHUB_ENV"; +const PATH_VARIABLE = "GITHUB_PATH"; +const SUMMARY_VARIABLE = "GITHUB_STEP_SUMMARY"; + interface Scope { readonly context: RunContext; readonly matrix: MatrixCombination; @@ -136,39 +151,133 @@ interface JobPlan { readonly failsGate: boolean; } +/** What one run of a job accumulates from its steps, as a runner process does on the host provider. Scoped to the job run, because no two job runs share a machine. */ +interface JobRunState { + /** Entries the steps wrote to their environment file, below the env the pipeline itself declares. */ + readonly env: Record; + /** Path entries the steps wrote, most preferred first; the host joins them onto the PATH it started from. Replaced, not appended, when a step writes more. */ + pathEntries: string[]; + /** Values the steps asked to mask, in the order they were registered. */ + readonly masks: string[]; +} + +/** The state one matrix combination runs against: what its steps read and what they accumulate. */ +interface CombinationRun { + readonly scope: Scope; + readonly state: JobRunState; +} + +/** What running one step produced: its report entry and what it handed to the job's later steps. */ +interface StepRun { + readonly outcome: Outcome; + readonly outputs: Readonly>; + /** Entries the step wrote to its environment file, applied to the job's later steps. */ + readonly env: readonly FileEntry[]; + /** Directories the step added, applied to the job's later steps. */ + readonly paths: readonly string[]; + /** The summary the step wrote, masked, when it wrote one. */ + readonly summary: string | undefined; +} + function inRange(options: RunOptions, index: number): boolean { return options.steps === undefined || (index >= options.steps.from && index <= options.steps.to); } -async function runStep(plan: JobPlan, scope: Scope, step: Step, index: number): Promise { +function recordOf(entries: readonly FileEntry[]): Record { + return Object.fromEntries(entries.map((entry) => [entry.key, entry.value])); +} + +/** + * Reads the workflow command stream from one step's stdout and echoes the rest through the run's output, masked. A mask covers only the lines after the command that registered it, as a runner reading the stream live does. + */ +function processStdout(plan: JobPlan, state: JobRunState, step: Step, stdout: string): void { + const lines = stdout.split("\n"); + // A trailing newline ends the last line rather than starting an empty one, and a line's own CR is dropped, so a command emitted with CRLF line endings is still read. + if (lines[lines.length - 1] === "") lines.pop(); + for (const ended of lines) { + const line = ended.endsWith("\r") ? ended.slice(0, -1) : ended; + const parsed = parseStdoutLine(line); + if (parsed.kind === "command") { + if (parsed.command.kind === "mask") { + state.masks.push(parsed.command.value); + continue; + } + plan.options.output.log({ + job: plan.job.name, + step: stepName(step), + level: parsed.command.level, + message: applyMasks(parsed.command.message, state.masks), + ...(parsed.command.annotation === undefined + ? {} + : { annotation: parsed.command.annotation }), + }); + continue; + } + plan.options.output.log({ + job: plan.job.name, + step: stepName(step), + level: "info", + message: applyMasks(parsed.text, state.masks), + }); + } +} + +async function runStep( + plan: JobPlan, + combination: CombinationRun, + step: Step, + index: number, +): Promise { + const { scope, state } = combination; const condition = plan.stepConditions[index]; if (condition === undefined || !inRange(plan.options, index) || !condition(scope)) { - return "skipped"; + return { outcome: "skipped", outputs: {}, env: [], paths: [], summary: undefined }; + } + if (step.kind === "provider") { + return { outcome: "skipped", outputs: {}, env: [], paths: [], summary: undefined }; } - if (step.kind === "provider") return "skipped"; - if (step.kind !== "run") return "success"; + if (step.kind !== "run") { + return { outcome: "success", outputs: {}, env: [], paths: [], summary: undefined }; + } + const files = await plan.options.scratch.create(); const result = await plan.options.executor.execute( { command: step.run, env: { + ...state.env, ...resolveEnv(plan.pipeline.env, scope), ...resolveEnv(plan.job.env, scope), ...resolveEnv(step.env, scope), + [OUTPUT_VARIABLE]: files.output, + [ENV_VARIABLE]: files.env, + [PATH_VARIABLE]: files.path, + [SUMMARY_VARIABLE]: files.summary, }, workingDirectory: step.workingDirectory, + pathEntries: [...state.pathEntries], }, plan.options.signal, ); + processStdout(plan, state, step, result.stdout); + const contents = await plan.options.scratch.read(files); - return result.exitCode === 0 ? "success" : "failure"; + return { + outcome: result.exitCode === 0 ? "success" : "failure", + outputs: recordOf(parseKeyValueFile(files.output, contents.output)), + env: parseKeyValueFile(files.env, contents.env), + paths: parsePathFile(contents.path), + summary: contents.summary === "" ? undefined : applyMasks(contents.summary, state.masks), + }; } async function runCombination(plan: JobPlan, matrix: MatrixCombination): Promise { - const scope: Scope = { context: plan.options.context, matrix }; + const state: JobRunState = { env: {}, pathEntries: [], masks: [] }; + const combination: CombinationRun = { scope: { context: plan.options.context, matrix }, state }; const { job, id } = plan; const skippedSteps = job.steps.map((step) => ({ name: stepName(step), outcome: "skipped" as const, + outputs: {}, })); // A gate's condition is read first, because the gate is evaluated even after a failed dependency and its own condition decides whether it reports. Any other job is not evaluated at all once a dependency has not succeeded, so a condition over a value the host never supplied cannot mask the upstream failure. if (plan.failsGate) { @@ -176,26 +285,33 @@ async function runCombination(plan: JobPlan, matrix: MatrixCombination): Promise id, name: job.name, matrix, - outcome: plan.jobCondition(scope) ? "failure" : "skipped", + outcome: plan.jobCondition(combination.scope) ? "failure" : "skipped", steps: skippedSteps, }; } - if (!plan.mayRun || !plan.jobCondition(scope)) { + if (!plan.mayRun || !plan.jobCondition(combination.scope)) { return { id, name: job.name, matrix, outcome: "skipped", - steps: job.steps.map((step) => ({ name: stepName(step), outcome: "skipped" })), + steps: skippedSteps, }; } const steps = await job.steps.reduce>(async (previous, step, index) => { const reports = await previous; const failed = reports.some((report) => report.outcome === "failure"); - const outcome = failed ? "skipped" : await runStep(plan, scope, step, index); + const run = failed + ? { outcome: "skipped" as const, outputs: {}, env: [], paths: [], summary: undefined } + : await runStep(plan, combination, step, index); + for (const entry of run.env) state.env[entry.key] = entry.value; + state.pathEntries = preferredPaths(run.paths, state.pathEntries); + if (run.summary !== undefined) { + plan.options.output.summary({ job: job.name, step: stepName(step), text: run.summary }); + } - return [...reports, { name: stepName(step), outcome }]; + return [...reports, { name: stepName(step), outcome: run.outcome, outputs: run.outputs }]; }, Promise.resolve([])); return { @@ -243,6 +359,8 @@ function aggregate(reports: readonly JobReport[]): Outcome { * Runs a pipeline in-process by asking an injected executor to run each `run` step. No provider configuration is emitted, so this is the way to use a pipeline without ever synthesising a workflow file: from a terminal, from a static bootstrap workflow, or under test. * * A job with a matrix runs once per combination, sequentially. `checkout` is a no-op, because the host has already placed the working tree. + * + * Around each step the runner applies the provider's file commands: it asks the host's scratch for fresh command files and passes their paths in the command's environment, then reads what the step wrote, records its outputs, and carries its environment and path entries to the job's later steps. The step's stdout is read for workflow commands, whose masks and levels are applied to the run's output. */ export function createLocalRunner(): PipelineRunner { return { diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 76ac945..4dab04d 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -1,7 +1,343 @@ -import { describe, it } from "vitest"; +import { spawn } from "node:child_process"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; -import { describeRunnerConformance } from "@pipewright/pipeline-contract"; +import { describe, expect, it } from "vitest"; + +import { + describeRunnerConformance, + pipelineSchema, + type CommandExecution, + type CommandExecutor, + type RunOutput, + type StepCommandFiles, + type StepFileContents, + type StepScratch, +} from "@pipewright/pipeline-contract"; import { createLocalRunner } from "../src/index.ts"; +import { + applyMasks, + parseKeyValueFile, + parsePathFile, + parseStdoutLine, + preferredPaths, +} from "../src/file-commands.ts"; describeRunnerConformance({ describe, it }, createLocalRunner); + +describe("parseKeyValueFile", () => { + it("reads key=value lines, splitting at the first equals", () => { + expect(parseKeyValueFile("GITHUB_ENV", "A=1\nB=x=y\n")).toEqual([ + { key: "A", value: "1" }, + { key: "B", value: "x=y" }, + ]); + }); + + it("reads a multi-line value between its delimiter lines", () => { + expect( + parseKeyValueFile("GITHUB_OUTPUT", "k< { + expect(parseKeyValueFile("GITHUB_OUTPUT", "k=a< { + expect(() => parseKeyValueFile("GITHUB_ENV", "A=1\nnot an entry\n")).toThrow( + "GITHUB_ENV line 2", + ); + expect(() => parseKeyValueFile("GITHUB_ENV", "=1\n")).toThrow("GITHUB_ENV line 1"); + expect(() => parseKeyValueFile("GITHUB_OUTPUT", "k< { + it("reads one directory per non-blank line", () => { + expect(parsePathFile("/one\n\n/two\n")).toEqual(["/one", "/two"]); + }); + + it("orders what a step wrote most preferred first, moving a repeated entry rather than duplicating it", () => { + expect(preferredPaths(["/one", "/two"], ["/old"])).toEqual(["/two", "/one", "/old"]); + expect(preferredPaths(["/one"], ["/one", "/old"])).toEqual(["/one", "/old"]); + }); +}); + +describe("parseStdoutLine", () => { + it("reads each level command with its annotation and unescapes the toolkit's codes", () => { + const parsed = parseStdoutLine("::error file=a%2Cb.ts,line=3::missing%20nothing"); + expect(parsed).toEqual({ + kind: "command", + command: { + kind: "level", + level: "error", + message: "missing%20nothing", + annotation: { file: "a,b.ts", line: 3 }, + }, + }); + }); + + it("reads an add-mask command and rejects one with no value", () => { + expect(parseStdoutLine("::add-mask::hunter2")).toEqual({ + kind: "command", + command: { kind: "mask", value: "hunter2" }, + }); + expect(() => parseStdoutLine("::add-mask::")).toThrow("add-mask"); + }); + + it("treats text that does not parse as a command as output, and rejects an unknown command name", () => { + expect(parseStdoutLine("a line ::error:: of text")).toEqual({ + kind: "text", + text: "a line ::error:: of text", + }); + expect(parseStdoutLine("::no closing marker")).toEqual({ + kind: "text", + text: "::no closing marker", + }); + expect(() => parseStdoutLine("::group::name")).toThrow("::group::"); + }); + + it("rejects a property outside the documented set and a line number that is not one", () => { + expect(() => parseStdoutLine("::warning column=3::m")).toThrow("column"); + expect(() => parseStdoutLine("::warning line=three::m")).toThrow("line"); + }); + + it("unescapes the codes the toolkit writes, in one pass so a literal code survives", () => { + expect(parseStdoutLine("::debug::a%0Ab%25")).toEqual({ + kind: "command", + command: { kind: "level", level: "debug", message: "a\nb%", annotation: undefined }, + }); + expect(parseStdoutLine("::debug::%250A")).toEqual({ + kind: "command", + command: { kind: "level", level: "debug", message: "%0A", annotation: undefined }, + }); + }); +}); + +describe("applyMasks", () => { + it("hides every occurrence, longer masks first", () => { + expect(applyMasks("a token and token2", ["token", "token2"])).toBe("a *** and ***"); + expect(applyMasks("abc and abcd", ["abc", "abcd"])).toBe("*** and ***"); + }); +}); + +/** A scratch in memory, pre-seeding each step's files empty and reading back exactly what was written. */ +function memoryScratch(): StepScratch & { readonly written: Map } { + const written = new Map(); + let stepCount = 0; + + return { + written, + async create(): Promise { + const files = { + output: `s${String(stepCount)}/output`, + env: `s${String(stepCount)}/env`, + path: `s${String(stepCount)}/path`, + summary: `s${String(stepCount)}/summary`, + }; + stepCount += 1; + for (const path of [files.output, files.env, files.path, files.summary]) { + written.set(path, ""); + } + + return Promise.resolve(files); + }, + async read(files): Promise { + const content = (path: string): string => { + const value = written.get(path); + if (value === undefined) throw new Error(`no scratch file at ${path}`); + + return value; + }; + + return Promise.resolve({ + output: content(files.output), + env: content(files.env), + path: content(files.path), + summary: content(files.summary), + }); + }, + }; +} + +/** Writes one of the command files the runner passed a command, as the command itself would have. */ +function write( + execution: CommandExecution, + file: "GITHUB_OUTPUT" | "GITHUB_ENV" | "GITHUB_PATH" | "GITHUB_STEP_SUMMARY", + scratch: ReturnType, + content: string, +): void { + const path = execution.env[file]; + if (path === undefined) throw new Error(`the runner did not pass ${file}`); + scratch.written.set(path, content); +} + +/** The entries and summaries a run produced, as text, so assertions read like the output a host would print. */ +interface RunOutputRecorder extends RunOutput { + readonly lines: string[]; +} + +function recordedOutput(): RunOutputRecorder { + const lines: string[] = []; + + return { + lines, + log(entry) { + lines.push(`${entry.level}:${entry.message}`); + }, + summary(summary) { + lines.push(`summary:${summary.text}`); + }, + }; +} + +describe("the local runner's file commands", () => { + it("applies what a step wrote and echoes its stdout masked, through fakes", async () => { + const scratch = memoryScratch(); + const executed: CommandExecution[] = []; + const executor: CommandExecutor = { + async execute(execution) { + executed.push(execution); + if (execution.command === "first") { + write(execution, "GITHUB_ENV", scratch, "STEP_ENV=written\n"); + write(execution, "GITHUB_OUTPUT", scratch, "out=here\n"); + + return Promise.resolve({ exitCode: 0, stdout: "::add-mask::shh\nshh out there\n" }); + } + + return Promise.resolve({ exitCode: 0, stdout: "" }); + }, + }; + const pipeline = pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + build: { + name: "Build", + runsOn: ["x"], + env: { STEP_ENV: "declared" }, + steps: [ + { kind: "run", run: "first" }, + { kind: "run", run: "second" }, + ], + }, + }, + }); + const output = recordedOutput(); + const report = await createLocalRunner().run(pipeline, { + executor, + scratch, + output, + context: {}, + }); + expect(report.jobs[0]?.steps[0]?.outputs).toEqual({ out: "here" }); + expect(output.lines).toEqual(["info:*** out there"]); + // The env the pipeline declares for the step wins over what an earlier step wrote, as the provider's runner applies the file below the declared env. + expect(executed[1]?.env.STEP_ENV).toBe("declared"); + }); + + it("applies the file commands a real shell step writes", async () => { + const root = await mkdtemp(join(tmpdir(), "pipewright-local-")); + try { + let stepCount = 0; + const scratch: StepScratch = { + async create() { + const directory = join(root, String(stepCount)); + stepCount += 1; + await mkdir(directory, { recursive: true }); + const files = { + output: join(directory, "output"), + env: join(directory, "env"), + path: join(directory, "path"), + summary: join(directory, "summary"), + }; + await Promise.all([ + writeFile(files.output, ""), + writeFile(files.env, ""), + writeFile(files.path, ""), + writeFile(files.summary, ""), + ]); + + return files; + }, + async read(files) { + const [output, env, path, summary] = await Promise.all([ + readFile(files.output, "utf8"), + readFile(files.env, "utf8"), + readFile(files.path, "utf8"), + readFile(files.summary, "utf8"), + ]); + + return { output, env, path, summary }; + }, + }; + // The real executor's shape from the CLI: spawn `sh -e -c`, pipe stdout, and search the job's path entries before the host's own PATH. + const executor: CommandExecutor = { + async execute(execution) { + return new Promise((done, fail) => { + const child = spawn("sh", ["-e", "-c", execution.command], { + stdio: ["inherit", "pipe", "inherit"], + env: { + ...process.env, + ...execution.env, + PATH: [...execution.pathEntries, process.env.PATH] + .filter((entry): entry is string => entry !== undefined) + .join(":"), + }, + }); + child.stdout.setEncoding("utf8"); + let stdout = ""; + child.stdout.on("data", (chunk: string) => { + stdout += chunk; + }); + child.on("error", fail); + child.on("close", (code) => { + done({ exitCode: code ?? 1, stdout }); + }); + }); + }, + }; + const output = recordedOutput(); + const pipeline = pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + build: { + name: "Build", + runsOn: ["x"], + steps: [ + { + kind: "run", + run: [ + 'echo "A=from-file" >> "$GITHUB_ENV"', + "printf 'multi<> \"$GITHUB_OUTPUT\"", + 'echo "::add-mask::hunter2"', + 'echo "token=hunter2"', + 'echo "## Summary" >> "$GITHUB_STEP_SUMMARY"', + ].join("\n"), + }, + { kind: "run", run: 'test "$A" = "from-file" && echo "env applied"' }, + ], + }, + }, + }); + const report = await createLocalRunner().run(pipeline, { + executor, + scratch, + output, + context: {}, + }); + expect(report.ok).toBe(true); + expect(report.jobs[0]?.steps[0]?.outputs).toEqual({ multi: "line one\nline two" }); + expect(output.lines).toContain("info:token=***"); + expect(output.lines).toContain("info:env applied"); + expect(output.lines).toContain("summary:## Summary\n"); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); +}); From 02ea6e52c16abfa10dd6950de020e09549925a7f Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 19:05:44 +0100 Subject: [PATCH 2/9] fix(runner): remove each step's command files once read and the scratch at the run's end --- packages/cli/src/bin.ts | 11 +++++++-- packages/cli/test/main.unit.test.ts | 14 +++++++++++ .../src/runner-conformance.ts | 12 ++++++++++ packages/pipeline-contract/src/runner.ts | 6 ++++- packages/pipeline-local/src/runner.ts | 2 ++ .../pipeline-local/test/runner.unit.test.ts | 23 ++++++++++++++++++- 6 files changed, 64 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts index 434a65b..46d1d48 100644 --- a/packages/cli/src/bin.ts +++ b/packages/cli/src/bin.ts @@ -1,6 +1,6 @@ #!/usr/bin/env node import { spawn } from "node:child_process"; -import { mkdir, mkdtemp, readFile, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { delimiter as pathDelimiter, dirname, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; @@ -21,7 +21,7 @@ async function readOptional(path: string): Promise { } /** - * The run's scratch area: one directory per run under the operating system's temporary directory, one empty file per command per step, left in place for the process's life as a hosted runner leaves its `_temp`. Reading a file that is gone is an error, not an empty read, so a step that deletes its own command file is heard. + * The run's scratch area: one directory per run under the operating system's temporary directory, one empty file per command per step, removed once the runner has read it and in whole when the run ends, so no step can read another's files and nothing the run wrote outlives it. Reading a file that is gone is an error, not an empty read, so a step that deletes its own command file is heard. */ async function stepScratch(): Promise { const root = await mkdtemp(join(tmpdir(), "pipewright-")); @@ -57,6 +57,13 @@ async function stepScratch(): Promise { return { output, env, path, summary }; }, + async release(files) { + // A later step's process must not read what an earlier one wrote, so the directory and its files go once the runner has read them. + await rm(dirname(files.output), { recursive: true, force: true }); + }, + async dispose() { + await rm(root, { recursive: true, force: true }); + }, }; } diff --git a/packages/cli/test/main.unit.test.ts b/packages/cli/test/main.unit.test.ts index 13f33b5..a529f09 100644 --- a/packages/cli/test/main.unit.test.ts +++ b/packages/cli/test/main.unit.test.ts @@ -68,6 +68,20 @@ function storeScratch(store: Map): StepScratch { return Promise.resolve(files); }, + async release(files) { + for (const path of [files.output, files.env, files.path, files.summary]) { + store.delete(path); + } + + return Promise.resolve(); + }, + async dispose() { + for (const path of [...store.keys()]) { + if (path.startsWith(".scratch/")) store.delete(path); + } + + return Promise.resolve(); + }, async read(files) { const content = (path: string): string => { const value = store.get(path); diff --git a/packages/pipeline-contract/src/runner-conformance.ts b/packages/pipeline-contract/src/runner-conformance.ts index e393768..eeec1a6 100644 --- a/packages/pipeline-contract/src/runner-conformance.ts +++ b/packages/pipeline-contract/src/runner-conformance.ts @@ -118,6 +118,18 @@ function memoryScratch(): MemoryScratch { summary: content(files.summary), }); }, + async release(files): Promise { + for (const path of [files.output, files.env, files.path, files.summary]) { + written.delete(path); + } + + return Promise.resolve(); + }, + async dispose(): Promise { + written.clear(); + + return Promise.resolve(); + }, }; } diff --git a/packages/pipeline-contract/src/runner.ts b/packages/pipeline-contract/src/runner.ts index 5e1ea33..03ce063 100644 --- a/packages/pipeline-contract/src/runner.ts +++ b/packages/pipeline-contract/src/runner.ts @@ -71,11 +71,15 @@ export interface StepFileContents { } /** - * Allocates and reads the per-step command files, the runner's filesystem boundary: the runner never touches a filesystem itself, so the host creates each step's files empty before the command runs and reads them back after it exits. Both are per step, so one step can never read what another wrote. + * Allocates, reads and reclaims the per-step command files, the runner's filesystem boundary: the runner never touches a filesystem itself, so the host creates each step's files empty before the command runs, reads them back after it exits and removes them once read, so one step can never read what another wrote, not even from disk. */ export interface StepScratch { readonly create: () => Promise; readonly read: (files: StepCommandFiles) => Promise; + /** Called once the runner has read a step's files, so the host removes them: a later step's process must not be able to read what an earlier one wrote. */ + readonly release: (files: StepCommandFiles) => Promise; + /** Called when the run ends, so the host removes the scratch root and nothing the run wrote outlives it. */ + readonly dispose: () => Promise; } export interface CommandExecutor { diff --git a/packages/pipeline-local/src/runner.ts b/packages/pipeline-local/src/runner.ts index a6d3f4c..9835098 100644 --- a/packages/pipeline-local/src/runner.ts +++ b/packages/pipeline-local/src/runner.ts @@ -260,6 +260,7 @@ async function runStep( ); processStdout(plan, state, step, result.stdout); const contents = await plan.options.scratch.read(files); + await plan.options.scratch.release(files); return { outcome: result.exitCode === 0 ? "success" : "failure", @@ -401,6 +402,7 @@ export function createLocalRunner(): PipelineRunner { }); const reports = jobReports.flat(); + await options.scratch.dispose(); return { jobs: reports, ok: reports.every((report) => report.outcome !== "failure") }; }, diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 4dab04d..3397141 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -1,7 +1,8 @@ import { spawn } from "node:child_process"; +import { existsSync } from "node:fs"; import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -161,6 +162,18 @@ function memoryScratch(): StepScratch & { readonly written: Map summary: content(files.summary), }); }, + async release(files): Promise { + for (const path of [files.output, files.env, files.path, files.summary]) { + written.delete(path); + } + + return Promise.resolve(); + }, + async dispose(): Promise { + written.clear(); + + return Promise.resolve(); + }, }; } @@ -274,6 +287,12 @@ describe("the local runner's file commands", () => { return { output, env, path, summary }; }, + async release(files) { + await rm(dirname(files.output), { recursive: true, force: true }); + }, + async dispose() { + await rm(root, { recursive: true, force: true }); + }, }; // The real executor's shape from the CLI: spawn `sh -e -c`, pipe stdout, and search the job's path entries before the host's own PATH. const executor: CommandExecutor = { @@ -336,6 +355,8 @@ describe("the local runner's file commands", () => { expect(output.lines).toContain("info:token=***"); expect(output.lines).toContain("info:env applied"); expect(output.lines).toContain("summary:## Summary\n"); + // Each step's directory is removed once read and the root at the run's end, so a later step's process cannot read what an earlier one wrote. + expect(existsSync(root)).toBe(false); } finally { await rm(root, { recursive: true, force: true }); } From 19efc09b587977cc3ba2b97677145898e286246c Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 19:39:21 +0100 Subject: [PATCH 3/9] fix(runner): release a step's files and dispose the scratch when a run throws --- packages/pipeline-local/src/runner.ts | 75 +++++++++++-------- .../pipeline-local/test/runner.unit.test.ts | 31 ++++++++ 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/packages/pipeline-local/src/runner.ts b/packages/pipeline-local/src/runner.ts index 9835098..42e5c2f 100644 --- a/packages/pipeline-local/src/runner.ts +++ b/packages/pipeline-local/src/runner.ts @@ -16,6 +16,7 @@ import { type RunContext, type RunOptions, type Step, + type StepFileContents, type StepReport, } from "@pipewright/pipeline-contract"; @@ -258,9 +259,14 @@ async function runStep( }, plan.options.signal, ); - processStdout(plan, state, step, result.stdout); - const contents = await plan.options.scratch.read(files); - await plan.options.scratch.release(files); + let contents: StepFileContents; + try { + processStdout(plan, state, step, result.stdout); + contents = await plan.options.scratch.read(files); + } finally { + // The files go even when reading or the command stream throws, so a failure leaves nothing a later process can read. + await plan.options.scratch.release(files); + } return { outcome: result.exitCode === 0 ? "success" : "failure", @@ -372,37 +378,42 @@ export function createLocalRunner(): PipelineRunner { ); const outcomes = new Map(); - const jobReports = await inSequence(ids, async (id) => { - const job = pipeline.jobs[id]; - if (job === undefined) throw new Error(`unknown job ${id}`); - checkPortable(id, job, options); - - const needsSatisfied = - selected !== undefined || - (job.needs ?? []).every((needed) => outcomes.get(needed) === "success"); - const plan: JobPlan = { - id, - job, - pipeline, - options, - jobCondition: - selected === undefined ? conditionOf(job.when, `/jobs/${id}/when`) : () => true, - stepConditions: job.steps.map((step, index) => - conditionOf(step.when, `/jobs/${id}/steps/${String(index)}/when`), - ), - mayRun: job.needsPolicy === "always" || needsSatisfied, - failsGate: job.needsPolicy === "gate" && !needsSatisfied, - }; - const reports = await inSequence(combinationsOf(job, id, options), async (matrix) => - runCombination(plan, matrix), - ); - outcomes.set(id, aggregate(reports)); - - return reports; - }); + let jobReports; + try { + jobReports = await inSequence(ids, async (id) => { + const job = pipeline.jobs[id]; + if (job === undefined) throw new Error(`unknown job ${id}`); + checkPortable(id, job, options); + + const needsSatisfied = + selected !== undefined || + (job.needs ?? []).every((needed) => outcomes.get(needed) === "success"); + const plan: JobPlan = { + id, + job, + pipeline, + options, + jobCondition: + selected === undefined ? conditionOf(job.when, `/jobs/${id}/when`) : () => true, + stepConditions: job.steps.map((step, index) => + conditionOf(step.when, `/jobs/${id}/steps/${String(index)}/when`), + ), + mayRun: job.needsPolicy === "always" || needsSatisfied, + failsGate: job.needsPolicy === "gate" && !needsSatisfied, + }; + const reports = await inSequence(combinationsOf(job, id, options), async (matrix) => + runCombination(plan, matrix), + ); + outcomes.set(id, aggregate(reports)); + + return reports; + }); + } finally { + // The scratch root goes even when a job threw, so a failed run leaves nothing behind. + await options.scratch.dispose(); + } const reports = jobReports.flat(); - await options.scratch.dispose(); return { jobs: reports, ok: reports.every((report) => report.outcome !== "failure") }; }, diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 3397141..7693ddd 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -361,4 +361,35 @@ describe("the local runner's file commands", () => { await rm(root, { recursive: true, force: true }); } }); + + it("cleans the scratch when a run throws", async () => { + const scratch = memoryScratch(); + const executor: CommandExecutor = { + async execute() { + return Promise.resolve({ exitCode: 0, stdout: "::no-such-command::x\n" }); + }, + }; + const pipeline = pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + build: { + name: "Build", + runsOn: ["x"], + steps: [{ kind: "run", run: "echo" }], + }, + }, + }); + + // An unknown workflow command throws between the step's execution and its files being read, and the run still releases the step's files and disposes the scratch. + await expect( + createLocalRunner().run(pipeline, { + executor, + scratch, + output: recordedOutput(), + context: {}, + }), + ).rejects.toThrow("unknown workflow command"); + expect(scratch.written.size).toBe(0); + }); }); From 0666234a1bc367a08b28f26183cd99c97829d50e Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 20:18:42 +0100 Subject: [PATCH 4/9] fix(runner): emit a step's stderr through the run's masks --- packages/cli/src/bin.ts | 11 ++++++++--- packages/cli/test/main.unit.test.ts | 1 + .../src/runner-conformance.ts | 3 +++ packages/pipeline-contract/src/runner.ts | 2 ++ packages/pipeline-local/src/runner.ts | 19 +++++++++++++++++++ .../pipeline-local/test/runner.unit.test.ts | 14 +++++++++----- 6 files changed, 42 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/bin.ts b/packages/cli/src/bin.ts index 46d1d48..14cf706 100644 --- a/packages/cli/src/bin.ts +++ b/packages/cli/src/bin.ts @@ -89,8 +89,8 @@ const io: Io = { return new Promise((done, fail) => { // A run step stops at its first failing command, as it does under GitHub's `bash -e`, so a multi-line step cannot pass with an early command failed. const child = spawn("sh", ["-e", "-c", execution.command], { - // stdout is piped, not inherited, because the runner reads it for workflow commands and echoes it after masking; stderr has no commands in it and stays on the terminal as it happens. - stdio: ["inherit", "pipe", "inherit"], + // Both streams are piped, not inherited, because either can carry a value a step masked, so the runner emits both through its masks; a step's output appears when the step ends. + stdio: ["inherit", "pipe", "pipe"], cwd: execution.workingDirectory === undefined ? cwd @@ -106,13 +106,18 @@ const io: Io = { ...(signal === undefined ? {} : { signal }), }); child.stdout.setEncoding("utf8"); + child.stderr.setEncoding("utf8"); let stdout = ""; + let stderr = ""; child.stdout.on("data", (chunk: string) => { stdout += chunk; }); + child.stderr.on("data", (chunk: string) => { + stderr += chunk; + }); child.on("error", fail); child.on("close", (code) => { - done({ exitCode: code ?? 1, stdout }); + done({ exitCode: code ?? 1, stdout, stderr }); }); }); }, diff --git a/packages/cli/test/main.unit.test.ts b/packages/cli/test/main.unit.test.ts index a529f09..5e35088 100644 --- a/packages/cli/test/main.unit.test.ts +++ b/packages/cli/test/main.unit.test.ts @@ -165,6 +165,7 @@ function harness(options: HarnessOptions = {}): Harness { return Promise.resolve({ exitCode: failing(execution.command) ? 1 : 0, stdout: answer.stdout, + stderr: "", }); }, }, diff --git a/packages/pipeline-contract/src/runner-conformance.ts b/packages/pipeline-contract/src/runner-conformance.ts index eeec1a6..02da744 100644 --- a/packages/pipeline-contract/src/runner-conformance.ts +++ b/packages/pipeline-contract/src/runner-conformance.ts @@ -39,6 +39,8 @@ interface RecordingExecutor extends CommandExecutor { /** What the kit's scripted executor answers one step with: the stdout it produced, and what it wrote to the command files its environment names. */ export interface StepScript { readonly stdout: string; + /** What the scripted command writes to stderr, so a case can cover its masking. */ + readonly stderr?: string; readonly writes: Readonly>; } @@ -73,6 +75,7 @@ function recordingExecutor( return Promise.resolve({ exitCode: failing(execution.command) ? 1 : 0, stdout: answer.stdout, + stderr: answer.stderr ?? "", }); }, }; diff --git a/packages/pipeline-contract/src/runner.ts b/packages/pipeline-contract/src/runner.ts index 03ce063..e2b0dbd 100644 --- a/packages/pipeline-contract/src/runner.ts +++ b/packages/pipeline-contract/src/runner.ts @@ -17,6 +17,8 @@ export interface CommandResult { readonly exitCode: number; /** Everything the command wrote to stdout, which the runner reads for workflow commands and echoes through the run's output after masking. */ readonly stdout: string; + /** Everything the command wrote to stderr, which carries no workflow commands but can carry a masked value, so the runner emits it through the same masks. */ + readonly stderr: string; } /** The severity a log entry carries. `info` is a step's ordinary output. */ diff --git a/packages/pipeline-local/src/runner.ts b/packages/pipeline-local/src/runner.ts index 42e5c2f..d956314 100644 --- a/packages/pipeline-local/src/runner.ts +++ b/packages/pipeline-local/src/runner.ts @@ -191,6 +191,24 @@ function recordOf(entries: readonly FileEntry[]): Record { /** * Reads the workflow command stream from one step's stdout and echoes the rest through the run's output, masked. A mask covers only the lines after the command that registered it, as a runner reading the stream live does. */ +/** + * A step's stderr carries no workflow commands, but it can carry a masked value, so each line is emitted through the masks the run has registered. + */ +function processStderr(plan: JobPlan, state: JobRunState, step: Step, stderr: string): void { + if (stderr === "") return; + const lines = stderr.split("\n"); + if (lines[lines.length - 1] === "") lines.pop(); + for (const ended of lines) { + const line = ended.endsWith("\r") ? ended.slice(0, -1) : ended; + plan.options.output.log({ + job: plan.job.name, + step: stepName(step), + level: "info", + message: applyMasks(line, state.masks), + }); + } +} + function processStdout(plan: JobPlan, state: JobRunState, step: Step, stdout: string): void { const lines = stdout.split("\n"); // A trailing newline ends the last line rather than starting an empty one, and a line's own CR is dropped, so a command emitted with CRLF line endings is still read. @@ -262,6 +280,7 @@ async function runStep( let contents: StepFileContents; try { processStdout(plan, state, step, result.stdout); + processStderr(plan, state, step, result.stderr); contents = await plan.options.scratch.read(files); } finally { // The files go even when reading or the command stream throws, so a failure leaves nothing a later process can read. diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 7693ddd..69338ea 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -219,10 +219,14 @@ describe("the local runner's file commands", () => { write(execution, "GITHUB_ENV", scratch, "STEP_ENV=written\n"); write(execution, "GITHUB_OUTPUT", scratch, "out=here\n"); - return Promise.resolve({ exitCode: 0, stdout: "::add-mask::shh\nshh out there\n" }); + return Promise.resolve({ + exitCode: 0, + stdout: "::add-mask::shh\nshh out there\n", + stderr: "shh on stderr\n", + }); } - return Promise.resolve({ exitCode: 0, stdout: "" }); + return Promise.resolve({ exitCode: 0, stdout: "", stderr: "" }); }, }; const pipeline = pipelineSchema.parse({ @@ -248,7 +252,7 @@ describe("the local runner's file commands", () => { context: {}, }); expect(report.jobs[0]?.steps[0]?.outputs).toEqual({ out: "here" }); - expect(output.lines).toEqual(["info:*** out there"]); + expect(output.lines).toEqual(["info:*** out there", "info:*** on stderr"]); // The env the pipeline declares for the step wins over what an earlier step wrote, as the provider's runner applies the file below the declared env. expect(executed[1]?.env.STEP_ENV).toBe("declared"); }); @@ -315,7 +319,7 @@ describe("the local runner's file commands", () => { }); child.on("error", fail); child.on("close", (code) => { - done({ exitCode: code ?? 1, stdout }); + done({ exitCode: code ?? 1, stdout, stderr: "" }); }); }); }, @@ -366,7 +370,7 @@ describe("the local runner's file commands", () => { const scratch = memoryScratch(); const executor: CommandExecutor = { async execute() { - return Promise.resolve({ exitCode: 0, stdout: "::no-such-command::x\n" }); + return Promise.resolve({ exitCode: 0, stdout: "::no-such-command::x\n", stderr: "" }); }, }; const pipeline = pipelineSchema.parse({ From edc4752424dab05b287a42a975ac1252c1e3e23c Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 20:43:53 +0100 Subject: [PATCH 5/9] fix(runner): never quote a command file's line in a parse error --- packages/pipeline-local/src/file-commands.ts | 7 ++++--- .../pipeline-local/test/runner.unit.test.ts | 20 ++++++++++++++----- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/pipeline-local/src/file-commands.ts b/packages/pipeline-local/src/file-commands.ts index 3c926d6..b691487 100644 --- a/packages/pipeline-local/src/file-commands.ts +++ b/packages/pipeline-local/src/file-commands.ts @@ -30,7 +30,8 @@ export function parseKeyValueFile(fileName: string, content: string): FileEntry[ const heredoc = line.indexOf("<<"); if (equals >= 0 && (heredoc < 0 || equals < heredoc)) { const key = line.slice(0, equals); - if (key === "") throw new Error(`${at} has an empty key: ${line}`); + // The line's text is never in the message, because a malformed line can carry a value a step masked. + if (key === "") throw new Error(`${at} has an empty key`); entries.push({ key, value: line.slice(equals + 1) }); continue; @@ -39,7 +40,7 @@ export function parseKeyValueFile(fileName: string, content: string): FileEntry[ const key = line.slice(0, heredoc); const delimiter = line.slice(heredoc + 2); if (key === "" || delimiter === "") { - throw new Error(`${at} opens a heredoc with an empty key or delimiter: ${line}`); + throw new Error(`${at} opens a heredoc with an empty key or delimiter`); } const end = lines.indexOf(delimiter, offset + 1); if (end === -1) { @@ -50,7 +51,7 @@ export function parseKeyValueFile(fileName: string, content: string): FileEntry[ continue; } - throw new Error(`${at} is neither key=value nor a heredoc: ${line}`); + throw new Error(`${at} is neither key=value nor a heredoc`); } return entries; diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 69338ea..0104bf7 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -46,14 +46,24 @@ describe("parseKeyValueFile", () => { expect(parseKeyValueFile("GITHUB_OUTPUT", "k=a< { - expect(() => parseKeyValueFile("GITHUB_ENV", "A=1\nnot an entry\n")).toThrow( - "GITHUB_ENV line 2", + it("rejects a malformed line, an empty key, and an unterminated heredoc, each naming the file and line and echoing none of it", () => { + expect(() => parseKeyValueFile("GITHUB_ENV", "A=1\nsecret-value-not-quoted\n")).toThrow( + "GITHUB_ENV line 2 is neither key=value nor a heredoc", + ); + expect(() => parseKeyValueFile("GITHUB_ENV", "=1\n")).toThrow( + "GITHUB_ENV line 1 has an empty key", ); - expect(() => parseKeyValueFile("GITHUB_ENV", "=1\n")).toThrow("GITHUB_ENV line 1"); expect(() => parseKeyValueFile("GITHUB_OUTPUT", "k< Date: Sun, 11 Oct 2026 20:56:34 +0100 Subject: [PATCH 6/9] fix(runner): mask a log command's annotation fields as its message --- packages/pipeline-local/src/runner.ts | 13 +++++- .../pipeline-local/test/runner.unit.test.ts | 40 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/packages/pipeline-local/src/runner.ts b/packages/pipeline-local/src/runner.ts index d956314..4348f3b 100644 --- a/packages/pipeline-local/src/runner.ts +++ b/packages/pipeline-local/src/runner.ts @@ -228,7 +228,18 @@ function processStdout(plan: JobPlan, state: JobRunState, step: Step, stdout: st message: applyMasks(parsed.command.message, state.masks), ...(parsed.command.annotation === undefined ? {} - : { annotation: parsed.command.annotation }), + : { + // The annotation's own strings can carry a masked value as the message can, so they go through the same masks. + annotation: { + ...parsed.command.annotation, + ...(parsed.command.annotation.file === undefined + ? {} + : { file: applyMasks(parsed.command.annotation.file, state.masks) }), + ...(parsed.command.annotation.title === undefined + ? {} + : { title: applyMasks(parsed.command.annotation.title, state.masks) }), + }, + }), }); continue; } diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 0104bf7..b3a6397 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -210,7 +210,11 @@ function recordedOutput(): RunOutputRecorder { return { lines, log(entry) { - lines.push(`${entry.level}:${entry.message}`); + const annotation = + entry.annotation === undefined + ? "" + : ` [${[entry.annotation.file, entry.annotation.title].filter((field) => field !== undefined).join(" ")}]`; + lines.push(`${entry.level}:${entry.message}${annotation}`); }, summary(summary) { lines.push(`summary:${summary.text}`); @@ -376,6 +380,40 @@ describe("the local runner's file commands", () => { } }); + it("masks the annotation's file and title as it masks the message", async () => { + const scratch = memoryScratch(); + const executor: CommandExecutor = { + async execute() { + return Promise.resolve({ + exitCode: 0, + stdout: "::add-mask::hunter2\n::error file=hunter2.ts,title=hunter2 token::leaked\n", + stderr: "", + }); + }, + }; + const output = recordedOutput(); + const pipeline = pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + build: { + name: "Build", + runsOn: ["x"], + steps: [{ kind: "run", run: "echo" }], + }, + }, + }); + const report = await createLocalRunner().run(pipeline, { + executor, + scratch, + output, + context: {}, + }); + + expect(report.ok).toBe(true); + expect(output.lines).toEqual(["error:leaked [***.ts *** token]"]); + }); + it("cleans the scratch when a run throws", async () => { const scratch = memoryScratch(); const executor: CommandExecutor = { From 9ce74eea3d61ccd407a4b2d9808b3ce949d63512 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 21:08:51 +0100 Subject: [PATCH 7/9] fix(runner): register each line of a multiline mask and quote no parser value --- packages/pipeline-local/src/file-commands.ts | 5 ++-- packages/pipeline-local/src/runner.ts | 3 +- .../pipeline-local/test/runner.unit.test.ts | 30 +++++++++++++++++++ 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/packages/pipeline-local/src/file-commands.ts b/packages/pipeline-local/src/file-commands.ts index b691487..e08eee1 100644 --- a/packages/pipeline-local/src/file-commands.ts +++ b/packages/pipeline-local/src/file-commands.ts @@ -44,7 +44,7 @@ export function parseKeyValueFile(fileName: string, content: string): FileEntry[ } const end = lines.indexOf(delimiter, offset + 1); if (end === -1) { - throw new Error(`${at} opens a heredoc for ${key} that no later line closes`); + throw new Error(`${at} opens a heredoc that no later line closes`); } entries.push({ key, value: lines.slice(offset + 1, end).join("\n") }); consumed = end + 1; @@ -129,8 +129,9 @@ function parseAnnotation(command: string, properties: string): Annotation | unde } else if (name === "title") { annotation.title = value; } else if (name === "line" || name === "endLine") { + // The value is never quoted in the message, because a property's value can carry a value a step masked. if (!/^\d+$/.test(value)) { - throw new Error(`::${command}:: property ${name} is not a line number: "${value}"`); + throw new Error(`::${command}:: property ${name} is not a line number`); } annotation[name] = Number(value); } else { diff --git a/packages/pipeline-local/src/runner.ts b/packages/pipeline-local/src/runner.ts index 4348f3b..02cbbe8 100644 --- a/packages/pipeline-local/src/runner.ts +++ b/packages/pipeline-local/src/runner.ts @@ -218,7 +218,8 @@ function processStdout(plan: JobPlan, state: JobRunState, step: Step, stdout: st const parsed = parseStdoutLine(line); if (parsed.kind === "command") { if (parsed.command.kind === "mask") { - state.masks.push(parsed.command.value); + // The runner's own command manager registers each decoded line of the value as its own mask, because output is filtered one line at a time. + for (const mask of parsed.command.value.split("\n")) state.masks.push(mask); continue; } plan.options.output.log({ diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index b3a6397..035111b 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -414,6 +414,36 @@ describe("the local runner's file commands", () => { expect(output.lines).toEqual(["error:leaked [***.ts *** token]"]); }); + it("registers each decoded line of a multiline mask", async () => { + const scratch = memoryScratch(); + const executor: CommandExecutor = { + async execute() { + return Promise.resolve({ + exitCode: 0, + stdout: "::add-mask::shh%0Atwo\nshh and two\n", + stderr: "", + }); + }, + }; + const output = recordedOutput(); + const pipeline = pipelineSchema.parse({ + name: "CI", + triggers: [{ kind: "push" }], + jobs: { + build: { + name: "Build", + runsOn: ["x"], + steps: [{ kind: "run", run: "echo" }], + }, + }, + }); + + await createLocalRunner().run(pipeline, { executor, scratch, output, context: {} }); + + // A mask of several lines hides each line of it in later output, which is filtered a line at a time. + expect(output.lines).toEqual(["info:*** and ***"]); + }); + it("cleans the scratch when a run throws", async () => { const scratch = memoryScratch(); const executor: CommandExecutor = { From 573e7a2e59ddf99589c7b63bb63ab687cd91f0c8 Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 21:28:40 +0100 Subject: [PATCH 8/9] fix(runner): quote no malformed property in an annotation error --- packages/pipeline-local/src/file-commands.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/pipeline-local/src/file-commands.ts b/packages/pipeline-local/src/file-commands.ts index e08eee1..ae576dc 100644 --- a/packages/pipeline-local/src/file-commands.ts +++ b/packages/pipeline-local/src/file-commands.ts @@ -119,8 +119,9 @@ function parseAnnotation(command: string, properties: string): Annotation | unde const annotation: { file?: string; line?: number; endLine?: number; title?: string } = {}; for (const part of properties.split(",")) { const separator = part.indexOf("="); + // The property's text is never quoted in the message, because a malformed property can carry a value a step masked. if (separator <= 0) { - throw new Error(`::${command}:: carries the property "${part}", which is not name=value`); + throw new Error(`::${command}:: carries a property that is not name=value`); } const name = part.slice(0, separator); const value = unescape(part.slice(separator + 1), PROPERTY_CODES); From 969426edb18dbe7ec7a36c8ec7ee57b023adb21e Mon Sep 17 00:00:00 2001 From: Joseph Mearman Date: Sun, 11 Oct 2026 21:30:02 +0100 Subject: [PATCH 9/9] fix(runner): quote no step-controlled name in a stream error --- .../pipeline-contract/src/runner-conformance.ts | 6 ++++-- packages/pipeline-local/src/file-commands.ts | 6 ++++-- packages/pipeline-local/test/runner.unit.test.ts | 13 +++++++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/pipeline-contract/src/runner-conformance.ts b/packages/pipeline-contract/src/runner-conformance.ts index 02da744..9245a45 100644 --- a/packages/pipeline-contract/src/runner-conformance.ts +++ b/packages/pipeline-contract/src/runner-conformance.ts @@ -560,7 +560,7 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli }, ); - api.it("rejects an unknown workflow command, naming it", async () => { + api.it("rejects an unknown workflow command without quoting it", async () => { const { scratch, output } = ports(); const executor = recordingExecutor(scratch, { script: () => ({ @@ -575,7 +575,9 @@ export function describeRunnerConformance(api: TestApi, makeRunner: () => Pipeli thrown = error; } check( - thrown instanceof Error && thrown.message.includes("::group::"), + thrown instanceof Error && + thrown.message.includes("unknown workflow command") && + !thrown.message.includes("group"), `the unknown command was rejected with ${String(thrown)}`, ); }); diff --git a/packages/pipeline-local/src/file-commands.ts b/packages/pipeline-local/src/file-commands.ts index ae576dc..a2e53ab 100644 --- a/packages/pipeline-local/src/file-commands.ts +++ b/packages/pipeline-local/src/file-commands.ts @@ -136,7 +136,8 @@ function parseAnnotation(command: string, properties: string): Annotation | unde } annotation[name] = Number(value); } else { - throw new Error(`::${command}:: carries the unknown property ${name}`); + // The property's name is step-controlled text, so it is not quoted: a step can put a masked value in it. + throw new Error(`::${command}:: carries an unknown property`); } } @@ -171,7 +172,8 @@ export function parseStdoutLine(line: string): StdoutLine { return { kind: "command", command: { kind: "mask", value: message } }; } const level = LOG_LEVELS.find((candidate) => candidate === name); - if (level === undefined) throw new Error(`unknown workflow command ::${name}::`); + // The command's name is step-controlled text, so it is not quoted: a step can put a masked value in it. + if (level === undefined) throw new Error("an unknown workflow command"); return { kind: "command", diff --git a/packages/pipeline-local/test/runner.unit.test.ts b/packages/pipeline-local/test/runner.unit.test.ts index 035111b..a254e57 100644 --- a/packages/pipeline-local/test/runner.unit.test.ts +++ b/packages/pipeline-local/test/runner.unit.test.ts @@ -109,12 +109,17 @@ describe("parseStdoutLine", () => { kind: "text", text: "::no closing marker", }); - expect(() => parseStdoutLine("::group::name")).toThrow("::group::"); + // The command's name is step-controlled text and is never quoted in the message. + expect(() => parseStdoutLine("::group::name")).toThrow("an unknown workflow command"); }); it("rejects a property outside the documented set and a line number that is not one", () => { - expect(() => parseStdoutLine("::warning column=3::m")).toThrow("column"); - expect(() => parseStdoutLine("::warning line=three::m")).toThrow("line"); + expect(() => parseStdoutLine("::warning column=3::m")).toThrow( + "::warning:: carries an unknown property", + ); + expect(() => parseStdoutLine("::warning line=three::m")).toThrow( + "property line is not a line number", + ); }); it("unescapes the codes the toolkit writes, in one pass so a literal code survives", () => { @@ -471,7 +476,7 @@ describe("the local runner's file commands", () => { output: recordedOutput(), context: {}, }), - ).rejects.toThrow("unknown workflow command"); + ).rejects.toThrow("an unknown workflow command"); expect(scratch.written.size).toBe(0); }); });