fix(gsd): break 3 circular dependencies in extension modules (#3730)

Phase 1 of #3631 — eliminate circular imports before screaming arch reorg.

Cycle 1 (auto.ts ↔ auto-direct-dispatch.ts):
  Remove redundant re-export of dispatchDirectPhase from auto.ts.
  No consumer imported it through auto.ts.

Cycle 2 (context-injector.ts ↔ custom-workflow-engine.ts):
  Extract readFrozenDefinition to new definition-io.ts.
  context-injector now imports from definition-io directly.

Cycle 3 (preferences.ts ↔ preferences-skills.ts):
  Move formatSkillRef to preferences-types.ts (pure fn, depends only on
  SkillResolution which is already there).
  Move resolveSkillDiscoveryMode + resolveSkillStalenessDays into
  preferences.ts (trivial wrappers over loadEffectiveGSDPreferences).

Tests: new definition-io.test.ts (3 tests), preferences-formatting.test.ts
(6 tests covering all formatSkillRef branches).

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

src/resources/extensions/gsd/auto.ts

@@ -1717,9 +1717,6 @@ export async function dispatchHookUnit(
   return true;
 }
 
-// Direct phase dispatch → auto-direct-dispatch.ts
-export { dispatchDirectPhase } from "./auto-direct-dispatch.js";
-
 // Re-export recovery functions for external consumers
 export {
   buildLoopRemediationSteps,

src/resources/extensions/gsd/context-injector.ts

@@ -16,7 +16,7 @@
 import { readFileSync, existsSync } from "node:fs";
 import { join, resolve, sep } from "node:path";
 import type { StepDefinition } from "./definition-loader.js";
-import { readFrozenDefinition } from "./custom-workflow-engine.js";
+import { readFrozenDefinition } from "./definition-io.js";
 
 /** Maximum characters per artifact to prevent context window blowout. */
 const MAX_CONTEXT_CHARS = 10_000;

src/resources/extensions/gsd/custom-workflow-engine.ts

@@ -22,7 +22,6 @@ import type {
 } from "./engine-types.js";
 import { readFileSync } from "node:fs";
 import { join } from "node:path";
-import { parse } from "yaml";
 import {
   readGraph,
   writeGraph,
@@ -32,16 +31,13 @@ import {
   type WorkflowGraph,
 } from "./graph.js";
 import { injectContext } from "./context-injector.js";
-import type { WorkflowDefinition, StepDefinition } from "./definition-loader.js";
+import type { StepDefinition } from "./definition-loader.js";
+import { readFrozenDefinition } from "./definition-io.js";
 import { parseUnitId } from "./unit-id.js";
 import { withFileLock } from "./file-lock.js";
 
-/** Read and parse the frozen DEFINITION.yaml from a run directory. */
-export function readFrozenDefinition(runDir: string): WorkflowDefinition {
-  const defPath = join(runDir, "DEFINITION.yaml");
-  const raw = readFileSync(defPath, "utf-8");
-  return parse(raw, { schema: "core" }) as WorkflowDefinition;
-}
+// Re-export for downstream consumers
+export { readFrozenDefinition } from "./definition-io.js";
 
 export class CustomWorkflowEngine implements WorkflowEngine {
   readonly engineId = "custom";

src/resources/extensions/gsd/definition-io.ts

@@ -0,0 +1,18 @@
+/**
+ * definition-io.ts — Read frozen DEFINITION.yaml from a run directory.
+ *
+ * Extracted from custom-workflow-engine.ts to break the circular dependency
+ * between context-injector.ts and custom-workflow-engine.ts.
+ */
+
+import { readFileSync } from "node:fs";
+import { join } from "node:path";
+import { parse } from "yaml";
+import type { WorkflowDefinition } from "./definition-loader.js";
+
+/** Read and parse the frozen DEFINITION.yaml from a run directory. */
+export function readFrozenDefinition(runDir: string): WorkflowDefinition {
+  const defPath = join(runDir, "DEFINITION.yaml");
+  const raw = readFileSync(defPath, "utf-8");
+  return parse(raw, { schema: "core" }) as WorkflowDefinition;
+}

src/resources/extensions/gsd/preferences-skills.ts

@@ -17,7 +17,6 @@ import type {
   SkillResolutionReport,
 } from "./preferences-types.js";
 import { validatePreferences } from "./preferences-validation.js";
-import { loadEffectiveGSDPreferences } from "./preferences.js";
 
 // Re-export types so existing consumers of ./preferences-skills.js keep working
 export type { GSDSkillRule, SkillDiscoveryMode, SkillResolution, SkillResolutionReport } from "./preferences-types.js";
@@ -143,38 +142,5 @@ export function resolveAllSkillReferences(preferences: GSDPreferences, cwd: stri
   return { resolutions, warnings };
 }
 
-/**
- * Format a skill reference for the system prompt.
- * If resolved, shows the path so the agent knows exactly where to read.
- * If unresolved, marks it clearly.
- */
-export function formatSkillRef(ref: string, resolutions: Map<string, SkillResolution>): string {
-  const resolution = resolutions.get(ref);
-  if (!resolution || resolution.method === "unresolved") {
-    return `${ref} (⚠ not found — check skill name or path)`;
-  }
-  // For absolute paths where SKILL.md is just appended, don't clutter the output
-  if (resolution.method === "absolute-path" || resolution.method === "absolute-dir") {
-    return ref;
-  }
-  // For bare names resolved from skill directories, show the resolved path
-  return `${ref} → \`${resolution.resolvedPath}\``;
-}
-
-/**
- * Resolve the skill discovery mode from effective preferences.
- * Defaults to "suggest" -- skills are identified during research but not installed automatically.
- */
-export function resolveSkillDiscoveryMode(): SkillDiscoveryMode {
-  const prefs = loadEffectiveGSDPreferences();
-  return prefs?.preferences.skill_discovery ?? "suggest";
-}
-
-/**
- * Resolve the skill staleness threshold in days.
- * Returns 0 if disabled, default 60 if not configured.
- */
-export function resolveSkillStalenessDays(): number {
-  const prefs = loadEffectiveGSDPreferences();
-  return prefs?.preferences.skill_staleness_days ?? 60;
-}
+// resolveSkillDiscoveryMode and resolveSkillStalenessDays moved to
+// preferences.ts to break circular dependency (they need loadEffectiveGSDPreferences).

src/resources/extensions/gsd/preferences-types.ts

@@ -384,3 +384,19 @@ export interface SkillResolutionReport {
   /** References that could not be resolved. */
   warnings: string[];
 }
+
+/**
+ * Format a skill reference for the system prompt.
+ * If resolved, shows the path so the agent knows exactly where to read.
+ * If unresolved, marks it clearly.
+ */
+export function formatSkillRef(ref: string, resolutions: Map<string, SkillResolution>): string {
+  const resolution = resolutions.get(ref);
+  if (!resolution || resolution.method === "unresolved") {
+    return `${ref} (⚠ not found — check skill name or path)`;
+  }
+  if (resolution.method === "absolute-path" || resolution.method === "absolute-dir") {
+    return ref;
+  }
+  return `${ref} → \`${resolution.resolvedPath}\``;
+}

src/resources/extensions/gsd/preferences.ts

@@ -29,9 +29,10 @@ import {
   type GSDPreferences,
   type LoadedGSDPreferences,
   type SkillResolution,
+  type SkillDiscoveryMode,
+  formatSkillRef,
 } from "./preferences-types.js";
 import { validatePreferences } from "./preferences-validation.js";
-import { formatSkillRef } from "./preferences-skills.js";
 
 // ─── Re-exports: types ──────────────────────────────────────────────────────
 // Every type/interface that was previously exported from this file is
@@ -60,11 +61,20 @@ export type {
 export { validatePreferences } from "./preferences-validation.js";
 
 // ─── Re-exports: skills ─────────────────────────────────────────────────────
-export {
-  resolveAllSkillReferences,
-  resolveSkillDiscoveryMode,
-  resolveSkillStalenessDays,
-} from "./preferences-skills.js";
+export { resolveAllSkillReferences } from "./preferences-skills.js";
+
+// These lived in preferences-skills.ts but imported loadEffectiveGSDPreferences
+// back from this file, creating a circular dependency. Moved here since they
+// are trivial wrappers over loadEffectiveGSDPreferences.
+export function resolveSkillDiscoveryMode(): SkillDiscoveryMode {
+  const prefs = loadEffectiveGSDPreferences();
+  return prefs?.preferences.skill_discovery ?? "suggest";
+}
+
+export function resolveSkillStalenessDays(): number {
+  const prefs = loadEffectiveGSDPreferences();
+  return prefs?.preferences.skill_staleness_days ?? 60;
+}
 
 // ─── Re-exports: models ─────────────────────────────────────────────────────
 export {

src/resources/extensions/gsd/tests/definition-io.test.ts

@@ -0,0 +1,57 @@
+/**
+ * definition-io.ts — unit tests for readFrozenDefinition.
+ */
+
+import { describe, test, beforeEach, afterEach } from "node:test";
+import assert from "node:assert/strict";
+import { mkdtempSync, mkdirSync, writeFileSync, rmSync, realpathSync } from "node:fs";
+import { join } from "node:path";
+import { tmpdir } from "node:os";
+
+import { readFrozenDefinition } from "../definition-io.ts";
+
+function createTmpDir(): string {
+  return realpathSync(mkdtempSync(join(tmpdir(), "gsd-defio-test-")));
+}
+
+describe("readFrozenDefinition", () => {
+  let runDir: string;
+
+  beforeEach(() => {
+    runDir = createTmpDir();
+  });
+
+  afterEach(() => {
+    rmSync(runDir, { recursive: true, force: true });
+  });
+
+  test("parses a valid DEFINITION.yaml", () => {
+    const yaml = [
+      "version: 1",
+      "name: test-workflow",
+      "description: A test workflow",
+      "steps:",
+      "  - id: step-1",
+      "    prompt: do the thing",
+    ].join("\n");
+    writeFileSync(join(runDir, "DEFINITION.yaml"), yaml, "utf-8");
+
+    const def = readFrozenDefinition(runDir);
+    assert.equal(def.version, 1);
+    assert.equal(def.name, "test-workflow");
+    assert.equal(def.description, "A test workflow");
+    assert.equal(def.steps.length, 1);
+    assert.equal(def.steps[0].id, "step-1");
+  });
+
+  test("throws when DEFINITION.yaml is missing", () => {
+    assert.throws(() => readFrozenDefinition(runDir), {
+      code: "ENOENT",
+    });
+  });
+
+  test("throws on malformed YAML", () => {
+    writeFileSync(join(runDir, "DEFINITION.yaml"), ": : : not valid yaml [", "utf-8");
+    assert.throws(() => readFrozenDefinition(runDir));
+  });
+});

src/resources/extensions/gsd/tests/preferences-formatting.test.ts

@@ -0,0 +1,87 @@
+/**
+ * Tests for formatSkillRef — pure formatting logic for skill references
+ * in the system prompt. Moved from preferences-skills.ts to preferences-types.ts
+ * to break the preferences ↔ preferences-skills circular dependency.
+ */
+
+import { describe, test } from "node:test";
+import assert from "node:assert/strict";
+
+import { formatSkillRef } from "../preferences-types.ts";
+import type { SkillResolution } from "../preferences-types.ts";
+
+function makeResolutions(entries: [string, Partial<SkillResolution>][]): Map<string, SkillResolution> {
+  const map = new Map<string, SkillResolution>();
+  for (const [key, partial] of entries) {
+    map.set(key, {
+      original: partial.original ?? key,
+      resolvedPath: partial.resolvedPath ?? null,
+      method: partial.method ?? "unresolved",
+    });
+  }
+  return map;
+}
+
+describe("formatSkillRef", () => {
+  test("marks unresolved references with a warning", () => {
+    const resolutions = makeResolutions([
+      ["my-skill", { method: "unresolved" }],
+    ]);
+    const result = formatSkillRef("my-skill", resolutions);
+    assert.match(result, /my-skill/);
+    assert.match(result, /not found/);
+  });
+
+  test("marks unknown references (not in map) with a warning", () => {
+    const resolutions = new Map<string, SkillResolution>();
+    const result = formatSkillRef("unknown-skill", resolutions);
+    assert.match(result, /unknown-skill/);
+    assert.match(result, /not found/);
+  });
+
+  test("returns bare ref for absolute-path resolution", () => {
+    const resolutions = makeResolutions([
+      ["/home/user/skills/SKILL.md", {
+        method: "absolute-path",
+        resolvedPath: "/home/user/skills/SKILL.md",
+      }],
+    ]);
+    const result = formatSkillRef("/home/user/skills/SKILL.md", resolutions);
+    assert.equal(result, "/home/user/skills/SKILL.md");
+  });
+
+  test("returns bare ref for absolute-dir resolution", () => {
+    const resolutions = makeResolutions([
+      ["/home/user/skills/my-skill", {
+        method: "absolute-dir",
+        resolvedPath: "/home/user/skills/my-skill/SKILL.md",
+      }],
+    ]);
+    const result = formatSkillRef("/home/user/skills/my-skill", resolutions);
+    assert.equal(result, "/home/user/skills/my-skill");
+  });
+
+  test("shows resolved path for user-skill resolution", () => {
+    const resolutions = makeResolutions([
+      ["code-review", {
+        method: "user-skill",
+        resolvedPath: "/home/user/.claude/skills/code-review/SKILL.md",
+      }],
+    ]);
+    const result = formatSkillRef("code-review", resolutions);
+    assert.match(result, /code-review/);
+    assert.match(result, /\.claude\/skills\/code-review\/SKILL\.md/);
+  });
+
+  test("shows resolved path for project-skill resolution", () => {
+    const resolutions = makeResolutions([
+      ["lint-fix", {
+        method: "project-skill",
+        resolvedPath: "/repo/.gsd/skills/lint-fix/SKILL.md",
+      }],
+    ]);
+    const result = formatSkillRef("lint-fix", resolutions);
+    assert.match(result, /lint-fix/);
+    assert.match(result, /\.gsd\/skills\/lint-fix\/SKILL\.md/);
+  });
+});