diff --git a/workflow-canvas/README.md b/workflow-canvas/README.md index 73c7e0f7e..b8ea80ed1 100644 --- a/workflow-canvas/README.md +++ b/workflow-canvas/README.md @@ -5,18 +5,20 @@ independent npm project (Vite, React, TypeScript, Cloudscape) and is not part of the agent package or the CLI. The page has Designer and Execution tabs. The Designer tab loads the workflow -from the API and draws it, read-only: the intent, each stage in list order -(name, agent and harness) and the end, joined by run-order edges. It shows a +from the API and draws it: the intent, each stage in list order (name, agent +and harness) and the end, joined by run-order edges. You can pick the agent of +a stage, rename it, add a stage and delete one. It shows a loading message, an error message or an empty message when there is nothing to -draw. The Execution tab draws the same graph and runs it: a Run button starts -a run, and each node shows its live state until the run ends. The contract, the -stub that serves it and the tests around both are in this folder. +draw. The Execution tab draws the saved workflow and runs it: a Run button +starts a run, and each node shows its live state until the run ends. The +contract, the stub that serves it and the tests around both are in this +folder. ## Layout - `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/` edits it and `canvas/` draws it; `execution/` runs it and shows the run; `execution/core/` does no IO and reads a run once a second. A read that fails with no answer, a server error, 408 or 429 is tried again, up to three in a row; any other failure ends the following. A start @@ -300,16 +302,81 @@ The rest wraps the core, so the rules can be tested without a browser: the workflow or error already on the page stays, and the 120 seconds end the trade. - `designer/` shows the result: loading, error, empty (no file yet, or a file - with errors, with their messages), or the canvas. + with errors, with their messages), or the canvas with its editing (next + section). - `canvas/` draws the graph with [React Flow](https://reactflow.dev) - (`@xyflow/react`). Nodes cannot be moved, selected or connected; the view - can be panned, and zoomed with the controls. Each node can be reached with - the Tab key and has a name for screen readers. The view starts zoomed out + (`@xyflow/react`). Nodes cannot be moved or connected; the view can be + panned, and zoomed with the controls. On the Designer tab a stage can be + selected, with a click or with Enter or Space on the node; the Execution tab + is read-only. Each node can be reached with the Tab key and has a name for + screen readers, which ends with "selected" for the selected stage. The view + fits the graph again when a stage is added or deleted. The view starts zoomed out far enough to show the longest workflow the API allows (40 stages), where the text is small; use the zoom controls to read it. A node cuts a long name or outcome short, and keeps the whole text in its `title`; an outcome too long for the node is also written out in full under the graph. +## The Designer tab + +The tab has the graph on the left, with **Add stage** above it, and +**Properties** on the right. Properties shows the selected stage (the first +one when the page opens): its `name` and its `agent`, and **Delete stage**. A +stage is only a `name` and an `agent`, and the stages run in the order they are +listed, so there is no other field and no way to link or reorder them yet. + +- **Add stage** adds a stage at the end, named `agent-1` (the first free + `agent-N`) with the agent `k-developer`, selects it and moves focus to its + name. It is off at 40 stages, the most the API allows. +- Typing in **name** renames the stage, and the node changes at once. The name + is taken as typed; the API is what says whether it is a good name. The + **Complete** or **Needs a fix** mark of a stage shows only for a draft that a + check has answered for, so it never says a draft is complete before it was + checked. +- **agent** is a list of the agents that Konductor ships, each with a line on + what it is for. The API does not look an agent up in this version, so the + list is part of the page (`designer/core/agents.ts`); a file that names an + agent not on the list keeps it, shown first, and keeps it on offer when + another agent is chosen. There is no field to type any other id yet. +- **Delete stage** removes the stage, selects the one before it (or the next, + for the first) and moves focus to its name. A workflow keeps at least one + stage, so it is off for the last. +- Choosing a stage by its node (click, or Enter or Space) selects it and moves + focus to its name. Enter or Space on the stage that is already selected also + moves focus to its name; a click on it changes nothing. Escape on a node + leaves focus on it. A polite live region says what was added, deleted or + selected. +- **A read-only workflow** (the server says `read_only`) can be looked at, and + a stage can be selected, but the name is read-only and Add stage, Delete + stage and the agent list are off. A note above the graph says why, and the + buttons carry the reason in a tooltip. +- **Focus and keys.** Everything above works with the keyboard: Tab to **Add + stage** and press Enter; the new stage's name has focus. A button that is off + (Add stage, Delete stage) stays focusable and gives the reason in a tooltip, + so focus stays on the button that was pressed. +- **The two tabs.** The draft lives in `App`, so it is kept when you switch + tabs. The Execution tab draws the saved workflow, because a run uses the + saved file, and says so when the Designer holds changes that are not saved. + +The page does not write the file in this version: the draft is only kept in the +page. So while the draft differs from the file, closing or reloading the page +makes the browser ask whether to leave (`useLeaveGuard`, a `beforeunload` +handler that is on only then). A change that is put back, so the draft equals +the file again, does not ask. + +The code follows the split of the Execution tab: + +- `src/designer/core/` is the core and does no IO: the draft of the stages + (`draft.ts`), the text a save would write (`stage-text.ts`), the findings put + on the stage and field they are about (`problems.ts`), and the state of the + editor with its reducer. `test-support/designer-purity.test.ts` fails if the + core imports anything but its own modules and the wire types, or reads a + clock, randomness, the environment or the network. +- `useEditor` keeps the state in `App`; `DesignerEditor` and `PropertiesPane` + draw it. `useLeaveGuard` asks the browser to confirm leaving while the draft + has changes. A stage keeps an id of its own while it is renamed, and the node is + named from that id, so two stages with one name for a moment still have two + nodes. + ## The Execution tab The tab draws the same graph as the Designer, read-only, with a **Goal** box @@ -505,7 +572,9 @@ the theme button is pressed). The Execution tab is saved before a run (`execution-tab-idle.png`), while the first stage runs (`execution-tab-running.png`) and after the run passed (`execution-tab-finished.png`), all in the light theme, and while the first -stage runs in the dark theme (`execution-tab-dark.png`). Pull request +stage runs in the dark theme (`execution-tab-dark.png`). The Designer tab is +also saved with edits in the draft (`designer-editing.png`, and +`designer-editing-dark.png` in the dark theme). Pull request descriptions embed these files, so regenerate and commit them when the UI changes. diff --git a/workflow-canvas/api/agents.test.ts b/workflow-canvas/api/agents.test.ts new file mode 100644 index 000000000..66b79ccac --- /dev/null +++ b/workflow-canvas/api/agents.test.ts @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 +// @vitest-environment node +/** Checks that the agents the Designer offers are the specialists in `agents/`. */ +import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; +import { KNOWN_AGENTS } from '../src/designer/core/agents'; + +const AGENTS_DIR = fileURLToPath(new URL('../../agents/', import.meta.url)); +const SUFFIX = '.agent-spec.json'; +// This project can be used without the rest of the repository, which holds +// `agents/`; then there is nothing to compare with. +const inRepository = existsSync(AGENTS_DIR); + +// The specialists: their specs are named `k-`, unlike the orchestrators. +// A new specialist must follow that name, or this test does not see it. +function specialists(): string[] { + return readdirSync(AGENTS_DIR) + .filter((file) => file.startsWith('k-') && file.endsWith(SUFFIX)) + .map((file) => file.slice(0, -SUFFIX.length)) + .sort(); +} + +describe.skipIf(!inRepository)('the agents that the Designer offers', () => { + it('are exactly the specialists that have a spec in agents/', () => { + expect(KNOWN_AGENTS.map((agent) => agent.id).sort()).toEqual(specialists()); + }); + + it.each(KNOWN_AGENTS.map((agent) => agent.id))( + 'name %s in its own spec', + (id) => { + const spec = JSON.parse( + readFileSync(`${AGENTS_DIR}${id}${SUFFIX}`, 'utf8'), + ) as { name?: string }; + + expect(spec.name).toBe(id); + }, + ); +}); diff --git a/workflow-canvas/e2e/capture/screenshots.spec.ts b/workflow-canvas/e2e/capture/screenshots.spec.ts index 82432e8eb..24f3bf698 100644 --- a/workflow-canvas/e2e/capture/screenshots.spec.ts +++ b/workflow-canvas/e2e/capture/screenshots.spec.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -import { expect, test } from '../support/fixtures'; +import { expect, test } from '../support/editing-fixtures'; import { answerRunsWith, runReply } from '../support/run-routes'; import { saveFullPageScreenshot } from '../support/screenshots'; import type { WorkflowCanvasPage } from '../support/workflow-canvas-page'; @@ -87,3 +87,29 @@ test('capture the Execution tab in the dark mode during a run', async ({ await saveFullPageScreenshot(canvas.page, 'execution-tab-dark'); }); + +// Editing. The stub's workflow is not touched: these only change the draft. +// These two change what a stage says but add none: a longer graph is fitted at +// a fractional zoom, and the edges of such a view differ by a level of grey +// between machines, which the byte-exact check cannot take. +test('capture the Designer tab with unsaved edits', async ({ designer }) => { + await designer.chooseAgent('k-architect'); + await designer.stage('review').click(); + await designer.rename('ship'); + await designer.stage('ship').waitFor(); + + await saveFullPageScreenshot(designer.page, 'designer-editing'); +}); + +test('capture the Designer tab with unsaved edits in the dark mode', async ({ + canvas, + designer, +}) => { + await canvas.switchTheme(); + await designer.chooseAgent('k-architect'); + await designer.stage('review').click(); + await designer.rename('ship'); + await designer.stage('ship').waitFor(); + + await saveFullPageScreenshot(designer.page, 'designer-editing-dark'); +}); diff --git a/workflow-canvas/e2e/screenshots/designer-editing-dark.png b/workflow-canvas/e2e/screenshots/designer-editing-dark.png new file mode 100644 index 000000000..d200628d0 Binary files /dev/null and b/workflow-canvas/e2e/screenshots/designer-editing-dark.png differ diff --git a/workflow-canvas/e2e/screenshots/designer-editing.png b/workflow-canvas/e2e/screenshots/designer-editing.png new file mode 100644 index 000000000..7d3abca00 Binary files /dev/null and b/workflow-canvas/e2e/screenshots/designer-editing.png differ diff --git a/workflow-canvas/e2e/screenshots/designer-tab-dark.png b/workflow-canvas/e2e/screenshots/designer-tab-dark.png index cb4c690d3..812bb83ef 100644 Binary files a/workflow-canvas/e2e/screenshots/designer-tab-dark.png and b/workflow-canvas/e2e/screenshots/designer-tab-dark.png differ diff --git a/workflow-canvas/e2e/screenshots/designer-tab.png b/workflow-canvas/e2e/screenshots/designer-tab.png index 05bdcd504..c8acc3da8 100644 Binary files a/workflow-canvas/e2e/screenshots/designer-tab.png and b/workflow-canvas/e2e/screenshots/designer-tab.png differ diff --git a/workflow-canvas/e2e/support/designer-page.ts b/workflow-canvas/e2e/support/designer-page.ts new file mode 100644 index 000000000..e01d9847b --- /dev/null +++ b/workflow-canvas/e2e/support/designer-page.ts @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { Locator, Page } from '@playwright/test'; + +/** Page object for the editing parts of the Designer tab. */ +export class DesignerPage { + readonly name: Locator; + readonly agent: Locator; + readonly add: Locator; + readonly remove: Locator; + readonly selected: Locator; + + constructor(readonly page: Page) { + this.name = page.getByRole('textbox', { name: 'name', exact: true }); + this.agent = page.getByRole('button', { name: /^agent / }); + this.add = page.getByRole('button', { name: 'Add stage' }); + this.remove = page.getByRole('button', { name: 'Delete stage' }); + this.selected = page.getByText('Selected:'); + } + + /** The node of the stage with the name. */ + stage(name: string): Locator { + return this.page.getByTestId(`node-stage-${name}`); + } + + /** Types the name into the name field, replacing what is there. */ + async rename(name: string): Promise { + await this.name.fill(name); + } + + /** Opens the list of agents and picks one. */ + async chooseAgent(id: string): Promise { + await this.agent.click(); + await this.page + .getByRole('option', { name: new RegExp(`^${id}\\b`) }) + .click(); + } +} diff --git a/workflow-canvas/e2e/support/editing-fixtures.ts b/workflow-canvas/e2e/support/editing-fixtures.ts new file mode 100644 index 000000000..053b34b4d --- /dev/null +++ b/workflow-canvas/e2e/support/editing-fixtures.ts @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: Apache-2.0 +import { DesignerPage } from './designer-page'; +import { test as base } from './fixtures'; + +export { expect } from './fixtures'; + +/** `test` for specs that edit on the Designer tab. */ +export const test = base.extend<{ designer: DesignerPage }>({ + designer: async ({ canvas }, use) => { + await canvas.expectWorkflowDrawn(); + await use(new DesignerPage(canvas.page)); + }, +}); diff --git a/workflow-canvas/e2e/support/workflow-canvas-page.ts b/workflow-canvas/e2e/support/workflow-canvas-page.ts index 4c57275fe..6967bbf0a 100644 --- a/workflow-canvas/e2e/support/workflow-canvas-page.ts +++ b/workflow-canvas/e2e/support/workflow-canvas-page.ts @@ -19,10 +19,13 @@ export const NODE_IDS = [ /** The test id of one node. */ export type NodeId = (typeof NODE_IDS)[number]; -/** What a screen reader says for each node, top to bottom. */ +/** + * What a screen reader says for each node of the Designer tab, top to bottom. + * The first stage is the one that Properties shows when the page opens. + */ export const NODE_NAMES = [ 'Intent: Add a greeting to the README', - 'Stage draft, agent k-developer, harness kiro-cli', + 'Stage draft, agent k-developer, harness kiro-cli, selected', 'Stage review, agent k-quality-assurance, harness kiro-cli', 'End', ] as const; diff --git a/workflow-canvas/e2e/tests/canvas.spec.ts b/workflow-canvas/e2e/tests/canvas.spec.ts index caefaa569..8a4371c4b 100644 --- a/workflow-canvas/e2e/tests/canvas.spec.ts +++ b/workflow-canvas/e2e/tests/canvas.spec.ts @@ -266,7 +266,7 @@ test('reaches each node with the Tab key and names it', async ({ canvas }) => { await expect(node).toBeFocused(); await expect(node).toHaveAccessibleName(name); - await expect(node).toHaveAccessibleDescription(/^Read-only workflow graph/); + await expect(node).toHaveAccessibleDescription(/^Workflow graph node\./); await expect(node.locator('.wf-focus-ring')).toBeVisible(); } }); diff --git a/workflow-canvas/e2e/tests/designer.spec.ts b/workflow-canvas/e2e/tests/designer.spec.ts new file mode 100644 index 000000000..0d9e915dd --- /dev/null +++ b/workflow-canvas/e2e/tests/designer.spec.ts @@ -0,0 +1,97 @@ +// SPDX-License-Identifier: Apache-2.0 +import { STUB_LAUNCH_CODE } from '../../playwright.shared'; +import { test as editing, expect } from '../support/editing-fixtures'; +import { test as base } from '../support/fixtures'; +import { sharedSession } from '../support/session'; +import { WorkflowCanvasPage } from '../support/workflow-canvas-page'; + +// The stage that is selected when the page opens, named the way a screen +// reader hears it. +const SELECTED_DRAFT = /^Stage draft, .*, selected$/; + +editing( + 'moves focus to the name when Enter is pressed on the selected stage', + async ({ designer }) => { + const node = designer.page.getByRole('group', { name: SELECTED_DRAFT }); + await node.focus(); + + await designer.page.keyboard.press('Enter'); + + await expect(designer.name).toBeFocused(); + }, +); + +editing( + 'moves focus to the name when Space is pressed on the selected stage', + async ({ designer }) => { + const node = designer.page.getByRole('group', { name: SELECTED_DRAFT }); + await node.focus(); + + await designer.page.keyboard.press('Space'); + + await expect(designer.name).toBeFocused(); + }, +); + +editing( + 'replaces the generated name of a new stage when the person types', + async ({ designer }) => { + await designer.add.click(); + await expect(designer.name).toBeFocused(); + + await designer.page.keyboard.type('kb-stage'); + + await expect(designer.name).toHaveValue('kb-stage'); + }, +); + +editing( + 'keeps focus on a stage when Escape is pressed on it', + async ({ designer }) => { + const node = designer.page.getByRole('group', { name: SELECTED_DRAFT }); + await node.focus(); + + await designer.page.keyboard.press('Escape'); + + await expect(node).toBeFocused(); + }, +); + +// The Designer is a grid of the graph and Properties; a note above them takes +// the whole row, so the graph keeps the wide column. Layout is not drawn in +// jsdom, so this needs a browser. +base( + 'keeps the graph wide and beside Properties under the note of a read-only workflow', + async ({ page }) => { + await page.route('**/api/session', (route) => + route.fulfill({ json: sharedSession() }), + ); + await page.route('**/api/workflow', async (route) => { + const response = await route.fetch(); + const document = (await response.json()) as object; + await route.fulfill({ json: { ...document, read_only: true } }); + }); + const canvas = new WorkflowCanvasPage(page); + await canvas.open(STUB_LAUNCH_CODE); + await canvas.expectWorkflowDrawn(); + + const note = await page.getByTestId('read-only-note').boundingBox(); + const [, , card, properties] = await page + .locator('.wf-designer > *') + .evaluateAll((items) => + items.map((item) => { + const { x, y, width, height } = item.getBoundingClientRect(); + return { x, y, width, height }; + }), + ); + + expect(note).not.toBeNull(); + expect(card!.width).toBeGreaterThan(properties!.width); + expect(card!.x).toBeLessThan(properties!.x); + expect(card!.y).toBeGreaterThanOrEqual(note!.y + note!.height); + expect(Math.abs(card!.y - properties!.y)).toBeLessThan(1); + expect(note!.x + note!.width).toBeGreaterThanOrEqual( + properties!.x + properties!.width - 1, + ); + }, +); diff --git a/workflow-canvas/e2e/tests/leave-page.spec.ts b/workflow-canvas/e2e/tests/leave-page.spec.ts new file mode 100644 index 000000000..87b08ef2a --- /dev/null +++ b/workflow-canvas/e2e/tests/leave-page.spec.ts @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: Apache-2.0 +import { expect, test } from '../support/editing-fixtures'; +import type { Page } from '@playwright/test'; + +// The draft lives in the page only, so closing or reloading the page throws +// it away. The browser asks first, but only if the page asks it to. Nothing +// here saves, so the stub keeps its workflow. `close` with `runBeforeUnload` +// is the way a test leaves the page like a person does: it returns before the +// browser has asked or closed, so each helper waits for what it expects. + +/** Leaves the page and returns the kind of dialog the browser asked, then stays. */ +async function leaveAndStay(page: Page): Promise { + const asked = page.waitForEvent('dialog'); + await page.close({ runBeforeUnload: true }); + const dialog = await asked; + await dialog.dismiss(); + return dialog.type(); +} + +/** Leaves the page and returns the kinds of dialog asked until it closed. */ +async function leaveWithoutStaying(page: Page): Promise { + const asked: string[] = []; + page.on('dialog', (dialog) => { + asked.push(dialog.type()); + void dialog.dismiss(); + }); + const closed = page.waitForEvent('close'); + await page.close({ runBeforeUnload: true }); + await closed; + return asked; +} + +test('asks before the page is closed with changes that are not saved', async ({ + designer, +}) => { + // A click first: the browser asks only after the person used the page. + await designer.stage('draft').click(); + await designer.rename('drafting'); + + expect(await leaveAndStay(designer.page)).toBe('beforeunload'); + + expect(designer.page.isClosed()).toBe(false); + await expect(designer.name).toHaveValue('drafting'); +}); + +test('lets the page go without asking when nothing has changed', async ({ + designer, +}) => { + await designer.stage('draft').click(); + + expect(await leaveWithoutStaying(designer.page)).toEqual([]); +}); + +test('lets the page go without asking when the change is put back', async ({ + designer, +}) => { + await designer.stage('draft').click(); + await designer.rename('drafting'); + await designer.rename('draft'); + + expect(await leaveWithoutStaying(designer.page)).toEqual([]); +}); diff --git a/workflow-canvas/e2e/tests/swap.spec.ts b/workflow-canvas/e2e/tests/swap.spec.ts new file mode 100644 index 000000000..f5a894919 --- /dev/null +++ b/workflow-canvas/e2e/tests/swap.spec.ts @@ -0,0 +1,55 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { Page } from '@playwright/test'; +import { expect, test } from '../support/editing-fixtures'; +import { sharedSession } from '../support/session'; + +const NEW_CODE = 'nQ3x7vJ2kLmN9pRs4TuVw5Y_-ab12'; + +// Answers the next session call with another harness, and records the calls. +async function offerOtherSessions(page: Page): Promise { + const sessions: string[] = []; + await page.route('**/api/session', (route) => { + sessions.push(route.request().postData() ?? ''); + return route.fulfill({ + json: { ...sharedSession(), harness: 'other-harness' }, + }); + }); + await page.route('**/barrier', (route) => route.fulfill({ body: 'ok' })); + return sessions; +} + +// Pastes the launch address and returns what the page asked in the dialog. +async function pasteCode(page: Page, accept: boolean): Promise { + const asked = page.waitForEvent('dialog'); + await page.evaluate((code) => (window.location.hash = code), NEW_CODE); + const dialog = await asked; + const message = dialog.message(); + await (accept ? dialog.accept() : dialog.dismiss()); + return message; +} + +test('asks before a pasted launch address drops unsaved changes, and keeps them when declined', async ({ + designer, +}) => { + const sessions = await offerOtherSessions(designer.page); + await designer.rename('drafting'); + + const question = await pasteCode(designer.page, false); + + expect(question).toContain('drops your unsaved changes'); + await designer.page.evaluate(() => fetch('/barrier')); + expect(sessions).toEqual([]); + await expect(designer.name).toHaveValue('drafting'); +}); + +test('opens the pasted workflow when the person lets the changes go', async ({ + designer, +}) => { + await offerOtherSessions(designer.page); + await designer.rename('drafting'); + + await pasteCode(designer.page, true); + + await expect(designer.stage('draft')).toContainText('other-harness'); + await expect(designer.name).toHaveValue('draft'); +}); diff --git a/workflow-canvas/src/App.test.tsx b/workflow-canvas/src/App.test.tsx index 8e782fce7..be5861139 100644 --- a/workflow-canvas/src/App.test.tsx +++ b/workflow-canvas/src/App.test.tsx @@ -1,9 +1,22 @@ // SPDX-License-Identifier: Apache-2.0 -import { fireEvent, render, screen, within } from '@testing-library/react'; -import { afterEach, describe, expect, it } from 'vitest'; +import { + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import { MODE_STORAGE_KEY } from './theme/mode'; import { App } from './App'; -import { HELLO_DOCUMENT, failed, loaded } from './test-support/fixtures'; +import { FakeScheduler } from './test-support/fake-scheduler'; +import { + HELLO_DOCUMENT, + failed, + helloRun, + loaded, +} from './test-support/fixtures'; +import { createSwapRisk } from './launch/swap-guard'; import type { LoadResult } from './workflow/load-workflow'; const settled = (result: LoadResult) => Promise.resolve(result); @@ -154,3 +167,48 @@ describe('App', () => { expect(window.localStorage.getItem(MODE_STORAGE_KEY)).toBeNull(); }); }); + +describe('App and the risk of opening another workflow', () => { + it('has nothing at risk before anything is changed or run', async () => { + const swapRisk = createSwapRisk(); + render(); + await screen.findByTestId('node-end'); + + expect(swapRisk.get()).toEqual({ draft: false, run: false }); + }); + + it('reports unsaved changes in the Designer', async () => { + const swapRisk = createSwapRisk(); + render(); + await screen.findByTestId('node-end'); + + fireEvent.change(screen.getByRole('textbox', { name: 'name' }), { + target: { value: 'write' }, + }); + + expect(swapRisk.get()).toEqual({ draft: true, run: false }); + }); + + it('reports a run that is being followed', async () => { + const swapRisk = createSwapRisk(); + const runs = { + start: vi.fn().mockResolvedValue(helloRun(0).runId), + get: vi.fn().mockResolvedValue(helloRun(0)), + }; + render( + , + ); + await screen.findByTestId('node-end'); + fireEvent.click(screen.getByRole('tab', { name: 'Execution' })); + + fireEvent.click(await screen.findByRole('button', { name: 'Run' })); + + await waitFor(() => + expect(swapRisk.get()).toEqual({ draft: false, run: true }), + ); + }); +}); diff --git a/workflow-canvas/src/App.tsx b/workflow-canvas/src/App.tsx index 3a25017f3..7b7522bf1 100644 --- a/workflow-canvas/src/App.tsx +++ b/workflow-canvas/src/App.tsx @@ -3,18 +3,22 @@ import AppLayout from '@cloudscape-design/components/app-layout'; import ContentLayout from '@cloudscape-design/components/content-layout'; import Header from '@cloudscape-design/components/header'; import Tabs from '@cloudscape-design/components/tabs'; -import type { ReactNode } from 'react'; +import { useEffect, type ReactNode } from 'react'; import { Breadcrumbs } from './chrome/Breadcrumbs'; import './chrome/chrome.css'; import { TOP_BAR_ID, TopBar } from './chrome/TopBar'; import { Designer } from './designer/Designer'; +import { isDirty } from './designer/core'; +import { useEditor } from './designer/use-editor'; +import { useLeaveGuard } from './designer/use-leave-guard'; import { useSettled } from './designer/use-settled'; -import type { RunMemory, Scheduler } from './execution/core'; +import { isInFlight, type RunMemory, type Scheduler } from './execution/core'; import { Execution } from './execution/Execution'; import { RunAnnouncements } from './execution/RunAnnouncements'; import { timerScheduler } from './execution/timer-scheduler'; import { useExecution } from './execution/use-execution'; import { useGoal } from './execution/use-goal'; +import type { SwapRisk } from './launch/swap-guard'; import { TAB_STRIP_STYLE } from './theme/tokens'; import { useThemeMode } from './theme/use-theme-mode'; import { openWorkflowName, type LoadResult } from './workflow/load-workflow'; @@ -30,6 +34,7 @@ export function App({ notice, scheduler = timerScheduler, runMemory, + swapRisk, }: { load: Promise; /** Shown above the tabs, such as the refusal of a pasted launch address. */ @@ -37,6 +42,8 @@ export function App({ scheduler?: Scheduler; /** Keeps the latest run across a reload; a page without it starts afresh. */ runMemory?: RunMemory; + /** Told what another workflow would take from the page. */ + swapRisk?: SwapRisk; }) { // Read here, not in the tab, so switching tabs does not load again. const result = useSettled(load); @@ -49,6 +56,17 @@ export function App({ ); const [goal, setGoal] = useGoal(result); const [mode, toggleMode] = useThemeMode(); + // The Designer edits a draft; the run and the Execution tab use the file + // as it is saved. + const editor = useEditor(result?.ok ? result.workflow : undefined); + const unsaved = editor ? isDirty(editor.state) : false; + // The draft lives in the page, so leaving the page loses the changes. + useLeaveGuard(unsaved, window); + const running = isInFlight(execution.state); + useEffect( + () => swapRisk?.set({ draft: unsaved, run: running }), + [swapRisk, unsaved, running], + ); return ( <> @@ -77,7 +95,7 @@ export function App({ { id: 'designer', label: 'Designer', - content: , + content: , }, { id: 'execution', @@ -88,6 +106,7 @@ export function App({ execution={execution} goal={goal} onGoalChange={setGoal} + unsaved={unsaved} /> ), }, diff --git a/workflow-canvas/src/canvas/FitWhenGrown.tsx b/workflow-canvas/src/canvas/FitWhenGrown.tsx new file mode 100644 index 000000000..9935049bd --- /dev/null +++ b/workflow-canvas/src/canvas/FitWhenGrown.tsx @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: Apache-2.0 +import { useReactFlow, useStore, type ReactFlowState } from '@xyflow/react'; +import { useEffect, useRef } from 'react'; +import { FIT_PADDING } from './view'; + +// How many nodes the canvas holds, and whether every one has been measured. +function measuredNodes(state: ReactFlowState): string { + const nodes = [...state.nodeLookup.values()]; + const measured = nodes.every((node) => node.measured.width !== undefined); + return `${nodes.length}:${measured}`; +} + +/** + * Fits the graph in the view again when it gains or loses a node, once every + * node is measured, so an added stage is never out of sight. It draws + * nothing. It must be a child of the canvas. + */ +export function FitWhenGrown({ count }: { count: number }) { + const { fitView } = useReactFlow(); + const seen = useStore(measuredNodes); + const fitted = useRef(count); + useEffect(() => { + if (seen !== `${count}:true` || fitted.current === count) return; + fitted.current = count; + void fitView({ padding: FIT_PADDING, maxZoom: 1 }); + }, [seen, count, fitView]); + return null; +} diff --git a/workflow-canvas/src/canvas/WorkflowCanvas.test.tsx b/workflow-canvas/src/canvas/WorkflowCanvas.test.tsx new file mode 100644 index 000000000..8b3064c69 --- /dev/null +++ b/workflow-canvas/src/canvas/WorkflowCanvas.test.tsx @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: Apache-2.0 +import { fireEvent, render, screen } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; +import { buildGraph } from '../graph'; +import { HELLO_VIEW } from '../test-support/fixtures'; +import { WorkflowCanvas, type CanvasSelection } from './WorkflowCanvas'; + +const GRAPH = buildGraph(HELLO_VIEW, 'kiro-cli'); + +// Draws the canvas under a parent that hears the keys which reach it. +function drawn(selection?: CanvasSelection) { + const heard = vi.fn(); + render( +
heard(event.key)}> + +
, + ); + return { heard, node: screen.getByTestId('node-stage-draft').parentElement! }; +} + +describe('WorkflowCanvas, on the Escape key', () => { + it('lets the key go on in a canvas that cannot select', () => { + const { heard, node } = drawn(); + + fireEvent.keyDown(node, { key: 'Escape' }); + + expect(heard).toHaveBeenCalledWith('Escape'); + }); + + it('lets the key go on to the page in a canvas that can select', () => { + const { heard, node } = drawn({ selectedId: undefined, onSelect: vi.fn() }); + + fireEvent.keyDown(node, { key: 'Escape' }); + + expect(heard).toHaveBeenCalledWith('Escape'); + }); + + it('lets an open dialog hear the key while a node has focus', () => { + const closed = vi.fn(); + const onKey = (event: KeyboardEvent) => { + if (event.key === 'Escape') closed(); + }; + document.addEventListener('keydown', onKey); + const { node } = drawn({ selectedId: 'stage-draft', onSelect: vi.fn() }); + + fireEvent.keyDown(node, { key: 'Escape' }); + document.removeEventListener('keydown', onKey); + + expect(closed).toHaveBeenCalledTimes(1); + }); + + it('keeps React Flow from handling the key on a node', () => { + const { node } = drawn({ selectedId: undefined, onSelect: vi.fn() }); + + expect(node.classList.contains('nokey')).toBe(true); + }); +}); + +describe('WorkflowCanvas, on Enter and Space', () => { + it('chooses the stage and lets the key go on', () => { + const onSelect = vi.fn(); + const { heard, node } = drawn({ selectedId: undefined, onSelect }); + + fireEvent.keyDown(node, { key: 'Enter' }); + + expect(onSelect).toHaveBeenCalledWith(node.dataset.id); + expect(heard).toHaveBeenCalledWith('Enter'); + }); +}); diff --git a/workflow-canvas/src/canvas/WorkflowCanvas.tsx b/workflow-canvas/src/canvas/WorkflowCanvas.tsx index a2753a736..02a7785b2 100644 --- a/workflow-canvas/src/canvas/WorkflowCanvas.tsx +++ b/workflow-canvas/src/canvas/WorkflowCanvas.tsx @@ -6,13 +6,17 @@ import { MarkerType, ReactFlow, type DefaultEdgeOptions, + type NodeChange, type NodeTypes, } from '@xyflow/react'; import '@xyflow/react/dist/style.css'; +import { useCallback, useMemo, type KeyboardEvent } from 'react'; import type { CanvasGraph } from '../graph'; import './canvas.css'; -import { READ_ONLY_ARIA_LABELS } from './aria-labels'; +import { READ_ONLY_ARIA_LABELS, SELECTABLE_ARIA_LABELS } from './aria-labels'; +import { FitWhenGrown } from './FitWhenGrown'; import { EndNode, IntentNode, StageNode } from './nodes'; +import { markSelectable } from './selection'; import { CANVAS_HEIGHT, FIT_PADDING, MIN_ZOOM } from './view'; const NODE_TYPES: NodeTypes = { @@ -32,35 +36,88 @@ const EDGE_OPTIONS: DefaultEdgeOptions = { }, }; +/** Which stage is selected and what to do when another one is chosen. */ +export interface CanvasSelection { + /** The id of the selected node. */ + selectedId: string | undefined; + /** + * Called with the id of the stage node that was clicked, or chosen with + * Enter or Space, also when it is the selected one already. + */ + onSelect(nodeId: string): void; +} + /** - * Draws a workflow graph, read-only. The view can be panned and zoomed, but - * nodes cannot be moved, selected, connected or edited. Each node can still - * be reached with the Tab key, which also brings it into view. + * Draws a workflow graph. The view can be panned and zoomed, but nodes + * cannot be moved or connected. With `selection`, a stage can be chosen by a + * click, or with the keyboard, Enter or Space on the node; without it the + * canvas is read-only. Each node can be reached with the Tab key, which also + * brings it into view. */ -export function WorkflowCanvas({ graph }: { graph: CanvasGraph }) { +export function WorkflowCanvas({ + graph, + selection, +}: { + graph: CanvasGraph; + selection?: CanvasSelection; +}) { + const selectedId = selection?.selectedId; + const onSelect = selection?.onSelect; + const nodes = useMemo( + () => (onSelect ? markSelectable(graph.nodes, selectedId) : graph.nodes), + [graph.nodes, onSelect, selectedId], + ); + // The nodes carry React Flow's `nokey` class (see markSelectable), so it + // leaves their keys alone: Escape no longer drops focus, and a key is not + // stopped on its way to the page. Enter or Space on a stage chooses it here, + // also when it is the selected one already. + const onKeyDown = useCallback( + (event: KeyboardEvent) => { + const node = event.target; + if (!onSelect || !(node instanceof HTMLElement)) return; + if (!node.classList.contains('react-flow__node-stage')) return; + if (event.key !== 'Enter' && event.key !== ' ') return; + event.preventDefault(); + onSelect(node.dataset.id ?? ''); + }, + [onSelect], + ); + const onNodesChange = useCallback( + (changes: NodeChange[]) => { + for (const change of changes) { + if (change.type === 'select' && change.selected) onSelect?.(change.id); + } + }, + [onSelect], + ); return (
+ INTENT_FITS_IN_NODE; if (!showIntent && stages.length === 0) return null; @@ -20,8 +25,8 @@ function FullText({ view, harness }: { view: WorkflowView; harness: string }) { Intent: {view.intent.outcome} )} - {stages.map(({ name, agent }) => ( - + {stages.map(({ id, name, agent }) => ( + Stage {name}: {agent} · {harness} ))} @@ -31,18 +36,31 @@ function FullText({ view, harness }: { view: WorkflowView; harness: string }) { const NO_STATUSES: ReadonlyMap = new Map(); +const READ_ONLY_NOTE = + 'Read-only. The stages run in the order they are listed.'; +const EDIT_NOTE = + 'Select a stage to edit it. The stages run in the order they are listed.'; + /** - * The card both tabs draw the workflow in: its name, then the read-only graph. - * `statuses` puts a live state on the nodes it names, by node id. + * The card both tabs draw the workflow in: its name, then the graph. The + * graph is read-only unless `selection` is given, and `statuses` puts a live + * state on the nodes it names, by node id. `toolbar` sits above the graph. */ export function WorkflowGraphCard({ view, harness, statuses = NO_STATUSES, + selection, + readOnly = false, + toolbar, }: { - view: WorkflowView; + view: WorkflowSpec; harness: string; statuses?: ReadonlyMap; + selection?: CanvasSelection; + /** The workflow cannot be changed, so the note says it is read-only. */ + readOnly?: boolean; + toolbar?: ReactNode; }) { const graph = useMemo( () => withStatuses(buildGraph(view, harness), statuses), @@ -53,13 +71,16 @@ export function WorkflowGraphCard({ header={
Workflow: {view.name}
} > - + + {toolbar} + + ); diff --git a/workflow-canvas/src/canvas/aria-labels.ts b/workflow-canvas/src/canvas/aria-labels.ts index f747901f1..6d6eccab0 100644 --- a/workflow-canvas/src/canvas/aria-labels.ts +++ b/workflow-canvas/src/canvas/aria-labels.ts @@ -14,3 +14,17 @@ export const READ_ONLY_ARIA_LABELS: Partial = { 'node.a11yDescription.ariaLiveMessage': () => '', 'edge.a11yDescription.default': READ_ONLY, }; + +const SELECTABLE = + 'Workflow graph node. On a stage, press Enter or Space to edit it in Properties. The intent and the end cannot be edited.'; + +/** + * Instructions for a canvas on which a stage can be selected. Nothing else + * can be done to a node, and the nodes still cannot be moved or connected. + */ +export const SELECTABLE_ARIA_LABELS: Partial = { + 'node.a11yDescription.default': SELECTABLE, + 'node.a11yDescription.keyboardDisabled': SELECTABLE, + 'node.a11yDescription.ariaLiveMessage': () => '', + 'edge.a11yDescription.default': SELECTABLE, +}; diff --git a/workflow-canvas/src/canvas/canvas.css b/workflow-canvas/src/canvas/canvas.css index 527c41348..ed2571def 100644 --- a/workflow-canvas/src/canvas/canvas.css +++ b/workflow-canvas/src/canvas/canvas.css @@ -319,3 +319,13 @@ color: CanvasText; } } + +/* + * The stage that Properties shows. Its outline takes the focus colour and is + * thicker, so the choice shows without colour too. The name of the node says + * "selected" for a screen reader. + */ +.react-flow__node.selected .wf-node-body { + stroke: var(--wf-focus); + stroke-width: 3px; +} diff --git a/workflow-canvas/src/canvas/clipping.ts b/workflow-canvas/src/canvas/clipping.ts index 0a39a649b..fd25ebd72 100644 --- a/workflow-canvas/src/canvas/clipping.ts +++ b/workflow-canvas/src/canvas/clipping.ts @@ -1,5 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 -import type { WorkflowView } from '../api/types'; +import type { StageSpec, WorkflowSpec } from '../graph'; /** * The node geometry that the limits below were estimated for. `canvas.css` @@ -44,7 +44,10 @@ export function textWidth(text: string): number { } /** The stages of `view` whose name or agent line a node cuts short. */ -export function clippedStages(view: WorkflowView, harness: string) { +export function clippedStages( + view: WorkflowSpec, + harness: string, +): StageSpec[] { return view.stages.filter( ({ name, agent }) => textWidth(name) > NAME_FITS_IN_NODE || diff --git a/workflow-canvas/src/canvas/selection.test.ts b/workflow-canvas/src/canvas/selection.test.ts new file mode 100644 index 000000000..2da1565dc --- /dev/null +++ b/workflow-canvas/src/canvas/selection.test.ts @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: Apache-2.0 +import { describe, expect, it } from 'vitest'; +import { buildGraph } from '../graph'; +import { markSelectable } from './selection'; + +const GRAPH = buildGraph( + { + name: 'hello', + intent: { outcome: 'Say hi' }, + stages: [ + { id: 's1', name: 'draft', agent: 'k-developer' }, + { id: 's2', name: 'review', agent: 'k-tpm' }, + ], + }, + 'kiro-cli', +); + +describe('markSelectable', () => { + const nodes = markSelectable(GRAPH.nodes, 'stage-s2'); + + it('lets only the stages be selected', () => { + expect(nodes.map((node) => node.selectable)).toEqual([ + false, + true, + true, + false, + ]); + }); + + it('marks the node with the id as selected, and no other', () => { + expect(nodes.map((node) => node.selected)).toEqual([ + false, + false, + true, + false, + ]); + }); + + it('says selected in the name of the selected node for a screen reader', () => { + expect(nodes[2]?.ariaLabel).toBe( + 'Stage review, agent k-tpm, harness kiro-cli, selected', + ); + expect(nodes[1]?.ariaLabel).toBe( + 'Stage draft, agent k-developer, harness kiro-cli', + ); + }); + + it('selects nothing when no id is given', () => { + expect( + markSelectable(GRAPH.nodes, undefined).some((node) => node.selected), + ).toBe(false); + }); + + it('does not change the graph it is given', () => { + expect(GRAPH.nodes.some((node) => 'selected' in node)).toBe(false); + }); +}); diff --git a/workflow-canvas/src/canvas/selection.ts b/workflow-canvas/src/canvas/selection.ts new file mode 100644 index 000000000..4bddcc2bb --- /dev/null +++ b/workflow-canvas/src/canvas/selection.ts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: Apache-2.0 +import type { GraphNode } from '../graph'; + +/** + * The nodes with the selection drawn on them. Only a stage can be selected, + * and the selected one says so in its name for a screen reader. The `nokey` + * class makes React Flow skip its own key handling on the node. + */ +export function markSelectable( + nodes: readonly GraphNode[], + selectedId: string | undefined, +) { + return nodes.map((node) => { + const selected = node.id === selectedId; + return { + ...node, + selected, + selectable: node.type === 'stage', + className: 'nokey', + ariaLabel: selected ? `${node.ariaLabel}, selected` : node.ariaLabel, + }; + }); +} diff --git a/workflow-canvas/src/designer/Designer.tsx b/workflow-canvas/src/designer/Designer.tsx index 07446f1e0..4fb1316e2 100644 --- a/workflow-canvas/src/designer/Designer.tsx +++ b/workflow-canvas/src/designer/Designer.tsx @@ -2,18 +2,31 @@ import { WorkflowGraphCard } from '../canvas/WorkflowGraphCard'; import type { LoadResult } from '../workflow/load-workflow'; import { WorkflowGate } from '../workflow/WorkflowGate'; +import { DesignerEditor } from './DesignerEditor'; +import type { Editor } from './use-editor'; /** - * The Designer tab: the workflow drawn as a graph, or a message while it - * loads (`result` is `undefined`), when it failed to load, or when there is - * nothing to draw. + * The Designer tab: the workflow as a graph with its stages editable, or a + * message while it loads (`result` is `undefined`), when it failed to load, + * or when there is nothing to draw. Without an `editor` the graph is only + * drawn. */ -export function Designer({ result }: { result: LoadResult | undefined }) { +export function Designer({ + result, + editor, +}: { + result: LoadResult | undefined; + editor?: Editor; +}) { return ( - {({ view, harness }) => ( - - )} + {({ view, harness }) => + editor ? ( + + ) : ( + + ) + } ); } diff --git a/workflow-canvas/src/designer/DesignerEditor.test.tsx b/workflow-canvas/src/designer/DesignerEditor.test.tsx new file mode 100644 index 000000000..2903b3298 --- /dev/null +++ b/workflow-canvas/src/designer/DesignerEditor.test.tsx @@ -0,0 +1,484 @@ +// SPDX-License-Identifier: Apache-2.0 +import createWrapper from '@cloudscape-design/components/test-utils/dom'; +import { + fireEvent, + render, + screen, + waitFor, + within, +} from '@testing-library/react'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { App } from '../App'; +import { FakeScheduler } from '../test-support/fake-scheduler'; +import { HELLO_DOCUMENT, HELLO_VIEW, loaded } from '../test-support/fixtures'; +import type { WorkflowDocument } from '../api/types'; + +// The Cloudscape test utils wrap their updates in `act`, which React only +// accepts when the environment says so. Testing Library does that on its own +// only with test globals, so it is set for this file. +const actFlag = globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }; +const actBefore = actFlag.IS_REACT_ACT_ENVIRONMENT; +beforeAll(() => { + actFlag.IS_REACT_ACT_ENVIRONMENT = true; +}); +afterAll(() => { + actFlag.IS_REACT_ACT_ENVIRONMENT = actBefore; +}); + +async function openDesigner(document: WorkflowDocument = HELLO_DOCUMENT) { + render( + , + ); + await screen.findByTestId('node-end'); +} + +const wrapper = () => createWrapper(document.body); +const nameInput = () => + screen.getByRole('textbox', { name: 'name' }) as HTMLInputElement; +const agentSelect = () => wrapper().findSelect()!; +// The reason of a button that is off, which the button carries after its label. +function reasonOf(label: string): string | undefined { + const text = wrapper() + .findAllButtons() + .map((button) => button.getElement().textContent ?? '') + .find((content) => content.startsWith(label)); + return text?.slice(label.length) || undefined; +} +const addButton = () => screen.getByRole('button', { name: 'Add stage' }); +const deleteButton = () => screen.getByRole('button', { name: 'Delete stage' }); +const isOff = (button: HTMLElement) => + button.hasAttribute('disabled') || + button.getAttribute('aria-disabled') === 'true'; +const type = (value: string) => + fireEvent.change(nameInput(), { target: { value } }); + +function chooseAgent(id: string) { + const select = agentSelect(); + select.openDropdown(); + const index = select + .findDropdown() + .findOptions() + .findIndex((option) => option.getElement().textContent?.startsWith(id)); + if (index < 0) throw new Error(`The agent ${id} is not offered.`); + select.selectOption(index + 1); +} + +const stageLabels = () => + screen + .getAllByRole('group') + .map((node) => node.getAttribute('aria-label') ?? '') + .filter((label) => label.startsWith('Stage ')); + +describe('the Properties of the selected stage', () => { + it('shows the name and agent of the first stage when the workflow opens', async () => { + await openDesigner(); + + expect(screen.getByRole('heading', { name: 'Properties' })).toBeTruthy(); + expect(screen.getByText('Selected:').textContent).toContain('draft'); + expect(nameInput().value).toBe('draft'); + expect(agentSelect().findTrigger().getElement().textContent).toContain( + 'k-developer', + ); + }); + + it('says in the name of the node which stage is selected', async () => { + await openDesigner(); + + expect(stageLabels()).toEqual([ + 'Stage draft, agent k-developer, harness kiro-cli, selected', + 'Stage review, agent k-quality-assurance, harness kiro-cli', + ]); + }); + + it('switches to a stage that is clicked, and moves focus to its name', async () => { + await openDesigner(); + + fireEvent.click(screen.getByTestId('node-stage-review')); + + await waitFor(() => expect(nameInput().value).toBe('review')); + expect(document.activeElement).toBe(nameInput()); + expect(stageLabels()[1]).toMatch(/, selected$/); + }); + + it('switches to a stage that is chosen with Enter on its node', async () => { + await openDesigner(); + const node = screen.getByRole('group', { + name: 'Stage review, agent k-quality-assurance, harness kiro-cli', + }); + + node.focus(); + fireEvent.keyDown(node, { key: 'Enter' }); + + await waitFor(() => expect(nameInput().value).toBe('review')); + expect(document.activeElement).toBe(nameInput()); + }); + + it('moves focus to the name when Enter is pressed on the stage that is already selected', async () => { + await openDesigner(); + const node = screen.getByRole('group', { + name: 'Stage draft, agent k-developer, harness kiro-cli, selected', + }); + + node.focus(); + fireEvent.keyDown(node, { key: 'Enter' }); + + await waitFor(() => expect(document.activeElement).toBe(nameInput())); + }); + + it('moves focus to the name when Space is pressed on the stage that is already selected', async () => { + await openDesigner(); + const node = screen.getByRole('group', { + name: 'Stage draft, agent k-developer, harness kiro-cli, selected', + }); + + node.focus(); + fireEvent.keyDown(node, { key: ' ' }); + + await waitFor(() => expect(document.activeElement).toBe(nameInput())); + }); + + it('keeps focus on a node when Escape is pressed on it', async () => { + await openDesigner(); + const node = screen.getByRole('group', { + name: 'Stage draft, agent k-developer, harness kiro-cli, selected', + }); + node.focus(); + + fireEvent.keyDown(node, { key: 'Escape' }); + await new Promise((done) => requestAnimationFrame(() => done(undefined))); + await new Promise((done) => requestAnimationFrame(() => done(undefined))); + + expect(document.activeElement).toBe(node); + }); + + it('does not select the intent or the end', async () => { + await openDesigner(); + + fireEvent.click(screen.getByTestId('node-end')); + fireEvent.click(screen.getByTestId('node-intent')); + + expect(nameInput().value).toBe('draft'); + }); + + it('labels the fields and describes the agent', async () => { + await openDesigner(); + + expect(nameInput().getAttribute('aria-describedby')).toBeTruthy(); + expect(screen.getByText(/A Konductor agent id/).textContent).toContain( + 'Backend and frontend code', + ); + }); +}); + +describe('renaming a stage', () => { + it('shows the new name on the node at once', async () => { + await openDesigner(); + + type('write'); + + expect(screen.getByTestId('node-stage-write').textContent).toContain( + 'write', + ); + }); + + it('keeps focus in the field while it is typed in', async () => { + await openDesigner(); + nameInput().focus(); + + type('write'); + type('write-it'); + + expect(document.activeElement).toBe(nameInput()); + expect(nameInput().value).toBe('write-it'); + }); + + it('lets two stages share a name for a moment without losing either node', async () => { + await openDesigner(); + + type('review'); + + expect(screen.getAllByTestId('node-stage-review')).toHaveLength(2); + }); + + it('draws the old name again when the name is put back', async () => { + await openDesigner(); + + type('write'); + type('draft'); + + expect(screen.queryByTestId('node-stage-write')).toBeNull(); + expect(screen.getByTestId('node-stage-draft')).toBeTruthy(); + }); +}); + +describe('picking the agent of a stage', () => { + it('offers the agents that Konductor ships, with what each is for', async () => { + await openDesigner(); + + agentSelect().openDropdown(); + + const options = agentSelect() + .findDropdown() + .findOptions() + .map((option) => option.getElement().textContent ?? ''); + expect(options).toHaveLength(8); + expect(options[2]).toContain('k-developer'); + expect(options[2]).toContain('Backend and frontend code'); + }); + + it('shows the chosen agent on the node', async () => { + await openDesigner(); + + chooseAgent('k-architect'); + + expect(screen.getByTestId('node-stage-draft').textContent).toContain( + 'k-architect · kiro-cli', + ); + }); + + it('keeps an agent that Konductor does not ship, and offers it first', async () => { + const odd = { + ...HELLO_DOCUMENT, + view: { + ...HELLO_VIEW, + stages: [{ name: 'draft', agent: 'my-agent' }], + }, + }; + await openDesigner(odd); + + expect(agentSelect().findTrigger().getElement().textContent).toContain( + 'my-agent', + ); + agentSelect().openDropdown(); + expect( + agentSelect().findDropdown().findOptions()[0]?.getElement().textContent, + ).toContain('my-agent'); + }); +}); + +describe('adding a stage', () => { + it('adds an agent stage at the end, selects it and moves focus to its name', async () => { + await openDesigner(); + addButton().focus(); + + fireEvent.click(addButton()); + + expect(screen.getByTestId('node-stage-agent-1').textContent).toContain( + 'k-developer', + ); + expect(nameInput().value).toBe('agent-1'); + await waitFor(() => expect(document.activeElement).toBe(nameInput())); + expect(stageLabels()).toHaveLength(3); + }); + + it('selects the generated name, so typing replaces it', async () => { + await openDesigner(); + + fireEvent.click(addButton()); + + await waitFor(() => expect(document.activeElement).toBe(nameInput())); + expect([nameInput().selectionStart, nameInput().selectionEnd]).toEqual([ + 0, + 'agent-1'.length, + ]); + }); + + it('does not select the name of a stage that is only clicked', async () => { + await openDesigner(); + + fireEvent.click(screen.getByTestId('node-stage-review')); + + await waitFor(() => expect(document.activeElement).toBe(nameInput())); + expect(nameInput().selectionStart).toBe(nameInput().selectionEnd); + }); + + it('names the next stage agent-2', async () => { + await openDesigner(); + + fireEvent.click(addButton()); + fireEvent.click(addButton()); + + expect(screen.getByTestId('node-stage-agent-2')).toBeTruthy(); + }); + + it('is off, and says why, at the most stages the API allows', async () => { + const stages = Array.from({ length: 40 }, (_, index) => ({ + name: `stage-${index}`, + agent: 'k-developer', + })); + await openDesigner({ + ...HELLO_DOCUMENT, + view: { ...HELLO_VIEW, stages }, + }); + + expect(isOff(addButton())).toBe(true); + expect(reasonOf('Add stage')).toBe('A workflow has at most 40 stages.'); + fireEvent.click(addButton()); + + expect(stageLabels()).toHaveLength(40); + }); +}); + +describe('deleting a stage', () => { + it('removes the selected stage, selects its neighbour and moves focus to the name', async () => { + await openDesigner(); + fireEvent.click(screen.getByTestId('node-stage-review')); + await waitFor(() => expect(nameInput().value).toBe('review')); + + fireEvent.click(deleteButton()); + + expect(screen.queryByTestId('node-stage-review')).toBeNull(); + expect(nameInput().value).toBe('draft'); + await waitFor(() => expect(document.activeElement).toBe(nameInput())); + }); + + it('is off, and says why, for the last stage', async () => { + await openDesigner(); + fireEvent.click(deleteButton()); + + expect(isOff(deleteButton())).toBe(true); + expect(reasonOf('Delete stage')).toBe( + 'A workflow needs at least one stage.', + ); + fireEvent.click(deleteButton()); + + expect(stageLabels()).toHaveLength(1); + }); +}); + +describe('the two tabs', () => { + it('keeps the draft when the tab is switched away and back', async () => { + await openDesigner(); + type('write'); + + fireEvent.click(screen.getByRole('tab', { name: 'Execution' })); + fireEvent.click(screen.getByRole('tab', { name: 'Designer' })); + + expect(await screen.findByTestId('node-stage-write')).toBeTruthy(); + expect(nameInput().value).toBe('write'); + }); + + it('draws the saved workflow on Execution and says the draft is not part of a run', async () => { + await openDesigner(); + type('write'); + + fireEvent.click(screen.getByRole('tab', { name: 'Execution' })); + + const panel = screen.getByRole('tabpanel', { name: 'Execution' }); + expect(await within(panel).findByTestId('node-stage-draft')).toBeTruthy(); + expect(within(panel).queryByTestId('node-stage-write')).toBeNull(); + expect(within(panel).getByTestId('unsaved-note')).toBeTruthy(); + }); +}); + +describe('a workflow that the server says is read-only', () => { + it('turns off every control that would change it', async () => { + await openDesigner({ + ...HELLO_DOCUMENT, + readOnly: true, + }); + + expect(nameInput().readOnly).toBe(true); + expect( + agentSelect().findTrigger().getElement().hasAttribute('disabled') || + agentSelect() + .findTrigger() + .getElement() + .getAttribute('aria-disabled') === 'true', + ).toBe(true); + expect(isOff(addButton())).toBe(true); + expect(isOff(deleteButton())).toBe(true); + expect(reasonOf('Add stage')).toBe('The workflow is read-only.'); + expect(reasonOf('Delete stage')).toBe('The workflow is read-only.'); + }); + + it('puts the note in a block that takes the whole row of the layout', async () => { + await openDesigner({ ...HELLO_DOCUMENT, readOnly: true }); + + const block = screen + .getByTestId('read-only-note') + .closest('.wf-designer-note'); + + expect(block?.parentElement?.classList.contains('wf-designer')).toBe(true); + }); + + it('says in a note that it is read-only', async () => { + await openDesigner({ ...HELLO_DOCUMENT, readOnly: true }); + + expect(screen.getByTestId('read-only-note').textContent).toContain( + 'The workflow is read-only.', + ); + expect(screen.queryByText(/Select a stage to edit it\./)).toBeNull(); + }); +}); + +describe('what a screen reader is told', () => { + const live = () => screen.getByTestId('designer-live').textContent; + + it('says a stage was added', async () => { + await openDesigner(); + + fireEvent.click(addButton()); + + expect(live()).toBe('Stage agent-1 added and selected.'); + }); + + it('says a second stage was added in different words', async () => { + await openDesigner(); + + fireEvent.click(addButton()); + const first = live(); + fireEvent.click(addButton()); + + expect(live()).toBe('Stage agent-2 added and selected.'); + expect(live()).not.toBe(first); + }); + + it('says again that the same stage is selected, in a new element', async () => { + await openDesigner(); + const region = () => screen.getByTestId('designer-live'); + const selectedNode = () => + screen.getByRole('group', { name: /^Stage review, .*, selected$/ }); + fireEvent.click(screen.getByTestId('node-stage-review')); + await waitFor(() => expect(live()).toBe('Stage review selected.')); + const first = region().firstElementChild; + + selectedNode().focus(); + fireEvent.keyDown(selectedNode(), { key: 'Enter' }); + + await waitFor(() => expect(region().firstElementChild).not.toBe(first)); + expect(live()).toBe('Stage review selected.'); + }); + it('says which stage was deleted', async () => { + await openDesigner(); + fireEvent.click(screen.getByTestId('node-stage-review')); + await waitFor(() => expect(nameInput().value).toBe('review')); + + fireEvent.click(deleteButton()); + + expect(live()).toBe('Stage review deleted.'); + }); + + it('says which stage was selected', async () => { + await openDesigner(); + + fireEvent.click(screen.getByTestId('node-stage-review')); + + await waitFor(() => expect(live()).toBe('Stage review selected.')); + }); +}); + +describe('the verdict on a stage', () => { + it('is not given for a draft that no check has spoken for', async () => { + await openDesigner(); + expect(screen.getByText('Complete')).toBeTruthy(); + + type('BAD NAME!!'); + + expect(screen.queryByText('Complete')).toBeNull(); + expect(screen.queryByText('Needs a fix')).toBeNull(); + }); +}); diff --git a/workflow-canvas/src/designer/DesignerEditor.tsx b/workflow-canvas/src/designer/DesignerEditor.tsx new file mode 100644 index 000000000..767df0eb2 --- /dev/null +++ b/workflow-canvas/src/designer/DesignerEditor.tsx @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: Apache-2.0 +import Button from '@cloudscape-design/components/button'; +import type { InputProps } from '@cloudscape-design/components/input'; +import Alert from '@cloudscape-design/components/alert'; +import { useCallback, useMemo, useRef, useState } from 'react'; +import { WorkflowGraphCard } from '../canvas/WorkflowGraphCard'; +import type { CanvasSelection } from '../canvas/WorkflowCanvas'; +import { stageNodeId } from '../graph'; +import { + canAddStage, + canEdit, + MAX_STAGES, + nextStageName, + READ_ONLY_REASON, +} from './core'; +import '../execution/announcements.css'; +import './designer.css'; +import { PropertiesPane } from './PropertiesPane'; +import { useFocusRequest } from './use-focus'; +import type { Editor } from './use-editor'; + +const STAGE_LIMIT = `A workflow has at most ${MAX_STAGES} stages.`; + +// The words for a screen reader. Each call gets a new element, so a repeated +// sentence, such as selecting the same stage twice, is read again. +function useAnnouncer(): [ + { text: string; count: number }, + (text: string) => void, +] { + const [announcement, set] = useState({ text: '', count: 0 }); + const announce = useCallback( + (text: string) => set((last) => ({ text, count: last.count + 1 })), + [], + ); + return [announcement, announce]; +} + +/** + * The editable Designer: the graph with Add stage, and Properties for the + * selected stage. Focus goes to the name of a stage after it is added, + * selected or deleted, so the keyboard never loses its place. + */ +export function DesignerEditor({ + editor, + harness, +}: { + editor: Editor; + harness: string; +}) { + const { state } = editor; + const nameRef = useRef(null); + const focusName = useFocusRequest(nameRef); + + const spec = useMemo( + () => ({ + name: state.saved.view.name, + intent: state.saved.view.intent, + stages: state.draft.stages, + }), + [state.saved.view, state.draft.stages], + ); + const [announcement, announce] = useAnnouncer(); + const { select } = editor; + const selection = useMemo( + () => ({ + selectedId: stageNodeId(state.selectedId), + onSelect: (nodeId) => { + const stage = state.draft.stages.find( + (candidate) => stageNodeId(candidate.id) === nodeId, + ); + if (!stage) return; + select(stage.id); + focusName(); + announce(`Stage ${stage.name || 'with no name'} selected.`); + }, + }), + [state.selectedId, state.draft.stages, select, focusName, announce], + ); + + const editable = canEdit(state); + const addable = editable && canAddStage(state.draft); + return ( +
+
+ {announcement.text} +
+ {!editable && ( +
+ + {READ_ONLY_REASON} You can look at the stages and select one, but + not change them. + +
+ )} + { + const name = nextStageName(state.draft); + editor.add(); + focusName({ select: true }); + announce(`Stage ${name} added and selected.`); + }} + > + Add stage + + } + /> + { + const stage = state.draft.stages.find((s) => s.id === id); + editor.remove(id); + focusName(); + announce(`Stage ${stage?.name || 'with no name'} deleted.`); + }} + /> +
+ ); +} diff --git a/workflow-canvas/src/designer/PropertiesPane.test.tsx b/workflow-canvas/src/designer/PropertiesPane.test.tsx new file mode 100644 index 000000000..29555292d --- /dev/null +++ b/workflow-canvas/src/designer/PropertiesPane.test.tsx @@ -0,0 +1,98 @@ +// SPDX-License-Identifier: Apache-2.0 +import { render, screen } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { HELLO_DOCUMENT } from '../test-support/fixtures'; +import { reduce, type EditEvent, type EditState } from './core'; +import { openDocument } from './core/edit-state'; +import { PropertiesPane } from './PropertiesPane'; + +function stateAfter(...events: EditEvent[]): EditState { + let state = openDocument(HELLO_DOCUMENT); + for (const event of events) state = reduce(state, event); + if (!state) throw new Error('The sample cannot be edited.'); + return state; +} + +function show(state: EditState) { + render( + {}} + onAgent={() => {}} + onRemove={() => {}} + />, + ); +} + +describe('PropertiesPane', () => { + it('marks a name that the check refused, with its sentence, and says the stage needs a fix', () => { + show( + stateAfter( + { type: 'rename', id: 's1', name: 'review' }, + { + type: 'checked', + revision: 1, + diagnostics: [ + { + message: 'Duplicate stage name: review', + path: 'stages[0].name', + line: 4, + severity: 'error', + }, + ], + }, + ), + ); + + expect(screen.getByText('Needs a fix')).toBeTruthy(); + expect(screen.getByText('Duplicate stage name: review')).toBeTruthy(); + expect( + screen + .getByRole('textbox', { name: 'name' }) + .getAttribute('aria-invalid'), + ).toBe('true'); + }); + + it('shows the name in an error as code, without backticks', () => { + show( + stateAfter( + { type: 'rename', id: 's1', name: 'review' }, + { + type: 'checked', + revision: 1, + diagnostics: [ + { + message: 'The stage name `review` is already used.', + path: 'stages[0].name', + line: 4, + severity: 'error', + }, + ], + }, + ), + ); + + const field = screen.getByText(/is already used\./).closest('div'); + expect(field?.textContent).toBe('The stage name review is already used.'); + expect(field?.querySelector('code')?.textContent).toBe('review'); + }); + + it('says a stage is complete once a check has found nothing', () => { + show( + stateAfter( + { type: 'rename', id: 's1', name: 'write' }, + { type: 'checked', revision: 1, diagnostics: [] }, + ), + ); + + expect(screen.getByText('Complete')).toBeTruthy(); + }); + + it('says nothing about a draft that no check has spoken for', () => { + show(stateAfter({ type: 'rename', id: 's1', name: 'write' })); + + expect(screen.queryByText('Complete')).toBeNull(); + expect(screen.queryByText('Needs a fix')).toBeNull(); + }); +}); diff --git a/workflow-canvas/src/designer/PropertiesPane.tsx b/workflow-canvas/src/designer/PropertiesPane.tsx new file mode 100644 index 000000000..4ac654646 --- /dev/null +++ b/workflow-canvas/src/designer/PropertiesPane.tsx @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: Apache-2.0 +import Box from '@cloudscape-design/components/box'; +import Button from '@cloudscape-design/components/button'; +import Container from '@cloudscape-design/components/container'; +import FormField from '@cloudscape-design/components/form-field'; +import Header from '@cloudscape-design/components/header'; +import Input, { type InputProps } from '@cloudscape-design/components/input'; +import Select from '@cloudscape-design/components/select'; +import SpaceBetween from '@cloudscape-design/components/space-between'; +import StatusIndicator from '@cloudscape-design/components/status-indicator'; +import type { ReactNode, Ref } from 'react'; +import { + agentChoices, + canEdit, + canRemoveStage, + errorsOf, + joinMessages, + notesOf, + problemsFor, + READ_ONLY_REASON, + problemsOfDraft, + type DraftStage, + type EditState, + type Problem, +} from './core'; +import { MessageText } from '../workflow/MessageText'; + +const NAME_HINT = + 'Lower-case letters, digits and hyphens. Each stage has its own name.'; +// The text of a field's problems, with the names in it shown as code. +function messageNode(problems: Problem[]): ReactNode { + const text = joinMessages(problems); + return text === undefined ? undefined : ; +} + +const AGENT_NOTE = 'A Konductor agent id: the name of an agent spec.'; + +function StageFields({ + stage, + locked, + extraAgents, + problems, + nameRef, + onRename, + onAgent, +}: { + stage: DraftStage; + /** True when the workflow is read-only, so nothing here changes it. */ + locked: boolean; + /** Agents the saved file names, which stay on offer once another is chosen. */ + extraAgents: readonly string[]; + problems: Problem[]; + nameRef: Ref; + onRename: (name: string) => void; + onAgent: (agent: string) => void; +}) { + const choices = agentChoices(stage.agent, extraAgents); + const chosen = choices.find((agent) => agent.id === stage.agent); + const nameProblems = problemsFor(problems, stage.id, 'name'); + const agentProblems = problemsFor(problems, stage.id, 'agent'); + const nameErrors = errorsOf(nameProblems); + const agentErrors = errorsOf(agentProblems); + return ( + + + 0} + onChange={({ detail }) => onRename(detail.value)} + /> + + + {AGENT_NOTE} + {chosen &&
{chosen.summary}
} + + } + errorText={messageNode(agentErrors)} + warningText={messageNode(notesOf(agentProblems))} + > +