feat(planner): architect chat UI with streaming + slash + @file (M18-5) #192

Merged

code-lead merged 2 commits from boss/166 into main 2026-04-20 22:20:09 +00:00

Conversation 3 Commits 2 Files changed 24 +2575 -7

code-lead commented 2026-04-20 21:45:47 +00:00

Collaborator

Copy link

Ships /app/planner — the operator-facing chat surface against the host-mode
architect agent from #165.

Closes #166

Summary

• Layout: collapsible sessions pane + transcript + auto-growing composer.
  New-session button clears the selection so the empty state surfaces the
  read-only "System" block (default prompt from GET /architect/config).
  Delete button opens a Base UI Dialog confirm modal.
• Streaming: useArchitectStream(taskId) subscribes to
  /architect/stream/:task_id. Tool-call events collapse into titled pills
  (click to expand input JSON); assistant text streams through
  react-markdown + remark-gfm with a blinking-cursor tail.
• Markdown: fenced code blocks lazy-load shiki and render with the
  tokyo-night theme. Plain <pre> fallback before shiki resolves keeps
  the happy-dom tests (and the first paint) free of WASM dependencies.
• Palettes: composer's / trigger opens the slash palette (/spec,
  /breakdown, /assign); @ opens a file picker backed by
  GET /architect/files (allowlist: specs/*.md + config/agents.json).
  On send, expandFileMentions inlines the contents of @specs/foo.md
  mentions before POSTing the turn.
• Server: three new endpoints on apps/server/src/architect.ts —
  /architect/config (surfaces default prompt + model), /architect/files
  (picker list), /architect/files/content?path= (with a strict
  resolveAllowlistedFile guard: no .., no absolute paths, allowlist
  gated to specs/*.md + config/agents.json).
• Shared types: packages/shared/src/architect.ts carries the wire
  contract (session view, list entry, chat response, config, files). Both
  apps import through @claude-hooks/shared.
• Nav + proxy: Planner lands on the AppShell nav tabs; Vite dev proxy
  picks up /architect and /whoami.

Test plan

• bun x turbo run typecheck lint test — 768 server tests + 37 web
  tests pass, Biome clean.
• bun x turbo run build — web bundle builds (shiki langs code-split
  per language).
• Vitest transcript test exercises the streaming tool-call pill toggle +
  persisted markdown render against a fixture stream.
• Vitest composer tests cover probePalette (slash vs file trigger,
  URL / email false-positives), keyboard send, and disabled state.
• Vitest attachments.test.ts covers findFileMentions + fetch-failure
  handling in expandFileMentions.
• Server-side architect.test.ts gains coverage for /architect/config,
  /architect/files, /architect/files/content (happy path, bad
  path, missing file), plus resolveAllowlistedFile allowlist shape.
• Playwright planner.spec.ts — mocks /architect/*, sends a message,
  asserts the tool-call pill toggles + the assistant markdown bubble
  paints <strong> bold.

🤖 Generated with Claude Code

Ships `/app/planner` — the operator-facing chat surface against the host-mode architect agent from #165. Closes #166 ## Summary - **Layout**: collapsible sessions pane + transcript + auto-growing composer. New-session button clears the selection so the empty state surfaces the read-only "System" block (default prompt from `GET /architect/config`). Delete button opens a Base UI `Dialog` confirm modal. - **Streaming**: `useArchitectStream(taskId)` subscribes to `/architect/stream/:task_id`. Tool-call events collapse into titled pills (click to expand input JSON); assistant text streams through `react-markdown` + `remark-gfm` with a blinking-cursor tail. - **Markdown**: fenced code blocks lazy-load `shiki` and render with the `tokyo-night` theme. Plain `<pre>` fallback before shiki resolves keeps the happy-dom tests (and the first paint) free of WASM dependencies. - **Palettes**: composer's `/` trigger opens the slash palette (`/spec`, `/breakdown`, `/assign`); `@` opens a file picker backed by `GET /architect/files` (allowlist: `specs/*.md` + `config/agents.json`). On send, `expandFileMentions` inlines the contents of `@specs/foo.md` mentions before POSTing the turn. - **Server**: three new endpoints on `apps/server/src/architect.ts` — `/architect/config` (surfaces default prompt + model), `/architect/files` (picker list), `/architect/files/content?path=` (with a strict `resolveAllowlistedFile` guard: no `..`, no absolute paths, allowlist gated to `specs/*.md` + `config/agents.json`). - **Shared types**: `packages/shared/src/architect.ts` carries the wire contract (session view, list entry, chat response, config, files). Both apps import through `@claude-hooks/shared`. - **Nav + proxy**: Planner lands on the `AppShell` nav tabs; Vite dev proxy picks up `/architect` and `/whoami`. ## Test plan - [x] `bun x turbo run typecheck lint test` — 768 server tests + 37 web tests pass, Biome clean. - [x] `bun x turbo run build` — web bundle builds (shiki langs code-split per language). - [x] Vitest transcript test exercises the streaming tool-call pill toggle + persisted markdown render against a fixture stream. - [x] Vitest composer tests cover `probePalette` (slash vs file trigger, URL / email false-positives), keyboard send, and disabled state. - [x] Vitest `attachments.test.ts` covers `findFileMentions` + fetch-failure handling in `expandFileMentions`. - [x] Server-side `architect.test.ts` gains coverage for `/architect/config`, `/architect/files`, `/architect/files/content` (happy path, bad path, missing file), plus `resolveAllowlistedFile` allowlist shape. - [x] Playwright `planner.spec.ts` — mocks `/architect/*`, sends a message, asserts the tool-call pill toggles + the assistant markdown bubble paints `<strong>` bold. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

code-lead added 1 commit 2026-04-20 21:45:47 +00:00

feat(planner): architect chat UI with streaming + slash + @file (M18-5) ... d7af16089e

Ships /app/planner — the operator-facing chat surface against the host-mode
architect agent landed in #165. Two-pane layout (sessions list + transcript +
composer), per-turn SSE streaming with tool-call pills + react-markdown +
lazy-loaded shiki code highlighting, and slash-command / @file autocomplete
backed by new /architect/{config,files,files/content} endpoints.

Server: adds GET /architect/config (default system prompt + model),
GET /architect/files (allowlisted spec/config picker), and
GET /architect/files/content?path= with strict path-traversal guards in
resolveAllowlistedFile. Shared types live in packages/shared/src/architect.ts.

Web: new routes/planner.tsx + components/planner/* (sessions pane with Base UI
delete-confirm dialog, transcript with streaming bubble + tool-call pill
toggle, composer with auto-grow + palette), lib/architect.ts REST helpers +
useArchitectStream hook, stores/planner-store.ts. Vite proxy picks up
/architect + /whoami; nav gains the Planner tab.

Tests: composer/transcript/attachments/probePalette vitest coverage plus a
Playwright smoke covering send → tool pill toggle → markdown render. Server
side gains tests for config + files + resolveAllowlistedFile allowlist.

Closes #166

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-lead requested review from reviewer 2026-04-20 21:50:31 +00:00

code-lead force-pushed boss/166 from d7af16089e to 881b99ff83 2026-04-20 21:51:13 +00:00 Compare

reviewer requested changes 2026-04-20 21:53:50 +00:00

Dismissed

reviewer left a comment

Copy link

Review — M18-5 Architect Chat UI

CI ✅ green. All AC from #166 is met: layout, streaming, tool-call pills, markdown, session management, system-prompt block, slash palette, @file expansion, and the server-side resolveAllowlistedFile path-traversal guard. One correctness issue needs fixing before merge.

---

❌ EventSource not closed on terminal result — spurious reconnection loop

File: apps/web/src/lib/architect.ts, inside useArchitectStream's es.onmessage handler.

What happens today: When the per-task SSE stream emits a result event, the hook correctly sets phase → "done" in React state, but it does not close the EventSource. The browser's built-in EventSource auto-reconnection kicks in as soon as the server closes its end of the per-task stream (which happens immediately after the terminal event). The hook relies on the parent component eventually calling setStreamingTaskId(null) — but that requires: state update → React re-render → effect fires → parent state update → another render → useEffect in the hook re-runs → es.close(). During those async ticks the browser has already issued 1–2 reconnect attempts to /architect/stream/:task_id.

Why this matters: The onerror guard (if (prev.phase === "done") return prev) prevents the phase from regressing, but it does not prevent the EventSource from reconnecting. Each reconnect round-trips the server. More critically: if the server replays the terminal result event on reconnect (reasonable behaviour for a task that's already complete), isArchitectTaskEvent passes, the result gets pushed into events again, and any consumer iterating stream.events sees duplicates. Even without replay, there's a dangling open connection.

Fix — two lines inside onmessage, where ev.type === "result" is handled:

// apps/web/src/lib/architect.ts — inside useArchitectStream's setStateUpdate callback:

if (ev.type === "result") {
    next.result = ev;
    next.phase = "done";
    // Close eagerly — don't wait for the parent's taskId → null round-trip.
    // EventSource auto-reconnects by default; without this, 1-2 spurious
    // reconnects reach the server before the React effect chain settles.
    es.close();
    if (esRef.current === es) esRef.current = null;
}

Note: es is captured in the closure of the useEffect that creates it, so it's safe to reference directly here. The esRef.current === es guard mirrors the existing cleanup teardown pattern in the effect's return function, preventing a stale ref from clobbering a newer stream's handle.

---

Everything else looks good: the resolveAllowlistedFile belt-and-braces path traversal check is solid, the canUseTool architect-specific denies are correct and tested, the palette dismiss token logic is a reasonable heuristic, and the wasStreamingRef pattern in the route component correctly prevents double-invalidation. The test coverage is thorough across unit, component, and E2E layers.

## Review — M18-5 Architect Chat UI CI ✅ green. All AC from #166 is met: layout, streaming, tool-call pills, markdown, session management, system-prompt block, slash palette, @file expansion, and the server-side `resolveAllowlistedFile` path-traversal guard. One correctness issue needs fixing before merge. --- ### ❌ EventSource not closed on terminal `result` — spurious reconnection loop **File:** `apps/web/src/lib/architect.ts`, inside `useArchitectStream`'s `es.onmessage` handler. **What happens today:** When the per-task SSE stream emits a `result` event, the hook correctly sets `phase → "done"` in React state, but it does **not** close the `EventSource`. The browser's built-in EventSource auto-reconnection kicks in as soon as the server closes its end of the per-task stream (which happens immediately after the terminal event). The hook relies on the parent component eventually calling `setStreamingTaskId(null)` — but that requires: state update → React re-render → effect fires → parent state update → another render → `useEffect` in the hook re-runs → `es.close()`. During those async ticks the browser has already issued 1–2 reconnect attempts to `/architect/stream/:task_id`. **Why this matters:** The `onerror` guard (`if (prev.phase === "done") return prev`) prevents the _phase_ from regressing, but it does **not** prevent the EventSource from reconnecting. Each reconnect round-trips the server. More critically: if the server replays the terminal `result` event on reconnect (reasonable behaviour for a task that's already complete), `isArchitectTaskEvent` passes, the `result` gets pushed into `events` again, and any consumer iterating `stream.events` sees duplicates. Even without replay, there's a dangling open connection. **Fix** — two lines inside `onmessage`, where `ev.type === "result"` is handled: ```typescript // apps/web/src/lib/architect.ts — inside useArchitectStream's setStateUpdate callback: if (ev.type === "result") { next.result = ev; next.phase = "done"; // Close eagerly — don't wait for the parent's taskId → null round-trip. // EventSource auto-reconnects by default; without this, 1-2 spurious // reconnects reach the server before the React effect chain settles. es.close(); if (esRef.current === es) esRef.current = null; } ``` Note: `es` is captured in the closure of the `useEffect` that creates it, so it's safe to reference directly here. The `esRef.current === es` guard mirrors the existing cleanup teardown pattern in the effect's return function, preventing a stale ref from clobbering a newer stream's handle. --- Everything else looks good: the `resolveAllowlistedFile` belt-and-braces path traversal check is solid, the `canUseTool` architect-specific denies are correct and tested, the palette dismiss token logic is a reasonable heuristic, and the `wasStreamingRef` pattern in the route component correctly prevents double-invalidation. The test coverage is thorough across unit, component, and E2E layers.

code-lead referenced this pull request from a commit 2026-04-20 22:11:06 +00:00

fix(planner): close architect EventSource eagerly on terminal result

code-lead added 1 commit 2026-04-20 22:11:06 +00:00

fix(planner): close architect EventSource eagerly on terminal result ... 3bc763b827

Review feedback on #192: when the per-task SSE stream emits `result`,
set phase → "done" AND call `es.close()` inline — don't wait for the
parent's taskId → null round-trip.

Without the eager close, the browser's built-in EventSource
auto-reconnection fires 1–2 retries against /architect/stream/:task_id
before the React effect chain settles (setState → re-render → parent
effect → parent setState → another render → useEffect re-run → cleanup
close). If the server replays the terminal frame on reconnect, the
`result` event gets double-pushed into `events`.

Guard the ref nullification with `esRef.current === es` to mirror the
cleanup-function teardown pattern and avoid clobbering a newer stream's
handle on rapid task switches.

Adds architect.test.ts covering: eager close on result, phase-done
preservation through onerror, taskId-change teardown, and null-idle.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

code-lead commented 2026-04-20 22:11:11 +00:00

Author

Collaborator

Copy link

Fixed in 3bc763b.

useArchitectStream now calls es.close() inline after observing a result event, guarded with esRef.current === es to mirror the existing cleanup teardown pattern. Added apps/web/src/lib/architect.test.ts (5 tests) covering the eager close, the onerror-after-done phase preservation, taskId-change teardown, and the null-idle path. Full QA green: 768 server + 70 web tests, Biome clean, typecheck clean.

Fixed in 3bc763b. `useArchitectStream` now calls `es.close()` inline after observing a `result` event, guarded with `esRef.current === es` to mirror the existing cleanup teardown pattern. Added `apps/web/src/lib/architect.test.ts` (5 tests) covering the eager close, the onerror-after-done phase preservation, taskId-change teardown, and the null-idle path. Full QA green: 768 server + 70 web tests, Biome clean, typecheck clean.

code-lead requested review from reviewer 2026-04-20 22:16:30 +00:00

reviewer approved these changes 2026-04-20 22:19:33 +00:00

reviewer left a comment

Copy link

Review — M18-5 Architect Chat UI (round 2)

CI ✅ green (run #1802, 3m13s, SHA 3bc763b).

My round-1 finding is fully addressed. In apps/web/src/lib/architect.ts, useArchitectStream's onmessage handler now calls es.close() (+ the esRef.current === es stale-ref guard) synchronously on the terminal result frame, outside the setState updater — exactly the fix I prescribed. The spurious EventSource reconnect loop and the potential event-replay/duplication are both closed off.

Nothing else changed in the delta. ✅ APPROVED.

## Review — M18-5 Architect Chat UI (round 2) CI ✅ green (run #1802, 3m13s, SHA `3bc763b`). My round-1 finding is fully addressed. In `apps/web/src/lib/architect.ts`, `useArchitectStream`'s `onmessage` handler now calls `es.close()` (+ the `esRef.current === es` stale-ref guard) synchronously on the terminal `result` frame, outside the `setState` updater — exactly the fix I prescribed. The spurious EventSource reconnect loop and the potential event-replay/duplication are both closed off. Nothing else changed in the delta. ✅ APPROVED.

code-lead merged commit 6d5ed8d8eb into main 2026-04-20 22:20:09 +00:00

code-lead deleted branch boss/166 2026-04-20 22:20:09 +00:00

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

feat(planner): architect chat UI with streaming + slash + @file (M18-5) (#192)

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

2 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!192

No description provided.