From f8a0e6f145e5e96c1ea9aef921d16e8714b48027 Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Sun, 26 Apr 2026 10:18:08 -0400
Subject: [PATCH] test(query): add Vitest regression tests for
 milestone.complete version arg (fix #2644)

milestoneComplete was imported in decomposed-handlers.test.ts but had zero
test coverage. The original defect (6f79b1d) called phasesArchive([], ...)
instead of forwarding the positional version arg; the wrapping try/catch
swallowed the GSDError into { completed: false, reason: String(err) },
masking a programming error as a legitimate negative answer.

Add five Vitest tests that lock in the correct contract:
- positional version arg is extracted from args[0] and echoed in response
- missing version throws GSDError (not masked as completed: false)
- --archive-phases flag is processed
- --name flag sets milestone name
- response shape has version/date/phases/milestones_updated fields

Closes #2644

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 sdk/package-lock.json                     |  2 +-
 sdk/src/query/decomposed-handlers.test.ts | 68 +++++++++++++++++++++++
 2 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/sdk/package-lock.json b/sdk/package-lock.json
index 81d09b38f..2c5feb86b 100644
--- a/sdk/package-lock.json
+++ b/sdk/package-lock.json
@@ -22,7 +22,7 @@
         "vitest": "^3.1.1"
       },
       "engines": {
-        "node": ">=20"
+        "node": ">=22.0.0"
       }
     },
     "node_modules/@anthropic-ai/claude-agent-sdk": {
diff --git a/sdk/src/query/decomposed-handlers.test.ts b/sdk/src/query/decomposed-handlers.test.ts
index a4ca5e376..806e44228 100644
--- a/sdk/src/query/decomposed-handlers.test.ts
+++ b/sdk/src/query/decomposed-handlers.test.ts
@@ -170,6 +170,74 @@ describe('progressBar', () => {
   });
 });
 
+// ─── phase-lifecycle.ts — milestoneComplete ──────────────────────────────
+
+/**
+ * Regression tests for bug #2644: milestone.complete handler drops version arg.
+ *
+ * Original defect (first introduced in 6f79b1d): the handler called
+ * `phasesArchive([], projectDir)` instead of forwarding the version positional
+ * arg. phasesArchive read args[0] and threw GSDError('version required for
+ * phases archive'); the surrounding try/catch swallowed the throw into
+ * { completed: false, reason: String(err) }, masking it as a legitimate
+ * negative answer.
+ *
+ * Fixed in c5b1445: handler now validates version upfront and uses inline
+ * archive logic instead of delegating to phasesArchive.
+ */
+describe('milestoneComplete', () => {
+  it('accepts version as first positional arg and returns it in data', async () => {
+    const result = await milestoneComplete(['v1.19', '--name', 'Test Milestone'], tmpDir);
+    const data = result.data as Record<string, unknown>;
+
+    // Must NOT return the error shape from the old bug
+    expect(data.completed).not.toBe(false);
+    expect((data as Record<string, unknown>).reason).toBeUndefined();
+
+    // Must return version echoed in data
+    expect(data.version).toBe('v1.19');
+  });
+
+  it('does not call phasesArchive with empty args (regression: bug #2644)', async () => {
+    // If the old bug were present, this would return { completed: false, reason: 'GSDError: version required for phases archive' }
+    // The fix ensures version is extracted from args[0] before any archive operation
+    const result = await milestoneComplete(['v1.0'], tmpDir);
+    const data = result.data as Record<string, unknown>;
+
+    // Should return the success shape with version, never the error shape
+    expect(data.version).toBe('v1.0');
+    expect(typeof data.date).toBe('string');
+    expect(typeof data.phases).toBe('number');
+    expect(data.milestones_updated).toBe(true);
+  });
+
+  it('throws GSDError when version arg is missing (not masked as completed: false)', async () => {
+    // The old bug swallowed ALL errors into { completed: false, reason: String(err) }
+    // The fix explicitly throws so callers can distinguish validation failure from "not complete"
+    await expect(milestoneComplete([], tmpDir)).rejects.toThrow('version required for milestone complete');
+  });
+
+  it('archives with --archive-phases when flag is present', async () => {
+    const result = await milestoneComplete(['v1.0', '--archive-phases'], tmpDir);
+    const data = result.data as Record<string, unknown>;
+
+    expect(data.version).toBe('v1.0');
+    const archived = data.archived as Record<string, unknown>;
+    // --archive-phases was passed; phases dir should have been scoped but
+    // may result in 0 if the milestone filter finds no matching dirs.
+    // The important assertion: no error, version is correctly forwarded.
+    expect(typeof archived.phases).toBe('boolean');
+  });
+
+  it('returns name from --name flag', async () => {
+    const result = await milestoneComplete(['v2.0', '--name', 'My Release'], tmpDir);
+    const data = result.data as Record<string, unknown>;
+
+    expect(data.version).toBe('v2.0');
+    expect(data.name).toBe('My Release');
+  });
+});
+
 // ─── summary.ts ──────────────────────────────────────────────────────────
 
 describe('summaryExtract', () => {