refactor: remove threads layer, use sessions directly for ACP (#9078)

Co-authored-by: Douwe Osinga <douwe@squareup.com>
This commit is contained in:
Douwe Osinga
2026-05-08 16:10:55 -04:00
committed by GitHub
parent a999f52056
commit ea5802c380
32 changed files with 525 additions and 1922 deletions
+9 -4
View File
@@ -7720,6 +7720,11 @@
"format": "int32",
"nullable": true
},
"archived_at": {
"type": "string",
"format": "date-time",
"nullable": true
},
"conversation": {
"allOf": [
{
@@ -7766,6 +7771,10 @@
"format": "int32",
"nullable": true
},
"project_id": {
"type": "string",
"nullable": true
},
"provider_name": {
"type": "string",
"nullable": true
@@ -7785,10 +7794,6 @@
"session_type": {
"$ref": "#/components/schemas/SessionType"
},
"thread_id": {
"type": "string",
"nullable": true
},
"total_tokens": {
"type": "integer",
"format": "int32",
+2 -1
View File
@@ -1256,6 +1256,7 @@ export type Session = {
accumulated_input_tokens?: number | null;
accumulated_output_tokens?: number | null;
accumulated_total_tokens?: number | null;
archived_at?: string | null;
conversation?: Conversation | null;
created_at: string;
extension_data: ExtensionData;
@@ -1266,11 +1267,11 @@ export type Session = {
model_config?: ModelConfig | null;
name: string;
output_tokens?: number | null;
project_id?: string | null;
provider_name?: string | null;
recipe?: Recipe | null;
schedule_id?: string | null;
session_type?: SessionType;
thread_id?: string | null;
total_tokens?: number | null;
updated_at: string;
user_recipe_values?: {
@@ -80,11 +80,7 @@ describe("useChat compaction", () => {
await result.current.compactConversation();
});
expect(mockAcpSendMessage).toHaveBeenCalledWith(
"session-1",
"/compact",
undefined,
);
expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact");
expect(mockAcpLoadSession).toHaveBeenCalledWith("session-1", undefined);
const messages = useChatStore.getState().messagesBySession["session-1"];
@@ -121,35 +117,6 @@ describe("useChat compaction", () => {
);
});
it("prepares and compacts the override persona session", async () => {
let preparedPersonaId: string | undefined;
const ensurePrepared = vi.fn(async (personaId?: string) => {
preparedPersonaId = personaId;
return undefined;
});
const { result } = renderHook(() =>
useChat(
"session-1",
undefined,
undefined,
{ id: "persona-b", name: "Persona B" },
{ ensurePrepared },
),
);
await act(async () => {
await result.current.compactConversation({ id: "persona-a" });
});
expect(ensurePrepared).toHaveBeenCalledWith("persona-a");
expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact", {
personaId: "persona-a",
});
expect(mockAcpLoadSession).toHaveBeenCalledWith("session-1", undefined);
expect(preparedPersonaId).toBe("persona-a");
});
it("blocks new sends while compaction is in flight", async () => {
const compactDeferred = createDeferredPromise();
mockAcpSendMessage.mockImplementation(
@@ -174,11 +141,7 @@ describe("useChat compaction", () => {
});
expect(mockAcpSendMessage).toHaveBeenCalledTimes(1);
expect(mockAcpSendMessage).toHaveBeenCalledWith(
"session-1",
"/compact",
undefined,
);
expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact");
expect(
useChatStore.getState().messagesBySession["session-1"],
).toBeUndefined();
@@ -214,11 +177,7 @@ describe("useChat compaction", () => {
});
expect(mockAcpSendMessage).toHaveBeenCalledTimes(1);
expect(mockAcpSendMessage).toHaveBeenCalledWith(
"session-1",
"/compact",
undefined,
);
expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "/compact");
expect(mockAcpLoadSession).not.toHaveBeenCalled();
expect(
useChatStore.getState().getSessionRuntime("session-1").chatState,
@@ -1,96 +0,0 @@
import { act, renderHook } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { useAgentStore } from "@/features/agents/stores/agentStore";
import { useChatStore } from "../../stores/chatStore";
import { useChatSessionStore } from "../../stores/chatSessionStore";
import { clearReplayBuffer } from "../replayBuffer";
const mockAcpSendMessage = vi.fn();
const mockAcpCancelSession = vi.fn();
const mockAcpLoadSession = vi.fn();
vi.mock("@/shared/api/acp", () => ({
acpSendMessage: (...args: unknown[]) => mockAcpSendMessage(...args),
acpCancelSession: (...args: unknown[]) => mockAcpCancelSession(...args),
acpLoadSession: (...args: unknown[]) => mockAcpLoadSession(...args),
}));
import { useChat } from "../useChat";
describe("useChat persona preparation", () => {
beforeEach(() => {
mockAcpSendMessage.mockReset();
mockAcpCancelSession.mockReset();
mockAcpLoadSession.mockReset();
clearReplayBuffer("session-1");
useChatStore.setState({
messagesBySession: {},
sessionStateById: {},
activeSessionId: null,
isConnected: true,
});
useChatSessionStore.setState({
sessions: [],
activeSessionId: null,
isLoading: false,
contextPanelOpenBySession: {},
activeWorkspaceBySession: {},
});
useAgentStore.setState({
personas: [
{
id: "persona-a",
displayName: "Persona A",
systemPrompt: "",
isBuiltin: false,
createdAt: "",
updatedAt: "",
},
{
id: "persona-b",
displayName: "Persona B",
systemPrompt: "",
isBuiltin: false,
createdAt: "",
updatedAt: "",
},
],
personasLoading: false,
agents: [],
agentsLoading: false,
activeAgentId: null,
isLoading: false,
personaEditorOpen: false,
editingPersona: null,
});
mockAcpSendMessage.mockResolvedValue(undefined);
mockAcpCancelSession.mockResolvedValue(true);
mockAcpLoadSession.mockResolvedValue(undefined);
});
it("prepares the override persona before prompting", async () => {
const ensurePrepared = vi.fn().mockResolvedValue(undefined);
const { result } = renderHook(() =>
useChat(
"session-1",
undefined,
undefined,
{ id: "persona-a", name: "Persona A" },
{ ensurePrepared },
),
);
await act(async () => {
await result.current.sendMessage("Hello", { id: "persona-b" });
});
expect(ensurePrepared).toHaveBeenCalledWith("persona-b");
expect(mockAcpSendMessage).toHaveBeenCalledWith("session-1", "Hello", {
systemPrompt: undefined,
personaId: "persona-b",
personaName: "Persona B",
images: undefined,
});
});
});
+2 -6
View File
@@ -268,8 +268,7 @@ export function useChat(
...(sendOptions?.assistantPrompt
? { assistantPrompt: sendOptions.assistantPrompt }
: {}),
personaId: effectivePersonaInfo?.id,
personaName: effectivePersonaInfo?.name,
images: images?.map(
(img) => [img.base64, img.mimeType] as [string, string],
),
@@ -415,10 +414,7 @@ export function useChat(
clearReplayBuffer(sessionId);
try {
const sendOptions = effectivePersonaInfo?.id
? { personaId: effectivePersonaInfo.id }
: undefined;
await acpSendMessage(sessionId, MANUAL_COMPACT_TRIGGER, sendOptions);
await acpSendMessage(sessionId, MANUAL_COMPACT_TRIGGER);
// Command responses are streamed via prompt notifications, but the ACP
// layer does not currently forward history replacement events. Drop those
@@ -108,7 +108,7 @@ export function useChatSessionController({
const selectedPersonaId =
pendingPersonaId !== undefined
? pendingPersonaId
: (session?.personaId ?? null);
: (session?.agentId ?? null);
const selectedPersona = personas.find(
(persona) => persona.id === selectedPersonaId,
);
@@ -378,7 +378,7 @@ export function useChatSessionController({
}
useChatSessionStore
.getState()
.patchSession(sessionId, { personaId: personaId ?? undefined });
.patchSession(sessionId, { agentId: personaId ?? undefined });
},
[
handleProviderChange,
@@ -399,7 +399,7 @@ export function useChatSessionController({
if (sessionId) {
useChatSessionStore
.getState()
.patchSession(sessionId, { personaId: undefined });
.patchSession(sessionId, { agentId: undefined });
} else {
setPendingPersonaId(undefined);
}
@@ -595,7 +595,7 @@ export function useChatSessionController({
const nextPersonaId =
pendingPersonaId !== undefined
? (pendingPersonaId ?? undefined)
: session?.personaId;
: session?.agentId;
const nextProjectId =
pendingProjectId !== undefined
? pendingProjectId
@@ -610,10 +610,10 @@ export function useChatSessionController({
const patch: {
providerId?: string;
personaId?: string | undefined;
agentId?: string | undefined;
projectId?: string | null;
modelId?: string | undefined;
modelName?: string | undefined;
projectId?: string | null;
} = {};
if (hasPendingProvider) {
@@ -622,7 +622,7 @@ export function useChatSessionController({
patch.modelName = undefined;
}
if (hasPendingPersona) {
patch.personaId = nextPersonaId;
patch.agentId = nextPersonaId;
}
if (hasPendingProject) {
patch.projectId = nextProjectId ?? null;
@@ -690,7 +690,7 @@ export function useChatSessionController({
pendingQueuedMessage,
prepareCurrentSession,
selectedProvider,
session?.personaId,
session?.agentId,
session?.projectId,
sessionId,
]);
@@ -61,7 +61,7 @@ describe("chatSessionStore", () => {
title: "New Chat",
providerId: "openai",
projectId: "project-1",
personaId: "persona-1",
agentId: "persona-1",
modelId: "gpt-4.1",
modelName: "GPT-4.1",
workingDir: "/tmp/project",
@@ -72,7 +72,6 @@ describe("chatSessionStore", () => {
"/tmp/project",
{
projectId: "project-1",
personaId: "persona-1",
modelId: "gpt-4.1",
},
);
@@ -81,7 +80,7 @@ describe("chatSessionStore", () => {
title: "New Chat",
projectId: "project-1",
providerId: "openai",
personaId: "persona-1",
agentId: "persona-1",
modelId: "gpt-4.1",
modelName: "GPT-4.1",
workingDir: "/tmp/project",
@@ -146,7 +145,6 @@ describe("chatSessionStore", () => {
workingDir: "/tmp/project-123",
projectId: "project-123",
providerId: "anthropic",
personaId: "persona-1",
modelId: "claude-sonnet-4",
},
]);
@@ -157,7 +155,6 @@ describe("chatSessionStore", () => {
expect(session.title).toBe("Renamed Chat");
expect(session.projectId).toBe("project-123");
expect(session.providerId).toBe("anthropic");
expect(session.personaId).toBe("persona-1");
expect(session.createdAt).toBe("2026-03-31");
expect(session.updatedAt).toBe("2026-04-02");
expect(session.messageCount).toBe(7);
@@ -19,7 +19,7 @@ export interface ChatSession {
title: string;
projectId?: string | null;
providerId?: string;
personaId?: string;
agentId?: string;
modelId?: string;
modelName?: string;
workingDir?: string | null;
@@ -66,7 +66,7 @@ interface CreateSessionOpts {
title?: string;
projectId?: string;
providerId?: string;
personaId?: string;
agentId?: string;
workingDir?: string;
modelId?: string;
modelName?: string;
@@ -100,7 +100,6 @@ function acpSessionToChatSession(session: AcpSessionInfo): ChatSession {
title: normalizeAcpTitle(session.title) ?? "Untitled",
projectId: session.projectId ?? undefined,
providerId: session.providerId ?? undefined,
personaId: session.personaId ?? undefined,
modelId: session.modelId ?? undefined,
workingDir: session.workingDir ?? undefined,
createdAt: session.createdAt ?? session.updatedAt ?? now,
@@ -123,7 +122,6 @@ export function sessionToChatSession(session: Session): ChatSession {
title: session.title,
projectId: session.projectId,
providerId: session.providerId,
personaId: session.personaId,
modelId: session.modelId,
modelName: session.modelName,
workingDir: session.workingDir,
@@ -150,7 +148,6 @@ export const useChatSessionStore = create<ChatSessionStore>((set, get) => ({
const now = new Date().toISOString();
const providerId = opts.providerId ?? "goose";
const { sessionId } = await acpCreateSession(providerId, opts.workingDir, {
personaId: opts.personaId,
modelId: opts.modelId,
projectId: opts.projectId,
});
@@ -159,7 +156,7 @@ export const useChatSessionStore = create<ChatSessionStore>((set, get) => ({
title: opts.title ?? DEFAULT_CHAT_TITLE,
projectId: opts.projectId,
providerId,
personaId: opts.personaId,
agentId: opts.agentId,
modelId: opts.modelId,
modelName: opts.modelName,
workingDir: opts.workingDir,
@@ -30,7 +30,7 @@ describe("buildSessionSearchResults", () => {
makeSession({
id: "session-2",
title: "Builder notes",
personaId: "persona-1",
agentId: "persona-1",
updatedAt: "2026-04-09T12:00:00Z",
}),
];
@@ -22,7 +22,7 @@ function makeSession(
describe("filterSessions", () => {
const sessions: ChatSession[] = [
makeSession({ id: "1", title: "Fix sidebar bug", personaId: "p1" }),
makeSession({ id: "1", title: "Fix sidebar bug", agentId: "p1" }),
makeSession({
id: "2",
title: "Add pagination",
@@ -23,8 +23,8 @@ function buildSearchableString(
parts.push(session.title);
}
if (session.personaId) {
const name = resolvers.getPersonaName(session.personaId);
if (session.agentId) {
const name = resolvers.getPersonaName(session.agentId);
if (name) parts.push(name);
}
@@ -236,8 +236,8 @@ export function SessionHistoryView({
title={result.session.title}
updatedAt={result.session.updatedAt}
personaName={
result.session.personaId
? getPersonaName(result.session.personaId)
result.session.agentId
? getPersonaName(result.session.agentId)
: undefined
}
projectName={
@@ -299,8 +299,8 @@ export function SessionHistoryView({
title={session.title}
updatedAt={session.updatedAt}
personaName={
session.personaId
? getPersonaName(session.personaId)
session.agentId
? getPersonaName(session.agentId)
: undefined
}
projectName={
@@ -41,8 +41,8 @@ export function SidebarSearchResults({
session.title,
t("common:session.defaultTitle"),
);
const personaName = session.personaId
? getPersonaName(session.personaId)
const personaName = session.agentId
? getPersonaName(session.agentId)
: undefined;
const projectName = session.projectId
? getProjectName(session.projectId)
@@ -69,7 +69,6 @@ describe("acpCreateSession", () => {
await expect(
acpCreateSession("openai", "/tmp/project", {
projectId: "project-1",
personaId: "persona-1",
modelId: "gpt-4.1",
}),
).resolves.toEqual({ sessionId: "acp-session-1" });
@@ -78,7 +77,6 @@ describe("acpCreateSession", () => {
"/tmp/project",
"openai",
"project-1",
"persona-1",
);
expect(mockLoadSession).not.toHaveBeenCalled();
expect(mockSetProvider).toHaveBeenCalledWith("acp-session-1", "openai");
+2 -12
View File
@@ -21,14 +21,11 @@ export interface AcpProvider {
export interface AcpSendMessageOptions {
systemPrompt?: string;
assistantPrompt?: string;
personaId?: string;
personaName?: string;
/** Image attachments as [base64Data, mimeType] pairs. */
images?: [string, string][];
}
export interface AcpCreateSessionOptions {
personaId?: string;
projectId?: string;
modelId?: string | null;
}
@@ -79,7 +76,7 @@ export async function acpSendMessage(
prompt: string,
options: AcpSendMessageOptions = {},
): Promise<void> {
const { systemPrompt, assistantPrompt, personaId, images } = options;
const { systemPrompt, assistantPrompt, images } = options;
const sid = sessionId.slice(0, 8);
const tStart = performance.now();
@@ -116,14 +113,8 @@ export async function acpSendMessage(
`[perf:send] ${sid} acpSendMessage → prompt(len=${prompt.length}, imgs=${images?.length ?? 0})`,
);
const tPrompt = performance.now();
const meta: Record<string, unknown> = {};
if (personaId) meta.personaId = personaId;
try {
await directAcp.prompt(
sessionId,
content,
Object.keys(meta).length > 0 ? meta : undefined,
);
await directAcp.prompt(sessionId, content);
const tDone = performance.now();
perfLog(
`[perf:send] ${sid} prompt() resolved in ${(tDone - tPrompt).toFixed(1)}ms (total acpSendMessage ${(tDone - tStart).toFixed(1)}ms)`,
@@ -159,7 +150,6 @@ export async function acpCreateSession(
workingDir,
providerId,
options.projectId,
options.personaId,
);
const sessionId = response.sessionId;
await directAcp.setProvider(sessionId, providerId);
+2 -7
View File
@@ -26,7 +26,6 @@ export interface AcpSessionInfo {
projectId?: string | null;
providerId: string | null;
modelId: string | null;
personaId: string | null;
}
export const DEPRECATED_PROVIDER_IDS = new Set([
@@ -83,7 +82,6 @@ export async function listSessions(): Promise<AcpSessionInfo[]> {
projectId: (info._meta?.projectId as string) ?? null,
providerId: (info._meta?.providerId as string) ?? null,
modelId: (info._meta?.modelId as string) ?? null,
personaId: (info._meta?.personaId as string) ?? null,
}));
}
@@ -118,7 +116,6 @@ export async function forkSession(sessionId: string): Promise<AcpSessionInfo> {
projectId: (response._meta?.projectId as string) ?? null,
providerId: (response._meta?.providerId as string) ?? null,
modelId: (response._meta?.modelId as string) ?? null,
personaId: (response._meta?.personaId as string) ?? null,
};
}
@@ -207,7 +204,6 @@ export async function newSession(
workingDir: string,
providerId?: string,
projectId?: string,
personaId?: string,
): Promise<NewSessionResponse> {
const tClient = performance.now();
const client = await getClient();
@@ -216,11 +212,10 @@ export async function newSession(
mcpServers: [],
};
const meta: Record<string, string> = {};
const meta: Record<string, string> = { client: "goose" };
if (providerId) meta.provider = providerId;
if (projectId) meta.projectId = projectId;
if (personaId) meta.personaId = personaId;
if (Object.keys(meta).length > 0) request._meta = meta;
request._meta = meta;
const tCall = performance.now();
const response = await client.newSession(request);
-1
View File
@@ -54,7 +54,6 @@ export interface Session {
title: string;
projectId?: string | null;
providerId?: string;
personaId?: string;
modelId?: string;
modelName?: string;
workingDir?: string | null;
+10 -5
View File
@@ -8,11 +8,16 @@ async function clickNewChatInProject(
name: projectName,
exact: true,
});
await projectButton.hover();
await projectButton
.locator("xpath=..")
.getByTitle("New chat in project")
.click();
// The "New chat" button uses group-hover:visible and is invisible by default.
// Headless Playwright on Linux doesn't reliably trigger CSS :hover,
// so we force the button visible via JS before clicking.
const row = projectButton.locator("xpath=..");
const newChatBtn = row.getByTitle("New chat in project");
await newChatBtn.evaluate((el) => {
el.style.visibility = "visible";
el.style.opacity = "1";
});
await newChatBtn.click();
}
test.describe("Draft persistence", () => {
+27 -1
View File
@@ -124,6 +124,27 @@ export function buildInitScript(options?: {
supportingFiles: [],
});
const projectToSourceEntry = (p) => ({
type: "project",
name: p.id ?? p.name?.toLowerCase(),
description: p.description ?? "",
content: p.prompt ?? "",
path: "/mock/.agents/projects/" + (p.id ?? p.name?.toLowerCase()),
global: true,
supportingFiles: [],
properties: {
title: p.name,
icon: p.icon ?? "",
color: p.color ?? "",
preferredProvider: p.preferredProvider ?? null,
preferredModel: p.preferredModel ?? null,
workingDirs: p.workingDirs ?? [],
useWorktrees: p.useWorktrees ?? false,
order: p.order ?? 0,
archivedAt: null,
},
});
function nowIso() {
return new Date().toISOString();
}
@@ -240,8 +261,13 @@ export function buildInitScript(options?: {
case "_goose/working_dir/update":
case "goose/working_dir/update":
return jsonRpcResult(message.id, {});
case "_goose/sources/list":
case "_goose/sources/list": {
const sourceType = message.params?.type;
if (sourceType === "project") {
return jsonRpcResult(message.id, { sources: PROJECTS.map(projectToSourceEntry) });
}
return jsonRpcResult(message.id, { sources: SKILLS.map(skillToSourceEntry) });
}
case "_goose/sources/create":
return jsonRpcResult(message.id, {
source: {
+6 -2
View File
@@ -52,8 +52,12 @@ test.describe("Skills view", () => {
await page.getByPlaceholder("Search skills").fill("review");
await expect(page.getByText("code-review")).toBeVisible();
await expect(page.getByText("test-writer")).not.toBeVisible();
await expect(
page.getByRole("button", { name: "Open code-review details" }),
).toBeVisible();
await expect(
page.getByRole("button", { name: "Open test-writer details" }),
).not.toBeVisible();
});
test("project filtering isolates project skills", async ({