From c0d25dd275366cad2815dfcd1214cb6cc0be9ec7 Mon Sep 17 00:00:00 2001 From: Vishal Habib Date: Mon, 12 Oct 2026 20:02:57 -0400 Subject: [PATCH] Fix(tasks): list_tasks and zn task list leave cancelled tasks out of open A [-] cancelled task is abandoned, and isTaskOpen already keeps it off the calendar and Kanban surfaces (#450, #476). The MCP list_tasks "open" filter and the zn task list default/--unchecked filters still returned it next to actionable tasks. Exclude t.cancelled in both; status "all" and --all still list it. --- apps/desktop/src/cli/commands/tasks.test.ts | 66 +++++++++++++++++++++ apps/desktop/src/cli/commands/tasks.ts | 7 ++- apps/desktop/src/mcp/server.test.ts | 36 +++++++++++ apps/desktop/src/mcp/server.ts | 7 ++- 4 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 apps/desktop/src/cli/commands/tasks.test.ts diff --git a/apps/desktop/src/cli/commands/tasks.test.ts b/apps/desktop/src/cli/commands/tasks.test.ts new file mode 100644 index 000000000..e80deb088 --- /dev/null +++ b/apps/desktop/src/cli/commands/tasks.test.ts @@ -0,0 +1,66 @@ +import { promises as fsp } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { createBackend } from '../backend' +import type { ParsedArgs } from '../args' +import { cmdTaskList } from './tasks' + +function makeArgs(flags: Array<[string, string]> = []): ParsedArgs { + const map = new Map() + for (const [k, v] of flags) map.set(k, [...(map.get(k) ?? []), v]) + return { positionals: [], flags: map } +} + +let tmpDir: string +let root: string +let out: string[] + +beforeAll(async () => { + tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'zen-task-cli-')) + root = path.join(tmpDir, 'vault') + for (const d of ['inbox', 'quick', 'archive', 'trash']) { + await fsp.mkdir(path.join(root, d), { recursive: true }) + } + await fsp.writeFile( + path.join(root, 'inbox', 'todo.md'), + '- [ ] Buy milk\n- [-] Cancelled plan\n- [x] Done thing\n' + ) +}) + +afterAll(async () => { + await fsp.rm(tmpDir, { recursive: true, force: true }) +}) + +beforeEach(() => { + out = [] + vi.spyOn(process.stdout, 'write').mockImplementation((chunk) => { + out.push(String(chunk)) + return true + }) +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + +const backend = (): ReturnType => createBackend({ kind: 'local', root }) +const listed = (): string[] => + (JSON.parse(out.join('')) as Array<{ content: string }>).map((t) => t.content) + +describe('zn task list', () => { + it('leaves `[-]` cancelled tasks out of the open list (#450, #476)', async () => { + await cmdTaskList(backend(), makeArgs([['json', 'true']])) + expect(listed()).toEqual(['Buy milk']) + }) + + it('leaves cancelled tasks out of --unchecked', async () => { + await cmdTaskList(backend(), makeArgs([['unchecked', 'true'], ['json', 'true']])) + expect(listed()).toEqual(['Buy milk']) + }) + + it('still lists cancelled tasks with --all', async () => { + await cmdTaskList(backend(), makeArgs([['all', 'true'], ['json', 'true']])) + expect(listed()).toContain('Cancelled plan') + }) +}) diff --git a/apps/desktop/src/cli/commands/tasks.ts b/apps/desktop/src/cli/commands/tasks.ts index 52455e260..bc252d492 100644 --- a/apps/desktop/src/cli/commands/tasks.ts +++ b/apps/desktop/src/cli/commands/tasks.ts @@ -20,9 +20,10 @@ export async function cmdTaskList(vault: VaultBackend, args: ParsedArgs): Promis if (!showAll) { // A `[>]` forwarded record is history of a move; its live copy is in the // destination note and lists on its own, so showing both doubled every - // carried task (#611 review). `--all` still surfaces the records. - if (onlyUnchecked) tasks = tasks.filter((t) => !t.checked && !t.forwarded) - else tasks = tasks.filter((t) => !t.checked && !t.waiting && !t.forwarded) + // carried task (#611 review). A `[-]` cancelled task is abandoned, so it is + // not open either (#450, #476). `--all` still surfaces both. + if (onlyUnchecked) tasks = tasks.filter((t) => !t.checked && !t.forwarded && !t.cancelled) + else tasks = tasks.filter((t) => !t.checked && !t.waiting && !t.forwarded && !t.cancelled) } if (tag) tasks = tasks.filter((t) => t.tags.includes(tag)) diff --git a/apps/desktop/src/mcp/server.test.ts b/apps/desktop/src/mcp/server.test.ts index 3e63d32e0..d1a427335 100644 --- a/apps/desktop/src/mcp/server.test.ts +++ b/apps/desktop/src/mcp/server.test.ts @@ -141,6 +141,42 @@ describe('tools run through the backend', () => { }) }) +describe('list_tasks', () => { + const task = (content: string, extra: Record = {}): unknown => ({ + content, + noteFolder: 'inbox', + checked: false, + waiting: false, + forwarded: false, + cancelled: false, + tags: [], + ...extra + }) + const tasksBackend = backend({ + scanAllTasks: async () => + [ + task('Buy milk', { due: '2026-10-20' }), + task('Cancelled plan', { cancelled: true, due: '2026-10-15' }), + task('Done thing', { checked: true }) + ] as never + }) + const contents = (rows: unknown): string[] => + (rows as Array<{ content: string }>).map((t) => t.content) + + it('leaves `[-]` cancelled tasks out of the default open list (#450, #476)', async () => { + expect(contents(await callTool('list_tasks', {}, tasksBackend))).toEqual(['Buy milk']) + expect( + contents(await callTool('list_tasks', { dueBefore: '2026-10-31' }, tasksBackend)) + ).toEqual(['Buy milk']) + }) + + it('still returns cancelled tasks for status "all"', async () => { + expect(contents(await callTool('list_tasks', { status: 'all' }, tasksBackend))).toContain( + 'Cancelled plan' + ) + }) +}) + describe('describeToolError', () => { it('turns a 401 from the server into the token hint', () => { const text = describeToolError( diff --git a/apps/desktop/src/mcp/server.ts b/apps/desktop/src/mcp/server.ts index 3fed6e0ff..ad703c32f 100644 --- a/apps/desktop/src/mcp/server.ts +++ b/apps/desktop/src/mcp/server.ts @@ -673,7 +673,7 @@ const TOOLS: ToolDef[] = [ type: 'string', enum: ['open', 'done', 'waiting', 'all'], description: - 'open = unchecked & not waiting (default). done = checked. waiting = has @waiting. all = everything.' + 'open = not done, cancelled or waiting (default). done = checked. waiting = has @waiting. all = everything.' }, priority: { type: 'string', @@ -721,7 +721,10 @@ const TOOLS: ToolDef[] = [ if (folder && t.noteFolder !== folder) return false // A `[>]` record's live copy sits in the destination note; listing the // record as open doubled every carried task for agents (#611 review). - if (status === 'open' && (t.checked || t.waiting || t.forwarded)) return false + // A `[-]` cancelled task is abandoned, so it is not open either (#450, #476). + if (status === 'open' && (t.checked || t.waiting || t.forwarded || t.cancelled)) { + return false + } if (status === 'done' && !t.checked) return false if (status === 'waiting' && !t.waiting) return false if (priority && t.priority !== priority) return false