Edit stages in the Designer: Properties, agent list, add, rename and delete

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
@@ -274,16 +276,81 @@ The rest wraps the core, so the rules can be tested without a browser:
   the page run only its own files and allows no `<base>` or form target; a
   server that sends the page should send the same policy as a header.
 - `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
@@ -479,7 +546,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.
 

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-<role>`, 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);
+    },
+  );
+});

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');
+});

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<void> {
+    await this.name.fill(name);
+  }
+
+  /** Opens the list of agents and picks one. */
+  async chooseAgent(id: string): Promise<void> {
+    await this.agent.click();
+    await this.page
+      .getByRole('option', { name: new RegExp(`^${id}\\b`) })
+      .click();
+  }
+}

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));
+  },
+});

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;

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();
   }
 });

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,
+    );
+  },
+);