Skip to content

fix(harness): a project row's two verbs are New agent and Remove, as hover actions [SAP-2981] - #842

Merged
gwitwer merged 3 commits into
fix/studio-onboarding-followupsfrom
dave/rail-row-grammar-on-onboarding-followups
Sep 7, 2026
Merged

fix(harness): a project row's two verbs are New agent and Remove, as hover actions [SAP-2981]#842
gwitwer merged 3 commits into
fix/studio-onboarding-followupsfrom
dave/rail-row-grammar-on-onboarding-followups

Conversation

@gwitwer

@gwitwer gwitwer commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Primary change type

  • Bug fix
  • Documentation
  • Feature
  • Tests
  • Dependency update
  • Maintenance or refactor

Problem and motivation

Based on fix/studio-onboarding-followups, not main — this is the rail-grammar slice of the now-closed #837, rebased onto your branch. Everything else in #837 you had already done independently: the planner deletion, and un-gating creation. This is the part that existed nowhere else.

A project row carries three controls acting on two different nouns. The + starts a session; the holds Create an agent and Remove the project. So the frequent build verb is buried in a popover while a session shortcut owns the +, and the overflow's own accessible name — "Actions for acme-app" — names no action at all.

design-eng ruled on this on 2026-09-04, and the code has not followed:

Source Ruling
DECISIONS.md D33 (rev low) "a project row's verbs are hover actions on the header … a per-row menu would be a new idiom". Rejected: "A ⋯ menu on every project row"
DECISIONS.md D34(a) "the row's + keeps RAIL-REBUILD's placement and testid (project-create-agent-{label}) and now opens the new-agent screen"
IA.md 219 "New agent, on every project row: the row's + … hover-revealed like every row action"
IA.md 224, D34(e), D35 item 6 "a plain session starts from the tab strip"

D33's rationale names this branch's version explicitly: "The harness's row menu carries the verb because the harness rows have menus; v2's rows have actions."

Summary and scope

The row has two verbs, both hover actions, each naming its own subject.

  • + is New agent, scoped to the row's project (project-create-agent-{label}). A bare project keeps its distinct scaffold verb.
  • X is Remove, opening the same RemoveProjectConfirm as before — project named, running sessions counted, "nothing on disk is touched".
  • project-start-session-{label} is removed from the row. A plain session comes from the tab strip, or the project pane's project-start-session Start.
  • ProjectRowMenu and its .project-row-menu 248px card are deleted, along with the collapsed-row CSS exclusion that existed only because a menu trigger always carries aria-expanded.

Row actions before and after, read off the DOM:

before  project-start-session-acme-app :: Start a session in acme-app
        project-menu-acme-app          :: Actions for acme-app        <- names no action

after   project-create-agent-acme-app  :: Create an agent in acme-app
        project-remove-acme-app        :: Remove acme-app from the rail

The menu being removed, and the row after

Before — open After — + and ×

A 248px card for two items, whose CSS comment stated its own purpose: "Wide enough for the longest item to state its subject on one line."

Before — row After — row

The confirmation is unchanged and still reachable, now in one click:

How to review

Start at WorkflowsRail.tsxProjectRowMenu becomes ProjectRowActions, and the row's trailing block loses the session button. Focus handling improves: the menu item that opened the confirm unmounted with its popover, so focus was released to <body>; the X is still mounted, so focus returns to it. dialog-shell's CreateAgentDialog case gains the trigger it could not previously have.

The third commit re-points three of your specs that reached through the deleted controls to get somewhere else. None was about those controls.

One behaviour difference, recorded rather than hidden. In "a failed project session keeps the map and active conversation intact", the project pane's Start is not a substitute for the row +: it is the no-session empty state's CTA, and that scenario has a live session by design. A second session now comes from the tab strip — which reports "Couldn't start the session." where the row + passed the server's message through ("mock: couldn't create session"). Less specific, worth fixing separately. The guarantee the test exists for is unchanged.

Verification

playwright mock suite   550 passed
web typecheck           clean

Three specs failed only under full-suite load and pass in isolation — canvas-inspector, popover-crop, project-map-navigation — the known flake band, all canvas-timing and none touching the rail.

Local runs are the only evidence here. test.yml and harness.yml are pull_request: branches: [main], so a PR based on a feature branch gets neither. claude-review has no branch filter and will run, but its pass is a code opinion, not a test result.

Not included, deliberately

