fix(tasks): count indented sub-tasks so a change with unfinished work isn't reported complete

.changeset/count-indented-subtasks.md

@@ -0,0 +1,7 @@
+---
+'@fission-ai/openspec': patch
+---
+
+Task progress now counts indented sub-tasks. A `tasks.md` whose sub-tasks were unfinished reported `✓ Complete` in `openspec list` and `openspec view`, was missing those tasks from the `openspec instructions apply` list, and archived with no incomplete-task warning, because both checkbox parsers only matched checkboxes at column 0.
+
+Progress counting and the apply task list now share one parser, so `list`, `view`, `archive` and `apply` agree about which lines of a tasks file are tasks. A checkbox with no text after it is left out of the apply list, which has nothing to act on, but still counts toward every progress number; a file of nothing but such checkboxes now asks to be rewritten rather than reporting itself done. The shared pattern matches every line the two it replaced matched, and more, so task counts can rise but never fall: no change starts reporting less work than before, and archive's incomplete-task warning can only become stricter. Checkboxes are still counted wherever they appear, including inside a code fence, an HTML comment or an indented block, so a `tasks.md` that shows a checklist as a format example can now count that example as work — remove it from the file, or pass `--yes` to archive.

src/commands/workflow/instructions.ts

@@ -46,6 +46,7 @@ import {
   type ApplyInstructions,
   type ArchiveInstructions,
 } from './shared.js';
+import { parseTaskLines, type ParsedTask } from '../../utils/task-progress.js';
 
 // -----------------------------------------------------------------------------
 // Types
