From de600c1db01af89356b0d32821f351e6f6971644 Mon Sep 17 00:00:00 2001
From: Iouri Goussev <i.gouss@gmail.com>
Date: Thu, 26 Mar 2026 20:14:43 -0400
Subject: [PATCH] refactor(gsd): extract duplicated status guards and
 validation helpers (#2767)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* fix: rebuild stale workspace packages after git pull

ensure-workspace-builds.cjs only triggered a build when dist/index.js
was missing entirely. After `git pull` updates package sources, the old
dist/ stayed in place causing TypeScript type errors (bash_transform,
authMode, malformedArguments missing from compiled .d.ts files).

Now compares newest .ts mtime under src/ against dist/index.js mtime
and rebuilds any package whose sources are newer than its dist.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(rtk): trust explicit binaryPath without existsSync check; add options object to shared rewriteCommandWithRtk

resolveRtkBinaryPath was calling existsSync on options.binaryPath, making
it impossible to inject a non-existent test binary — tests expected the
options-object API to bypass filesystem checks.

Also brings src/resources/extensions/shared/rtk.ts rewriteCommandWithRtk
in line with the same options-object signature already in src/rtk.ts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* refactor(gsd): extract duplicated status guards and validation helpers

isClosedStatus(), isNonEmptyString(), and validateStringArray() were
each copy-pasted across 5-10 tool handler files with no shared module.
Extract them into status-guards.ts and validation.ts, replace all 26
inline status checks and 8 duplicated validation functions with imports.

Standardizes "inside a closed" -> "in a closed" in two reopen error
messages as a side effect of the normalization pass.

Closes #2727

* refactor(gsd): migrate state.ts isStatusDone to isClosedStatus; fix blank lines and import order

- state.ts had a private isStatusDone() identical to isClosedStatus() —
  replace with import from status-guards.ts
- Remove double blank lines left behind in plan-{milestone,slice,task}.ts
  and replan-slice.ts after local function extraction
- Fix import ordering in reassess-roadmap.ts (node built-ins first,
  status-guards/validation before gsd-db block)

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 scripts/ensure-workspace-builds.cjs           | 44 +++++++++---
 .../extensions/gsd/markdown-renderer.ts       |  9 +--
 src/resources/extensions/gsd/state.ts         | 32 ++++-----
 src/resources/extensions/gsd/status-guards.ts | 13 ++++
 .../gsd/tests/status-guards.test.ts           | 30 ++++++++
 .../extensions/gsd/tests/validation.test.ts   | 72 +++++++++++++++++++
 .../gsd/tools/complete-milestone.ts           |  7 +-
 .../extensions/gsd/tools/complete-slice.ts    |  7 +-
 .../extensions/gsd/tools/complete-task.ts     |  7 +-
 .../extensions/gsd/tools/plan-milestone.ts    | 20 ++----
 .../extensions/gsd/tools/plan-slice.ts        | 20 ++----
 .../extensions/gsd/tools/plan-task.ts         | 20 ++----
 .../extensions/gsd/tools/reassess-roadmap.ts  | 13 ++--
 .../extensions/gsd/tools/reopen-slice.ts      |  7 +-
 .../extensions/gsd/tools/reopen-task.ts       |  9 +--
 .../extensions/gsd/tools/replan-slice.ts      | 12 ++--
 src/resources/extensions/gsd/validation.ts    | 23 ++++++
 src/resources/extensions/shared/rtk.ts        | 15 +++-
 src/rtk.ts                                    |  3 +-
 19 files changed, 250 insertions(+), 113 deletions(-)
 create mode 100644 src/resources/extensions/gsd/status-guards.ts
 create mode 100644 src/resources/extensions/gsd/tests/status-guards.test.ts
 create mode 100644 src/resources/extensions/gsd/tests/validation.test.ts
 create mode 100644 src/resources/extensions/gsd/validation.ts

diff --git a/scripts/ensure-workspace-builds.cjs b/scripts/ensure-workspace-builds.cjs
index ddbba3488..840a818d4 100644
--- a/scripts/ensure-workspace-builds.cjs
+++ b/scripts/ensure-workspace-builds.cjs
@@ -3,15 +3,18 @@
  * ensure-workspace-builds.cjs
  *
  * Checks whether workspace packages have been compiled (dist/ exists with
- * index.js). If any are missing, runs the build for those packages.
+ * index.js) and that the build is not stale (no src/ file newer than dist/).
+ * If any are missing or stale, runs the build for those packages.
  *
  * Designed for the postinstall hook so that `npm install` in a fresh clone
- * produces a working runtime without a manual `npm run build` step.
+ * produces a working runtime without a manual `npm run build` step. Also
+ * catches the common case where `git pull` updates package sources but the
+ * old dist/ remains, causing TypeScript type errors.
  *
  * Skipped in CI (where the full build pipeline handles this) and when
  * installing as an end-user dependency (no packages/ directory).
  */
-const { existsSync } = require('fs')
+const { existsSync, statSync, readdirSync } = require('fs')
 const { resolve, join } = require('path')
 const { execSync } = require('child_process')
 
@@ -34,19 +37,44 @@ const WORKSPACE_PACKAGES = [
   'pi-coding-agent',
 ]
 
