mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-16 13:03:31 +08:00
fix: keep completed task lists from leaking into the next turn
Completed desktop task bars were only being dismissed locally, which let the persisted task list resurface on refresh or bleed into the next user turn. This wires the existing server-side reset path into the desktop stores and session flow so a finished task cycle is summarized once, cleared locally, and removed remotely before the next round starts. Constraint: Existing task persistence already lives behind the server task-list API and must stay compatible with persisted JSON task files Rejected: Only hide the completed task bar in UI state | left stale persisted tasks behind and reintroduced them on reload Rejected: Clear desktop state without a task summary | dropped useful completion context from the chat transcript Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep desktop task dismissal and task-list persistence behavior aligned; do not reintroduce local-only clearing without covering reload and next-turn flows Tested: `cd desktop && bun run lint` Tested: `cd desktop && bun run test src/stores/cliTaskStore.test.ts src/stores/chatStore.test.ts src/components/chat/SessionTaskBar.test.tsx src/components/chat/ComputerUsePermissionModal.test.tsx` Tested: `bun test src/server/__tests__/e2e/business-flow.test.ts --test-name-pattern "Task Lists API"` Not-tested: Full `bun test src/server/__tests__/e2e/business-flow.test.ts` suite still has unrelated pre-existing failures in Models and Sessions sections
This commit is contained in:
parent
c69c07fcfe
commit
16d716e773
@ -21,6 +21,11 @@ export const cliTasksApi = {
|
||||
return api.get<TaskResponse>(`/api/tasks/lists/${encodeURIComponent(taskListId)}/${taskId}`)
|
||||
},
|
||||
|
||||
/** Clear all persisted tasks for a completed task list */
|
||||
resetTaskList(taskListId: string) {
|
||||
return api.post<{ ok: true }>(`/api/tasks/lists/${encodeURIComponent(taskListId)}/reset`)
|
||||
},
|
||||
|
||||
/** List all tasks across all task lists */
|
||||
listAll() {
|
||||
return api.get<TasksResponse>('/api/tasks')
|
||||
|
||||
@ -60,6 +60,7 @@ vi.mock('../../stores/cliTaskStore', () => ({
|
||||
clearTasks: vi.fn(),
|
||||
setTasksFromTodos: vi.fn(),
|
||||
markCompletedAndDismissed: vi.fn(),
|
||||
resetCompletedTasks: vi.fn(async () => {}),
|
||||
refreshTasks: vi.fn(),
|
||||
}),
|
||||
},
|
||||
|
||||
@ -4,6 +4,13 @@ import '@testing-library/jest-dom'
|
||||
import { SessionTaskBar } from './SessionTaskBar'
|
||||
import { useCLITaskStore } from '../../stores/cliTaskStore'
|
||||
|
||||
vi.mock('../../api/cliTasks', () => ({
|
||||
cliTasksApi: {
|
||||
getTasksForList: vi.fn(),
|
||||
resetTaskList: vi.fn(async () => ({ ok: true })),
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('../../i18n', () => ({
|
||||
useTranslation: () => (key: string) => {
|
||||
const translations: Record<string, string> = {
|
||||
@ -46,7 +53,7 @@ describe('SessionTaskBar', () => {
|
||||
expect(screen.queryByRole('button', { name: 'Hide completed tasks' })).toBeNull()
|
||||
})
|
||||
|
||||
it('hides the bar after dismissing a completed task set', () => {
|
||||
it('hides the bar after dismissing a completed task set', async () => {
|
||||
act(() => {
|
||||
useCLITaskStore.getState().setTasksFromTodos([
|
||||
{ content: 'first', status: 'completed' },
|
||||
@ -58,10 +65,13 @@ describe('SessionTaskBar', () => {
|
||||
render(<SessionTaskBar />)
|
||||
})
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Hide completed tasks' }))
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Hide completed tasks' }))
|
||||
await Promise.resolve()
|
||||
})
|
||||
|
||||
expect(screen.queryByText('Tasks')).toBeNull()
|
||||
expect(useCLITaskStore.getState().completedAndDismissed).toBe(true)
|
||||
expect(useCLITaskStore.getState().tasks).toEqual([])
|
||||
})
|
||||
|
||||
it('shows the bar again for a new task cycle after a previous completed set was dismissed', () => {
|
||||
|
||||
@ -26,7 +26,7 @@ export function SessionTaskBar() {
|
||||
expanded,
|
||||
toggleExpanded,
|
||||
completedAndDismissed,
|
||||
markCompletedAndDismissed,
|
||||
resetCompletedTasks,
|
||||
} = useCLITaskStore()
|
||||
const t = useTranslation()
|
||||
|
||||
@ -91,7 +91,7 @@ export function SessionTaskBar() {
|
||||
<button
|
||||
type="button"
|
||||
aria-label={t('tasks.dismissCompleted')}
|
||||
onClick={markCompletedAndDismissed}
|
||||
onClick={() => { void resetCompletedTasks() }}
|
||||
className="flex shrink-0 items-center justify-center rounded-[var(--radius-md)] p-1.5 text-[var(--color-text-tertiary)] hover:bg-[var(--color-surface-container-low)] hover:text-[var(--color-text-primary)] transition-colors"
|
||||
>
|
||||
<span className="material-symbols-outlined text-[16px]">close</span>
|
||||
|
||||
@ -12,7 +12,9 @@ const {
|
||||
clearTasksMock,
|
||||
setTasksFromTodosMock,
|
||||
markCompletedAndDismissedMock,
|
||||
resetCompletedTasksMock,
|
||||
refreshTasksMock,
|
||||
cliTaskStoreSnapshot,
|
||||
} = vi.hoisted(() => ({
|
||||
sendMock: vi.fn(),
|
||||
getMemberBySessionIdMock: vi.fn<(sessionId: string) => any>(() => null),
|
||||
@ -24,7 +26,12 @@ const {
|
||||
clearTasksMock: vi.fn(),
|
||||
setTasksFromTodosMock: vi.fn(),
|
||||
markCompletedAndDismissedMock: vi.fn(),
|
||||
resetCompletedTasksMock: vi.fn(async () => {}),
|
||||
refreshTasksMock: vi.fn(),
|
||||
cliTaskStoreSnapshot: {
|
||||
tasks: [] as Array<{ id: string; subject: string; status: string; activeForm?: string }>,
|
||||
sessionId: null as string | null,
|
||||
},
|
||||
}))
|
||||
|
||||
vi.mock('../api/websocket', () => ({
|
||||
@ -77,11 +84,12 @@ vi.mock('./cliTaskStore', () => ({
|
||||
useCLITaskStore: {
|
||||
getState: () => ({
|
||||
fetchSessionTasks: fetchSessionTasksMock,
|
||||
tasks: [],
|
||||
sessionId: null,
|
||||
tasks: cliTaskStoreSnapshot.tasks,
|
||||
sessionId: cliTaskStoreSnapshot.sessionId,
|
||||
clearTasks: clearTasksMock,
|
||||
setTasksFromTodos: setTasksFromTodosMock,
|
||||
markCompletedAndDismissed: markCompletedAndDismissedMock,
|
||||
resetCompletedTasks: resetCompletedTasksMock,
|
||||
refreshTasks: refreshTasksMock,
|
||||
}),
|
||||
},
|
||||
@ -102,7 +110,10 @@ describe('chatStore history mapping', () => {
|
||||
clearTasksMock.mockReset()
|
||||
setTasksFromTodosMock.mockReset()
|
||||
markCompletedAndDismissedMock.mockReset()
|
||||
resetCompletedTasksMock.mockReset()
|
||||
refreshTasksMock.mockReset()
|
||||
cliTaskStoreSnapshot.tasks = []
|
||||
cliTaskStoreSnapshot.sessionId = null
|
||||
useChatStore.setState({
|
||||
...initialState,
|
||||
sessions: {},
|
||||
@ -413,6 +424,54 @@ describe('chatStore history mapping', () => {
|
||||
expect(useChatStore.getState().sessions[TEST_SESSION_ID]?.streamingText).toBe('')
|
||||
})
|
||||
|
||||
it('resets completed CLI tasks before continuing the next user turn', () => {
|
||||
cliTaskStoreSnapshot.sessionId = TEST_SESSION_ID
|
||||
cliTaskStoreSnapshot.tasks = [
|
||||
{ id: '1', subject: 'Existing completed task', status: 'completed' },
|
||||
{ id: '2', subject: 'Another completed task', status: 'completed' },
|
||||
]
|
||||
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
[TEST_SESSION_ID]: {
|
||||
messages: [],
|
||||
chatState: 'idle',
|
||||
connectionState: 'connected',
|
||||
streamingText: '',
|
||||
streamingToolInput: '',
|
||||
activeToolUseId: null,
|
||||
activeToolName: null,
|
||||
activeThinkingId: null,
|
||||
pendingPermission: null,
|
||||
pendingComputerUsePermission: null,
|
||||
tokenUsage: { input_tokens: 0, output_tokens: 0 },
|
||||
elapsedSeconds: 0,
|
||||
statusVerb: '',
|
||||
slashCommands: [],
|
||||
agentTaskNotifications: {},
|
||||
elapsedTimer: null,
|
||||
},
|
||||
},
|
||||
})
|
||||
|
||||
useChatStore.getState().sendMessage(TEST_SESSION_ID, '继续下一轮')
|
||||
|
||||
expect(resetCompletedTasksMock).toHaveBeenCalledTimes(1)
|
||||
expect(useChatStore.getState().sessions[TEST_SESSION_ID]?.messages).toMatchObject([
|
||||
{
|
||||
type: 'task_summary',
|
||||
tasks: [
|
||||
{ id: '1', subject: 'Existing completed task', status: 'completed' },
|
||||
{ id: '2', subject: 'Another completed task', status: 'completed' },
|
||||
],
|
||||
},
|
||||
{
|
||||
type: 'user_text',
|
||||
content: '继续下一轮',
|
||||
},
|
||||
])
|
||||
})
|
||||
|
||||
it('tracks Computer Use approval requests separately from generic tool permissions', () => {
|
||||
useChatStore.setState({
|
||||
sessions: {
|
||||
|
||||
@ -199,6 +199,13 @@ export const useChatStore = create<ChatStore>((set, get) => ({
|
||||
|
||||
const taskStore = useCLITaskStore.getState()
|
||||
const allTasksDone = taskStore.tasks.length > 0 && taskStore.tasks.every((t) => t.status === 'completed')
|
||||
const completedTaskSummary = allTasksDone
|
||||
? taskStore.tasks.map((t) => ({ id: t.id, subject: t.subject, status: t.status, activeForm: t.activeForm }))
|
||||
: []
|
||||
|
||||
if (!isMemberSession && allTasksDone) {
|
||||
void taskStore.resetCompletedTasks()
|
||||
}
|
||||
|
||||
set((s) => {
|
||||
const session = s.sessions[sessionId] ?? createDefaultSessionState()
|
||||
@ -225,10 +232,9 @@ export const useChatStore = create<ChatStore>((set, get) => ({
|
||||
newMessages.push({
|
||||
id: nextId(),
|
||||
type: 'task_summary',
|
||||
tasks: taskStore.tasks.map((t) => ({ id: t.id, subject: t.subject, status: t.status, activeForm: t.activeForm })),
|
||||
tasks: completedTaskSummary,
|
||||
timestamp: Date.now(),
|
||||
})
|
||||
taskStore.clearTasks()
|
||||
}
|
||||
newMessages.push({
|
||||
id: nextId(),
|
||||
|
||||
@ -6,6 +6,7 @@ import { useCLITaskStore } from './cliTaskStore'
|
||||
vi.mock('../api/cliTasks', () => ({
|
||||
cliTasksApi: {
|
||||
getTasksForList: vi.fn(),
|
||||
resetTaskList: vi.fn(),
|
||||
},
|
||||
}))
|
||||
|
||||
@ -67,4 +68,42 @@ describe('cliTaskStore', () => {
|
||||
{ taskListId: 'session-2', status: 'completed' },
|
||||
])
|
||||
})
|
||||
|
||||
it('resets a completed task list locally before clearing it remotely', async () => {
|
||||
let resolveReset: ((value: { ok: true }) => void) | null = null
|
||||
|
||||
vi.mocked(cliTasksApi.resetTaskList).mockImplementation(
|
||||
() => new Promise<{ ok: true }>((resolve) => {
|
||||
resolveReset = resolve
|
||||
}),
|
||||
)
|
||||
|
||||
useCLITaskStore.setState({
|
||||
sessionId: 'session-1',
|
||||
tasks: [
|
||||
makeTask('session-1', 'completed'),
|
||||
{ ...makeTask('session-1', 'completed'), id: '2', subject: 'Second completed task' },
|
||||
],
|
||||
expanded: true,
|
||||
completedAndDismissed: true,
|
||||
dismissedCompletionKey: 'session-1::done',
|
||||
})
|
||||
|
||||
const resetPromise = useCLITaskStore.getState().resetCompletedTasks()
|
||||
|
||||
expect(vi.mocked(cliTasksApi.resetTaskList)).toHaveBeenCalledWith('session-1')
|
||||
expect(useCLITaskStore.getState()).toMatchObject({
|
||||
tasks: [],
|
||||
resetting: true,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
expanded: false,
|
||||
})
|
||||
|
||||
expect(resolveReset).not.toBeNull()
|
||||
resolveReset!({ ok: true })
|
||||
await resetPromise
|
||||
|
||||
expect(useCLITaskStore.getState().resetting).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@ -13,6 +13,8 @@ type CLITaskStore = {
|
||||
sessionId: string | null
|
||||
/** Tasks for the current session */
|
||||
tasks: CLITask[]
|
||||
/** True while the persisted task list is being cleared remotely */
|
||||
resetting: boolean
|
||||
/** Whether the task bar is expanded */
|
||||
expanded: boolean
|
||||
/** True when all tasks completed and the user already continued chatting.
|
||||
@ -29,6 +31,8 @@ type CLITaskStore = {
|
||||
setTasksFromTodos: (todos: TodoItem[]) => void
|
||||
/** Mark that completed tasks were already dismissed (conversation continued) */
|
||||
markCompletedAndDismissed: () => void
|
||||
/** Clear a completed task list locally and remotely so the next cycle starts clean */
|
||||
resetCompletedTasks: () => Promise<void>
|
||||
/** Clear task tracking state */
|
||||
clearTasks: () => void
|
||||
/** Toggle expanded state */
|
||||
@ -78,6 +82,7 @@ function mapTodosToTasks(todos: TodoItem[], sessionId: string | null): CLITask[]
|
||||
export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
sessionId: null,
|
||||
tasks: [],
|
||||
resetting: false,
|
||||
expanded: false,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
@ -87,6 +92,7 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
set({
|
||||
sessionId,
|
||||
tasks: [],
|
||||
resetting: false,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
expanded: false,
|
||||
@ -96,7 +102,7 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
try {
|
||||
const { tasks } = await cliTasksApi.getTasksForList(sessionId)
|
||||
// Only update if still tracking the same session
|
||||
if (get().sessionId === sessionId) {
|
||||
if (get().sessionId === sessionId && !get().resetting) {
|
||||
set((state) => ({
|
||||
tasks,
|
||||
...resolveDismissState(tasks, state.dismissedCompletionKey),
|
||||
@ -104,7 +110,7 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
}
|
||||
} catch {
|
||||
// No tasks for this session — that's fine
|
||||
if (get().sessionId === sessionId) {
|
||||
if (get().sessionId === sessionId && !get().resetting) {
|
||||
set({ tasks: [], completedAndDismissed: false, dismissedCompletionKey: null, expanded: false })
|
||||
}
|
||||
}
|
||||
@ -115,7 +121,7 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
if (!sessionId) return
|
||||
try {
|
||||
const { tasks } = await cliTasksApi.getTasksForList(sessionId)
|
||||
if (get().sessionId === sessionId) {
|
||||
if (get().sessionId === sessionId && !get().resetting) {
|
||||
set((state) => ({
|
||||
tasks,
|
||||
...resolveDismissState(tasks, state.dismissedCompletionKey),
|
||||
@ -145,10 +151,33 @@ export const useCLITaskStore = create<CLITaskStore>((set, get) => ({
|
||||
})
|
||||
},
|
||||
|
||||
resetCompletedTasks: async () => {
|
||||
const { sessionId, tasks } = get()
|
||||
const completionKey = buildCompletedTaskKey(tasks)
|
||||
if (!sessionId || !completionKey) return
|
||||
|
||||
set({
|
||||
tasks: [],
|
||||
resetting: true,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
expanded: false,
|
||||
})
|
||||
|
||||
try {
|
||||
await cliTasksApi.resetTaskList(sessionId)
|
||||
} finally {
|
||||
if (get().sessionId === sessionId) {
|
||||
set({ resetting: false })
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
clearTasks: () => {
|
||||
set({
|
||||
sessionId: null,
|
||||
tasks: [],
|
||||
resetting: false,
|
||||
completedAndDismissed: false,
|
||||
dismissedCompletionKey: null,
|
||||
expanded: false,
|
||||
|
||||
@ -204,6 +204,64 @@ describe('Business Flow: Permission Modes', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('Business Flow: Task Lists API', () => {
|
||||
beforeAll(startTestServer)
|
||||
afterAll(async () => {
|
||||
server?.stop()
|
||||
await fs.rm(tmpDir, { recursive: true, force: true })
|
||||
})
|
||||
|
||||
it('should reset a persisted task list through the API', async () => {
|
||||
const taskListDir = path.join(tmpDir, 'tasks', 'desktop-session-1')
|
||||
await fs.mkdir(taskListDir, { recursive: true })
|
||||
await fs.writeFile(
|
||||
path.join(taskListDir, '1.json'),
|
||||
JSON.stringify({
|
||||
id: '1',
|
||||
subject: 'First task',
|
||||
description: '',
|
||||
status: 'completed',
|
||||
blocks: [],
|
||||
blockedBy: [],
|
||||
}),
|
||||
'utf-8',
|
||||
)
|
||||
await fs.writeFile(
|
||||
path.join(taskListDir, '2.json'),
|
||||
JSON.stringify({
|
||||
id: '2',
|
||||
subject: 'Second task',
|
||||
description: '',
|
||||
status: 'completed',
|
||||
blocks: [],
|
||||
blockedBy: [],
|
||||
}),
|
||||
'utf-8',
|
||||
)
|
||||
|
||||
const { status: beforeStatus, data: beforeData } = await api(
|
||||
'GET',
|
||||
'/api/tasks/lists/desktop-session-1',
|
||||
)
|
||||
expect(beforeStatus).toBe(200)
|
||||
expect(beforeData.tasks).toHaveLength(2)
|
||||
|
||||
const { status: resetStatus, data: resetData } = await api(
|
||||
'POST',
|
||||
'/api/tasks/lists/desktop-session-1/reset',
|
||||
)
|
||||
expect(resetStatus).toBe(200)
|
||||
expect(resetData.ok).toBe(true)
|
||||
|
||||
const { status: afterStatus, data: afterData } = await api(
|
||||
'GET',
|
||||
'/api/tasks/lists/desktop-session-1',
|
||||
)
|
||||
expect(afterStatus).toBe(200)
|
||||
expect(afterData.tasks).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('Business Flow: Agent Management', () => {
|
||||
beforeAll(startTestServer)
|
||||
afterAll(async () => {
|
||||
|
||||
@ -14,6 +14,7 @@
|
||||
import { AgentService } from '../services/agentService.js'
|
||||
import { taskService } from '../services/taskService.js'
|
||||
import { ApiError, errorResponse } from '../middleware/errorHandler.js'
|
||||
import { resetTaskList } from '../../utils/tasks.js'
|
||||
import {
|
||||
resolveAgentModelDisplay,
|
||||
resolveAgentOverrides,
|
||||
@ -124,26 +125,31 @@ async function handleAgents(
|
||||
// GET /api/tasks/lists → list all task lists with summaries
|
||||
// GET /api/tasks/lists/:taskListId → get all tasks for a specific task list
|
||||
// GET /api/tasks/lists/:taskListId/:id → get a single task
|
||||
// POST /api/tasks/lists/:taskListId/reset → clear a completed task list
|
||||
|
||||
async function handleTasksApi(
|
||||
req: Request,
|
||||
segments: string[],
|
||||
): Promise<Response> {
|
||||
if (req.method !== 'GET') {
|
||||
throw new ApiError(
|
||||
405,
|
||||
`Method ${req.method} not allowed on /api/tasks`,
|
||||
'METHOD_NOT_ALLOWED',
|
||||
)
|
||||
}
|
||||
|
||||
const sub = segments[2] // 'lists' or undefined
|
||||
|
||||
// GET /api/tasks/lists — list all task lists
|
||||
if (sub === 'lists') {
|
||||
const taskListId = segments[3]
|
||||
const taskId = segments[4]
|
||||
|
||||
if (req.method === 'POST' && taskListId && taskId === 'reset') {
|
||||
await resetTaskList(taskListId)
|
||||
return Response.json({ ok: true })
|
||||
}
|
||||
|
||||
if (req.method !== 'GET') {
|
||||
throw new ApiError(
|
||||
405,
|
||||
`Method ${req.method} not allowed on /api/tasks/lists`,
|
||||
'METHOD_NOT_ALLOWED',
|
||||
)
|
||||
}
|
||||
|
||||
if (taskListId && taskId) {
|
||||
// GET /api/tasks/lists/:taskListId/:taskId
|
||||
const task = await taskService.getTask(taskListId, taskId)
|
||||
@ -162,6 +168,14 @@ async function handleTasksApi(
|
||||
return Response.json({ lists })
|
||||
}
|
||||
|
||||
if (req.method !== 'GET') {
|
||||
throw new ApiError(
|
||||
405,
|
||||
`Method ${req.method} not allowed on /api/tasks`,
|
||||
'METHOD_NOT_ALLOWED',
|
||||
)
|
||||
}
|
||||
|
||||
// GET /api/tasks — list all tasks
|
||||
const tasks = await taskService.listTasks()
|
||||
return Response.json({ tasks })
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user