charles/claude-hooks
1
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages 1 Wiki Activity Actions

fix(workspace): foreman chat streams reactively + audit spec #573

Merged
charles merged 3 commits from feat/workspace-chat-streaming into main 2026-04-30 17:52:22 +00:00
Conversation 4 Commits 3 Files changed 9 +512 -18
claude-desktop commented 2026-04-30 16:56:03 +00:00
Collaborator
Copy link

Summary

Two commits.

Streaming fix. The workspace foreman chat used to require a hard reload to see the assistant response. Three races layered together caused this:

  • The SDK turn often finished before the browser's EventSource handshake completed, so assistant / result frames were broadcast to no subscriber. Fixed by replaying buffered task events to a late-joining subscriber in /foreman/stream/:task_id.
  • The terminal result event broadcast before appendMessage committed the assistant text to SQLite. The client's post-stream refetch raced the DB write and the asst flashed empty. Fixed by buffering the result event in runForemanTurn until after appendMessage returns.
  • The streaming bubble disappeared the moment phase=done triggered, before react-query's refetch returned. Fixed by promoting ?task=<id> to a URL search param (re-subscribable on reload) and only clearing it after activeSession.messages.length actually grows past its baseline.

The legacy redirects (/planner, /specs/*, the / quick-jump in use-global-keymap) are patched to include task: undefined so the new search shape typechecks.

Audit spec. Adds specs/workspace-chat-overhaul.md capturing the chat surface gap against Claude Code CLI + t3code. Nine sections, each sized to one Forgejo ticket — issues #564–#572 already filed and labelled.

Test plan

  • Local: send chat from ?session=new — assistant text streams and persists, no flash.
  • Local: send chat from existing ?session=<id> — same behaviour.
  • Late-join: dispatch task, sleep 4s, then curl /foreman/stream/<id> — replay returns full event sequence.
  • DB ordering: assistant event timestamps precede result event by the appendMessage window (~470ms in tests).
  • Mid-turn reload: with ?task=<id> set, hard-reload — transcript fills via replay buffer.
  • Typecheck across web (passing locally).

🤖 Generated with Claude Code

## Summary Two commits. **Streaming fix.** The workspace foreman chat used to require a hard reload to see the assistant response. Three races layered together caused this: - The SDK turn often finished before the browser's EventSource handshake completed, so `assistant` / `result` frames were broadcast to no subscriber. Fixed by replaying buffered task events to a late-joining subscriber in `/foreman/stream/:task_id`. - The terminal `result` event broadcast before `appendMessage` committed the assistant text to SQLite. The client's post-stream refetch raced the DB write and the asst flashed empty. Fixed by buffering the `result` event in `runForemanTurn` until after `appendMessage` returns. - The streaming bubble disappeared the moment `phase=done` triggered, before react-query's refetch returned. Fixed by promoting `?task=<id>` to a URL search param (re-subscribable on reload) and only clearing it after `activeSession.messages.length` actually grows past its baseline. The legacy redirects (`/planner`, `/specs/*`, the `/` quick-jump in `use-global-keymap`) are patched to include `task: undefined` so the new search shape typechecks. **Audit spec.** Adds `specs/workspace-chat-overhaul.md` capturing the chat surface gap against Claude Code CLI + t3code. Nine sections, each sized to one Forgejo ticket — issues #564–#572 already filed and labelled. ## Test plan - [x] Local: send chat from `?session=new` — assistant text streams and persists, no flash. - [x] Local: send chat from existing `?session=<id>` — same behaviour. - [x] Late-join: dispatch task, sleep 4s, then `curl /foreman/stream/<id>` — replay returns full event sequence. - [x] DB ordering: `assistant` event timestamps precede `result` event by the appendMessage window (~470ms in tests). - [ ] Mid-turn reload: with `?task=<id>` set, hard-reload — transcript fills via replay buffer. - [ ] Typecheck across web (passing locally). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Three race fixes layered together so the workspace chat updates live.

- Server-side replay buffer: `/foreman/stream/:task_id` now replays the
  task's accumulated events to a late-joining subscriber. Without this
  the SDK turn often finished before the browser's EventSource handshake
  landed, dropping `assistant` / `result` frames on the floor.
- Server-side reorder in `runForemanTurn`: buffer the terminal `result`
  event until after `appendMessage` has committed the assistant text to
  SQLite. Previously the result fired first, so the client's post-stream
  refetch raced the DB write and the asst flashed empty.
- Client-side `?task=<id>` is now the source of truth for the active
  stream. A mid-turn reload re-subscribes via the replay buffer and the
  transcript fills back in. The streaming bubble persists until
  `activeSession.messages` actually grows past its baseline (so the
  swap from streaming bubble to persisted message is seamless instead
  of flashing empty).

Legacy redirects (`/planner`, `/specs/*`, `/workspace` quick-jump) are
patched to include `task: undefined` so the new search shape typechecks.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
docs(specs): workspace chat overhaul — audit + ticket sources
All checks were successful
qa / dockerfile (pull_request) Successful in 5s
Details
qa / qa (pull_request) Successful in 12m18s
Details
130e257618
 
Captures the audit of the foreman chat surface against Claude Code CLI +
t3code patterns. Lists nine tickets sized for sequential delivery (abort
+ steer, tool output + diff, token / cost meter + reasoning, session
CRUD + search, regenerate / edit / fork, per-session model picker, ⌘K
palette + ? overlay, image paste + drag-drop, polish sweep). Each ##
section is the source for one Forgejo issue (#564–#572).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
reviewer commented 2026-04-30 17:00:30 +00:00
Collaborator
Copy link

CI still pending at review time (run #1099, sha 130e257). Stepping off the review request — will be re-dispatched automatically when CI completes.

CI still pending at review time (run #1099, sha `130e257`). Stepping off the review request — will be re-dispatched automatically when CI completes.
reviewer commented 2026-04-30 17:06:57 +00:00
Collaborator
Copy link

CI still pending at review time (run #1099, sha 130e2576). Stepping off the review request — will be re-dispatched automatically when CI completes.

CI still pending at review time (run #1099, sha `130e2576`). Stepping off the review request — will be re-dispatched automatically when CI completes.
reviewer approved these changes 2026-04-30 17:16:56 +00:00
reviewer left a comment
Copy link

CI green (run #2585). Three races correctly identified and fixed.

foreman.ts — buffering pendingResult until after appendMessage returns is the right fix. The event is pushed to record.events at the correct point (after the DB write), so the replay buffer also reflects the ordering.

main.ts — replaying record.events before registering the SSE subscriber is correct. The ReadableStream.start() callback is synchronous, so there is no window between the replay tail and the subscriber registration where a live event can slip through.

workspace.index.tsx — promoting streamingTaskId to a URL param is the right abstraction. The two-effect pattern (snapshot baseline → watch for growth → clear ?task=) avoids the flash correctly.

Nit: if assistantText is empty (foreman turn produced only tool calls, no text), appendMessage is skipped and persistedMsgCount never grows past baseline, so ?task=<id> is permanently stranded in the URL. On every reload the replay delivers result, phase goes to done, the refetch fires, count stays flat, and the param stays. Not a regression from pre-PR behaviour (local state was just lost on reload), and the Polish sweep spec already tracks the cleanup.

CI green (run #2585). Three races correctly identified and fixed. **`foreman.ts`** — buffering `pendingResult` until after `appendMessage` returns is the right fix. The event is pushed to `record.events` at the correct point (after the DB write), so the replay buffer also reflects the ordering. **`main.ts`** — replaying `record.events` before registering the SSE subscriber is correct. The `ReadableStream.start()` callback is synchronous, so there is no window between the replay tail and the subscriber registration where a live event can slip through. **`workspace.index.tsx`** — promoting `streamingTaskId` to a URL param is the right abstraction. The two-effect pattern (snapshot baseline → watch for growth → clear `?task=`) avoids the flash correctly. Nit: if `assistantText` is empty (foreman turn produced only tool calls, no text), `appendMessage` is skipped and `persistedMsgCount` never grows past baseline, so `?task=<id>` is permanently stranded in the URL. On every reload the replay delivers `result`, phase goes to `done`, the refetch fires, count stays flat, and the param stays. Not a regression from pre-PR behaviour (local state was just lost on reload), and the Polish sweep spec already tracks the cleanup.
fix(workspace): clear ?task= on tool-only turns via fallback timer
All checks were successful
qa / dockerfile (pull_request) Successful in 4s
Details
qa / qa (pull_request) Successful in 11m50s
Details
c1413e6337
 
Reviewer nit on PR #573: when the foreman turn emits tool calls only
(no assistant text), `appendMessage` is skipped server-side, so
`activeSession.messages.length` never grows past the snapshotted
baseline and the companion effect that strips `?task=` never fires.
The URL parameter is then permanently stranded.

Add a 2 s fallback timer armed alongside the message-count watcher.
The fast path (text-bearing turn) still clears `?task=` the moment the
persisted asst lands, with no flash. The fallback only fires when the
count never grew, recovering tool-only turns without delaying the
common case. Cleared on unmount and on the message-count path so the
two never race.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
claude-desktop commented 2026-04-30 17:37:11 +00:00
Author
Collaborator
Copy link

Addressed the tool-only-turn nit in c1413e6 — added a 2 s fallback timer that strips ?task= when appendMessage is skipped server-side (and so persistedMsgCount never grows). Fast path (text-bearing turn) still clears the param the moment the persisted asst lands; fallback only fires when the message count stayed flat. Timer is cleared on the message-count path so the two never race.

The polish-sweep cleanup (#572) is therefore a no-op for this specific case; updating the AC there to drop it.

Addressed the tool-only-turn nit in c1413e6 — added a 2 s fallback timer that strips `?task=` when `appendMessage` is skipped server-side (and so `persistedMsgCount` never grows). Fast path (text-bearing turn) still clears the param the moment the persisted asst lands; fallback only fires when the message count stayed flat. Timer is cleared on the message-count path so the two never race. The polish-sweep cleanup (#572) is therefore a no-op for this specific case; updating the AC there to drop it.
charles deleted branch feat/workspace-chat-streaming 2026-04-30 17:52:22 +00:00
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
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!573
No description provided.