-const missing = []
+/**
+ * Returns the most recent mtime (ms) of any .ts file under dir, recursively.
+ * Returns 0 if no .ts files found.
+ */
+function newestSrcMtime(dir) {
+  if (!existsSync(dir)) return 0
+  let newest = 0
+  for (const entry of readdirSync(dir, { withFileTypes: true })) {
+    if (entry.name === 'node_modules') continue
+    const full = join(dir, entry.name)
+    if (entry.isDirectory()) {
+      newest = Math.max(newest, newestSrcMtime(full))
+    } else if (entry.isFile() && entry.name.endsWith('.ts')) {
+      newest = Math.max(newest, statSync(full).mtimeMs)
+    }
+  }
+  return newest
+}
+
+const stale = []
 for (const pkg of WORKSPACE_PACKAGES) {
   const distIndex = join(packagesDir, pkg, 'dist', 'index.js')
   if (!existsSync(distIndex)) {
-    missing.push(pkg)
+    stale.push(pkg)
+    continue
+  }
+  const distMtime = statSync(distIndex).mtimeMs
+  const srcMtime = newestSrcMtime(join(packagesDir, pkg, 'src'))
+  if (srcMtime > distMtime) {
+    stale.push(pkg)
   }
 }
 
-if (missing.length === 0) process.exit(0)
+if (stale.length === 0) process.exit(0)
 
-process.stderr.write(`  Building ${missing.length} workspace package(s) missing dist/: ${missing.join(', ')}\n`)
+process.stderr.write(`  Building ${stale.length} workspace package(s) with stale or missing dist/: ${stale.join(', ')}\n`)
 