The header's Add a project control (rail-add-project) is an unlabeled folder-plus sitting beside the settings . Reported as hard to find; it is a separate change and no decision covers it yet.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KQCZymBi59bqzQ1dEKQYbw

gwitwer and others added 3 commits September 5, 2026 18:25
…menu

The ⋮ on every project row opened a 248px card to hold one item. Its create
item is suppressed wherever the Agent Map owns creation, which is every
plan-first project, so on the product path the popover existed to carry a
single "Remove <project> from the rail" — a card, an anchor and a min-width
whose stated purpose in the CSS was fitting that one sentence on one line.

Remove is now an X beside the session shortcut, hover-revealed like every other
row action, opening the same RemoveProjectConfirm as before: the project named,
the count of running sessions it ends, and the statement that nothing on disk is
touched. The legacy create action, where a server still offers one, keeps its
own glyph rather than a menu row.

design-eng D33 (2026-09-04) already ruled this way and was not followed: "a
project row's verbs are hover actions on the header ... a per-row menu would be
a new idiom", rejecting "a ⋯ menu on every project row". Reversibility: low.

Focus survives the change rather than degrading. The menu item that opened the
confirm unmounted with its popover, so focus was released to <body>; the X is
still mounted when the dialog closes, so focus returns to the control that
opened it. dialog-shell's CreateAgentDialog case gains the trigger it could not
have before.

The collapsed-row exclusion in styles.css goes with the menu: it existed only
because a menu trigger always carries aria-expanded, which made every collapsed
project row wear a standing ⋮. Plain row actions carry no such attribute.

The hover-reveal spec now asserts the contract across every action on the row
instead of a hard-coded pair, so adding or removing a verb cannot leave one
standing at full strength unnoticed.

Verified: 544 playwright mock specs pass; web typecheck clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQCZymBi59bqzQ1dEKQYbw
Creating an agent inside an existing project had no door left. `mapOwnsCreation`
is `studioProject != null`, and the comment above it says current servers issue
a durable Studio project for every scope — so it is true everywhere. It gated
both the row's create action and the empty project's create row, on the stated
grounds that "its pinned Agent Map is the only route to generating agents".

AgentMapPane has no create control. The route was the planner session driving
the map's MCP tools, which SAP-3143 deletes in this same branch. A project that
already holds agents was therefore left with nothing: the row offered a session
and a remove, the empty-project row never rendered, and the rail's top CTA opens
the composer with no project context, so it cannot create INTO a project.

The `+` is New agent again, which is also what the design has said since
2026-09-04 — design-eng IA.md 219 and D34(a) put `project-create-agent-{label}`
on the row itself, hover-revealed. A bare project keeps its distinct scaffold
verb, which grows an agent inside the session already running there.

`project-start-session-{label}` comes off the row. D34(e) and D35 item 6 give a
plain session to the tab strip, and SAP-3143 added the project pane's own Start
a session, which is the map-altitude path. `startProjectSession` in App.tsx is
untouched and still serves both.

The empty project still gets no create row of its own: D36 makes its Agent Map
row the CTA, and that gate is the one thing `mapOwnsCreation` still owns.

Verified: 544 playwright mock specs pass; web typecheck clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQCZymBi59bqzQ1dEKQYbw
Three specs on this branch drive controls the two commits before this one
remove. None of them is about those controls; each reaches through one to get
somewhere else.

- `project-map-navigation` opened the ⋮ only to click Remove inside it. Remove
  is a row action now, so the menu click goes and the Remove click stays.
- `project-axis` "a failed project session keeps the map and active
  conversation intact" used the row's session `+`. That `+` is New agent. The
  project pane's Start is not a substitute HERE: it is the no-session empty
  state's CTA, and this scenario has a live session by design. A SECOND session
  comes from the tab strip, which is where D34(e) and D35 item 6 put a plain
  session to begin with.
- The same spec's row-action inventory now expects New agent plus Remove, and
  asserts the session shortcut is absent rather than visible.

One behaviour difference is recorded rather than hidden: the tab strip reports
"Couldn't start the session." where the row `+` passed the server's own message
through ("mock: couldn't create session"). Less specific, and worth fixing
separately. The guarantee the test exists for is unchanged — a failed start
leaves the map and the active conversation exactly as they were.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQCZymBi59bqzQ1dEKQYbw
gwitwer added a commit that referenced this pull request Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review — PR #842

🔒 CONFIDENTIALITY — internal design-doc and roadmap references in both changesets

