From e77ab98361de4c7c09bee1cc8d563123829cc805 Mon Sep 17 00:00:00 2001 From: Federico Jaramillo Martinez Date: Wed, 17 Jun 2026 00:02:04 +0200 Subject: [PATCH] fix(sessions): stop duplicating user message for slash commands A slash command sent as the first (or any idle) message showed the raw command text twice until reload: once from the client's optimistic insert and once from the server's message.append echo, neither of which converges with the agent's canonical expanded message (e.g. a /skill:* block). Make commands obey the same source-of-truth contract as prompts: - client no longer inserts the raw command text optimistically; it shows the existing per-session sending indicator instead - forwarded runtime/skill commands return a bare done result rather than a synthetic "Accepted ..." line - server suppresses the raw message.append echo for command-forwarded prompts (threaded through the compaction queue too) Result: pre-reload state matches reload, with no transient duplicate. --- .../src/controllers/sessionController.test.ts | 30 +++++++++++++++++++ .../src/controllers/sessionController.ts | 11 ++++++- src/server/sessions/piSessionService.test.ts | 26 ++++++++++++++++ src/server/sessions/piSessionService.ts | 26 +++++++++------- .../sessions/sessionCommandService.test.ts | 4 ++- src/server/sessions/sessionCommandService.ts | 6 +++- 6 files changed, 90 insertions(+), 13 deletions(-) diff --git a/src/client/src/controllers/sessionController.test.ts b/src/client/src/controllers/sessionController.test.ts index 6a1cc3e..1c9609c 100644 --- a/src/client/src/controllers/sessionController.test.ts +++ b/src/client/src/controllers/sessionController.test.ts @@ -308,6 +308,36 @@ describe("SessionController", () => { expect(state.sendingPrompts).toEqual({}); }); + it("sends slash commands without inserting an optimistic transcript line and toggles the sending state", async () => { + let state: AppState = { ...initialAppState(), selectedWorkspace: workspace, selectedSession: oldSession, sessions: [oldSession] }; + let resolveCommand: (() => void) | undefined; + const seenDuringCommand: Record[] = []; + const api: typeof defaultApi = { + ...defaultApi, + runCommand: (_session, text) => new Promise((resolve) => { + seenDuringCommand.push({ ...state.sendingPrompts }); + resolveCommand = () => { resolve(text.startsWith("/skill") ? { type: "done" } : { type: "done", message: "stats" }); }; + }), + }; + const controller = new SessionController( + () => state, + (patch) => { state = { ...state, ...patch }; }, + () => undefined, + undefined, + { api, socket: new FakeSocket() }, + ); + + const run = controller.send("/skill:skill-creator"); + expect(seenDuringCommand).toEqual([{ [oldSession.id]: true }]); + // No raw command text is added to the transcript; the agent streams the + // canonical expanded message back instead. + expect(state.messages).toEqual([]); + resolveCommand?.(); + await run; + expect(state.messages).toEqual([]); + expect(state.sendingPrompts).toEqual({}); + }); + it("keeps live message count updates when a cached new session becomes persisted", async () => { const cachedSession = markCachedNewSessionInfo(oldSession); let resolvePrompt: (() => void) | undefined; diff --git a/src/client/src/controllers/sessionController.ts b/src/client/src/controllers/sessionController.ts index e4e7710..b4a4836 100644 --- a/src/client/src/controllers/sessionController.ts +++ b/src/client/src/controllers/sessionController.ts @@ -235,12 +235,21 @@ export class SessionController { async runCommand(text: string) { const session = this.getState().selectedSession; if (!session || session.archived === true) return; - this.setState({ messages: [...this.getState().messages, textMessage("user", text)] }); + // Commands are not inserted into the transcript optimistically: a builtin + // command produces its own result line, and a runtime/skill command is + // forwarded to the agent, which streams back the canonical (expanded) + // message. Inserting the raw text here would leave a line that doesn't + // converge with server history and disappears on reload. Surface the same + // per-session sending indicator that send() uses for the pre-receipt window. + const sessionId = session.id; + this.markSendingPrompt(sessionId, true); try { this.applyCommandResult(await this.api.runCommand(session, text, selectedMachineId(this.getState()))); this.markCachedNewSessionPersisted(session); } catch (error) { this.setState({ messages: [...this.getState().messages, textMessage("system", String(error))], error: String(error) }); + } finally { + this.markSendingPrompt(sessionId, false); } } diff --git a/src/server/sessions/piSessionService.test.ts b/src/server/sessions/piSessionService.test.ts index 0e1e97e..22d2d9a 100644 --- a/src/server/sessions/piSessionService.test.ts +++ b/src/server/sessions/piSessionService.test.ts @@ -542,6 +542,32 @@ describe("PiSessionService", () => { await service.dispose(); }); + it("echoes the user message for direct prompts but not command-forwarded ones", async () => { + const fake = fakeRuntime("echo-session", { + resourceLoader: { getSkills: () => ({ skills: [{ name: "skill-creator" }] }) }, + }); + const hub = new CapturingSessionEventHub(); + const service = new PiSessionService(hub, { + createAgentRuntime: runtimeCreator(fake.runtime), + sessionManager: sessionGateway([sessionRecord("echo-session")]), + heartbeatIntervalMs: 60_000, + }); + + await service.prompt(sessionRef("echo-session"), "Build the thing"); + expect(hub.sessionEvents.filter(({ event }) => event.type === "message.append")).toHaveLength(1); + + // The client optimistically renders command-forwarded prompts (e.g. /skill:*), + // so the server must not publish a second copy via message.append. + await service.runCommand(sessionRef("echo-session"), "/skill:skill-creator"); + expect(hub.sessionEvents.filter(({ event }) => event.type === "message.append")).toHaveLength(1); + expect(fake.calls.prompt).toEqual([ + { text: "Build the thing", options: undefined }, + { text: "/skill:skill-creator", options: undefined }, + ]); + + await service.dispose(); + }); + it("rejects malformed prompt text before opening the runtime", async () => { const fake = fakeRuntime("prompt-session"); const service = new PiSessionService(new CapturingSessionEventHub(), { diff --git a/src/server/sessions/piSessionService.ts b/src/server/sessions/piSessionService.ts index 8457c8d..5152e45 100644 --- a/src/server/sessions/piSessionService.ts +++ b/src/server/sessions/piSessionService.ts @@ -76,6 +76,7 @@ interface QueuedPrompt { kind: QueuedPromptKind; text: string; images?: ImageContent[]; + echoUserMessage?: boolean; } function requirePromptText(value: unknown): string { @@ -300,7 +301,7 @@ export class PiSessionService { this.heartbeat = setInterval(() => { this.publishHeartbeats(); }, deps.heartbeatIntervalMs ?? 2000); this.commandService = new SessionCommandService( (sessionId) => this.getActive(sessionId), - (sessionId, text) => this.prompt(sessionId, text), + (sessionId, text) => this.prompt(sessionId, text, undefined, undefined, { echoUserMessage: false }), events, { onCompactionStart: (session) => { @@ -478,8 +479,13 @@ export class PiSessionService { return commands.sort((a, b) => a.name.localeCompare(b.name)); } - async prompt(ref: PiSessionLookup, text: unknown, streamingBehavior?: unknown, attachments?: unknown): Promise { + async prompt(ref: PiSessionLookup, text: unknown, streamingBehavior?: unknown, attachments?: unknown, options?: { echoUserMessage?: boolean }): Promise { const promptText = requirePromptText(text); + // Command-forwarded prompts (e.g. /skill:*) are expanded by the agent, which + // streams the canonical message back. The client doesn't render the raw + // command text, so the server must not echo it either, or it would show up + // as a transient line that vanishes on reload. + const echoUserMessage = options?.echoUserMessage !== false; const requestedBehavior = parsePromptStreamingBehavior(streamingBehavior); const parsedAttachments = parsePromptAttachments(attachments, { enforceInlineSizeLimit: false }); const images = (await attachmentsToInlineImages(parsedAttachments)).map((entry) => entry.image); @@ -494,15 +500,15 @@ export class PiSessionService { return; } if (session.isCompacting) { - this.enqueuePromptDuringCompaction(session, promptText, behavior ?? "followUp", images); + this.enqueuePromptDuringCompaction(session, promptText, behavior ?? "followUp", images, echoUserMessage); return; } - void this.submitPrompt(session, promptText, behavior, images); + void this.submitPrompt(session, promptText, behavior, images, echoUserMessage); } - private submitPrompt(session: PiAgentSession, text: string, behavior: QueuedPromptKind | undefined, images: ImageContent[] = []): Promise { + private submitPrompt(session: PiAgentSession, text: string, behavior: QueuedPromptKind | undefined, images: ImageContent[] = [], echoUserMessage = true): Promise { this.publishActivity(session, behavior === "steer" ? "steering queued" : behavior === "followUp" ? "message queued" : "prompt accepted", "active"); - if (behavior === undefined) this.events.publish(session.sessionId, { type: "message.append", message: userMessage(text, images) }); + if (behavior === undefined && echoUserMessage) this.events.publish(session.sessionId, { type: "message.append", message: userMessage(text, images) }); const promptOptions = buildPromptOptions(behavior, images); const promptPromise = session.prompt(text, promptOptions).catch((error: unknown) => { const message = error instanceof Error ? error.message : String(error); @@ -513,9 +519,9 @@ export class PiSessionService { return promptPromise; } - private enqueuePromptDuringCompaction(session: PiAgentSession, text: string, kind: QueuedPromptKind, images: ImageContent[] = []): void { + private enqueuePromptDuringCompaction(session: PiAgentSession, text: string, kind: QueuedPromptKind, images: ImageContent[] = [], echoUserMessage = true): void { const queue = this.compactionPromptQueues.get(session.sessionId) ?? []; - queue.push({ kind, text, ...(images.length > 0 ? { images } : {}) }); + queue.push({ kind, text, ...(images.length > 0 ? { images } : {}), ...(echoUserMessage ? {} : { echoUserMessage: false }) }); this.compactionPromptQueues.set(session.sessionId, queue); this.publishActivity(session, "message queued during compaction", "active"); this.publishStatus(session); @@ -859,14 +865,14 @@ export class PiSessionService { const queued = this.takeCompactionPromptQueue(sessionId); if (queued.length === 0) return; this.publishStatus(session); - for (const prompt of queued) void this.submitPrompt(session, prompt.text, prompt.kind, prompt.images); + for (const prompt of queued) void this.submitPrompt(session, prompt.text, prompt.kind, prompt.images, prompt.echoUserMessage ?? true); return; } const prompt = this.shiftCompactionPrompt(sessionId); if (prompt === undefined) return; this.publishStatus(session); - const submitted = this.submitPrompt(session, prompt.text, undefined, prompt.images); + const submitted = this.submitPrompt(session, prompt.text, undefined, prompt.images, prompt.echoUserMessage ?? true); void submitted.finally(() => { this.scheduleCompactionQueueDrain(sessionId); }); } diff --git a/src/server/sessions/sessionCommandService.test.ts b/src/server/sessions/sessionCommandService.test.ts index 168e22c..f3a3cb1 100644 --- a/src/server/sessions/sessionCommandService.test.ts +++ b/src/server/sessions/sessionCommandService.test.ts @@ -60,7 +60,9 @@ describe("SessionCommandService", () => { const service = new SessionCommandService(() => getActive(active), prompt, eventPublisher()); await expect(service.run("s1", "/missing")).resolves.toEqual({ type: "unsupported", message: "Unknown command: /missing" }); - await expect(service.run("s1", "/ext arg")).resolves.toEqual({ type: "done", message: "Accepted /ext arg" }); + // Forwarded runtime commands return a bare done result: the agent streams + // back the canonical expanded message, so no synthetic "Accepted" line. + await expect(service.run("s1", "/ext arg")).resolves.toEqual({ type: "done" }); await expect(service.run("s1", "/template arg")).resolves.toMatchObject({ type: "done" }); await expect(service.run("s1", "/skill:skill-a arg")).resolves.toMatchObject({ type: "done" }); expect(prompt).toHaveBeenCalledTimes(3); diff --git a/src/server/sessions/sessionCommandService.ts b/src/server/sessions/sessionCommandService.ts index 059a988..2c1f7b6 100644 --- a/src/server/sessions/sessionCommandService.ts +++ b/src/server/sessions/sessionCommandService.ts @@ -80,8 +80,12 @@ export class SessionCommandService