@@ -323,26 +324,26 @@ export function printInstructionsText(instructions: ArtifactInstructions, isBloc
 // -----------------------------------------------------------------------------
 
 /**
- * Parses tasks.md content and extracts task items with their completion status.
+ * Turns parsed task lines into the listed task items.
+ *
+ * A checkbox with no text after it is left out of the list: this is work for an
+ * agent to act on and tick off, and a bare `- [ ]` gives it nothing to match.
+ * It still counts toward progress, which is taken from every parsed line, so
+ * this list can be shorter than the totals beside it but never disagrees with
+ * `openspec list` or archive about how much work is left. An empty list is also
+ * what puts apply in its "nothing to work on" state, so a file of nothing but
+ * text-less checkboxes asks to be rewritten instead of being called done.
  */
-function parseTasksFile(content: string): TaskItem[] {
+function toTaskItems(parsed: ParsedTask[]): TaskItem[] {
   const tasks: TaskItem[] = [];
-  const lines = content.split('\n');
-  let taskIndex = 0;
-
-  for (const line of lines) {
-    // Match checkbox patterns: - [ ] or - [x] or - [X]
-    const checkboxMatch = line.match(/^[-*]\s*\[([ xX])\]\s*(.+)\s*$/);
-    if (checkboxMatch) {
-      taskIndex++;
-      const done = checkboxMatch[1].toLowerCase() === 'x';
-      const description = checkboxMatch[2].trim();
-      tasks.push({
-        id: `${taskIndex}`,
-        description,
-        done,
-      });
-    }
+
+  for (const task of parsed) {
+    if (task.description.length === 0) continue;
+    tasks.push({
+      id: `${tasks.length + 1}`,
+      description: task.description,
+      done: task.done,
+    });
   }
 
   return tasks;
@@ -411,20 +412,22 @@ export async function generateApplyInstructions(
   }
 
   // Parse tasks if tracking file exists
-  let tasks: TaskItem[] = [];
+  let parsedTasks: ParsedTask[] = [];
   let tracksFileExists = false;
   if (tracksFile) {
     const tracksPath = path.join(changeDir, tracksFile);
     tracksFileExists = fs.existsSync(tracksPath);
     if (tracksFileExists) {
       const tasksContent = await fs.promises.readFile(tracksPath, 'utf-8');
-      tasks = parseTasksFile(tasksContent);
+      parsedTasks = parseTaskLines(tasksContent);
     }
   }
+  const tasks = toTaskItems(parsedTasks);
 
-  // Calculate progress
-  const total = tasks.length;
-  const complete = tasks.filter((t) => t.done).length;
+  // Calculate progress over every checkbox in the file, listed or not, so these
+  // numbers match `openspec list` and archive's incomplete-task check.
+  const total = parsedTasks.length;
+  const complete = parsedTasks.filter((task) => task.done).length;
   const remaining = total - complete;
 
   // Determine state and instruction
@@ -439,11 +442,12 @@ export async function generateApplyInstructions(
     const tracksFilename = path.basename(tracksFile);
     state = 'blocked';
     instruction = `The ${tracksFilename} file is missing and must be created.\nUse openspec-continue-change to generate the tracking file.`;
-  } else if (tracksFile && tracksFileExists && total === 0) {
-    // Tracking file exists but contains no tasks
+  } else if (tracksFile && tracksFileExists && tasks.length === 0) {
+    // Tracking file exists but lists nothing an agent can work on: either no
+    // checkboxes at all, or only checkboxes with no text after them.
     const tracksFilename = path.basename(tracksFile);
     state = 'blocked';
-    instruction = `The ${tracksFilename} file exists but contains no tasks.\nAdd tasks to ${tracksFilename} or regenerate it with openspec-continue-change.`;
+    instruction = `The ${tracksFilename} file exists but contains no tasks to work on.\nAdd tasks to ${tracksFilename} or regenerate it with openspec-continue-change.`;
   } else if (tracksFile && remaining === 0 && total > 0) {
     state = 'all_done';
     instruction = 'All tasks are complete! This change is ready to be archived.\nConsider running tests and reviewing the changes before archiving.';

src/utils/task-progress.ts

@@ -4,27 +4,65 @@ import type { Artifact, SchemaYaml } from '../core/artifact-graph/index.js';
 import { resolveArtifactOutputs, resolveSchema } from '../core/artifact-graph/index.js';
 import { resolveSchemaForChange } from './change-metadata.js';
 
-const TASK_PATTERN = /^[-*]\s+\[[\sx]\]/i;
-const COMPLETED_TASK_PATTERN = /^[-*]\s+\[x\]/i;
+/**
+ * A Markdown task line: a `-`/`*` bullet carrying a `[ ]` or `[x]` checkbox.
+ *
+ * Leading whitespace is allowed so nested sub-tasks count like their parents.
+ * Anchoring at column 0 made `  - [ ] 1.1.1 ...` invisible to progress, to the
+ * apply task list, and to archive's incomplete-task check, so a change with
+ * unfinished sub-tasks reported "✓ Complete" and archived without a warning.
+ *
+ * Permissive on purpose, and safe to keep that way: any character class
+ * tightened here - the `\s` inside the brackets, which lets a tab or
+ * non-breaking space stand for an empty box - drops lines that used to count,
+ * and a task this parser drops is a task `openspec archive` stops warning about.
+ *
+ * Deliberately unanchored at the end: `.` does not match `\r`, so writing the
+ * description group as `(.*)$` would reject every line of a CRLF tasks.md.
+ */
+const TASK_LINE_PATTERN = /^\s*[-*]\s*\[([\sxX])\]\s*(.*)/;
+
+export interface ParsedTask {
+  /** Checkbox state: `[x]`/`[X]` is done, anything else is not. */
+  done: boolean;
+  /** Task text after the checkbox, trimmed (may be empty). */
+  description: string;
+}
+
+/**
+ * Parses every task line in a tasks file, in document order.
+ *
+ * Every line matching the pattern counts, wherever it sits - inside a code
+ * fence, an HTML comment or an indented block, as before. Skipping fenced
+ * checkboxes was tried and dropped: every rule for deciding which fence is
+ * "real" has an input where a stray or unbalanced ``` swallows genuine tasks.
+ * Counting a documented example as work is a loud, bypassable false positive;
+ * losing a real task is a silent one.
+ */
+export function parseTaskLines(content: string): ParsedTask[] {
+  const tasks: ParsedTask[] = [];
+
+  for (const line of content.split('\n')) {
+    const match = line.match(TASK_LINE_PATTERN);
+    if (match) {
+      tasks.push({ done: match[1].toLowerCase() === 'x', description: match[2].trim() });
+    }
+  }
+
+  return tasks;
+}
 
 export interface TaskProgress {
   total: number;
   completed: number;
 }
 
 export function countTasksFromContent(content: string): TaskProgress {
-  const lines = content.split('\n');
-  let total = 0;
-  let completed = 0;
-  for (const line of lines) {
-    if (line.match(TASK_PATTERN)) {
-      total++;
-      if (line.match(COMPLETED_TASK_PATTERN)) {
-        completed++;
-      }
-    }
-  }
-  return { total, completed };
+  const tasks = parseTaskLines(content);
+  return {
+    total: tasks.length,
+    completed: tasks.filter((task) => task.done).length,
+  };
 }
 
 /**

test/commands/apply-instructions-tasks.test.ts

@@ -0,0 +1,118 @@
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import * as fs from 'node:fs';
+import * as path from 'node:path';
+import * as os from 'node:os';
+import { generateApplyInstructions } from '../../src/commands/workflow/instructions.js';
+import { getTaskProgressForChange } from '../../src/utils/task-progress.js';
+
+/**
+ * The apply task list and task progress read the same tasks file, so they must
+ * see the same tasks - including indented sub-tasks, which the apply parser
+ * used to drop.
+ */
+describe('generateApplyInstructions task list', () => {
+  let tempDir: string;
+  let changeDir: string;
+
+  beforeEach(() => {
+    tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-apply-tasks-'));
+    changeDir = path.join(tempDir, 'openspec', 'changes', 'my-change');
+    fs.mkdirSync(path.join(changeDir, 'specs', 'demo'), { recursive: true });
+    fs.writeFileSync(path.join(changeDir, '.openspec.yaml'), 'schema: spec-driven\n');
+    fs.writeFileSync(path.join(changeDir, 'proposal.md'), '## Why\nx\n');
+    fs.writeFileSync(
+      path.join(changeDir, 'specs', 'demo', 'spec.md'),
+      '## ADDED Requirements\n\n### Requirement: Demo\nThe system SHALL demo.\n\n#### Scenario: Works\n- **WHEN** run\n- **THEN** works\n'
+    );
+  });
+
+  afterEach(() => {
+    fs.rmSync(tempDir, { recursive: true, force: true });
+  });
+
+  function writeTasks(content: string): void {
+    fs.writeFileSync(path.join(changeDir, 'tasks.md'), content);
+  }
+
+  it('lists indented sub-tasks alongside their parents', async () => {
+    writeTasks(
+      [
+        '## 1. Implementation',
+        '- [x] 1.1 Parent task',
+        '  - [ ] 1.1.1 Unfinished sub-task',
+        '- [ ] 1.2 Second parent',
+        '',
+      ].join('\n')
+    );
+
+    const instructions = await generateApplyInstructions(tempDir, 'my-change');
+
+    expect(instructions.tasks.map((task) => task.description)).toEqual([
+      '1.1 Parent task',
+      '1.1.1 Unfinished sub-task',
+      '1.2 Second parent',
+    ]);
+    expect(instructions.progress).toEqual({ total: 3, complete: 1, remaining: 2 });
+  });
+
+  it('reports the totals openspec list reports for the same change', async () => {
+    writeTasks(
+      ['## 1. Implementation', '- [x] 1.1 Parent task', '  - [ ] 1.1.1 Unfinished sub-task', ''].join(
+        '\n'
+      )
+    );
+
+    const instructions = await generateApplyInstructions(tempDir, 'my-change');
+    // `openspec list` reads progress through getTaskProgressForChange, not the
+    // apply parser. The two must not disagree about the same file.
+    const listProgress = await getTaskProgressForChange(
+      path.join(tempDir, 'openspec', 'changes'),
+      'my-change',
+      tempDir
+    );
+
+    expect(listProgress).toEqual({ total: 2, completed: 1 });
+    expect(instructions.progress.total).toBe(listProgress.total);
+    expect(instructions.progress.complete).toBe(listProgress.completed);
+  });
+
+  it('reports a file of text-less checkboxes as having nothing to work on', async () => {
+    writeTasks('## 1. Implementation\n- [x]\n');
+
+    const instructions = await generateApplyInstructions(tempDir, 'my-change');
+
+    // As before the shared parser: apply points at regenerating the file
+    // rather than listing a blank row an agent cannot act on.
+    expect(instructions.tasks).toEqual([]);
+    expect(instructions.state).toBe('blocked');
+    expect(instructions.instruction).toContain('contains no tasks');
+  });
+
+  it('counts a text-less checkbox toward progress even though it lists none', async () => {
+    // Progress must not disagree with `openspec list` or archive's gate just
+    // because a line carries no text an agent could act on: hiding the row is
+    // presentation, dropping it from the count would understate the work left.
+    writeTasks('## 1. Implementation\n- [x] 1.1 Real task\n- [ ]   \n');
+
+    const instructions = await generateApplyInstructions(tempDir, 'my-change');
+    const listProgress = await getTaskProgressForChange(
+      path.join(tempDir, 'openspec', 'changes'),
+      'my-change',
+      tempDir
+    );
+
+    expect(instructions.tasks.map((task) => task.description)).toEqual(['1.1 Real task']);
+    expect(instructions.progress).toEqual({ total: 2, complete: 1, remaining: 1 });
+    expect(instructions.state).toBe('ready');
+    expect(listProgress).toEqual({ total: 2, completed: 1 });
+  });
+
+  it('does not call a change done while a bare checkbox is still unchecked', async () => {
+    writeTasks('## 1. Implementation\n- [x] 1.1 Real task\n- [ ]\n');
+
+    const instructions = await generateApplyInstructions(tempDir, 'my-change');
+
+    expect(instructions.progress).toEqual({ total: 2, complete: 1, remaining: 1 });
+    expect(instructions.state).toBe('ready');
+  });
+});

test/core/archive.test.ts

@@ -236,6 +236,31 @@ describe('ArchiveCommand', () => {
       );
     });
 
+    it('detects incomplete indented sub-tasks (#1485 data-safety gate)', async () => {
+      // Before the fix the gate only saw checkboxes at column 0, so a change
+      // whose sub-tasks were unfinished archived with no warning at all.
+      const changeName = 'nested-subtasks-feature';
+      const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
+      await fs.mkdir(changeDir, { recursive: true });
+      await fs.writeFile(
+        path.join(changeDir, 'tasks.md'),
+        [
+          '## 1. Implementation',
+          '- [x] 1.1 Parent task',
+          '  - [ ] 1.1.1 Unfinished sub-task',
+          '  - [ ] 1.1.2 Another unfinished sub-task',
+          '- [x] 1.2 Second parent',
+          '',
+        ].join('\n')
+      );
+
+      await archiveCommand.execute(changeName, { yes: true });
+
+      expect(console.log).toHaveBeenCalledWith(
+        expect.stringContaining('Warning: 2 incomplete task(s) found')
+      );
+    });
+
     it('should update specs when archiving (delta-based ADDED) and include change name in skeleton', async () => {
       const changeName = 'spec-feature';
       const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
@@ -2767,6 +2792,38 @@ The system SHALL do the thing differently.
       // Verify change was not archived
       await expect(fs.access(changeDir)).resolves.not.toThrow();
     });
+
+    it('prompts before archiving a change whose only unfinished work is a sub-task (#1485)', async () => {
+      // The other half of the gate: without --yes the user is asked, and
+      // declining leaves the change in place. Before the fix there was no
+      // question to answer - the sub-task was invisible and archive ran.
+      const { confirm } = await import('@inquirer/prompts');
+      const mockConfirm = confirm as unknown as ReturnType<typeof vi.fn>;
+
+      const changeName = 'subtask-prompt';
+      const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
+      await fs.mkdir(changeDir, { recursive: true });
+      await fs.writeFile(
+        path.join(changeDir, 'tasks.md'),
+        '- [x] 1.1 Parent task\n  - [ ] 1.1.1 Unfinished sub-task\n'
+      );
+
+      // Drain answers queued by earlier tests: vi.clearAllMocks() resets calls
+      // but not a pending mockResolvedValueOnce queue.
+      mockConfirm.mockReset();
+      // First confirm is the skip-validation prompt, second is the task warning.
+      mockConfirm.mockResolvedValueOnce(true);
+      mockConfirm.mockResolvedValueOnce(false);
+
+      await archiveCommand.execute(changeName, { noValidate: true });
+
+      expect(mockConfirm).toHaveBeenCalledWith({
+        message: 'Warning: 1 incomplete task(s) found. Continue?',
+        default: false,
+      });
+      expect(console.log).toHaveBeenCalledWith('Archive cancelled.');
+      await expect(fs.access(changeDir)).resolves.not.toThrow();
+    });
   });
 
   describe('proposal warnings (#498)', () => {

test/core/list.test.ts

@@ -114,6 +114,22 @@ Regular text that should be ignored
       expect(logOutput.some(line => line.includes('✓ Complete'))).toBe(true);
     });
 
+    it('does not report a change with unfinished sub-tasks as complete (#1485)', async () => {
+      const changesDir = path.join(tempDir, 'openspec', 'changes');
+      await fs.mkdir(path.join(changesDir, 'nested-change'), { recursive: true });
+
+      await fs.writeFile(
+        path.join(changesDir, 'nested-change', 'tasks.md'),
+        '- [x] 1.1 Parent task\n  - [ ] 1.1.1 Unfinished sub-task\n'
+      );
+
+      const listCommand = new ListCommand();
+      await listCommand.execute(tempDir, 'changes');
+
+      expect(logOutput.some(line => line.includes('1/2 tasks'))).toBe(true);
+      expect(logOutput.some(line => line.includes('✓ Complete'))).toBe(false);
+    });
+
     it('should handle changes without tasks.md', async () => {
       const changesDir = path.join(tempDir, 'openspec', 'changes');
       await fs.mkdir(path.join(changesDir, 'no-tasks'), { recursive: true });