diff --git a/LAWS/CHAT.md b/LAWS/CHAT.md index 90f52f25..917e9174 100644 --- a/LAWS/CHAT.md +++ b/LAWS/CHAT.md @@ -5,7 +5,6 @@ - The composer MUST queue every accepted message into the selected chat's queue, including before that chat's session is ready. - The composer MUST queue accepted messages into the selected chat's queue in their acceptance order. - The selected chat's queue MUST retain the message and persona intent most recently accepted from the composer or a user edit. -- A chat's queue MUST dispatch each message to that chat's session with the model and provider shown when the composer queued it. - A chat's queue MUST NOT dispatch a message to that chat's session before every message ahead of it in the queue. - A message MUST NOT be dispatched from the queue until its session is ready. - A session MUST be ready for dispatch from its queue only when it can begin processing that queue's first message. diff --git a/src/features/chat/hooks/__tests__/useChatSessionController.test.ts b/src/features/chat/hooks/__tests__/useChatSessionController.test.ts index 09985619..d12c33d7 100644 --- a/src/features/chat/hooks/__tests__/useChatSessionController.test.ts +++ b/src/features/chat/hooks/__tests__/useChatSessionController.test.ts @@ -1188,6 +1188,48 @@ describe("useChatSessionController", () => { }); }); + it("discards in-flight Agent Builder preparation when its queue record is removed", async () => { + const pendingDraft = deferred<{ path: string; slug: string }>(); + mockPreSeedDraftAgent.mockReturnValueOnce(pendingDraft.promise); + useChatStore.getState().enqueueTransportReadyMessage("session-1", { + persona: { kind: "inherit" }, + text: "make a reviewer", + sendOptions: { + chips: [{ label: "agent-builder", type: "skill" }], + }, + }); + const queuedRecord = + useChatStore.getState().queuedMessageBySession["session-1"]?.[0]; + + renderHook(() => useChatSessionController({ sessionId: "session-1" })); + + await waitFor(() => { + expect(mockPreSeedDraftAgent).toHaveBeenCalledWith("session-1"); + }); + act(() => { + useChatStore + .getState() + .dismissQueuedMessage("session-1", queuedRecord?.recordId); + }); + await act(async () => { + pendingDraft.resolve({ + path: "/Users/x/.agents/agents/removed-queue-record.md", + slug: "removed-queue-record", + }); + await pendingDraft.promise; + }); + + await waitFor(() => { + expect(mockDeletePersonaSource).toHaveBeenCalledWith( + "/Users/x/.agents/agents/removed-queue-record.md", + ); + }); + const session = useChatSessionStore.getState().getSession("session-1"); + expect(session?.intent).toBeUndefined(); + expect(session?.targetAgentPath).toBeUndefined(); + expect(mockUseChatSendMessage).not.toHaveBeenCalled(); + }); + it("marks queued Agent Builder preparation as failed without dropping its send", async () => { mockPreSeedDraftAgent.mockRejectedValueOnce( new Error("draft creation failed"), diff --git a/src/features/chat/hooks/useBackgroundQueuedMessageDrain.test.tsx b/src/features/chat/hooks/useBackgroundQueuedMessageDrain.test.tsx index d8922248..b569c4b0 100644 --- a/src/features/chat/hooks/useBackgroundQueuedMessageDrain.test.tsx +++ b/src/features/chat/hooks/useBackgroundQueuedMessageDrain.test.tsx @@ -86,6 +86,22 @@ function releasedRecord(): QueuedMessageRecord & { kind: "transport-ready" } { }; } +function agentBuilderRecord(): QueuedMessageRecord & { + kind: "transport-ready"; +} { + return { + kind: "transport-ready", + recordId: "agent-builder-record", + payload: { + text: "make a reviewer", + persona: { kind: "inherit" }, + sendOptions: { + chips: [{ label: "agent-builder", type: "skill" }], + }, + }, + }; +} + function ordinaryRecord(): QueuedMessageRecord & { kind: "transport-ready" } { return { kind: "transport-ready", @@ -1210,6 +1226,53 @@ describe("useBackgroundQueuedMessageDrain", () => { ).toEqual([BACKEND_SESSION_ID]); }); + it("keeps an unmounted Agent Builder head parked after promotion until its draft target is prepared", async () => { + const builder = agentBuilderRecord(); + seedDraftSession(); + useChatStore.setState({ + queuedMessageBySession: { [DRAFT_SESSION_ID]: [builder] }, + }); + + render(); + act(() => promoteDraft()); + + expect( + mocks.sendQueuedPromptToExistingSessionInBackground, + ).not.toHaveBeenCalled(); + expect( + useChatStore.getState().queuedMessageBySession[BACKEND_SESSION_ID]?.[0], + ).toBe(builder); + + const releaseOwner = registerForegroundQueueOwner(BACKEND_SESSION_ID); + act(() => { + useChatSessionStore.getState().patchSession(BACKEND_SESSION_ID, { + intent: "build-agent", + agentBuilderOpen: true, + targetAgentPath: "/Users/x/.agents/agents/reviewer.md", + targetAgentSlug: "reviewer", + }); + }); + expect( + mocks.sendQueuedPromptToExistingSessionInBackground, + ).not.toHaveBeenCalled(); + + act(() => releaseOwner()); + + await waitFor(() => + expect( + mocks.sendQueuedPromptToExistingSessionInBackground, + ).toHaveBeenCalledOnce(), + ); + expect( + mocks.sendQueuedPromptToExistingSessionInBackground, + ).toHaveBeenCalledWith( + BACKEND_SESSION_ID, + builder, + expect.any(Function), + expect.any(Function), + ); + }); + it("keeps a queued head parked without toasting when creation failed", async () => { const ordinary = ordinaryRecord(); seedDraftSession("failed"); diff --git a/src/features/chat/hooks/useBackgroundQueuedMessageDrain.ts b/src/features/chat/hooks/useBackgroundQueuedMessageDrain.ts index 9107ca96..783e0a6f 100644 --- a/src/features/chat/hooks/useBackgroundQueuedMessageDrain.ts +++ b/src/features/chat/hooks/useBackgroundQueuedMessageDrain.ts @@ -5,6 +5,7 @@ import { i18n } from "@/shared/i18n"; import { assertQueuedSessionReady, isQueuedSessionReady, + QueuedSessionNotReadyError, } from "@/features/chat/lib/queuedMessageReadiness"; import { PreCommitSendRejectedError } from "@/features/chat/lib/preCommitSendRejection"; import { @@ -17,6 +18,7 @@ import { subscribeForegroundQueueOwnership, } from "@/features/chat/lib/foregroundQueueOwnership"; import { isBerdctlCrossSessionQueuedMessage } from "@/features/chat/lib/queuedMessageOrigin"; +import { isAgentBuilderQueuePreparationReady } from "@/features/chat/lib/agentBuilderQueueReadiness"; import { useChatSessionStore } from "@/features/chat/stores/chatSessionStore"; import { type QueuedMessageRecord, @@ -138,7 +140,13 @@ function isBackgroundDrainableHead( record: QueuedMessageRecord & { kind: "transport-ready" }, sessionId: string, ): boolean { - if (isBerdctlCrossSessionQueuedMessage(record)) { + if ( + isBerdctlCrossSessionQueuedMessage(record) || + !isAgentBuilderQueuePreparationReady( + record, + useChatSessionStore.getState().getSession(sessionId), + ) + ) { return false; } if (record.releasedFromDeferred) { @@ -246,6 +254,14 @@ function drainQueuedMessage(sessionId: string, ownerId: string): void { queuedMessage, ); assertQueuedSessionReady(state.getSessionRuntime(sessionId)); + if ( + !isAgentBuilderQueuePreparationReady( + queuedMessage, + useChatSessionStore.getState().getSession(sessionId), + ) + ) { + throw new QueuedSessionNotReadyError(); + } }, () => { useChatStore diff --git a/src/features/chat/hooks/useChatSessionController.ts b/src/features/chat/hooks/useChatSessionController.ts index 6ba6d0e8..3cbe97a4 100644 --- a/src/features/chat/hooks/useChatSessionController.ts +++ b/src/features/chat/hooks/useChatSessionController.ts @@ -2227,18 +2227,53 @@ export function useChatSessionController({ isQueuePreparationReady, ); const pendingBuilderActivationRef = useRef< - Record> + Record< + string, + { + promise: Promise; + queueRecordId?: string; + } + > >({}); + const isQueuedAgentBuilderRecordAuthoritative = useCallback( + (recordId: string) => { + const record = + useChatStore.getState().queuedMessageBySession[stateSessionId]?.[0]; + return Boolean( + record?.recordId === recordId && + record.kind === "transport-ready" && + isAgentBuilderSkillSendOptions(record.payload.sendOptions), + ); + }, + [stateSessionId], + ); + const ensureCurrentSessionIsAgentBuilder = useCallback( - async (options?: { requireSelectedSkill?: boolean }) => { + async (options?: { + requireSelectedSkill?: boolean; + queueRecordId?: string; + }) => { if (!sessionId) { return null; } const pendingActivation = pendingBuilderActivationRef.current[sessionId]; if (pendingActivation) { - return pendingActivation; + if ( + !options?.queueRecordId || + pendingActivation.queueRecordId === options.queueRecordId + ) { + return pendingActivation.promise; + } + await pendingActivation.promise; + } + + if ( + options?.queueRecordId && + !isQueuedAgentBuilderRecordAuthoritative(options.queueRecordId) + ) { + return null; } const activation = (async () => { @@ -2259,6 +2294,16 @@ export function useChatSessionController({ } const target = await preSeedDraftAgent(sessionId); + if ( + options?.queueRecordId && + !isQueuedAgentBuilderRecordAuthoritative(options.queueRecordId) + ) { + await deletePersonaSource(target.path).catch((error) => { + console.error("Failed to delete superseded agent draft:", error); + }); + return null; + } + const liveChatSessions = useChatSessionStore.getState(); const liveSession = liveChatSessions.getSession(sessionId); const liveSkills = @@ -2308,29 +2353,44 @@ export function useChatSessionController({ return { ...currentSession, ...patch }; })(); - pendingBuilderActivationRef.current[sessionId] = activation; + const pendingEntry = { + promise: activation, + queueRecordId: options?.queueRecordId, + }; + pendingBuilderActivationRef.current[sessionId] = pendingEntry; try { return await activation; } finally { - if (pendingBuilderActivationRef.current[sessionId] === activation) { + if (pendingBuilderActivationRef.current[sessionId] === pendingEntry) { delete pendingBuilderActivationRef.current[sessionId]; } } }, - [sessionId, stateSessionId], + [isQueuedAgentBuilderRecordAuthoritative, sessionId, stateSessionId], ); useEffect(() => { if (!queuedAgentBuilderSendNeedsPreparation || !sessionId) { return; } - void ensureCurrentSessionIsAgentBuilder().catch((error) => { - console.error("Failed to prepare queued agent builder:", error); - markAgentBuilderSessionPreparationFailed(sessionId); - }); + const queueRecordId = queuedHead?.recordId; + if (!queueRecordId) { + return; + } + void ensureCurrentSessionIsAgentBuilder({ queueRecordId }).catch( + (error) => { + if (!isQueuedAgentBuilderRecordAuthoritative(queueRecordId)) { + return; + } + console.error("Failed to prepare queued agent builder:", error); + markAgentBuilderSessionPreparationFailed(sessionId); + }, + ); }, [ ensureCurrentSessionIsAgentBuilder, + isQueuedAgentBuilderRecordAuthoritative, queuedAgentBuilderSendNeedsPreparation, + queuedHead?.recordId, sessionId, ]); diff --git a/src/features/chat/lib/__tests__/queuedSessionSend.test.ts b/src/features/chat/lib/__tests__/queuedSessionSend.test.ts index bf4eb7e0..5e3a1e67 100644 --- a/src/features/chat/lib/__tests__/queuedSessionSend.test.ts +++ b/src/features/chat/lib/__tests__/queuedSessionSend.test.ts @@ -46,6 +46,20 @@ function seedSession(creationState?: "pending" | "failed"): void { }); } +function agentBuilderRecord(): QueuedMessageRecord & { + kind: "transport-ready"; +} { + return { + kind: "transport-ready", + recordId: "builder-record", + payload: { + text: "make a reviewer", + persona: { kind: "inherit" }, + sendOptions: { chips: [{ label: "agent-builder", type: "skill" }] }, + }, + }; +} + function queuedRecord(): QueuedMessageRecord & { kind: "transport-ready" } { return { kind: "transport-ready", @@ -194,6 +208,28 @@ describe("sendQueuedPromptToExistingSessionInBackground", () => { mocks.loadSessionMessages.mockResolvedValue(true); }); + it("rejects an Agent Builder send until the session owns a prepared draft target", async () => { + seedSession(); + useChatSessionStore.setState((state) => ({ + sessions: state.sessions.map((session) => ({ + ...session, + intent: "build-agent" as const, + agentBuilderOpen: true, + })), + })); + const beforeUserMessageCommitted = vi.fn(); + + const error = await sendQueuedPromptToExistingSessionInBackground( + SESSION_ID, + agentBuilderRecord(), + beforeUserMessageCommitted, + ).catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(PreCommitSendRejectedError); + expect(mocks.loadSessionMessages).not.toHaveBeenCalled(); + expect(beforeUserMessageCommitted).not.toHaveBeenCalled(); + }); + it("rejects a send to a creating session without committing anything", async () => { seedSession("pending"); const beforeUserMessageCommitted = vi.fn(); diff --git a/src/features/chat/lib/agentBuilderQueueReadiness.ts b/src/features/chat/lib/agentBuilderQueueReadiness.ts new file mode 100644 index 00000000..c13eb18f --- /dev/null +++ b/src/features/chat/lib/agentBuilderQueueReadiness.ts @@ -0,0 +1,32 @@ +import { isAgentBuilderSkillSendOptions } from "@/features/chat/lib/agentBuilderSkill"; +import type { ChatSession } from "@/features/chat/stores/chatSessionStore"; +import type { QueuedMessageRecord } from "@/features/chat/stores/chatStore"; + +/** + * Agent Builder queue records are dispatchable only after a foreground owner + * has created and adopted the final-session-owned draft target. The background + * drain must not bypass that preparation when no chat is mounted. + */ +export function getAgentBuilderQueuePreparedTargetPath( + record: QueuedMessageRecord & { kind: "transport-ready" }, + session: ChatSession | null | undefined, +): string | null | undefined { + if (!isAgentBuilderSkillSendOptions(record.payload.sendOptions)) { + return undefined; + } + if ( + session?.intent !== "build-agent" || + session.agentBuilderOpen === false || + !session.targetAgentPath + ) { + return null; + } + return session.targetAgentPath; +} + +export function isAgentBuilderQueuePreparationReady( + record: QueuedMessageRecord & { kind: "transport-ready" }, + session: ChatSession | null | undefined, +): boolean { + return getAgentBuilderQueuePreparedTargetPath(record, session) !== null; +} diff --git a/src/features/chat/lib/queuedSessionSend.ts b/src/features/chat/lib/queuedSessionSend.ts index 992e12cc..cd8ba53a 100644 --- a/src/features/chat/lib/queuedSessionSend.ts +++ b/src/features/chat/lib/queuedSessionSend.ts @@ -13,11 +13,14 @@ import { import { loadWorkspaceInstructionFiles } from "@/features/chat/api/workspaceContext"; import { sendPromptInBackground } from "@/features/chat/lib/backgroundSend"; +import { composeBuilderSendOptions } from "@/features/chat/hooks/useBuilderSendInterceptor"; +import { getAgentBuilderQueuePreparedTargetPath } from "@/features/chat/lib/agentBuilderQueueReadiness"; import { isFirstCommittedUserMessage } from "@/features/chat/lib/chatFirstMessage"; import { trackChatMessageSent, trackChatSessionStarted, } from "@/features/chat/lib/chatTelemetry"; +import { QueuedSessionNotReadyError } from "@/features/chat/lib/queuedMessageReadiness"; import { loadSessionMessages } from "@/features/chat/lib/sessionActivation"; import { SessionDispatchContentionError, @@ -259,6 +262,22 @@ export async function sendQueuedPromptToExistingSessionInBackground( beforeUserMessageCommitted?: () => void, onPromptDispatched?: () => void, ): Promise { + let preparedBuilderTargetPath: string | undefined; + const assertAgentBuilderPreparationReady = () => { + const targetPath = getAgentBuilderQueuePreparedTargetPath( + queuedMessage, + useChatSessionStore.getState().getSession(sessionId), + ); + if ( + targetPath === null || + (preparedBuilderTargetPath !== undefined && + targetPath !== preparedBuilderTargetPath) + ) { + throw new QueuedSessionNotReadyError(); + } + return targetPath; + }; + assertAgentBuilderPreparationReady(); const acquisition = await acquireExistingSessionForBackgroundSend(sessionId); if (acquisition.status === "contended") { throw new SessionDispatchContentionError(acquisition.waiter); @@ -275,7 +294,6 @@ export async function sendQueuedPromptToExistingSessionInBackground( const targetLease = acquisition; try { const { payload } = queuedMessage; - const sendOptions = payload.sendOptions ?? {}; const payloadPersonaIntent = payload.persona; const payloadPersona = payloadPersonaIntent.kind === "persona" @@ -338,6 +356,14 @@ export async function sendQueuedPromptToExistingSessionInBackground( formatAvailableSkillsCatalogPrompt(skills), ) : undefined; + const sessionBeforeSend = useChatSessionStore + .getState() + .getSession(sessionId); + preparedBuilderTargetPath = assertAgentBuilderPreparationReady(); + const sendOptions = composeBuilderSendOptions( + sessionBeforeSend, + payload.sendOptions ?? {}, + ); const personaSystemPrompt = sendOptions.capturedPersonaSystemPrompt ?? formatPersonaSystemPrompt(persona); @@ -408,7 +434,10 @@ export async function sendQueuedPromptToExistingSessionInBackground( executionSystemPrompt, }, payload.attachments, - beforeUserMessageCommitted, + () => { + assertAgentBuilderPreparationReady(); + beforeUserMessageCommitted?.(); + }, fireSendTelemetry, () => assertSessionExecutionTarget(sessionId, preparedExecutionTarget), onPromptDispatched,