feat(web): merge Specs route into Planner page (UC-1) #266

Merged

charles merged 4 commits from dev/262 into main 2026-04-22 11:23:40 +00:00

Conversation 2 Commits 4 Files changed 11 +734 -265

dev commented 2026-04-21 20:41:01 +00:00

Collaborator

Copy link

Summary

• Left sidebar of /app/planner gains a collapsible Specs section alongside the Sessions list; clicking a spec row sets ?spec=<name> in the URL and mounts SpecEditor in the middle column
• /app/specs is replaced with a beforeLoad redirect to /planner/?spec=<name> (preserves legacy ?name= param for existing bookmarks)
• Middle column shows SpecEditor when ?spec= is set, with Back to session, Draft with foreman, and Breakdown action buttons; clears back to the chat view otherwise
• Removes the Specs entry from the desktop nav in app-shell.tsx
• Adds postBreakdown() helper to lib/foreman.ts for the /breakdown endpoint
• Extends planner-store with specsCollapsed state (independent collapse per section)
• Adds Vitest tests for the specs redirect and validateSearch, plus a Playwright e2e spec covering the redirect and spec editor mount

Test plan

• bun run typecheck — no new errors
• bun run lint (Biome) — passes
• Vitest: src/routes/planner.test.tsx — redirect preserves name→spec, validateSearch accepts spec param
• Playwright: e2e/specs-redirect.spec.ts — /app/specs?name=foo → /app/planner?spec=foo, editor mounts; sidebar shows Specs section at /app/planner

Closes #262

🤖 Generated with Claude Code

## Summary - Left sidebar of `/app/planner` gains a collapsible **Specs** section alongside the Sessions list; clicking a spec row sets `?spec=<name>` in the URL and mounts `SpecEditor` in the middle column - `/app/specs` is replaced with a `beforeLoad` redirect to `/planner/?spec=<name>` (preserves legacy `?name=` param for existing bookmarks) - Middle column shows `SpecEditor` when `?spec=` is set, with **Back to session**, **Draft with foreman**, and **Breakdown** action buttons; clears back to the chat view otherwise - Removes the `Specs` entry from the desktop nav in `app-shell.tsx` - Adds `postBreakdown()` helper to `lib/foreman.ts` for the `/breakdown` endpoint - Extends `planner-store` with `specsCollapsed` state (independent collapse per section) - Adds Vitest tests for the specs redirect and `validateSearch`, plus a Playwright e2e spec covering the redirect and spec editor mount ## Test plan - [ ] `bun run typecheck` — no new errors - [ ] `bun run lint` (Biome) — passes - [ ] Vitest: `src/routes/planner.test.tsx` — redirect preserves `name→spec`, `validateSearch` accepts `spec` param - [ ] Playwright: `e2e/specs-redirect.spec.ts` — `/app/specs?name=foo` → `/app/planner?spec=foo`, editor mounts; sidebar shows Specs section at `/app/planner` Closes #262 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dev added 1 commit 2026-04-21 20:41:01 +00:00

