From 9832addd9c27c0b84f5cf4039c6348daed3d4465 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Mon, 26 Oct 2026 04:48:15 +0000 Subject: [PATCH 01/11] Read runs through the typed client and add the run core The API client gains startRun and getRun, parsed against the contract. The run core has no IO: the state of a run, the loop that starts and follows it through a clock port, the state of each node and the status sentences. A test holds the core to the same no-IO rule as the graph core. --- workflow-canvas/src/api/api.ts | 6 +- workflow-canvas/src/api/http-api.test.ts | 175 ++++++++++++++ workflow-canvas/src/api/http-api.ts | 26 ++- workflow-canvas/src/api/parse.ts | 46 ++++ workflow-canvas/src/api/types.ts | 21 ++ .../execution/core/execution-state.test.ts | 138 +++++++++++ .../src/execution/core/execution-state.ts | 76 +++++++ .../src/execution/core/follow-run.test.ts | 215 ++++++++++++++++++ .../src/execution/core/follow-run.ts | 46 ++++ .../src/execution/core/goal.test.ts | 29 +++ workflow-canvas/src/execution/core/goal.ts | 15 ++ workflow-canvas/src/execution/core/index.ts | 20 ++ .../src/execution/core/node-statuses.test.ts | 72 ++++++ .../src/execution/core/node-statuses.ts | 54 +++++ workflow-canvas/src/execution/core/ports.ts | 22 ++ .../src/execution/core/status-text.test.ts | 105 +++++++++ .../src/execution/core/status-text.ts | 63 +++++ workflow-canvas/src/graph/build-graph.ts | 6 +- workflow-canvas/src/graph/index.ts | 5 +- workflow-canvas/src/graph/model.ts | 16 +- workflow-canvas/src/graph/status.test.ts | 72 ++++++ workflow-canvas/src/graph/status.ts | 25 ++ .../src/test-support/fake-scheduler.ts | 39 ++++ workflow-canvas/src/test-support/fixtures.ts | 53 ++++- .../src/workflow/load-workflow.test.ts | 26 ++- workflow-canvas/src/workflow/load-workflow.ts | 21 +- .../test-support/execution-purity.test.ts | 50 ++++ 27 files changed, 1427 insertions(+), 15 deletions(-) create mode 100644 workflow-canvas/src/execution/core/execution-state.test.ts create mode 100644 workflow-canvas/src/execution/core/execution-state.ts create mode 100644 workflow-canvas/src/execution/core/follow-run.test.ts create mode 100644 workflow-canvas/src/execution/core/follow-run.ts create mode 100644 workflow-canvas/src/execution/core/goal.test.ts create mode 100644 workflow-canvas/src/execution/core/goal.ts create mode 100644 workflow-canvas/src/execution/core/index.ts create mode 100644 workflow-canvas/src/execution/core/node-statuses.test.ts create mode 100644 workflow-canvas/src/execution/core/node-statuses.ts create mode 100644 workflow-canvas/src/execution/core/ports.ts create mode 100644 workflow-canvas/src/execution/core/status-text.test.ts create mode 100644 workflow-canvas/src/execution/core/status-text.ts create mode 100644 workflow-canvas/src/graph/status.test.ts create mode 100644 workflow-canvas/src/graph/status.ts create mode 100644 workflow-canvas/src/test-support/fake-scheduler.ts create mode 100644 workflow-canvas/test-support/execution-purity.test.ts diff --git a/workflow-canvas/src/api/api.ts b/workflow-canvas/src/api/api.ts index 740cddfd8..dcabb7fa9 100644 --- a/workflow-canvas/src/api/api.ts +++ b/workflow-canvas/src/api/api.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -import type { Session, WorkflowDocument } from './types'; +import type { RunView, Session, WorkflowDocument } from './types'; /** * The calls of the workflow API that the page makes. The page depends on @@ -10,6 +10,10 @@ export interface WorkflowApi { createSession(code: string): Promise; /** Loads the root workflow of the session. */ getWorkflow(token: string): Promise; + /** Starts a run of the root workflow with the goal; resolves to its id. */ + startRun(token: string, goal: string): Promise; + /** Reads the state of a run, live or finished. */ + getRun(token: string, runId: string): Promise; } /** A call that failed, with a message that is fit to show to a person. */ diff --git a/workflow-canvas/src/api/http-api.test.ts b/workflow-canvas/src/api/http-api.test.ts index 07b203ed7..87c0d3754 100644 --- a/workflow-canvas/src/api/http-api.test.ts +++ b/workflow-canvas/src/api/http-api.test.ts @@ -24,6 +24,28 @@ const DOCUMENT_BODY = { diagnostics: [], }; +const RUN_STARTED_BODY = { run_id: '20261024-0412-a7b3c' }; + +const RUN_BODY = { + run: { + run_id: '20261024-0412-a7b3c', + workflow: 'hello', + harness: 'kiro-cli', + status: 'running', + started_at: '2026-10-24T04:12:05Z', + finished_at: null, + exit_code: null, + order: ['write', 'review'], + stages: { write: { status: 'passed' }, review: { status: 'running' } }, + future_field: true, + }, +}; + +// The run above with some fields replaced. +function run(changes: Record) { + return { run: { ...RUN_BODY.run, ...changes } }; +} + function reply(body: unknown, status = 200): Response { return new Response(JSON.stringify(body), { status }); } @@ -213,4 +235,157 @@ describe('createHttpApi', () => { await expect(api.getWorkflow('t')).rejects.toBeInstanceOf(ApiError); }); + + it('posts the goal as JSON to /api/run with the bearer token', async () => { + const { api, fetchFn } = apiFor(reply(RUN_STARTED_BODY, 202)); + + await api.startRun('t-1', 'Ship it'); + + const [path, init] = fetchFn.mock.calls[0] as unknown as [ + string, + RequestInit, + ]; + expect(path).toBe('/api/run'); + expect(init.method).toBe('POST'); + expect(init.headers).toEqual({ + Authorization: 'Bearer t-1', + 'Content-Type': 'application/json', + }); + expect(JSON.parse(init.body as string)).toEqual({ + params: { goal: 'Ship it' }, + }); + }); + + it('reads the id of the run that was accepted', async () => { + const { api } = apiFor(reply(RUN_STARTED_BODY, 202)); + + expect(await api.startRun('t', 'g')).toBe('20261024-0412-a7b3c'); + }); + + it('fails with the sentence of a refused run, such as one already active', async () => { + const { api } = apiFor( + reply( + { + error: 'A run is already active.', + code: 'already_running', + run_id: '20261024-0412-a7b3c', + }, + 409, + ), + ); + + await expect(api.startRun('t', 'g')).rejects.toMatchObject({ + message: 'A run is already active.', + status: 409, + code: 'already_running', + }); + }); + + it('gets /api/runs/{id} with the bearer token', async () => { + const { api, fetchFn } = apiFor(reply(RUN_BODY)); + + await api.getRun('t-1', '20261024-0412-a7b3c'); + + const [path, init] = fetchFn.mock.calls[0] as unknown as [ + string, + RequestInit, + ]; + expect(path).toBe('/api/runs/20261024-0412-a7b3c'); + expect(init.headers).toEqual({ Authorization: 'Bearer t-1' }); + }); + + it('encodes an id so it cannot change the path', async () => { + const { api, fetchFn } = apiFor(reply(RUN_BODY)); + + await api.getRun('t', '../workflow?x=1'); + + const [path] = fetchFn.mock.calls[0] as unknown as [string]; + expect(path).toBe('/api/runs/..%2Fworkflow%3Fx%3D1'); + }); + + it('reads a run that is running, with its stages in run order', async () => { + const { api } = apiFor(reply(RUN_BODY)); + + expect(await api.getRun('t', 'r')).toEqual({ + runId: '20261024-0412-a7b3c', + status: 'running', + exitCode: null, + stages: [ + { name: 'write', status: 'passed' }, + { name: 'review', status: 'running' }, + ], + }); + }); + + it('reads a run that halted, with the stage that was blocked', async () => { + const halted = { + run: { + ...RUN_BODY.run, + status: 'halted', + finished_at: '2026-10-24T04:12:49Z', + exit_code: 1, + stages: { write: { status: 'blocked' }, review: { status: 'pending' } }, + }, + }; + const { api } = apiFor(reply(halted)); + + expect(await api.getRun('t', 'r')).toMatchObject({ + status: 'halted', + exitCode: 1, + stages: [ + { name: 'write', status: 'blocked' }, + { name: 'review', status: 'pending' }, + ], + }); + }); + + it('takes the order from `order`, not from the keys of `stages`', async () => { + const swapped = { + run: { + ...RUN_BODY.run, + order: ['review', 'write'], + stages: RUN_BODY.run.stages, + }, + }; + const { api } = apiFor(reply(swapped)); + + expect((await api.getRun('t', 'r')).stages.map((s) => s.name)).toEqual([ + 'review', + 'write', + ]); + }); + + it('fails with the sentence of a run that is not there', async () => { + const { api } = apiFor( + reply({ error: 'There is no run with this id.', code: 'not_found' }, 404), + ); + + await expect(api.getRun('t', 'r')).rejects.toMatchObject({ + message: 'There is no run with this id.', + status: 404, + }); + }); + + it.each([ + ['a body with no run', {}], + ['a run with an unknown status', run({ status: 'paused' })], + ['a run with no run id', run({ run_id: undefined })], + ['an order that is not a list', run({ order: 'write' })], + ['a stage in `order` that `stages` lacks', run({ order: ['nope'] })], + [ + 'a stage with an unknown status', + run({ stages: { write: { status: 'skipped' }, review: {} } }), + ], + ['stages that are not an object', run({ stages: [] })], + ])('fails on %s', async (_, body) => { + const { api } = apiFor(reply(body)); + + await expect(api.getRun('t', 'r')).rejects.toBeInstanceOf(ApiError); + }); + + it('fails when the run is accepted with no id', async () => { + const { api } = apiFor(reply({}, 202)); + + await expect(api.startRun('t', 'g')).rejects.toBeInstanceOf(ApiError); + }); }); diff --git a/workflow-canvas/src/api/http-api.ts b/workflow-canvas/src/api/http-api.ts index 035fd60f9..a9ef52959 100644 --- a/workflow-canvas/src/api/http-api.ts +++ b/workflow-canvas/src/api/http-api.ts @@ -3,6 +3,8 @@ import { ApiError, type WorkflowApi } from './api'; import { errorCodeOf, errorMessageOf, + parseRunStarted, + parseRunStatus, parseSession, parseWorkflowDocument, } from './parse'; @@ -40,6 +42,10 @@ async function send( ); } +function bearer(token: string): Record { + return { Authorization: `Bearer ${token}` }; +} + /** * The adapter of the workflow API over HTTP. It calls the relative `/api`, so * the page and the server share one origin, and keeps no token itself. @@ -56,9 +62,27 @@ export function createHttpApi(fetchFn: Fetch = fetch): WorkflowApi { }, async getWorkflow(token) { const body = await send(fetchFn, '/api/workflow', { - headers: { Authorization: `Bearer ${token}` }, + headers: bearer(token), }); return parseWorkflowDocument(body); }, + async startRun(token, goal) { + const body = await send(fetchFn, '/api/run', { + method: 'POST', + headers: { ...bearer(token), 'Content-Type': 'application/json' }, + body: JSON.stringify({ params: { goal } }), + }); + return parseRunStarted(body); + }, + async getRun(token, runId) { + const body = await send( + fetchFn, + `/api/runs/${encodeURIComponent(runId)}`, + { + headers: bearer(token), + }, + ); + return parseRunStatus(body); + }, }; } diff --git a/workflow-canvas/src/api/parse.ts b/workflow-canvas/src/api/parse.ts index 56f99dc8d..a7f05a834 100644 --- a/workflow-canvas/src/api/parse.ts +++ b/workflow-canvas/src/api/parse.ts @@ -2,7 +2,11 @@ import { ApiError } from './api'; import type { DiagnosticView, + RunStageView, + RunStatus, + RunView, Session, + StageRunStatus, StageView, WorkflowDocument, WorkflowView, @@ -70,6 +74,48 @@ function parseDiagnostic(value: unknown): DiagnosticView { }; } +const RUN_STATUSES: readonly RunStatus[] = ['running', 'passed', 'halted']; +const STAGE_STATUSES: readonly StageRunStatus[] = [ + 'pending', + 'running', + 'passed', + 'blocked', +]; + +function oneOf(value: unknown, allowed: readonly T[]): T { + return allowed.find((candidate) => candidate === value) ?? fail(); +} + +// The stages in `order`, each with its status from `stages`. +function parseRunStages(order: unknown, stages: unknown): RunStageView[] { + const byName = objectOf(stages); + return arrayOf(order).map((entry) => { + const name = stringOf(entry); + const stage = Object.hasOwn(byName, name) ? byName[name] : fail(); + return { + name, + status: oneOf(objectOf(stage).status, STAGE_STATUSES), + }; + }); +} + +/** Reads a `RunStarted`: the id of the run that was accepted. */ +export function parseRunStarted(body: unknown): string { + return stringOf(objectOf(body).run_id); +} + +/** Reads a `RunStatusResponse`; unknown fields are ignored. */ +export function parseRunStatus(body: unknown): RunView { + const run = objectOf(objectOf(body).run); + const exitCode = run.exit_code; + return { + runId: stringOf(run.run_id), + status: oneOf(run.status, RUN_STATUSES), + exitCode: typeof exitCode === 'number' ? exitCode : null, + stages: parseRunStages(run.order, run.stages), + }; +} + /** Reads a `SessionResponse`; unknown fields are ignored. */ export function parseSession(body: unknown): Session { const session = objectOf(body); diff --git a/workflow-canvas/src/api/types.ts b/workflow-canvas/src/api/types.ts index 368e5ff28..a088d456f 100644 --- a/workflow-canvas/src/api/types.ts +++ b/workflow-canvas/src/api/types.ts @@ -39,3 +39,24 @@ export interface WorkflowDocument { view: WorkflowView | null; diagnostics: DiagnosticView[]; } + +/** How far a stage of a run has got. `blocked` failed and stopped the run. */ +export type StageRunStatus = 'pending' | 'running' | 'passed' | 'blocked'; + +/** How a run as a whole is going. `halted` means a stage failed. */ +export type RunStatus = 'running' | 'passed' | 'halted'; + +/** One stage of a run, in run order. */ +export interface RunStageView { + name: string; + status: StageRunStatus; +} + +/** What `GET /api/runs/{id}` returns: the state of a run, live or finished. */ +export interface RunView { + runId: string; + status: RunStatus; + /** `null` while the run is `running`. */ + exitCode: number | null; + stages: RunStageView[]; +} diff --git a/workflow-canvas/src/execution/core/execution-state.test.ts b/workflow-canvas/src/execution/core/execution-state.test.ts new file mode 100644 index 000000000..347e69eba --- /dev/null +++ b/workflow-canvas/src/execution/core/execution-state.test.ts @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { helloRun } from '../../test-support/fixtures'; +import { + IDLE, + isInFlight, + reduce, + runOf, + type ExecutionEvent, + type ExecutionState, +} from './execution-state'; + +const RUN_ID = helloRun(0).runId; + +function after(...events: ExecutionEvent[]): ExecutionState { + return events.reduce(reduce, IDLE); +} + +const STARTED: ExecutionEvent[] = [ + { type: 'start' }, + { type: 'started', runId: RUN_ID }, +]; + +describe('reduce', () => { + it('starts idle', () => { + expect(IDLE).toEqual({ phase: 'idle' }); + expect(isInFlight(IDLE)).toBe(false); + }); + + it('is starting once a run is asked for, then following once it has an id', () => { + expect(after({ type: 'start' })).toEqual({ phase: 'starting' }); + expect(after(...STARTED)).toEqual({ + phase: 'following', + runId: RUN_ID, + run: undefined, + }); + }); + + it('keeps the latest reading while the run is running', () => { + const state = after( + ...STARTED, + { type: 'snapshot', run: helloRun(0) }, + { type: 'snapshot', run: helloRun(1) }, + ); + + expect(state).toEqual({ + phase: 'following', + runId: RUN_ID, + run: helloRun(1), + }); + expect(runOf(state)).toEqual(helloRun(1)); + expect(isInFlight(state)).toBe(true); + }); + + it.each([ + ['passed', helloRun(2)], + ['halted', helloRun(0, { blockedAt: 0 })], + ])('finishes with a run that %s', (_, run) => { + const state = after(...STARTED, { type: 'snapshot', run }); + + expect(state).toEqual({ phase: 'finished', run }); + expect(isInFlight(state)).toBe(false); + expect(runOf(state)).toBe(run); + }); + + it('ignores a start while a run is in flight', () => { + const starting = after({ type: 'start' }); + const following = after(...STARTED); + + expect(reduce(starting, { type: 'start' })).toBe(starting); + expect(reduce(following, { type: 'start' })).toBe(following); + }); + + it('can start again after a run finished or failed', () => { + const finished = after(...STARTED, { + type: 'snapshot', + run: helloRun(2), + }); + const failed = after({ type: 'start' }, { type: 'failed', message: 'x' }); + + expect(reduce(finished, { type: 'start' })).toEqual({ phase: 'starting' }); + expect(reduce(failed, { type: 'start' })).toEqual({ phase: 'starting' }); + }); + + it('fails before the run started, or after, and says which', () => { + expect( + after({ type: 'start' }, { type: 'failed', message: 'No.' }), + ).toEqual({ + phase: 'error', + message: 'No.', + started: false, + }); + expect(after(...STARTED, { type: 'failed', message: 'Lost.' })).toEqual({ + phase: 'error', + message: 'Lost.', + started: true, + }); + }); + + it('shows no run in an error', () => { + const state = after( + ...STARTED, + { type: 'snapshot', run: helloRun(0) }, + { type: 'failed', message: 'Lost.' }, + ); + + expect(runOf(state)).toBeUndefined(); + }); + + it('ignores a reading of another run', () => { + const following = after(...STARTED); + const other = { ...helloRun(1), runId: '20261024-0412-zzzzz' }; + + expect(reduce(following, { type: 'snapshot', run: other })).toBe(following); + }); + + it.each<[string, ExecutionState, ExecutionEvent]>([ + ['a reading when idle', IDLE, { type: 'snapshot', run: helloRun(0) }], + ['an id when idle', IDLE, { type: 'started', runId: RUN_ID }], + ['a failure when idle', IDLE, { type: 'failed', message: 'x' }], + [ + 'an id when following', + after(...STARTED), + { type: 'started', runId: 'other' }, + ], + ])('ignores %s', (_, state, event) => { + expect(reduce(state, event)).toBe(state); + }); + + it('does not move on a failure after the run finished', () => { + const finished = after(...STARTED, { + type: 'snapshot', + run: helloRun(2), + }); + + expect(reduce(finished, { type: 'failed', message: 'x' })).toBe(finished); + }); +}); diff --git a/workflow-canvas/src/execution/core/execution-state.ts b/workflow-canvas/src/execution/core/execution-state.ts new file mode 100644 index 000000000..55b59fa2d --- /dev/null +++ b/workflow-canvas/src/execution/core/execution-state.ts @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { RunView } from '../../api/types'; + +/** Where the Execution tab is in the life of a run. */ +export type ExecutionState = + | { phase: 'idle' } + | { phase: 'starting' } + | { phase: 'following'; runId: string; run: RunView | undefined } + | { phase: 'finished'; run: RunView } + | { phase: 'error'; message: string; started: boolean }; + +/** Something that happened to the run, in the order it can. */ +export type ExecutionEvent = + | { type: 'start' } + | { type: 'started'; runId: string } + | { type: 'snapshot'; run: RunView } + | { type: 'failed'; message: string }; + +/** The state before any run has been started. */ +export const IDLE: ExecutionState = { phase: 'idle' }; + +/** Whether a run has been asked for and has not ended. */ +export function isInFlight(state: ExecutionState): boolean { + return state.phase === 'starting' || state.phase === 'following'; +} + +/** The run that the tab shows, if it has heard of one. */ +export function runOf(state: ExecutionState): RunView | undefined { + return state.phase === 'following' || state.phase === 'finished' + ? state.run + : undefined; +} + +function startOf(state: ExecutionState): ExecutionState { + return isInFlight(state) ? state : { phase: 'starting' }; +} + +function startedOf(state: ExecutionState, runId: string): ExecutionState { + return state.phase === 'starting' + ? { phase: 'following', runId, run: undefined } + : state; +} + +function snapshotOf(state: ExecutionState, run: RunView): ExecutionState { + if (state.phase !== 'following' || state.runId !== run.runId) return state; + return run.status === 'running' + ? { phase: 'following', runId: run.runId, run } + : { phase: 'finished', run }; +} + +function failedOf(state: ExecutionState, message: string): ExecutionState { + return isInFlight(state) + ? { phase: 'error', message, started: state.phase === 'following' } + : state; +} + +/** + * The next state after an event. An event that does not fit the state, such as + * a start while a run is in flight or a reply for an earlier run, changes + * nothing. + */ +export function reduce( + state: ExecutionState, + event: ExecutionEvent, +): ExecutionState { + switch (event.type) { + case 'start': + return startOf(state); + case 'started': + return startedOf(state, event.runId); + case 'snapshot': + return snapshotOf(state, event.run); + case 'failed': + return failedOf(state, event.message); + } +} diff --git a/workflow-canvas/src/execution/core/follow-run.test.ts b/workflow-canvas/src/execution/core/follow-run.test.ts new file mode 100644 index 000000000..652d3ce26 --- /dev/null +++ b/workflow-canvas/src/execution/core/follow-run.test.ts @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it, vi } from 'vitest'; +import type { RunView } from '../../api/types'; +import { FakeScheduler } from '../../test-support/fake-scheduler'; +import { helloRun } from '../../test-support/fixtures'; +import type { ExecutionEvent } from './execution-state'; +import { POLL_INTERVAL_MS, startAndFollow } from './follow-run'; +import type { RunPort } from './ports'; + +const RUN_ID = helloRun(0).runId; + +// Lets the loop run until it waits for the scheduler or ends. +async function settle(): Promise { + for (let turn = 0; turn < 20; turn += 1) await Promise.resolve(); +} + +function setup(readings: (RunView | Error)[], start?: RunPort['start']) { + const queue = [...readings]; + const runs: RunPort = { + start: start ?? vi.fn().mockResolvedValue(RUN_ID), + get: vi.fn(async () => { + const next = queue.shift(); + if (next === undefined) throw new Error('No reading left.'); + if (next instanceof Error) throw next; + return next; + }), + }; + const scheduler = new FakeScheduler(); + const events: ExecutionEvent[] = []; + const abort = new AbortController(); + const done = startAndFollow({ + runs, + scheduler, + goal: 'Ship it', + emit: (event) => events.push(event), + signal: abort.signal, + }); + return { runs, scheduler, events, abort, done }; +} + +describe('startAndFollow', () => { + it('starts the run with the goal, then reads it at once', async () => { + const { runs, events, done } = setup([helloRun(2)]); + + await done; + + expect(runs.start).toHaveBeenCalledWith('Ship it'); + expect(runs.get).toHaveBeenCalledWith(RUN_ID); + expect(events).toEqual([ + { type: 'started', runId: RUN_ID }, + { type: 'snapshot', run: helloRun(2) }, + ]); + }); + + it('reads again after each wait of a second, until the run is no longer running', async () => { + const { runs, scheduler, events, done } = setup([ + helloRun(0), + helloRun(1), + helloRun(2), + ]); + + await settle(); + expect(runs.get).toHaveBeenCalledTimes(1); + expect(scheduler.waits).toEqual([POLL_INTERVAL_MS]); + + scheduler.tick(); + await settle(); + expect(runs.get).toHaveBeenCalledTimes(2); + + scheduler.tick(); + await done; + + expect(runs.get).toHaveBeenCalledTimes(3); + expect(scheduler.waits).toEqual([POLL_INTERVAL_MS, POLL_INTERVAL_MS]); + expect(events.map((event) => event.type)).toEqual([ + 'started', + 'snapshot', + 'snapshot', + 'snapshot', + ]); + }); + + it('polls once a second', () => { + expect(POLL_INTERVAL_MS).toBe(1000); + }); + + it('stops after a halted run', async () => { + const halted = helloRun(0, { blockedAt: 0 }); + const { runs, scheduler, done } = setup([halted]); + + await done; + + expect(runs.get).toHaveBeenCalledTimes(1); + expect(scheduler.waits).toEqual([]); + }); + + it('does not wait before the first reading', async () => { + const { scheduler, done } = setup([helloRun(2)]); + + await done; + + expect(scheduler.waits).toEqual([]); + }); + + it('ends with one failed event when the run does not start', async () => { + const { runs, events, done } = setup( + [], + vi.fn().mockRejectedValue(new Error('A run is already active.')), + ); + + await done; + + expect(runs.get).not.toHaveBeenCalled(); + expect(events).toEqual([ + { type: 'failed', message: 'A run is already active.' }, + ]); + }); + + it('ends with one failed event when a reading fails, and reads no more', async () => { + const { runs, scheduler, events, done } = setup([ + helloRun(0), + new Error('The server did not answer.'), + ]); + + await settle(); + scheduler.tick(); + await done; + + expect(runs.get).toHaveBeenCalledTimes(2); + expect(events.map((event) => event.type)).toEqual([ + 'started', + 'snapshot', + 'failed', + ]); + expect(events.at(-1)).toEqual({ + type: 'failed', + message: 'The server did not answer.', + }); + }); + + it('gives a plain message for a failure that is not an Error', async () => { + const { events, done } = setup([], vi.fn().mockRejectedValue('boom')); + + await done; + + expect(events).toEqual([ + { type: 'failed', message: 'The run did not go as expected.' }, + ]); + }); + + it('emits nothing more once aborted while waiting', async () => { + const { runs, scheduler, events, abort, done } = setup([ + helloRun(0), + helloRun(1), + ]); + await settle(); + + abort.abort(); + await done; + + expect(scheduler.pendingCount).toBe(0); + expect(runs.get).toHaveBeenCalledTimes(1); + expect(events.map((event) => event.type)).toEqual(['started', 'snapshot']); + }); + + it('emits nothing when aborted before the run is accepted', async () => { + let accept: (runId: string) => void = () => {}; + const { runs, events, abort, done } = setup( + [], + () => new Promise((resolve) => (accept = resolve)), + ); + + abort.abort(); + accept(RUN_ID); + await done; + + expect(runs.get).not.toHaveBeenCalled(); + expect(events).toEqual([]); + }); + + it('emits no reading that arrives after an abort', async () => { + let arrive: (run: RunView) => void = () => {}; + const { scheduler, events, abort, done } = setup([]); + const runs: RunPort = { + start: async () => RUN_ID, + get: () => new Promise((resolve) => (arrive = resolve)), + }; + const late = startAndFollow({ + runs, + scheduler, + goal: 'Ship it', + emit: (event) => events.push(event), + signal: abort.signal, + }); + await settle(); + + abort.abort(); + arrive(helloRun(0)); + await Promise.all([done, late]); + + expect(events.map((event) => event.type)).not.toContain('snapshot'); + }); + + it('does not report a failure after it was aborted', async () => { + const { events, abort, done } = setup( + [], + vi.fn().mockRejectedValue(new Error('Late.')), + ); + abort.abort(); + + await done; + + expect(events).toEqual([]); + }); +}); diff --git a/workflow-canvas/src/execution/core/follow-run.ts b/workflow-canvas/src/execution/core/follow-run.ts new file mode 100644 index 000000000..66898a7b5 --- /dev/null +++ b/workflow-canvas/src/execution/core/follow-run.ts @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { ExecutionEvent } from './execution-state'; +import type { RunPort, Scheduler } from './ports'; + +/** How long to wait between two reads of a run, as the API asks. */ +export const POLL_INTERVAL_MS = 1000; + +const UNKNOWN_FAILURE = 'The run did not go as expected.'; + +function messageOf(error: unknown): string { + return error instanceof Error ? error.message : UNKNOWN_FAILURE; +} + +/** What `startAndFollow` needs: the ports, the goal and where events go. */ +export interface FollowRequest { + runs: RunPort; + scheduler: Scheduler; + goal: string; + emit: (event: ExecutionEvent) => void; + /** Aborting stops the loop quietly: no further event is emitted. */ + signal: AbortSignal; +} + +/** + * Starts a run, then reads it every `POLL_INTERVAL_MS` until it is no longer + * running, telling `emit` each step. It never rejects: a failed call ends the + * loop with one `failed` event. + */ +export async function startAndFollow(request: FollowRequest): Promise { + const { runs, scheduler, goal, emit, signal } = request; + try { + const runId = await runs.start(goal); + if (signal.aborted) return; + emit({ type: 'started', runId }); + for (;;) { + const run = await runs.get(runId); + if (signal.aborted) return; + emit({ type: 'snapshot', run }); + if (run.status !== 'running') return; + await scheduler.wait(POLL_INTERVAL_MS, signal); + if (signal.aborted) return; + } + } catch (error) { + if (!signal.aborted) emit({ type: 'failed', message: messageOf(error) }); + } +} diff --git a/workflow-canvas/src/execution/core/goal.test.ts b/workflow-canvas/src/execution/core/goal.test.ts new file mode 100644 index 000000000..f2523d62f --- /dev/null +++ b/workflow-canvas/src/execution/core/goal.test.ts @@ -0,0 +1,29 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { GOAL_MAX_LENGTH, goalProblem } from './goal'; + +describe('goalProblem', () => { + it('accepts a sentence', () => { + expect(goalProblem('Add a greeting to the README')).toBeUndefined(); + }); + + it.each(['', ' ', '\t\n'])('asks for a goal when it is %j', (goal) => { + expect(goalProblem(goal)).toBe('Enter a goal to run.'); + }); + + it('accepts a goal of the most characters', () => { + expect(goalProblem('x'.repeat(GOAL_MAX_LENGTH))).toBeUndefined(); + }); + + it('refuses a goal of one character more', () => { + expect(goalProblem('x'.repeat(GOAL_MAX_LENGTH + 1))).toBe( + 'The goal can have at most 2048 characters.', + ); + }); + + it('counts a character outside the basic plane once, as the API does', () => { + // Each of these is two UTF-16 units but one character. + expect(goalProblem('😀'.repeat(GOAL_MAX_LENGTH))).toBeUndefined(); + expect(goalProblem('😀'.repeat(GOAL_MAX_LENGTH + 1))).toBeDefined(); + }); +}); diff --git a/workflow-canvas/src/execution/core/goal.ts b/workflow-canvas/src/execution/core/goal.ts new file mode 100644 index 000000000..e865a10b6 --- /dev/null +++ b/workflow-canvas/src/execution/core/goal.ts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: Apache-2.0 + +/** The most characters a goal may have; the API refuses a longer one. */ +export const GOAL_MAX_LENGTH = 2048; + +/** + * Why the goal cannot be sent, as a sentence for a person, or `undefined` + * when it can. Length counts characters, not UTF-16 units, as the API does. + */ +export function goalProblem(goal: string): string | undefined { + if (goal.trim() === '') return 'Enter a goal to run.'; + return [...goal].length > GOAL_MAX_LENGTH + ? `The goal can have at most ${GOAL_MAX_LENGTH} characters.` + : undefined; +} diff --git a/workflow-canvas/src/execution/core/index.ts b/workflow-canvas/src/execution/core/index.ts new file mode 100644 index 000000000..5958c13a6 --- /dev/null +++ b/workflow-canvas/src/execution/core/index.ts @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: Apache-2.0 +/** + * The core of the Execution tab: the state of a run, the loop that follows + * it, and the words and node states drawn from it. It does no IO; the calls + * of the API and the time come in through its ports. + * @module + */ +export { + IDLE, + isInFlight, + reduce, + runOf, + type ExecutionEvent, + type ExecutionState, +} from './execution-state'; +export { POLL_INTERVAL_MS, startAndFollow } from './follow-run'; +export { GOAL_MAX_LENGTH, goalProblem } from './goal'; +export { nodeStatuses } from './node-statuses'; +export type { RunPort, Scheduler } from './ports'; +export { statusKind, statusText, type StatusKind } from './status-text'; diff --git a/workflow-canvas/src/execution/core/node-statuses.test.ts b/workflow-canvas/src/execution/core/node-statuses.test.ts new file mode 100644 index 000000000..a82aec7b0 --- /dev/null +++ b/workflow-canvas/src/execution/core/node-statuses.test.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { helloRun } from '../../test-support/fixtures'; +import { nodeStatuses } from './node-statuses'; + +function labels(run: Parameters[0]) { + return [...nodeStatuses(run)].map(([id, { state, label }]) => [ + id, + state, + label, + ]); +} + +describe('nodeStatuses', () => { + it('gives no node a status without a run', () => { + expect(nodeStatuses(undefined).size).toBe(0); + }); + + it('has the intent done, the first stage running, the rest and the end queued', () => { + expect(labels(helloRun(0))).toEqual([ + ['intent', 'done', 'Done'], + ['stage-draft', 'running', 'Running'], + ['stage-review', 'queued', 'Queued'], + ['end', 'queued', 'Queued'], + ]); + }); + + it('moves on to the next stage once the first has passed', () => { + expect(labels(helloRun(1))).toEqual([ + ['intent', 'done', 'Done'], + ['stage-draft', 'done', 'Done'], + ['stage-review', 'running', 'Running'], + ['end', 'queued', 'Queued'], + ]); + }); + + it('shows the end as passed when every stage passed', () => { + expect(labels(helloRun(2))).toEqual([ + ['intent', 'done', 'Done'], + ['stage-draft', 'done', 'Done'], + ['stage-review', 'done', 'Done'], + ['end', 'done', 'Passed'], + ]); + }); + + it('shows the end as halted, the failed stage as failed and the rest as not run', () => { + expect(labels(helloRun(0, { blockedAt: 0 }))).toEqual([ + ['intent', 'done', 'Done'], + ['stage-draft', 'failed', 'Failed'], + ['stage-review', 'queued', 'Not run'], + ['end', 'failed', 'Halted'], + ]); + }); + + it('keeps the stages that passed before the failed one', () => { + expect(labels(helloRun(0, { blockedAt: 1 }))).toEqual([ + ['intent', 'done', 'Done'], + ['stage-draft', 'done', 'Done'], + ['stage-review', 'failed', 'Failed'], + ['end', 'failed', 'Halted'], + ]); + }); + + it('names a stage by the id the graph gives it', () => { + const run = { + ...helloRun(0), + stages: [{ name: 'a-b', status: 'running' as const }], + }; + + expect(nodeStatuses(run).get('stage-a-b')?.state).toBe('running'); + }); +}); diff --git a/workflow-canvas/src/execution/core/node-statuses.ts b/workflow-canvas/src/execution/core/node-statuses.ts new file mode 100644 index 000000000..73dc644c9 --- /dev/null +++ b/workflow-canvas/src/execution/core/node-statuses.ts @@ -0,0 +1,54 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { RunStageView, RunView } from '../../api/types'; +import { END_ID, INTENT_ID, stageNodeId, type NodeStatus } from '../../graph'; + +const QUEUED: NodeStatus = { state: 'queued', label: 'Queued' }; +const RUNNING: NodeStatus = { state: 'running', label: 'Running' }; +const DONE: NodeStatus = { state: 'done', label: 'Done' }; +const FAILED: NodeStatus = { state: 'failed', label: 'Failed' }; +const NOT_RUN: NodeStatus = { state: 'queued', label: 'Not run' }; + +function stageStatus(stage: RunStageView, run: RunView): NodeStatus { + switch (stage.status) { + case 'pending': + // After a halt the stages behind the failed one will not start. + return run.status === 'halted' ? NOT_RUN : QUEUED; + case 'running': + return RUNNING; + case 'passed': + return DONE; + case 'blocked': + return FAILED; + } +} + +// The end shows the result of the run once there is one. +function endStatus(run: RunView): NodeStatus { + switch (run.status) { + case 'running': + return QUEUED; + case 'passed': + return { state: 'done', label: 'Passed' }; + case 'halted': + return { state: 'failed', label: 'Halted' }; + } +} + +/** + * The status of each node of the graph for a run, by node id: the intent is + * done once the run has it, each stage follows its stage of the run, and the + * end shows the result. Without a run no node has a status. + */ +export function nodeStatuses( + run: RunView | undefined, +): ReadonlyMap { + if (!run) return new Map(); + return new Map([ + [INTENT_ID, DONE], + ...run.stages.map((stage): [string, NodeStatus] => [ + stageNodeId(stage.name), + stageStatus(stage, run), + ]), + [END_ID, endStatus(run)], + ]); +} diff --git a/workflow-canvas/src/execution/core/ports.ts b/workflow-canvas/src/execution/core/ports.ts new file mode 100644 index 000000000..34a40b804 --- /dev/null +++ b/workflow-canvas/src/execution/core/ports.ts @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { RunView } from '../../api/types'; + +/** + * The calls of the API that a run needs, already tied to a session. The + * page depends on this port; `workflow/load-workflow.ts` builds the adapter. + */ +export interface RunPort { + /** Starts a run with the goal and resolves to its id. */ + start(goal: string): Promise; + /** Reads the state of the run with the id. */ + get(runId: string): Promise; +} + +/** + * The only source of time for the core, so a test can move it by hand. The + * timer adapter is `execution/timer-scheduler.ts`. + */ +export interface Scheduler { + /** Resolves after `ms` milliseconds, or at once when the signal aborts. */ + wait(ms: number, signal: AbortSignal): Promise; +} diff --git a/workflow-canvas/src/execution/core/status-text.test.ts b/workflow-canvas/src/execution/core/status-text.test.ts new file mode 100644 index 000000000..2736540d0 --- /dev/null +++ b/workflow-canvas/src/execution/core/status-text.test.ts @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { helloRun } from '../../test-support/fixtures'; +import { IDLE, reduce, type ExecutionState } from './execution-state'; +import { statusKind, statusText } from './status-text'; + +const STARTING: ExecutionState = { phase: 'starting' }; + +function following(run?: ReturnType): ExecutionState { + return { phase: 'following', runId: helloRun(0).runId, run }; +} + +describe('statusText and statusKind', () => { + it.each<[string, ExecutionState, string, string]>([ + ['idle', IDLE, 'Ready. Press Run to start.', 'idle'], + ['starting', STARTING, 'Starting the run.', 'busy'], + [ + 'following with no reading yet', + following(), + 'Waiting for the first status of the run.', + 'busy', + ], + [ + 'the first stage running', + following(helloRun(0)), + 'Running draft. 0 of 2 stages done.', + 'busy', + ], + [ + 'the second stage running', + following(helloRun(1)), + 'Running review. 1 of 2 stages done.', + 'busy', + ], + [ + 'a run that passed', + { phase: 'finished', run: helloRun(2) }, + 'Run passed. 2 of 2 stages done. Exit code 0.', + 'success', + ], + [ + 'a run that halted', + { phase: 'finished', run: helloRun(0, { blockedAt: 1 }) }, + 'Run halted at review. 1 of 2 stages done. Exit code 1.', + 'error', + ], + [ + 'an error before the run started', + { phase: 'error', message: 'x', started: false }, + 'The run did not start.', + 'error', + ], + [ + 'an error after the run started', + { phase: 'error', message: 'x', started: true }, + 'Stopped following the run.', + 'error', + ], + ])('says %s', (_, state, text, kind) => { + expect(statusText(state)).toBe(text); + expect(statusKind(state)).toBe(kind); + }); + + it('says it is waiting between two stages', () => { + const between = { + ...helloRun(0), + stages: [ + { name: 'draft', status: 'passed' as const }, + { name: 'review', status: 'pending' as const }, + ], + }; + + expect(statusText(following(between))).toBe( + 'Waiting for the next stage. 1 of 2 stages done.', + ); + }); + + it('leaves out the exit code when the reply has none', () => { + const run = { ...helloRun(2), exitCode: null }; + + expect(statusText({ phase: 'finished', run })).toBe( + 'Run passed. 2 of 2 stages done.', + ); + }); + + it('does not name a stage when a halted run has no failed one', () => { + const run = { ...helloRun(0, { blockedAt: 0 }), stages: [] }; + + expect(statusText({ phase: 'finished', run })).toBe( + 'Run halted. 0 of 0 stages done. Exit code 1.', + ); + }); + + it('follows a whole run from the first click to the end', () => { + const states = [ + reduce(IDLE, { type: 'start' }), + reduce(STARTING, { type: 'started', runId: helloRun(0).runId }), + ]; + + expect(states.map(statusText)).toEqual([ + 'Starting the run.', + 'Waiting for the first status of the run.', + ]); + }); +}); diff --git a/workflow-canvas/src/execution/core/status-text.ts b/workflow-canvas/src/execution/core/status-text.ts new file mode 100644 index 000000000..a59f04721 --- /dev/null +++ b/workflow-canvas/src/execution/core/status-text.ts @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { RunView } from '../../api/types'; +import type { ExecutionState } from './execution-state'; + +/** How the status line is drawn: at rest, at work, passed or not working. */ +export type StatusKind = 'idle' | 'busy' | 'success' | 'error'; + +function stagesDone(run: RunView): string { + const done = run.stages.filter((stage) => stage.status === 'passed').length; + return `${done} of ${run.stages.length} stages done.`; +} + +function followingText(run: RunView | undefined): string { + if (!run) return 'Waiting for the first status of the run.'; + const current = run.stages.find((stage) => stage.status === 'running'); + const lead = current + ? `Running ${current.name}.` + : 'Waiting for the next stage.'; + return `${lead} ${stagesDone(run)}`; +} + +function finishedText(run: RunView): string { + const exit = run.exitCode === null ? '' : ` Exit code ${run.exitCode}.`; + const blocked = run.stages.find((stage) => stage.status === 'blocked'); + const lead = + run.status === 'passed' + ? 'Run passed.' + : `Run halted${blocked ? ` at ${blocked.name}` : ''}.`; + return `${lead} ${stagesDone(run)}${exit}`; +} + +/** The sentence that says where the run is. A screen reader announces it. */ +export function statusText(state: ExecutionState): string { + switch (state.phase) { + case 'idle': + return 'Ready. Press Run to start.'; + case 'starting': + return 'Starting the run.'; + case 'following': + return followingText(state.run); + case 'finished': + return finishedText(state.run); + case 'error': + return state.started + ? 'Stopped following the run.' + : 'The run did not start.'; + } +} + +/** What kind of status the sentence is, so it can be drawn to match. */ +export function statusKind(state: ExecutionState): StatusKind { + switch (state.phase) { + case 'idle': + return 'idle'; + case 'starting': + case 'following': + return 'busy'; + case 'finished': + return state.run.status === 'passed' ? 'success' : 'error'; + case 'error': + return 'error'; + } +} diff --git a/workflow-canvas/src/graph/build-graph.ts b/workflow-canvas/src/graph/build-graph.ts index 19b901685..580116913 100644 --- a/workflow-canvas/src/graph/build-graph.ts +++ b/workflow-canvas/src/graph/build-graph.ts @@ -14,8 +14,10 @@ import { type WorkflowSpec, } from './model'; -const INTENT_ID = 'intent'; -const END_ID = 'end'; +/** The id of the node that draws the intent. */ +export const INTENT_ID = 'intent'; +/** The id of the node that draws the end. */ +export const END_ID = 'end'; /** The id of the node that draws the stage called `name`. */ export function stageNodeId(name: string): string { diff --git a/workflow-canvas/src/graph/index.ts b/workflow-canvas/src/graph/index.ts index fae7da231..e0bb4d046 100644 --- a/workflow-canvas/src/graph/index.ts +++ b/workflow-canvas/src/graph/index.ts @@ -1,13 +1,16 @@ // SPDX-License-Identifier: Apache-2.0 -export { buildGraph, stageNodeId } from './build-graph'; +export { END_ID, INTENT_ID, buildGraph, stageNodeId } from './build-graph'; export { NODE_HEIGHT, NODE_WIDTH } from './layout'; export { SEQUENCE_IN, SEQUENCE_OUT } from './model'; +export { withStatuses } from './status'; export type { CanvasGraph, EndGraphNode, GraphEdge, GraphNode, IntentGraphNode, + NodeState, + NodeStatus, StageGraphNode, WorkflowSpec, } from './model'; diff --git a/workflow-canvas/src/graph/model.ts b/workflow-canvas/src/graph/model.ts index 99546241a..b4735ba69 100644 --- a/workflow-canvas/src/graph/model.ts +++ b/workflow-canvas/src/graph/model.ts @@ -7,6 +7,16 @@ export interface WorkflowSpec { stages: readonly { name: string; agent: string }[]; } +/** How far a node has got in a run. */ +export type NodeState = 'queued' | 'running' | 'done' | 'failed'; + +/** The live state of a node in a run, with the words that say it. */ +export interface NodeStatus { + state: NodeState; + /** The text shown on the node and read by a screen reader. */ + label: string; +} + interface Placed { id: string; /** What a screen reader says for the node. */ @@ -19,19 +29,19 @@ interface Placed { /** The start of the workflow: what it is meant to achieve. */ export type IntentGraphNode = Placed & { type: 'intent'; - data: { outcome: string }; + data: { outcome: string; status?: NodeStatus }; }; /** One unit of work, done by one agent on one harness. */ export type StageGraphNode = Placed & { type: 'stage'; - data: { name: string; agent: string; harness: string }; + data: { name: string; agent: string; harness: string; status?: NodeStatus }; }; /** The finish of the workflow. */ export type EndGraphNode = Placed & { type: 'end'; - data: Record; + data: { status?: NodeStatus }; }; export type GraphNode = IntentGraphNode | StageGraphNode | EndGraphNode; diff --git a/workflow-canvas/src/graph/status.test.ts b/workflow-canvas/src/graph/status.test.ts new file mode 100644 index 000000000..bd564974f --- /dev/null +++ b/workflow-canvas/src/graph/status.test.ts @@ -0,0 +1,72 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { buildGraph } from './build-graph'; +import type { NodeStatus, WorkflowSpec } from './model'; +import { withStatuses } from './status'; + +const HELLO: WorkflowSpec = { + name: 'hello', + intent: { outcome: 'Add a greeting to the README' }, + stages: [ + { name: 'draft', agent: 'k-developer' }, + { name: 'review', agent: 'k-quality-assurance' }, + ], +}; + +const RUNNING: NodeStatus = { state: 'running', label: 'Running' }; +const PASSED: NodeStatus = { state: 'done', label: 'Passed' }; + +describe('withStatuses', () => { + const graph = buildGraph(HELLO, 'kiro-cli'); + + it('puts the status in the data of the node it names', () => { + const { nodes } = withStatuses(graph, new Map([['stage-draft', RUNNING]])); + + expect(nodes.find((node) => node.id === 'stage-draft')?.data).toEqual({ + name: 'draft', + agent: 'k-developer', + harness: 'kiro-cli', + status: RUNNING, + }); + }); + + it('says the status in the name a screen reader reads', () => { + const { nodes } = withStatuses( + graph, + new Map([ + ['stage-draft', RUNNING], + ['end', PASSED], + ]), + ); + + expect(nodes.map((node) => node.ariaLabel)).toEqual([ + 'Intent: Add a greeting to the README', + 'Stage draft, agent k-developer, harness kiro-cli, running', + 'Stage review, agent k-quality-assurance, harness kiro-cli', + 'End, passed', + ]); + }); + + it('leaves the other nodes, the edges and the positions as they were', () => { + const next = withStatuses(graph, new Map([['end', PASSED]])); + + expect(next.edges).toBe(graph.edges); + expect(next.nodes[0]).toBe(graph.nodes[0]); + expect(next.nodes.map((node) => node.position)).toEqual( + graph.nodes.map((node) => node.position), + ); + }); + + it('does not change the graph it is given', () => { + withStatuses(graph, new Map([['end', PASSED]])); + + expect(graph.nodes.at(-1)?.data).toEqual({}); + expect(graph.nodes.at(-1)?.ariaLabel).toBe('End'); + }); + + it('ignores an id that no node has', () => { + const next = withStatuses(graph, new Map([['stage-nope', RUNNING]])); + + expect(next.nodes).toEqual(graph.nodes); + }); +}); diff --git a/workflow-canvas/src/graph/status.ts b/workflow-canvas/src/graph/status.ts new file mode 100644 index 000000000..dc60debd2 --- /dev/null +++ b/workflow-canvas/src/graph/status.ts @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { CanvasGraph, GraphNode, NodeStatus } from './model'; + +function withStatus(node: N, status?: NodeStatus): N { + if (!status) return node; + return { + ...node, + ariaLabel: `${node.ariaLabel}, ${status.label.toLowerCase()}`, + data: { ...node.data, status }, + }; +} + +/** + * Gives each node the status that `statuses` holds for its id, in its data + * and in its name for a screen reader. Nodes without an entry are unchanged. + */ +export function withStatuses( + graph: CanvasGraph, + statuses: ReadonlyMap, +): CanvasGraph { + return { + ...graph, + nodes: graph.nodes.map((node) => withStatus(node, statuses.get(node.id))), + }; +} diff --git a/workflow-canvas/src/test-support/fake-scheduler.ts b/workflow-canvas/src/test-support/fake-scheduler.ts new file mode 100644 index 000000000..47b470147 --- /dev/null +++ b/workflow-canvas/src/test-support/fake-scheduler.ts @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { Scheduler } from '../execution/core'; + +/** + * A scheduler that waits until a test says so. `tick()` ends every wait that + * is pending, as if its time had come; aborting a wait ends it too. + */ +export class FakeScheduler implements Scheduler { + /** The `ms` of each wait asked for, in order. */ + readonly waits: number[] = []; + private pending = new Set<() => void>(); + + wait(ms: number, signal: AbortSignal): Promise { + this.waits.push(ms); + return new Promise((resolve) => { + if (signal.aborted) { + resolve(); + return; + } + this.pending.add(resolve); + signal.addEventListener('abort', () => { + this.pending.delete(resolve); + resolve(); + }); + }); + } + + /** How many waits are pending. */ + get pendingCount(): number { + return this.pending.size; + } + + /** Ends the pending waits. */ + tick(): void { + const ending = [...this.pending]; + this.pending.clear(); + for (const end of ending) end(); + } +} diff --git a/workflow-canvas/src/test-support/fixtures.ts b/workflow-canvas/src/test-support/fixtures.ts index d21ee6222..5a3d64cd1 100644 --- a/workflow-canvas/src/test-support/fixtures.ts +++ b/workflow-canvas/src/test-support/fixtures.ts @@ -1,5 +1,11 @@ // SPDX-License-Identifier: Apache-2.0 -import type { Session, WorkflowDocument, WorkflowView } from '../api/types'; +import type { + RunView, + Session, + WorkflowDocument, + WorkflowView, +} from '../api/types'; +import type { RunPort } from '../execution/core'; import type { LoadResult } from '../workflow/load-workflow'; /** The workflow that the stub serves: a draft stage, then a review stage. */ @@ -28,15 +34,56 @@ export const HELLO_DOCUMENT: WorkflowDocument = { diagnostics: [], }; +/** Run calls that are never used, for a test that does not run anything. */ +export const NO_RUNS: RunPort = { + start: () => Promise.reject(new Error('This test starts no run.')), + get: () => Promise.reject(new Error('This test reads no run.')), +}; + /** A load that succeeded with the document. */ -export function loaded(document: WorkflowDocument): LoadResult { +export function loaded( + document: WorkflowDocument, + runs: RunPort = NO_RUNS, +): LoadResult { return { ok: true, - workflow: { harness: 'kiro-cli', document }, + workflow: { harness: 'kiro-cli', document, runs }, remember: () => undefined, }; } +const HELLO_STAGES = HELLO_VIEW.stages.map((stage) => stage.name); + +/** + * The state of a run of the workflow above: stages before `at` have passed, + * the one at `at` runs and the ones after it wait. An `at` past the last + * stage is a run that passed. With `blockedAt`, that stage failed instead + * and the run halted. + */ +export function helloRun( + at: number, + { blockedAt }: { blockedAt?: number } = {}, +): RunView { + const halted = blockedAt !== undefined; + const finished = halted || at >= HELLO_STAGES.length; + return { + runId: '20261024-0412-a7b3c', + status: halted ? 'halted' : finished ? 'passed' : 'running', + exitCode: halted ? 1 : finished ? 0 : null, + stages: HELLO_STAGES.map((name, index) => ({ + name, + status: + index === blockedAt + ? 'blocked' + : index < (blockedAt ?? at) + ? 'passed' + : index === at && !halted + ? 'running' + : 'pending', + })), + }; +} + /** A load that failed with the message. */ export function failed(message: string): LoadResult { return { ok: false, message }; diff --git a/workflow-canvas/src/workflow/load-workflow.test.ts b/workflow-canvas/src/workflow/load-workflow.test.ts index 20dda9fd1..e7515efde 100644 --- a/workflow-canvas/src/workflow/load-workflow.test.ts +++ b/workflow-canvas/src/workflow/load-workflow.test.ts @@ -1,8 +1,12 @@ // SPDX-License-Identifier: Apache-2.0 import { describe, expect, it, vi } from 'vitest'; import { ApiError, type WorkflowApi } from '../api/api'; -import { HELLO_DOCUMENT, HELLO_SESSION } from '../test-support/fixtures'; import { sessionStorageMemory } from '../launch/session-memory'; +import { + HELLO_DOCUMENT, + HELLO_SESSION, + helloRun, +} from '../test-support/fixtures'; import { freshMemory, memoryStorage } from '../test-support/memory-storage'; import { loadOnOpen, loadWorkflow } from './load-workflow'; @@ -10,6 +14,8 @@ function fakeApi(overrides: Partial = {}): WorkflowApi { return { createSession: vi.fn().mockResolvedValue(HELLO_SESSION), getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), + startRun: vi.fn().mockResolvedValue('20261024-0412-a7b3c'), + getRun: vi.fn().mockResolvedValue(helloRun(0)), ...overrides, }; } @@ -24,11 +30,27 @@ describe('loadWorkflow', () => { expect(api.getWorkflow).toHaveBeenCalledWith('token-1'); expect(result).toEqual({ ok: true, - workflow: { harness: 'kiro-cli', document: HELLO_DOCUMENT }, + workflow: { + harness: 'kiro-cli', + document: HELLO_DOCUMENT, + runs: { start: expect.any(Function), get: expect.any(Function) }, + }, remember: expect.any(Function), }); }); + it('ties the run calls to the session token', async () => { + const api = fakeApi(); + const result = await loadWorkflow(api, 'code-1'); + if (!result.ok) throw new Error('The load failed.'); + + const runId = await result.workflow.runs.start('Ship it'); + await result.workflow.runs.get(runId); + + expect(api.startRun).toHaveBeenCalledWith('token-1', 'Ship it'); + expect(api.getRun).toHaveBeenCalledWith('token-1', runId); + }); + it('fails without calling the API when there is no launch code', async () => { const api = fakeApi(); diff --git a/workflow-canvas/src/workflow/load-workflow.ts b/workflow-canvas/src/workflow/load-workflow.ts index d9138d62e..2fed62165 100644 --- a/workflow-canvas/src/workflow/load-workflow.ts +++ b/workflow-canvas/src/workflow/load-workflow.ts @@ -1,12 +1,18 @@ // SPDX-License-Identifier: Apache-2.0 import { ApiError, type WorkflowApi } from '../api/api'; import type { WorkflowDocument } from '../api/types'; +import type { RunPort } from '../execution/core'; import type { SessionMemory, SessionRecord } from '../launch/session-memory'; -/** A workflow loaded from the server, with the harness that runs its stages. */ +/** + * A workflow loaded from the server, with the harness that runs its stages + * and the calls that run it. The calls hold the session token, so no other + * part of the page does. + */ export interface LoadedWorkflow { harness: string; document: WorkflowDocument; + runs: RunPort; } /** @@ -68,6 +74,13 @@ function guardSession( }; } +function runPortFor(api: WorkflowApi, token: string): RunPort { + return { + start: (goal) => api.startRun(token, goal), + get: (runId) => api.getRun(token, runId), + }; +} + async function loadWith( api: WorkflowApi, session: SessionRecord, @@ -77,7 +90,11 @@ async function loadWith( const document = await guard(api.getWorkflow(session.token)); return { ok: true, - workflow: { harness: session.harness, document }, + workflow: { + harness: session.harness, + document, + runs: runPortFor(api, session.token), + }, remember: () => memory.remember(session), }; } diff --git a/workflow-canvas/test-support/execution-purity.test.ts b/workflow-canvas/test-support/execution-purity.test.ts new file mode 100644 index 000000000..62bc1d448 --- /dev/null +++ b/workflow-canvas/test-support/execution-purity.test.ts @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: Apache-2.0 +/** + * Checks that the run core does no IO and imports no UI: it may import its + * own modules, the graph core and the wire types, and nothing else. Time and + * the calls of the API come in through its ports. + */ +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { + problemsIn, + problemsUnder, + sourcesUnder, + type PurityRules, +} from './purity.ts'; + +const RULES: PurityRules = { + dir: fileURLToPath(new URL('../src/execution/core/', import.meta.url)), + allowedPackages: new Set(['../../graph', '../../api/types']), + allowedNames: {}, +}; + +describe('the run core', () => { + it('has sources to check', () => { + expect(sourcesUnder(RULES.dir).length).toBeGreaterThanOrEqual(6); + }); + + it('imports only its own modules, the graph core and the wire types, and reads no clock, randomness, environment or network', () => { + expect(problemsUnder(RULES)).toEqual([]); + }); + + it.each([ + ['react', "import { useState } from 'react';"], + ['the HTTP adapter', "import { createHttpApi } from '../../api/http-api';"], + [ + 'the timer adapter', + "import { timerScheduler } from '../timer-scheduler';", + ], + ['a Node module', "import { readFileSync } from 'node:fs';"], + ])('rejects an import of %s', (_, line) => { + expect(problemsIn('x.ts', line, RULES)).not.toEqual([]); + }); + + it.each([ + ['a timer', 'setTimeout(() => {}, 1000);'], + ['the clock', 'const now = Date.now();'], + ['a network call', "await fetch('/api/run');"], + ])('rejects %s', (_, line) => { + expect(problemsIn('x.ts', line, RULES)).not.toEqual([]); + }); +}); From 84f3bddc869138f19548d0131996ef8d5b90ddae Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:54:39 +0000 Subject: [PATCH 02/11] Keep the run_id of a 409 and reject a run reply that breaks the contract ApiError carries the run_id of the error body, so a caller can find the run that is already active. A run reply with an exit_code that is not an integer or null, or with a stage name twice in order, is a bad reply instead of being read as null or as two stages of one name. --- workflow-canvas/src/api/api.ts | 5 ++++- workflow-canvas/src/api/http-api.test.ts | 21 +++++++++++++++++++++ workflow-canvas/src/api/http-api.ts | 2 ++ workflow-canvas/src/api/parse.ts | 23 ++++++++++++++++++++--- 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/workflow-canvas/src/api/api.ts b/workflow-canvas/src/api/api.ts index dcabb7fa9..90e06e97c 100644 --- a/workflow-canvas/src/api/api.ts +++ b/workflow-canvas/src/api/api.ts @@ -22,11 +22,14 @@ export class ApiError extends Error { readonly status: number; /** The `code` of the error body, when there was one. */ readonly code?: string; + /** The `run_id` of the error body, when there was one. */ + readonly runId?: string; - constructor(message: string, status: number, code?: string) { + constructor(message: string, status: number, code?: string, runId?: string) { super(message); this.name = 'ApiError'; this.status = status; this.code = code; + this.runId = runId; } } diff --git a/workflow-canvas/src/api/http-api.test.ts b/workflow-canvas/src/api/http-api.test.ts index 87c0d3754..f19766be8 100644 --- a/workflow-canvas/src/api/http-api.test.ts +++ b/workflow-canvas/src/api/http-api.test.ts @@ -278,9 +278,26 @@ describe('createHttpApi', () => { message: 'A run is already active.', status: 409, code: 'already_running', + runId: '20261024-0412-a7b3c', }); }); + it('carries no run id when the error body has none', async () => { + const { api } = apiFor( + reply( + { error: 'kiro-cli is not installed.', code: 'harness_unavailable' }, + 409, + ), + ); + + const error = await api + .startRun('t', 'g') + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).runId).toBeUndefined(); + }); + it('gets /api/runs/{id} with the bearer token', async () => { const { api, fetchFn } = apiFor(reply(RUN_BODY)); @@ -377,6 +394,10 @@ describe('createHttpApi', () => { run({ stages: { write: { status: 'skipped' }, review: {} } }), ], ['stages that are not an object', run({ stages: [] })], + ['a stage that `order` lists twice', run({ order: ['write', 'write'] })], + ['an exit code that is a string', run({ exit_code: '1' })], + ['an exit code that is not whole', run({ exit_code: 1.5 })], + ['a run with no exit code', run({ exit_code: undefined })], ])('fails on %s', async (_, body) => { const { api } = apiFor(reply(body)); diff --git a/workflow-canvas/src/api/http-api.ts b/workflow-canvas/src/api/http-api.ts index a9ef52959..4ce179029 100644 --- a/workflow-canvas/src/api/http-api.ts +++ b/workflow-canvas/src/api/http-api.ts @@ -3,6 +3,7 @@ import { ApiError, type WorkflowApi } from './api'; import { errorCodeOf, errorMessageOf, + errorRunIdOf, parseRunStarted, parseRunStatus, parseSession, @@ -39,6 +40,7 @@ async function send( errorMessageOf(body) ?? `The server answered ${response.status}.`, response.status, errorCodeOf(body), + errorRunIdOf(body), ); } diff --git a/workflow-canvas/src/api/parse.ts b/workflow-canvas/src/api/parse.ts index a7f05a834..329c3f2a6 100644 --- a/workflow-canvas/src/api/parse.ts +++ b/workflow-canvas/src/api/parse.ts @@ -36,6 +36,13 @@ function arrayOf(value: unknown): unknown[] { return Array.isArray(value) ? value : fail(); } +// The contract has `exit_code` as an integer or null, and requires the key. +function exitCodeOf(value: unknown): number | null { + return value === null || Number.isInteger(value) + ? (value as number | null) + : fail(); +} + function parseStage(value: unknown): StageView { const stage = objectOf(value); return { name: stringOf(stage.name), agent: stringOf(stage.agent) }; @@ -86,11 +93,15 @@ function oneOf(value: unknown, allowed: readonly T[]): T { return allowed.find((candidate) => candidate === value) ?? fail(); } -// The stages in `order`, each with its status from `stages`. +// The stages in `order`, each with its status from `stages`. The contract +// lists each stage once, so a name that repeats is a bad reply. function parseRunStages(order: unknown, stages: unknown): RunStageView[] { const byName = objectOf(stages); + const seen = new Set(); return arrayOf(order).map((entry) => { const name = stringOf(entry); + if (seen.has(name)) fail(); + seen.add(name); const stage = Object.hasOwn(byName, name) ? byName[name] : fail(); return { name, @@ -107,11 +118,10 @@ export function parseRunStarted(body: unknown): string { /** Reads a `RunStatusResponse`; unknown fields are ignored. */ export function parseRunStatus(body: unknown): RunView { const run = objectOf(objectOf(body).run); - const exitCode = run.exit_code; return { runId: stringOf(run.run_id), status: oneOf(run.status, RUN_STATUSES), - exitCode: typeof exitCode === 'number' ? exitCode : null, + exitCode: exitCodeOf(run.exit_code), stages: parseRunStages(run.order, run.stages), }; } @@ -147,6 +157,13 @@ export function errorMessageOf(body: unknown): string | undefined { : undefined; } +/** The `run_id` of an `Error` body, or `undefined` when it has none. */ +export function errorRunIdOf(body: unknown): string | undefined { + return isObject(body) && typeof body.run_id === 'string' && body.run_id !== '' + ? body.run_id + : undefined; +} + /** The `code` of an `Error` body, or `undefined` when it has none. */ export function errorCodeOf(body: unknown): string | undefined { return isObject(body) && typeof body.code === 'string' From 5095c3bcd811ca9583f6bac73635f7f48c57eb94 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Tue, 27 Oct 2026 22:55:14 +0000 Subject: [PATCH 03/11] Follow only the run that was started, and tidy the run core A reading of another run ends the loop with a failed event instead of leaving the tab in the following state. The abort listener of the fake scheduler is removed once it fires, the abort test builds its own loop so it asserts the whole event list, and the README describes the four calls of the port and the purity test of the run core. --- workflow-canvas/README.md | 15 ++++++--- .../src/execution/core/follow-run.test.ts | 31 +++++++++++++------ .../src/execution/core/follow-run.ts | 6 ++++ workflow-canvas/src/execution/core/goal.ts | 2 ++ workflow-canvas/src/execution/core/ports.ts | 2 +- .../src/test-support/fake-scheduler.ts | 14 ++++++--- 6 files changed, 49 insertions(+), 21 deletions(-) diff --git a/workflow-canvas/README.md b/workflow-canvas/README.md index aff4835b9..2cf1dd28e 100644 --- a/workflow-canvas/README.md +++ b/workflow-canvas/README.md @@ -15,7 +15,8 @@ serves it and the tests around both are in this folder. - `src/`: the UI. `graph/` turns a workflow into nodes and edges and does no IO; `api/` is the typed client of the API; `launch/` and `workflow/` open the - session and load the workflow; `designer/` and `canvas/` draw it. + session and load the workflow; `designer/` and `canvas/` draw it; + `execution/core/` starts a run and follows it, and does no IO. - `api/`: the HTTP API contract (`openapi.yaml`) and its test. - `stub-api/`: a TypeScript stub server for the contract, for local development and tests. @@ -226,13 +227,17 @@ network, and the stub core is held to the same rule by the same scan catches an alias such as `const f = fetch`, `XMLHttpRequest`, `WebSocket`, `sendBeacon`, storage, `import.meta`, animation frames and dynamic imports, and it ignores a word in a comment or a string. It guards against accidents; -it is not a sandbox. +it is not a sandbox. The run core, `src/execution/core/`, is held to it too, +by `test-support/execution-purity.test.ts`: it may import only its own +modules, the graph core and the wire types. The rest wraps the core, so the rules can be tested without a browser: -- `api/`: `WorkflowApi` is the port with the two calls the page makes; - `createHttpApi` is its adapter over `fetch`. It ignores fields it does not - know and turns a failed call into an `ApiError` with a message fit to show. +- `api/`: `WorkflowApi` is the port with the calls the page makes (open a + session, load the workflow, start a run, read a run); `createHttpApi` is its + adapter over `fetch`. It ignores fields it does not know, refuses a reply + that breaks the contract, and turns a failed call into an `ApiError` with a + message fit to show. - `launch/launch-code.ts` and `workflow/` take the launch code from the address and load the workflow. `loadWorkflow` never rejects: it returns `{ ok: true, workflow }` or `{ ok: false, message }`. `createWorkflowSource` diff --git a/workflow-canvas/src/execution/core/follow-run.test.ts b/workflow-canvas/src/execution/core/follow-run.test.ts index 652d3ce26..b6e43993d 100644 --- a/workflow-canvas/src/execution/core/follow-run.test.ts +++ b/workflow-canvas/src/execution/core/follow-run.test.ts @@ -180,14 +180,14 @@ describe('startAndFollow', () => { it('emits no reading that arrives after an abort', async () => { let arrive: (run: RunView) => void = () => {}; - const { scheduler, events, abort, done } = setup([]); - const runs: RunPort = { - start: async () => RUN_ID, - get: () => new Promise((resolve) => (arrive = resolve)), - }; - const late = startAndFollow({ - runs, - scheduler, + const events: ExecutionEvent[] = []; + const abort = new AbortController(); + const done = startAndFollow({ + runs: { + start: async () => RUN_ID, + get: () => new Promise((resolve) => (arrive = resolve)), + }, + scheduler: new FakeScheduler(), goal: 'Ship it', emit: (event) => events.push(event), signal: abort.signal, @@ -196,9 +196,20 @@ describe('startAndFollow', () => { abort.abort(); arrive(helloRun(0)); - await Promise.all([done, late]); + await done; + + expect(events).toEqual([{ type: 'started', runId: RUN_ID }]); + }); + + it('fails when the server answers with a run that is not the one started', async () => { + const other: RunView = { ...helloRun(0), runId: 'other' }; + const { runs, scheduler, events, done } = setup([other]); - expect(events.map((event) => event.type)).not.toContain('snapshot'); + await done; + + expect(runs.get).toHaveBeenCalledTimes(1); + expect(scheduler.waits).toEqual([]); + expect(events.map((event) => event.type)).toEqual(['started', 'failed']); }); it('does not report a failure after it was aborted', async () => { diff --git a/workflow-canvas/src/execution/core/follow-run.ts b/workflow-canvas/src/execution/core/follow-run.ts index 66898a7b5..75046b0d5 100644 --- a/workflow-canvas/src/execution/core/follow-run.ts +++ b/workflow-canvas/src/execution/core/follow-run.ts @@ -6,6 +6,7 @@ import type { RunPort, Scheduler } from './ports'; export const POLL_INTERVAL_MS = 1000; const UNKNOWN_FAILURE = 'The run did not go as expected.'; +const OTHER_RUN = 'The server answered with a run that is not the one started.'; function messageOf(error: unknown): string { return error instanceof Error ? error.message : UNKNOWN_FAILURE; @@ -16,6 +17,7 @@ export interface FollowRequest { runs: RunPort; scheduler: Scheduler; goal: string; + /** Must not throw: a throw would be taken for a failure of the run. */ emit: (event: ExecutionEvent) => void; /** Aborting stops the loop quietly: no further event is emitted. */ signal: AbortSignal; @@ -35,6 +37,10 @@ export async function startAndFollow(request: FollowRequest): Promise { for (;;) { const run = await runs.get(runId); if (signal.aborted) return; + if (run.runId !== runId) { + emit({ type: 'failed', message: OTHER_RUN }); + return; + } emit({ type: 'snapshot', run }); if (run.status !== 'running') return; await scheduler.wait(POLL_INTERVAL_MS, signal); diff --git a/workflow-canvas/src/execution/core/goal.ts b/workflow-canvas/src/execution/core/goal.ts index e865a10b6..68cc55b8b 100644 --- a/workflow-canvas/src/execution/core/goal.ts +++ b/workflow-canvas/src/execution/core/goal.ts @@ -6,6 +6,8 @@ export const GOAL_MAX_LENGTH = 2048; /** * Why the goal cannot be sent, as a sentence for a person, or `undefined` * when it can. Length counts characters, not UTF-16 units, as the API does. + * A goal of only white space is refused here on purpose: the API would take + * it, but it asks the agent for nothing. */ export function goalProblem(goal: string): string | undefined { if (goal.trim() === '') return 'Enter a goal to run.'; diff --git a/workflow-canvas/src/execution/core/ports.ts b/workflow-canvas/src/execution/core/ports.ts index 34a40b804..0767f58fb 100644 --- a/workflow-canvas/src/execution/core/ports.ts +++ b/workflow-canvas/src/execution/core/ports.ts @@ -14,7 +14,7 @@ export interface RunPort { /** * The only source of time for the core, so a test can move it by hand. The - * timer adapter is `execution/timer-scheduler.ts`. + * timer adapter lives in the layer that draws the tab. */ export interface Scheduler { /** Resolves after `ms` milliseconds, or at once when the signal aborts. */ diff --git a/workflow-canvas/src/test-support/fake-scheduler.ts b/workflow-canvas/src/test-support/fake-scheduler.ts index 47b470147..69479c2ab 100644 --- a/workflow-canvas/src/test-support/fake-scheduler.ts +++ b/workflow-canvas/src/test-support/fake-scheduler.ts @@ -8,7 +8,7 @@ import type { Scheduler } from '../execution/core'; export class FakeScheduler implements Scheduler { /** The `ms` of each wait asked for, in order. */ readonly waits: number[] = []; - private pending = new Set<() => void>(); + private readonly pending = new Set<() => void>(); wait(ms: number, signal: AbortSignal): Promise { this.waits.push(ms); @@ -18,10 +18,14 @@ export class FakeScheduler implements Scheduler { return; } this.pending.add(resolve); - signal.addEventListener('abort', () => { - this.pending.delete(resolve); - resolve(); - }); + signal.addEventListener( + 'abort', + () => { + this.pending.delete(resolve); + resolve(); + }, + { once: true }, + ); }); } From 68dcb0f05cfff2693d86746b5ac4d71158db0422 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:28:30 +0000 Subject: [PATCH 04/11] Follow a run through a blip, join an active run, and bound every call - A failure names the run it belongs to, so a loop that lost cannot end the state of the one that won. - A start refused with 409 already_running and a run id follows that run. - A read that fails with no answer or a 5xx is tried again, up to three in a row. Any other 4xx, or a reply the page cannot read, ends at once. - Every call takes an AbortSignal and gives up after 15 seconds. A body that stops arriving counts as no answer. - A run reply must hold together: id pattern, exit code and stage states for its status, and the stages in order. A 2xx start with an unreadable reply says the run may have started. The fix of an error body is shown. --- workflow-canvas/README.md | 7 +- workflow-canvas/src/api/api.test.ts | 13 + workflow-canvas/src/api/api.ts | 17 +- workflow-canvas/src/api/http-api.test.ts | 220 ++++++++++++++- workflow-canvas/src/api/http-api.ts | 106 ++++++-- workflow-canvas/src/api/parse.ts | 52 +++- .../execution/core/execution-state.test.ts | 72 ++++- .../src/execution/core/execution-state.ts | 60 +++-- .../src/execution/core/follow-run.test.ts | 255 ++++++++++++++++-- .../src/execution/core/follow-run.ts | 105 ++++++-- workflow-canvas/src/execution/core/index.ts | 14 +- .../src/execution/core/node-statuses.test.ts | 8 + workflow-canvas/src/execution/core/ports.ts | 35 ++- .../src/execution/core/status-text.test.ts | 16 ++ .../src/execution/core/status-text.ts | 6 +- .../src/workflow/load-workflow.test.ts | 80 +++++- workflow-canvas/src/workflow/load-workflow.ts | 41 ++- 17 files changed, 989 insertions(+), 118 deletions(-) create mode 100644 workflow-canvas/src/api/api.test.ts diff --git a/workflow-canvas/README.md b/workflow-canvas/README.md index 2cf1dd28e..c04981bd7 100644 --- a/workflow-canvas/README.md +++ b/workflow-canvas/README.md @@ -16,7 +16,12 @@ serves it and the tests around both are in this folder. - `src/`: the UI. `graph/` turns a workflow into nodes and edges and does no IO; `api/` is the typed client of the API; `launch/` and `workflow/` open the session and load the workflow; `designer/` and `canvas/` draw it; - `execution/core/` starts a run and follows it, and does no IO. + `execution/core/` starts a run and follows it, and does no IO. It reads a run + once a second. A read that fails with no answer or a server error is tried + again, up to three in a row; any other failure ends the following. A start + that finds a run already active follows that run. Every call takes a signal + and ends on its own after 15 seconds, so a server that never answers cannot + hold the tab. A failure counts only for the run it belongs to. - `api/`: the HTTP API contract (`openapi.yaml`) and its test. - `stub-api/`: a TypeScript stub server for the contract, for local development and tests. diff --git a/workflow-canvas/src/api/api.test.ts b/workflow-canvas/src/api/api.test.ts new file mode 100644 index 000000000..7f8710156 --- /dev/null +++ b/workflow-canvas/src/api/api.test.ts @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { canRetryStatus } from './api'; + +describe('canRetryStatus', () => { + it.each([0, 500, 503])('retries status %i', (status) => { + expect(canRetryStatus(status)).toBe(true); + }); + + it.each([200, 202, 401, 404, 499])('does not retry status %i', (status) => { + expect(canRetryStatus(status)).toBe(false); + }); +}); diff --git a/workflow-canvas/src/api/api.ts b/workflow-canvas/src/api/api.ts index 90e06e97c..d572dc00b 100644 --- a/workflow-canvas/src/api/api.ts +++ b/workflow-canvas/src/api/api.ts @@ -10,10 +10,13 @@ export interface WorkflowApi { createSession(code: string): Promise; /** Loads the root workflow of the session. */ getWorkflow(token: string): Promise; - /** Starts a run of the root workflow with the goal; resolves to its id. */ - startRun(token: string, goal: string): Promise; + /** + * Starts a run of the root workflow with the goal; resolves to its id. + * Aborting the signal ends the call. + */ + startRun(token: string, goal: string, signal?: AbortSignal): Promise; /** Reads the state of a run, live or finished. */ - getRun(token: string, runId: string): Promise; + getRun(token: string, runId: string, signal?: AbortSignal): Promise; } /** A call that failed, with a message that is fit to show to a person. */ @@ -33,3 +36,11 @@ export class ApiError extends Error { this.runId = runId; } } + +/** + * Whether a failed call can succeed on the next try: no answer (status 0) or a + * server error (5xx). Any 4xx will not change. + */ +export function canRetryStatus(status: number): boolean { + return status === 0 || status >= 500; +} diff --git a/workflow-canvas/src/api/http-api.test.ts b/workflow-canvas/src/api/http-api.test.ts index f19766be8..4670a2d5d 100644 --- a/workflow-canvas/src/api/http-api.test.ts +++ b/workflow-canvas/src/api/http-api.test.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { ApiError } from './api'; import { createHttpApi } from './http-api'; @@ -398,6 +398,49 @@ describe('createHttpApi', () => { ['an exit code that is a string', run({ exit_code: '1' })], ['an exit code that is not whole', run({ exit_code: 1.5 })], ['a run with no exit code', run({ exit_code: undefined })], + ['a run id that is not in the pattern', run({ run_id: 'not-a-run-id' })], + ['an empty run id', run({ run_id: '' })], + ['a stage that `order` leaves out', run({ order: ['write'] })], + [ + 'a stage in `stages` that `order` does not list', + run({ + order: [], + stages: { ghost: { status: 'running' } }, + }), + ], + ['a running run with an exit code', run({ exit_code: 0 })], + [ + 'a passed run with a stage that is not passed', + run({ status: 'passed', exit_code: 0 }), + ], + [ + 'a passed run with a non-zero exit code', + run({ + status: 'passed', + exit_code: 1, + stages: { write: { status: 'passed' }, review: { status: 'passed' } }, + }), + ], + [ + 'a halted run with no stage blocked', + run({ status: 'halted', exit_code: 1 }), + ], + [ + 'a halted run with exit code 0', + run({ + status: 'halted', + exit_code: 0, + stages: { write: { status: 'blocked' }, review: { status: 'pending' } }, + }), + ], + [ + 'a halted run with two stages blocked', + run({ + status: 'halted', + exit_code: 1, + stages: { write: { status: 'blocked' }, review: { status: 'blocked' } }, + }), + ], ])('fails on %s', async (_, body) => { const { api } = apiFor(reply(body)); @@ -409,4 +452,179 @@ describe('createHttpApi', () => { await expect(api.startRun('t', 'g')).rejects.toBeInstanceOf(ApiError); }); + + it.each(['constructor', '__proto__', 'toString'])( + 'does not take the stage name %s for a stage the run lacks', + async (name) => { + const { api } = apiFor( + reply(run({ order: [name], stages: { write: { status: 'passed' } } })), + ); + + await expect(api.getRun('t', 'r')).rejects.toBeInstanceOf(ApiError); + }, + ); + + it('adds the fix of an error body to its sentence', async () => { + const { api } = apiFor( + reply( + { + error: 'kiro-cli is not installed.', + code: 'harness_unavailable', + fix: 'Install kiro-cli and sign in, then try again.', + }, + 409, + ), + ); + + await expect(api.startRun('t', 'g')).rejects.toMatchObject({ + message: + 'kiro-cli is not installed. Install kiro-cli and sign in, then try again.', + }); + }); + + it('says the run may have started when it is accepted with a reply it cannot read', async () => { + const { api } = apiFor(new Response('ok', { status: 202 })); + + await expect(api.startRun('t', 'g')).rejects.toMatchObject({ + status: 202, + message: expect.stringContaining('may have started'), + }); + }); + + it('says the run may have started when it is accepted with an empty id', async () => { + const { api } = apiFor(reply({ run_id: '' }, 202)); + + await expect(api.startRun('t', 'g')).rejects.toMatchObject({ + message: expect.stringContaining('may have started'), + }); + }); +}); + +describe('createHttpApi, when a call does not go through', () => { + it('reports no answer, status 0, when the connection fails', async () => { + const { api } = apiFor(new TypeError('fetch failed')); + + await expect(api.getRun('t', 'r')).rejects.toMatchObject({ status: 0 }); + await expect(api.startRun('t', 'g')).rejects.toMatchObject({ status: 0 }); + }); + + it('reports the status of a server error whose body is not JSON', async () => { + const { api } = apiFor( + new Response('Bad gateway', { status: 502 }), + ); + + await expect(api.getRun('t', 'r')).rejects.toMatchObject({ + status: 502, + message: 'The server answered 502.', + }); + }); + + it('reports no answer when the body stops arriving, so the read can be tried again', async () => { + const stream = new ReadableStream({ + start: (controller) => controller.error(new TypeError('terminated')), + }); + const { api } = apiFor(new Response(stream, { status: 200 })); + + await expect(api.getRun('t', 'r')).rejects.toMatchObject({ status: 0 }); + }); + + it('passes a signal to fetch, and aborts it when the caller aborts', async () => { + let seen: AbortSignal | undefined; + const fetchFn = vi.fn( + (_path: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + seen = init?.signal ?? undefined; + seen?.addEventListener('abort', () => reject(new Error('aborted'))); + }), + ); + const api = createHttpApi(fetchFn as unknown as typeof fetch); + const caller = new AbortController(); + + const call = api.getRun('t', 'r', caller.signal); + expect(seen?.aborted).toBe(false); + caller.abort(); + + await expect(call).rejects.toMatchObject({ status: 0 }); + expect(seen?.aborted).toBe(true); + }); + + it('does not call at all when the caller has already aborted', async () => { + let aborted = false; + const fetchFn = vi.fn( + (_path: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + aborted = init?.signal?.aborted ?? false; + reject(new Error('aborted')); + }), + ); + const api = createHttpApi(fetchFn as unknown as typeof fetch); + const caller = new AbortController(); + caller.abort(); + + await expect(api.getRun('t', 'r', caller.signal)).rejects.toMatchObject({ + status: 0, + }); + expect(aborted).toBe(true); + }); + + describe('with a server that never answers', () => { + afterEach(() => { + vi.useRealTimers(); + }); + + const hanging = () => + vi.fn( + (_path: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => + reject(new Error('aborted')), + ); + }), + ); + + it('gives up after the timeout and reports no answer', async () => { + vi.useFakeTimers(); + const api = createHttpApi(hanging() as unknown as typeof fetch, 5000); + + const call = api.getRun('t', 'r'); + const outcome = expect(call).rejects.toMatchObject({ status: 0 }); + await vi.advanceTimersByTimeAsync(4999); + expect(vi.getTimerCount()).toBe(1); + await vi.advanceTimersByTimeAsync(1); + + await outcome; + }); + + it('gives up when the body hangs after the headers came', async () => { + vi.useFakeTimers(); + let hang: ReadableStreamDefaultController | undefined; + const stream = new ReadableStream({ start: (c) => (hang = c) }); + const fetchFn = vi.fn( + (_path: string, init?: RequestInit) => + new Promise((resolve) => { + const response = new Response(stream); + init?.signal?.addEventListener('abort', () => + hang?.error(new Error('aborted')), + ); + resolve(response); + }), + ); + const api = createHttpApi(fetchFn as unknown as typeof fetch, 5000); + + const call = api.getRun('t', 'r'); + const outcome = expect(call).rejects.toMatchObject({ status: 0 }); + await vi.advanceTimersByTimeAsync(5000); + + await outcome; + }); + + it('stops its timer when the call ends', async () => { + vi.useFakeTimers(); + const { api } = apiFor(reply(RUN_BODY)); + + await api.getRun('t', 'r'); + + expect(vi.getTimerCount()).toBe(0); + }); + }); }); diff --git a/workflow-canvas/src/api/http-api.ts b/workflow-canvas/src/api/http-api.ts index 4ce179029..01c4630d8 100644 --- a/workflow-canvas/src/api/http-api.ts +++ b/workflow-canvas/src/api/http-api.ts @@ -15,26 +15,73 @@ type Fetch = typeof fetch; const UNREACHABLE = 'The server did not answer. Check that it is still running.'; +const ACCEPTED_STATUS = 202; +const RUN_ACCEPTED_UNREADABLE = + 'The server accepted the run, but its reply could not be read. The run may have started.'; + +/** How long a call may take before the page gives up on it. */ +export const REQUEST_TIMEOUT_MS = 15_000; + +// A body that is not JSON is `undefined`. Any other failure to read it, such +// as a connection that dropped or a timeout, is thrown. async function bodyOf(response: Response): Promise { try { return await response.json(); - } catch { - return undefined; + } catch (error) { + // Text that is not JSON is a bad reply. A body that stopped arriving is + // not a reply at all, so it counts as no answer. + if (error instanceof SyntaxError) return undefined; + throw error; + } +} + +// Gives the call a signal that aborts after the timeout or when `outer` +// aborts. The body is read inside the call, so a hung body is covered, and the +// timer and the listener never outlive the call. +async function withTimeout( + timeoutMs: number, + outer: AbortSignal | undefined, + call: (signal: AbortSignal) => Promise, +): Promise { + const controller = new AbortController(); + const abort = () => controller.abort(); + const timer = setTimeout(abort, timeoutMs); + if (outer?.aborted) abort(); + outer?.addEventListener('abort', abort, { once: true }); + try { + return await call(controller.signal); + } finally { + clearTimeout(timer); + outer?.removeEventListener('abort', abort); } } +async function exchange( + fetchFn: Fetch, + path: string, + init: RequestInit, + signal: AbortSignal, +): Promise<{ response: Response; body: unknown }> { + const response = await fetchFn(path, { ...init, signal }); + return { response, body: await bodyOf(response) }; +} + async function send( fetchFn: Fetch, + timeoutMs: number, path: string, init: RequestInit, + outer?: AbortSignal, ): Promise { - let response: Response; + let reply: { response: Response; body: unknown }; try { - response = await fetchFn(path, init); + reply = await withTimeout(timeoutMs, outer, (signal) => + exchange(fetchFn, path, init, signal), + ); } catch { throw new ApiError(UNREACHABLE, 0); } - const body = await bodyOf(response); + const { response, body } = reply; if (response.ok) return body; throw new ApiError( errorMessageOf(body) ?? `The server answered ${response.status}.`, @@ -44,6 +91,16 @@ async function send( ); } +// The server accepted the run, so a reply that cannot be read does not mean +// that the run did not start. +function acceptedRunId(body: unknown): string { + try { + return parseRunStarted(body); + } catch { + throw new ApiError(RUN_ACCEPTED_UNREADABLE, ACCEPTED_STATUS); + } +} + function bearer(token: string): Record { return { Authorization: `Bearer ${token}` }; } @@ -52,10 +109,15 @@ function bearer(token: string): Record { * The adapter of the workflow API over HTTP. It calls the relative `/api`, so * the page and the server share one origin, and keeps no token itself. */ -export function createHttpApi(fetchFn: Fetch = fetch): WorkflowApi { +export function createHttpApi( + fetchFn: Fetch = fetch, + timeoutMs = REQUEST_TIMEOUT_MS, +): WorkflowApi { + const call = (path: string, init: RequestInit, signal?: AbortSignal) => + send(fetchFn, timeoutMs, path, init, signal); return { async createSession(code) { - const body = await send(fetchFn, '/api/session', { + const body = await call('/api/session', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ code }), @@ -63,26 +125,28 @@ export function createHttpApi(fetchFn: Fetch = fetch): WorkflowApi { return parseSession(body); }, async getWorkflow(token) { - const body = await send(fetchFn, '/api/workflow', { + const body = await call('/api/workflow', { headers: bearer(token), }); return parseWorkflowDocument(body); }, - async startRun(token, goal) { - const body = await send(fetchFn, '/api/run', { - method: 'POST', - headers: { ...bearer(token), 'Content-Type': 'application/json' }, - body: JSON.stringify({ params: { goal } }), - }); - return parseRunStarted(body); - }, - async getRun(token, runId) { - const body = await send( - fetchFn, - `/api/runs/${encodeURIComponent(runId)}`, + async startRun(token, goal, signal) { + const body = await call( + '/api/run', { - headers: bearer(token), + method: 'POST', + headers: { ...bearer(token), 'Content-Type': 'application/json' }, + body: JSON.stringify({ params: { goal } }), }, + signal, + ); + return acceptedRunId(body); + }, + async getRun(token, runId, signal) { + const body = await call( + `/api/runs/${encodeURIComponent(runId)}`, + { headers: bearer(token) }, + signal, ); return parseRunStatus(body); }, diff --git a/workflow-canvas/src/api/parse.ts b/workflow-canvas/src/api/parse.ts index 329c3f2a6..81e654034 100644 --- a/workflow-canvas/src/api/parse.ts +++ b/workflow-canvas/src/api/parse.ts @@ -98,7 +98,7 @@ function oneOf(value: unknown, allowed: readonly T[]): T { function parseRunStages(order: unknown, stages: unknown): RunStageView[] { const byName = objectOf(stages); const seen = new Set(); - return arrayOf(order).map((entry) => { + const listed = arrayOf(order).map((entry) => { const name = stringOf(entry); if (seen.has(name)) fail(); seen.add(name); @@ -108,22 +108,55 @@ function parseRunStages(order: unknown, stages: unknown): RunStageView[] { status: oneOf(objectOf(stage).status, STAGE_STATUSES), }; }); + // `order` holds the keys of `stages`: a stage that is not listed is a bad reply. + return Object.keys(byName).length === listed.length ? listed : fail(); +} + +// The pattern of `RunId` in the contract, which holds in this version. +const RUN_ID_PATTERN = /^[0-9]{8}-[0-9]{4}-[a-z0-9]{5}$/; + +function runIdOf(value: unknown): string { + const id = stringOf(value); + return RUN_ID_PATTERN.test(id) ? id : fail(); +} + +// The contract says what a run in each status looks like: a running run has +// no exit code, a passed run has every stage passed and exit code 0, and a +// halted run has exactly one stage blocked and an exit code above 0. +function isCoherent( + status: RunStatus, + exitCode: number | null, + stages: readonly RunStageView[], +): boolean { + switch (status) { + case 'running': + return exitCode === null; + case 'passed': + return exitCode === 0 && stages.every((s) => s.status === 'passed'); + case 'halted': + return ( + exitCode !== null && + exitCode > 0 && + stages.filter((s) => s.status === 'blocked').length === 1 + ); + } } /** Reads a `RunStarted`: the id of the run that was accepted. */ export function parseRunStarted(body: unknown): string { - return stringOf(objectOf(body).run_id); + return runIdOf(objectOf(body).run_id); } /** Reads a `RunStatusResponse`; unknown fields are ignored. */ export function parseRunStatus(body: unknown): RunView { const run = objectOf(objectOf(body).run); - return { - runId: stringOf(run.run_id), + const view: RunView = { + runId: runIdOf(run.run_id), status: oneOf(run.status, RUN_STATUSES), exitCode: exitCodeOf(run.exit_code), stages: parseRunStages(run.order, run.stages), }; + return isCoherent(view.status, view.exitCode, view.stages) ? view : fail(); } /** Reads a `SessionResponse`; unknown fields are ignored. */ @@ -150,11 +183,14 @@ export function parseWorkflowDocument(body: unknown): WorkflowDocument { }; } -/** The sentence of an `Error` body, or `undefined` when it has none. */ +/** + * The sentence of an `Error` body, followed by its `fix` (what to do about it), + * or `undefined` when it has no sentence. + */ export function errorMessageOf(body: unknown): string | undefined { - return isObject(body) && typeof body.error === 'string' - ? body.error - : undefined; + if (!isObject(body) || typeof body.error !== 'string') return undefined; + const fix = typeof body.fix === 'string' ? body.fix : ''; + return fix === '' ? body.error : `${body.error} ${fix}`; } /** The `run_id` of an `Error` body, or `undefined` when it has none. */ diff --git a/workflow-canvas/src/execution/core/execution-state.test.ts b/workflow-canvas/src/execution/core/execution-state.test.ts index 347e69eba..678ec2951 100644 --- a/workflow-canvas/src/execution/core/execution-state.test.ts +++ b/workflow-canvas/src/execution/core/execution-state.test.ts @@ -90,18 +90,76 @@ describe('reduce', () => { message: 'No.', started: false, }); - expect(after(...STARTED, { type: 'failed', message: 'Lost.' })).toEqual({ + expect( + after(...STARTED, { type: 'failed', message: 'Lost.', runId: RUN_ID }), + ).toEqual({ phase: 'error', message: 'Lost.', started: true, }); }); + it('ignores a failure of a loop that has no run once the tab follows one', () => { + // Two loops were started; one was refused, and it never had a run. + const following = after(...STARTED); + + expect( + reduce(following, { + type: 'failed', + message: 'A run is already active.', + }), + ).toBe(following); + }); + + it('ignores a failure that names another run', () => { + const following = after(...STARTED); + + expect( + reduce(following, { + type: 'failed', + message: 'Lost.', + runId: '20261024-0412-zzzzz', + }), + ).toBe(following); + }); + + it('ignores a failure that names a run while the start has no run yet', () => { + const starting = after({ type: 'start' }); + + expect( + reduce(starting, { type: 'failed', message: 'x', runId: RUN_ID }), + ).toBe(starting); + }); + + it('follows a run that was already active, and keeps saying so', () => { + const joined = after( + { type: 'start' }, + { type: 'started', runId: RUN_ID, joined: true }, + ); + + expect(joined).toEqual({ + phase: 'following', + runId: RUN_ID, + run: undefined, + joined: true, + }); + expect(reduce(joined, { type: 'snapshot', run: helloRun(0) })).toEqual({ + phase: 'following', + runId: RUN_ID, + run: helloRun(0), + joined: true, + }); + expect(reduce(joined, { type: 'snapshot', run: helloRun(2) })).toEqual({ + phase: 'finished', + run: helloRun(2), + }); + }); + it('shows no run in an error', () => { const state = after( ...STARTED, { type: 'snapshot', run: helloRun(0) }, - { type: 'failed', message: 'Lost.' }, + { type: 'failed', message: 'Lost.', runId: RUN_ID }, ); expect(runOf(state)).toBeUndefined(); @@ -117,7 +175,11 @@ describe('reduce', () => { it.each<[string, ExecutionState, ExecutionEvent]>([ ['a reading when idle', IDLE, { type: 'snapshot', run: helloRun(0) }], ['an id when idle', IDLE, { type: 'started', runId: RUN_ID }], - ['a failure when idle', IDLE, { type: 'failed', message: 'x' }], + [ + 'a failure when idle', + IDLE, + { type: 'failed', message: 'x', runId: RUN_ID }, + ], [ 'an id when following', after(...STARTED), @@ -133,6 +195,8 @@ describe('reduce', () => { run: helloRun(2), }); - expect(reduce(finished, { type: 'failed', message: 'x' })).toBe(finished); + expect( + reduce(finished, { type: 'failed', message: 'x', runId: RUN_ID }), + ).toBe(finished); }); }); diff --git a/workflow-canvas/src/execution/core/execution-state.ts b/workflow-canvas/src/execution/core/execution-state.ts index 55b59fa2d..f7c164ee4 100644 --- a/workflow-canvas/src/execution/core/execution-state.ts +++ b/workflow-canvas/src/execution/core/execution-state.ts @@ -5,16 +5,23 @@ import type { RunView } from '../../api/types'; export type ExecutionState = | { phase: 'idle' } | { phase: 'starting' } - | { phase: 'following'; runId: string; run: RunView | undefined } + | { + phase: 'following'; + runId: string; + run: RunView | undefined; + /** Set when the tab joined a run that was already active. */ + joined?: true; + } | { phase: 'finished'; run: RunView } | { phase: 'error'; message: string; started: boolean }; /** Something that happened to the run, in the order it can. */ export type ExecutionEvent = | { type: 'start' } - | { type: 'started'; runId: string } + | { type: 'started'; runId: string; joined?: boolean } | { type: 'snapshot'; run: RunView } - | { type: 'failed'; message: string }; + /** `runId` is the run the failure belongs to, once the loop has one. */ + | { type: 'failed'; message: string; runId?: string }; /** The state before any run has been started. */ export const IDLE: ExecutionState = { phase: 'idle' }; @@ -35,29 +42,46 @@ function startOf(state: ExecutionState): ExecutionState { return isInFlight(state) ? state : { phase: 'starting' }; } -function startedOf(state: ExecutionState, runId: string): ExecutionState { - return state.phase === 'starting' - ? { phase: 'following', runId, run: undefined } - : state; +function startedOf( + state: ExecutionState, + runId: string, + joined: boolean | undefined, +): ExecutionState { + if (state.phase !== 'starting') return state; + return joined + ? { phase: 'following', runId, run: undefined, joined: true } + : { phase: 'following', runId, run: undefined }; } function snapshotOf(state: ExecutionState, run: RunView): ExecutionState { if (state.phase !== 'following' || state.runId !== run.runId) return state; - return run.status === 'running' - ? { phase: 'following', runId: run.runId, run } - : { phase: 'finished', run }; + if (run.status !== 'running') return { phase: 'finished', run }; + return state.joined + ? { phase: 'following', runId: run.runId, run, joined: true } + : { phase: 'following', runId: run.runId, run }; } -function failedOf(state: ExecutionState, message: string): ExecutionState { - return isInFlight(state) - ? { phase: 'error', message, started: state.phase === 'following' } - : state; +// A failure counts only for the run it belongs to: one that names no run is +// for a start that has not got an id yet, and one that names a run is for the +// run being followed. A loop that lost to another cannot end the winner's run. +function failedOf( + state: ExecutionState, + message: string, + runId: string | undefined, +): ExecutionState { + if (state.phase === 'starting' && runId === undefined) { + return { phase: 'error', message, started: false }; + } + if (state.phase === 'following' && runId === state.runId) { + return { phase: 'error', message, started: true }; + } + return state; } /** * The next state after an event. An event that does not fit the state, such as - * a start while a run is in flight or a reply for an earlier run, changes - * nothing. + * a start while a run is in flight or a reading or failure of another run, + * changes nothing. */ export function reduce( state: ExecutionState, @@ -67,10 +91,10 @@ export function reduce( case 'start': return startOf(state); case 'started': - return startedOf(state, event.runId); + return startedOf(state, event.runId, event.joined); case 'snapshot': return snapshotOf(state, event.run); case 'failed': - return failedOf(state, event.message); + return failedOf(state, event.message, event.runId); } } diff --git a/workflow-canvas/src/execution/core/follow-run.test.ts b/workflow-canvas/src/execution/core/follow-run.test.ts index b6e43993d..fe9e50e61 100644 --- a/workflow-canvas/src/execution/core/follow-run.test.ts +++ b/workflow-canvas/src/execution/core/follow-run.test.ts @@ -4,8 +4,13 @@ import type { RunView } from '../../api/types'; import { FakeScheduler } from '../../test-support/fake-scheduler'; import { helloRun } from '../../test-support/fixtures'; import type { ExecutionEvent } from './execution-state'; -import { POLL_INTERVAL_MS, startAndFollow } from './follow-run'; -import type { RunPort } from './ports'; +import { IDLE, reduce } from './execution-state'; +import { + MAX_READ_FAILURES, + POLL_INTERVAL_MS, + startAndFollow, +} from './follow-run'; +import { RunAlreadyActive, RunReadRefused, type RunPort } from './ports'; const RUN_ID = helloRun(0).runId; @@ -44,8 +49,8 @@ describe('startAndFollow', () => { await done; - expect(runs.start).toHaveBeenCalledWith('Ship it'); - expect(runs.get).toHaveBeenCalledWith(RUN_ID); + expect(runs.start).toHaveBeenCalledWith('Ship it', expect.any(AbortSignal)); + expect(runs.get).toHaveBeenCalledWith(RUN_ID, expect.any(AbortSignal)); expect(events).toEqual([ { type: 'started', runId: RUN_ID }, { type: 'snapshot', run: helloRun(2) }, @@ -80,10 +85,6 @@ describe('startAndFollow', () => { ]); }); - it('polls once a second', () => { - expect(POLL_INTERVAL_MS).toBe(1000); - }); - it('stops after a halted run', async () => { const halted = helloRun(0, { blockedAt: 0 }); const { runs, scheduler, done } = setup([halted]); @@ -112,29 +113,231 @@ describe('startAndFollow', () => { expect(runs.get).not.toHaveBeenCalled(); expect(events).toEqual([ - { type: 'failed', message: 'A run is already active.' }, + { type: 'failed', message: 'A run is already active.', runId: undefined }, ]); }); - it('ends with one failed event when a reading fails, and reads no more', async () => { - const { runs, scheduler, events, done } = setup([ - helloRun(0), - new Error('The server did not answer.'), - ]); + describe('when a reading fails', () => { + const blip = () => new Error('The server did not answer.'); + + it('reads again after a poll interval and goes on when the next one works', async () => { + const { runs, scheduler, events, done } = setup([ + helloRun(0), + blip(), + helloRun(2), + ]); + + await settle(); + scheduler.tick(); + await settle(); + expect(runs.get).toHaveBeenCalledTimes(2); + scheduler.tick(); + await done; + + expect(runs.get).toHaveBeenCalledTimes(3); + expect(events.map((event) => event.type)).toEqual([ + 'started', + 'snapshot', + 'snapshot', + ]); + }); - await settle(); - scheduler.tick(); - await done; + it('gives up after MAX_READ_FAILURES in a row, and says which run it was', async () => { + const { runs, scheduler, events, done } = setup([ + helloRun(0), + ...Array.from({ length: MAX_READ_FAILURES }, blip), + ]); + + for (let turn = 0; turn < MAX_READ_FAILURES + 1; turn += 1) { + await settle(); + scheduler.tick(); + } + await done; + + expect(runs.get).toHaveBeenCalledTimes(1 + MAX_READ_FAILURES); + expect(events.at(-1)).toEqual({ + type: 'failed', + message: 'The server did not answer.', + runId: RUN_ID, + }); + expect(events.filter((e) => e.type === 'failed')).toHaveLength(1); + }); - expect(runs.get).toHaveBeenCalledTimes(2); - expect(events.map((event) => event.type)).toEqual([ - 'started', - 'snapshot', - 'failed', - ]); - expect(events.at(-1)).toEqual({ - type: 'failed', - message: 'The server did not answer.', + it('counts the failures in a row, so a good reading in between starts the count again', async () => { + const readings = [ + helloRun(0), + blip(), + blip(), + helloRun(1), + blip(), + blip(), + helloRun(2), + ]; + const { runs, scheduler, events, done } = setup(readings); + + for (let turn = 0; turn < readings.length; turn += 1) { + await settle(); + scheduler.tick(); + } + await done; + + expect(runs.get).toHaveBeenCalledTimes(readings.length); + expect(events.map((event) => event.type)).toEqual([ + 'started', + 'snapshot', + 'snapshot', + 'snapshot', + ]); + }); + + it('does not try again when the server refuses the read', async () => { + const refused = new RunReadRefused('There is no run with this id.'); + const { runs, scheduler, events, done } = setup([refused]); + + await done; + + expect(runs.get).toHaveBeenCalledTimes(1); + expect(scheduler.waits).toEqual([]); + expect(events).toEqual([ + { type: 'started', runId: RUN_ID }, + { type: 'failed', message: refused.message, runId: RUN_ID }, + ]); + }); + + it('stops quietly when aborted while it waits to try again', async () => { + const { runs, events, abort, done } = setup([blip(), helloRun(0)]); + await settle(); + + abort.abort(); + await done; + + expect(runs.get).toHaveBeenCalledTimes(1); + expect(events).toEqual([{ type: 'started', runId: RUN_ID }]); + }); + }); + + describe('when a run is already active', () => { + const active = () => + vi.fn().mockRejectedValue(new RunAlreadyActive('Busy.', RUN_ID)); + + it('follows that run and says it joined it', async () => { + const { runs, events, done } = setup([helloRun(2)], active()); + + await done; + + expect(runs.get).toHaveBeenCalledWith(RUN_ID, expect.any(AbortSignal)); + expect(events).toEqual([ + { type: 'started', runId: RUN_ID, joined: true }, + { type: 'snapshot', run: helloRun(2) }, + ]); + }); + + it('does not start another run', async () => { + const start = active(); + const { done } = setup([helloRun(2)], start); + + await done; + + expect(start).toHaveBeenCalledTimes(1); + }); + }); + + describe('when the server never answers a read', () => { + // A port honours its signal: aborting rejects the call that is pending. + function hangingRuns(): RunPort { + const hang = (signal: AbortSignal) => + new Promise((_resolve, reject) => + signal.addEventListener('abort', () => reject(new Error('aborted'))), + ); + return { + start: vi.fn().mockResolvedValue(RUN_ID), + get: vi.fn((_id: string, signal: AbortSignal) => hang(signal)), + }; + } + + it('ends without a word when it is aborted', async () => { + const events: ExecutionEvent[] = []; + const abort = new AbortController(); + const done = startAndFollow({ + runs: hangingRuns(), + scheduler: new FakeScheduler(), + goal: 'Ship it', + emit: (event) => events.push(event), + signal: abort.signal, + }); + await settle(); + + abort.abort(); + await done; + + expect(events).toEqual([{ type: 'started', runId: RUN_ID }]); + }); + + it('ends without a word when the start hangs and it is aborted', async () => { + const events: ExecutionEvent[] = []; + const abort = new AbortController(); + const runs: RunPort = { + start: (_goal, signal) => + new Promise((_resolve, reject) => + signal.addEventListener('abort', () => reject(new Error('x'))), + ), + get: vi.fn(), + }; + const done = startAndFollow({ + runs, + scheduler: new FakeScheduler(), + goal: 'Ship it', + emit: (event) => events.push(event), + signal: abort.signal, + }); + await settle(); + + abort.abort(); + await done; + + expect(events).toEqual([]); + }); + }); + + describe('when two loops are started for one click', () => { + it('keeps the state of the loop that won when the other is refused', async () => { + let state = reduce(IDLE, { type: 'start' }); + const emit = (event: ExecutionEvent) => { + state = reduce(state, event); + }; + const scheduler = new FakeScheduler(); + const stop = new AbortController(); + const winner = startAndFollow({ + runs: { + start: async () => RUN_ID, + get: async () => helloRun(0), + }, + scheduler, + goal: 'Ship it', + emit, + signal: stop.signal, + }); + await settle(); + const loser = startAndFollow({ + runs: { + start: () => Promise.reject(new Error('A run is already active.')), + get: vi.fn(), + }, + scheduler: new FakeScheduler(), + goal: 'Ship it', + emit, + signal: new AbortController().signal, + }); + await loser; + + expect(state).toMatchObject({ phase: 'following', runId: RUN_ID }); + + scheduler.tick(); + await settle(); + expect(state).toMatchObject({ phase: 'following', run: helloRun(0) }); + + stop.abort(); + await winner; }); }); diff --git a/workflow-canvas/src/execution/core/follow-run.ts b/workflow-canvas/src/execution/core/follow-run.ts index 75046b0d5..c650dcacf 100644 --- a/workflow-canvas/src/execution/core/follow-run.ts +++ b/workflow-canvas/src/execution/core/follow-run.ts @@ -1,10 +1,19 @@ // SPDX-License-Identifier: Apache-2.0 +import type { RunView } from '../../api/types'; import type { ExecutionEvent } from './execution-state'; -import type { RunPort, Scheduler } from './ports'; +import { + RunAlreadyActive, + RunReadRefused, + type RunPort, + type Scheduler, +} from './ports'; /** How long to wait between two reads of a run, as the API asks. */ export const POLL_INTERVAL_MS = 1000; +/** How many reads in a row may fail before the run is given up on. */ +export const MAX_READ_FAILURES = 3; + const UNKNOWN_FAILURE = 'The run did not go as expected.'; const OTHER_RUN = 'The server answered with a run that is not the one started.'; @@ -23,30 +32,86 @@ export interface FollowRequest { signal: AbortSignal; } +// Starts a run, or names the run that is already active in its place. +async function begin({ + runs, + goal, + signal, +}: FollowRequest): Promise<{ runId: string; joined: boolean }> { + try { + return { runId: await runs.start(goal, signal), joined: false }; + } catch (error) { + if (error instanceof RunAlreadyActive) { + return { runId: error.runId, joined: true }; + } + throw error; + } +} + +// One reading of the run. A failed read is tried again after a poll interval +// until `MAX_READ_FAILURES` in a row have failed; then the last error is thrown. +// A refused read is thrown at once, as trying again cannot change the answer. +// Resolves to `undefined` when aborted. +async function read( + { runs, scheduler, signal }: FollowRequest, + runId: string, +): Promise { + for (let failures = 1; ; failures += 1) { + try { + return await runs.get(runId, signal); + } catch (error) { + if ( + signal.aborted || + error instanceof RunReadRefused || + failures >= MAX_READ_FAILURES + ) { + throw error; + } + await scheduler.wait(POLL_INTERVAL_MS, signal); + if (signal.aborted) return undefined; + } + } +} + +// Reads the run every poll interval until it is no longer running. +async function follow(request: FollowRequest, runId: string): Promise { + const { scheduler, emit, signal } = request; + for (;;) { + const run = await read(request, runId); + if (signal.aborted || run === undefined) return; + if (run.runId !== runId) { + emit({ type: 'failed', message: OTHER_RUN, runId }); + return; + } + emit({ type: 'snapshot', run }); + if (run.status !== 'running') return; + await scheduler.wait(POLL_INTERVAL_MS, signal); + if (signal.aborted) return; + } +} + /** - * Starts a run, then reads it every `POLL_INTERVAL_MS` until it is no longer - * running, telling `emit` each step. It never rejects: a failed call ends the - * loop with one `failed` event. + * Starts a run, or joins the one that is already active, then reads it every + * `POLL_INTERVAL_MS` until it is no longer running, telling `emit` each step. + * It never rejects: a failure ends the loop with one `failed` event, which + * names the run once there is one, so it cannot be taken for another's. */ export async function startAndFollow(request: FollowRequest): Promise { - const { runs, scheduler, goal, emit, signal } = request; + const { emit, signal } = request; + let runId: string | undefined; try { - const runId = await runs.start(goal); + const begun = await begin(request); + runId = begun.runId; if (signal.aborted) return; - emit({ type: 'started', runId }); - for (;;) { - const run = await runs.get(runId); - if (signal.aborted) return; - if (run.runId !== runId) { - emit({ type: 'failed', message: OTHER_RUN }); - return; - } - emit({ type: 'snapshot', run }); - if (run.status !== 'running') return; - await scheduler.wait(POLL_INTERVAL_MS, signal); - if (signal.aborted) return; - } + emit( + begun.joined + ? { type: 'started', runId, joined: true } + : { type: 'started', runId }, + ); + await follow(request, runId); } catch (error) { - if (!signal.aborted) emit({ type: 'failed', message: messageOf(error) }); + if (!signal.aborted) { + emit({ type: 'failed', message: messageOf(error), runId }); + } } } diff --git a/workflow-canvas/src/execution/core/index.ts b/workflow-canvas/src/execution/core/index.ts index 5958c13a6..d39e6fd03 100644 --- a/workflow-canvas/src/execution/core/index.ts +++ b/workflow-canvas/src/execution/core/index.ts @@ -13,8 +13,18 @@ export { type ExecutionEvent, type ExecutionState, } from './execution-state'; -export { POLL_INTERVAL_MS, startAndFollow } from './follow-run'; +export { + MAX_READ_FAILURES, + POLL_INTERVAL_MS, + startAndFollow, + type FollowRequest, +} from './follow-run'; export { GOAL_MAX_LENGTH, goalProblem } from './goal'; export { nodeStatuses } from './node-statuses'; -export type { RunPort, Scheduler } from './ports'; +export { + RunAlreadyActive, + RunReadRefused, + type RunPort, + type Scheduler, +} from './ports'; export { statusKind, statusText, type StatusKind } from './status-text'; diff --git a/workflow-canvas/src/execution/core/node-statuses.test.ts b/workflow-canvas/src/execution/core/node-statuses.test.ts index a82aec7b0..aa60dbf01 100644 --- a/workflow-canvas/src/execution/core/node-statuses.test.ts +++ b/workflow-canvas/src/execution/core/node-statuses.test.ts @@ -69,4 +69,12 @@ describe('nodeStatuses', () => { expect(nodeStatuses(run).get('stage-a-b')?.state).toBe('running'); }); + + it('gives a status to the stages of the run only, by name, when the workflow was edited during the run', () => { + // A save is allowed during a run, so the graph may name other stages. + const statuses = nodeStatuses(helloRun(1)); + + expect(statuses.has('stage-draft')).toBe(true); + expect(statuses.has('stage-renamed-after-the-run-began')).toBe(false); + }); }); diff --git a/workflow-canvas/src/execution/core/ports.ts b/workflow-canvas/src/execution/core/ports.ts index 0767f58fb..fee315f37 100644 --- a/workflow-canvas/src/execution/core/ports.ts +++ b/workflow-canvas/src/execution/core/ports.ts @@ -4,12 +4,39 @@ import type { RunView } from '../../api/types'; /** * The calls of the API that a run needs, already tied to a session. The * page depends on this port; `workflow/load-workflow.ts` builds the adapter. + * A call rejects when its signal aborts, and gives up on its own when the + * server does not answer, so the caller never waits for ever. */ export interface RunPort { - /** Starts a run with the goal and resolves to its id. */ - start(goal: string): Promise; - /** Reads the state of the run with the id. */ - get(runId: string): Promise; + /** + * Starts a run with the goal and resolves to its id. When a run is already + * active it rejects with `RunAlreadyActive`, so the caller can follow that run. + */ + start(goal: string, signal: AbortSignal): Promise; + /** + * Reads the state of the run with the id. It rejects with `RunReadRefused` + * when the server will not give the run, so the caller does not try again. + */ + get(runId: string, signal: AbortSignal): Promise; +} + +/** A run was asked for while another is active; `runId` is the active one. */ +export class RunAlreadyActive extends Error { + readonly runId: string; + + constructor(message: string, runId: string) { + super(message); + this.name = 'RunAlreadyActive'; + this.runId = runId; + } +} + +/** A read that failed in a way that reading again cannot fix. */ +export class RunReadRefused extends Error { + constructor(message: string) { + super(message); + this.name = 'RunReadRefused'; + } } /** diff --git a/workflow-canvas/src/execution/core/status-text.test.ts b/workflow-canvas/src/execution/core/status-text.test.ts index 2736540d0..1a9bbec55 100644 --- a/workflow-canvas/src/execution/core/status-text.test.ts +++ b/workflow-canvas/src/execution/core/status-text.test.ts @@ -10,6 +10,22 @@ function following(run?: ReturnType): ExecutionState { return { phase: 'following', runId: helloRun(0).runId, run }; } +describe('statusText for a run that was already active', () => { + it('says that the tab follows it', () => { + const state: ExecutionState = { + phase: 'following', + runId: helloRun(0).runId, + run: helloRun(1), + joined: true, + }; + + expect(statusText(state)).toBe( + 'Following the run that was already in progress. Running review. 1 of 2 stages done.', + ); + expect(statusKind(state)).toBe('busy'); + }); +}); + describe('statusText and statusKind', () => { it.each<[string, ExecutionState, string, string]>([ ['idle', IDLE, 'Ready. Press Run to start.', 'idle'], diff --git a/workflow-canvas/src/execution/core/status-text.ts b/workflow-canvas/src/execution/core/status-text.ts index a59f04721..fce794936 100644 --- a/workflow-canvas/src/execution/core/status-text.ts +++ b/workflow-canvas/src/execution/core/status-text.ts @@ -5,6 +5,8 @@ import type { ExecutionState } from './execution-state'; /** How the status line is drawn: at rest, at work, passed or not working. */ export type StatusKind = 'idle' | 'busy' | 'success' | 'error'; +const JOINED = 'Following the run that was already in progress.'; + function stagesDone(run: RunView): string { const done = run.stages.filter((stage) => stage.status === 'passed').length; return `${done} of ${run.stages.length} stages done.`; @@ -37,7 +39,9 @@ export function statusText(state: ExecutionState): string { case 'starting': return 'Starting the run.'; case 'following': - return followingText(state.run); + return state.joined + ? `${JOINED} ${followingText(state.run)}` + : followingText(state.run); case 'finished': return finishedText(state.run); case 'error': diff --git a/workflow-canvas/src/workflow/load-workflow.test.ts b/workflow-canvas/src/workflow/load-workflow.test.ts index e7515efde..58d1ebd49 100644 --- a/workflow-canvas/src/workflow/load-workflow.test.ts +++ b/workflow-canvas/src/workflow/load-workflow.test.ts @@ -44,11 +44,81 @@ describe('loadWorkflow', () => { const result = await loadWorkflow(api, 'code-1'); if (!result.ok) throw new Error('The load failed.'); - const runId = await result.workflow.runs.start('Ship it'); - await result.workflow.runs.get(runId); - - expect(api.startRun).toHaveBeenCalledWith('token-1', 'Ship it'); - expect(api.getRun).toHaveBeenCalledWith('token-1', runId); + const { signal } = new AbortController(); + const runId = await result.workflow.runs.start('Ship it', signal); + await result.workflow.runs.get(runId, signal); + + expect(api.startRun).toHaveBeenCalledWith('token-1', 'Ship it', signal); + expect(api.getRun).toHaveBeenCalledWith('token-1', runId, signal); + }); + + describe('the run calls', () => { + const { signal } = new AbortController(); + const portFor = async (api: WorkflowApi) => { + const result = await loadWorkflow(api, 'code-1'); + if (!result.ok) throw new Error('The load failed.'); + return result.workflow.runs; + }; + + it('name the active run when the server answers 409 already_running with its id', async () => { + const busy = new ApiError( + 'A run is already active.', + 409, + 'already_running', + '20261024-0412-a7b3c', + ); + const runs = await portFor( + fakeApi({ startRun: vi.fn().mockRejectedValue(busy) }), + ); + + await expect(runs.start('g', signal)).rejects.toMatchObject({ + name: 'RunAlreadyActive', + runId: '20261024-0412-a7b3c', + message: 'A run is already active.', + }); + }); + + it.each([ + [ + 'a 409 with another code', + new ApiError('Busy.', 409, 'harness_unavailable', 'r1'), + ], + ['a 409 with no run id', new ApiError('Busy.', 409, 'already_running')], + ['a 500', new ApiError('Broken.', 500)], + ])('leave %s as the error it is', async (_, error) => { + const runs = await portFor( + fakeApi({ startRun: vi.fn().mockRejectedValue(error) }), + ); + + await expect(runs.start('g', signal)).rejects.toBe(error); + }); + + it.each([ + ['a 401', new ApiError('No session.', 401)], + ['a 404', new ApiError('No such run.', 404)], + ['a reply the page cannot read', new ApiError('Bad reply.', 200)], + ])('refuse to read again after %s', async (_, error) => { + const runs = await portFor( + fakeApi({ getRun: vi.fn().mockRejectedValue(error) }), + ); + + await expect(runs.get('r', signal)).rejects.toMatchObject({ + name: 'RunReadRefused', + message: error.message, + }); + }); + + it.each([ + ['no answer', new ApiError('No answer.', 0)], + ['a 502', new ApiError('Bad gateway.', 502)], + ['an error that is not an ApiError', new Error('Odd.')], + ])('leave %s open to another try', async (_, error) => { + const runs = await portFor( + fakeApi({ getRun: vi.fn().mockRejectedValue(error) }), + ); + + await expect(runs.get('r', signal)).rejects.toBe(error); + }); }); it('fails without calling the API when there is no launch code', async () => { diff --git a/workflow-canvas/src/workflow/load-workflow.ts b/workflow-canvas/src/workflow/load-workflow.ts index 2fed62165..325a565b7 100644 --- a/workflow-canvas/src/workflow/load-workflow.ts +++ b/workflow-canvas/src/workflow/load-workflow.ts @@ -1,7 +1,11 @@ // SPDX-License-Identifier: Apache-2.0 -import { ApiError, type WorkflowApi } from '../api/api'; +import { ApiError, canRetryStatus, type WorkflowApi } from '../api/api'; import type { WorkflowDocument } from '../api/types'; -import type { RunPort } from '../execution/core'; +import { + RunAlreadyActive, + RunReadRefused, + type RunPort, +} from '../execution/core'; import type { SessionMemory, SessionRecord } from '../launch/session-memory'; /** @@ -47,6 +51,23 @@ function messageOf(error: unknown): string { return error instanceof Error ? error.message : 'The workflow did not load.'; } +// The API names the active run in its 409; the core is told as its own error. +function activeRunOf(error: unknown): RunAlreadyActive | undefined { + return error instanceof ApiError && + error.code === 'already_running' && + error.runId !== undefined + ? new RunAlreadyActive(error.message, error.runId) + : undefined; +} + +// Only a missing answer or a server error can be gone on the next try; any +// 4xx and a reply the page cannot read will not change. +function readRefusedOf(error: unknown): RunReadRefused | undefined { + return error instanceof ApiError && !canRetryStatus(error.status) + ? new RunReadRefused(error.message) + : undefined; +} + function failure(message: string): LoadResult { return { ok: false, message }; } @@ -76,8 +97,20 @@ function guardSession( function runPortFor(api: WorkflowApi, token: string): RunPort { return { - start: (goal) => api.startRun(token, goal), - get: (runId) => api.getRun(token, runId), + start: async (goal, signal) => { + try { + return await api.startRun(token, goal, signal); + } catch (error) { + throw activeRunOf(error) ?? error; + } + }, + get: async (runId, signal) => { + try { + return await api.getRun(token, runId, signal); + } catch (error) { + throw readRefusedOf(error) ?? error; + } + }, }; } From f2ebd4cd24a4843ca0f5611e09e455fc225f8915 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:57:42 +0000 Subject: [PATCH 05/11] Give the fake API of the source tests the run calls --- workflow-canvas/src/workflow/use-workflow-source.test.tsx | 2 ++ workflow-canvas/src/workflow/workflow-source.test.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/workflow-canvas/src/workflow/use-workflow-source.test.tsx b/workflow-canvas/src/workflow/use-workflow-source.test.tsx index d4b0aea41..d4cb95ac5 100644 --- a/workflow-canvas/src/workflow/use-workflow-source.test.tsx +++ b/workflow-canvas/src/workflow/use-workflow-source.test.tsx @@ -14,6 +14,8 @@ describe('the page, under strict mode', () => { const api: WorkflowApi = { createSession: vi.fn().mockResolvedValue(HELLO_SESSION), getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), + startRun: vi.fn(), + getRun: vi.fn(), }; const source = createWorkflowSource(api, 'code-1', freshMemory()); function Root() { diff --git a/workflow-canvas/src/workflow/workflow-source.test.ts b/workflow-canvas/src/workflow/workflow-source.test.ts index ea4c12c36..d9538e23e 100644 --- a/workflow-canvas/src/workflow/workflow-source.test.ts +++ b/workflow-canvas/src/workflow/workflow-source.test.ts @@ -9,6 +9,8 @@ function apiWith(createSession: WorkflowApi['createSession']): WorkflowApi { return { createSession, getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), + startRun: vi.fn(), + getRun: vi.fn(), }; } From 2abff9aba795a6f0739f3a66d27044224606762f Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Wed, 28 Oct 2026 19:26:59 +0000 Subject: [PATCH 06/11] Say when a start may have started, retry 408 and 429, and tighten the run core docs A start with no answer or an unreadable accepted reply now ends as an error that says the run may have started. A run_id in an error body must look like a run id. FollowRequest says one loop runs at a time, and startAndFollow says it rejects only when emit throws. --- workflow-canvas/README.md | 6 ++-- workflow-canvas/src/api/api.test.ts | 11 ++++--- workflow-canvas/src/api/api.ts | 7 +++-- workflow-canvas/src/api/http-api.test.ts | 16 ++++++++++ workflow-canvas/src/api/parse.ts | 6 ++-- .../execution/core/execution-state.test.ts | 22 ++++++++++++++ .../src/execution/core/execution-state.ts | 29 ++++++++++++++----- .../src/execution/core/follow-run.test.ts | 25 +++++++++++++++- .../src/execution/core/follow-run.ts | 27 +++++++++++++---- workflow-canvas/src/execution/core/index.ts | 1 + workflow-canvas/src/execution/core/ports.ts | 12 ++++++++ .../src/execution/core/status-text.test.ts | 6 ++++ .../src/execution/core/status-text.ts | 11 +++++-- .../src/workflow/load-workflow.test.ts | 15 ++++++++++ workflow-canvas/src/workflow/load-workflow.ts | 12 +++++++- 15 files changed, 176 insertions(+), 30 deletions(-) diff --git a/workflow-canvas/README.md b/workflow-canvas/README.md index c04981bd7..95dce46f1 100644 --- a/workflow-canvas/README.md +++ b/workflow-canvas/README.md @@ -19,9 +19,9 @@ serves it and the tests around both are in this folder. `execution/core/` starts a run and follows it, and does no IO. It reads a run once a second. A read that fails with no answer or a server error is tried again, up to three in a row; any other failure ends the following. A start - that finds a run already active follows that run. Every call takes a signal - and ends on its own after 15 seconds, so a server that never answers cannot - hold the tab. A failure counts only for the run it belongs to. + that finds a run already active follows that run. Every run call takes a + signal, and every call ends on its own after 15 seconds, so a server that + never answers cannot hold the tab. A failure counts only for the run it belongs to. - `api/`: the HTTP API contract (`openapi.yaml`) and its test. - `stub-api/`: a TypeScript stub server for the contract, for local development and tests. diff --git a/workflow-canvas/src/api/api.test.ts b/workflow-canvas/src/api/api.test.ts index 7f8710156..9155bd199 100644 --- a/workflow-canvas/src/api/api.test.ts +++ b/workflow-canvas/src/api/api.test.ts @@ -3,11 +3,14 @@ import { describe, expect, it } from 'vitest'; import { canRetryStatus } from './api'; describe('canRetryStatus', () => { - it.each([0, 500, 503])('retries status %i', (status) => { + it.each([0, 408, 429, 500, 503])('retries status %i', (status) => { expect(canRetryStatus(status)).toBe(true); }); - it.each([200, 202, 401, 404, 499])('does not retry status %i', (status) => { - expect(canRetryStatus(status)).toBe(false); - }); + it.each([200, 202, 400, 401, 404, 409, 499])( + 'does not retry status %i', + (status) => { + expect(canRetryStatus(status)).toBe(false); + }, + ); }); diff --git a/workflow-canvas/src/api/api.ts b/workflow-canvas/src/api/api.ts index d572dc00b..c113d59d5 100644 --- a/workflow-canvas/src/api/api.ts +++ b/workflow-canvas/src/api/api.ts @@ -38,9 +38,10 @@ export class ApiError extends Error { } /** - * Whether a failed call can succeed on the next try: no answer (status 0) or a - * server error (5xx). Any 4xx will not change. + * Whether a failed call can succeed on the next try: no answer (status 0), a + * server error (5xx), or a request timeout or rate limit (408, 429) from a + * proxy. Any other 4xx will not change. */ export function canRetryStatus(status: number): boolean { - return status === 0 || status >= 500; + return status === 0 || status >= 500 || status === 408 || status === 429; } diff --git a/workflow-canvas/src/api/http-api.test.ts b/workflow-canvas/src/api/http-api.test.ts index 4670a2d5d..b69a84cf4 100644 --- a/workflow-canvas/src/api/http-api.test.ts +++ b/workflow-canvas/src/api/http-api.test.ts @@ -282,6 +282,22 @@ describe('createHttpApi', () => { }); }); + it('carries no run id when the error body has one that is not a run id', async () => { + const { api } = apiFor( + reply( + { error: 'Busy.', code: 'already_running', run_id: '../etc/passwd' }, + 409, + ), + ); + + const error = await api + .startRun('t', 'g') + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ApiError); + expect((error as ApiError).runId).toBeUndefined(); + }); + it('carries no run id when the error body has none', async () => { const { api } = apiFor( reply( diff --git a/workflow-canvas/src/api/parse.ts b/workflow-canvas/src/api/parse.ts index 81e654034..27e54e6dd 100644 --- a/workflow-canvas/src/api/parse.ts +++ b/workflow-canvas/src/api/parse.ts @@ -193,9 +193,11 @@ export function errorMessageOf(body: unknown): string | undefined { return fix === '' ? body.error : `${body.error} ${fix}`; } -/** The `run_id` of an `Error` body, or `undefined` when it has none. */ +/** The `run_id` of an `Error` body, or `undefined` when it has none or it is not a run id. */ export function errorRunIdOf(body: unknown): string | undefined { - return isObject(body) && typeof body.run_id === 'string' && body.run_id !== '' + return isObject(body) && + typeof body.run_id === 'string' && + RUN_ID_PATTERN.test(body.run_id) ? body.run_id : undefined; } diff --git a/workflow-canvas/src/execution/core/execution-state.test.ts b/workflow-canvas/src/execution/core/execution-state.test.ts index 678ec2951..7db9332c2 100644 --- a/workflow-canvas/src/execution/core/execution-state.test.ts +++ b/workflow-canvas/src/execution/core/execution-state.test.ts @@ -82,6 +82,28 @@ describe('reduce', () => { expect(reduce(failed, { type: 'start' })).toEqual({ phase: 'starting' }); }); + it('keeps that a start got no clear answer', () => { + expect( + after( + { type: 'start' }, + { type: 'failed', message: 'No.', maybeStarted: true }, + ), + ).toEqual({ + phase: 'error', + message: 'No.', + started: false, + maybeStarted: true, + }); + }); + + it('takes a failure that names no run for the start in progress', () => { + // Why the caller runs one loop at a time: a second loop's failure with no + // run id cannot be told from the failure of the start. + expect( + after({ type: 'start' }, { type: 'failed', message: 'Other loop.' }), + ).toMatchObject({ phase: 'error', started: false }); + }); + it('fails before the run started, or after, and says which', () => { expect( after({ type: 'start' }, { type: 'failed', message: 'No.' }), diff --git a/workflow-canvas/src/execution/core/execution-state.ts b/workflow-canvas/src/execution/core/execution-state.ts index f7c164ee4..296e72a17 100644 --- a/workflow-canvas/src/execution/core/execution-state.ts +++ b/workflow-canvas/src/execution/core/execution-state.ts @@ -13,15 +13,27 @@ export type ExecutionState = joined?: true; } | { phase: 'finished'; run: RunView } - | { phase: 'error'; message: string; started: boolean }; + | { + phase: 'error'; + message: string; + started: boolean; + /** Set when the start got no clear answer, so the run may be going. */ + maybeStarted?: true; + }; /** Something that happened to the run, in the order it can. */ export type ExecutionEvent = | { type: 'start' } | { type: 'started'; runId: string; joined?: boolean } | { type: 'snapshot'; run: RunView } - /** `runId` is the run the failure belongs to, once the loop has one. */ - | { type: 'failed'; message: string; runId?: string }; + | { + type: 'failed'; + message: string; + /** The run the failure belongs to, once the loop has one. */ + runId?: string; + /** The start got no clear answer, so the run may have started. */ + maybeStarted?: true; + }; /** The state before any run has been started. */ export const IDLE: ExecutionState = { phase: 'idle' }; @@ -64,13 +76,16 @@ function snapshotOf(state: ExecutionState, run: RunView): ExecutionState { // A failure counts only for the run it belongs to: one that names no run is // for a start that has not got an id yet, and one that names a run is for the // run being followed. A loop that lost to another cannot end the winner's run. +// A failure with no run id cannot be told from the start of another loop, so +// the caller runs one loop at a time. function failedOf( state: ExecutionState, - message: string, - runId: string | undefined, + { message, runId, maybeStarted }: Extract, ): ExecutionState { if (state.phase === 'starting' && runId === undefined) { - return { phase: 'error', message, started: false }; + return maybeStarted + ? { phase: 'error', message, started: false, maybeStarted: true } + : { phase: 'error', message, started: false }; } if (state.phase === 'following' && runId === state.runId) { return { phase: 'error', message, started: true }; @@ -95,6 +110,6 @@ export function reduce( case 'snapshot': return snapshotOf(state, event.run); case 'failed': - return failedOf(state, event.message, event.runId); + return failedOf(state, event); } } diff --git a/workflow-canvas/src/execution/core/follow-run.test.ts b/workflow-canvas/src/execution/core/follow-run.test.ts index fe9e50e61..3e292e97b 100644 --- a/workflow-canvas/src/execution/core/follow-run.test.ts +++ b/workflow-canvas/src/execution/core/follow-run.test.ts @@ -10,7 +10,12 @@ import { POLL_INTERVAL_MS, startAndFollow, } from './follow-run'; -import { RunAlreadyActive, RunReadRefused, type RunPort } from './ports'; +import { + RunAlreadyActive, + RunReadRefused, + RunStartUnsure, + type RunPort, +} from './ports'; const RUN_ID = helloRun(0).runId; @@ -117,6 +122,24 @@ describe('startAndFollow', () => { ]); }); + it('says the run may have started when the start got no clear answer', async () => { + const { events, done } = setup( + [], + vi.fn().mockRejectedValue(new RunStartUnsure('No answer.')), + ); + + await done; + + expect(events).toEqual([ + { + type: 'failed', + message: 'No answer.', + runId: undefined, + maybeStarted: true, + }, + ]); + }); + describe('when a reading fails', () => { const blip = () => new Error('The server did not answer.'); diff --git a/workflow-canvas/src/execution/core/follow-run.ts b/workflow-canvas/src/execution/core/follow-run.ts index c650dcacf..1e2c37315 100644 --- a/workflow-canvas/src/execution/core/follow-run.ts +++ b/workflow-canvas/src/execution/core/follow-run.ts @@ -4,6 +4,7 @@ import type { ExecutionEvent } from './execution-state'; import { RunAlreadyActive, RunReadRefused, + RunStartUnsure, type RunPort, type Scheduler, } from './ports'; @@ -21,7 +22,11 @@ function messageOf(error: unknown): string { return error instanceof Error ? error.message : UNKNOWN_FAILURE; } -/** What `startAndFollow` needs: the ports, the goal and where events go. */ +/** + * What `startAndFollow` needs: the ports, the goal and where events go. The + * caller runs one loop at a time: a failure with no run id is taken for the + * failure of the start in progress. + */ export interface FollowRequest { runs: RunPort; scheduler: Scheduler; @@ -32,6 +37,17 @@ export interface FollowRequest { signal: AbortSignal; } +// The failure event of a loop. A start that got no clear answer says so. +function failure( + error: unknown, + runId: string | undefined, +): Extract { + const failed = { type: 'failed', message: messageOf(error), runId } as const; + return error instanceof RunStartUnsure + ? { ...failed, maybeStarted: true } + : failed; +} + // Starts a run, or names the run that is already active in its place. async function begin({ runs, @@ -93,8 +109,9 @@ async function follow(request: FollowRequest, runId: string): Promise { /** * Starts a run, or joins the one that is already active, then reads it every * `POLL_INTERVAL_MS` until it is no longer running, telling `emit` each step. - * It never rejects: a failure ends the loop with one `failed` event, which - * names the run once there is one, so it cannot be taken for another's. + * It does not reject unless `emit` throws, which `FollowRequest` forbids. A + * failure ends the loop with one `failed` event, which names the run once + * there is one, so it cannot be taken for another's. */ export async function startAndFollow(request: FollowRequest): Promise { const { emit, signal } = request; @@ -110,8 +127,6 @@ export async function startAndFollow(request: FollowRequest): Promise { ); await follow(request, runId); } catch (error) { - if (!signal.aborted) { - emit({ type: 'failed', message: messageOf(error), runId }); - } + if (!signal.aborted) emit(failure(error, runId)); } } diff --git a/workflow-canvas/src/execution/core/index.ts b/workflow-canvas/src/execution/core/index.ts index d39e6fd03..2a8cd288d 100644 --- a/workflow-canvas/src/execution/core/index.ts +++ b/workflow-canvas/src/execution/core/index.ts @@ -24,6 +24,7 @@ export { nodeStatuses } from './node-statuses'; export { RunAlreadyActive, RunReadRefused, + RunStartUnsure, type RunPort, type Scheduler, } from './ports'; diff --git a/workflow-canvas/src/execution/core/ports.ts b/workflow-canvas/src/execution/core/ports.ts index fee315f37..3f21beb9f 100644 --- a/workflow-canvas/src/execution/core/ports.ts +++ b/workflow-canvas/src/execution/core/ports.ts @@ -11,6 +11,7 @@ export interface RunPort { /** * Starts a run with the goal and resolves to its id. When a run is already * active it rejects with `RunAlreadyActive`, so the caller can follow that run. + * When it cannot tell whether the run started it rejects with `RunStartUnsure`. */ start(goal: string, signal: AbortSignal): Promise; /** @@ -31,6 +32,17 @@ export class RunAlreadyActive extends Error { } } +/** + * A start whose outcome is not known: the server did not answer, or accepted + * the run in a reply that could not be read. The run may be going. + */ +export class RunStartUnsure extends Error { + constructor(message: string) { + super(message); + this.name = 'RunStartUnsure'; + } +} + /** A read that failed in a way that reading again cannot fix. */ export class RunReadRefused extends Error { constructor(message: string) { diff --git a/workflow-canvas/src/execution/core/status-text.test.ts b/workflow-canvas/src/execution/core/status-text.test.ts index 1a9bbec55..ed5ec30b2 100644 --- a/workflow-canvas/src/execution/core/status-text.test.ts +++ b/workflow-canvas/src/execution/core/status-text.test.ts @@ -66,6 +66,12 @@ describe('statusText and statusKind', () => { 'The run did not start.', 'error', ], + [ + 'an error before an unsure start knew if the run started', + { phase: 'error', message: 'x', started: false, maybeStarted: true }, + 'The run may not have started.', + 'error', + ], [ 'an error after the run started', { phase: 'error', message: 'x', started: true }, diff --git a/workflow-canvas/src/execution/core/status-text.ts b/workflow-canvas/src/execution/core/status-text.ts index fce794936..7af55e942 100644 --- a/workflow-canvas/src/execution/core/status-text.ts +++ b/workflow-canvas/src/execution/core/status-text.ts @@ -31,6 +31,13 @@ function finishedText(run: RunView): string { return `${lead} ${stagesDone(run)}${exit}`; } +function errorText(state: Extract): string { + if (state.started) return 'Stopped following the run.'; + return state.maybeStarted + ? 'The run may not have started.' + : 'The run did not start.'; +} + /** The sentence that says where the run is. A screen reader announces it. */ export function statusText(state: ExecutionState): string { switch (state.phase) { @@ -45,9 +52,7 @@ export function statusText(state: ExecutionState): string { case 'finished': return finishedText(state.run); case 'error': - return state.started - ? 'Stopped following the run.' - : 'The run did not start.'; + return errorText(state); } } diff --git a/workflow-canvas/src/workflow/load-workflow.test.ts b/workflow-canvas/src/workflow/load-workflow.test.ts index 58d1ebd49..6b8bfd28a 100644 --- a/workflow-canvas/src/workflow/load-workflow.test.ts +++ b/workflow-canvas/src/workflow/load-workflow.test.ts @@ -93,6 +93,20 @@ describe('loadWorkflow', () => { await expect(runs.start('g', signal)).rejects.toBe(error); }); + it.each([ + ['no answer', new ApiError('No answer.', 0)], + ['an accepted run in an unreadable reply', new ApiError('Odd.', 202)], + ])('tell that the run may have started after %s', async (_, error) => { + const runs = await portFor( + fakeApi({ startRun: vi.fn().mockRejectedValue(error) }), + ); + + await expect(runs.start('g', signal)).rejects.toMatchObject({ + name: 'RunStartUnsure', + message: error.message, + }); + }); + it.each([ ['a 401', new ApiError('No session.', 401)], ['a 404', new ApiError('No such run.', 404)], @@ -111,6 +125,7 @@ describe('loadWorkflow', () => { it.each([ ['no answer', new ApiError('No answer.', 0)], ['a 502', new ApiError('Bad gateway.', 502)], + ['a 429', new ApiError('Slow down.', 429)], ['an error that is not an ApiError', new Error('Odd.')], ])('leave %s open to another try', async (_, error) => { const runs = await portFor( diff --git a/workflow-canvas/src/workflow/load-workflow.ts b/workflow-canvas/src/workflow/load-workflow.ts index 325a565b7..91d0d94fd 100644 --- a/workflow-canvas/src/workflow/load-workflow.ts +++ b/workflow-canvas/src/workflow/load-workflow.ts @@ -4,6 +4,7 @@ import type { WorkflowDocument } from '../api/types'; import { RunAlreadyActive, RunReadRefused, + RunStartUnsure, type RunPort, } from '../execution/core'; import type { SessionMemory, SessionRecord } from '../launch/session-memory'; @@ -68,6 +69,15 @@ function readRefusedOf(error: unknown): RunReadRefused | undefined { : undefined; } +// No answer, or an accepted run in a reply the page cannot read: the run may +// have started. +function startUnsureOf(error: unknown): RunStartUnsure | undefined { + return error instanceof ApiError && + (error.status === 0 || error.status === 202) + ? new RunStartUnsure(error.message) + : undefined; +} + function failure(message: string): LoadResult { return { ok: false, message }; } @@ -101,7 +111,7 @@ function runPortFor(api: WorkflowApi, token: string): RunPort { try { return await api.startRun(token, goal, signal); } catch (error) { - throw activeRunOf(error) ?? error; + throw activeRunOf(error) ?? startUnsureOf(error) ?? error; } }, get: async (runId, signal) => { From 30d98de0639c5b99e4bf7b844ca6225ff5f5f594 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Wed, 28 Oct 2026 22:31:42 +0000 Subject: [PATCH 07/11] Give the session memory tests the run calls and the memory argument --- workflow-canvas/src/workflow/load-workflow.test.ts | 4 ++-- workflow-canvas/src/workflow/workflow-source.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/workflow-canvas/src/workflow/load-workflow.test.ts b/workflow-canvas/src/workflow/load-workflow.test.ts index 6b8bfd28a..6ed808b5c 100644 --- a/workflow-canvas/src/workflow/load-workflow.test.ts +++ b/workflow-canvas/src/workflow/load-workflow.test.ts @@ -41,7 +41,7 @@ describe('loadWorkflow', () => { it('ties the run calls to the session token', async () => { const api = fakeApi(); - const result = await loadWorkflow(api, 'code-1'); + const result = await loadWorkflow(api, 'code-1', freshMemory()); if (!result.ok) throw new Error('The load failed.'); const { signal } = new AbortController(); @@ -55,7 +55,7 @@ describe('loadWorkflow', () => { describe('the run calls', () => { const { signal } = new AbortController(); const portFor = async (api: WorkflowApi) => { - const result = await loadWorkflow(api, 'code-1'); + const result = await loadWorkflow(api, 'code-1', freshMemory()); if (!result.ok) throw new Error('The load failed.'); return result.workflow.runs; }; diff --git a/workflow-canvas/src/workflow/workflow-source.test.ts b/workflow-canvas/src/workflow/workflow-source.test.ts index d9538e23e..763cb08fa 100644 --- a/workflow-canvas/src/workflow/workflow-source.test.ts +++ b/workflow-canvas/src/workflow/workflow-source.test.ts @@ -611,7 +611,7 @@ describe('createWorkflowSource session memory', () => { it('forgets a kept session that the server no longer knows', async () => { const memory = memoryWith(KEPT); const api = { - createSession: vi.fn(), + ...apiWith(vi.fn()), getWorkflow: vi.fn().mockRejectedValue(new ApiError('Wrong.', 401)), }; const source = createWorkflowSource(api, undefined, memory); From f40cb7adbbefea7d4ab4ba1d20a422946404d0f4 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:33:10 +0000 Subject: [PATCH 08/11] Drop the session on a 401 from a run call, with the message of a lost session A run start or read that the server answers with 401 now clears the kept token through the same guard as the load, and fails with the same message. The test fakes of the new session tests carry the run calls. --- .../src/workflow/load-workflow.test.ts | 54 ++++++++++++++++++- workflow-canvas/src/workflow/load-workflow.ts | 16 ++++-- .../src/workflow/workflow-source.test.ts | 43 ++++++++------- 3 files changed, 87 insertions(+), 26 deletions(-) diff --git a/workflow-canvas/src/workflow/load-workflow.test.ts b/workflow-canvas/src/workflow/load-workflow.test.ts index 6ed808b5c..525870fbd 100644 --- a/workflow-canvas/src/workflow/load-workflow.test.ts +++ b/workflow-canvas/src/workflow/load-workflow.test.ts @@ -8,6 +8,7 @@ import { helloRun, } from '../test-support/fixtures'; import { freshMemory, memoryStorage } from '../test-support/memory-storage'; +import type { RunPort } from '../execution/core'; import { loadOnOpen, loadWorkflow } from './load-workflow'; function fakeApi(overrides: Partial = {}): WorkflowApi { @@ -108,7 +109,6 @@ describe('loadWorkflow', () => { }); it.each([ - ['a 401', new ApiError('No session.', 401)], ['a 404', new ApiError('No such run.', 404)], ['a reply the page cannot read', new ApiError('Bad reply.', 200)], ])('refuse to read again after %s', async (_, error) => { @@ -902,3 +902,55 @@ describe('a session that answers after a newer one was kept', () => { expect(memory.recallPending()).toEqual(PASTED); }); }); + +describe('a 401 on a call after the load', () => { + const lost = () => + vi + .fn() + .mockRejectedValue( + new ApiError('The session token is missing or wrong.', 401), + ); + const { signal } = new AbortController(); + + async function runsOf(api: WorkflowApi, memory = memoryWith(STORED).memory) { + const result = await loadWorkflow(api, undefined, memory); + if (!result.ok) throw new Error('The load failed.'); + return { runs: result.workflow.runs, memory }; + } + + it.each([ + ['a run start', (runs: RunPort) => runs.start('g', signal)], + ['a run read', (runs: RunPort) => runs.get('r-1', signal)], + ])( + 'on %s forgets the session and says the server lost it', + async (_, call) => { + const { runs, memory } = await runsOf( + fakeApi({ startRun: lost(), getRun: lost() }), + ); + + await expect(call(runs)).rejects.toThrow( + /server no longer knows this session.*fresh launch address/s, + ); + expect(memory.recall()).toBeUndefined(); + }, + ); + + it('keeps the session on a failure that is not a 401', async () => { + const { runs, memory } = await runsOf( + fakeApi({ + startRun: vi.fn().mockRejectedValue(new ApiError('Not reached.', 0)), + }), + ); + + await expect(runs.start('g', signal)).rejects.toThrow('Not reached.'); + expect(memory.recall()).toEqual(STORED); + }); + + it('does not forget a newer session', async () => { + const { runs, memory } = await runsOf(fakeApi({ startRun: lost() })); + memory.remember({ token: 'newer-token', harness: 'kiro-cli' }); + + await expect(runs.start('g', signal)).rejects.toThrow(); + expect(memory.recall()?.token).toBe('newer-token'); + }); +}); diff --git a/workflow-canvas/src/workflow/load-workflow.ts b/workflow-canvas/src/workflow/load-workflow.ts index 91d0d94fd..dc8ae9e2d 100644 --- a/workflow-canvas/src/workflow/load-workflow.ts +++ b/workflow-canvas/src/workflow/load-workflow.ts @@ -88,12 +88,14 @@ function forgetIfStill(memory: SessionMemory, session: SessionRecord): void { if (memory.recallPending()?.token === session.token) memory.dropPending(); } +type SessionGuard = (call: Promise) => Promise; + // A 401 on a call that carries the token means the server no longer knows the // session: the token is dropped and the call fails with one message. function guardSession( memory: SessionMemory, session: SessionRecord, -): (call: Promise) => Promise { +): SessionGuard { return async (call) => { try { return await call; @@ -105,18 +107,22 @@ function guardSession( }; } -function runPortFor(api: WorkflowApi, token: string): RunPort { +function runPortFor( + api: WorkflowApi, + token: string, + guard: SessionGuard, +): RunPort { return { start: async (goal, signal) => { try { - return await api.startRun(token, goal, signal); + return await guard(api.startRun(token, goal, signal)); } catch (error) { throw activeRunOf(error) ?? startUnsureOf(error) ?? error; } }, get: async (runId, signal) => { try { - return await api.getRun(token, runId, signal); + return await guard(api.getRun(token, runId, signal)); } catch (error) { throw readRefusedOf(error) ?? error; } @@ -136,7 +142,7 @@ async function loadWith( workflow: { harness: session.harness, document, - runs: runPortFor(api, session.token), + runs: runPortFor(api, session.token, guard), }, remember: () => memory.remember(session), }; diff --git a/workflow-canvas/src/workflow/workflow-source.test.ts b/workflow-canvas/src/workflow/workflow-source.test.ts index 763cb08fa..bf7cf0c55 100644 --- a/workflow-canvas/src/workflow/workflow-source.test.ts +++ b/workflow-canvas/src/workflow/workflow-source.test.ts @@ -5,10 +5,15 @@ import { HELLO_DOCUMENT, HELLO_SESSION } from '../test-support/fixtures'; import { freshMemory } from '../test-support/memory-storage'; import { createWorkflowSource } from './workflow-source'; -function apiWith(createSession: WorkflowApi['createSession']): WorkflowApi { +function apiWith( + createSession: WorkflowApi['createSession'], + getWorkflow: WorkflowApi['getWorkflow'] = vi + .fn() + .mockResolvedValue(HELLO_DOCUMENT), +): WorkflowApi { return { createSession, - getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), + getWorkflow, startRun: vi.fn(), getRun: vi.fn(), }; @@ -361,13 +366,13 @@ describe('createWorkflowSource session memory', () => { it('keeps the session when the first load fails after the code worked, so a reload resumes', async () => { const memory = memoryWith(); - const api = { - createSession: vi.fn().mockResolvedValue(HELLO_SESSION), - getWorkflow: vi + const api = apiWith( + vi.fn().mockResolvedValue(HELLO_SESSION), + vi .fn() .mockRejectedValueOnce(new ApiError('Server error.', 500)) .mockResolvedValue(HELLO_DOCUMENT), - }; + ); expect( await createWorkflowSource(api, 'first', memory).current(), @@ -380,15 +385,13 @@ describe('createWorkflowSource session memory', () => { it('keeps the old session when the load of a pasted code fails', async () => { const memory = memoryWith(KEPT); - const api = { - createSession: vi - .fn() - .mockResolvedValue({ ...HELLO_SESSION, token: 'pasted-token' }), - getWorkflow: vi + const api = apiWith( + vi.fn().mockResolvedValue({ ...HELLO_SESSION, token: 'pasted-token' }), + vi .fn() .mockResolvedValueOnce(HELLO_DOCUMENT) .mockRejectedValueOnce(new ApiError('Server error.', 500)), - }; + ); const source = createWorkflowSource(api, undefined, memory); await source.current(); @@ -398,10 +401,10 @@ describe('createWorkflowSource session memory', () => { }); it('opens the kept session when the code in the address is spent', async () => { - const api = { - createSession: vi.fn().mockImplementation(refused), - getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), - }; + const api = apiWith( + vi.fn().mockImplementation(refused), + vi.fn().mockResolvedValue(HELLO_DOCUMENT), + ); const source = createWorkflowSource(api, 'spent', memoryWith(KEPT)); expect(await source.current()).toMatchObject({ ok: true }); @@ -434,10 +437,10 @@ describe('createWorkflowSource session memory', () => { }); it('does not open the kept session in place of a refused pasted code', async () => { - const api = { - createSession: vi.fn().mockImplementation(refused), - getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), - }; + const api = apiWith( + vi.fn().mockImplementation(refused), + vi.fn().mockResolvedValue(HELLO_DOCUMENT), + ); const source = createWorkflowSource(api, undefined, memoryWith(KEPT)); const before = source.current(); await before; From 4d8a49de172b8cb359c54efef8363ddf270e7993 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:41:43 +0000 Subject: [PATCH 09/11] Build the API of the refused-address notice test with the helper that has every method --- workflow-canvas/src/workflow/workflow-source.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/workflow-canvas/src/workflow/workflow-source.test.ts b/workflow-canvas/src/workflow/workflow-source.test.ts index bf7cf0c55..417b8b6a6 100644 --- a/workflow-canvas/src/workflow/workflow-source.test.ts +++ b/workflow-canvas/src/workflow/workflow-source.test.ts @@ -411,10 +411,7 @@ describe('createWorkflowSource session memory', () => { }); it('tells that the code in the address was refused when it opens the kept session', async () => { - const api = { - createSession: vi.fn().mockImplementation(refused), - getWorkflow: vi.fn().mockResolvedValue(HELLO_DOCUMENT), - }; + const api = apiWith(vi.fn().mockImplementation(refused)); const source = createWorkflowSource(api, 'spent', memoryWith(KEPT)); await source.current(); From c4b1816ee5e9ee7cc8b828e7ab6d2e0b76b2daeb Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Thu, 29 Oct 2026 01:42:11 +0000 Subject: [PATCH 10/11] Build the api of the new session tests with the shared helper The api of a test now carries the run calls, so the two tests that resume a pasted session and keep a quiet notice build it with apiWith like their neighbours. --- .../src/workflow/workflow-source.test.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/workflow-canvas/src/workflow/workflow-source.test.ts b/workflow-canvas/src/workflow/workflow-source.test.ts index 417b8b6a6..5909d2c66 100644 --- a/workflow-canvas/src/workflow/workflow-source.test.ts +++ b/workflow-canvas/src/workflow/workflow-source.test.ts @@ -576,18 +576,16 @@ describe('createWorkflowSource session memory', () => { it('resumes a pasted session on a reload when its first load failed and the kept session is dead', async () => { const memory = memoryWith(KEPT); const server = { deadToken: '', pastedLoads: false }; - const api = { - createSession: vi - .fn() - .mockResolvedValue({ ...HELLO_SESSION, token: 'pasted-token' }), - getWorkflow: vi.fn((token: string) => { + const api = apiWith( + vi.fn().mockResolvedValue({ ...HELLO_SESSION, token: 'pasted-token' }), + vi.fn((token: string) => { if (token === server.deadToken) return Promise.reject(new ApiError('Wrong.', 401)); return token === 'pasted-token' && !server.pastedLoads ? Promise.reject(new ApiError('Server error.', 500)) : Promise.resolve(HELLO_DOCUMENT); }), - }; + ); const open = createWorkflowSource(api, undefined, memory); await open.current(); server.deadToken = KEPT.token; @@ -623,19 +621,19 @@ describe('createWorkflowSource session memory', () => { it('tells nothing when the kept session answers after a pasted workflow was shown', async () => { const memory = memoryWith(KEPT); let answerKept!: (document: typeof HELLO_DOCUMENT) => void; - const api = { - createSession: vi + const api = apiWith( + vi .fn() .mockImplementationOnce(refused) .mockResolvedValueOnce({ ...HELLO_SESSION, token: 'pasted-token' }), - getWorkflow: vi.fn((token: string) => + vi.fn((token: string) => token === KEPT.token ? new Promise( (resolve) => (answerKept = resolve), ) : Promise.resolve(HELLO_DOCUMENT), ), - }; + ); const source = createWorkflowSource(api, 'spent', memory); await source.offer('pasted'); From 2df85f82db650e058bb94a7444f39447c1878863 Mon Sep 17 00:00:00 2001 From: Nihit <24500178+knihit@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:43:26 +0000 Subject: [PATCH 11/11] Build the API of the pending session test with the shared helper --- workflow-canvas/src/workflow/workflow-source.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/workflow-canvas/src/workflow/workflow-source.test.ts b/workflow-canvas/src/workflow/workflow-source.test.ts index 5909d2c66..36977585b 100644 --- a/workflow-canvas/src/workflow/workflow-source.test.ts +++ b/workflow-canvas/src/workflow/workflow-source.test.ts @@ -648,14 +648,14 @@ describe('createWorkflowSource session memory', () => { const memory = memoryWith(KEPT); const pending = { token: 'pending-token', harness: 'kiro-cli' }; memory.holdPending(pending); - const api = { - createSession: vi.fn(), - getWorkflow: vi.fn((token: string) => + const api = apiWith( + vi.fn(), + vi.fn((token: string) => token === pending.token ? Promise.reject(new ApiError('Server error.', 500)) : Promise.resolve(HELLO_DOCUMENT), ), - }; + ); const source = createWorkflowSource(api, undefined, memory); await source.current();