mirror of
https://github.com/nexu-io/open-design.git
synced 2026-06-01 03:14:35 +07:00
Fix conversation run isolation (#1271)
This commit is contained in:
parent
3524a43d18
commit
87a95b7fb4
9 changed files with 988 additions and 47 deletions
|
|
@ -4,6 +4,18 @@ import { describe, expect, it, vi } from 'vitest';
|
|||
import { createChatRunService } from '../src/runs.js';
|
||||
|
||||
describe('chat run service shutdown', () => {
|
||||
it('filters active runs by conversation within the same project', () => {
|
||||
const runs = createRuns();
|
||||
const runA = runs.create({ projectId: 'project-1', conversationId: 'conv-a' });
|
||||
const runB = runs.create({ projectId: 'project-1', conversationId: 'conv-b' });
|
||||
runA.status = 'running';
|
||||
runB.status = 'running';
|
||||
|
||||
expect(
|
||||
runs.list({ projectId: 'project-1', conversationId: 'conv-b', status: 'active' }),
|
||||
).toEqual([runB]);
|
||||
});
|
||||
|
||||
it('cancels active runs and terminates their child process during daemon shutdown', async () => {
|
||||
const runs = createRuns();
|
||||
const child = new FakeChildProcess({ closeOn: 'SIGTERM' });
|
||||
|
|
|
|||
|
|
@ -40,6 +40,7 @@ interface Props {
|
|||
projectId: string | null;
|
||||
projectFiles: ProjectFile[];
|
||||
streaming: boolean;
|
||||
sendDisabled?: boolean;
|
||||
initialDraft?: string;
|
||||
// Lazy ensure — the composer calls this before its first upload, so the
|
||||
// project folder exists on disk before files land in it. Returns the
|
||||
|
|
@ -111,6 +112,7 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
projectId,
|
||||
projectFiles,
|
||||
streaming,
|
||||
sendDisabled = false,
|
||||
initialDraft,
|
||||
onEnsureProject,
|
||||
commentAttachments = [],
|
||||
|
|
@ -670,6 +672,7 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
|
||||
async function submit() {
|
||||
const prompt = draft.trim();
|
||||
if (sendDisabled) return;
|
||||
// Intercept `/pet …` and `/mcp` before sending so the slash command
|
||||
// never hits the agent — these are local UX hooks, not model prompts.
|
||||
if (tryHandlePetSlash()) return;
|
||||
|
|
@ -1031,7 +1034,7 @@ export const ChatComposer = forwardRef<ChatComposerHandle, Props>(
|
|||
className="composer-send"
|
||||
data-testid="chat-send"
|
||||
onClick={() => void submit()}
|
||||
disabled={!draft.trim() && commentAttachments.length === 0}
|
||||
disabled={sendDisabled || (!draft.trim() && commentAttachments.length === 0)}
|
||||
>
|
||||
<Icon name="send" size={13} />
|
||||
<span>{t('chat.send')}</span>
|
||||
|
|
|
|||
|
|
@ -199,6 +199,7 @@ interface Props {
|
|||
error: string | null;
|
||||
projectId: string | null;
|
||||
projectFiles: ProjectFile[];
|
||||
sendDisabled?: boolean;
|
||||
// Names that exist in the project folder. Tool cards and chips use this
|
||||
// set to decide whether a path can be opened as a tab.
|
||||
projectFileNames?: Set<string>;
|
||||
|
|
@ -229,6 +230,7 @@ interface Props {
|
|||
onContinueRemainingTasks?: (assistantMessage: ChatMessage, todos: TodoItem[]) => void;
|
||||
// Header "+" button — kicks off ProjectView's create-conversation flow.
|
||||
onNewConversation?: () => void;
|
||||
newConversationDisabled?: boolean;
|
||||
// Conversation list that used to live in the topbar. The chat tab now
|
||||
// owns the list so users can browse + switch conversations without
|
||||
// leaving the pane.
|
||||
|
|
@ -260,6 +262,7 @@ type Tab = 'chat' | 'comments';
|
|||
export function ChatPane({
|
||||
messages,
|
||||
streaming,
|
||||
sendDisabled = false,
|
||||
error,
|
||||
projectId,
|
||||
projectFiles,
|
||||
|
|
@ -277,6 +280,7 @@ export function ChatPane({
|
|||
onSubmitForm,
|
||||
onContinueRemainingTasks,
|
||||
onNewConversation,
|
||||
newConversationDisabled = false,
|
||||
conversations,
|
||||
activeConversationId,
|
||||
onSelectConversation,
|
||||
|
|
@ -569,7 +573,9 @@ export function ChatPane({
|
|||
type="button"
|
||||
className="chat-history-new"
|
||||
data-testid="conversation-history-new"
|
||||
disabled={newConversationDisabled}
|
||||
onClick={() => {
|
||||
if (newConversationDisabled) return;
|
||||
onNewConversation();
|
||||
setShowConvList(false);
|
||||
}}
|
||||
|
|
@ -611,7 +617,7 @@ export function ChatPane({
|
|||
title={t('chat.newConversationsTitle')}
|
||||
aria-label={t('chat.newConversation')}
|
||||
onClick={onNewConversation}
|
||||
disabled={!onNewConversation}
|
||||
disabled={!onNewConversation || newConversationDisabled}
|
||||
>
|
||||
<Icon name="plus" size={16} />
|
||||
</button>
|
||||
|
|
@ -731,7 +737,8 @@ export function ChatPane({
|
|||
projectId={projectId}
|
||||
projectFiles={projectFiles}
|
||||
skills={skills}
|
||||
streaming={streaming || hasActiveRunMessage}
|
||||
streaming={streaming}
|
||||
sendDisabled={sendDisabled}
|
||||
initialDraft={initialDraft}
|
||||
onEnsureProject={onEnsureProject}
|
||||
commentAttachments={commentsToAttachments(attachedComments)}
|
||||
|
|
|
|||
|
|
@ -268,7 +268,10 @@ export function ProjectView({
|
|||
const [activeConversationId, setActiveConversationId] = useState<string | null>(
|
||||
null,
|
||||
);
|
||||
const [messagesConversationId, setMessagesConversationId] = useState<string | null>(null);
|
||||
const [failedMessagesConversationId, setFailedMessagesConversationId] = useState<string | null>(null);
|
||||
const [conversationLoadError, setConversationLoadError] = useState<string | null>(null);
|
||||
const [messageLoadRetryNonce, setMessageLoadRetryNonce] = useState(0);
|
||||
const [messages, setMessages] = useState<ChatMessage[]>([]);
|
||||
const [previewComments, setPreviewComments] = useState<PreviewComment[]>([]);
|
||||
const [attachedComments, setAttachedComments] = useState<PreviewComment[]>([]);
|
||||
|
|
@ -353,6 +356,28 @@ export function ProjectView({
|
|||
// Track which conversation the current messages belong to, so we can
|
||||
// correctly gate new-conversation creation even during async loads.
|
||||
const messagesConversationIdRef = useRef<string | null>(null);
|
||||
const creatingConversationRef = useRef(false);
|
||||
const [creatingConversation, setCreatingConversation] = useState(false);
|
||||
const currentConversationHasActiveRun = useMemo(
|
||||
() => messages.some((m) => m.role === 'assistant' && isActiveRunStatus(m.runStatus)),
|
||||
[messages],
|
||||
);
|
||||
const currentConversationLoading = Boolean(
|
||||
activeConversationId
|
||||
&& messagesConversationId !== activeConversationId
|
||||
&& failedMessagesConversationId !== activeConversationId,
|
||||
);
|
||||
const currentConversationStreaming = streaming;
|
||||
const currentConversationBusy = currentConversationLoading
|
||||
|| currentConversationStreaming
|
||||
|| currentConversationHasActiveRun;
|
||||
const currentConversationSendDisabled = currentConversationLoading
|
||||
|| currentConversationHasActiveRun
|
||||
|| failedMessagesConversationId === activeConversationId;
|
||||
const currentConversationActionDisabled = currentConversationBusy || currentConversationSendDisabled;
|
||||
const newConversationDisabled = creatingConversation;
|
||||
const activeCompletionNotificationRunsRef = useRef<Set<string>>(new Set());
|
||||
const completedNotificationRunsRef = useRef<Set<string>>(new Set());
|
||||
|
||||
// Load conversations on project switch. If none exist (older projects
|
||||
// pre-conversations, or a freshly created one whose default seed got
|
||||
|
|
@ -361,6 +386,9 @@ export function ProjectView({
|
|||
let cancelled = false;
|
||||
setConversations([]);
|
||||
setActiveConversationId(null);
|
||||
setMessagesConversationId(null);
|
||||
setFailedMessagesConversationId(null);
|
||||
setMessageLoadRetryNonce(0);
|
||||
setConversationLoadError(null);
|
||||
setMessages([]);
|
||||
setPreviewComments([]);
|
||||
|
|
@ -413,29 +441,61 @@ export function ProjectView({
|
|||
setMessages([]);
|
||||
setPreviewComments([]);
|
||||
setAttachedComments([]);
|
||||
setMessagesConversationId(null);
|
||||
setFailedMessagesConversationId(null);
|
||||
messagesConversationIdRef.current = null;
|
||||
setStreaming(false);
|
||||
return;
|
||||
}
|
||||
let cancelled = false;
|
||||
setMessages([]);
|
||||
setPreviewComments([]);
|
||||
setAttachedComments([]);
|
||||
setArtifact(null);
|
||||
setMessagesConversationId(null);
|
||||
setFailedMessagesConversationId(null);
|
||||
setStreaming(false);
|
||||
savedArtifactRef.current = null;
|
||||
pendingWritesRef.current.clear();
|
||||
if (messagesConversationIdRef.current !== activeConversationId) {
|
||||
messagesConversationIdRef.current = null;
|
||||
}
|
||||
(async () => {
|
||||
const [list, comments] = await Promise.all([
|
||||
listMessages(project.id, activeConversationId),
|
||||
fetchPreviewComments(project.id, activeConversationId),
|
||||
]);
|
||||
if (cancelled) return;
|
||||
setMessages(list);
|
||||
setPreviewComments(comments);
|
||||
setAttachedComments([]);
|
||||
setArtifact(null);
|
||||
setError(null);
|
||||
savedArtifactRef.current = null;
|
||||
pendingWritesRef.current.clear();
|
||||
messagesConversationIdRef.current = activeConversationId;
|
||||
try {
|
||||
const [list, comments] = await Promise.all([
|
||||
listMessages(project.id, activeConversationId),
|
||||
fetchPreviewComments(project.id, activeConversationId),
|
||||
]);
|
||||
if (cancelled) return;
|
||||
setMessages(list);
|
||||
setPreviewComments(comments);
|
||||
setAttachedComments([]);
|
||||
setArtifact(null);
|
||||
setError(null);
|
||||
savedArtifactRef.current = null;
|
||||
pendingWritesRef.current.clear();
|
||||
messagesConversationIdRef.current = activeConversationId;
|
||||
setMessagesConversationId(activeConversationId);
|
||||
setFailedMessagesConversationId(null);
|
||||
} catch (err) {
|
||||
if (cancelled) return;
|
||||
const message = err instanceof Error ? err.message : 'Could not load messages for this conversation.';
|
||||
setMessages([]);
|
||||
setPreviewComments([]);
|
||||
setAttachedComments([]);
|
||||
setArtifact(null);
|
||||
setError(message);
|
||||
savedArtifactRef.current = null;
|
||||
pendingWritesRef.current.clear();
|
||||
messagesConversationIdRef.current = null;
|
||||
setMessagesConversationId(null);
|
||||
setFailedMessagesConversationId(activeConversationId);
|
||||
}
|
||||
})();
|
||||
return () => {
|
||||
cancelled = true;
|
||||
};
|
||||
}, [project.id, activeConversationId]);
|
||||
}, [project.id, activeConversationId, messageLoadRetryNonce]);
|
||||
|
||||
useEffect(() => {
|
||||
return () => {
|
||||
|
|
@ -478,27 +538,13 @@ export function ProjectView({
|
|||
reattachTextBuffersRef.current.clear();
|
||||
}, []);
|
||||
|
||||
// Detect the streaming `true → false` edge so we can fire the optional
|
||||
// completion sound / desktop notification exactly once per turn. Initial
|
||||
// mount keeps `prevStreamingRef.current = false`, so loading historical
|
||||
// conversations (where `streaming` is also false) never triggers a stray
|
||||
// ding. `messages` is on the dep array so the latest assistant message's
|
||||
// runStatus is visible at the moment we edge-detect; the early-return
|
||||
// guarantees only the edge actually does anything.
|
||||
const prevStreamingRef = useRef(false);
|
||||
useEffect(() => {
|
||||
const wasStreaming = prevStreamingRef.current;
|
||||
prevStreamingRef.current = streaming;
|
||||
if (!(wasStreaming && !streaming)) return;
|
||||
|
||||
const notifyCompletedRun = useCallback((last: ChatMessage) => {
|
||||
// Round 7 (mrcfps @ useDesignMdState.ts:131): a chat turn just
|
||||
// settled — conversation updatedAt almost certainly moved, so
|
||||
// recompute DESIGN.md staleness even when the turn produced no
|
||||
// file mutations or live artifacts.
|
||||
setDesignMdRefreshKey((n) => n + 1);
|
||||
|
||||
const last = [...messages].reverse().find((m) => m.role === 'assistant');
|
||||
if (!last) return;
|
||||
const status = last.runStatus;
|
||||
if (status !== 'succeeded' && status !== 'failed') return;
|
||||
|
||||
|
|
@ -532,7 +578,30 @@ export function ProjectView({
|
|||
});
|
||||
}
|
||||
}
|
||||
}, [streaming, messages, config.notifications, t]);
|
||||
}, [config.notifications, t]);
|
||||
|
||||
// Fire completion feedback from assistant run-status transitions rather than
|
||||
// from the local SSE listener state. A run can finish while its conversation
|
||||
// is detached; when the user returns, the terminal status should still produce
|
||||
// the one completion notification for runs this view previously saw active.
|
||||
useEffect(() => {
|
||||
const completedMessages: ChatMessage[] = [];
|
||||
for (const message of messages) {
|
||||
if (message.role !== 'assistant') continue;
|
||||
const keys = message.runId ? [message.runId, message.id] : [message.id];
|
||||
if (isActiveRunStatus(message.runStatus)) {
|
||||
for (const key of keys) activeCompletionNotificationRunsRef.current.add(key);
|
||||
continue;
|
||||
}
|
||||
if (message.runStatus !== 'succeeded' && message.runStatus !== 'failed') continue;
|
||||
if (!keys.some((key) => activeCompletionNotificationRunsRef.current.has(key))) continue;
|
||||
if (keys.some((key) => completedNotificationRunsRef.current.has(key))) continue;
|
||||
for (const key of keys) completedNotificationRunsRef.current.add(key);
|
||||
completedMessages.push(message);
|
||||
}
|
||||
|
||||
for (const message of completedMessages) notifyCompletedRun(message);
|
||||
}, [messages, notifyCompletedRun]);
|
||||
|
||||
// Hydrate the open-tabs state once per project. After this initial
|
||||
// load, every mutation flows through saveTabsState() which keeps DB +
|
||||
|
|
@ -1098,7 +1167,8 @@ export function ProjectView({
|
|||
meta?: { research?: ResearchOptions; skillIds?: string[] },
|
||||
) => {
|
||||
if (!activeConversationId) return;
|
||||
if (streaming) return;
|
||||
if (messagesConversationIdRef.current !== activeConversationId) return;
|
||||
if (currentConversationBusy) return;
|
||||
if (!prompt.trim() && attachments.length === 0 && commentAttachments.length === 0) return;
|
||||
setError(null);
|
||||
const startedAt = Date.now();
|
||||
|
|
@ -1142,6 +1212,7 @@ export function ProjectView({
|
|||
runStatus: config.mode === 'daemon' ? 'running' : undefined,
|
||||
startedAt,
|
||||
};
|
||||
activeCompletionNotificationRunsRef.current.add(assistantId);
|
||||
const nextHistory = [...messages, userMsg];
|
||||
setMessages([...nextHistory, assistantMsg]);
|
||||
setStreaming(true);
|
||||
|
|
@ -1315,6 +1386,7 @@ export function ProjectView({
|
|||
(prev) => ({
|
||||
...prev,
|
||||
endedAt: Date.now(),
|
||||
runStatus: 'failed',
|
||||
events: [
|
||||
...(prev.events ?? []),
|
||||
{ kind: 'status', label: 'empty_response', detail: config.model },
|
||||
|
|
@ -1530,7 +1602,7 @@ export function ProjectView({
|
|||
[
|
||||
attachedComments,
|
||||
activeConversationId,
|
||||
streaming,
|
||||
currentConversationBusy,
|
||||
messages,
|
||||
config,
|
||||
agentsById,
|
||||
|
|
@ -1551,10 +1623,10 @@ export function ProjectView({
|
|||
|
||||
const handleSendBoardCommentAttachments = useCallback(
|
||||
async (commentAttachments: ChatCommentAttachment[]) => {
|
||||
if (streaming || commentAttachments.length === 0) return;
|
||||
if (currentConversationActionDisabled || commentAttachments.length === 0) return;
|
||||
await handleSend('', [], commentAttachments);
|
||||
},
|
||||
[handleSend, streaming],
|
||||
[handleSend, currentConversationActionDisabled],
|
||||
);
|
||||
|
||||
const persistArtifact = useCallback(
|
||||
|
|
@ -1653,7 +1725,7 @@ export function ProjectView({
|
|||
|
||||
const handleContinueRemainingTasks = useCallback(
|
||||
(_assistantMessage: ChatMessage, todos: TodoItem[]) => {
|
||||
if (streaming || todos.length === 0) return;
|
||||
if (currentConversationActionDisabled || todos.length === 0) return;
|
||||
const remainingList = todos
|
||||
.map((todo, i) => {
|
||||
const label =
|
||||
|
|
@ -1669,12 +1741,12 @@ export function ProjectView({
|
|||
'Update TodoWrite as you complete each remaining task.';
|
||||
void handleSend(prompt, [], []);
|
||||
},
|
||||
[streaming, handleSend],
|
||||
[currentConversationActionDisabled, handleSend],
|
||||
);
|
||||
|
||||
const handleExportAsPptx = useCallback(
|
||||
(fileName: string) => {
|
||||
if (streaming) return;
|
||||
if (currentConversationActionDisabled) return;
|
||||
const baseTitle = fileName.replace(/\.html?$/i, '') || fileName;
|
||||
const prompt =
|
||||
`Export @${fileName} as an editable PPTX file titled "${baseTitle}".\n\n` +
|
||||
|
|
@ -1712,7 +1784,7 @@ export function ProjectView({
|
|||
};
|
||||
void handleSend(prompt, [attachment], []);
|
||||
},
|
||||
[streaming, handleSend],
|
||||
[currentConversationActionDisabled, handleSend],
|
||||
);
|
||||
|
||||
const handleStop = useCallback(() => {
|
||||
|
|
@ -1754,6 +1826,7 @@ export function ProjectView({
|
|||
}, [cancelSendTextBuffer, cancelReattachTextBuffers, persistMessage]);
|
||||
|
||||
const handleNewConversation = useCallback(async () => {
|
||||
if (creatingConversationRef.current) return;
|
||||
// Only block if we're sure the current conversation is empty:
|
||||
// messages must be loaded AND match the active conversation.
|
||||
if (
|
||||
|
|
@ -1762,6 +1835,8 @@ export function ProjectView({
|
|||
) {
|
||||
return;
|
||||
}
|
||||
creatingConversationRef.current = true;
|
||||
setCreatingConversation(true);
|
||||
setConversationLoadError(null);
|
||||
try {
|
||||
const fresh = await createConversation(project.id);
|
||||
|
|
@ -1769,6 +1844,8 @@ export function ProjectView({
|
|||
// Eagerly clear messages and update ref so rapid clicks don't create
|
||||
// duplicate empty conversations before the effect resolves.
|
||||
setMessages([]);
|
||||
setStreaming(false);
|
||||
setMessagesConversationId(null);
|
||||
messagesConversationIdRef.current = fresh.id;
|
||||
setConversations((curr) => [fresh, ...curr]);
|
||||
setActiveConversationId(fresh.id);
|
||||
|
|
@ -1777,12 +1854,26 @@ export function ProjectView({
|
|||
const message = err instanceof Error ? err.message : 'Could not create a conversation for this project.';
|
||||
setConversationLoadError(message);
|
||||
setError(message);
|
||||
} finally {
|
||||
creatingConversationRef.current = false;
|
||||
setCreatingConversation(false);
|
||||
}
|
||||
}, [project.id, activeConversationId, messages.length]);
|
||||
|
||||
const handleSelectConversation = useCallback((id: string) => {
|
||||
if (id === activeConversationId && failedMessagesConversationId !== id) return;
|
||||
setMessages([]);
|
||||
setPreviewComments([]);
|
||||
setAttachedComments([]);
|
||||
setArtifact(null);
|
||||
setStreaming(false);
|
||||
setMessagesConversationId(null);
|
||||
setFailedMessagesConversationId(null);
|
||||
setConversationLoadError(null);
|
||||
messagesConversationIdRef.current = null;
|
||||
setActiveConversationId(id);
|
||||
}, []);
|
||||
setMessageLoadRetryNonce((nonce) => nonce + 1);
|
||||
}, [activeConversationId, failedMessagesConversationId]);
|
||||
|
||||
const handleDeleteConversation = useCallback(
|
||||
async (id: string) => {
|
||||
|
|
@ -2223,7 +2314,8 @@ export function ProjectView({
|
|||
// resets internal scroll/draft state inside ChatPane and ChatComposer.
|
||||
key={`${project.id}:${activeConversationId ?? 'conversation-unavailable'}`}
|
||||
messages={messages}
|
||||
streaming={streaming}
|
||||
streaming={currentConversationStreaming}
|
||||
sendDisabled={currentConversationSendDisabled}
|
||||
error={conversationLoadError ?? error}
|
||||
projectId={project.id}
|
||||
projectFiles={projectFiles}
|
||||
|
|
@ -2240,11 +2332,12 @@ export function ProjectView({
|
|||
onRequestOpenFile={requestOpenFile}
|
||||
initialDraft={chatInitialDraft}
|
||||
onSubmitForm={(text) => {
|
||||
if (streaming) return;
|
||||
if (currentConversationActionDisabled) return;
|
||||
void handleSend(text, [], []);
|
||||
}}
|
||||
onContinueRemainingTasks={handleContinueRemainingTasks}
|
||||
onNewConversation={handleNewConversation}
|
||||
newConversationDisabled={newConversationDisabled}
|
||||
conversations={conversations}
|
||||
activeConversationId={activeConversationId}
|
||||
onSelectConversation={handleSelectConversation}
|
||||
|
|
@ -2294,7 +2387,7 @@ export function ProjectView({
|
|||
}}
|
||||
isDeck={isDeck}
|
||||
onExportAsPptx={handleExportAsPptx}
|
||||
streaming={streaming}
|
||||
streaming={currentConversationActionDisabled}
|
||||
openRequest={openRequest}
|
||||
liveArtifactEvents={liveArtifactEvents}
|
||||
tabsState={openTabsState}
|
||||
|
|
|
|||
|
|
@ -149,4 +149,28 @@ describe('ChatComposer /search command', () => {
|
|||
expect(commentAttachments).toEqual([]);
|
||||
expect(meta).toBeUndefined();
|
||||
});
|
||||
|
||||
it('keeps keyboard submits blocked when sending is disabled', () => {
|
||||
const onSend = vi.fn();
|
||||
|
||||
render(
|
||||
<ChatComposer
|
||||
projectId="project-1"
|
||||
projectFiles={[]}
|
||||
streaming={false}
|
||||
sendDisabled
|
||||
researchAvailable
|
||||
onEnsureProject={async () => 'project-1'}
|
||||
onSend={onSend}
|
||||
onStop={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
|
||||
const input = screen.getByTestId('chat-composer-input');
|
||||
fireEvent.change(input, { target: { value: 'keep this draft' } });
|
||||
fireEvent.keyDown(input, { key: 'Enter', metaKey: true });
|
||||
|
||||
expect(onSend).not.toHaveBeenCalled();
|
||||
expect((input as HTMLTextAreaElement).value).toBe('keep this draft');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
78
apps/web/tests/components/ChatPane.streaming.test.tsx
Normal file
78
apps/web/tests/components/ChatPane.streaming.test.tsx
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { cleanup, render, screen } from '@testing-library/react';
|
||||
import { forwardRef } from 'react';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { ChatPane } from '../../src/components/ChatPane';
|
||||
import type { ChatMessage, Conversation, ProjectMetadata } from '../../src/types';
|
||||
|
||||
vi.mock('../../src/i18n', () => ({
|
||||
useT: () => (key: string) => key,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/AssistantMessage', () => ({
|
||||
AssistantMessage: ({ streaming, message }: { streaming: boolean; message: ChatMessage }) => (
|
||||
<output data-testid={`assistant-streaming-${message.id}`}>{streaming ? 'streaming' : 'idle'}</output>
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/ChatComposer', () => ({
|
||||
ChatComposer: forwardRef(({ streaming }: { streaming: boolean }, _ref) => (
|
||||
<output data-testid="composer-streaming">{streaming ? 'streaming' : 'idle'}</output>
|
||||
)),
|
||||
}));
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
});
|
||||
|
||||
describe('ChatPane streaming state', () => {
|
||||
it('keeps composer idle while active-run messages still render as streaming', () => {
|
||||
const messages: ChatMessage[] = [
|
||||
{
|
||||
id: 'assistant-1',
|
||||
role: 'assistant',
|
||||
content: 'still running',
|
||||
createdAt: 1,
|
||||
runId: 'run-1',
|
||||
runStatus: 'running',
|
||||
},
|
||||
];
|
||||
|
||||
render(
|
||||
<ChatPane
|
||||
messages={messages}
|
||||
streaming={false}
|
||||
error={null}
|
||||
projectId="project-1"
|
||||
projectFiles={[]}
|
||||
onEnsureProject={async () => 'project-1'}
|
||||
onSend={vi.fn()}
|
||||
onStop={vi.fn()}
|
||||
conversations={conversations}
|
||||
activeConversationId="conv-1"
|
||||
onSelectConversation={vi.fn()}
|
||||
onDeleteConversation={vi.fn()}
|
||||
projectMetadata={projectMetadata}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId('composer-streaming').textContent).toBe('idle');
|
||||
expect(screen.getByTestId('assistant-streaming-assistant-1').textContent).toBe('streaming');
|
||||
});
|
||||
});
|
||||
|
||||
const conversations: Conversation[] = [
|
||||
{
|
||||
id: 'conv-1',
|
||||
projectId: 'project-1',
|
||||
title: 'Conversation 1',
|
||||
createdAt: 1,
|
||||
updatedAt: 1,
|
||||
},
|
||||
];
|
||||
|
||||
const projectMetadata: ProjectMetadata = {
|
||||
kind: 'prototype',
|
||||
};
|
||||
|
|
@ -9,6 +9,7 @@ import { streamMessage } from '../../src/providers/anthropic';
|
|||
import type { StreamHandlers } from '../../src/providers/anthropic';
|
||||
import { patchPreviewCommentStatus, writeProjectTextFile } from '../../src/providers/registry';
|
||||
import { listMessages, saveMessage } from '../../src/state/projects';
|
||||
import { playSound } from '../../src/utils/notifications';
|
||||
import type {
|
||||
AgentEvent,
|
||||
AgentInfo,
|
||||
|
|
@ -45,6 +46,16 @@ vi.mock('../../src/providers/project-events', () => ({
|
|||
useProjectFileEvents: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/utils/notifications', async () => {
|
||||
const actual = await vi.importActual<typeof import('../../src/utils/notifications')>(
|
||||
'../../src/utils/notifications',
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
playSound: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock('../../src/providers/registry', async () => {
|
||||
const actual = await vi.importActual<typeof import('../../src/providers/registry')>(
|
||||
'../../src/providers/registry',
|
||||
|
|
@ -142,6 +153,7 @@ const mockedListMessages = vi.mocked(listMessages);
|
|||
const mockedSaveMessage = vi.mocked(saveMessage);
|
||||
const mockedWriteProjectTextFile = vi.mocked(writeProjectTextFile);
|
||||
const mockedPatchPreviewCommentStatus = vi.mocked(patchPreviewCommentStatus);
|
||||
const mockedPlaySound = vi.mocked(playSound);
|
||||
|
||||
const config: AppConfig = {
|
||||
mode: 'api',
|
||||
|
|
@ -152,6 +164,12 @@ const config: AppConfig = {
|
|||
agentId: null,
|
||||
skillId: null,
|
||||
designSystemId: null,
|
||||
notifications: {
|
||||
soundEnabled: true,
|
||||
successSoundId: 'success-sound',
|
||||
failureSoundId: 'failure-sound',
|
||||
desktopEnabled: false,
|
||||
},
|
||||
};
|
||||
|
||||
const project: Project = {
|
||||
|
|
@ -196,6 +214,7 @@ describe('ProjectView API empty response handling', () => {
|
|||
mockedSaveMessage.mockClear();
|
||||
mockedWriteProjectTextFile.mockClear();
|
||||
mockedPatchPreviewCommentStatus.mockClear();
|
||||
mockedPlaySound.mockClear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
@ -229,7 +248,7 @@ describe('ProjectView API empty response handling', () => {
|
|||
const message = call[2] as ChatMessage;
|
||||
return (
|
||||
message.role === 'assistant' &&
|
||||
message.runStatus === undefined &&
|
||||
message.runStatus === 'failed' &&
|
||||
message.events?.some(
|
||||
(event: AgentEvent) => event.kind === 'status' && event.label === 'empty_response',
|
||||
)
|
||||
|
|
@ -237,6 +256,7 @@ describe('ProjectView API empty response handling', () => {
|
|||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
expect(mockedPlaySound).toHaveBeenCalledWith('failure-sound');
|
||||
});
|
||||
|
||||
it('marks attached saved comments as failed when an API completion has no output', async () => {
|
||||
|
|
@ -278,7 +298,7 @@ describe('ProjectView API empty response handling', () => {
|
|||
});
|
||||
await waitFor(() => {
|
||||
expect(hasSavedAssistantMessage((message) => (
|
||||
message.runStatus === undefined &&
|
||||
message.runStatus === 'failed' &&
|
||||
message.events?.some((event) => event.kind === 'status' && event.label === 'empty_response') === true
|
||||
))).toBe(true);
|
||||
});
|
||||
|
|
@ -306,6 +326,27 @@ describe('ProjectView API empty response handling', () => {
|
|||
expect(screen.queryByText(/provider ended the request/i)).toBeNull();
|
||||
});
|
||||
|
||||
it('plays the success sound for API completions that become succeeded after starting without runStatus', async () => {
|
||||
mockedStreamMessage.mockImplementation(async (
|
||||
_cfg: AppConfig,
|
||||
_system: string,
|
||||
_history: ChatMessage[],
|
||||
_signal: AbortSignal,
|
||||
handlers: StreamHandlers,
|
||||
) => {
|
||||
handlers.onDelta('hello');
|
||||
handlers.onDone('hello');
|
||||
});
|
||||
renderProjectView();
|
||||
|
||||
await sendTestPrompt();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(hasSavedAssistantMessage((message) => message.runStatus === 'succeeded')).toBe(true);
|
||||
});
|
||||
await waitFor(() => expect(mockedPlaySound).toHaveBeenCalledWith('success-sound'));
|
||||
});
|
||||
|
||||
it('keeps API artifact completions on the succeeded path even when done text is empty', async () => {
|
||||
const artifact =
|
||||
'<artifact identifier="landing-page" type="text/html" title="Landing Page">' +
|
||||
|
|
|
|||
475
apps/web/tests/components/ProjectView.run-isolation.test.tsx
Normal file
475
apps/web/tests/components/ProjectView.run-isolation.test.tsx
Normal file
|
|
@ -0,0 +1,475 @@
|
|||
// @vitest-environment jsdom
|
||||
|
||||
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import type { ReactNode } from 'react';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { ProjectView } from '../../src/components/ProjectView';
|
||||
import type { AppConfig, ChatMessage, Conversation, Project } from '../../src/types';
|
||||
|
||||
const listConversations = vi.fn();
|
||||
const listMessages = vi.fn();
|
||||
const fetchPreviewComments = vi.fn();
|
||||
const loadTabs = vi.fn();
|
||||
const fetchProjectFiles = vi.fn();
|
||||
const fetchLiveArtifacts = vi.fn();
|
||||
const fetchSkill = vi.fn();
|
||||
const fetchDesignSystem = vi.fn();
|
||||
const getTemplate = vi.fn();
|
||||
const fetchChatRunStatus = vi.fn();
|
||||
const listActiveChatRuns = vi.fn();
|
||||
const reattachDaemonRun = vi.fn();
|
||||
const streamViaDaemon = vi.fn();
|
||||
const streamMessage = vi.fn();
|
||||
const saveMessage = vi.fn();
|
||||
const createConversation = vi.fn();
|
||||
const patchConversation = vi.fn();
|
||||
const patchProject = vi.fn();
|
||||
const saveTabs = vi.fn();
|
||||
const playSound = vi.fn();
|
||||
const showCompletionNotification = vi.fn();
|
||||
|
||||
vi.mock('../../src/i18n', () => ({
|
||||
useT: () => (key: string) => key,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/providers/anthropic', () => ({
|
||||
streamMessage: (...args: unknown[]) => streamMessage(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/providers/daemon', () => ({
|
||||
fetchChatRunStatus: (...args: unknown[]) => fetchChatRunStatus(...args),
|
||||
listActiveChatRuns: (...args: unknown[]) => listActiveChatRuns(...args),
|
||||
reattachDaemonRun: (...args: unknown[]) => reattachDaemonRun(...args),
|
||||
streamViaDaemon: (...args: unknown[]) => streamViaDaemon(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/providers/project-events', () => ({
|
||||
useProjectFileEvents: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/utils/notifications', async (importOriginal) => ({
|
||||
...(await importOriginal<typeof import('../../src/utils/notifications')>()),
|
||||
playSound: (...args: unknown[]) => playSound(...args),
|
||||
showCompletionNotification: (...args: unknown[]) => showCompletionNotification(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/providers/registry', () => ({
|
||||
deletePreviewComment: vi.fn(),
|
||||
fetchPreviewComments: (...args: unknown[]) => fetchPreviewComments(...args),
|
||||
fetchDesignSystem: (...args: unknown[]) => fetchDesignSystem(...args),
|
||||
fetchLiveArtifacts: (...args: unknown[]) => fetchLiveArtifacts(...args),
|
||||
fetchProjectFiles: (...args: unknown[]) => fetchProjectFiles(...args),
|
||||
fetchSkill: (...args: unknown[]) => fetchSkill(...args),
|
||||
patchPreviewCommentStatus: vi.fn(),
|
||||
upsertPreviewComment: vi.fn(),
|
||||
writeProjectTextFile: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/router', () => ({
|
||||
navigate: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/state/projects', () => ({
|
||||
createConversation: (...args: unknown[]) => createConversation(...args),
|
||||
deleteConversation: vi.fn(),
|
||||
getTemplate: (...args: unknown[]) => getTemplate(...args),
|
||||
listConversations: (...args: unknown[]) => listConversations(...args),
|
||||
listMessages: (...args: unknown[]) => listMessages(...args),
|
||||
loadTabs: (...args: unknown[]) => loadTabs(...args),
|
||||
patchConversation: (...args: unknown[]) => patchConversation(...args),
|
||||
patchProject: (...args: unknown[]) => patchProject(...args),
|
||||
saveMessage: (...args: unknown[]) => saveMessage(...args),
|
||||
saveTabs: (...args: unknown[]) => saveTabs(...args),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/AppChromeHeader', () => ({
|
||||
AppChromeHeader: ({ children }: { children: ReactNode }) => <header>{children}</header>,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/AvatarMenu', () => ({
|
||||
AvatarMenu: () => null,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/FileWorkspace', () => ({
|
||||
FileWorkspace: ({
|
||||
streaming,
|
||||
onSendBoardCommentAttachments,
|
||||
}: {
|
||||
streaming: boolean;
|
||||
onSendBoardCommentAttachments: (attachments: unknown[]) => void;
|
||||
}) => (
|
||||
<>
|
||||
<output data-testid="workspace-streaming-state">{streaming ? 'streaming' : 'idle'}</output>
|
||||
<button
|
||||
type="button"
|
||||
data-testid="workspace-send-comment"
|
||||
onClick={() => onSendBoardCommentAttachments([{ id: 'comment-1' }])}
|
||||
>
|
||||
workspace send
|
||||
</button>
|
||||
</>
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/Loading', () => ({
|
||||
CenteredLoader: () => null,
|
||||
}));
|
||||
|
||||
vi.mock('../../src/components/ChatPane', () => ({
|
||||
ChatPane: ({
|
||||
activeConversationId,
|
||||
conversations,
|
||||
streaming,
|
||||
sendDisabled,
|
||||
onSelectConversation,
|
||||
onSend,
|
||||
onNewConversation,
|
||||
error,
|
||||
}: {
|
||||
activeConversationId: string | null;
|
||||
conversations: Conversation[];
|
||||
streaming: boolean;
|
||||
sendDisabled?: boolean;
|
||||
error: string | null;
|
||||
onSelectConversation: (id: string) => void;
|
||||
onSend: (prompt: string, attachments: unknown[], commentAttachments: unknown[]) => void;
|
||||
onNewConversation: () => void;
|
||||
}) => (
|
||||
<section>
|
||||
<output data-testid="active-conversation">{activeConversationId}</output>
|
||||
<output data-testid="streaming-state">{streaming ? 'streaming' : 'idle'}</output>
|
||||
<output data-testid="chat-error">{error}</output>
|
||||
{conversations.map((conversation) => (
|
||||
<button
|
||||
key={conversation.id}
|
||||
type="button"
|
||||
data-testid={`conversation-select-${conversation.id}`}
|
||||
onClick={() => onSelectConversation(conversation.id)}
|
||||
>
|
||||
{conversation.id}
|
||||
</button>
|
||||
))}
|
||||
<button
|
||||
type="button"
|
||||
data-testid="send-message"
|
||||
onClick={() => onSend('hello from b', [], [])}
|
||||
disabled={sendDisabled}
|
||||
>
|
||||
send
|
||||
</button>
|
||||
<button type="button" data-testid="new-conversation" onClick={onNewConversation}>
|
||||
new
|
||||
</button>
|
||||
</section>
|
||||
),
|
||||
}));
|
||||
|
||||
const config: AppConfig = {
|
||||
mode: 'daemon',
|
||||
apiKey: '',
|
||||
baseUrl: '',
|
||||
model: '',
|
||||
agentId: 'agent-1',
|
||||
agentModels: {},
|
||||
skillId: null,
|
||||
designSystemId: null,
|
||||
notifications: {
|
||||
soundEnabled: true,
|
||||
successSoundId: 'success-sound',
|
||||
failureSoundId: 'failure-sound',
|
||||
desktopEnabled: false,
|
||||
},
|
||||
};
|
||||
|
||||
const project: Project = {
|
||||
id: 'project-1',
|
||||
name: 'Project',
|
||||
skillId: null,
|
||||
designSystemId: null,
|
||||
createdAt: 1,
|
||||
updatedAt: 1,
|
||||
};
|
||||
|
||||
const conversations: Conversation[] = [
|
||||
{ id: 'conv-a', projectId: project.id, title: 'A', createdAt: 1, updatedAt: 1 },
|
||||
{ id: 'conv-b', projectId: project.id, title: 'B', createdAt: 1, updatedAt: 1 },
|
||||
];
|
||||
|
||||
const createdConversation: Conversation = {
|
||||
id: 'conv-c',
|
||||
projectId: project.id,
|
||||
title: null,
|
||||
createdAt: 2,
|
||||
updatedAt: 2,
|
||||
};
|
||||
|
||||
const runningAssistant: ChatMessage = {
|
||||
id: 'assistant-a',
|
||||
role: 'assistant',
|
||||
content: 'still running',
|
||||
createdAt: 1,
|
||||
runId: 'run-a',
|
||||
runStatus: 'running',
|
||||
};
|
||||
|
||||
const succeededAssistant: ChatMessage = {
|
||||
...runningAssistant,
|
||||
content: 'done',
|
||||
runStatus: 'succeeded',
|
||||
endedAt: 2,
|
||||
};
|
||||
|
||||
describe('ProjectView conversation run isolation', () => {
|
||||
let resolveConversationBMessages: ((messages: ChatMessage[]) => void) | null = null;
|
||||
let conversationAMessages: ChatMessage[] = [runningAssistant];
|
||||
|
||||
beforeEach(() => {
|
||||
resolveConversationBMessages = null;
|
||||
conversationAMessages = [runningAssistant];
|
||||
listConversations.mockResolvedValue(conversations);
|
||||
listMessages.mockImplementation(async (_projectId: string, conversationId: string) => {
|
||||
if (conversationId === 'conv-a') return conversationAMessages;
|
||||
if (conversationId === 'conv-b') {
|
||||
return new Promise<ChatMessage[]>((resolve) => {
|
||||
resolveConversationBMessages = resolve;
|
||||
});
|
||||
}
|
||||
return new Promise<ChatMessage[]>(() => {});
|
||||
});
|
||||
createConversation.mockResolvedValue(createdConversation);
|
||||
fetchPreviewComments.mockResolvedValue([]);
|
||||
loadTabs.mockResolvedValue({ tabs: [], active: null });
|
||||
fetchProjectFiles.mockResolvedValue([]);
|
||||
fetchLiveArtifacts.mockResolvedValue([]);
|
||||
fetchSkill.mockResolvedValue(null);
|
||||
fetchDesignSystem.mockResolvedValue(null);
|
||||
getTemplate.mockResolvedValue(null);
|
||||
listActiveChatRuns.mockResolvedValue([]);
|
||||
fetchChatRunStatus.mockResolvedValue({
|
||||
id: 'run-a',
|
||||
status: 'running',
|
||||
createdAt: 1,
|
||||
updatedAt: 1,
|
||||
exitCode: null,
|
||||
signal: null,
|
||||
});
|
||||
reattachDaemonRun.mockImplementation(async () => new Promise<void>(() => {}));
|
||||
streamViaDaemon.mockImplementation(async () => {});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup();
|
||||
vi.unstubAllGlobals();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it('allows sending in another conversation while the previous conversation has an active run', async () => {
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('streaming'));
|
||||
|
||||
fireEvent.click(screen.getByTestId('conversation-select-conv-b'));
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-b'));
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('idle'));
|
||||
expect(screen.getByTestId('send-message')).toHaveProperty('disabled', true);
|
||||
|
||||
fireEvent.click(screen.getByTestId('send-message'));
|
||||
expect(streamViaDaemon).not.toHaveBeenCalled();
|
||||
|
||||
if (!resolveConversationBMessages) throw new Error('Expected conv-b message load to be pending');
|
||||
resolveConversationBMessages([]);
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('idle'));
|
||||
expect(screen.getByTestId('send-message')).toHaveProperty('disabled', false);
|
||||
|
||||
fireEvent.click(screen.getByTestId('send-message'));
|
||||
|
||||
await waitFor(() => expect(streamViaDaemon).toHaveBeenCalledTimes(1));
|
||||
expect(streamViaDaemon).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
projectId: 'project-1',
|
||||
conversationId: 'conv-b',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not create duplicate empty conversations while a fresh conversation is loading', async () => {
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
|
||||
fireEvent.click(screen.getByTestId('new-conversation'));
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-c'));
|
||||
|
||||
fireEvent.click(screen.getByTestId('new-conversation'));
|
||||
|
||||
expect(createConversation).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('blocks duplicate new conversations while creation is in flight', async () => {
|
||||
let resolveCreate!: (conversation: Conversation) => void;
|
||||
createConversation.mockImplementationOnce(
|
||||
() => new Promise<Conversation>((resolve) => {
|
||||
resolveCreate = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
|
||||
fireEvent.click(screen.getByTestId('new-conversation'));
|
||||
fireEvent.click(screen.getByTestId('new-conversation'));
|
||||
|
||||
expect(createConversation).toHaveBeenCalledTimes(1);
|
||||
|
||||
await act(async () => {
|
||||
resolveCreate(createdConversation);
|
||||
});
|
||||
});
|
||||
|
||||
it('notifies when a detached active run is terminal after returning to its conversation', async () => {
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('streaming'));
|
||||
|
||||
fireEvent.click(screen.getByTestId('conversation-select-conv-b'));
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-b'));
|
||||
if (!resolveConversationBMessages) throw new Error('Expected conv-b message load to be pending');
|
||||
resolveConversationBMessages([]);
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('idle'));
|
||||
|
||||
conversationAMessages = [succeededAssistant];
|
||||
fireEvent.click(screen.getByTestId('conversation-select-conv-a'));
|
||||
|
||||
await waitFor(() => expect(playSound).toHaveBeenCalledWith('success-sound'));
|
||||
expect(showCompletionNotification).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not reload or reattach when selecting the active streaming conversation', async () => {
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('streaming'));
|
||||
|
||||
listMessages.mockClear();
|
||||
reattachDaemonRun.mockClear();
|
||||
|
||||
fireEvent.click(screen.getByTestId('conversation-select-conv-a'));
|
||||
await new Promise((resolve) => setTimeout(resolve, 0));
|
||||
|
||||
expect(screen.getByTestId('streaming-state').textContent).toBe('streaming');
|
||||
expect(listMessages).not.toHaveBeenCalled();
|
||||
expect(reattachDaemonRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('keeps Stop hidden and Send disabled until active-run cancellation is attached', async () => {
|
||||
fetchChatRunStatus.mockImplementation(async () => new Promise(() => {}));
|
||||
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('idle'));
|
||||
expect(screen.getByTestId('send-message')).toHaveProperty('disabled', true);
|
||||
|
||||
fireEvent.click(screen.getByTestId('send-message'));
|
||||
fireEvent.click(screen.getByTestId('workspace-send-comment'));
|
||||
|
||||
expect(streamViaDaemon).not.toHaveBeenCalled();
|
||||
expect(reattachDaemonRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('surfaces conversation message load errors and keeps sends disabled until messages load', async () => {
|
||||
let conversationBLoadAttempts = 0;
|
||||
listMessages.mockImplementation(async (_projectId: string, conversationId: string) => {
|
||||
if (conversationId === 'conv-a') return [];
|
||||
if (conversationId === 'conv-b') {
|
||||
conversationBLoadAttempts += 1;
|
||||
if (conversationBLoadAttempts === 1) throw new Error('messages unavailable');
|
||||
return [];
|
||||
}
|
||||
return [];
|
||||
});
|
||||
|
||||
renderProjectView();
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
fireEvent.click(screen.getByTestId('conversation-select-conv-b'));
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('chat-error').textContent).toBe('messages unavailable'));
|
||||
await waitFor(() => expect(screen.getByTestId('streaming-state').textContent).toBe('idle'));
|
||||
expect(screen.getByTestId('send-message')).toHaveProperty('disabled', true);
|
||||
expect(screen.getByTestId('workspace-streaming-state').textContent).toBe('streaming');
|
||||
|
||||
fireEvent.click(screen.getByTestId('send-message'));
|
||||
|
||||
expect(streamViaDaemon).not.toHaveBeenCalled();
|
||||
|
||||
fireEvent.click(screen.getByTestId('conversation-select-conv-b'));
|
||||
|
||||
await waitFor(() => expect(conversationBLoadAttempts).toBe(2));
|
||||
await waitFor(() => expect(screen.getByTestId('chat-error').textContent).toBe(''));
|
||||
expect(screen.getByTestId('send-message')).toHaveProperty('disabled', false);
|
||||
});
|
||||
|
||||
it('notifies when an API-mode chat completes without a daemon run status transition', async () => {
|
||||
listMessages.mockResolvedValue([]);
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValue({ ok: true }));
|
||||
streamMessage.mockImplementation(
|
||||
async (
|
||||
_config: unknown,
|
||||
_systemPrompt: unknown,
|
||||
_history: unknown,
|
||||
_signal: unknown,
|
||||
handlers: { onDelta: (delta: string) => void; onDone: () => void },
|
||||
) => {
|
||||
handlers.onDelta('api response');
|
||||
handlers.onDone();
|
||||
},
|
||||
);
|
||||
|
||||
renderProjectView({
|
||||
...config,
|
||||
mode: 'api',
|
||||
apiKey: 'test-key',
|
||||
model: 'api-model',
|
||||
});
|
||||
|
||||
await waitFor(() => expect(screen.getByTestId('active-conversation').textContent).toBe('conv-a'));
|
||||
await waitFor(() => expect(screen.getByTestId('send-message')).toHaveProperty('disabled', false));
|
||||
|
||||
fireEvent.click(screen.getByTestId('send-message'));
|
||||
|
||||
await waitFor(() => expect(streamMessage).toHaveBeenCalledTimes(1));
|
||||
await waitFor(() => expect(playSound).toHaveBeenCalledWith('success-sound'));
|
||||
});
|
||||
});
|
||||
|
||||
function renderProjectView(renderConfig = config) {
|
||||
return render(
|
||||
<ProjectView
|
||||
project={project}
|
||||
routeFileName={null}
|
||||
config={renderConfig}
|
||||
agents={[{ id: 'agent-1', name: 'OpenCode', bin: 'opencode', available: true, models: [] }]}
|
||||
skills={[]}
|
||||
designTemplates={[]}
|
||||
designSystems={[]}
|
||||
daemonLive
|
||||
onModeChange={() => {}}
|
||||
onAgentChange={() => {}}
|
||||
onAgentModelChange={() => {}}
|
||||
onRefreshAgents={() => {}}
|
||||
onOpenSettings={() => {}}
|
||||
onBack={() => {}}
|
||||
onClearPendingPrompt={() => {}}
|
||||
onTouchProject={() => {}}
|
||||
onProjectChange={() => {}}
|
||||
onProjectsRefresh={() => {}}
|
||||
/>,
|
||||
);
|
||||
}
|
||||
|
|
@ -0,0 +1,208 @@
|
|||
---
|
||||
id: 20260511-issue-138-conversation-run-isolation
|
||||
name: Issue 138 Conversation Run Isolation
|
||||
status: implemented
|
||||
created: '2026-05-11'
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
Issue #138 reports that an unfinished task in one conversation blocks sending in another conversation in the same project. Related issue #148 describes the same underlying run-state isolation bug as leaked running/failed/elapsed state across conversations.
|
||||
|
||||
Goal: make chat run UI state conversation-scoped while keeping the project workspace shared.
|
||||
|
||||
Constraints:
|
||||
- Do not introduce a project-level writer lock or run scheduler for this fix.
|
||||
- Allow multiple conversations in the same project to run concurrently.
|
||||
- Let real file-write conflicts fail visibly through the existing agent/tool path.
|
||||
- Keep daemon run cancellation explicit: switching conversations should detach local SSE listeners, not cancel background runs.
|
||||
|
||||
Open questions:
|
||||
- Whether future UX should surface a project-wide “another conversation is modifying files” hint.
|
||||
- Whether future persistence should guarantee long-running background transcript recovery beyond the current daemon event buffer behavior.
|
||||
|
||||
## Research
|
||||
|
||||
### Existing System
|
||||
|
||||
- `ProjectView` currently stores chat busy state as one project-level `streaming` boolean. Source: `apps/web/src/components/ProjectView.tsx:263`
|
||||
- `handleSend` blocks every send when `streaming` is true, regardless of active conversation. Source: `apps/web/src/components/ProjectView.tsx:1073-1082`
|
||||
- `ChatComposer` uses the same `streaming` prop to prevent submit and switch the send button into Stop mode. Source: `apps/web/src/components/ChatComposer.tsx:599-627,910-929`
|
||||
- Conversation switches abort browser-side SSE listeners and leave daemon runs alive; cleanup intentionally avoids aborting cancel signals. Source: `apps/web/src/components/ProjectView.tsx:428-453`
|
||||
- Reattach already queries active runs by `projectId` and `conversationId`. Source: `apps/web/src/components/ProjectView.tsx:864-899`, `apps/web/src/providers/daemon.ts:204-210`
|
||||
- The daemon run service stores `projectId`, `conversationId`, and `assistantMessageId` on each run. Source: `apps/daemon/src/runs.ts:15-24`
|
||||
- The daemon run list supports filtering by project, conversation, and active status. Source: `apps/daemon/src/runs.ts:117-123`, `apps/daemon/src/chat-routes.ts:65-70`
|
||||
- Run creation does not enforce a project-wide mutex. Source: `apps/daemon/src/chat-routes.ts:46-63`
|
||||
|
||||
### Available Approaches
|
||||
|
||||
- **Conversation-scoped UI busy state**: track active run/listener state per conversation and derive current composer state from `activeConversationId`. Source: `apps/web/src/components/ProjectView.tsx:256-263,1073-1082`
|
||||
- **Project-level scheduler/lock**: serialize runs per project, which conflicts with #138’s requirement that a fresh conversation can send while another remains unfinished. Source: issue #138 discussion and `apps/daemon/src/chat-routes.ts:46-63`
|
||||
- **Daemon API rewrite**: unnecessary for the primary fix because run identity and filters already include `conversationId`. Source: `apps/daemon/src/runs.ts:15-24,117-123`
|
||||
|
||||
### Constraints & Dependencies
|
||||
|
||||
- Project files and live artifacts remain project-scoped shared resources, so concurrent conversations can produce interleaved file events. Source: `apps/web/src/components/ProjectView.tsx:552-617,1173-1226`
|
||||
- Stop handling currently cancels all active assistant messages visible in the current `messages` array and uses shared cancel refs; it must become current-conversation scoped. Source: `apps/web/src/components/ProjectView.tsx:1640-1674`
|
||||
- Completion notification logic depends on a `streaming` true-to-false edge and must be adjusted when busy state becomes conversation-scoped. Source: `apps/web/src/components/ProjectView.tsx:469-523`
|
||||
|
||||
### Key References
|
||||
|
||||
- GitHub issue #138 - user-facing send block symptom.
|
||||
- GitHub issue #148 - duplicate state-leak regression case.
|
||||
|
||||
## Design
|
||||
|
||||
### Architecture Overview
|
||||
|
||||
```mermaid
|
||||
flowchart TD
|
||||
Project[Project workspace] --> Files[Shared files/artifacts]
|
||||
Project --> Conversations[Conversations]
|
||||
Conversations --> ConvA[Conversation A run state]
|
||||
Conversations --> ConvB[Conversation B run state]
|
||||
ConvA --> RunA[Daemon run A]
|
||||
ConvB --> RunB[Daemon run B]
|
||||
RunA --> Files
|
||||
RunB --> Files
|
||||
```
|
||||
|
||||
### Change Scope
|
||||
|
||||
- Area: `ProjectView` run lifecycle. Impact: replace project-wide chat busy assumptions with current-conversation busy derivation. Source: `apps/web/src/components/ProjectView.tsx:263,1073-1082`
|
||||
- Area: `ChatPane` / `ChatComposer` props. Impact: composer should receive whether the active conversation is busy, not whether any project run is busy. Source: `apps/web/src/components/ProjectView.tsx:2129-2152`, `apps/web/src/components/ChatComposer.tsx:599-627,910-929`
|
||||
- Area: run reattach and stop behavior. Impact: reattach and cancellation should apply to the active conversation’s run only. Source: `apps/web/src/components/ProjectView.tsx:864-899,1640-1674`
|
||||
- Area: daemon tests. Impact: add/keep regression coverage for conversation-filtered active run listing. Source: `apps/daemon/src/runs.ts:117-123`
|
||||
|
||||
### Design Decisions
|
||||
|
||||
- Decision: Track chat run busy state at conversation granularity in the web UI, and derive `currentConversationBusy` from `activeConversationId`. Source: `apps/web/src/components/ProjectView.tsx:256-263,1073-1082`
|
||||
- Decision: Keep project files/artifacts project-scoped and allow concurrent writes to fail through the existing tool/file path. Source: `apps/web/src/components/ProjectView.tsx:552-617,1173-1226`
|
||||
- Decision: Do not add daemon-level project serialization in this fix because daemon runs are already conversation-identifiable and unblocked at creation. Source: `apps/daemon/src/runs.ts:15-24`, `apps/daemon/src/chat-routes.ts:46-63`
|
||||
- Decision: Conversation switching should detach browser SSE listeners while preserving daemon background runs. Source: `apps/web/src/components/ProjectView.tsx:428-453`
|
||||
- Decision: Stop should target only the current conversation’s active run/controller. Source: `apps/web/src/components/ProjectView.tsx:1640-1674`
|
||||
|
||||
### Why this design
|
||||
|
||||
- It fixes the state boundary mismatch directly: conversation owns messaging/run UI state, project owns shared files.
|
||||
- It keeps the implementation small and observable.
|
||||
- It preserves fail-fast behavior for real concurrent file conflicts instead of hiding them behind queues or optimistic merges.
|
||||
|
||||
### Test Strategy
|
||||
|
||||
Primary validation should be unit/component tests. E2E is optional later because reliably creating a long-running agent run requires a deterministic test agent or fixture.
|
||||
|
||||
#### Web component/unit tests
|
||||
|
||||
Add a focused regression file such as `apps/web/tests/components/ProjectView.run-isolation.test.tsx`.
|
||||
|
||||
- Test #138 send-lock regression:
|
||||
- Arrange project `p1` with conversations `conv-a` and `conv-b`.
|
||||
- Load `conv-a` messages with an assistant message whose `runStatus` is `running` and `runId` is `run-a`.
|
||||
- Switch to `conv-b` or create/select `conv-b` with no active run.
|
||||
- Type in the composer and click Send.
|
||||
- Assert the composer shows Send mode for `conv-b` and sending is not blocked by `conv-a`.
|
||||
- Assert the daemon run creation body includes `conversationId: 'conv-b'`.
|
||||
- Source: `apps/web/src/components/ProjectView.tsx:1073-1082,1353-1362,2129-2152`
|
||||
|
||||
- Test #148 state-leak regression:
|
||||
- Arrange `conv-a` with `runStatus: 'running'` or `runStatus: 'failed'` and elapsed/error state.
|
||||
- Switch to `conv-b` whose messages have no active assistant run.
|
||||
- Assert `conv-b` does not render `conv-a` running/failed/elapsed state.
|
||||
- Assert the composer is in Send mode and the last `conv-b` assistant message is not treated as streaming.
|
||||
- Source: `apps/web/src/components/ChatPane.tsx:304-307`, `apps/web/src/components/ProjectView.tsx:2129-2152`
|
||||
|
||||
- Test conversation switch preserves background run:
|
||||
- Arrange `conv-a` with active `run-a` and a mounted SSE listener.
|
||||
- Switch to `conv-b`.
|
||||
- Assert browser-side listener cleanup runs without calling `/api/runs/run-a/cancel`.
|
||||
- Switch back to `conv-a`.
|
||||
- Assert reattach queries active runs for `projectId: 'p1'` and `conversationId: 'conv-a'`.
|
||||
- Source: `apps/web/src/components/ProjectView.tsx:428-453,864-899`, `apps/web/src/providers/daemon.ts:204-210`
|
||||
|
||||
- Test Stop scope:
|
||||
- Arrange `conv-a` and `conv-b` each with active run state.
|
||||
- Make `conv-b` active and click Stop.
|
||||
- Assert only `/api/runs/<run-b>/cancel` is called.
|
||||
- Assert `conv-a` remains active and is not marked canceled by the current view.
|
||||
- Source: `apps/web/src/components/ProjectView.tsx:1640-1674`
|
||||
|
||||
#### Daemon unit test
|
||||
|
||||
Add to `apps/daemon/tests/runs.test.ts`:
|
||||
|
||||
- Create two active runs in the same project with `conversationId: 'conv-a'` and `conversationId: 'conv-b'`.
|
||||
- Call `runs.list({ projectId: 'p1', conversationId: 'conv-b', status: 'active' })`.
|
||||
- Assert only the `conv-b` run is returned.
|
||||
- Source: `apps/daemon/src/runs.ts:117-123`
|
||||
|
||||
#### Validation commands
|
||||
|
||||
- `pnpm --filter @open-design/web test`
|
||||
- `pnpm --filter @open-design/daemon test`
|
||||
- `pnpm typecheck`
|
||||
|
||||
### Pseudocode
|
||||
|
||||
Flow:
|
||||
On send:
|
||||
if activeConversationId is missing, return
|
||||
if currentConversationBusy, return
|
||||
create user + assistant message in active conversation
|
||||
start daemon run with active conversationId
|
||||
mark activeConversationId busy with run/controller metadata
|
||||
|
||||
On conversation switch:
|
||||
detach current browser SSE listener
|
||||
keep daemon run alive
|
||||
load selected conversation messages
|
||||
derive composer busy from selected conversation state/messages
|
||||
reattach selected conversation active run when present
|
||||
|
||||
On stop:
|
||||
find current activeConversationId run/controller
|
||||
abort its cancel signal
|
||||
mark only current conversation messages canceled
|
||||
|
||||
### File Structure
|
||||
|
||||
- `apps/web/src/components/ProjectView.tsx` - conversation-scoped run state, send gate, stop, reattach, notification edge adjustments.
|
||||
- `apps/web/src/components/ChatPane.tsx` - pass active-conversation busy state to composer and message streaming render.
|
||||
- `apps/web/src/components/ChatComposer.tsx` - unchanged semantics, receives scoped busy prop via existing `streaming` prop or renamed prop.
|
||||
- `apps/web/tests/components/*` - regression coverage for #138/#148 behavior.
|
||||
- `apps/daemon/tests/runs.test.ts` - daemon run filtering regression.
|
||||
|
||||
## Plan
|
||||
|
||||
- [x] Step 1: Add regression coverage
|
||||
- [x] Substep 1.1 Verify: reproduce blocked send when another conversation has an active run.
|
||||
- [x] Substep 1.2 Verify: cover conversation-filtered active run listing in daemon service.
|
||||
- [x] Step 2: Scope web run UI state to active conversation
|
||||
- [x] Substep 2.1 Implement: derive busy state from active conversation load/run state.
|
||||
- [x] Substep 2.2 Implement: clear stale message/render state on conversation switches.
|
||||
- [x] Substep 2.3 Implement: block sends while target conversation messages are still loading.
|
||||
- [x] Substep 2.4 Verify: prevent duplicate empty conversation creation while a fresh conversation is loading.
|
||||
- [x] Step 3: Validate implementation
|
||||
- [x] Substep 3.1 Verify: run focused ProjectView regression tests.
|
||||
- [x] Substep 3.2 Verify: run focused daemon run service test.
|
||||
- [x] Substep 3.3 Verify: run guard and typecheck.
|
||||
|
||||
## Notes
|
||||
|
||||
<!-- Optional sections — add what's relevant. -->
|
||||
|
||||
### Implementation
|
||||
|
||||
- `apps/web/src/components/ProjectView.tsx` - added active-conversation message ownership state, loading-aware busy derivation, send gating, and synchronous conversation-switch cleanup.
|
||||
- `apps/web/tests/components/ProjectView.run-isolation.test.tsx` - added regression tests for sending in a different conversation, loading-window send blocking, and fresh-conversation duplicate prevention.
|
||||
- `apps/daemon/tests/runs.test.ts` - added service-level regression coverage for active run filtering by conversation within one project.
|
||||
|
||||
### Verification
|
||||
|
||||
- Confirmed new web regression failed before the fix: `expected 'streaming' to be 'idle'` after switching to `conv-b`.
|
||||
- `pnpm --filter @open-design/web exec vitest run -c vitest.config.ts tests/components/ProjectView.run-isolation.test.tsx tests/components/ProjectView.run-cleanup.test.tsx tests/components/ProjectView.pendingPrompt.test.tsx` - passed.
|
||||
- `pnpm --filter @open-design/daemon exec vitest run -c vitest.config.ts tests/runs.test.ts` - passed.
|
||||
- `pnpm --filter @open-design/web typecheck` - passed.
|
||||
- `pnpm guard` - passed.
|
||||
- `pnpm typecheck` - passed.
|
||||
- Reviewer subagent final pass: no remaining blocking issues.
|
||||
Loading…
Reference in a new issue