diff --git a/.changeset/project-row-plus-is-new-agent.md b/.changeset/project-row-plus-is-new-agent.md new file mode 100644 index 00000000..970c47f1 --- /dev/null +++ b/.changeset/project-row-plus-is-new-agent.md @@ -0,0 +1,11 @@ +--- +"@sapiom/harness": minor +--- + +A project row's `+` is **New agent**, scoped to that project, and a plain session is no longer a row verb. + +Creation had been delegated to the pinned Agent Map: `mapOwnsCreation` is true for every project on a current server, and it gated both the row's create action and the empty project's create row, so neither rendered. The Agent Map has no create control of its own — its only route to generating agents was the planner session, which SAP-3143 removes. A project that already held agents was left with no scoped way to grow another; the rail's top CTA opens the composer with no project context and cannot create into an existing project. + +The `+` now opens the new-agent screen for the row's own project (`project-create-agent-{label}`), and a bare project keeps its distinct scaffold verb (`workspace-scaffold-{label}`). `project-start-session-{label}` is removed: a plain session starts from the tab strip, or from the **Start a session** on the project's own pane. The empty project still gets no create row of its own — its Agent Map row is the CTA. + +Follows design-eng `IA.md` 219 and D34(a); D34(e) and D35 item 6 for sessions belonging to the tab strip. diff --git a/.changeset/project-row-remove-action.md b/.changeset/project-row-remove-action.md new file mode 100644 index 00000000..59a7ce51 --- /dev/null +++ b/.changeset/project-row-remove-action.md @@ -0,0 +1,9 @@ +--- +"@sapiom/harness": patch +--- + +A project row's remove verb is a hover action, not an overflow menu. The `⋮` on every project row opened a 248px card to hold a single item — on plan-first projects its create item is suppressed, because the Agent Map owns creation, so the popover existed to carry one `Remove … from the rail`. That verb is now an `X` beside the session shortcut, hover-revealed like every other row action, and it opens the same confirmation as before: the project named, the count of running sessions it will end, and the statement that nothing on disk is touched. + +Row actions state their subject in the accessible name and the tooltip rather than in visible menu text. The `project-remove-{label}` testid is unchanged and now belongs to the button itself; `project-menu-{label}` and `project-menu-card-{label}` are gone, as is the `openProjectMenu` e2e helper. + +Follows design-eng D33: a project row's verbs are hover actions on the header, and a per-row menu would be a new idiom. diff --git a/packages/harness/web/e2e/accumulation-guard.spec.ts b/packages/harness/web/e2e/accumulation-guard.spec.ts index e81a1da3..06481454 100644 --- a/packages/harness/web/e2e/accumulation-guard.spec.ts +++ b/packages/harness/web/e2e/accumulation-guard.spec.ts @@ -22,7 +22,6 @@ import { expect, test } from "@playwright/test"; import type { Page } from "@playwright/test"; -import { openProjectMenu } from "./mock-navigation"; const ACME = "/Users/demo/acme-app"; @@ -98,54 +97,47 @@ test.describe("Remove project", () => { // A destructive action standing at full strength on every project row // would be the loudest thing in the rail; invisible even to the keyboard // would be worse. Both halves are CSS, so both are asserted on screen. - // Remove lives inside the row's ⋮ (SAP-2982); the session shortcut sits - // beside that menu and shares the same row-owned reveal contract. + // Remove is a row action of its own now, beside the session shortcut, and + // every action on the row shares one reveal contract — asserted across the + // whole set rather than a fixed count, so adding or removing a verb cannot + // quietly leave one of them standing. const row = page.getByTestId("workspace-group-acme-app").locator(":scope > .workspace-row"); const actions = row.locator(":scope > .workspace-row-action"); const opacities = (): Promise => actions.evaluateAll((elements) => elements.map((element) => getComputedStyle(element).opacity), ); + const count = await actions.count(); + expect(count).toBeGreaterThan(1); + const all = (value: string): string[] => Array(count).fill(value); - expect(await opacities()).toEqual(["0", "0"]); + expect(await opacities()).toEqual(all("0")); await row.hover(); - await expect.poll(opacities).toEqual(["1", "1"]); + await expect.poll(opacities).toEqual(all("1")); await page.mouse.move(0, 0); - await expect.poll(opacities).toEqual(["0", "0"]); - await actions.first().focus(); - await expect - .poll(() => - actions.first().evaluate((element) => getComputedStyle(element).opacity), - ) - .toBe("1"); - await actions.nth(1).focus(); - await expect - .poll(() => - actions.nth(1).evaluate((element) => getComputedStyle(element).opacity), - ) - .toBe("1"); - }); - - test("an OPEN menu holds its trigger on screen after the pointer leaves", async ({ page }) => { - // The popover is anchored to the ⋮. Letting the trigger fade back to - // opacity 0 when the pointer leaves the row leaves a card floating beside - // nothing — the anchor is invisible and the menu looks unmoored. - const menu = page.getByTestId("project-menu-acme-app"); - await menu.click(); - await page.mouse.move(0, 0); - await expect(page.getByTestId("project-menu-card-acme-app")).toBeVisible(); - await expect - .poll(() => menu.evaluate((element) => getComputedStyle(element).opacity)) - .toBe("1"); + await expect.poll(opacities).toEqual(all("0")); + // Keyboard reveal, one control at a time: a row action the pointer never + // touches must still show itself when it takes focus. + for (let index = 0; index < count; index += 1) { + await actions.nth(index).focus(); + await expect + .poll(() => + actions + .nth(index) + .evaluate((element) => getComputedStyle(element).opacity), + ) + .toBe("1"); + } }); - test("a COLLAPSED project row does not grow a standing ⋮", async ({ page }) => { + test("a COLLAPSED project row does not grow a standing remove", async ({ page }) => { // `.workspace-row.is-collapsed .workspace-row-action[aria-expanded]` tests - // only that the attribute is PRESENT, and a menu trigger always carries - // one — so without the exclusion in styles.css every collapsed project row - // wore a permanent ⋮, which is exactly the standing control the rail's - // hover-reveal exists to avoid. + // only that the attribute is PRESENT. The ⋮ this replaced always carried + // one, so every collapsed project row wore a permanent overflow until an + // exclusion was added for it. Plain row actions carry no `aria-expanded`, + // so the destructive one must stay hidden at rest on its own — the standing + // control the rail's hover-reveal exists to avoid. await page.getByTestId("project-disclosure-acme-app").click(); const row = page.getByTestId("workspace-group-acme-app").locator(":scope > .workspace-row"); await expect(row).toHaveClass(/is-collapsed/); @@ -153,7 +145,7 @@ test.describe("Remove project", () => { await expect .poll(() => page - .getByTestId("project-menu-acme-app") + .getByTestId("project-remove-acme-app") .evaluate((element) => getComputedStyle(element).opacity), ) .toBe("0"); @@ -162,7 +154,6 @@ test.describe("Remove project", () => { test("the confirm NAMES the number of sessions it ends, and says nothing on disk is touched", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); const confirm = page.getByTestId("remove-project-confirm"); await expect(confirm).toBeVisible(); @@ -179,7 +170,6 @@ test.describe("Remove project", () => { test("says so plainly when there is nothing to end", async ({ page }) => { // rfq-agent has one exited session and no live one. An abstract warning // here would be a lie in the only direction that matters. - await openProjectMenu(page, "rfq-agent"); await page.getByTestId("project-remove-rfq-agent").click(); await expect(page.getByTestId("remove-project-confirm-count")).toHaveText( "No running sessions to end.", @@ -187,7 +177,6 @@ test.describe("Remove project", () => { }); test("Keep project changes nothing", async ({ page }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); await page.getByRole("button", { name: "Keep project" }).click(); await expect(page.getByTestId("remove-project-confirm")).toHaveCount(0); @@ -203,7 +192,6 @@ test.describe("Remove project", () => { await expect(page.getByTestId("workflow-leasing")).toBeVisible(); expect(await recentDirPaths(page)).toContain(ACME); - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); await page.getByTestId("remove-project-confirm-btn").click(); @@ -247,7 +235,6 @@ test.describe("Remove project", () => { // into a hidden project would leave an agent that exists and nothing // shows; giving it a root of its own would mint `acme-app/leasing`, which // is the accumulation this ticket closes. - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); await page.getByTestId("remove-project-confirm-btn").click(); await expect(page.getByTestId("workspace-group-acme-app")).toHaveCount(0); @@ -271,7 +258,6 @@ test.describe("Remove project", () => { // so it cannot be confused with the `workers` subdirectory row inside it. await expect(page.getByTestId("workspace-group-polsia/services/workers")).toBeVisible(); - await openProjectMenu(page, "polsia"); await page.getByTestId("project-remove-polsia").click(); await page.getByTestId("remove-project-confirm-btn").click(); diff --git a/packages/harness/web/e2e/create-agent.spec.ts b/packages/harness/web/e2e/create-agent.spec.ts index 030421d2..2f06143b 100644 --- a/packages/harness/web/e2e/create-agent.spec.ts +++ b/packages/harness/web/e2e/create-agent.spec.ts @@ -26,7 +26,6 @@ import { expect, test } from "@playwright/test"; import type { Page } from "@playwright/test"; -import { openProjectMenu } from "./mock-navigation"; const ROOT = "/Users/demo/acme-app"; @@ -59,7 +58,6 @@ test.describe("legacy-server agent creation compatibility", () => { test("the menu opens a dialog that STATES the project, and starts nothing", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); const dialog = page.getByTestId("create-agent-dialog"); @@ -81,7 +79,6 @@ test.describe("legacy-server agent creation compatibility", () => { }); test("creation completes BEFORE the session starts", async ({ page }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); await page.getByTestId("create-agent-name").fill("billing-bot"); await page.getByTestId("create-agent-submit").click(); @@ -102,7 +99,6 @@ test.describe("legacy-server agent creation compatibility", () => { test("a first instruction reaches the session, and never asks for a scaffold", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); await page.getByTestId("create-agent-name").fill("digest-bot"); await page @@ -125,7 +121,6 @@ test.describe("legacy-server agent creation compatibility", () => { test("a duplicate name is refused by the SERVER, in the dialog, and nothing starts", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); // `leasing` is a fixture agent in this project. The field has no opinion // about it — only the endpoint knows what is already there. @@ -150,7 +145,6 @@ test.describe("legacy-server agent creation compatibility", () => { test("a name that is not one folder segment is refused before it is sent", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); const name = page.getByTestId("create-agent-name"); const submit = page.getByTestId("create-agent-submit"); @@ -181,7 +175,6 @@ test.describe("legacy-server agent creation compatibility", () => { test("Return submits from the name field — and Return on Cancel cancels", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); await page.getByTestId("create-agent-name").fill("returned"); await page.getByTestId("create-agent-name").press("Enter"); @@ -190,7 +183,6 @@ test.describe("legacy-server agent creation compatibility", () => { // The dialog took Return for the whole form, so a focused Cancel took it // too: pressing Return on "Cancel" closed the dialog AND created the // agent — the opposite of what was pressed. Measured, before the guard. - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); await page.getByTestId("create-agent-name").fill("cancelled"); await page.getByRole("button", { name: "Cancel" }).focus(); @@ -215,7 +207,6 @@ test.describe("legacy-server agent creation compatibility", () => { name: /^Create (the first |an )agent here$/, }), ).toHaveCount(0); - await openProjectMenu(page, "blank-slate"); await page.getByTestId("project-create-agent-blank-slate").click(); await expect(page.getByTestId("create-agent-dialog")).toBeVisible(); diff --git a/packages/harness/web/e2e/dialog-shell.spec.ts b/packages/harness/web/e2e/dialog-shell.spec.ts index 263de90a..8f985915 100644 --- a/packages/harness/web/e2e/dialog-shell.spec.ts +++ b/packages/harness/web/e2e/dialog-shell.spec.ts @@ -18,7 +18,6 @@ import { expect, test } from "@playwright/test"; import type { Locator, Page } from "@playwright/test"; -import { openProjectMenu } from "./mock-navigation"; interface DialogCase { name: string; @@ -67,12 +66,11 @@ const CASES: DialogCase[] = [ open: async (page) => { await page.goto("/"); await expect(page.locator(".rail-workflows")).toBeVisible(); - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); await expect(page.getByTestId("remove-project-confirm")).toBeVisible(); }, surface: (page) => page.getByTestId("remove-project-confirm"), - trigger: (page) => page.getByTestId("project-menu-acme-app"), + trigger: (page) => page.getByTestId("project-remove-acme-app"), // The SAFE action, on a destructive dialog: Enter keeps the project. opensFocusedOn: (page) => page.getByRole("button", { name: "Keep project" }), behind: (page) => page.getByTestId("rail-create-new"), @@ -82,11 +80,13 @@ const CASES: DialogCase[] = [ open: async (page) => { await page.goto("/?seed=0&mockStudioProjects=absent"); await expect(page.getByTestId("workspace-group-acme-app")).toBeVisible(); - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); await expect(page.getByTestId("create-agent-dialog")).toBeVisible(); }, surface: (page) => page.getByTestId("create-agent-dialog"), + // The row action survives the dialog now. It used to be a menu item that + // unmounted with its popover, so focus had nowhere to go but the document. + trigger: (page) => page.getByTestId("project-create-agent-acme-app"), opensFocusedOn: (page) => page.getByTestId("create-agent-name"), behind: (page) => page.getByTestId("rail-create-new"), }, diff --git a/packages/harness/web/e2e/mock-navigation.ts b/packages/harness/web/e2e/mock-navigation.ts index fe742029..5587fd20 100644 --- a/packages/harness/web/e2e/mock-navigation.ts +++ b/packages/harness/web/e2e/mock-navigation.ts @@ -37,16 +37,3 @@ export async function selectMockSessionFromPalette( await item.click(); } -/** - * Open a project row's ⋮ menu. - * - * Every action a project row offers now lives behind one control (SAP-2982). - * `+` and `×` used to sit on the row itself — adjacent, same size, same - * hover-reveal — while acting on different nouns: `+` created an AGENT in the - * project, `×` removed the PROJECT. A menu of named items has no adjacency to - * misread, and the specs open it before acting. - */ -export async function openProjectMenu(page: Page, label: string): Promise { - await page.getByTestId(`project-menu-${label}`).click(); - await expect(page.getByTestId(`project-menu-card-${label}`)).toBeVisible(); -} diff --git a/packages/harness/web/e2e/open-project.spec.ts b/packages/harness/web/e2e/open-project.spec.ts index 709da843..e8054844 100644 --- a/packages/harness/web/e2e/open-project.spec.ts +++ b/packages/harness/web/e2e/open-project.spec.ts @@ -21,7 +21,6 @@ */ import { expect, test } from "@playwright/test"; -import { openProjectMenu } from "./mock-navigation"; /* A folder that is NOTHING yet: no agent, no session, no recentDirs entry. `scratch` cannot play this part — it is the fixture's bare-session project, @@ -156,26 +155,24 @@ test.describe("the header + opens a project", () => { await expect(page.getByTestId("agent-map-frame")).toBeVisible(); await expect(page.locator(".harness-terminal .xterm")).toBeVisible(); + // D36: an empty project gets no create ROW of its own — its Agent Map row + // is the CTA. The row's `+` is a different control and is always there. await expect(group.getByTestId("project-empty-blank-slate")).toHaveCount(0); await expect( group.getByRole("button", { name: /^Create (the first |an )agent here$/ }), ).toHaveCount(0); - await expect( - group.getByTestId("project-start-session-blank-slate"), - ).toHaveAttribute("aria-label", "Start a session in blank-slate"); - // The map is a view, not an authorization gate. Both direct agent creation - // and project removal remain ordinary project-level actions. - await openProjectMenu(page, "blank-slate"); + // New agent, scoped to this project, on the row itself (IA.md 219, D34a). + // A plain session is not a row verb: the tab strip owns it, and the + // project's own pane carries the Start (D34e, D35 item 6). await expect( page.getByTestId("project-create-agent-blank-slate"), - ).toBeVisible(); + ).toHaveAttribute("aria-label", "Create an agent in blank-slate"); await expect(page.getByTestId("project-remove-blank-slate")).toBeVisible(); await page.keyboard.press("Escape"); // A bare project with an existing ordinary session retains its scaffold // action too. - await openProjectMenu(page, "scratch"); await expect(page.getByTestId("workspace-scaffold-scratch")).toBeVisible(); await expect(page.getByTestId("project-remove-scratch")).toBeVisible(); @@ -283,7 +280,6 @@ test.describe("round trip: removed, then back", () => { const before = await projectRows(page); expect(before).toContain("project-row-acme-app"); - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); await page.getByTestId("remove-project-confirm-btn").click(); await expect(page.getByTestId("project-row-acme-app")).toHaveCount(0); @@ -325,7 +321,6 @@ test.describe("round trip: removed, then back", () => { test("opening a folder ABOVE a removed project un-hides what is inside it", async ({ page, }) => { - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-remove-acme-app").click(); await page.getByTestId("remove-project-confirm-btn").click(); await expect(page.getByTestId("workflow-leasing")).toHaveCount(0); diff --git a/packages/harness/web/e2e/project-axis.spec.ts b/packages/harness/web/e2e/project-axis.spec.ts index 28f8d296..101fff41 100644 --- a/packages/harness/web/e2e/project-axis.spec.ts +++ b/packages/harness/web/e2e/project-axis.spec.ts @@ -165,7 +165,7 @@ test.describe("ordering", () => { }); test.describe("durable Studio project navigation", () => { - test("the project plus starts a coding session at its root without creating an agent", async ({ + test("the row's plus is New agent; a coding session starts from the project's own pane", async ({ page, }) => { await page.evaluate(() => { @@ -181,13 +181,14 @@ test.describe("durable Studio project navigation", () => { }); const group = page.getByTestId("workspace-group-dashboard-keeper"); const row = group.getByTestId("project-row-dashboard-keeper"); - const start = group.getByTestId("project-start-session-dashboard-keeper"); + const create = group.getByTestId("project-create-agent-dashboard-keeper"); - await expect(start).toHaveAttribute( + await expect(create).toHaveAttribute( "aria-label", - "Start a session in dashboard-keeper", + "Create an agent in dashboard-keeper", ); - await expect(start).toHaveAttribute("data-tooltip", "Start a session here"); + // A plain session is NOT a row verb: it starts from the tab strip or from + // the Start on the project's own pane (D34e, D35 item 6). expect( await row .locator(":scope > .workspace-row-action") @@ -195,17 +196,17 @@ test.describe("durable Studio project navigation", () => { actions.map((action) => action.getAttribute("data-testid")), ), ).toEqual([ - "project-start-session-dashboard-keeper", - "project-menu-dashboard-keeper", + "project-create-agent-dashboard-keeper", + "project-remove-dashboard-keeper", ]); - // The ordinary project action also works while its read-only map is open. - // A successful create selects the exact new conversation and no scaffold - // operation is smuggled into that session action. + // At map altitude the project's own pane carries the Start. The new + // generic session becomes the visible workbench, rooted at the project and + // on the preferred harness, and no scaffold request is smuggled into it. const map = group.getByTestId("project-select-dashboard-keeper"); await map.click(); await expect(map).toHaveAttribute("aria-pressed", "true"); - await start.click(); + await page.getByTestId("project-start-session").click(); await expect .poll(() => page.evaluate( @@ -252,7 +253,12 @@ test.describe("durable Studio project navigation", () => { }) => { const group = page.getByTestId("workspace-group-dashboard-keeper"); const map = group.getByTestId("project-select-dashboard-keeper"); - const start = group.getByTestId("project-start-session-dashboard-keeper"); + // The row's `+` is New agent (D34a). The project pane's Start only exists + // while the project has NO session — it is the empty state's CTA. A SECOND + // session therefore comes from the tab strip, which is where D34(e) and D35 + // item 6 put a plain session in the first place. + const start = page.getByTestId("project-start-session"); + const anotherSession = page.getByTestId("session-tab-new"); // Establish a real conversation in this project first. Cross-project map // navigation deliberately clears an unrelated active session, so it cannot @@ -279,9 +285,14 @@ test.describe("durable Studio project navigation", () => { ).__MOCK_CREATE_SESSION_FAIL_ONCE__ = true; }); - await start.click(); + await anotherSession.click(); + // The tab strip reports its own failure copy rather than passing the + // server's message through, which the deleted row `+` did. Less specific, + // and worth fixing — but the guarantee this test exists for is unchanged: + // a failed start leaves the map and the active conversation exactly as they + // were. await expect(page.getByTestId("toast")).toContainText( - "mock: couldn't create session", + "Couldn't start the session.", ); await expect(map).toHaveAttribute("aria-pressed", "true"); await expect(page.getByTestId("agent-map-frame")).toBeVisible(); @@ -344,10 +355,14 @@ test.describe("durable Studio project navigation", () => { .getByTestId("workflow-dashboard-keeper") .locator(".workflow-status"), ).toHaveCount(1); + // The row's one `+` is New agent (D34a). A plain session is not a row verb: + // the tab strip owns it, and the project's pane carries the Start. await expect( - group.getByTestId("project-start-session-dashboard-keeper"), + group.getByTestId("project-create-agent-dashboard-keeper"), ).toBeVisible(); - // The removed legacy shortcut is not a second project-level `+`. + await expect( + group.getByTestId("project-start-session-dashboard-keeper"), + ).toHaveCount(0); await expect( page.locator('.rail-list [data-testid^="workspace-new-session-"]'), ).toHaveCount(0); diff --git a/packages/harness/web/e2e/project-map-navigation.spec.ts b/packages/harness/web/e2e/project-map-navigation.spec.ts index efd75160..2af06bf8 100644 --- a/packages/harness/web/e2e/project-map-navigation.spec.ts +++ b/packages/harness/web/e2e/project-map-navigation.spec.ts @@ -195,7 +195,6 @@ test.describe("SAP-3148 project Agent Map navigation", () => { }) => { const project = page.getByTestId("workspace-group-acme-app"); await expect(project.getByTestId("workflow-report-reviewer")).toBeVisible(); - await project.getByTestId("project-menu-acme-app").click(); await page.getByTestId("project-remove-acme-app").click(); await page.getByTestId("remove-project-confirm-btn").click(); await expect(project).toHaveCount(0); diff --git a/packages/harness/web/e2e/rail-grammar.spec.ts b/packages/harness/web/e2e/rail-grammar.spec.ts index 94a8c2f8..d0911f6f 100644 --- a/packages/harness/web/e2e/rail-grammar.spec.ts +++ b/packages/harness/web/e2e/rail-grammar.spec.ts @@ -19,7 +19,6 @@ import { expect, test } from "@playwright/test"; import type { Page } from "@playwright/test"; -import { openProjectMenu } from "./mock-navigation"; const ROW = (page: Page, label: string) => page @@ -34,57 +33,52 @@ test.describe("legacy-server project row grammar", () => { await expect(page.getByTestId("workspace-group-acme-app")).toBeVisible(); }); - test("a session shortcut sits immediately before the named project menu", async ({ + test("the row's verbs are hover actions, each naming its own subject", async ({ page, }) => { const row = ROW(page, "acme-app"); - // The frequent session action is one click away. Destructive project - // management remains behind the named overflow menu instead of returning - // as an adjacent `×`. + // Hover actions, not an overflow menu (design-eng D33). The `+` is New + // agent (IA.md 219, D34a); the destructive one is last. The adjacency + // SAP-2982 worried about is answered by the accessible name and the + // confirmation, not by hiding one verb behind a popover. A plain session is + // not a row verb at all — the tab strip owns it (D34e). const actions = row.locator(".workspace-row-action"); await expect(actions).toHaveCount(2); await expect(actions.nth(0)).toHaveAttribute( "data-testid", - "project-start-session-acme-app", - ); - await expect(actions.nth(0)).toHaveAttribute( - "aria-label", - "Start a session in acme-app", + "project-create-agent-acme-app", ); await expect(actions.nth(1)).toHaveAttribute( "data-testid", - "project-menu-acme-app", + "project-remove-acme-app", ); - // And the actions themselves state their subject in words. - await openProjectMenu(page, "acme-app"); - await expect(page.getByTestId("project-create-agent-acme-app")).toHaveText( - "Create an agent in acme-app", - ); - await expect(page.getByTestId("project-remove-acme-app")).toHaveText( + // Each states its subject where a glyph cannot: the accessible name. + await expect( + page.getByTestId("project-create-agent-acme-app"), + ).toHaveAttribute("aria-label", "Create an agent in acme-app"); + await expect(page.getByTestId("project-remove-acme-app")).toHaveAttribute( + "aria-label", "Remove acme-app from the rail", ); }); - test("a bare project's session shortcut stays distinct from scaffolding", async ({ + test("a bare project's create verb is scaffold, and says so", async ({ page, }) => { - // `scratch` has live sessions and no Sapiom agent. The row `+` can start - // another coding session; the legacy scaffold operation remains named in - // the menu so the two operations do not masquerade as one another. + // `scratch` has live sessions and no Sapiom agent, so its create verb is + // SCAFFOLD — it grows an agent inside the session already running there + // rather than starting a new one. The distinct glyph and the distinct name + // are what keep it from reading as the ordinary create. const row = ROW(page, "scratch"); await expect(row.locator(".workspace-row-action")).toHaveCount(2); await expect( - page.getByTestId("project-start-session-scratch"), - ).toBeVisible(); - await openProjectMenu(page, "scratch"); - await expect(page.getByTestId("workspace-scaffold-scratch")).toHaveText( - "Scaffold an agent in scratch", - ); + page.getByTestId("workspace-scaffold-scratch"), + ).toHaveAttribute("aria-label", "Scaffold an agent in scratch"); await expect(page.getByTestId("project-remove-scratch")).toBeVisible(); }); - test("creating from the menu creates IN that project, and only then talks", async ({ + test("creating from the row creates IN that project, and only then talks", async ({ page, }) => { // The menu changed what the control SAYS; SAP-2981 changed what it does — @@ -108,9 +102,7 @@ test.describe("legacy-server project row grammar", () => { ).__HARNESS_TEST__?.createOrder ?? []) as string[], ); - await openProjectMenu(page, "acme-app"); await page.getByTestId("project-create-agent-acme-app").click(); - await expect(page.getByTestId("project-menu-card-acme-app")).toHaveCount(0); await expect(page.getByTestId("create-agent-project")).toHaveText( "acme-app", ); diff --git a/packages/harness/web/e2e/smoke.spec.ts b/packages/harness/web/e2e/smoke.spec.ts index 5643ebdf..727064f1 100644 --- a/packages/harness/web/e2e/smoke.spec.ts +++ b/packages/harness/web/e2e/smoke.spec.ts @@ -13,7 +13,6 @@ import type { Page } from "@playwright/test"; import { focusRfqAgent, - openProjectMenu, selectMockSessionFromPalette, } from "./mock-navigation"; @@ -404,7 +403,6 @@ test.describe("three-zone IA (rail explorer, tab strip, right pane)", () => { // The scaffold action moved into the row's ⋮ with every other project // action (SAP-2982): a Sparkles glyph acting on an AGENT sat adjacent to // an `×` acting on the PROJECT, same size, same reveal. - await openProjectMenu(page, "scratch"); await expect(page.getByTestId("workspace-scaffold-scratch")).toBeVisible(); await page.keyboard.press("Escape"); await expect(page.getByTestId("workspace-focus-scratch")).toHaveCount(0); diff --git a/packages/harness/web/e2e/unrooted-agents.spec.ts b/packages/harness/web/e2e/unrooted-agents.spec.ts index 65f651a6..ec5428e1 100644 --- a/packages/harness/web/e2e/unrooted-agents.spec.ts +++ b/packages/harness/web/e2e/unrooted-agents.spec.ts @@ -21,7 +21,6 @@ import { expect, test } from "@playwright/test"; import type { Page } from "@playwright/test"; -import { openProjectMenu } from "./mock-navigation"; const ORCHESTRATION = "/Users/demo/design-eng/ari/orchestration"; const FIX_ORCHESTRATION = "/Users/demo/design-eng-fix/ari/orchestration"; @@ -426,7 +425,6 @@ test.describe("(c) there is a way OUT", () => { // removal whose rows merely move somewhere else has renamed the project, // not removed it — `accumulation-guard.spec.ts` pins that), so the count // does NOT climb here. - await openProjectMenu(page, "design-eng"); await page.getByTestId("project-remove-design-eng").click(); await page.getByTestId("remove-project-confirm-btn").click(); await expect(page.getByTestId("project-row-design-eng")).toHaveCount(0); diff --git a/packages/harness/web/src/App.tsx b/packages/harness/web/src/App.tsx index 3720207e..27461cdd 100644 --- a/packages/harness/web/src/App.tsx +++ b/packages/harness/web/src/App.tsx @@ -2922,7 +2922,6 @@ export const App = (): JSX.Element => { }} launchDir={state.launchDir ?? null} listDir={harness.listDir} - onStartProjectSession={handleStartProjectSession} listHarnesses={harness.listHarnesses} onCreateAgent={handleCreateAgentInProject} onScaffoldInSession={handleScaffoldInSession} diff --git a/packages/harness/web/src/components/WorkflowsRail.tsx b/packages/harness/web/src/components/WorkflowsRail.tsx index fcce99b6..3e52d7fc 100644 --- a/packages/harness/web/src/components/WorkflowsRail.tsx +++ b/packages/harness/web/src/components/WorkflowsRail.tsx @@ -177,8 +177,6 @@ interface WorkflowsRailProps { onOpenProject: (root: string) => Promise; launchDir: string | null; listDir: (path?: string) => Promise; - /** Starts a coding-agent session and owns its failure feedback. */ - onStartProjectSession: (root: string, label: string) => Promise; /** Adapter registry fetch — the add dialog's picker and MCP setup block. */ listHarnesses: () => Promise; /** @@ -263,22 +261,20 @@ const SORT_LABELS: Record = { }; /** - * The project row's overflow menu. + * The project row's trailing actions. * - * The adjacent `+` has one stable meaning: start a coding-agent session at this - * project's root. This menu keeps the lower-frequency, explicitly named - * project actions, including creating or scaffolding a Sapiom agent. Opening - * the Agent Map never takes ownership of those ordinary build controls. + * HOVER ACTIONS, NOT A MENU (design-eng D33: "a project row's verbs are hover + * actions on the header ... a per-row menu would be a new idiom"). The overflow + * this replaces held a create item and a destructive one behind a popover and a + * 248px card whose stated purpose was fitting "Remove from the rail" + * on one line. Both are hover actions now, each naming its own subject. * - * Named items say it instead. Each carries the project's own label, so the - * subject is read rather than inferred, and the destructive one is last and - * marked. - * - * The trigger keeps its own open state and its own ref: `triggerRef` is what - * the remove confirmation returns focus to, and the menu item that opened it - * has unmounted by then. + * The X removes the project from the rail; it never touches a file. `onRemove` + * is handed the button so the confirmation returns focus to the control that + * opened it — the reason the menu needed a ref of its own, and the reason this + * still does. */ -function ProjectRowMenu({ +function ProjectRowActions({ label, create, onRemove, @@ -295,80 +291,41 @@ function ProjectRowMenu({ } | null; onRemove: (trigger: HTMLButtonElement | null) => void; }): JSX.Element { - const [open, setOpen] = useState(false); - const triggerRef = useRef(null); + const removeRef = useRef(null); return ( <> + {create && ( + + )} + {/* REMOVE. An `X`, not a trash can: this closes a project and ends its + sessions, and never touches a file — a bin glyph would say the + opposite of the copy in the confirm. The subject the menu item spelled + out ("Remove acme-app from the rail") now rides the accessible name and + the tooltip, and the confirmation restates it, with the count of + sessions it will end, before anything happens. */} - setOpen(false)} - placement="down-end" - className="menu-flyer" - testid={`project-menu-card-${label}`} - > -
-
- {create && ( - - )} - {/* REMOVE. An `X`, not a trash can: this closes a project and ends - its sessions, and never touches a file — a bin glyph would say - the opposite of the copy in the confirm. */} - -
-
- ); } @@ -486,7 +443,6 @@ export function WorkflowsRail({ onOpenProject, launchDir, listDir, - onStartProjectSession, listHarnesses, onCreateAgent, onScaffoldInSession, @@ -1419,34 +1375,13 @@ export function WorkflowsRail({ )} - {/* START A SESSION HERE. This is the frequent project-row - action and therefore stays one click away, immediately - before the overflow menu. Its accessible name supplies - the noun the glyph cannot: this starts a coding-agent - SESSION at the project root. It does not scaffold a - Sapiom agent. */} - {!pending && ( - - )} - {/* NAMED PROJECT ACTIONS. The destructive action stays in - this menu instead of masquerading as a peer of the - session shortcut. Legacy-only agent creation also - remains spelled out here rather than sharing the `+`. */} - { - // Focus returns to the ⋮, not to the menu item that - // opened the dialog: that item unmounts with the - // popover, and a `triggerRef` pointing at a detached - // node restores focus to . + // Focus returns to the X itself. The menu this + // replaced had to hand back its trigger instead: the + // item that opened the dialog unmounted with the + // popover, and a ref on a detached node restores + // focus to . removeTriggerRef.current = trigger; setRemoving({ root: project.root, diff --git a/packages/harness/web/src/styles.css b/packages/harness/web/src/styles.css index 0657aa7f..3359b5b5 100644 --- a/packages/harness/web/src/styles.css +++ b/packages/harness/web/src/styles.css @@ -2048,13 +2048,13 @@ button.rail-footer-card:hover { /* A collapsed merged group keeps its caret button visible, matching the inline caret's "there's more here" cue on plain headers. - NOT the project row's ⋮. This selector tests only that `aria-expanded` is - PRESENT, and the row menu's trigger always carries it — so without the - exclusion every collapsed project row grew a standing ⋮, which is the - "loudest thing in the rail" the trailing actions are hover-revealed to - avoid. An open menu keeps its own trigger visible through the rule below. */ -.workspace-row.is-collapsed - .workspace-row-action[aria-expanded]:not(.project-row-menu-trigger) { + The selector tests only that `aria-expanded` is PRESENT, so it catches any + row action that can open something. It used to need an exclusion for the + project row's ⋮, whose trigger always carried the attribute and therefore + stood permanently visible on every collapsed row — "the loudest thing in the + rail", which hover-revealing the trailing actions exists to avoid. That menu + is gone; the exclusion went with it. */ +.workspace-row.is-collapsed .workspace-row-action[aria-expanded] { opacity: 1; } @@ -2136,24 +2136,16 @@ button.rail-footer-card:hover { color: var(--text); } -/* ---- Project row ⋮ menu ---------------------------------------------- */ -/* Wide enough for the longest item to state its subject on one line — the - whole point of the menu is that "Remove acme-app from the rail" is read, - not inferred from a glyph, and a wrapped or ellipsed label gives that back. */ -.project-row-menu { - min-width: 248px; -} - -/* The destructive item, marked on hover the way the session menu's is. A wash - (`color-mix`), never a full-strength surface. +/* ---- Project row remove ------------------------------------------------ */ +/* The destructive row action, marked on hover with a wash (`color-mix`), never + a full-strength surface — the same weight the menu item it replaced carried. - The `.session-dropdown-item` qualifier is load-bearing, not decoration: the - menu items wear that class for their anatomy, its own `:hover` rule ties - this one on specificity, and it is declared LATER in this file — so the - neutral hover won and the destructive item marked itself in text colour - alone. Measured, not guessed: the computed background came back - `rgba(17, 17, 20, 0.04)`. */ -.session-dropdown-item.project-row-menu-danger:hover { + The `.workspace-row-action` qualifier is load-bearing, not decoration: row + actions wear that class for their anatomy, its own `:hover` rule ties this + one on specificity, and it is declared EARLIER in this file, so the neutral + hover would otherwise win on source order and the destructive action would + mark itself in text colour alone. */ +.workspace-row-action.project-row-remove:hover { color: var(--red-text); background: color-mix(in srgb, var(--red) 12%, transparent); }