Skip to content

feat(tasks): add interactive status management - #1377

Open
mikemikimike wants to merge 2 commits into
Nano-Collective:mainfrom
mikemikimike:fix/tasks-status-1307
Open

mikemikimike wants to merge 2 commits into
Nano-Collective:mainfrom
mikemikimike:fix/tasks-status-1307

Conversation

@mikemikimike

Copy link
Copy Markdown
Contributor

Summary

  • add /tasks list, /tasks done <number>, /tasks complete <number>, and /tasks start <number>
  • update task status and timestamps while preserving task artifacts
  • validate task numbers and reject unknown subcommands instead of creating an unintended task
  • add regression coverage for status updates, aliases, validation, and explicit listing

Fixes #1307

Testing

  • pnpm run build
  • pnpm run test:ava source/commands/tasks.spec.ts source/tools/tasks/storage.spec.ts source/components/task-list-display.spec.tsx
  • pnpm test:format
  • pnpm test:lint
  • pnpm test:types
  • pnpm test:types:vscode
  • pnpm test:knip
  • pnpm test:audit --audit-level=high --ignore-unfixable

@github-actions

Copy link
Copy Markdown
Contributor

nc-review: comments — 2 important, 2 nits

@mikemikimike — a few things worth a look, none blocking.

Adds /tasks list, /tasks done|complete <n>, and /tasks start <n> plus validation for unknown subcommands to source/commands/tasks.tsx, matching the four asks in #1307. The implementation mutates a freshly-parsed Task in place, sets updatedAt, sets/clears completedAt correctly, and rejects unknown subcommands. Tests cover the new paths, but a regression test that exercises the original symptom from the issue (/tasks done 1 no longer creates a task titled done 1) is missing, and the new tests do not assert that the mutated task's title/id are preserved despite the PR description claiming that.

🟠 important · tests · source/commands/tasks.spec.ts

The PR description claims the change update[s] task status and timestamps while preserving task artifacts, but the regression tests only assert the new status/timestamp fields; they never assert that the mutated task's title, description (if any), or id survived the update. A test for done 1 should pin t.is(tasks[0]?.title, 'First Task') and t.is(tasks[0]?.id, 'task-1') so a future regression that drops or rewrites those fields is caught. The same applies to the start 3 case (verify the title on task 3 is unchanged).

🟠 important · completeness · source/commands/tasks.spec.ts

The headline symptom in #1307 is that /tasks done 1 silently created a task titled done 1. The new unknown-subcommand test covers a typo (completee), which is good, but there is no test that proves /tasks done 1 is no longer treated as a title. Adding one — e.g. save three sample tasks, run tasksCommand.handler(['done', '1']), and assert tasks.length === 3 and tasks[0]?.status === 'completed' — closes the loop on the issue and guards against regressing straight back to the old behaviour.

⚪ nit · design · source/commands/tasks.tsx

The remove path validates with parseInt(rest.trim(), 10) (which silently accepts 3abc as 3), while the new done/complete/start path uses Number(...) + Number.isInteger(...) (strictly rejects 3abc). The new path is the better behaviour, but the inconsistency is now visible in the same file. Worth aligning — either both strict, or both lenient — so the two branches do not disagree about what a task number looks like.

⚪ nit · tests · source/commands/tasks.spec.ts

The list and unknown-subcommand tests only assert t.truthy(result). They do not verify the error/list surface (e.g. via the rendered message) nor that list is equivalent to the bare /tasks invocation in terms of what is rendered. A more discriminating assertion would distinguish a successful list from any other rendered React element.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:comments nc-review left non-blocking findings label Sep 18, 2026
@mikemikimike

Copy link
Copy Markdown
Contributor Author

/re-review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Add interactive task completion and status management to /tasks (/tasks done <n>)

1 participant