-for (const pkg of missing) {
+for (const pkg of stale) {
   const pkgDir = join(packagesDir, pkg)
   try {
     execSync('npm run build', { cwd: pkgDir, stdio: 'pipe' })
diff --git a/src/resources/extensions/gsd/markdown-renderer.ts b/src/resources/extensions/gsd/markdown-renderer.ts
index 669eb1e0e..5e9eda89b 100644
--- a/src/resources/extensions/gsd/markdown-renderer.ts
+++ b/src/resources/extensions/gsd/markdown-renderer.ts
@@ -9,6 +9,7 @@
 // parseRoadmap(), parsePlan(), parseSummary() in files.ts.
 
 import { readFileSync, existsSync, mkdirSync } from "node:fs";
+import { isClosedStatus } from "./status-guards.js";
 import { join, relative } from "node:path";
 import { createRequire } from "node:module";
 import {
@@ -337,7 +338,7 @@ function renderSlicePlanMarkdown(slice: SliceRow, tasks: TaskRow[], gates: GateR
   lines.push("## Tasks");
   lines.push("");
   for (const task of tasks) {
-    const done = task.status === "done" || task.status === "complete" ? "x" : " ";
+    const done = isClosedStatus(task.status) ? "x" : " ";
     const estimate = task.estimate.trim() ? ` \`est:${task.estimate.trim()}\`` : "";
     lines.push(`- [${done}] **${task.id}: ${task.title || task.id}**${estimate}`);
     if (task.description.trim()) {
@@ -573,7 +574,7 @@ export async function renderPlanCheckboxes(
   // Apply checkbox patches for each task
   let updated = content;
   for (const task of tasks) {
-    const isDone = task.status === "done" || task.status === "complete";
+    const isDone = isClosedStatus(task.status);
     const tid = task.id;
 
     if (isDone) {
@@ -857,7 +858,7 @@ export function detectStaleRenders(basePath: string): StaleEntry[] {
           const parsed = parsePlan(content);
 
           for (const task of tasks) {
-            const isDoneInDb = task.status === "done" || task.status === "complete";
+            const isDoneInDb = isClosedStatus(task.status);
             const planTask = parsed.tasks.find((t: { id: string }) => t.id === task.id);
             if (!planTask) continue;
 
@@ -880,7 +881,7 @@ export function detectStaleRenders(basePath: string): StaleEntry[] {
 
       // Check missing task summary files
       for (const task of tasks) {
-        if ((task.status === "done" || task.status === "complete") && task.full_summary_md) {
+        if (isClosedStatus(task.status) && task.full_summary_md) {
           const slicePath = resolveSlicePath(basePath, milestone.id, slice.id);
           if (slicePath) {
             const tasksDir = join(slicePath, "tasks");
diff --git a/src/resources/extensions/gsd/state.ts b/src/resources/extensions/gsd/state.ts
index 0f91aca90..2d84782c5 100644
--- a/src/resources/extensions/gsd/state.ts
+++ b/src/resources/extensions/gsd/state.ts
@@ -36,6 +36,7 @@ import {
 
 import { findMilestoneIds } from './milestone-ids.js';
 import { loadQueueOrder, sortByQueueOrder } from './queue-order.js';
+import { isClosedStatus } from './status-guards.js';
 import { nativeBatchParseGsdFiles, type BatchParsedFile } from './native-parser-bridge.js';
 
 import { join, resolve } from 'path';
@@ -272,13 +273,6 @@ function extractContextTitle(content: string | null, fallback: string): string {
 
 // ─── DB-backed State Derivation ────────────────────────────────────────────
 
-/**
- * Helper: check if a DB status counts as "done" (handles K002 ambiguity).
- */
-function isStatusDone(status: string): boolean {
-  return status === 'complete' || status === 'done';
-}
-
 /**
  * Derive GSD state from the milestones/slices/tasks DB tables.
  * Flag files (PARKED, VALIDATION, CONTINUE, REPLAN, REPLAN-TRIGGER, CONTEXT-DRAFT)
@@ -368,7 +362,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
       continue;
     }
 
-    if (isStatusDone(m.status)) {
+    if (isClosedStatus(m.status)) {
       completeMilestoneIds.add(m.id);
       continue;
     }
@@ -382,7 +376,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
 
     // Check roadmap: all slices done means milestone is complete
     const slices = getMilestoneSlices(m.id);
-    if (slices.length > 0 && slices.every(s => isStatusDone(s.status))) {
+    if (slices.length > 0 && slices.every(s => isClosedStatus(s.status))) {
       // All slices done but no summary — still counts as complete for dep resolution
       // if a summary file exists
       // Note: without summary file, the milestone is in validating/completing state, not complete
@@ -404,7 +398,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
 
     // Ghost milestone check: no slices in DB AND no substantive files on disk
     const slices = getMilestoneSlices(m.id);
-    if (slices.length === 0 && !isStatusDone(m.status)) {
+    if (slices.length === 0 && !isClosedStatus(m.status)) {
       // Check disk for ghost detection
       if (isGhostMilestone(basePath, m.id)) continue;
     }
@@ -427,7 +421,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
     }
 
     // Not complete — determine if it should be active
-    const allSlicesDone = slices.length > 0 && slices.every(s => isStatusDone(s.status));
+    const allSlicesDone = slices.length > 0 && slices.every(s => isClosedStatus(s.status));
 
     // Get title — prefer DB, fall back to context file extraction
     let title = stripMilestonePrefix(m.title) || m.id;
@@ -582,7 +576,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
   // Guard: [].every() === true (vacuous truth). Without the length check,
   // an empty slice array causes a premature phase transition to
   // validating-milestone. See: https://github.com/gsd-build/gsd-2/issues/2667
-  const allSlicesDone = activeMilestoneSlices.length > 0 && activeMilestoneSlices.every(s => isStatusDone(s.status));
+  const allSlicesDone = activeMilestoneSlices.length > 0 && activeMilestoneSlices.every(s => isClosedStatus(s.status));
   if (allSlicesDone) {
     const validationFile = resolveMilestoneFile(basePath, activeMilestone.id, "VALIDATION");
     const validationContent = validationFile ? await loadFile(validationFile) : null;
@@ -615,19 +609,19 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
 
   // ── Find active slice (first incomplete with deps satisfied) ─────────
   const sliceProgress = {
-    done: activeMilestoneSlices.filter(s => isStatusDone(s.status)).length,
+    done: activeMilestoneSlices.filter(s => isClosedStatus(s.status)).length,
     total: activeMilestoneSlices.length,
   };
 
   const doneSliceIds = new Set(
-    activeMilestoneSlices.filter(s => isStatusDone(s.status)).map(s => s.id)
+    activeMilestoneSlices.filter(s => isClosedStatus(s.status)).map(s => s.id)
   );
 
   let activeSlice: ActiveRef | null = null;
   let activeSliceRow: SliceRow | null = null;
 
   for (const s of activeMilestoneSlices) {
-    if (isStatusDone(s.status)) continue;
+    if (isClosedStatus(s.status)) continue;
     if (s.depends.every(dep => doneSliceIds.has(dep))) {
       activeSlice = { id: s.id, title: s.title };
       activeSliceRow = s;
@@ -670,7 +664,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
   // causing the dispatcher to re-dispatch the same completed task forever.
   let reconciled = false;
   for (const t of tasks) {
-    if (isStatusDone(t.status)) continue;
+    if (isClosedStatus(t.status)) continue;
     const summaryPath = resolveTaskFile(basePath, activeMilestone.id, activeSlice.id, t.id, "SUMMARY");
     if (summaryPath && existsSync(summaryPath)) {
       try {
@@ -693,11 +687,11 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
   }
 
   const taskProgress = {
-    done: tasks.filter(t => isStatusDone(t.status)).length,
+    done: tasks.filter(t => isClosedStatus(t.status)).length,
     total: tasks.length,
   };
 
-  const activeTaskRow = tasks.find(t => !isStatusDone(t.status));
+  const activeTaskRow = tasks.find(t => !isClosedStatus(t.status));
 
   if (!activeTaskRow && tasks.length > 0) {
     // All tasks done but slice not marked complete → summarizing
@@ -758,7 +752,7 @@ export async function deriveStateFromDb(basePath: string): Promise<GSDState> {
   }
 
   // ── Blocker detection: check completed tasks for blocker_discovered ──
-  const completedTasks = tasks.filter(t => isStatusDone(t.status));
+  const completedTasks = tasks.filter(t => isClosedStatus(t.status));
   let blockerTaskId: string | null = null;
   for (const ct of completedTasks) {
     if (ct.blocker_discovered) {
diff --git a/src/resources/extensions/gsd/status-guards.ts b/src/resources/extensions/gsd/status-guards.ts
new file mode 100644
index 000000000..650aefc6c
--- /dev/null
+++ b/src/resources/extensions/gsd/status-guards.ts
@@ -0,0 +1,13 @@
+/**
+ * Status predicates for GSD state-machine guards.
+ *
+ * The DB stores status as free-form strings. Two values indicate
+ * "closed": "complete" (canonical) and "done" (legacy / alias).
+ * Every inline `status === "complete" || status === "done"` should
+ * use isClosedStatus() instead.
+ */
+
+/** Returns true when a milestone, slice, or task status indicates closure. */
+export function isClosedStatus(status: string): boolean {
+  return status === "complete" || status === "done";
+}
diff --git a/src/resources/extensions/gsd/tests/status-guards.test.ts b/src/resources/extensions/gsd/tests/status-guards.test.ts
new file mode 100644
index 000000000..44ab72bfc
--- /dev/null
+++ b/src/resources/extensions/gsd/tests/status-guards.test.ts
@@ -0,0 +1,30 @@
+// GSD — status-guards unit tests
+
+import test from 'node:test';
+import assert from 'node:assert/strict';
+
+import { isClosedStatus } from '../status-guards.ts';
+
+test('isClosedStatus: "complete" returns true', () => {
+  assert.equal(isClosedStatus('complete'), true);
+});
+
+test('isClosedStatus: "done" returns true', () => {
+  assert.equal(isClosedStatus('done'), true);
+});
+
+test('isClosedStatus: "pending" returns false', () => {
+  assert.equal(isClosedStatus('pending'), false);
+});
+
+test('isClosedStatus: "in_progress" returns false', () => {
+  assert.equal(isClosedStatus('in_progress'), false);
+});
+
+test('isClosedStatus: "active" returns false', () => {
+  assert.equal(isClosedStatus('active'), false);
+});
+
+test('isClosedStatus: "" (empty string) returns false', () => {
+  assert.equal(isClosedStatus(''), false);
+});
diff --git a/src/resources/extensions/gsd/tests/validation.test.ts b/src/resources/extensions/gsd/tests/validation.test.ts
new file mode 100644
index 000000000..18a72b4f2
--- /dev/null
+++ b/src/resources/extensions/gsd/tests/validation.test.ts
@@ -0,0 +1,72 @@
+// GSD — validation unit tests
+
+import test from 'node:test';
+import assert from 'node:assert/strict';
+
+import { isNonEmptyString, validateStringArray } from '../validation.ts';
+
+// ─── isNonEmptyString ────────────────────────────────────────────────────────
+
+test('isNonEmptyString: "hello" returns true', () => {
+  assert.equal(isNonEmptyString('hello'), true);
+});
+
+test('isNonEmptyString: " " (whitespace only) returns false', () => {
+  assert.equal(isNonEmptyString(' '), false);
+});
+
+test('isNonEmptyString: "" (empty string) returns false', () => {
+  assert.equal(isNonEmptyString(''), false);
+});
+
+test('isNonEmptyString: null returns false', () => {
+  assert.equal(isNonEmptyString(null), false);
+});
+
+test('isNonEmptyString: undefined returns false', () => {
+  assert.equal(isNonEmptyString(undefined), false);
+});
+
+test('isNonEmptyString: 42 (number) returns false', () => {
+  assert.equal(isNonEmptyString(42), false);
+});
+
+// ─── validateStringArray ─────────────────────────────────────────────────────
+
+test('validateStringArray: ["a", "b"] returns ["a", "b"]', () => {
+  assert.deepEqual(validateStringArray(['a', 'b'], 'items'), ['a', 'b']);
+});
+
+test('validateStringArray: [] (empty array) returns []', () => {
+  assert.deepEqual(validateStringArray([], 'items'), []);
+});
+
+test('validateStringArray: "not an array" throws with "must be an array"', () => {
+  assert.throws(
+    () => validateStringArray('not an array', 'items'),
+    (err: Error) => {
+      assert.ok(err.message.includes('must be an array'));
+      return true;
+    },
+  );
+});
+
+test('validateStringArray: ["a", 42] throws with "must contain only non-empty strings"', () => {
+  assert.throws(
+    () => validateStringArray(['a', 42], 'items'),
+    (err: Error) => {
+      assert.ok(err.message.includes('must contain only non-empty strings'));
+      return true;
+    },
+  );
+});
+
+test('validateStringArray: ["a", ""] throws with "must contain only non-empty strings"', () => {
+  assert.throws(
+    () => validateStringArray(['a', ''], 'items'),
+    (err: Error) => {
+      assert.ok(err.message.includes('must contain only non-empty strings'));
+      return true;
+    },
+  );
+});
diff --git a/src/resources/extensions/gsd/tools/complete-milestone.ts b/src/resources/extensions/gsd/tools/complete-milestone.ts
index b9077bb35..939e07883 100644
--- a/src/resources/extensions/gsd/tools/complete-milestone.ts
+++ b/src/resources/extensions/gsd/tools/complete-milestone.ts
@@ -17,6 +17,7 @@ import {
   updateMilestoneStatus,
 } from "../gsd-db.js";
 import { resolveMilestonePath, clearPathCache } from "../paths.js";
+import { isClosedStatus } from "../status-guards.js";
 import { saveFile, clearParseCache } from "../files.js";
 import { invalidateStateCache } from "../state.js";
 import { renderAllProjections } from "../workflow-projections.js";
@@ -134,7 +135,7 @@ export async function handleCompleteMilestone(
       guardError = `milestone not found: ${params.milestoneId}`;
       return;
     }
-    if (milestone.status === "complete" || milestone.status === "done") {
+    if (isClosedStatus(milestone.status)) {
       guardError = `milestone ${params.milestoneId} is already complete`;
       return;
     }
@@ -146,7 +147,7 @@ export async function handleCompleteMilestone(
       return;
     }
 
-    const incompleteSlices = slices.filter(s => s.status !== "complete" && s.status !== "done");
+    const incompleteSlices = slices.filter(s => !isClosedStatus(s.status));
     if (incompleteSlices.length > 0) {
       const incompleteIds = incompleteSlices.map(s => `${s.id} (status: ${s.status})`).join(", ");
       guardError = `incomplete slices: ${incompleteIds}`;
@@ -156,7 +157,7 @@ export async function handleCompleteMilestone(
     // Deep check: verify all tasks in all slices are complete
     for (const slice of slices) {
       const tasks = getSliceTasks(params.milestoneId, slice.id);
-      const incompleteTasks = tasks.filter(t => t.status !== "complete" && t.status !== "done");
+      const incompleteTasks = tasks.filter(t => !isClosedStatus(t.status));
       if (incompleteTasks.length > 0) {
         const ids = incompleteTasks.map(t => `${t.id} (status: ${t.status})`).join(", ");
         guardError = `slice ${slice.id} has incomplete tasks: ${ids}`;
diff --git a/src/resources/extensions/gsd/tools/complete-slice.ts b/src/resources/extensions/gsd/tools/complete-slice.ts
index cf1adb2d8..759513319 100644
--- a/src/resources/extensions/gsd/tools/complete-slice.ts
+++ b/src/resources/extensions/gsd/tools/complete-slice.ts
@@ -11,6 +11,7 @@ import { join } from "node:path";
 import { mkdirSync } from "node:fs";
 
 import type { CompleteSliceParams } from "../types.js";
+import { isClosedStatus } from "../status-guards.js";
 import {
   transaction,
   insertMilestone,
@@ -225,13 +226,13 @@ export async function handleCompleteSlice(
     // Milestone/slice not existing is OK — insertMilestone/insertSlice below will auto-create.
     // Only block if they exist and are closed.
     const milestone = getMilestone(params.milestoneId);
-    if (milestone && (milestone.status === "complete" || milestone.status === "done")) {
+    if (milestone && isClosedStatus(milestone.status)) {
       guardError = `cannot complete slice in a closed milestone: ${params.milestoneId} (status: ${milestone.status})`;
       return;
     }
 
     const slice = getSlice(params.milestoneId, params.sliceId);
-    if (slice && (slice.status === "complete" || slice.status === "done")) {
+    if (slice && isClosedStatus(slice.status)) {
       guardError = `slice ${params.sliceId} is already complete — use gsd_slice_reopen first if you need to redo it`;
       return;
     }
@@ -243,7 +244,7 @@ export async function handleCompleteSlice(
       return;
     }
 
-    const incompleteTasks = tasks.filter(t => t.status !== "complete" && t.status !== "done");
+    const incompleteTasks = tasks.filter(t => !isClosedStatus(t.status));
     if (incompleteTasks.length > 0) {
       const incompleteIds = incompleteTasks.map(t => `${t.id} (status: ${t.status})`).join(", ");
       guardError = `incomplete tasks: ${incompleteIds}`;
diff --git a/src/resources/extensions/gsd/tools/complete-task.ts b/src/resources/extensions/gsd/tools/complete-task.ts
index fc0e3a005..d7805b20d 100644
--- a/src/resources/extensions/gsd/tools/complete-task.ts
+++ b/src/resources/extensions/gsd/tools/complete-task.ts
@@ -11,6 +11,7 @@ import { join } from "node:path";
 import { mkdirSync, existsSync } from "node:fs";
 
 import type { CompleteTaskParams } from "../types.js";
+import { isClosedStatus } from "../status-guards.js";
 import {
   transaction,
   insertMilestone,
@@ -159,19 +160,19 @@ export async function handleCompleteTask(
     // Milestone/slice not existing is OK — insertMilestone/insertSlice below will auto-create.
     // Only block if they exist and are closed.
     const milestone = getMilestone(params.milestoneId);
-    if (milestone && (milestone.status === "complete" || milestone.status === "done")) {
+    if (milestone && isClosedStatus(milestone.status)) {
       guardError = `cannot complete task in a closed milestone: ${params.milestoneId} (status: ${milestone.status})`;
       return;
     }
 
     const slice = getSlice(params.milestoneId, params.sliceId);
-    if (slice && (slice.status === "complete" || slice.status === "done")) {
+    if (slice && isClosedStatus(slice.status)) {
       guardError = `cannot complete task in a closed slice: ${params.sliceId} (status: ${slice.status})`;
       return;
     }
 
     const existingTask = getTask(params.milestoneId, params.sliceId, params.taskId);
-    if (existingTask && (existingTask.status === "complete" || existingTask.status === "done")) {
+    if (existingTask && isClosedStatus(existingTask.status)) {
       guardError = `task ${params.taskId} is already complete — use gsd_task_reopen first if you need to redo it`;
       return;
     }
diff --git a/src/resources/extensions/gsd/tools/plan-milestone.ts b/src/resources/extensions/gsd/tools/plan-milestone.ts
index 7cea0212d..6a09d4163 100644
--- a/src/resources/extensions/gsd/tools/plan-milestone.ts
+++ b/src/resources/extensions/gsd/tools/plan-milestone.ts
@@ -1,4 +1,6 @@
 import { clearParseCache } from "../files.js";
+import { isClosedStatus } from "../status-guards.js";
+import { isNonEmptyString, validateStringArray } from "../validation.js";
 import {
   transaction,
   getMilestone,
@@ -54,20 +56,6 @@ export interface PlanMilestoneResult {
   roadmapPath: string;
 }
 
-function isNonEmptyString(value: unknown): value is string {
-  return typeof value === "string" && value.trim().length > 0;
-}
-
-function validateStringArray(value: unknown, field: string): string[] {
-  if (!Array.isArray(value)) {
-    throw new Error(`${field} must be an array`);
-  }
-  if (value.some((item) => !isNonEmptyString(item))) {
-    throw new Error(`${field} must contain only non-empty strings`);
-  }
-  return value;
-}
-
 function validateRiskEntries(value: unknown): Array<{ risk: string; whyItMatters: string }> {
   if (!Array.isArray(value)) {
     throw new Error("keyRisks must be an array");
@@ -196,7 +184,7 @@ export async function handlePlanMilestone(
   try {
     transaction(() => {
       const existingMilestone = getMilestone(params.milestoneId);
-      if (existingMilestone && (existingMilestone.status === "complete" || existingMilestone.status === "done")) {
+      if (existingMilestone && isClosedStatus(existingMilestone.status)) {
         guardError = `cannot re-plan milestone ${params.milestoneId}: it is already complete`;
         return;
       }
@@ -209,7 +197,7 @@ export async function handlePlanMilestone(
             guardError = `depends_on references unknown milestone: ${depId}`;
             return;
           }
-          if (dep.status !== "complete" && dep.status !== "done") {
+          if (!isClosedStatus(dep.status)) {
             guardError = `depends_on milestone ${depId} is not yet complete (status: ${dep.status})`;
             return;
           }
diff --git a/src/resources/extensions/gsd/tools/plan-slice.ts b/src/resources/extensions/gsd/tools/plan-slice.ts
index 0f8e06a38..fa345a975 100644
--- a/src/resources/extensions/gsd/tools/plan-slice.ts
+++ b/src/resources/extensions/gsd/tools/plan-slice.ts
@@ -1,4 +1,6 @@
 import { clearParseCache } from "../files.js";
+import { isClosedStatus } from "../status-guards.js";
+import { isNonEmptyString, validateStringArray } from "../validation.js";
 import {
   transaction,
   getMilestone,
@@ -50,20 +52,6 @@ export interface PlanSliceResult {
   taskPlanPaths: string[];
 }
 
-function isNonEmptyString(value: unknown): value is string {
-  return typeof value === "string" && value.trim().length > 0;
-}
-
-function validateStringArray(value: unknown, field: string): string[] {
-  if (!Array.isArray(value)) {
-    throw new Error(`${field} must be an array`);
-  }
-  if (value.some((item) => !isNonEmptyString(item))) {
-    throw new Error(`${field} must contain only non-empty strings`);
-  }
-  return value;
-}
-
 function validateTasks(value: unknown): PlanSliceTaskInput[] {
   if (!Array.isArray(value) || value.length === 0) {
     throw new Error("tasks must be a non-empty array");
@@ -157,7 +145,7 @@ export async function handlePlanSlice(
         guardError = `milestone not found: ${params.milestoneId}`;
         return;
       }
-      if (parentMilestone.status === "complete" || parentMilestone.status === "done") {
+      if (isClosedStatus(parentMilestone.status)) {
         guardError = `cannot plan slice in a closed milestone: ${params.milestoneId} (status: ${parentMilestone.status})`;
         return;
       }
@@ -167,7 +155,7 @@ export async function handlePlanSlice(
         guardError = `missing parent slice: ${params.milestoneId}/${params.sliceId}`;
         return;
       }
-      if (parentSlice.status === "complete" || parentSlice.status === "done") {
+      if (isClosedStatus(parentSlice.status)) {
         guardError = `cannot re-plan slice ${params.sliceId}: it is already complete — use gsd_slice_reopen first`;
         return;
       }
diff --git a/src/resources/extensions/gsd/tools/plan-task.ts b/src/resources/extensions/gsd/tools/plan-task.ts
index 3fb9495ca..57b91ae0a 100644
--- a/src/resources/extensions/gsd/tools/plan-task.ts
+++ b/src/resources/extensions/gsd/tools/plan-task.ts
@@ -1,4 +1,6 @@
 import { clearParseCache } from "../files.js";
+import { isClosedStatus } from "../status-guards.js";
+import { isNonEmptyString, validateStringArray } from "../validation.js";
 import { transaction, getSlice, getTask, insertTask, upsertTaskPlanning } from "../gsd-db.js";
 import { invalidateStateCache } from "../state.js";
 import { renderTaskPlanFromDb } from "../markdown-renderer.js";
@@ -32,20 +34,6 @@ export interface PlanTaskResult {
   taskPlanPath: string;
 }
 
-function isNonEmptyString(value: unknown): value is string {
-  return typeof value === "string" && value.trim().length > 0;
-}
-
-function validateStringArray(value: unknown, field: string): string[] {
-  if (!Array.isArray(value)) {
-    throw new Error(`${field} must be an array`);
-  }
-  if (value.some((item) => !isNonEmptyString(item))) {
-    throw new Error(`${field} must contain only non-empty strings`);
-  }
-  return value;
-}
-
 function validateParams(params: PlanTaskParams): PlanTaskParams {
   if (!isNonEmptyString(params?.milestoneId)) throw new Error("milestoneId is required");
   if (!isNonEmptyString(params?.sliceId)) throw new Error("sliceId is required");
@@ -89,13 +77,13 @@ export async function handlePlanTask(
         guardError = `missing parent slice: ${params.milestoneId}/${params.sliceId}`;
         return;
       }
-      if (parentSlice.status === "complete" || parentSlice.status === "done") {
+      if (isClosedStatus(parentSlice.status)) {
         guardError = `cannot plan task in a closed slice: ${params.sliceId} (status: ${parentSlice.status})`;
         return;
       }
 
       const existingTask = getTask(params.milestoneId, params.sliceId, params.taskId);
-      if (existingTask && (existingTask.status === "complete" || existingTask.status === "done")) {
+      if (existingTask && isClosedStatus(existingTask.status)) {
         guardError = `cannot re-plan task ${params.taskId}: it is already complete — use gsd_task_reopen first`;
         return;
       }
diff --git a/src/resources/extensions/gsd/tools/reassess-roadmap.ts b/src/resources/extensions/gsd/tools/reassess-roadmap.ts
index 3aa832120..040aacf56 100644
--- a/src/resources/extensions/gsd/tools/reassess-roadmap.ts
+++ b/src/resources/extensions/gsd/tools/reassess-roadmap.ts
@@ -1,4 +1,7 @@
+import { join } from "node:path";
 import { clearParseCache } from "../files.js";
+import { isClosedStatus } from "../status-guards.js";
+import { isNonEmptyString } from "../validation.js";
 import {
   transaction,
   getMilestone,
@@ -14,7 +17,6 @@ import { renderRoadmapFromDb, renderAssessmentFromDb } from "../markdown-rendere
 import { renderAllProjections } from "../workflow-projections.js";
 import { writeManifest } from "../workflow-manifest.js";
 import { appendEvent } from "../workflow-events.js";
-import { join } from "node:path";
 
 export interface SliceChangeInput {
   sliceId: string;
@@ -47,9 +49,6 @@ export interface ReassessRoadmapResult {
   roadmapPath: string;
 }
 
-function isNonEmptyString(value: unknown): value is string {
-  return typeof value === "string" && value.trim().length > 0;
-}
 
 function validateParams(params: ReassessRoadmapParams): ReassessRoadmapParams {
   if (!isNonEmptyString(params?.milestoneId)) throw new Error("milestoneId is required");
@@ -125,7 +124,7 @@ export async function handleReassessRoadmap(
         guardError = `milestone not found: ${params.milestoneId}`;
         return;
       }
-      if (milestone.status === "complete" || milestone.status === "done") {
+      if (isClosedStatus(milestone.status)) {
         guardError = `cannot reassess a closed milestone: ${params.milestoneId} (status: ${milestone.status})`;
         return;
       }
@@ -136,7 +135,7 @@ export async function handleReassessRoadmap(
         guardError = `completedSliceId not found: ${params.milestoneId}/${params.completedSliceId}`;
         return;
       }
-      if (completedSlice.status !== "complete" && completedSlice.status !== "done") {
+      if (!isClosedStatus(completedSlice.status)) {
         guardError = `completedSliceId ${params.completedSliceId} is not complete (status: ${completedSlice.status}) — reassess can only be called after a slice finishes`;
         return;
       }
@@ -145,7 +144,7 @@ export async function handleReassessRoadmap(
       const existingSlices = getMilestoneSlices(params.milestoneId);
       const completedSliceIds = new Set<string>();
       for (const slice of existingSlices) {
-        if (slice.status === "complete" || slice.status === "done") {
+        if (isClosedStatus(slice.status)) {
           completedSliceIds.add(slice.id);
         }
       }
diff --git a/src/resources/extensions/gsd/tools/reopen-slice.ts b/src/resources/extensions/gsd/tools/reopen-slice.ts
index fbe1b1d92..9064167fd 100644
--- a/src/resources/extensions/gsd/tools/reopen-slice.ts
+++ b/src/resources/extensions/gsd/tools/reopen-slice.ts
@@ -20,6 +20,7 @@ import {
   transaction,
 } from "../gsd-db.js";
 import { invalidateStateCache } from "../state.js";
+import { isClosedStatus } from "../status-guards.js";
 import { renderAllProjections } from "../workflow-projections.js";
 import { writeManifest } from "../workflow-manifest.js";
 import { appendEvent } from "../workflow-events.js";
@@ -62,8 +63,8 @@ export async function handleReopenSlice(
       guardError = `milestone not found: ${params.milestoneId}`;
       return;
     }
-    if (milestone.status === "complete" || milestone.status === "done") {
-      guardError = `cannot reopen slice inside a closed milestone: ${params.milestoneId} (status: ${milestone.status})`;
+    if (isClosedStatus(milestone.status)) {
+      guardError = `cannot reopen slice in a closed milestone: ${params.milestoneId} (status: ${milestone.status})`;
       return;
     }
 
@@ -72,7 +73,7 @@ export async function handleReopenSlice(
       guardError = `slice not found: ${params.milestoneId}/${params.sliceId}`;
       return;
     }
-    if (slice.status !== "complete" && slice.status !== "done") {
+    if (!isClosedStatus(slice.status)) {
       guardError = `slice ${params.sliceId} is not complete (status: ${slice.status}) — nothing to reopen`;
       return;
     }
diff --git a/src/resources/extensions/gsd/tools/reopen-task.ts b/src/resources/extensions/gsd/tools/reopen-task.ts
index afa5e7a8c..5f5af1ddc 100644
--- a/src/resources/extensions/gsd/tools/reopen-task.ts
+++ b/src/resources/extensions/gsd/tools/reopen-task.ts
@@ -18,6 +18,7 @@ import {
   transaction,
 } from "../gsd-db.js";
 import { invalidateStateCache } from "../state.js";
+import { isClosedStatus } from "../status-guards.js";
 import { renderAllProjections } from "../workflow-projections.js";
 import { writeManifest } from "../workflow-manifest.js";
 import { appendEvent } from "../workflow-events.js";
@@ -63,7 +64,7 @@ export async function handleReopenTask(
       guardError = `milestone not found: ${params.milestoneId}`;
       return;
     }
-    if (milestone.status === "complete" || milestone.status === "done") {
+    if (isClosedStatus(milestone.status)) {
       guardError = `cannot reopen task in a closed milestone: ${params.milestoneId} (status: ${milestone.status})`;
       return;
     }
@@ -73,8 +74,8 @@ export async function handleReopenTask(
       guardError = `slice not found: ${params.milestoneId}/${params.sliceId}`;
       return;
     }
-    if (slice.status === "complete" || slice.status === "done") {
-      guardError = `cannot reopen task inside a closed slice: ${params.sliceId} (status: ${slice.status}) — use gsd_slice_reopen first`;
+    if (isClosedStatus(slice.status)) {
+      guardError = `cannot reopen task in a closed slice: ${params.sliceId} (status: ${slice.status}) — use gsd_slice_reopen first`;
       return;
     }
 
@@ -83,7 +84,7 @@ export async function handleReopenTask(
       guardError = `task not found: ${params.milestoneId}/${params.sliceId}/${params.taskId}`;
       return;
     }
-    if (task.status !== "complete" && task.status !== "done") {
+    if (!isClosedStatus(task.status)) {
       guardError = `task ${params.taskId} is not complete (status: ${task.status}) — nothing to reopen`;
       return;
     }
diff --git a/src/resources/extensions/gsd/tools/replan-slice.ts b/src/resources/extensions/gsd/tools/replan-slice.ts
index c6b060597..b55dae238 100644
--- a/src/resources/extensions/gsd/tools/replan-slice.ts
+++ b/src/resources/extensions/gsd/tools/replan-slice.ts
@@ -10,6 +10,8 @@ import {
   deleteTask,
 } from "../gsd-db.js";
 import { invalidateStateCache } from "../state.js";
+import { isClosedStatus } from "../status-guards.js";
+import { isNonEmptyString } from "../validation.js";
 import { renderPlanFromDb, renderReplanFromDb } from "../markdown-renderer.js";
 import { renderAllProjections } from "../workflow-projections.js";
 import { writeManifest } from "../workflow-manifest.js";
@@ -48,10 +50,6 @@ export interface ReplanSliceResult {
   planPath: string;
 }
 
-function isNonEmptyString(value: unknown): value is string {
-  return typeof value === "string" && value.trim().length > 0;
-}
-
 function validateParams(params: ReplanSliceParams): ReplanSliceParams {
   if (!isNonEmptyString(params?.milestoneId)) throw new Error("milestoneId is required");
   if (!isNonEmptyString(params?.sliceId)) throw new Error("sliceId is required");
@@ -104,7 +102,7 @@ export async function handleReplanSlice(
         guardError = `missing parent slice: ${params.milestoneId}/${params.sliceId}`;
         return;
       }
-      if (parentSlice.status === "complete" || parentSlice.status === "done") {
+      if (isClosedStatus(parentSlice.status)) {
         guardError = `cannot replan a closed slice: ${params.sliceId} (status: ${parentSlice.status})`;
         return;
       }
@@ -115,7 +113,7 @@ export async function handleReplanSlice(
         guardError = `blockerTaskId not found: ${params.milestoneId}/${params.sliceId}/${params.blockerTaskId}`;
         return;
       }
-      if (blockerTask.status !== "complete" && blockerTask.status !== "done") {
+      if (!isClosedStatus(blockerTask.status)) {
         guardError = `blockerTaskId ${params.blockerTaskId} is not complete (status: ${blockerTask.status}) — the blocker task must be finished before a replan is triggered`;
         return;
       }
@@ -124,7 +122,7 @@ export async function handleReplanSlice(
       const existingTasks = getSliceTasks(params.milestoneId, params.sliceId);
       const completedTaskIds = new Set<string>();
       for (const task of existingTasks) {
-        if (task.status === "complete" || task.status === "done") {
+        if (isClosedStatus(task.status)) {
           completedTaskIds.add(task.id);
         }
       }
diff --git a/src/resources/extensions/gsd/validation.ts b/src/resources/extensions/gsd/validation.ts
new file mode 100644
index 000000000..a64b3be6d
--- /dev/null
+++ b/src/resources/extensions/gsd/validation.ts
@@ -0,0 +1,23 @@
+/**
+ * Shared input-validation primitives for GSD tool handlers.
+ */
+
+/** Type guard: value is a string with at least one non-whitespace character. */
+export function isNonEmptyString(value: unknown): value is string {
+  return typeof value === "string" && value.trim().length > 0;
+}
+
+/**
+ * Validate that `value` is an array of non-empty strings.
+ * Throws with a message referencing `field` on failure.
+ * Returns the validated array (narrowed to string[]).
+ */
+export function validateStringArray(value: unknown, field: string): string[] {
+  if (!Array.isArray(value)) {
+    throw new Error(`${field} must be an array`);
+  }
+  if (value.some((item) => !isNonEmptyString(item))) {
+    throw new Error(`${field} must contain only non-empty strings`);
+  }
+  return value;
+}
diff --git a/src/resources/extensions/shared/rtk.ts b/src/resources/extensions/shared/rtk.ts
index 4ff6a320f..bf0d4880e 100644
--- a/src/resources/extensions/shared/rtk.ts
+++ b/src/resources/extensions/shared/rtk.ts
@@ -96,14 +96,23 @@ export function resolveRtkBinaryPath(options: ResolveRtkBinaryPathOptions = {}):
   return resolveSystemRtkPath(options.pathValue ?? getPathValue(env), platform);
 }
 
-export function rewriteCommandWithRtk(command: string, env: NodeJS.ProcessEnv = process.env): string {
+interface RewriteCommandOptions {
+  binaryPath?: string;
+  env?: NodeJS.ProcessEnv;
+  spawnSyncImpl?: typeof spawnSync;
+}
+
+export function rewriteCommandWithRtk(command: string, options: RewriteCommandOptions = {}): string {
+  const env = options.env ?? process.env;
+
   if (!command.trim()) return command;
   if (!isRtkEnabled(env)) return command;
 
-  const binaryPath = resolveRtkBinaryPath({ env });
+  const binaryPath = options.binaryPath ?? resolveRtkBinaryPath({ env });
   if (!binaryPath) return command;
 
-  const result = spawnSync(binaryPath, ["rewrite", command], {
+  const run = options.spawnSyncImpl ?? spawnSync;
+  const result = run(binaryPath, ["rewrite", command], {
     encoding: "utf-8",
     env: buildRtkEnv(env),
     stdio: ["ignore", "pipe", "ignore"],
diff --git a/src/rtk.ts b/src/rtk.ts
index 82ad0af52..5c28ce756 100644
--- a/src/rtk.ts
+++ b/src/rtk.ts
@@ -229,7 +229,8 @@ export function resolveRtkBinaryPath(options: ResolveRtkBinaryPathOptions = {}):
   const env = options.env ?? process.env;
   const platform = options.platform ?? process.platform;
 
-  const explicitPath = options.binaryPath ?? env[GSD_RTK_PATH_ENV];
+  if (options.binaryPath) return options.binaryPath;
+  const explicitPath = env[GSD_RTK_PATH_ENV];
   if (explicitPath && existsSync(explicitPath)) {
     return explicitPath;
   }