diff --git a/apps/core/src/agent/loop.ts b/apps/core/src/agent/loop.ts index 98a044b6..7fc0d139 100644 --- a/apps/core/src/agent/loop.ts +++ b/apps/core/src/agent/loop.ts @@ -30,7 +30,6 @@ import type { import type { SystemBlock } from "../providers/types.js"; import type { PermissionRequestResult } from "../hooks/PermissionRequest.js"; import { createInitialSessionState, DEFAULT_LOOP_HEURISTICS } from "./types.js"; -import type { StreamEvent } from "@thisisayande/freecode-shared"; import { Effect } from "effect"; import { createToolOrchestrator, getTool } from "../tools/index.js"; import type { ToolOrchestrator } from "../tools/orchestrator.js"; @@ -98,7 +97,6 @@ export class AgentLoop { private orchestrator: ToolOrchestrator; private recovery: RecoveryManager; private sessionStore: SessionStore | undefined; - private onToolEvent: ((event: StreamEvent) => void) | undefined; private lastThinking: string | undefined; private compiler: PromptCompiler; // Cancellation: aborted on interrupt(); threaded into provider requests and @@ -224,7 +222,6 @@ export class AgentLoop { try { this.state = { ...this.state, status: "running" }; - this.onToolEvent = input.onToolEvent; // Initialize compiler with project info and mode this.compiler = new PromptCompiler( @@ -405,7 +402,7 @@ export class AgentLoop { // Emit thinking content if present (for UI to display as streaming reasoning) if (providerResult.thinking) { this.lastThinking = providerResult.thinking; - this.onToolEvent?.({ + BusEvents.stream(this.state.sessionId, { type: "thinking", content: providerResult.thinking, }); @@ -413,7 +410,7 @@ export class AgentLoop { // Emit text content if present (for UI to display) if (providerResult.content) { - this.onToolEvent?.({ + BusEvents.stream(this.state.sessionId, { type: "text", content: providerResult.content, }); @@ -650,7 +647,7 @@ export class AgentLoop { // Prefer streaming when the provider supports it AND we have a listener. // If either is missing, fall back to the one-shot execute() path so callers // and downstream code paths are unchanged. - if (aiProvider.stream && this.onToolEvent) { + if (aiProvider.stream) { let content = ""; let thinking = ""; let toolCalls: @@ -669,11 +666,17 @@ export class AgentLoop { switch (chunk.type) { case "text_delta": content += chunk.delta; - this.onToolEvent({ type: "text_delta", delta: chunk.delta }); + BusEvents.stream(this.state.sessionId, { + type: "text_delta", + delta: chunk.delta, + }); break; case "thinking_delta": thinking += chunk.delta; - this.onToolEvent({ type: "thinking_delta", delta: chunk.delta }); + BusEvents.stream(this.state.sessionId, { + type: "thinking_delta", + delta: chunk.delta, + }); break; case "tool_call": (toolCalls ??= []).push({ @@ -831,12 +834,6 @@ export class AgentLoop { title: `Tool ${toolCall.tool}`, error: `Tool "${toolCall.tool}" is not allowed in plan mode (read-only)`, }; - BusEvents.toolCompleted( - this.state.sessionId, - toolCall.tool, - toolCall.id, - false, - ); return blockedResult; } } @@ -859,13 +856,6 @@ export class AgentLoop { hookContext.toolName ?? toolCall.tool, preResult.blockReason ?? "no reason", ); - // Emit tool.completed for blocked tool - BusEvents.toolCompleted( - this.state.sessionId, - toolCall.tool, - toolCall.id, - false, - ); return blockedResult; } @@ -899,12 +889,6 @@ export class AgentLoop { title: `Tool ${toolCall.tool}`, error: `Permission denied: ${permResult.reason ?? "requires approval"}`, }; - BusEvents.toolCompleted( - this.state.sessionId, - toolCall.tool, - toolCall.id, - false, - ); return blockedResult; } } @@ -921,21 +905,13 @@ export class AgentLoop { } // Emit tool_start event for streaming - this.onToolEvent?.({ + BusEvents.stream(this.state.sessionId, { type: "tool_start", toolCallId: toolCall.id, toolName: toolCall.tool, args: toolCall.args as Record, }); - // Emit tool.called event before execution - BusEvents.toolCalled( - this.state.sessionId, - toolCall.tool, - toolCall.id, - toolCall.args as Record, - ); - // Record function.call event this.recorder.recordFunctionCall( toolCall.tool, @@ -974,13 +950,6 @@ export class AgentLoop { error: String(error), duration_ms: Date.now() - startTime, }; - BusEvents.toolCompleted( - this.state.sessionId, - toolCall.tool, - toolCall.id, - false, - Date.now() - startTime, - ); return errorResult; } @@ -994,7 +963,7 @@ export class AgentLoop { .map((line) => line.length > MAX_LINE_LEN ? line.slice(0, MAX_LINE_LEN) + "..." : line, ); - this.onToolEvent?.({ + BusEvents.stream(this.state.sessionId, { type: "tool_output", toolCallId: toolCall.id, content: outputLines.join("\n"), @@ -1014,19 +983,10 @@ export class AgentLoop { Date.now() - startTime, ); - // Emit tool.completed event with duration + // Emit tool_complete event for streaming const duration_ms = Date.now() - startTime; const success = !result.error; - BusEvents.toolCompleted( - this.state.sessionId, - toolCall.tool, - toolCall.id, - success, - duration_ms, - ); - - // Emit tool_complete event for streaming - this.onToolEvent?.({ + BusEvents.stream(this.state.sessionId, { type: "tool_complete", toolCallId: toolCall.id, toolName: toolCall.tool, diff --git a/apps/core/src/agent/types.ts b/apps/core/src/agent/types.ts index b38ec716..7d11037e 100644 --- a/apps/core/src/agent/types.ts +++ b/apps/core/src/agent/types.ts @@ -267,8 +267,6 @@ export type MessagePart = // User Input / Loop Result - Main entry/exit types // ============================================================================= -import type { StreamEvent } from "@thisisayande/freecode-shared"; - export interface UserInput { prompt: string; sessionId: string; @@ -276,7 +274,6 @@ export interface UserInput { model?: string; projectPath: string; agentMode?: AgentMode; - onToolEvent?: (event: StreamEvent) => void; } export interface LoopResult { diff --git a/apps/core/src/bus/bridge.test.ts b/apps/core/src/bus/bridge.test.ts new file mode 100644 index 00000000..bc10480c --- /dev/null +++ b/apps/core/src/bus/bridge.test.ts @@ -0,0 +1,61 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { busEventToClientEvent } from "./bridge.js"; + +test("question.asked maps to a question_asked stream event", () => { + const out = busEventToClientEvent({ + type: "question.asked", + requestId: "r1", + sessionId: "s1", + questions: [ + { question: "Pick", options: [{ label: "A", description: "a" }] }, + ], + } as any); + assert.equal(out?.type, "question_asked"); + assert.equal((out as any).requestId, "r1"); + assert.equal((out as any).questions.length, 1); +}); + +test("internal cache-invalidation events are dropped (return undefined)", () => { + assert.equal( + busEventToClientEvent({ + type: "tools.changed", + added: [], + removed: [], + } as any), + undefined, + ); + assert.equal( + busEventToClientEvent({ type: "mcp.tools.changed", server: "x" } as any), + undefined, + ); +}); + +test("a forwarded lifecycle event keeps its type and payload", () => { + const out = busEventToClientEvent({ + type: "subagent.started", + subagentId: "a", + subagentType: "explore", + parentId: "p", + task: "t", + } as any); + assert.equal(out?.type, "subagent.started"); +}); + +test("a stream relay event is unwrapped to its inner StreamEvent", () => { + const inner = { type: "text_delta", delta: "hi" } as const; + const out = busEventToClientEvent({ + type: "stream", + sessionId: "s1", + event: inner, + } as any); + assert.deepEqual(out, inner); +}); + +test("redundant bus tool.called/tool.completed are dropped", () => { + assert.equal(busEventToClientEvent({ type: "tool.called" } as any), undefined); + assert.equal( + busEventToClientEvent({ type: "tool.completed" } as any), + undefined, + ); +}); diff --git a/apps/core/src/bus/bridge.ts b/apps/core/src/bus/bridge.ts new file mode 100644 index 00000000..b185ade6 --- /dev/null +++ b/apps/core/src/bus/bridge.ts @@ -0,0 +1,41 @@ +// ============================================================================= +// Bus → Frontend bridge (pure mapping) +// Decides how each internal bus event appears on the frontend wire. +// Returns undefined for internal-only events that must NOT reach frontends. +// ============================================================================= + +import type { BusEvent } from "./index.js"; +import type { StreamEvent } from "@thisisayande/freecode-shared"; + +const INTERNAL_ONLY = new Set([ + "tools.changed", + "mcp.tools.changed", + // Redundant with the stream tool_start/tool_complete events (the loop's + // authoritative tool lifecycle); dropped so tools are never double-emitted. + "tool.called", + "tool.completed", +]); + +export function busEventToClientEvent( + event: BusEvent, +): StreamEvent | undefined { + if (INTERNAL_ONLY.has(event.type)) return undefined; + + // The bus is only a carrier for stream events — unwrap to the wire language. + if (event.type === "stream") return event.event; + + if (event.type === "question.asked") { + return { + type: "question_asked", + requestId: event.requestId, + sessionId: event.sessionId, + questions: event.questions, + }; + } + + // Lifecycle/progress events (session.*, subagent.*, mcp.server.*, tool.*) + // are forwarded verbatim; frontends render what they recognize and ignore + // the rest. Cast: these carry richer payloads than the base StreamEvent + // union, which frontends read structurally. + return event as unknown as StreamEvent; +} diff --git a/apps/core/src/bus/index.ts b/apps/core/src/bus/index.ts index 838a518c..4a5deef2 100644 --- a/apps/core/src/bus/index.ts +++ b/apps/core/src/bus/index.ts @@ -6,6 +6,7 @@ // ============================================================================= import { EventEmitter } from "events"; +import type { StreamEvent } from "@thisisayande/freecode-shared"; // ============================================================================ // Event Definitions @@ -106,7 +107,7 @@ export interface ToolCompletedEvent { export interface QuestionAskedEvent { type: "question.asked"; requestId: string; - sessionId: string; + sessionId?: string; questions: Array<{ question: string; header?: string; @@ -149,11 +150,25 @@ export interface MCPServerErrorEvent { error: string; } +// ============================================================================ +// Stream Relay Event +// Transports a per-session StreamEvent (turn output: text/thinking/tool +// deltas) over the bus so it shares the single frontend egress. The bus is +// only the carrier — StreamEvent remains the wire language. +// ============================================================================ + +export interface StreamRelayEvent { + type: "stream"; + sessionId: string; + event: StreamEvent; +} + // ============================================================================ // Union of all Bus Events // ============================================================================ export type BusEvent = + | StreamRelayEvent | SessionCreatedEvent | SessionUpdatedEvent | SessionErrorEvent @@ -235,6 +250,7 @@ const pendingQuestions = new Map< export async function askQuestion( requestId: string, questions: QuestionAskedEvent["questions"], + sessionId?: string, ): Promise { return new Promise((resolve, reject) => { // Store the pending question @@ -244,11 +260,13 @@ export async function askQuestion( bus.publish({ type: "question.asked", requestId, + sessionId, questions, } as QuestionAskedEvent); - // Timeout after 5 minutes - setTimeout( + // Timeout after 5 minutes. unref() so a pending question never keeps the + // process alive on its own (it also lets tests exit once resolved). + const timer = setTimeout( () => { if (pendingQuestions.has(requestId)) { pendingQuestions.delete(requestId); @@ -257,6 +275,7 @@ export async function askQuestion( }, 5 * 60 * 1000, ); + timer.unref?.(); }); } @@ -287,6 +306,9 @@ export function rejectQuestion(requestId: string): void { // ============================================================================ export const BusEvents = { + stream: (sessionId: string, event: StreamEvent) => + bus.publish({ type: "stream", sessionId, event } as StreamRelayEvent), + sessionCreated: (sessionId: string, projectPath: string) => bus.publish({ type: "session.created", diff --git a/apps/core/src/bus/ordering.test.ts b/apps/core/src/bus/ordering.test.ts new file mode 100644 index 00000000..9c5f661d --- /dev/null +++ b/apps/core/src/bus/ordering.test.ts @@ -0,0 +1,33 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { bus, BusEvents } from "./index.js"; + +test("stream events preserve per-session FIFO order", () => { + const got: string[] = []; + const off = bus.subscribeAll((e) => { + if (e.type === "stream" && e.sessionId === "s1") { + got.push((e.event as any).delta); + } + }); + try { + ["a", "b", "c"].forEach((d) => + BusEvents.stream("s1", { type: "text_delta", delta: d }), + ); + } finally { + off(); + } + assert.deepEqual(got, ["a", "b", "c"]); +}); + +test("a subscriber filtering by sessionId never sees another session's events", () => { + const s1: unknown[] = []; + const off = bus.subscribeAll((e) => { + if (e.type === "stream" && e.sessionId === "s1") s1.push(e.event); + }); + try { + BusEvents.stream("s2", { type: "text_delta", delta: "x" }); + } finally { + off(); + } + assert.equal(s1.length, 0); +}); diff --git a/apps/core/src/server.test.ts b/apps/core/src/server.test.ts new file mode 100644 index 00000000..22b9bc2a --- /dev/null +++ b/apps/core/src/server.test.ts @@ -0,0 +1,31 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { handleRequest } from "./server.js"; +import { askQuestion } from "./bus/index.js"; + +test("question.answer resolves a pending askQuestion with the answers", async () => { + const p = askQuestion("req-1", [ + { question: "Pick", options: [{ label: "A", description: "a" }] }, + ] as any); + const res = await handleRequest({ + jsonrpc: "2.0", + id: 1, + method: "question.answer", + params: { requestId: "req-1", answers: ["A"] }, + }); + assert.equal((res as any).error, undefined); + assert.deepEqual(await p, ["A"]); +}); + +test("question.reject rejects a pending askQuestion", async () => { + const p = askQuestion("req-2", [ + { question: "Pick", options: [{ label: "A", description: "a" }] }, + ] as any); + await handleRequest({ + jsonrpc: "2.0", + id: 2, + method: "question.reject", + params: { requestId: "req-2" }, + }); + await assert.rejects(p); +}); diff --git a/apps/core/src/server.ts b/apps/core/src/server.ts index f41dee46..1c6a5c8d 100644 --- a/apps/core/src/server.ts +++ b/apps/core/src/server.ts @@ -24,7 +24,6 @@ import type { JsonRpcRequest, JsonRpcResponse, SessionConfig, - StreamEvent, } from "@thisisayande/freecode-shared"; import { getMemoryStore, @@ -43,6 +42,8 @@ import { import { getInterruptHandler } from "./session/interrupt.js"; import { generateTitleFromPrompt } from "./agent/title-generator.js"; import { initMcpServers } from "./mcp/index.js"; +import { bus, BusEvents, answerQuestion, rejectQuestion } from "./bus/index.js"; +import { busEventToClientEvent } from "./bus/bridge.js"; import { randomUUID } from "crypto"; import { existsSync } from "fs"; @@ -222,15 +223,6 @@ const methodHandlers: Record< agentMode, }); - // Emit events to stdout immediately for streaming - const emitEvent = (event: StreamEvent) => { - process.stdout.write(JSON.stringify(event) + "\n"); - const cb = sessionEventCallbacks.get(sessionId); - if (cb) { - cb(event); - } - }; - // Get session store for persisting messages const store = await getSessionStore(); @@ -250,15 +242,17 @@ const methodHandlers: Record< model: session.model, projectPath: session.projectPath, agentMode, - onToolEvent: emitEvent, }), ); } finally { activeLoops.delete(sessionId); } - // Emit done event - emitEvent({ type: "done", content: result.message || "Done" }); + // Emit done event through the single bus egress + BusEvents.stream(sessionId, { + type: "done", + content: result.message || "Done", + }); // Extract session title from first response (no extra API call) if (result.success && result.turnCount > 0 && result.content) { @@ -283,6 +277,19 @@ const methodHandlers: Record< } }, + "question.answer": async (params: Record): Promise => { + const { requestId, answers } = params as { + requestId: string; + answers: string[]; + }; + answerQuestion(requestId, answers); + }, + + "question.reject": async (params: Record): Promise => { + const { requestId } = params as { requestId: string }; + rejectQuestion(requestId); + }, + "providers.list": async (): Promise => { const providers = await getProviders(); return providers.map((p) => ({ @@ -536,6 +543,21 @@ export async function startServer() { await initProviders(); await initMcpServers(); + // Speaker wire: forward internal bus events to both frontend transports. + bus.subscribeAll((event) => { + const wire = busEventToClientEvent(event); + if (!wire) return; + const line = JSON.stringify(wire) + "\n"; + process.stdout.write(line); // TUI reads stdout lines + // Web SSE: route to the owning session if known, else broadcast. + const sid = (event as { sessionId?: string }).sessionId; + if (sid) { + sessionEventCallbacks.get(sid)?.(wire); + } else { + for (const cb of sessionEventCallbacks.values()) cb(wire); + } + }); + // Set up Ctrl+C interrupt handler for session resumption const handler = getInterruptHandler(); handler.setupSignalHandler(async (sessionId: string, messageId: string) => { diff --git a/apps/core/src/tools/question.ts b/apps/core/src/tools/question.ts index 1450dd52..706e72a2 100644 --- a/apps/core/src/tools/question.ts +++ b/apps/core/src/tools/question.ts @@ -96,7 +96,7 @@ function formatQuestions(params: QuestionParams): string { async function executeQuestion( params: QuestionParams, - _ctx: ToolContext, + ctx: ToolContext, ): Promise< ToolExecutionResult<{ title: string; @@ -129,7 +129,7 @@ async function executeQuestion( const formatted = formatQuestions({ questions }); try { - const answers = await askQuestion(requestId, questions); + const answers = await askQuestion(requestId, questions, ctx.sessionId); const formattedAnswers = questions .map((q, i) => `"${q.question}"="${answers[i] ?? "Unanswered"}"`) diff --git a/apps/tui/src/components/question-picker.ts b/apps/tui/src/components/question-picker.ts new file mode 100644 index 00000000..a92453be --- /dev/null +++ b/apps/tui/src/components/question-picker.ts @@ -0,0 +1,24 @@ +import { + SelectList, + type SelectItem, + type SelectListTheme, +} from "@earendil-works/pi-tui"; +import type { QuestionSpec } from "@thisisayande/freecode-shared"; + +// Renders one question's options as a SelectList. onSelect returns the chosen +// option label (answers are matched by label — see the question tool output). +export function createQuestionPicker( + question: QuestionSpec, + callbacks: { onSelect: (label: string) => void; onCancel: () => void }, + theme: SelectListTheme, +): SelectList { + const items: SelectItem[] = question.options.map((o) => ({ + label: o.label, + value: o.label, + description: o.description, + })); + const selector = new SelectList(items, Math.min(items.length, 5), theme); + selector.onSelect = async (item: SelectItem) => callbacks.onSelect(item.value); + selector.onCancel = () => callbacks.onCancel(); + return selector; +} diff --git a/apps/tui/src/index.ts b/apps/tui/src/index.ts index 4bb0f21a..c86e0b23 100644 --- a/apps/tui/src/index.ts +++ b/apps/tui/src/index.ts @@ -32,6 +32,8 @@ import { getCurrentModel, setCurrentModel, setApiKey, + answerQuestion, + rejectQuestion, type SessionInfo, type ModelInfo, } from "./ipc/client.js"; @@ -58,6 +60,7 @@ import { createProviderSelector, createModelSelector, } from "./components/model-picker.js"; +import { createQuestionPicker } from "./components/question-picker.js"; import type { StreamEvent } from "@thisisayande/freecode-shared"; registerBuiltInCommands(); @@ -457,6 +460,42 @@ function handleToolEvent(event: StreamEvent): void { tui.requestRender(); break; } + case "question_asked": { + // Render each question as a SelectList in sequence, collecting answers + // indexed by question, then reply once the last one is answered. + const answers: string[] = []; + const askAt = (i: number) => { + const picker = createQuestionPicker( + event.questions[i], + { + onSelect: (label) => { + answers[i] = label; + removeSelector(picker); + if (i + 1 < event.questions.length) { + askAt(i + 1); + } else { + void answerQuestion(event.requestId, answers); + tui.setFocus(editor); + } + tui.requestRender(); + }, + onCancel: () => { + removeSelector(picker); + void rejectQuestion(event.requestId); + tui.setFocus(editor); + tui.requestRender(); + }, + }, + defaultSelectListTheme, + ); + const editorIdx = tui.children.indexOf(editor); + tui.children.splice(editorIdx + 1, 0, picker); + tui.setFocus(picker); + tui.requestRender(); + }; + askAt(0); + break; + } } } diff --git a/apps/tui/src/ipc/client.ts b/apps/tui/src/ipc/client.ts index 3f65323f..9f10f596 100644 --- a/apps/tui/src/ipc/client.ts +++ b/apps/tui/src/ipc/client.ts @@ -253,6 +253,21 @@ export async function sessionSendStreaming( }); } +// ============================================================================= +// Question Reply Methods +// ============================================================================= + +export async function answerQuestion( + requestId: string, + answers: string[], +): Promise { + await sendRequest("question.answer", { requestId, answers }); +} + +export async function rejectQuestion(requestId: string): Promise { + await sendRequest("question.reject", { requestId }); +} + // ============================================================================= // Provider Methods // ============================================================================= diff --git a/docs/superpowers/plans/2026-07-14-bus-frontend-wiring.md b/docs/superpowers/plans/2026-07-14-bus-frontend-wiring.md new file mode 100644 index 00000000..ec52947c --- /dev/null +++ b/docs/superpowers/plans/2026-07-14-bus-frontend-wiring.md @@ -0,0 +1,512 @@ +# Bus → Frontend Wiring + Question Round-Trip — 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:** Turn the in-process event bus (`apps/core/src/bus/index.ts`) into a real backend→frontend channel, and close the interactive-question round-trip so the `question` tool no longer hangs. After this, frontends (TUI + web) *receive* bus events, and the user can *answer* the agent's questions. + +**Analogy (for reviewers):** The bus is the backend's internal intercom. Today workers make announcements but the user's screen isn't wired to the speakers, and there's no reply button. This plan runs the speaker wire out (bridge bus → stdout / SSE) and adds the reply button (`question.answer` / `question.reject` RPC + a TUI picker). + +**Architecture:** One process-wide `bus.subscribeAll(...)` in `startServer()` serializes bus events to the *existing* frontend transports — stdout lines (TUI) and `sessionEventCallbacks` (web SSE). A pure mapping function (`busEventToClientEvent`) decides what each bus event looks like on the wire; unknown/internal events are dropped. The question tool's parked Promise is resolved by two new JSON-RPC methods that call the already-existing `answerQuestion` / `rejectQuestion`. The TUI renders `question_asked` with a `SelectList` (same primitive as the model picker) and replies over IPC. + +**Scope:** Question round-trip end-to-end (backend + protocol + TUI picker) **plus** the generic bus→frontend bridge so `session.*`, `subagent.*`, and `mcp.*` events also reach frontends. Bespoke TUI rendering for those non-question events is **deferred** (they are forwarded; the TUI ignores unknown event types safely). The **web-app** receives all events via SSE but its interactive question UI is **deferred** — the reply RPC methods work for it already. + +**Tech Stack:** TypeScript ESM (imports use `.js` suffix), Node stdlib + existing deps only — no new dependencies. Tests: `node:test` + `node:assert/strict`, colocated `*.test.ts`, run via `pnpm tsx --test ` from `apps/core/`. TUI uses `@earendil-works/pi-tui` `SelectList` (already a dependency). + +## Global Constraints + +- No new npm dependencies. +- All local imports use the `.js` suffix (ESM, matches existing code). +- New files stay under ~150 lines (project convention from CLAUDE.md). +- Core tests use `node:test` + `assert` from `node:assert/strict`, colocated (pattern: `src/context/instructions.test.ts`). +- All core test commands run from `apps/core/`: `pnpm tsx --test ` for one file, `pnpm test` for the whole suite. +- Do **not** change the meaning of existing `StreamEvent` variants — only add new ones. Both frontends must keep working unchanged for non-question events. +- Commit after each task, message style: `feat(core): ...` / `feat(shared): ...` / `feat(tui): ...`. + +## Background (why this exists) + +- `apps/core/src/bus/index.ts` is a singleton `EventEmitter` with a 16-type event catalog and an `askQuestion`/`answerQuestion`/`rejectQuestion` request-reply helper. It is used heavily as a **publisher** (loop, hooks, subagent, mcp, recovery) but has only **one** real consumer: `tools/defs-cache.ts` subscribes to `tools.changed` + `mcp.tools.changed` for cache invalidation. +- **No bridge exists** from the bus to either frontend transport. The real frontend contract is `packages/shared/src/ipc/protocol.ts` (JSON-RPC `METHODS` for request/response + `StreamEvent` streamed to stdout during `session.send`). Tool/text/thinking streaming reaches the TUI via the loop's separate `onToolEvent → stdout` path (`server.ts:226`), **not** the bus. +- Consequently the **question loop is broken end-to-end**: `askQuestion` (`bus/index.ts:235`) publishes `question.asked` and awaits a Promise, but nothing forwards the event to a frontend and **no RPC method calls `answerQuestion`**, so every `question` tool call waits out the 5-minute timeout (`bus/index.ts:251`) and errors. +- Two mechanical gaps found while reading: + 1. `askQuestion(requestId, questions)` publishes `{ type, requestId, questions }` **without `sessionId`** (`bus/index.ts:244-248`), even though `QuestionAskedEvent` declares `sessionId` (`bus/index.ts:107-109`). Web SSE routing (`sessionEventCallbacks` keyed by `sessionId`, `web-server.ts:90`) needs it. The tool has it available as `ctx.sessionId`. + 2. The stdin request loop in `startServer` (`server.ts:562-572`) `await`s each `handleRequest` per data-chunk. The `question.answer` reply arrives in a **separate** stdin chunk (separate `data` event → separate handler invocation), so it is not blocked by the parked `session.send`. **This must be verified manually** (Task 5) — it is the load-bearing assumption of the whole round-trip. +- Reference implementations: opencode and claude-code both model this as an "ask" event on the stream + a reply on a side channel that resolves a parked call; this plan follows that shape with the codebase's existing transports. + +## File Structure + +| File | Status | Responsibility | +| --- | --- | --- | +| `packages/shared/src/ipc/protocol.ts` | Modify | Add `question_asked` to `StreamEvent`; add `question.answer` / `question.reject` to `METHODS` | +| `apps/core/src/bus/index.ts` | Modify | Thread `sessionId` through `askQuestion`; publish it on `question.asked` | +| `apps/core/src/tools/question.ts` | Modify | Pass `ctx.sessionId` into `askQuestion` | +| `apps/core/src/bus/bridge.ts` | Create | Pure `busEventToClientEvent(event)` mapping bus → wire event (or `undefined` to drop) (~60 lines) | +| `apps/core/src/bus/bridge.test.ts` | Create | Mapping behavior: question.asked → `question_asked`; internal events dropped; passthrough shape | +| `apps/core/src/server.ts` | Modify | `bus.subscribeAll` bridge in `startServer` (stdout + `sessionEventCallbacks`); add `question.answer` / `question.reject` handlers | +| `apps/core/src/server.test.ts` | Create | `question.answer` handler resolves a pending `askQuestion`; `question.reject` rejects it | +| `apps/tui/src/ipc/client.ts` | Modify | Add `answerQuestion` / `rejectQuestion` request senders | +| `apps/tui/src/components/question-picker.ts` | Create | `SelectList`-based picker for one question (~70 lines) | +| `apps/tui/src/index.ts` | Modify | Handle `question_asked` in `handleToolEvent`: mount picker, reply over IPC | + +--- + +### Task 1: Protocol — question stream event + reply methods (shared) + +**Files:** +- Modify: `packages/shared/src/ipc/protocol.ts` + +**Interfaces:** +- Produces: a new `StreamEvent` variant `{ type: "question_asked"; requestId: string; sessionId?: string; questions: QuestionSpec[] }` and two new `METHODS` entries. No breaking changes to existing variants. + +- [ ] **Step 1: Add the `question_asked` stream event** + +In `packages/shared/src/ipc/protocol.ts`, add a shared `QuestionSpec` type (mirrors what the `question` tool sends) above `StreamEvent`: + +```ts +export interface QuestionSpec { + question: string; + header?: string; + options: Array<{ label: string; description: string }>; + multiple?: boolean; + custom?: boolean; +} +``` + +Add this variant to the `StreamEvent` union (append, do not reorder): + +```ts + | { + type: "question_asked"; + requestId: string; + sessionId?: string; + questions: QuestionSpec[]; + } +``` + +- [ ] **Step 2: Add the reply methods to `METHODS`** + +```ts + "question.answer": { + params: { requestId: "", answers: [] as string[] }, + result: undefined as void, + }, + "question.reject": { + params: { requestId: "" }, + result: undefined as void, + }, +``` + +- [ ] **Step 3: Verify types compile (no runtime test — type-only change)** + +Run (from repo root): `pnpm --filter @thisisayande/freecode-shared exec tsc --noEmit` +Expected: exit 0. + +- [ ] **Step 4: Commit** *(skip if user mandates no commits)* + +```bash +git add packages/shared/src/ipc/protocol.ts +git commit -m "feat(shared): add question_asked stream event and question.answer/reject methods" +``` + +--- + +### Task 2: Pure bus→wire mapping (`bus/bridge.ts`) + +**Files:** +- Create: `apps/core/src/bus/bridge.ts` +- Test: `apps/core/src/bus/bridge.test.ts` + +**Interfaces:** +- Consumes: `BusEvent` from `./index.js`, `StreamEvent` from shared. +- Produces: `busEventToClientEvent(event: BusEvent): StreamEvent | undefined` — returns the wire event to forward, or `undefined` for internal-only events that must not reach frontends. + +- [ ] **Step 1: Write the failing test** + +Create `apps/core/src/bus/bridge.test.ts`: + +```ts +import test from "node:test"; +import assert from "node:assert/strict"; +import { busEventToClientEvent } from "./bridge.js"; + +test("question.asked maps to a question_asked stream event", () => { + const out = busEventToClientEvent({ + type: "question.asked", + requestId: "r1", + sessionId: "s1", + questions: [{ question: "Pick", options: [{ label: "A", description: "a" }] }], + } as any); + assert.equal(out?.type, "question_asked"); + assert.equal((out as any).requestId, "r1"); + assert.equal((out as any).questions.length, 1); +}); + +test("internal cache-invalidation events are dropped (return undefined)", () => { + assert.equal( + busEventToClientEvent({ type: "tools.changed", added: [], removed: [] } as any), + undefined, + ); + assert.equal( + busEventToClientEvent({ type: "mcp.tools.changed", server: "x" } as any), + undefined, + ); +}); + +test("a forwarded lifecycle event keeps its type and payload", () => { + const out = busEventToClientEvent({ + type: "subagent.started", + subagentId: "a", subagentType: "explore", parentId: "p", task: "t", + } as any); + assert.equal(out?.type, "subagent.started"); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run (from `apps/core/`): `pnpm tsx --test src/bus/bridge.test.ts` +Expected: FAIL — `Cannot find module './bridge.js'`. + +- [ ] **Step 3: Implement `bus/bridge.ts`** + +Map `question.asked` to the typed `question_asked` variant. Drop the two internal cache events (`tools.changed`, `mcp.tools.changed`) — frontends have no use for them and they are not `StreamEvent` shaped. Forward the remaining lifecycle events **as-is** (their `type` string is already frontend-friendly); the TUI's `handleToolEvent` switch has no `default` branch, so unknown types are ignored safely. + +```ts +// ============================================================================= +// Bus → Frontend bridge (pure mapping) +// Decides how each internal bus event appears on the frontend wire. +// Returns undefined for internal-only events that must NOT reach frontends. +// ============================================================================= + +import type { BusEvent } from "./index.js"; +import type { StreamEvent } from "@thisisayande/freecode-shared"; + +const INTERNAL_ONLY = new Set(["tools.changed", "mcp.tools.changed"]); + +export function busEventToClientEvent( + event: BusEvent, +): StreamEvent | undefined { + if (INTERNAL_ONLY.has(event.type)) return undefined; + + if (event.type === "question.asked") { + return { + type: "question_asked", + requestId: event.requestId, + sessionId: event.sessionId, + questions: event.questions, + }; + } + + // Lifecycle/progress events (session.*, subagent.*, mcp.server.*, tool.*) + // are forwarded verbatim; frontends render what they recognize and ignore + // the rest. Cast: these carry richer payloads than the base StreamEvent + // union, which frontends read structurally. + return event as unknown as StreamEvent; +} +``` + +- [ ] **Step 4: Run to verify it passes** + +Run (from `apps/core/`): `pnpm tsx --test src/bus/bridge.test.ts` +Expected: PASS — 3 tests, 0 failures. + +- [ ] **Step 5: Commit** *(skip if user mandates no commits)* + +```bash +git add apps/core/src/bus/bridge.ts apps/core/src/bus/bridge.test.ts +git commit -m "feat(core): add pure bus-to-frontend event mapping" +``` + +--- + +### Task 3: Thread `sessionId` into questions (bus + tool) + +**Files:** +- Modify: `apps/core/src/bus/index.ts` +- Modify: `apps/core/src/tools/question.ts` + +**Interfaces:** +- Changed: `askQuestion(requestId, questions, sessionId?)` — new optional third arg; when present, it is published on the `question.asked` event so web SSE (`sessionEventCallbacks` keyed by sessionId) can route it. TUI is single-process and does not need it. + +- [ ] **Step 1: Add `sessionId` to `askQuestion`** + +In `apps/core/src/bus/index.ts`, update the signature and the published event: + +```ts +export async function askQuestion( + requestId: string, + questions: QuestionAskedEvent["questions"], + sessionId?: string, +): Promise { + return new Promise((resolve, reject) => { + pendingQuestions.set(requestId, { resolve, reject }); + bus.publish({ + type: "question.asked", + requestId, + sessionId, + questions, + } as QuestionAskedEvent); + // ...existing timeout unchanged... + }); +} +``` + +(`QuestionAskedEvent.sessionId` already exists but is currently optional-in-practice; leave the interface as-is or mark `sessionId?` optional to match.) + +- [ ] **Step 2: Pass `ctx.sessionId` from the tool** + +In `apps/core/src/tools/question.ts`, change `executeQuestion(params, _ctx)` to use the context and forward the id: + +```ts +async function executeQuestion(params: QuestionParams, ctx: ToolContext) { + // ... + const answers = await askQuestion(requestId, questions, ctx.sessionId); + // ... +} +``` + +Confirm `ToolContext` exposes `sessionId` (the loop builds `{ cwd, sessionId, abort }` at `loop.ts:947-951`). If the local `ToolContext` type omits it, read it via `(ctx as { sessionId?: string }).sessionId`. + +- [ ] **Step 3: Verify the suite still compiles/passes** + +Run (from `apps/core/`): `pnpm tsx --test src/bus/bridge.test.ts` and `pnpm tsc --noEmit` +Expected: PASS / exit 0 (no dedicated runtime test — behavior is covered end-to-end by Task 4's test + Task 5 manual check). + +- [ ] **Step 4: Commit** *(skip if user mandates no commits)* + +```bash +git add apps/core/src/bus/index.ts apps/core/src/tools/question.ts +git commit -m "feat(core): carry sessionId on question.asked for per-session routing" +``` + +--- + +### Task 4: Server bridge + reply RPC methods + +**Files:** +- Modify: `apps/core/src/server.ts` +- Test: `apps/core/src/server.test.ts` (create) + +**Interfaces:** +- Consumes: `bus`, `answerQuestion`, `rejectQuestion` from `./bus/index.js`; `busEventToClientEvent` from `./bus/bridge.js`. +- Produces: two new `methodHandlers` entries (`question.answer`, `question.reject`) and a startup-time subscription that forwards bus events to stdout + `sessionEventCallbacks`. + +- [ ] **Step 1: Write the failing test** + +Create `apps/core/src/server.test.ts`: + +```ts +import test from "node:test"; +import assert from "node:assert/strict"; +import { handleRequest } from "./server.js"; +import { askQuestion } from "./bus/index.js"; + +test("question.answer resolves a pending askQuestion with the answers", async () => { + const p = askQuestion("req-1", [ + { question: "Pick", options: [{ label: "A", description: "a" }] }, + ] as any); + const res = await handleRequest({ + jsonrpc: "2.0", id: 1, method: "question.answer", + params: { requestId: "req-1", answers: ["A"] }, + }); + assert.equal((res as any).error, undefined); + assert.deepEqual(await p, ["A"]); +}); + +test("question.reject rejects a pending askQuestion", async () => { + const p = askQuestion("req-2", [ + { question: "Pick", options: [{ label: "A", description: "a" }] }, + ] as any); + await handleRequest({ + jsonrpc: "2.0", id: 2, method: "question.reject", + params: { requestId: "req-2" }, + }); + await assert.rejects(p); +}); +``` + +- [ ] **Step 2: Run to verify it fails** + +Run (from `apps/core/`): `pnpm tsx --test src/server.test.ts` +Expected: FAIL — `Method not found: question.answer` (both promises would otherwise hit the 5-min timeout; the reject test's `handleRequest` returns a method-not-found error and `p` never settles). Keep the test's own timeout short if the runner blocks. + +- [ ] **Step 3: Add the reply handlers** + +In `apps/core/src/server.ts`, import at top: + +```ts +import { bus, answerQuestion, rejectQuestion } from "./bus/index.js"; +import { busEventToClientEvent } from "./bus/bridge.js"; +``` + +Add to `methodHandlers`: + +```ts + "question.answer": async (params): Promise => { + const { requestId, answers } = params as { requestId: string; answers: string[] }; + answerQuestion(requestId, answers); + }, + "question.reject": async (params): Promise => { + const { requestId } = params as { requestId: string }; + rejectQuestion(requestId); + }, +``` + +- [ ] **Step 4: Add the bus → frontend bridge in `startServer()`** + +At the top of `startServer()` (after providers/mcp init), subscribe once for the process lifetime: + +```ts + // Speaker wire: forward internal bus events to both frontend transports. + bus.subscribeAll((event) => { + const wire = busEventToClientEvent(event); + if (!wire) return; + const line = JSON.stringify(wire) + "\n"; + process.stdout.write(line); // TUI reads stdout lines + // Web SSE: route to the owning session if known, else broadcast. + const sid = (event as { sessionId?: string }).sessionId; + if (sid) { + sessionEventCallbacks.get(sid)?.(wire); + } else { + for (const cb of sessionEventCallbacks.values()) cb(wire); + } + }); +``` + +(Note: `question_asked` carries `sessionId` after Task 3, so web routing works. Stdout is shared/global, which is correct for the single-TUI process.) + +- [ ] **Step 5: Run the test + full suite** + +Run (from `apps/core/`): `pnpm tsx --test src/server.test.ts` then `pnpm test` +Expected: PASS — new tests green, no regressions in the loop/instructions/compiler suites. (Pre-existing unrelated failures: `mcp/convert-tool.test.ts`, `session/manager.test.ts`, `session/store.test.ts` import a missing `vitest`; `memory/summarizer.test.ts` assertion drift. These are out of scope — do not fix here.) + +- [ ] **Step 6: Commit** *(skip if user mandates no commits)* + +```bash +git add apps/core/src/server.ts apps/core/src/server.test.ts +git commit -m "feat(core): bridge bus events to frontends and add question reply RPC" +``` + +--- + +### Task 5: TUI question picker + reply + +**Files:** +- Modify: `apps/tui/src/ipc/client.ts` +- Create: `apps/tui/src/components/question-picker.ts` +- Modify: `apps/tui/src/index.ts` + +**Interfaces:** +- Produces: `answerQuestion(requestId, answers)` / `rejectQuestion(requestId)` IPC senders; a `createQuestionPicker(question, callbacks, theme)` returning a `SelectList`; a `question_asked` case in `handleToolEvent` that mounts the picker and replies. + +- [ ] **Step 1: Add IPC reply senders** + +In `apps/tui/src/ipc/client.ts` (uses the existing `sendRequest` helper): + +```ts +export async function answerQuestion( + requestId: string, + answers: string[], +): Promise { + await sendRequest("question.answer", { requestId, answers }); +} + +export async function rejectQuestion(requestId: string): Promise { + await sendRequest("question.reject", { requestId }); +} +``` + +- [ ] **Step 2: Create the picker component** + +Create `apps/tui/src/components/question-picker.ts`, modeled on `components/model-picker.ts` (`SelectList` + `SelectItem`). v1 renders **one** question's options; `onSelect` returns the chosen `label`. + +```ts +import { SelectList, type SelectItem, type SelectListTheme } from "@earendil-works/pi-tui"; +import type { QuestionSpec } from "@thisisayande/freecode-shared"; + +export function createQuestionPicker( + question: QuestionSpec, + callbacks: { onSelect: (label: string) => void; onCancel: () => void }, + theme: SelectListTheme, +): SelectList { + const items: SelectItem[] = question.options.map((o) => ({ + label: o.label, + value: o.label, // answers are matched by label (see question tool output) + description: o.description, + })); + const selector = new SelectList(items, Math.min(items.length, 5), theme); + selector.onSelect = async (item: SelectItem) => callbacks.onSelect(item.value); + selector.onCancel = () => callbacks.onCancel(); + return selector; +} +``` + +- [ ] **Step 3: Handle `question_asked` in the TUI** + +In `apps/tui/src/index.ts`, import `createQuestionPicker`, `answerQuestion`, `rejectQuestion`, and add a `case "question_asked":` to `handleToolEvent` (mirrors how `showModelSelector` mounts a `SelectList` at `index.ts:293-296`). For a multi-question payload, render sequentially, accumulating an `answers: string[]` indexed by question; send once the last is answered. On cancel, call `rejectQuestion(requestId)`. + +```ts + case "question_asked": { + const answers: string[] = []; + const askAt = (i: number) => { + const q = event.questions[i]; + const picker = createQuestionPicker( + q, + { + onSelect: (label) => { + answers[i] = label; + removeSelector(picker); + if (i + 1 < event.questions.length) askAt(i + 1); + else { void answerQuestion(event.requestId, answers); tui.setFocus(editor); } + tui.requestRender(); + }, + onCancel: () => { + removeSelector(picker); + void rejectQuestion(event.requestId); + tui.setFocus(editor); + tui.requestRender(); + }, + }, + defaultSelectListTheme, + ); + const editorIdx = tui.children.indexOf(editor); + tui.children.splice(editorIdx + 1, 0, picker); + tui.setFocus(picker); + tui.requestRender(); + }; + askAt(0); + break; + } +``` + +- [ ] **Step 4: Typecheck the TUI** + +Run (from repo root): `pnpm --filter @thisisayande/freecode-tui exec tsc --noEmit` +Expected: exit 0. (Interactive rendering is verified manually in the section below — no unit test for the `SelectList` mount.) + +- [ ] **Step 5: Commit** *(skip if user mandates no commits)* + +```bash +git add apps/tui/src/ipc/client.ts apps/tui/src/components/question-picker.ts apps/tui/src/index.ts +git commit -m "feat(tui): render agent questions and reply over IPC" +``` + +--- + +## Manual verification (after all tasks) + +**This is required — it validates the load-bearing concurrency assumption (Background gap #2).** + +1. Build core: `pnpm --filter @thisisayande/freecode-core build`. +2. Start the TUI (`pnpm --filter @thisisayande/freecode-tui dev`) in a project. +3. Send a prompt that makes the agent ask, e.g. *"Ask me whether to use tabs or spaces before proceeding."* +4. Confirm: a `SelectList` of options appears in the TUI **mid-turn** (not after the turn ends). +5. Choose an option. Confirm the agent **continues** using the answer (the `question` tool returns `"User has answered..."`, not a timeout error). +6. Repeat and press Esc/cancel instead — confirm the tool reports rejection and the agent proceeds without hanging. +7. If the picker never appears or the turn deadlocks: the stdin loop is serializing the reply behind the parked `session.send`. Fix by making `startServer`'s stdin handler dispatch `handleRequest` **without awaiting in the for-loop** (fire-and-forget per line, preserving output ordering by id) — see Notes. + +## Notes / deferred (do NOT implement now) + +- **Concurrency fallback:** if Task 5 manual step 7 deadlocks, change `server.ts:562-572` so each line's `handleRequest(...).then(write)` is not `await`ed inside the `for` loop. This lets a parked `session.send` coexist with an incoming `question.answer`. Only do this if the manual test proves it necessary — it is a behavioral change to request handling. +- **Web-app question UI** — the web frontend receives `question_asked` over SSE and can already POST `question.answer`, but building its React picker is out of scope here. +- **Multi-select questions** (`multiple: true`) and **free-text custom answers** (`custom: true`) — v1 renders single-select option lists only. `SelectList` is single-select; multi-select + text entry are deferred. +- **Bespoke TUI rendering for `subagent.*` / `mcp.server.*` / `session.diff`** — these events now reach the TUI but are ignored (no `default` case). Dedicated panels are a separate phase. +- **Removing the redundant bus tool events** — `tool.called`/`tool.completed` are published on the bus *and* streamed via `onToolEvent`. The bridge forwards the bus copies too; if the TUI double-renders tools, add `tool.called`/`tool.completed` to `INTERNAL_ONLY` in `bridge.ts`. Check during manual verification. diff --git a/docs/superpowers/plans/2026-07-15-context-token-budget.md b/docs/superpowers/plans/2026-07-15-context-token-budget.md new file mode 100644 index 00000000..ac5ea65e --- /dev/null +++ b/docs/superpowers/plans/2026-07-15-context-token-budget.md @@ -0,0 +1,178 @@ +# Context & Token-Budget Cleanup — 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:** Close the unbounded surfaces in `apps/core`'s context assembly so the system prompt and exploration tools can't blow the token budget on a large or flat repo. Bring freecode in line with what claude-code, opencode, and jcode all converge on: a minimal orientation stub + tool-call-driven exploration, with numeric caps everywhere output can be unbounded. + +**Why (the smell):** A comparative read of `/home/ayan-de/Projects/claude-code`, `/home/ayan-de/Projects/githubProjects/opencode`, and `/home/ayan-de/Projects/githubProjects/jcode` shows all three do the same thing: no file tree or file contents in the system prompt, only cwd/git/platform, with every exploration surface (`ls`, `glob`, `read`, `grep`-equivalent) numerically capped and a truncation message telling the model how to page further. freecode mostly does this too (`read.ts`, `bash.ts` already cap output) — but `glob.ts`/`grep.ts`/`tree-cache.ts` have no upper bound at all, unlike every peer's `ls`/`glob`. (`context/collector.ts`'s eager full-file-content reader is a separate, **dead** — never-imported — module; left untouched, see Notes.) + +**CLAUDE.md correction (ignore, per user):** the "Two-Phase Context Collection" principle in this repo's `CLAUDE.md` (§ Key Design Decisions #2 — "LLM first returns which files it needs, then CLI reads those files") describes a flow that was never built and does not match any of the three reference implementations either. The actual live pattern (`tree-cache.ts` + tool-call exploration) is closer to what claude-code/opencode/jcode do. This plan does **not** implement the literal two-phase flow; Task 5 rewrites that CLAUDE.md section to describe what's actually there. + +**Tech Stack:** TypeScript ESM (`.js` import suffix), Node stdlib + existing deps (`fast-glob`). Tests: `node:test` + `node:assert/strict`, colocated `*.test.ts`, run via `pnpm tsx --test ` from `apps/core/`. + +## Global Constraints + +- No new npm dependencies. +- Local imports use the `.js` suffix (ESM). +- New/modified files stay under ~150 lines (CLAUDE.md convention) — split if a task pushes a file over. +- Do not change any tool's wire-visible schema (`parameters`) unless a task says so explicitly — caps are enforced in `execute()`, not by adding required params. +- Core test commands run from `apps/core/`. Commit after each task (`refactor(core): ...` / `feat(core): ...` / `docs: ...`); skip commits if the user mandates. + +## Background (current state — verified) + +- **Dead eager-context path (out of scope, left alone):** `context/collector.ts` (`collectContext()`) + `context/strategies/file-tree.ts` (`FileTreeStrategy`, recursively reads **every file's full contents** up to `maxDepth=3` into a `files: Record` map) + `context/strategies/index.ts` (registry). Verified via `grep -rn "context/collector|context/strategies|FileTreeStrategy" apps/` — **zero imports anywhere outside these three files.** The loop's own `collectContext()` (`agent/loop.ts:1007`) is an unrelated private method that calls `getProjectContext()` from `tree-cache.ts`, not this module. It costs nothing at runtime since it's never called — see Notes for why this plan doesn't delete it. +- **Live tree path, uncapped:** `context/tree-cache.ts:30-35` (`computeProjectContext`) lists the project root's **top-level directory entries only** (no recursion, good) via `fs.readdirSync`, but with **no cap** on entry count — a flat root with hundreds of files dumps them all into the cached tree, which flows into `compiler.ts:compileProjectSummary` and then the cached system-prompt block. +- **`tools/glob.ts:97-117`:** `fg.async(...)` result count has **no cap** — every matching path is joined into the tool result unconditionally. Peers cap this (opencode: 100 matches; jcode `ls`: `MAX_ENTRIES=100`). +- **`tools/grep.ts:165-166`:** `head_limit` is optional and only applied when the model explicitly supplies it — no default `--max-count`, so an unbounded pattern on a big repo returns unbounded matches. +- **Already fine, no change needed:** `tools/read.ts` (`MAX_LINE_LENGTH=2000`, `MAX_BYTES=50KB`, `DEFAULT_LIMIT=2000` lines, pagination hint), `tools/bash.ts` (`MAX_OUTPUT_BYTES=500_000`, truncation with `[output truncated]` marker), `context/instructions.ts` (global + project-root only, no ancestor walk-up, `MAX_INSTRUCTIONS_CHARS=40_000` — already matches jcode's simplest-of-the-three style). +- **No token/char accounting anywhere in `compiler.ts`** — `compileSystemBlocks` returns two opaque `SystemBlock` strings with no visibility into what each section costs. + +## Comparative reference (verified by prior investigation) + +| | claude-code | opencode | jcode | freecode (this plan) | +| --- | --- | --- | --- | --- | +| System prompt file context | cwd/git-bool/platform only | cwd/worktree/git/platform only | cwd/git-branch/±5 changed filenames | shallow 1-level dir listing + git head (kept, now capped) | +| File tree/contents | none — tool calls only | none — tool calls only | none — dedicated `ls` tool | none live — dead eager-reader untouched, unused | +| Explicit caps | git status capped 2000 chars | tool output capped 50KB/2000 lines, spilled to disk | `ls` capped 100 entries, `read` capped 5000 lines | tree capped (Task 1), glob capped (Task 2), grep capped (Task 3) | +| Token accounting | none explicit | none explicit | `ContextInfo` struct, `chars/4` estimate | lightweight char ledger (Task 4), no compaction (deferred) | +| AGENTS.md/CLAUDE.md | walk every ancestor, 40k cap | walk up, first match only | two fixed locations, no walk-up | two fixed locations, 40k cap — **already matches jcode, no change** | + +## File Structure + +| File | Status | Responsibility | +| --- | --- | --- | +| `apps/core/src/context/tree-cache.ts` | Modify | Cap top-level entry count with truncation marker | +| `apps/core/src/context/tree-cache.test.ts` | Modify | Cover the new cap | +| `apps/core/src/tools/glob.ts` | Modify | Cap result count with truncation marker | +| `apps/core/src/tools/glob.test.ts` | Create | Cover the new cap | +| `apps/core/src/tools/grep.ts` | Modify | Default `head_limit` when caller omits it | +| `apps/core/src/tools/grep.test.ts` | Create | Cover the default cap | +| `apps/core/src/context/compiler.ts` | Modify | Add `compileContextLedger()` — per-section char counts + `chars/4` estimate | +| `apps/core/src/context/compiler.test.ts` | Modify | Cover the ledger | +| `/home/ayan-de/Projects/freecode/CLAUDE.md` | Modify | Rewrite "Two-Phase Context Collection" to describe the real lazy/capped flow | + +--- + +### Task 1: Cap the project-tree listing + +**Files:** Modify `context/tree-cache.ts`; modify `context/tree-cache.test.ts`. + +**Interfaces:** `computeProjectContext` truncates the top-level listing past `MAX_ENTRIES`; `ProjectContext.tree` gains a trailing `... truncated at N entries` line when capped. No signature change. + +- [ ] **Step 1: Failing test** — add to `tree-cache.test.ts`: + ```ts + test("caps the tree listing at MAX_ENTRIES with a truncation marker", () => { + const dir = mkdtempSync(join(tmpdir(), "freecode-tree-cap-")); + for (let i = 0; i < 150; i++) writeFileSync(join(dir, `f${i}.txt`), "x"); + const ctx = getProjectContext(dir); + const lines = ctx.tree.split("\n").filter((l) => l.length > 0); + assert.ok(lines.length <= 101, "capped list plus one marker line"); + assert.match(ctx.tree, /truncated/); + }); + ``` + Run: `pnpm tsx --test src/context/tree-cache.test.ts` → FAIL. +- [ ] **Step 2: Implement** — in `tree-cache.ts`, add `const MAX_ENTRIES = 100;` and in `computeProjectContext`, slice `entries` to `MAX_ENTRIES` before mapping, appending `\n... truncated at ${MAX_ENTRIES} entries (${entries.length} total)` when `entries.length > MAX_ENTRIES`. +- [ ] **Step 3: Pass** — same command → PASS. +- [ ] **Step 4: Commit** *(skip if mandated)* — `refactor(core): cap project-tree listing at 100 entries` + +--- + +### Task 2: Cap glob results + +**Files:** Modify `tools/glob.ts`; create `tools/glob.test.ts`. + +**Interfaces:** `executeGlob` truncates `entries` to `MAX_RESULTS` before formatting; `metadata.count` reports the **total** match count (not the truncated display count) so the model knows to narrow its pattern; output gains a truncation line when capped. + +- [ ] **Step 1: Failing test** — create `tools/glob.test.ts`: + ```ts + import test from "node:test"; + import assert from "node:assert/strict"; + import { mkdtempSync, writeFileSync } from "fs"; + import { tmpdir } from "os"; + import { join } from "path"; + import { GlobTool } from "./glob.js"; + + test("caps results and reports the true total in metadata", async () => { + const dir = mkdtempSync(join(tmpdir(), "freecode-glob-cap-")); + for (let i = 0; i < 150; i++) writeFileSync(join(dir, `f${i}.ts`), ""); + const res = await GlobTool.execute( + { pattern: "*.ts", cwd: dir }, + { cwd: dir } as any, + ); + assert.ok(res.success); + const lines = res.result!.output.split("\n"); + assert.ok(lines.length <= 101); + assert.equal(res.result!.metadata?.count, 150); + assert.match(res.result!.output, /truncated/); + }); + ``` + Run: `pnpm tsx --test src/tools/glob.test.ts` → FAIL. +- [ ] **Step 2: Implement** — in `glob.ts`, add `const MAX_RESULTS = 100;` after the existing constants. In `executeGlob`, keep `metadata: { count: entries.length }` (true total) but build `formatted` from `entries.slice(0, MAX_RESULTS)`, appending `\n... truncated at ${MAX_RESULTS} matches (${entries.length} total, narrow the pattern)` when `entries.length > MAX_RESULTS`. +- [ ] **Step 3: Pass** — same command → PASS. +- [ ] **Step 4: Commit** *(skip if mandated)* — `refactor(core): cap glob tool results at 100 matches` + +--- + +### Task 3: Default-cap grep matches + +**Files:** Modify `tools/grep.ts`; create `tools/grep.test.ts`. + +**Interfaces:** `head_limit` defaults to `DEFAULT_HEAD_LIMIT` when the caller omits it (the model can still override with a larger explicit value — do not clamp an explicit `head_limit`, only supply the default). + +- [ ] **Step 1: Failing test** — create `tools/grep.test.ts` mirroring the existing grep tool's execute signature (read `tools/grep.ts` execute/params shape first to match exactly — do not guess field names). Assert that calling without `head_limit` on a fixture directory with >100 matching lines produces a capped result, and that passing an explicit `head_limit: 500` is respected as-is (not overridden by the default). +- [ ] **Step 2: Implement** — in `grep.ts`, add `const DEFAULT_HEAD_LIMIT = 100;`. Change the `--max-count` branch (currently `if (params.head_limit)`) to always push `--max-count=${params.head_limit ?? DEFAULT_HEAD_LIMIT}`. +- [ ] **Step 3: Pass** — `pnpm tsx --test src/tools/grep.test.ts` → PASS. +- [ ] **Step 4: Commit** *(skip if mandated)* — `refactor(core): default grep head_limit to 100 matches` + +--- + +### Task 4: Lightweight context ledger (visibility, not compaction) + +**Files:** Modify `context/compiler.ts`; modify `context/compiler.test.ts`. + +**Interfaces:** New `PromptCompiler.compileContextLedger(blocks: SystemBlock[]): { section: string; chars: number; estimatedTokens: number }[]` — pure function over the already-built `SystemBlock[]` (no new data collection, no new caching). `estimatedTokens = Math.ceil(chars / 4)`, matching jcode's `chars/4` heuristic. Not wired into any runtime call site by this task — it's a diagnostic entry point for a future `/context`-style command or log line, deliberately not auto-invoked (YAGNI: no consumer needed yet to be useful for debugging via a manual call). + +- [ ] **Step 1: Failing test** — add to `compiler.test.ts`: + ```ts + test("compileContextLedger reports char/token estimates per block", () => { + const compiler = new PromptCompiler("/tmp/proj", "proj"); + const blocks = [ + { text: "a".repeat(400), cache: true }, + { text: "b".repeat(40), cache: false }, + ]; + const ledger = compiler.compileContextLedger(blocks); + assert.equal(ledger.length, 2); + assert.equal(ledger[0].chars, 400); + assert.equal(ledger[0].estimatedTokens, 100); + assert.equal(ledger[1].section, "dynamic"); + }); + ``` + Run: `pnpm tsx --test src/context/compiler.test.ts` → FAIL. +- [ ] **Step 2: Implement** — in `compiler.ts`, add `compileContextLedger` as an instance method: map each block to `{ section: block.cache ? "static" : "dynamic", chars: block.text.length, estimatedTokens: Math.ceil(block.text.length / 4) }`. +- [ ] **Step 3: Pass** — `pnpm tsx --test src/context/compiler.test.ts` → PASS. +- [ ] **Step 4: Commit** *(skip if mandated)* — `feat(core): add context ledger for prompt token visibility` + +--- + +### Task 5 (docs-only): Correct CLAUDE.md's Two-Phase Context Collection claim + +**Files:** Modify `/home/ayan-de/Projects/freecode/CLAUDE.md`. + +- [ ] **Step 1:** Locate `### 2. Two-Phase Context Collection` under `## Key Design Decisions`. Replace its description with what's actually implemented after Task 1–4: a capped, one-level directory listing + git head is cached (`context/tree-cache.ts`, event-invalidated on mutating tools) and injected into the static/dynamic system-prompt split (`context/compiler.ts`); no eager file-content reading happens anywhere — the model reads files itself via the `read`/`glob`/`grep` tools, each capped (`read.ts`, `glob.ts`, `grep.ts`, `bash.ts`). Keep the section numbered/titled the same so cross-references don't break; only the body changes. +- [ ] **Step 2: Commit** *(skip if mandated)* — `docs: correct two-phase context collection description to match implementation` + +--- + +## Manual verification (after all tasks) + +1. `pnpm --filter @thisisayande/freecode-core exec tsc --noEmit` — clean build. +2. `cd apps/core && pnpm test` — expect green except the pre-existing unrelated failures already known from the last plan (`mcp/convert-tool.test.ts`, `session/manager.test.ts`, `session/store.test.ts` missing `vitest`; `memory/summarizer.test.ts` drift). +3. In a scratch dir with 200+ top-level files, start the TUI, send a prompt that triggers a broad `glob "**/*"` and a `grep` with no `head_limit` — confirm both tool outputs show the truncation marker and stay well under a few KB, and the system prompt's tree section is capped too (spot-check via a manual `compileContextLedger` call in a REPL/test, since it isn't wired to a live UI yet). + +## Notes / deferred (do NOT implement unless stated) + +- **`context/collector.ts` + `context/strategies/` (dead eager file-content reader) — intentionally left in place.** Verified zero imports anywhere; it costs nothing at runtime. Per project convention (don't remove pre-existing dead code unless asked) and user confirmation, this plan does not touch it. If it's ever a source of confusion (e.g. someone wires it up not realizing the cost), delete it then — not preemptively here. +- **jcode's EWMA-based proactive compaction** (projecting token growth over a lookahead window and firing compaction before overflow) is a meaningfully bigger system than this plan's scope — `compileContextLedger` (Task 5) gives the *measurement* primitive a future compaction plan would consume; do not build the compaction trigger itself here. +- **opencode's spill-to-disk-on-overflow** (writing truncated tool output to a temp file the model can grep/read-with-offset) is a nice ergonomic upgrade over freecode's current "just truncate and say so" but is a separate, generic `tools/` cross-cutting change (would touch `read.ts`, `bash.ts`, `glob.ts`, `grep.ts` uniformly) — out of scope; the truncation markers added here (Tasks 2–4) are the minimal fix for the actual token-cost problem. +- **AGENTS.md ancestor walk-up** (claude-code's every-ancestor-directory walk) is explicitly **not** being added — `instructions.ts`'s current two-fixed-locations approach already matches jcode's simplest-of-three pattern and there's no evidence freecode's monorepo users need per-subdirectory instruction files yet (YAGNI). +- **Wiring `compileContextLedger` into a live `/context`-style command or log line** is deferred — this plan only adds the measurement function; a consumer (TUI command, debug log) is a separate, smaller follow-up once it's clear what output format is useful. diff --git a/docs/superpowers/plans/2026-07-15-unify-event-egress.md b/docs/superpowers/plans/2026-07-15-unify-event-egress.md new file mode 100644 index 00000000..fa728a97 --- /dev/null +++ b/docs/superpowers/plans/2026-07-15-unify-event-egress.md @@ -0,0 +1,211 @@ +# Unify Core's Event Egress onto the Bus — 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:** Make the bus the **single source of truth** for everything `apps/core` emits to frontends, with **one egress**: `bus.subscribeAll → busEventToClientEvent → stdout + SSE` (the bridge already built in `2026-07-14-bus-frontend-wiring.md`). Eliminate the parallel `onToolEvent` callback path and the tool-event double-emit, so every outbound event has exactly one representation and one exit. + +**Why (the smell):** Core has two undesigned egress paths — a per-session `onToolEvent` callback (threaded by hand through `run()` → `executeTurn` → `executeTool`) and the process-global bus. The loop emits each tool's lifecycle on **both** (`onToolEvent` `tool_start`/`tool_complete` *and* `BusEvents.toolCalled`/`toolCompleted`). The bus copies have no subscribers and, since the bridge landed, are written to stdout and ignored. Two mechanisms, overlapping by accident, no single owner per event. + +**North star vs. interim:** The end state (this plan) is **Option A** — one channel. A deliberately smaller alternative, **Option B** (keep two channels but delete the overlap), is described in Notes as a fallback if the streaming migration proves too risky to land now. Prefer A; bail to B only if Task 4's manual streaming check regresses. + +**The core idea that makes A safe:** keep `StreamEvent` as the *wire language*, and let the bus merely *transport* it. Add one bus event `{ type: "stream"; sessionId: string; event: StreamEvent }`. The loop publishes `stream` events instead of calling `onToolEvent`; the bridge unwraps them back to the inner `StreamEvent`. Nothing about the frontend wire format changes — only *how the event travels inside core*. + +**Tech Stack:** TypeScript ESM (`.js` import suffix), Node stdlib + existing deps. Tests: `node:test` + `node:assert/strict`, colocated `*.test.ts`, run via `pnpm tsx --test ` from `apps/core/`. + +## Global Constraints + +- No new npm dependencies. +- Local imports use the `.js` suffix (ESM). +- New files stay under ~150 lines (CLAUDE.md convention). +- **Do not change the `StreamEvent` wire format** — frontends must keep parsing identical lines. This plan changes *internal transport only*. +- The streaming path is the hot path: every task that touches it must be guarded by a test asserting **per-session ordering** and **no double-delivery** before it lands. +- Core test commands run from `apps/core/`. Commit after each task (`feat(core): ...` / `refactor(core): ...`); skip commits if the user mandates. + +## Background (current egress map — verified) + +- **`onToolEvent` (per-session stream), emitted in `agent/loop.ts`:** `text_delta` (`:672`), `thinking_delta` (`:676`), plus `tool_start` (`:924`), `tool_output` (`:997`), `tool_complete` (`:1029`), `thinking` (`:408`), `text` (`:415`). Threaded as `input.onToolEvent` → `this.onToolEvent`; the only producer of the callback is `server.ts:226-255` (`emitEvent`, which writes `process.stdout` + fans out to `sessionEventCallbacks`). +- **bus (global), `BusEvents.*` call sites:** `agent/loop.ts` — `sessionCreated`, `sessionError`, `sessionUpdated`×2, `toolCalled`, `toolCompleted`×5; `agent/recovery/manager.ts` — `sessionError`; `agent/subagent.ts` + `tools/agent.ts` — `subagentStarted`, `subagentCompleted`; `mcp/init.ts` — `mcpServer*`, `mcpToolsChanged`. +- **Consumers of the bus:** only `tools/defs-cache.ts` (`tools.changed`, `mcp.tools.changed`) + the bridge (`server.ts`, `subscribeAll`). `tool.called`/`tool.completed` have **no subscriber** — pure redundancy with the stream `tool_*` events. +- **Gate quirk:** `loop.ts:653` chooses streaming vs one-shot via `if (aiProvider.stream && this.onToolEvent)`. Removing `onToolEvent` changes this gate — see Task 3 (decision: stream whenever the provider supports it). +- **Known adjacent risk (from the prior plan):** the stdin request loop `await`s each `handleRequest`; unrelated here but do not regress it. + +## Target architecture + +``` + loop / hooks / subagent / mcp ← publishers (only the bus, never a callback) + │ bus.publish(...) (every session-scoped event carries sessionId) + ▼ + ┌───────────┐ + │ BUS │ single source of truth + └─────┬─────┘ + │ ONE subscribeAll in startServer() + ▼ + busEventToClientEvent(event) ← map to StreamEvent | drop (internal-only) + │ + ┌─────┴─────┐ + ▼ ▼ + process.stdout sessionEventCallbacks (SSE, routed by sessionId; global events broadcast) + (TUI) (web) +``` + +No `onToolEvent`. No `emitEvent`. No event emitted twice. + +## File Structure + +| File | Status | Responsibility | +| --- | --- | --- | +| `apps/core/src/bus/index.ts` | Modify | Add `StreamRelayEvent` (`{ type:"stream"; sessionId; event: StreamEvent }`) to the union; add `BusEvents.stream(sessionId, event)` helper | +| `apps/core/src/bus/bridge.ts` | Modify | Unwrap `stream` events to the inner `StreamEvent`; drop redundant `tool.called`/`tool.completed` | +| `apps/core/src/bus/bridge.test.ts` | Modify | Cover `stream` unwrap + tool-event drop | +| `apps/core/src/agent/loop.ts` | Modify | Replace `this.onToolEvent?.(e)` with `BusEvents.stream(sessionId, e)`; delete `onToolEvent` field/param; drop redundant `BusEvents.toolCalled/toolCompleted`; fix the streaming gate | +| `apps/core/src/agent/types.ts` | Modify | Remove `onToolEvent` from `UserInput` | +| `apps/core/src/server.ts` | Modify | Delete `emitEvent`/`onToolEvent` wiring from `session.send`; rely solely on the `subscribeAll` bridge | +| `apps/core/src/bus/ordering.test.ts` | Create | Per-session FIFO ordering + cross-session isolation of `stream` events through the bridge | +| `apps/core/src/agent/loop-caching.test.ts` (+ siblings) | Modify | Update any test that passes `onToolEvent` to assert via the bus instead | + +--- + +### Task 1: Add `stream` transport event to the bus vocabulary + +**Files:** Modify `apps/core/src/bus/index.ts`; Modify `apps/core/src/bus/bridge.ts` + `bridge.test.ts`. + +**Interfaces:** +- Produces: `StreamRelayEvent = { type: "stream"; sessionId: string; event: StreamEvent }` in the `BusEvent` union; `BusEvents.stream(sessionId, event)` publisher. `busEventToClientEvent` unwraps `stream` → `event.event`. + +- [ ] **Step 1: Failing test** — extend `bridge.test.ts`: + +```ts +test("a stream relay event is unwrapped to its inner StreamEvent", () => { + const inner = { type: "text_delta", delta: "hi" } as const; + const out = busEventToClientEvent({ + type: "stream", sessionId: "s1", event: inner, + } as any); + assert.deepEqual(out, inner); +}); + +test("redundant bus tool.called/tool.completed are dropped", () => { + assert.equal(busEventToClientEvent({ type: "tool.called" } as any), undefined); + assert.equal(busEventToClientEvent({ type: "tool.completed" } as any), undefined); +}); +``` + +Run: `pnpm tsx --test src/bus/bridge.test.ts` → FAIL. + +- [ ] **Step 2: Implement** — in `bus/index.ts` add the interface, add it to the `BusEvent` union, and add: + +```ts + stream: (sessionId: string, event: StreamEvent) => + bus.publish({ type: "stream", sessionId, event } as StreamRelayEvent), +``` + +(Import `StreamEvent` from `@thisisayande/freecode-shared`.) In `bridge.ts`: add `"tool.called"`, `"tool.completed"` to `INTERNAL_ONLY`; handle `stream` before the passthrough: + +```ts + if (event.type === "stream") return event.event; +``` + +- [ ] **Step 3: Pass** — `pnpm tsx --test src/bus/bridge.test.ts` → PASS. + +- [ ] **Step 4: Commit** *(skip if user mandates no commits)* — `feat(core): add stream relay event to the bus vocabulary` + +--- + +### Task 2: Ordering + isolation guard (write the safety net BEFORE moving the loop) + +**Files:** Create `apps/core/src/bus/ordering.test.ts`. + +**Interfaces:** consumes `bus`, `BusEvents.stream`, `busEventToClientEvent`. Proves the invariants the loop migration must not break. + +- [ ] **Step 1: Write the guard test** + +```ts +import test from "node:test"; +import assert from "node:assert/strict"; +import { bus, BusEvents } from "./index.js"; + +test("stream events preserve per-session FIFO order", () => { + const got: string[] = []; + const off = bus.subscribeAll((e) => { + if (e.type === "stream" && e.sessionId === "s1") got.push((e.event as any).delta); + }); + ["a", "b", "c"].forEach((d) => + BusEvents.stream("s1", { type: "text_delta", delta: d })); + off(); + assert.deepEqual(got, ["a", "b", "c"]); +}); + +test("a subscriber filtering by sessionId never sees another session's events", () => { + const s1: unknown[] = []; + const off = bus.subscribeAll((e) => { + if (e.type === "stream" && e.sessionId === "s1") s1.push(e.event); + }); + BusEvents.stream("s2", { type: "text_delta", delta: "x" }); + off(); + assert.equal(s1.length, 0); +}); +``` + +- [ ] **Step 2: Run** — `pnpm tsx --test src/bus/ordering.test.ts` → PASS (Task 1 already provides `stream`). This is a **characterization** test: it locks the behavior the next task relies on. + +- [ ] **Step 3: Commit** *(skip if mandated)* — `test(core): guard per-session stream ordering and isolation` + +--- + +### Task 3: Move the loop from `onToolEvent` to `BusEvents.stream` + +**Files:** Modify `agent/loop.ts`, `agent/types.ts`, and any loop test passing `onToolEvent`. + +**Interfaces:** +- Removed: `UserInput.onToolEvent`; `AgentLoop.onToolEvent` field; `input.onToolEvent` usage. +- Changed: every `this.onToolEvent?.(e)` becomes `BusEvents.stream(this.state.sessionId, e)`. Remove `BusEvents.toolCalled`/`toolCompleted` (now redundant with the stream `tool_*` events). Streaming gate becomes `if (aiProvider.stream)` (stream whenever supported — the bus listener always exists). + +- [ ] **Step 1: Update loop tests first (red)** — in `agent/loop-caching.test.ts` (and siblings that pass `onToolEvent`), replace the `onToolEvent` collector with a `bus.subscribeAll` collector filtered to the test session; assert the same events arrive in the same order. Run → FAIL (loop still calls `onToolEvent`). + +- [ ] **Step 2: Migrate `loop.ts`** + - Delete the `onToolEvent` field and `this.onToolEvent = input.onToolEvent`. + - Replace each emission site (`:408`, `:415`, `:672`, `:676`, `:924`, `:997`, `:1029`) with `BusEvents.stream(this.state.sessionId, { ... })`. + - Delete `BusEvents.toolCalled(...)` (`:932`) and the `BusEvents.toolCompleted(...)` calls that duplicate a `tool_complete` stream event (audit each of the 5 — keep none; the stream event is authoritative). + - Change the streaming gate at `:653` to `if (aiProvider.stream)`. Document: one-shot `execute()` remains the fallback only when the provider has no `stream` method. + - Remove `onToolEvent` from `UserInput` in `agent/types.ts`. + +- [ ] **Step 3: Pass** — `pnpm tsx --test src/agent/*.test.ts src/bus/*.test.ts` → PASS. + +- [ ] **Step 4: Commit** *(skip if mandated)* — `refactor(core): publish stream events to the bus, drop onToolEvent` + +--- + +### Task 4: Collapse the server onto the single bridge + +**Files:** Modify `apps/core/src/server.ts`. + +**Interfaces:** `session.send` no longer builds `emitEvent` or passes `onToolEvent`. The already-present `bus.subscribeAll` bridge is the sole egress. The `session.send` JSON-RPC **response** (final `LoopResult`) is unchanged. + +- [ ] **Step 1: Remove the redundant emitter** + - Delete the `emitEvent` closure and `onToolEvent: emitEvent` from the `session.send` handler. + - Keep emitting the terminal `{ type: "done" }` — but publish it via `BusEvents.stream(sessionId, { type: "done", content })` so it flows through the one egress too. + - `sessionEventCallbacks` is now written **only** by the bridge (it already routes by `sessionId`); confirm web SSE still receives events. + +- [ ] **Step 2: Full suite** — from `apps/core/`: `pnpm test`. Expected: green except the pre-existing unrelated failures (`mcp/convert-tool.test.ts`, `session/manager.test.ts`, `session/store.test.ts` → missing `vitest`; `memory/summarizer.test.ts` drift). Do not fix those here. + +- [ ] **Step 3: MANUAL streaming verification (required — hot path)** + 1. `pnpm --filter @thisisayande/freecode-core build`; start the TUI in a project. + 2. Send a prompt that produces a long streamed answer + a couple of tool calls. + 3. Confirm: text streams token-by-token in order, tool rows appear once (not doubled), thinking renders, and the turn ends cleanly. + 4. Trigger the `question` tool; confirm the picker still works end-to-end (regression of the prior plan). + 5. If tokens arrive out of order or duplicated → **stop**; the bug is in Task 3's emission order or a stray second writer. Do not ship. + +- [ ] **Step 4: Commit** *(skip if mandated)* — `refactor(core): make bus.subscribeAll the sole frontend egress` + +--- + +## Manual verification (after all tasks) + +Run Task 4 Step 3 in full. Additionally: open the web app (`/events` SSE) for a session and confirm it receives the same stream, and that a second session's events do not leak into the first (cross-session isolation — the invariant from Task 2, now exercised end-to-end). + +## Notes / deferred (do NOT implement unless stated) + +- **Option B (fallback, smaller):** if Task 4's manual streaming check regresses and can't be quickly fixed, abandon the loop migration and instead land only: (1) delete the redundant `BusEvents.toolCalled/toolCompleted` from the loop, (2) add them to `INTERNAL_ONLY`. This removes the double-emit and clarifies ownership (stream = turn output via `onToolEvent`; bus = notifications) **without** touching the hot path. It is strictly less unified than A but zero-risk. Ship B, keep A for later. +- **Ordering across concurrent sessions:** `EventEmitter` is synchronous and FIFO per emitter, so per-session order is preserved even when two sessions interleave (each frontend filters by `sessionId`). No queue needed. Do not add async buffering — it would *introduce* reordering. +- **Global singleton in tests:** the bus is process-global; tests that drive a loop must scope their `subscribeAll` collector by `sessionId` and call the returned unsubscribe in a `finally`/after each, or events leak between tests (this is why Task 2 exists first). +- **Observability sink:** once everything is on the bus, a future `subscribeAll` logger/metrics/rollout sink is trivial to add at the single egress — but that is a separate plan; do not build speculative sinks now (YAGNI). +- **`compileHistorySection` / `buildContinuationPrompt` dead code** in the loop is out of scope; do not touch here. diff --git a/packages/shared/src/ipc/protocol.ts b/packages/shared/src/ipc/protocol.ts index 2061e973..a5ec1e11 100644 --- a/packages/shared/src/ipc/protocol.ts +++ b/packages/shared/src/ipc/protocol.ts @@ -57,6 +57,14 @@ export type StreamResponse = toolResult?: undefined; }; +export interface QuestionSpec { + question: string; + header?: string; + options: Array<{ label: string; description: string }>; + multiple?: boolean; + custom?: boolean; +} + export type StreamEvent = | { type: "tool_start"; @@ -78,7 +86,13 @@ export type StreamEvent = | { type: "text_delta"; delta: string } // incremental assistant text chunk (streaming path) | { type: "thinking_delta"; delta: string } // incremental reasoning chunk (streaming path) | { type: "done"; content: string } - | { type: "error"; content: string }; + | { type: "error"; content: string } + | { + type: "question_asked"; + requestId: string; + sessionId?: string; + questions: QuestionSpec[]; + }; // ============================================================================= // IPC Method Signatures @@ -109,6 +123,14 @@ export const METHODS = { params: undefined, result: [] as import("../types.js").ProviderInfo[], }, + "question.answer": { + params: { requestId: "", answers: [] as string[] }, + result: undefined as void, + }, + "question.reject": { + params: { requestId: "" }, + result: undefined as void, + }, } as const; export type MethodName = keyof typeof METHODS;