nntrivi2001
57dc91585d
Internal SmartGift build of a Claude Code monitoring dashboard. Lanes: a durable unit of parallel agent work, one per working directory, tracked across session restarts. Managed lanes are git worktrees the dashboard provisions and can reset or remove behind a three-check destroy guard and a counted preflight; adopted lanes are directories you already own and are never destroyable. Pipelines: a lane moves through pipeline stages. A stage the agent declares with evidence renders green; a stage inferred from the tool-event stream renders dashed amber and never counts as done. Detection is forward-only within a 30-minute window, and never writes the declared stage. Workspace: one page at /run with a lane grid, the selected lane's pipeline, and a full Claude console behind a disclosure.
274 lines
18 KiB
Markdown
274 lines
18 KiB
Markdown
# Workspace UI Rebuild Implementation Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Rebuild the Workspace page to the reference screen's legibility — card grid, large pipeline, collapsible console — and fill the two data gaps that make lanes look emptier than they are (git facts, expiring detection).
|
|
|
|
**Architecture:** Design doc: `docs/superpowers/specs/2026-07-29-workspace-ui-design.md` — read it once before Task 1. Two server tasks land first because the client renders what they produce: detection expiry in `recordDetection`, and a read-only `GET /api/lanes/:id/git` reusing `worktree.js`'s existing `git()` and `statusCounts()`. Then the card is rebuilt, then the page shell around it.
|
|
|
|
**Tech Stack:** Node 18+, Express, better-sqlite3, `node:test` (server); React 18 + TypeScript + Vite + Tailwind, Vitest + Testing Library (client).
|
|
|
|
## Global Constraints
|
|
|
|
- Branch: `feat/workspace-ui`, cut from the head of `feat/workspace-page`. Never work on `master`.
|
|
- Every `.js/.ts/.tsx` created or modified MUST start with a file overview comment plus the exact line `@author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>`. Verify with `bash .claude/skills/file-headers/scripts/check-headers.sh` (must exit 0).
|
|
- **Detection never writes `lanes.stage`**, and **an inferred node never renders `done`.** Both are load-bearing invariants from sub-project B; a change that lets either slip is a failed task regardless of what else it achieves.
|
|
- **No git command may be built as a shell string.** `execFile` with an argv array only, through the existing `git()` wrapper in `server/lib/worktree.js` — it scrubs the inherited `GIT_*` environment, and that scrub exists because a real bug was traced to it.
|
|
- The destroy guard (`assertDestroyable`) and the preflight/`expect` echo are not touched by this plan.
|
|
- `GET /api/lanes` stays free of git subprocesses. Git facts are their own endpoint.
|
|
- Schema changes are additive with a per-column probe (`try { SELECT col } catch { ALTER }`).
|
|
- Server CommonJS. No new npm dependencies. Server tests `node:test` + `node:assert/strict`; client tests Vitest + Testing Library. Exact-value assertions, no bare sleeps.
|
|
- i18n strings in all four locales (`en`, `zh`, `vi`, `ko`), genuinely translated — no English copied into the other three.
|
|
- **Node 24 is required to run the suites.** Node 25 breaks 20 client tests (global `localStorage`) and 6 server tests (better-sqlite3 ABI). Run with `PATH="$HOME/.nvm/versions/node/v24.14.1/bin:$PATH"`.
|
|
- The pre-commit hook runs Prettier plus both suites and takes minutes. Let it finish. NEVER `--no-verify`.
|
|
- Baseline at branch point: 906 server tests, 347 client tests, all passing. Each task leaves `git status --short` empty.
|
|
|
|
---
|
|
|
|
## Task 1 (D1): detection expires
|
|
|
|
**Files:** Modify `server/lib/lanes.js`, `server/__tests__/lanes-lib.test.js`.
|
|
|
|
**Produces:** `recordDetection` gains a staleness window. When the lane's
|
|
`detected_at` is older than `DETECTION_TTL_MS` (read from `process.env`, default
|
|
`1_800_000`), the forward-only comparison against `detected_stage` is skipped
|
|
entirely and a fresh detection is accepted even if it sits behind. Inside the
|
|
window, behaviour is byte-for-byte what it is today.
|
|
|
|
The declared-wins rule is NOT affected by the window: a lane whose declared
|
|
stage leads still refuses the detection, stale or not. Only the
|
|
detected-vs-detected comparison expires.
|
|
|
|
A lane with `detected_stage` set but `detected_at` NULL (rows written before
|
|
this column was populated) is treated as stale — an unknown age cannot be
|
|
proven fresh.
|
|
|
|
- [ ] **Step 1: write the failing tests** in `lanes-lib.test.js`: a backward detection inside the window still returns `behind-detected` and writes nothing; the same backward detection with `detected_at` set beyond the TTL is written and returns `{written: true}`; a stale detection that is behind the DECLARED stage still returns `behind-declared`; a lane with `detected_stage` set and `detected_at` NULL accepts a backward detection; the TTL reads from `DETECTION_TTL_MS`. Set `detected_at` by writing the column directly in the fixture — do not sleep.
|
|
- [ ] **Step 2: run, confirm they fail** — `node --test server/__tests__/lanes-lib.test.js`.
|
|
- [ ] **Step 3: implement.** One added branch in `recordDetection`. Do not touch `withDetected`, `clearLane`, or the payload shape.
|
|
- [ ] **Step 4: run, confirm they pass;** then the full server suite.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): expire a stale detection so a lane can move backwards between sessions`.
|
|
|
|
---
|
|
|
|
## Task 2 (D2): the detected signal says what matched
|
|
|
|
**Files:** Modify `server/lib/stage-detect.js`, `server/__tests__/stage-detect.test.js`.
|
|
|
|
**Produces:** `detect()` returns a `signal` built from the span the rule's regex
|
|
actually matched plus surrounding context, instead of the whole flattened input.
|
|
A rule with no `match` (it fired on the tool name alone) keeps today's behaviour:
|
|
the flattened input, capped. The existing `capSignal` cap (120 chars, whitespace
|
|
collapsed, ellipsis) still applies last.
|
|
|
|
Concretely: `Bash` with
|
|
`cd /very/long/path && npm run test:server 2>&1 | tail -5` currently yields the
|
|
whole string; it must yield a signal containing `npm run test:server` and not the
|
|
`cd` prefix.
|
|
|
|
`detect()` must remain total — it never throws on any input.
|
|
|
|
- [ ] **Step 1: write the failing tests:** the Bash example above yields a signal containing `npm run test:server` and not `/very/long/path`; a rule with no `match` still yields the flattened input; a signal longer than the cap is still capped with the ellipsis; a matched span at the very start and at the very end of the input both survive; `detect` still returns null for an unmentioned tool.
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4: run, confirm they pass;** then the full server suite.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): report the matched span as the detection signal`.
|
|
|
|
---
|
|
|
|
## Task 3 (D3): `gitFacts()` in the worktree library
|
|
|
|
**Files:** Modify `server/lib/worktree.js`; modify `server/__tests__/worktree.test.js`.
|
|
|
|
**Produces:** `gitFacts(dir)` returning `{branch, head, subject, dirty, untracked}`.
|
|
It reuses the EXISTING `git()` wrapper and `statusCounts(dir)` in the same file —
|
|
do NOT add a second subprocess helper and do NOT build any command as a shell
|
|
string. `branch` comes from `rev-parse --abbrev-ref HEAD`, `subject` from
|
|
`log -1 --format=%s`, and `head`/`dirty`/`untracked` come from `statusCounts`.
|
|
It throws nothing the caller must catch beyond what `git()` already throws; the
|
|
route in D4 decides what a failure means.
|
|
|
|
No route, no HTTP, no OpenAPI in this task.
|
|
|
|
- [ ] **Step 1: write the failing tests** against a real temporary git repo fixture (`git init`, one commit, then one modified tracked file and one untracked file): the branch name, the short head matching `rev-parse --short HEAD`, the exact commit subject, `dirty: 1`, `untracked: 1`. Also: a detached HEAD yields a `branch` of `HEAD` (assert the exact value the command returns, do not invent one); a repo whose only commit has a subject containing spaces returns it whole.
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** full server suite.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): read branch, head, subject and working-tree counts from a worktree`.
|
|
|
|
---
|
|
|
|
## Task 4 (D4): the `GET /api/lanes/:id/git` route
|
|
|
|
**Files:** Modify `server/routes/lanes.js`, `server/openapi-extra/lanes.js` (+ regenerate `openapi.yaml`); modify `server/__tests__/lanes-api.test.js`.
|
|
|
|
**Produces:** `GET /api/lanes/:id/git` → `200 {available: true, ...facts}` for a git
|
|
worktree; `200 {available: false}` when the lane's `cwd` is missing, is not a git
|
|
repo, or git fails for any reason. A missing lane is `404`. It is a READ endpoint:
|
|
no same-origin guard (that guard is for the destructive actions), and it must
|
|
never mutate a lane.
|
|
|
|
Register the route **before** the `/:id/:action` catch-all, the same way
|
|
`/ensure` had to be — otherwise `git` is swallowed as an action name. State in
|
|
your report that you checked the ordering and how.
|
|
|
|
- [ ] **Step 1: write the failing tests:** a lane pointing at a real temporary git repo returns the branch, short head, subject, `dirty` and `untracked`; a lane whose `cwd` is a plain directory returns `{available: false}` with HTTP 200; a lane whose `cwd` does not exist returns `{available: false}`; an unknown lane id returns 404; the route is NOT shadowed by `/:id/:action` — assert the response body shape, not merely the status.
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** full server suite; `node scripts/generate-openapi-yaml.js` then confirm `git diff openapi.yaml` shows only the added path.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): expose a lane's git facts over the API`.
|
|
|
|
---
|
|
|
|
## Task 5 (D5): client API + types for git facts
|
|
|
|
**Files:** Modify `client/src/lib/api.ts`, `client/src/lib/types.ts`; modify the matching api test if one exists, else add the assertion to `client/src/lib/__tests__/`.
|
|
|
|
**Produces:** `api.lanes.git(id)` calling `GET /api/lanes/:id/git`, and a
|
|
`LaneGitFacts` type (`{available: true, branch, head, subject, dirty, untracked} | {available: false}`)
|
|
exported from `client/src/lib/types.ts`. Nothing renders it yet.
|
|
|
|
Keep the discriminated union — a caller must be forced to check `available`
|
|
before reading `branch`. Do not make the fields optional on one flat type.
|
|
|
|
- [ ] **Step 1: write the failing test:** `api.lanes.git(3)` requests exactly `/api/lanes/3/git` with method GET, and returns the parsed body.
|
|
- [ ] **Step 2: run, confirm it fails.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** `npm run test:client`, `npm run build`.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): client binding for a lane's git facts`.
|
|
|
|
---
|
|
|
|
## Task 6 (D6): rebuild the lane card's own fields
|
|
|
|
**Files:** Modify `client/src/components/lanes/LaneCard.tsx`, `client/src/i18n/locales/{en,zh,vi,ko}/lanes.json`; create `client/src/components/lanes/__tests__/LaneCard.test.tsx`.
|
|
|
|
**Produces:** the card laid out per the design doc's table, using only fields the
|
|
lane payload ALREADY carries: header row (`LANE <id>`, liveness dot, status),
|
|
title, declared-stage chip with progress bar / `%` / time-on-stage, the
|
|
dashed-amber `auto: <stage>` chip carrying `detected_signal` as its tooltip, the
|
|
kind and CI tags, the needs-you banner, and the action row.
|
|
|
|
**No git block in this task** — that is D7. Do not call `api.lanes.git` here.
|
|
|
|
The existing action wiring and `DestructiveLaneModal` usage are preserved
|
|
exactly: `reset` and `remove` keep their preflight and `expect` echo. Every
|
|
string goes through i18n in all four locales, genuinely translated.
|
|
|
|
The card shows a chip, never a node state — it must not render a detected stage
|
|
as done.
|
|
|
|
- [ ] **Step 1: write the failing tests** in `LaneCard.test.tsx`: every field of a fully-populated fixture lane renders with its exact value; the `auto` chip appears only when the detected stage leads the declared one, and its `title` contains the signal; a lane whose detected stage equals or trails the declared one shows NO auto chip; clicking `reset` opens the destructive modal rather than firing the action directly; the plain action callbacks fire with the right action name.
|
|
- [ ] **Step 2: run, confirm they fail** — `cd client && npx vitest run src/components/lanes/__tests__/LaneCard.test.tsx`.
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** `npm run test:client`, `npm run build`.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): rebuild the lane card for legibility`.
|
|
|
|
---
|
|
|
|
## Task 7 (D7): the card's git block
|
|
|
|
**Files:** Modify `client/src/components/lanes/LaneCard.tsx`, `client/src/i18n/locales/{en,zh,vi,ko}/lanes.json`; modify `client/src/components/lanes/__tests__/LaneCard.test.tsx`.
|
|
|
|
**Produces:** the card fetches its own facts through `api.lanes.git(lane.id)` on
|
|
mount and every 30s, and renders a git row — branch, short head, commit subject,
|
|
and the dirty/untracked counts. It renders the rest of the card unchanged while
|
|
the facts are still loading and whenever `available` is false. A failed request
|
|
is silent: no error banner, no retry storm.
|
|
|
|
The interval must be cleared on unmount.
|
|
|
|
- [ ] **Step 1: write the failing tests:** the git row renders each fact from a mocked `available: true` response; an `available: false` response renders the card with NO git row and no error; a rejected request renders the card with no git row and no error; unmounting clears the interval (assert the timer count, or that no further request is made after unmount).
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** `npm run test:client`, `npm run build`.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): show a lane's branch and working-tree state on its card`.
|
|
|
|
---
|
|
|
|
## Task 8 (D8): the page header and the card grid
|
|
|
|
**Files:** Modify `client/src/pages/Workspace.tsx`, `client/src/i18n/locales/{en,zh,vi,ko}/lanes.json`; modify `client/src/pages/__tests__/Workspace.test.tsx`.
|
|
|
|
**Produces:** the header bar — page title, the four counters (`lanes`, `running`,
|
|
`needs you`, `dead`) from the API's `counts`, and the Add-lane control — and the
|
|
responsive card grid (1 column, 2 at `md`, 3 at `xl`) replacing today's
|
|
horizontal lane strip. Selecting a card still drives the same `selectedLaneId`
|
|
state it does now.
|
|
|
|
Do NOT touch the console or the pipeline panel in this task; leave them exactly
|
|
where they are, below the grid, however they currently render.
|
|
|
|
- [ ] **Step 1: write the failing tests:** the four counters render the exact values from the API's `counts`; every lane in the response gets a card; clicking a card sets it selected (assert an observable consequence, e.g. the pipeline panel's lane, not an internal state variable).
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** `npm run test:client`, `npm run build`. The screens snapshot WILL change — read the diff, confirm it is only the header and grid, then regenerate.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): lane grid and counters in the Workspace header`.
|
|
|
|
---
|
|
|
|
## Task 9 (D9): the selected-lane detail panel
|
|
|
|
**Files:** Modify `client/src/pages/Workspace.tsx`, `client/src/i18n/locales/{en,zh,vi,ko}/lanes.json`; modify `client/src/pages/__tests__/Workspace.test.tsx`.
|
|
|
|
**Produces:** the detail panel between the header and the grid: the selected
|
|
lane's title, its declared stage and — when detection leads — the inferred one,
|
|
a large `PipelineMap`, and the legend naming the five node states plus the
|
|
dashed-amber inferred treatment.
|
|
|
|
**Do not change `PipelineMap` itself** — not its node-state logic, not its props.
|
|
This task places and sizes it.
|
|
|
|
- [ ] **Step 1: write the failing tests:** the panel shows the selected lane's title and declared stage; selecting a different card switches the panel's pipeline; **no node rendered in the panel carries both `data-detected="true"` and `data-state="done"`** (the sub-project B premise guard, re-asserted at the new layout); the legend names each of the five states.
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** `npm run test:client`, `npm run build`, snapshot diff read.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): a full-width pipeline panel for the selected lane`.
|
|
|
|
---
|
|
|
|
## Task 10 (D10): collapse the console
|
|
|
|
**Files:** Modify `client/src/pages/Workspace.tsx`, `client/src/i18n/locales/{en,zh,vi,ko}/run.json`; modify `client/src/pages/__tests__/Workspace.test.tsx`.
|
|
|
|
**Produces:** `RunSetup` + `RunConsole` + `RunHistory` wrapped in a disclosure
|
|
that starts collapsed and expands on click, with the console section moved above
|
|
the card grid so an expanded console sits beside the lane it belongs to.
|
|
|
|
**The subscription must stay mounted while collapsed.** Collapse the visual
|
|
container with CSS; do NOT conditionally unmount `RunConsole` — unmounting
|
|
disposes `useRunStream`'s subscription and a live run's envelopes are lost.
|
|
State in your report which mechanism you used and how you proved the
|
|
subscription survived.
|
|
|
|
No prop of `RunSetup`, `RunConsole` or `RunHistory` changes. The console still
|
|
never posts a stage.
|
|
|
|
- [ ] **Step 1: write the failing tests:** the console is collapsed on first render and expands on click; **an envelope delivered through the mocked event bus while the console is collapsed is present in the DOM once it is expanded**; after a full start-and-message cycle no request is made to any `/stage` URL.
|
|
- [ ] **Step 2: run, confirm they fail.**
|
|
- [ ] **Step 3: implement.**
|
|
- [ ] **Step 4:** `npm run test:client`, `npm run build`, snapshot diff read then regenerated.
|
|
- [ ] **Step 5:** header audit, commit — `feat(lanes): collapse the console without dropping its stream`.
|
|
|
|
---
|
|
|
|
## Task 11 (D11): docs
|
|
|
|
**Files:** Modify `docs/LANES.md`, `docs/API.md`, `README.md`, `ARCHITECTURE.md`, `CLAUDE.md`.
|
|
|
|
**Produces:** the new layout described where the old one was; `GET /api/lanes/:id/git` documented with its `available: false` contract and the reason it is not folded into `GET /api/lanes`; the detection TTL documented with `DETECTION_TTL_MS`, its default, and the explicit note that expiry does NOT weaken declared-wins or let inference render `done`. Every path, route and command printed must exist — verify each.
|
|
|
|
- [ ] **Step 1:** write the docs.
|
|
- [ ] **Step 2:** verify every referenced file, route and command exists (`ls`, `grep`, or run it).
|
|
- [ ] **Step 3:** full server suite, full client suite, `node scripts/generate-openapi-yaml.js` then `git diff openapi.yaml` empty.
|
|
- [ ] **Step 4:** header audit, commit — `docs(lanes): document the rebuilt Workspace, git facts and detection expiry`.
|
|
|
|
---
|
|
|
|
## Out of scope
|
|
|
|
- Tickets, preview-port links, per-lane credentials, and the `agents`/`creds` buttons from the reference screen — CCAM has no data behind any of them.
|
|
- Any change to `PipelineMap`'s node-state logic, the destroy guard, or the preflight contract.
|
|
- Inferring `done` or a gate outcome. Still forbidden, TTL or not.
|
|
- Re-styling any page other than `/run`.
|