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

* 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>
This commit is contained in:
Iouri Goussev 2026-03-26 20:14:43 -04:00 committed by GitHub
parent 34bbee21bc
commit de600c1db0
19 changed files with 250 additions and 113 deletions

View file

@ -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' })

View file

@ -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");

View file

@ -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) {

View file

@ -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";
}

View file

@ -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);
});

View file

@ -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;
},
);
});

View file

@ -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}`;

View file

@ -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}`;

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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);
}
}

View file

@ -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;
}

View file

@ -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;
}

View file

@ -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);
}
}

View file

@ -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;
}

View file

@ -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"],

View file

@ -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;
}