Merge pull request #2673 from gsd-build/refine/use-parseUnitId-everywhere

refine: replace manual unitId.split() with parseUnitId() across 12 files
This commit is contained in:
TÂCHES 2026-03-26 09:35:41 -06:00 committed by GitHub
commit 0db5edd7fe
12 changed files with 53 additions and 74 deletions

View file

@ -12,6 +12,7 @@ import {
formatValidationIssues,
} from "./observability-validator.js";
import type { ValidationIssue } from "./observability-validator.js";
import { parseUnitId } from "./unit-id.js";
export async function collectObservabilityWarnings(
ctx: ExtensionContext,
@ -22,10 +23,7 @@ export async function collectObservabilityWarnings(
// Hook units have custom artifacts — skip standard observability checks
if (unitType.startsWith("hook/")) return [];
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const tid = parts[2];
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
if (!mid || !sid) return [];

View file

@ -195,13 +195,8 @@ export function verifyExpectedArtifact(
// Reactive-execute: verify that each dispatched task's summary exists.
// The unitId encodes the batch: "{mid}/{sid}/reactive+T02,T03"
if (unitType === "reactive-execute") {
const parts = unitId.split("/");
const mid = parts[0];
const sidAndBatch = parts[1];
const batchPart = parts[2]; // "reactive+T02,T03"
if (!mid || !sidAndBatch || !batchPart) return false;
const sid = sidAndBatch;
const { milestone: mid, slice: sid, task: batchPart } = parseUnitId(unitId);
if (!mid || !sid || !batchPart) return false;
const plusIdx = batchPart.indexOf("+");
if (plusIdx === -1) {
// Legacy format "reactive" without batch IDs — fall back to "any summary"
@ -233,10 +228,7 @@ export function verifyExpectedArtifact(
// Gate-evaluate: verify that each dispatched gate has been resolved in the DB.
// The unitId encodes the batch: "{mid}/{sid}/gates+Q3,Q4"
if (unitType === "gate-evaluate") {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const batchPart = parts[2]; // "gates+Q3,Q4"
const { milestone: mid, slice: sid, task: batchPart } = parseUnitId(unitId);
if (!mid || !sid || !batchPart) return false;
const plusIdx = batchPart.indexOf("+");
@ -286,10 +278,7 @@ export function verifyExpectedArtifact(
// execute-task: DB status is authoritative. Fall back to heading-style plan
// detection when the DB is unavailable (unmigrated projects).
if (unitType === "execute-task") {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const tid = parts[2];
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
if (mid && sid && tid) {
const dbTask = getTask(mid, sid, tid);
if (dbTask) {
@ -319,9 +308,7 @@ export function verifyExpectedArtifact(
// but omitted T{tid}-PLAN.md files would be marked complete, causing execute-task
// to dispatch with a missing task plan (see issue #739).
if (unitType === "plan-slice") {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const { milestone: mid, slice: sid } = parseUnitId(unitId);
if (mid && sid) {
try {
// DB primary path — get task IDs to verify task plan files exist
@ -356,9 +343,7 @@ export function verifyExpectedArtifact(
// complete-slice: DB status is authoritative for whether the slice is done.
// Fall back to file-based check (roadmap [x]) when DB is unavailable.
if (unitType === "complete-slice") {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const { milestone: mid, slice: sid } = parseUnitId(unitId);
if (mid && sid) {
const dir = resolveSlicePath(base, mid, sid);
if (dir) {
@ -430,10 +415,7 @@ export function writeBlockerPlaceholder(
// Without this, the DB status stays "pending" and the dispatch loop
// re-derives the same task indefinitely (#2531).
if (unitType === "execute-task" && isDbAvailable()) {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const tid = parts[2];
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
if (mid && sid && tid) {
try { updateTaskStatus(mid, sid, tid, "complete", new Date().toISOString()); } catch { /* non-fatal */ }
}
@ -558,10 +540,7 @@ export function buildLoopRemediationSteps(
unitId: string,
base: string,
): string | null {
const parts = unitId.split("/");
const mid = parts[0];
const sid = parts[1];
const tid = parts[2];
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
switch (unitType) {
case "execute-task": {
if (!mid || !sid || !tid) break;

View file

@ -17,6 +17,7 @@ import type {
} from "@gsd/pi-coding-agent";
import { deriveState } from "./state.js";
import { parseUnitId } from "./unit-id.js";
import type { GSDState } from "./types.js";
import { getManifestStatus } from "./files.js";
export { inlinePriorMilestoneSummary } from "./files.js";
@ -1288,8 +1289,7 @@ function ensurePreconditions(
base: string,
state: GSDState,
): void {
const parts = unitId.split("/");
const mid = parts[0]!;
const { milestone: mid, slice: sid } = parseUnitId(unitId);
const mDir = resolveMilestonePath(base, mid);
if (!mDir) {
@ -1297,8 +1297,7 @@ function ensurePreconditions(
mkdirSync(join(newDir, "slices"), { recursive: true });
}
if (parts.length >= 2) {
const sid = parts[1]!;
if (sid !== undefined) {
const mDirResolved = resolveMilestonePath(base, mid);
if (mDirResolved) {

View file

@ -14,6 +14,7 @@
import type { ExecutionPolicy } from "./execution-policy.js";
import type { RecoveryAction, CloseoutResult } from "./engine-types.js";
import { runCustomVerification } from "./custom-verification.js";
import { parseUnitId } from "./unit-id.js";
export class CustomExecutionPolicy implements ExecutionPolicy {
private readonly runDir: string;
@ -48,8 +49,8 @@ export class CustomExecutionPolicy implements ExecutionPolicy {
unitId: string,
_context: { basePath: string },
): Promise<"continue" | "retry" | "pause"> {
const parts = unitId.split("/");
const stepId = parts[parts.length - 1];
const { milestone, slice, task } = parseUnitId(unitId);
const stepId = task ?? slice ?? milestone;
return runCustomVerification(this.runDir, stepId);
}

View file

@ -33,6 +33,7 @@ import {
} from "./graph.js";
import { injectContext } from "./context-injector.js";
import type { WorkflowDefinition, StepDefinition } from "./definition-loader.js";
import { parseUnitId } from "./unit-id.js";
/** Read and parse the frozen DEFINITION.yaml from a run directory. */
export function readFrozenDefinition(runDir: string): WorkflowDefinition {
@ -181,8 +182,8 @@ export class CustomWorkflowEngine implements WorkflowEngine {
const graph = state.raw as WorkflowGraph;
// Extract stepId from "<workflowName>/<stepId>"
const parts = completedStep.unitId.split("/");
const stepId = parts[parts.length - 1];
const { milestone, slice, task } = parseUnitId(completedStep.unitId);
const stepId = task ?? slice ?? milestone;
const updatedGraph = markStepComplete(graph, stepId);
writeGraph(this.runDir, updatedGraph);

View file

@ -2,6 +2,7 @@
import { resolveMilestoneFile } from "./paths.js";
import { findMilestoneIds } from "./guided-flow.js";
import { parseUnitId } from "./unit-id.js";
import { isDbAvailable, getMilestoneSlices } from "./gsd-db.js";
import { parseRoadmap } from "./parsers-legacy.js";
import { readFileSync } from "node:fs";
@ -22,7 +23,7 @@ export function getPriorSliceCompletionBlocker(
): string | null {
if (!SLICE_DISPATCH_TYPES.has(unitType)) return null;
const [targetMid, targetSid] = unitId.split("/");
const { milestone: targetMid, slice: targetSid } = parseUnitId(unitId);
if (!targetMid || !targetSid) return null;
// Use findMilestoneIds to respect custom queue order.

View file

@ -20,20 +20,19 @@ import type {
import { resolvePostUnitHooks, resolvePreDispatchHooks } from "./preferences.js";
import { existsSync, readFileSync, writeFileSync, mkdirSync } from "node:fs";
import { join } from "node:path";
import { parseUnitId } from "./unit-id.js";
// ─── Artifact Path Resolution ──────────────────────────────────────────────
export function resolveHookArtifactPath(basePath: string, unitId: string, artifactName: string): string {
const parts = unitId.split("/");
if (parts.length === 3) {
const [mid, sid, tid] = parts;
return join(basePath, ".gsd", "milestones", mid, "slices", sid, "tasks", `${tid}-${artifactName}`);
const { milestone, slice, task } = parseUnitId(unitId);
if (task !== undefined && slice !== undefined) {
return join(basePath, ".gsd", "milestones", milestone, "slices", slice, "tasks", `${task}-${artifactName}`);
}
if (parts.length === 2) {
const [mid, sid] = parts;
return join(basePath, ".gsd", "milestones", mid, "slices", sid, artifactName);
if (slice !== undefined) {
return join(basePath, ".gsd", "milestones", milestone, "slices", slice, artifactName);
}
return join(basePath, ".gsd", "milestones", parts[0], artifactName);
return join(basePath, ".gsd", "milestones", milestone, artifactName);
}
// ─── Dispatch Rule Conversion ──────────────────────────────────────────────
@ -212,7 +211,7 @@ export class RuleRegistry {
};
// Build prompt with variable substitution
const [mid, sid, tid] = triggerUnitId.split("/");
const { milestone: mid, slice: sid, task: tid } = parseUnitId(triggerUnitId);
let prompt = config.prompt
.replace(/\{milestoneId\}/g, mid ?? "")
.replace(/\{sliceId\}/g, sid ?? "")
@ -291,7 +290,7 @@ export class RuleRegistry {
return { action: "proceed", prompt, firedHooks: [] };
}
const [mid, sid, tid] = unitId.split("/");
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
const substitute = (text: string): string =>
text
.replace(/\{milestoneId\}/g, mid ?? "")
@ -506,7 +505,7 @@ export class RuleRegistry {
this.cycleCounts.set(cycleKey, currentCycle);
this.activeHook.cycle = currentCycle;
const [mid, sid, tid] = unitId.split("/");
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
const prompt = hook.prompt
.replace(/\{milestoneId\}/g, mid ?? "")
.replace(/\{sliceId\}/g, sid ?? "")

View file

@ -5,6 +5,7 @@ import { join, dirname } from "node:path";
import { tmpdir } from "node:os";
import { fileURLToPath } from "node:url";
import { invalidateAllCaches } from '../cache.ts';
import { parseUnitId } from "../unit-id.ts";
// loadPrompt reads from ~/.gsd/agent/extensions/gsd/prompts/ (main checkout).
// In a worktree the file may not exist there yet, so we resolve prompts
@ -222,8 +223,7 @@ describe("complete-milestone", () => {
const unitType = "complete-milestone";
const unitId = "M001";
const parts = unitId.split("/");
const mid = parts[0]!;
const { milestone: mid } = parseUnitId(unitId);
// This is the exact logic from diagnoseExpectedArtifact for "complete-milestone"
const result = `${relMilestoneFile(base, mid, "SUMMARY")} (milestone summary)`;

View file

@ -15,6 +15,7 @@ import {
} from "../reactive-graph.ts";
import { validatePreferences } from "../preferences-validation.ts";
import type { ReactiveExecutionState } from "../types.ts";
import { parseUnitId } from "../unit-id.ts";
// ─── Preference Validation ────────────────────────────────────────────────
@ -441,13 +442,12 @@ test("unitId batch encoding round-trips correctly", () => {
const unitId = `${mid}/${sid}/reactive+${selected.join(",")}`;
// Parse it back
const parts = unitId.split("/");
assert.equal(parts[0], "M001");
assert.equal(parts[1], "S01");
const batchPart = parts[2];
const plusIdx = batchPart.indexOf("+");
const { milestone, slice, task: batchPart } = parseUnitId(unitId);
assert.equal(milestone, "M001");
assert.equal(slice, "S01");
const plusIdx = batchPart!.indexOf("+");
assert.ok(plusIdx > 0, "Should have + separator");
const batchIds = batchPart.slice(plusIdx + 1).split(",");
const batchIds = batchPart!.slice(plusIdx + 1).split(",");
assert.deepEqual(batchIds, ["T02", "T03", "T05"]);
});

View file

@ -16,6 +16,7 @@ import {
resolveHookArtifactPath,
} from "../post-unit-hooks.ts";
import { uncheckTaskInPlan } from "../undo.ts";
import { parseUnitId } from "../unit-id.ts";
// ─── Fixture Helpers ───────────────────────────────────────────────────────
@ -183,8 +184,7 @@ test('Full retry reset: all steps combined', () => {
retryArtifact: "NEEDS-REWORK.md",
};
const parts = trigger.unitId.split("/");
const [mid, sid, tid] = parts;
const { milestone: mid, slice: sid, task: tid } = parseUnitId(trigger.unitId);
// Simulate completedUnits
let completedUnits = [
@ -264,11 +264,10 @@ test('Retry reset: idempotent when artifacts already missing', () => {
};
// These should not throw even with missing files
const parts = trigger.unitId.split("/");
const [mid, sid, tid] = parts;
const { milestone: mid, slice: sid, task: tid } = parseUnitId(trigger.unitId);
// Uncheck — returns false because no PLAN file
const uncheckResult = uncheckTaskInPlan(base, mid, sid, tid);
const uncheckResult = uncheckTaskInPlan(base, mid, sid!, tid!);
assert.ok(!uncheckResult, "uncheck returns false when no PLAN exists");
// Summary does not exist — no crash

View file

@ -7,6 +7,7 @@ import type { ExtensionCommandContext, ExtensionAPI } from "@gsd/pi-coding-agent
import { existsSync, readFileSync, writeFileSync, unlinkSync, readdirSync } from "node:fs";
import { join } from "node:path";
import { nativeRevertCommit, nativeRevertAbort } from "./native-git-bridge.js";
import { parseUnitId } from "./unit-id.js";
import { deriveState } from "./state.js";
import { invalidateAllCaches } from "./cache.js";
import { gsdRoot, resolveTasksDir, resolveSlicePath, resolveTaskFile, buildTaskFileName, buildSliceFileName } from "./paths.js";
@ -65,11 +66,11 @@ export async function handleUndo(args: string, ctx: ExtensionCommandContext, _pi
}
// 1. Delete summary artifact
const parts = unitId.split("/");
const { milestone, slice, task } = parseUnitId(unitId);
let summaryRemoved = false;
if (parts.length === 3) {
if (task !== undefined && slice !== undefined) {
// Task-level: M001/S01/T01
const [mid, sid, tid] = parts;
const [mid, sid, tid] = [milestone, slice, task];
const tasksDir = resolveTasksDir(basePath, mid, sid);
if (tasksDir) {
const summaryFile = join(tasksDir, buildTaskFileName(tid, "SUMMARY"));
@ -78,9 +79,9 @@ export async function handleUndo(args: string, ctx: ExtensionCommandContext, _pi
summaryRemoved = true;
}
}
} else if (parts.length === 2) {
} else if (slice !== undefined) {
// Slice-level: M001/S01
const [mid, sid] = parts;
const [mid, sid] = [milestone, slice];
const slicePath = resolveSlicePath(basePath, mid, sid);
if (slicePath) {
for (const suffix of ["SUMMARY", "COMPLETE"]) {
@ -95,8 +96,8 @@ export async function handleUndo(args: string, ctx: ExtensionCommandContext, _pi
// 2. Uncheck task in PLAN if execute-task
let planUpdated = false;
if (unitType === "execute-task" && parts.length === 3) {
const [mid, sid, tid] = parts;
if (unitType === "execute-task" && task !== undefined && slice !== undefined) {
const [mid, sid, tid] = [milestone, slice, task];
planUpdated = uncheckTaskInPlan(basePath, mid, sid, tid);
}

View file

@ -8,6 +8,7 @@ import {
resolveTaskFile,
} from "./paths.js";
import { loadFile, parseTaskPlanMustHaves, countMustHavesMentionedInSummary } from "./files.js";
import { parseUnitId } from "./unit-id.js";
export type UnitRuntimePhase =
| "dispatched"
@ -128,7 +129,7 @@ export async function inspectExecuteTaskDurability(
basePath: string,
unitId: string,
): Promise<ExecuteTaskRecoveryStatus | null> {
const [mid, sid, tid] = unitId.split("/");
const { milestone: mid, slice: sid, task: tid } = parseUnitId(unitId);
if (!mid || !sid || !tid) return null;
const planAbs = resolveSliceFile(basePath, mid, sid, "PLAN");