.changeset/*.md compile into packages/harness/CHANGELOG.md and ship in the npm tarball, permanently.

  • project-row-plus-is-new-agent.md:17 — "Follows design-eng IA.md 219 and D34(a); D34(e) and D35 item 6…"
  • project-row-remove-action.md:9 — "Follows design-eng D33: …"

These are pointers into a private design doc with internal decision numbers. An npm consumer cannot resolve them and does not need to; they exist to satisfy an internal reviewer. The repo's precedent is a bare ticket id (SAP-2060 in CHANGELOG.md:1063), not a citation of internal doc sections.

Same file, line 13: "its only route to generating agents was the planner session, which SAP-3143 removes" — publishing unshipped internal roadmap. State the behaviour that changed, not the ticket that will change it next.

Also fold out the internal-implementation asides that only mean something to this repo: "a 248px card", "the openProjectMenu e2e helper", the mapOwnsCreation variable name. Rewrite both bodies to what the user sees and what they should do instead.

Regression — legacy servers lose the only way to start a plain session in a project

project-start-session-{label} is deleted from the row. The changeset names two replacements; neither exists when the server has no Studio projects:

  • The pane CTA project-start-session (App.tsx:3253) is gated on selectedStudioScope, which is null unless state.studioProjects has a match (App.tsx:1372-1389).
  • session-tab-new renders only when the subject already has a session — SessionTabs.tsx:199 is inside the strip, and the SessionBar.tsx:387-390 variant requires activeSession?.status === "exited".

So on a legacy server, a project with no session (e.g. the fixture's blank-slate) has no session starter at all: the row + now creates an agent, and the row is not focusable (focusable={!planFirst && (creating || bare != null)}, WorkflowsRail.tsx:1308). The remaining path is a folder-browse round trip through composer-open-folder. The PR's own legacy suites (rail-grammar.spec.ts:32, create-agent.spec.ts:54, both mockStudioProjects=absent) never exercise starting a session, so nothing caught this.

Either keep the row verb when !planFirst, or correct the changeset — as written it tells legacy-server users to use controls they don't have.

Regression shipped knowingly — failed session start lost its server message

project-axis.spec.ts:288-292 documents it: the assertion went from "mock: couldn't create session" to "Couldn't start the session.", with a comment calling it "Less specific, and worth fixing". A user whose session fails for a nameable reason now gets a sentence that names nothing. Fix it, or at minimum say so in the changeset instead of only in a test comment.

Semver — project-row-remove-action.md is a patch for a user-facing interaction change

Replacing an overflow menu with a hover-revealed X is not a bug fix; it changes how a destructive action is reached and removes a named-in-text label in favour of an unlabelled glyph. It should be minor, matching its sibling changeset for the same rework.

Separately, the minor changeset documents a removed control (project-start-session-{label}) in body prose with no Breaking marker or migration line. Given the finding above, that removal deserves one.

Test coverage — the headline fix is never exercised on the server it targets

The changeset's premise is that mapOwnsCreation was true "for every project on a current server", so the + was invisible there. Every test that clicks project-create-agent-* runs under mockStudioProjects=absent (create-agent.spec.ts, rail-grammar.spec.ts:105, dialog-shell.spec.ts:83). project-axis.spec.ts runs with Studio projects present but only asserts the aria-label (line 184-190) and visibility (line 361) — it never clicks.

That gap is load-bearing: createAgentInProject (App.tsx:1921+) does Studio-only work after creation — resolving the server-issued project-scoped binding from getState() and persisting the workspace preference. None of it is covered.

Nits

  • accumulation-guard.spec.ts:61-63 — the fixed ["0","0"] became all("0") derived from actions.count(), guarded only by toBeGreaterThan(1). Silently dropping a row verb still passes.
  • smoke.spec.ts:403 and open-project.spec.ts keep await page.keyboard.press("Escape") and comments about "the row's ⋮ (SAP-2982)" after the menu is gone.
  • create-agent.spec.ts:58 still titled "the menu opens a dialog…".

Verdict

Request changes: strip the internal design-doc and roadmap references from both changesets before this can publish, and resolve the legacy-server session-start gap (fix or accurate changeset).

@gwitwer
gwitwer merged commit 219e036 into fix/studio-onboarding-followups Sep 7, 2026
1 check passed
@gwitwer
gwitwer deleted the dave/rail-row-grammar-on-onboarding-followups branch September 7, 2026 02:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants