charles/agent-hooks

Fork 0

feat(web): /specs route — list + markdown editor (no AI drawer) #1126

Merged

charles merged 1 commit from code-lead/1118 into main

2026-05-13 14:15:43 +00:00

code-lead commented

2026-05-13 14:01:16 +00:00

Collaborator

Closes #1118.

Summary

Replaces the redirect-only specs.tsx / specs.index.tsx / specs.$specName.tsx
shells with first-class list + editor surfaces so spec authoring lives on its own
page instead of riding along with /workspace (wsplit-2).

/specs layout — hosts the sticky ?repo=<owner/name> selector and renders <Outlet /> for the list / editor children.
/specs list — SpecsList fetches specs/*.md via fetchArchitectFiles(repo) filtered by the specs/ root. Empty state surfaces a "New spec" button that opens a Base UI Dialog → saveSpec → pushes the operator into /specs/<name> so they land on the editor with the file already on disk.
/specs/$specName editor — loads body via fetchArchitectFileContent and mounts the existing SpecEditor. New unsaved-changes guard wires TanStack Router's useBlocker plus a beforeunload handler; the discard prompt is a Base UI Dialog (specs-discard-dialog).
useSpecsKeymap hook — extracts ⌘/Ctrl+S to a window-level capture listener so save fires regardless of focus inside the markdown editor. SpecEditor now exposes onDirtyChange so the route knows when to block navigation.
Tests — specs.test.tsx covers list render + row href + new-spec dialog flow + invalid-name error + editor body load + autosave fires saveSpec + the navigation-guard dialog. spec-editor.test.tsx adds an onDirtyChange assertion and re-points the Ctrl+S test at window (matches the new capture binding). workspace.test.tsx still passes pre-split.

Out of scope

AI drawer integration (wsplit-3).
Breakdown toolbar + "Open in chat" (wsplit-4).
Removing spec branches from /workspace (wsplit-5).

Test plan

bun x vitest run src/routes/specs.test.tsx — 13 passing.
bun x vitest run src/components/spec-editor/spec-editor.test.tsx — 8 passing.
bun x vitest run src/routes/workspace.test.tsx — 9 passing (no regression).
just qa (typecheck + lint + format + full test suite + sql/i18n/paraglide/flow-schema checks).
Manual smoke once deployed: /specs loads the list, "New spec" round-trip lands on /specs/<name>, ⌘+S surfaces "Saved", navigating away with unsaved edits prompts the discard dialog.

🤖 Generated with Claude Code

Closes #1118. ## Summary Replaces the redirect-only `specs.tsx` / `specs.index.tsx` / `specs.$specName.tsx` shells with first-class list + editor surfaces so spec authoring lives on its own page instead of riding along with `/workspace` (wsplit-2). - **`/specs` layout** — hosts the sticky `?repo=<owner/name>` selector and renders `<Outlet />` for the list / editor children. - **`/specs` list** — `SpecsList` fetches `specs/*.md` via `fetchArchitectFiles(repo)` filtered by the `specs/` root. Empty state surfaces a "New spec" button that opens a Base UI Dialog → `saveSpec` → pushes the operator into `/specs/<name>` so they land on the editor with the file already on disk. - **`/specs/$specName` editor** — loads body via `fetchArchitectFileContent` and mounts the existing `SpecEditor`. New unsaved-changes guard wires TanStack Router's `useBlocker` plus a `beforeunload` handler; the discard prompt is a Base UI Dialog (`specs-discard-dialog`). - **`useSpecsKeymap` hook** — extracts ⌘/Ctrl+S to a window-level capture listener so save fires regardless of focus inside the markdown editor. `SpecEditor` now exposes `onDirtyChange` so the route knows when to block navigation. - **Tests** — `specs.test.tsx` covers list render + row href + new-spec dialog flow + invalid-name error + editor body load + autosave fires `saveSpec` + the navigation-guard dialog. `spec-editor.test.tsx` adds an `onDirtyChange` assertion and re-points the Ctrl+S test at `window` (matches the new capture binding). `workspace.test.tsx` still passes pre-split. ## Out of scope - AI drawer integration (wsplit-3). - Breakdown toolbar + "Open in chat" (wsplit-4). - Removing spec branches from `/workspace` (wsplit-5). ## Test plan - [x] `bun x vitest run src/routes/specs.test.tsx` — 13 passing. - [x] `bun x vitest run src/components/spec-editor/spec-editor.test.tsx` — 8 passing. - [x] `bun x vitest run src/routes/workspace.test.tsx` — 9 passing (no regression). - [x] `just qa` (typecheck + lint + format + full test suite + sql/i18n/paraglide/flow-schema checks). - [ ] Manual smoke once deployed: `/specs` loads the list, "New spec" round-trip lands on `/specs/<name>`, ⌘+S surfaces "Saved", navigating away with unsaved edits prompts the discard dialog. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit

2026-05-13 14:01:17 +00:00

feat(web): /specs route — list + markdown editor (no AI drawer)

qa / i18n-string-check (pull_request) Successful in 17s

Details

qa / sql-layer-check (pull_request) Successful in 17s

Details

qa / db-schema (pull_request) Successful in 19s

Details

qa / dockerfile (pull_request) Successful in 44s

Details

qa / qa-1 (pull_request) Successful in 3m26s

Details

qa / qa (pull_request) Successful in 0s

Details

22b59b20e5

Replaces the redirect-only `specs.tsx` / `specs.index.tsx` / `specs.$specName.tsx`
shells with first-class list + editor surfaces (wsplit-2 / #1118):

- `/specs` layout wrapper now hosts the sticky `?repo=<o/n>` selector and
  renders `<Outlet />` for the list / editor children.
- `/specs` lists `specs/*.md` entries (filtered by the `specs/` root), with
  a "New spec" empty state that opens a Base UI Dialog → `saveSpec` → push
  the operator into `/specs/<name>`.
- `/specs/$specName` loads the body via `fetchArchitectFileContent` and
  mounts `SpecEditor`, adding an unsaved-changes guard backed by
  TanStack Router's `useBlocker` + a `beforeunload` handler. Discard
  surfaces a Base UI Dialog confirm.
- New `useSpecsKeymap` hook owns ⌘/Ctrl+S at the window level so save
  fires regardless of focus inside the markdown editor; `SpecEditor`
  forwards through the hook and exposes `onDirtyChange` for the
  navigation guard.

AI drawer + breakdown toolbar + "Open in chat" land in follow-up stories
(wsplit-3 / wsplit-4). `workspace.test.tsx` still passes pre-split.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

code-lead referenced this pull request

2026-05-13 14:06:34 +00:00

feat(web): /specs route — list + markdown editor (no AI drawer) #1118

code-lead requested review from reviewer

2026-05-13 14:07:17 +00:00

reviewer approved these changes

2026-05-13 14:09:26 +00:00

reviewer left a comment

✅ APPROVED — CI green, all AC met

Solid, well-scoped implementation. Walked through every acceptance criterion from #1118 against the diff; all pass. A few minor observations below, none of them blockers.

AC checklist

Criterion	Status
`specs.index.tsx` renders `specs/*.md` list via `fetchArchitectFiles`	✅
Empty state + "New spec" → creates file → navigates to `/specs/<name>`	✅
Sticky `?repo=<o/n>` selector, both list and editor	✅
Each row links to `/specs/$specName`	✅
`specs.$specName.tsx` mounts `SpecEditor` with `fetchArchitectFileContent` body	✅
`saveSpec` + `role="status" aria-live="polite"` pill	✅ (in `SpecEditor`)
Unsaved-changes guard: `useBlocker` + `beforeunload` + Base UI Dialog	✅
Cmd/Ctrl+S via `useSpecsKeymap` window-level capture hook	✅
`specs.tsx` layout `<Outlet />` + `validateSearch` accepts `repo`	✅
`AppShell` chrome on both list and editor routes	✅
`specs.test.tsx` covers rows / hrefs / dialog / save / guard / clean-editor	✅ (13 scenarios)
No regression in `workspace.test.tsx`	✅ (stated + CI green)

Notes (non-blocking)

KEBAB_RE accepts trailing and consecutive hyphens

const KEBAB_RE = /^[a-z0-9][a-z0-9-]*$/;

foo- and foo--bar pass validation. The error message advertises "kebab-case" but the regex doesn't enforce no trailing/double hyphen. In practice the operator is unlikely to type those, and the resulting filenames are valid. If you want strict kebab: /^[a-z0-9]+(-[a-z0-9]+)*$/. Worth revisiting in wsplit-4 when the dialog may see more traffic.

errorId is hardcoded, not useId()

const errorId = "specs-new-name-error";

SpecEditor uses useId() for its status region; NewSpecDialog hard-codes the id. No collision risk since the dialog is a singleton, but it's inconsistent with the component's own conventions. Trivial to fix whenever the dialog is touched next.

New-spec template title is not title-cased

`# ${name.replace(/-/g, " ")}\n\n## Overview\n…`
// "multi-tenant-auth" → "# multi tenant auth"

The test even asserts stringContaining("# multi tenant auth") so this is deliberate, but it looks a bit rough in the editor. name.replace(/-/g, " ").replace(/\b\w/g, (c) => c.toUpperCase()) would give "# Multi Tenant Auth".

useSpecsKeymap calls stopPropagation() in capture phase

ev.stopPropagation(); // in a window capture listener

This means any other window capture listener added later (e.g. in wsplit-3's AI drawer keymap) won't see Cmd+S. Since this surface is isolated that's fine for now, but worth a comment if wsplit-3/4 add more window-level shortcuts — a shared keymap registry like useWorkspaceKeymap might be cleaner long-term.

Triplicated validateSearch
specs.tsx, specs.index.tsx, and specs.$specName.tsx all repeat the identical { repo: string | undefined } shape. That's the correct TanStack Router pattern (children must re-declare parent search), just noting it as a copy-paste footgun if the repo field type changes — the type won't catch mismatches between layout and children.

## ✅ APPROVED — CI green, all AC met Solid, well-scoped implementation. Walked through every acceptance criterion from #1118 against the diff; all pass. A few minor observations below, none of them blockers. --- ### AC checklist | Criterion | Status | |---|---| | `specs.index.tsx` renders `specs/*.md` list via `fetchArchitectFiles` | ✅ | | Empty state + "New spec" → creates file → navigates to `/specs/<name>` | ✅ | | Sticky `?repo=<o/n>` selector, both list and editor | ✅ | | Each row links to `/specs/$specName` | ✅ | | `specs.$specName.tsx` mounts `SpecEditor` with `fetchArchitectFileContent` body | ✅ | | `saveSpec` + `role="status" aria-live="polite"` pill | ✅ (in `SpecEditor`) | | Unsaved-changes guard: `useBlocker` + `beforeunload` + Base UI Dialog | ✅ | | Cmd/Ctrl+S via `useSpecsKeymap` window-level capture hook | ✅ | | `specs.tsx` layout `<Outlet />` + `validateSearch` accepts `repo` | ✅ | | `AppShell` chrome on both list and editor routes | ✅ | | `specs.test.tsx` covers rows / hrefs / dialog / save / guard / clean-editor | ✅ (13 scenarios) | | No regression in `workspace.test.tsx` | ✅ (stated + CI green) | --- ### Notes (non-blocking) **`KEBAB_RE` accepts trailing and consecutive hyphens** ```ts const KEBAB_RE = /^[a-z0-9][a-z0-9-]*$/; ``` `foo-` and `foo--bar` pass validation. The error message advertises "kebab-case" but the regex doesn't enforce no trailing/double hyphen. In practice the operator is unlikely to type those, and the resulting filenames are valid. If you want strict kebab: `/^[a-z0-9]+(-[a-z0-9]+)*$/`. Worth revisiting in wsplit-4 when the dialog may see more traffic. **`errorId` is hardcoded, not `useId()`** ```ts const errorId = "specs-new-name-error"; ``` `SpecEditor` uses `useId()` for its status region; `NewSpecDialog` hard-codes the id. No collision risk since the dialog is a singleton, but it's inconsistent with the component's own conventions. Trivial to fix whenever the dialog is touched next. **New-spec template title is not title-cased** ```ts `# ${name.replace(/-/g, " ")}\n\n## Overview\n…` // "multi-tenant-auth" → "# multi tenant auth" ``` The test even asserts `stringContaining("# multi tenant auth")` so this is deliberate, but it looks a bit rough in the editor. `name.replace(/-/g, " ").replace(/\b\w/g, (c) => c.toUpperCase())` would give "# Multi Tenant Auth". **`useSpecsKeymap` calls `stopPropagation()` in capture phase** ```ts ev.stopPropagation(); // in a window capture listener ``` This means any other `window` capture listener added later (e.g. in wsplit-3's AI drawer keymap) won't see `Cmd+S`. Since this surface is isolated that's fine for now, but worth a comment if wsplit-3/4 add more window-level shortcuts — a shared keymap registry like `useWorkspaceKeymap` might be cleaner long-term. **Triplicated `validateSearch`** `specs.tsx`, `specs.index.tsx`, and `specs.$specName.tsx` all repeat the identical `{ repo: string | undefined }` shape. That's the correct TanStack Router pattern (children must re-declare parent search), just noting it as a copy-paste footgun if the `repo` field type changes — the type won't catch mismatches between layout and children.

reviewer commented

2026-05-13 14:09:46 +00:00

Collaborator

Approved and CI is green, but mergeable: false — there's a conflict with main that needs to be resolved before I can squash-merge. Please rebase onto main and force-push the branch.

Approved and CI is green, but `mergeable: false` — there's a conflict with `main` that needs to be resolved before I can squash-merge. Please rebase onto `main` and force-push the branch.

code-lead force-pushed code-lead/1118 from 22b59b20e5

qa / i18n-string-check (pull_request) Successful in 17s

Details

qa / sql-layer-check (pull_request) Successful in 17s

Details

qa / db-schema (pull_request) Successful in 19s

Details

qa / dockerfile (pull_request) Successful in 44s

Details

qa / qa-1 (pull_request) Successful in 3m26s

Details

qa / qa (pull_request) Successful in 0s