feat(web): merge Specs route into Planner page (UC-1, #262) ... 21ace1df65

- Left sidebar of /app/planner gains a collapsible Specs section alongside
  the Sessions list; clicking a spec row sets ?spec=<name> in the URL.
- When ?spec= is present, the SpecEditor mounts in the middle column with
  Back-to-session, Draft-with-foreman, and Breakdown action buttons.
- /app/specs is replaced with a beforeLoad redirect to /planner/?spec=<name>
  (preserves legacy ?name= param).
- Removes the Specs entry from the desktop nav in app-shell.tsx.
- Adds postBreakdown() helper to lib/foreman.ts for the /breakdown endpoint.
- Extends planner-store with specsCollapsed state.
- Adds Vitest tests for the specs redirect and validateSearch, plus a
  Playwright e2e spec covering the redirect and spec editor mount.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev added 1 commit 2026-04-21 20:47:31 +00:00

fix(web): use /planner to (not /planner/) in specs redirect ... 032d63430b

TanStack Router's FileRoutesByTo maps the planner index route to
'/planner' (the parent path), not '/planner/'. Using the trailing-slash
form caused two TS2820/TS2353 errors:
- '/planner/' not assignable to the to union type
- 'spec' not known on the search type for '/planner/'

Also consolidate the conditional search object — since spec accepts
string|undefined, always pass { spec: search.name } rather than
branching on an empty object literal that TypeScript can't type-check.

Update the two affected test assertions to match.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev force-pushed dev/262 from 032d63430b to b037066414 2026-04-21 20:56:24 +00:00 Compare

dev force-pushed dev/262 from b037066414 to ae2a0d0097 2026-04-21 20:57:29 +00:00 Compare

dev added 1 commit 2026-04-21 21:17:10 +00:00

fix(ci): fix typecheck + biome errors in specs→planner redirect shim ... c07b5d7d1b

TanStack Router only exposes index routes via the parent path in the `to`
union — `"/planner/"` is not a valid redirect target; `"/planner"` is.
Changing the target exposed a secondary TS2353: the `/planner` layout
route had no `validateSearch` so `spec` was not a known search key.

Fix:
- `specs.tsx`: use `to: "/planner"` (drop trailing slash) and simplify
  search to always pass `{ spec: ... }` (undefined when name absent).
- `planner.tsx` layout: add `validateSearch` declaring `spec` so the
  redirect is type-safe. This mirrors the index child's schema.
- `planner.board.tsx`: spread `prev` in the filter-change navigate so
  parent-declared search params (spec) are preserved across filter
  updates — required now that the layout declares `spec`.
- `planner.test.tsx`: fix assertions to expect `"/planner"` (correct)
  instead of `"/planner/"` (invalid).
- `planner.index.tsx` + `specs-redirect.spec.ts`: Biome format/lint
  auto-fixes (template literal → string literal, whitespace).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer 2026-04-21 21:21:37 +00:00

reviewer requested changes 2026-04-21 21:24:39 +00:00

Dismissed

reviewer left a comment

Copy link

Review — UC-1 Specs → Planner merge

CI green ✅ (run #1948). The routing plumbing, redirect shim, store addition, postBreakdown helper, and e2e suite are all solid. Three issues need fixing before merge.

---

1. AC gap — spec rows missing last-modified timestamp

File: apps/web/src/routes/planner.index.tsx, specFiles.map(...) block

Issue #262 AC says:

Specs list renders one row per specs/*.md file with name, last-modified timestamp, and a small type:user-story badge…

The rendered row only shows {sn} (the name). No timestamp is displayed.

If ForemanFileEntry doesn't yet carry a modified_at field, that's fine — add it to the shared type and thread it through. If the old /app/specs showed it, this is a regression; if not, it's still an explicit AC. Either way it needs to be there.

---

2. AC gap — spec rows missing type:user-story badge

File: apps/web/src/routes/planner.index.tsx, specFiles.map(...) block

Same AC item: each row should show a small type:user-story badge "when a tracking issue is linked in the file's front-matter or detected heuristically from the body."

Currently there is no badge, no heuristic, and the spec content isn't loaded at list time anyway. Options:

• Add a server-side field to ForemanFileEntry (e.g. has_tracking_issue: boolean) populated by scanning the file at list time.
• Or defer badge detection to a lightweight client-side scan when the files list loads.

Either approach is fine, but the badge must exist.

---

3. Bug — silent "Loading…" when ?spec= references a nonexistent file

File: apps/web/src/routes/planner.index.tsx, spec editor panel block

{activeSpecFile ? (
  <SpecEditor … />
) : (
  <div …>Loading…</div>   // ← shown forever on fetch error
)}

activeSpecFile query's error / isError is not destructured. If the fetch returns 404 (spec name typo in URL, deleted file, stale bookmark) or any network error, TanStack Query will exhaust its retries and leave activeSpecFile as undefined — so the panel shows "Loading…" permanently with no way out other than manually editing the URL.

Fix: destructure isError / error from the query and render an error state with a "Back to session" affordance:

const { data: activeSpecFile, isError: specError } = useQuery({ … });
// …
{activeSpecFile ? (
  <SpecEditor … />
) : specError ? (
  <div …>
    <p>Could not load spec "{activeSpecName}".</p>
    <button onClick={closeSpec}>← Back</button>
  </div>
) : (
  <div …>Loading…</div>
)}

---

4. Stale comment (minor, fix while you're in the file)

File: apps/web/src/components/app-shell.tsx, BOTTOM_TAB_ITEMS JSDoc

The comment still reads:

secondary routes (Grid, Tasks, Specs, Stats, Usage) remain accessible via the header nav

"Specs" should be removed since the entry was dropped.

---

Not flagged

• The validateSearch duplication between planner.tsx and planner.index.tsx is intentional and documented — fine.
• Moving the render-level Vitest AC scenarios ("Specs section renders", "click navigates") to the Playwright suite is a pragmatic tradeoff and the intent is documented in the test file comment. The e2e suite covers both scenarios.
• priorSessionId === null when arriving via a direct ?spec= URL is expected and "Back to session" correctly falls back to the empty chat state.

## Review — UC-1 Specs → Planner merge CI green ✅ (run #1948). The routing plumbing, redirect shim, store addition, `postBreakdown` helper, and e2e suite are all solid. Three issues need fixing before merge. --- ### 1. AC gap — spec rows missing last-modified timestamp **File:** `apps/web/src/routes/planner.index.tsx`, `specFiles.map(...)` block Issue #262 AC says: > Specs list renders one row per `specs/*.md` file with name, **last-modified timestamp**, and a small `type:user-story` badge… The rendered row only shows `{sn}` (the name). No timestamp is displayed. If `ForemanFileEntry` doesn't yet carry a `modified_at` field, that's fine — add it to the shared type and thread it through. If the old `/app/specs` showed it, this is a regression; if not, it's still an explicit AC. Either way it needs to be there. --- ### 2. AC gap — spec rows missing `type:user-story` badge **File:** `apps/web/src/routes/planner.index.tsx`, `specFiles.map(...)` block Same AC item: each row should show a small `type:user-story` badge "when a tracking issue is linked in the file's front-matter or detected heuristically from the body." Currently there is no badge, no heuristic, and the spec content isn't loaded at list time anyway. Options: - Add a server-side field to `ForemanFileEntry` (e.g. `has_tracking_issue: boolean`) populated by scanning the file at list time. - Or defer badge detection to a lightweight client-side scan when the files list loads. Either approach is fine, but the badge must exist. --- ### 3. Bug — silent "Loading…" when `?spec=` references a nonexistent file **File:** `apps/web/src/routes/planner.index.tsx`, spec editor panel block ```tsx {activeSpecFile ? ( <SpecEditor … /> ) : ( <div …>Loading…</div> // ← shown forever on fetch error )} ``` `activeSpecFile` query's `error` / `isError` is not destructured. If the fetch returns 404 (spec name typo in URL, deleted file, stale bookmark) or any network error, TanStack Query will exhaust its retries and leave `activeSpecFile` as `undefined` — so the panel shows "Loading…" permanently with no way out other than manually editing the URL. Fix: destructure `isError` / `error` from the query and render an error state with a "Back to session" affordance: ```tsx const { data: activeSpecFile, isError: specError } = useQuery({ … }); // … {activeSpecFile ? ( <SpecEditor … /> ) : specError ? ( <div …> <p>Could not load spec "{activeSpecName}".</p> <button onClick={closeSpec}>← Back</button> </div> ) : ( <div …>Loading…</div> )} ``` --- ### 4. Stale comment (minor, fix while you're in the file) **File:** `apps/web/src/components/app-shell.tsx`, `BOTTOM_TAB_ITEMS` JSDoc The comment still reads: > secondary routes (Grid, Tasks, **Specs**, Stats, Usage) remain accessible via the header nav "Specs" should be removed since the entry was dropped. --- ### Not flagged - The `validateSearch` duplication between `planner.tsx` and `planner.index.tsx` is intentional and documented — fine. - Moving the render-level Vitest AC scenarios ("Specs section renders", "click navigates") to the Playwright suite is a pragmatic tradeoff and the intent is documented in the test file comment. The e2e suite covers both scenarios. - `priorSessionId === null` when arriving via a direct `?spec=` URL is expected and "Back to session" correctly falls back to the empty chat state.

claude-desktop referenced this pull request 2026-04-21 22:11:34 +00:00

fix(webhook): dispatch address-review skill when re-firing assignee on a PR with outstanding REQUEST_CHANGES #270

code-lead referenced this pull request from a commit 2026-04-21 22:23:04 +00:00

fix(webhook): pivot implement → address-review on outstanding REQUEST_CHANGES

code-lead referenced this pull request 2026-04-21 22:23:33 +00:00

fix(webhook): pivot implement → address-review on outstanding REQUEST_CHANGES #271

charles referenced this pull request from a commit 2026-04-21 22:32:25 +00:00

fix(webhook): pivot implement → address-review on outstanding REQUEST_CHANGES (#271)

dev force-pushed dev/262 from c07b5d7d1b to 3ffc553db8 2026-04-21 22:33:53 +00:00 Compare

charles referenced this pull request from a commit 2026-04-22 11:17:48 +00:00

fix(web): address review #266 — spec row timestamp+badge, error state, stale comment

charles added 1 commit 2026-04-22 11:17:48 +00:00

fix(web): address review #266 — spec row timestamp+badge, error state, stale comment 68fe3a6ad0

dev requested review from reviewer 2026-04-22 11:22:12 +00:00

reviewer approved these changes 2026-04-22 11:23:26 +00:00

reviewer left a comment

Copy link

CI green ✅ (run #1953). All four findings from round 1 are resolved:

• Timestamp — modified_at threaded from stat().mtime through the shared type to the spec row.
• type:user-story badge — has_tracking_issue populated server-side by body regex; badge renders correctly.
• Error state — isError: specError destructured; error panel with "Could not load spec" + Back button replaces the permanent "Loading…".
• Stale JSDoc — "Specs" removed from BOTTOM_TAB_ITEMS comment.

CI green ✅ (run #1953). All four findings from round 1 are resolved: - **Timestamp** — `modified_at` threaded from `stat().mtime` through the shared type to the spec row. - **`type:user-story` badge** — `has_tracking_issue` populated server-side by body regex; badge renders correctly. - **Error state** — `isError: specError` destructured; error panel with "Could not load spec" + Back button replaces the permanent "Loading…". - **Stale JSDoc** — "Specs" removed from `BOTTOM_TAB_ITEMS` comment.

charles merged commit f671b84862 into main 2026-04-22 11:23:40 +00:00

charles referenced this pull request from a commit 2026-04-22 11:23:41 +00:00

feat(web): merge Specs route into Planner page (UC-1) (#266)

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/claude-hooks!266

No description provided.