Archived
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.
This commit is contained in:
@@ -308,6 +308,36 @@ describe("SessionController", () => {
|
|||||||
expect(state.sendingPrompts).toEqual({});
|
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<string, true>[] = [];
|
||||||
|
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 () => {
|
it("keeps live message count updates when a cached new session becomes persisted", async () => {
|
||||||
const cachedSession = markCachedNewSessionInfo(oldSession);
|
const cachedSession = markCachedNewSessionInfo(oldSession);
|
||||||
let resolvePrompt: (() => void) | undefined;
|
let resolvePrompt: (() => void) | undefined;
|
||||||
|
|||||||
@@ -235,12 +235,21 @@ export class SessionController {
|
|||||||
async runCommand(text: string) {
|
async runCommand(text: string) {
|
||||||
const session = this.getState().selectedSession;
|
const session = this.getState().selectedSession;
|
||||||
if (!session || session.archived === true) return;
|
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 {
|
try {
|
||||||
this.applyCommandResult(await this.api.runCommand(session, text, selectedMachineId(this.getState())));
|
this.applyCommandResult(await this.api.runCommand(session, text, selectedMachineId(this.getState())));
|
||||||
this.markCachedNewSessionPersisted(session);
|
this.markCachedNewSessionPersisted(session);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
this.setState({ messages: [...this.getState().messages, textMessage("system", String(error))], error: String(error) });
|
this.setState({ messages: [...this.getState().messages, textMessage("system", String(error))], error: String(error) });
|
||||||
|
} finally {
|
||||||
|
this.markSendingPrompt(sessionId, false);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -542,6 +542,32 @@ describe("PiSessionService", () => {
|
|||||||
await service.dispose();
|
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 () => {
|
it("rejects malformed prompt text before opening the runtime", async () => {
|
||||||
const fake = fakeRuntime("prompt-session");
|
const fake = fakeRuntime("prompt-session");
|
||||||
const service = new PiSessionService(new CapturingSessionEventHub(), {
|
const service = new PiSessionService(new CapturingSessionEventHub(), {
|
||||||
|
|||||||
@@ -76,6 +76,7 @@ interface QueuedPrompt {
|
|||||||
kind: QueuedPromptKind;
|
kind: QueuedPromptKind;
|
||||||
text: string;
|
text: string;
|
||||||
images?: ImageContent[];
|
images?: ImageContent[];
|
||||||
|
echoUserMessage?: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
function requirePromptText(value: unknown): string {
|
function requirePromptText(value: unknown): string {
|
||||||
@@ -300,7 +301,7 @@ export class PiSessionService {
|
|||||||
this.heartbeat = setInterval(() => { this.publishHeartbeats(); }, deps.heartbeatIntervalMs ?? 2000);
|
this.heartbeat = setInterval(() => { this.publishHeartbeats(); }, deps.heartbeatIntervalMs ?? 2000);
|
||||||
this.commandService = new SessionCommandService(
|
this.commandService = new SessionCommandService(
|
||||||
(sessionId) => this.getActive(sessionId),
|
(sessionId) => this.getActive(sessionId),
|
||||||
(sessionId, text) => this.prompt(sessionId, text),
|
(sessionId, text) => this.prompt(sessionId, text, undefined, undefined, { echoUserMessage: false }),
|
||||||
events,
|
events,
|
||||||
{
|
{
|
||||||
onCompactionStart: (session) => {
|
onCompactionStart: (session) => {
|
||||||
@@ -478,8 +479,13 @@ export class PiSessionService {
|
|||||||
return commands.sort((a, b) => a.name.localeCompare(b.name));
|
return commands.sort((a, b) => a.name.localeCompare(b.name));
|
||||||
}
|
}
|
||||||
|
|
||||||
async prompt(ref: PiSessionLookup, text: unknown, streamingBehavior?: unknown, attachments?: unknown): Promise<void> {
|
async prompt(ref: PiSessionLookup, text: unknown, streamingBehavior?: unknown, attachments?: unknown, options?: { echoUserMessage?: boolean }): Promise<void> {
|
||||||
const promptText = requirePromptText(text);
|
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 requestedBehavior = parsePromptStreamingBehavior(streamingBehavior);
|
||||||
const parsedAttachments = parsePromptAttachments(attachments, { enforceInlineSizeLimit: false });
|
const parsedAttachments = parsePromptAttachments(attachments, { enforceInlineSizeLimit: false });
|
||||||
const images = (await attachmentsToInlineImages(parsedAttachments)).map((entry) => entry.image);
|
const images = (await attachmentsToInlineImages(parsedAttachments)).map((entry) => entry.image);
|
||||||
@@ -494,15 +500,15 @@ export class PiSessionService {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (session.isCompacting) {
|
if (session.isCompacting) {
|
||||||
this.enqueuePromptDuringCompaction(session, promptText, behavior ?? "followUp", images);
|
this.enqueuePromptDuringCompaction(session, promptText, behavior ?? "followUp", images, echoUserMessage);
|
||||||
return;
|
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<void> {
|
private submitPrompt(session: PiAgentSession, text: string, behavior: QueuedPromptKind | undefined, images: ImageContent[] = [], echoUserMessage = true): Promise<void> {
|
||||||
this.publishActivity(session, behavior === "steer" ? "steering queued" : behavior === "followUp" ? "message queued" : "prompt accepted", "active");
|
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 promptOptions = buildPromptOptions(behavior, images);
|
||||||
const promptPromise = session.prompt(text, promptOptions).catch((error: unknown) => {
|
const promptPromise = session.prompt(text, promptOptions).catch((error: unknown) => {
|
||||||
const message = error instanceof Error ? error.message : String(error);
|
const message = error instanceof Error ? error.message : String(error);
|
||||||
@@ -513,9 +519,9 @@ export class PiSessionService {
|
|||||||
return promptPromise;
|
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) ?? [];
|
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.compactionPromptQueues.set(session.sessionId, queue);
|
||||||
this.publishActivity(session, "message queued during compaction", "active");
|
this.publishActivity(session, "message queued during compaction", "active");
|
||||||
this.publishStatus(session);
|
this.publishStatus(session);
|
||||||
@@ -859,14 +865,14 @@ export class PiSessionService {
|
|||||||
const queued = this.takeCompactionPromptQueue(sessionId);
|
const queued = this.takeCompactionPromptQueue(sessionId);
|
||||||
if (queued.length === 0) return;
|
if (queued.length === 0) return;
|
||||||
this.publishStatus(session);
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const prompt = this.shiftCompactionPrompt(sessionId);
|
const prompt = this.shiftCompactionPrompt(sessionId);
|
||||||
if (prompt === undefined) return;
|
if (prompt === undefined) return;
|
||||||
this.publishStatus(session);
|
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); });
|
void submitted.finally(() => { this.scheduleCompactionQueueDrain(sessionId); });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -60,7 +60,9 @@ describe("SessionCommandService", () => {
|
|||||||
const service = new SessionCommandService(() => getActive(active), prompt, eventPublisher());
|
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", "/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", "/template arg")).resolves.toMatchObject({ type: "done" });
|
||||||
await expect(service.run("s1", "/skill:skill-a arg")).resolves.toMatchObject({ type: "done" });
|
await expect(service.run("s1", "/skill:skill-a arg")).resolves.toMatchObject({ type: "done" });
|
||||||
expect(prompt).toHaveBeenCalledTimes(3);
|
expect(prompt).toHaveBeenCalledTimes(3);
|
||||||
|
|||||||
@@ -80,8 +80,12 @@ export class SessionCommandService<TSession extends CommandSession = CommandSess
|
|||||||
|
|
||||||
if (!isBuiltinCommand(name)) {
|
if (!isBuiltinCommand(name)) {
|
||||||
if (this.isRuntimeCommand(session, name)) {
|
if (this.isRuntimeCommand(session, name)) {
|
||||||
|
// The command is forwarded to the agent, which expands it (e.g. /skill:*
|
||||||
|
// into a skill block) and streams the canonical message back. That is the
|
||||||
|
// authoritative feedback, so we don't synthesize an extra "Accepted" line
|
||||||
|
// that would only vanish on reload.
|
||||||
await this.prompt(sessionId, text);
|
await this.prompt(sessionId, text);
|
||||||
return { type: "done", message: `Accepted ${text}` };
|
return { type: "done" };
|
||||||
}
|
}
|
||||||
return { type: "unsupported", message: `Unknown command: /${name}` };
|
return { type: "unsupported", message: `Unknown command: /${name}` };
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user