From 397c34142a669a555505292e76269a043d070605 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 8 May 2026 09:06:43 -0400 Subject: [PATCH] Deepen SDK package seam and converge runtime skills policy (#3238) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Deepen SDK package seam and converge runtime skills policy * fix(sdk): unified install-root resolution for workflows and agents (CR finding 1) Use the already-resolved gsdInstallDir constant instead of calling resolveLegacyInstallDir() again when computing agentsDir, ensuring workflowsDir and agentsDir share the same install root. * fix(sdk): tilde shortening requires path-boundary match (CR finding 2) Both renderGlobalSkillsBaseDisplayPath and renderGlobalSkillDisplayPath used startsWith(home) which could incorrectly shorten unrelated paths sharing the same prefix. Now checks for home === base or base.startsWith(home + sep) to ensure a real directory boundary. * fix(sdk): validate loadConfig export before invocation (CR finding 3) After requiring core.cjs, check typeof mod.loadConfig === 'function' before calling it. Throws a classified GSDError with the module path if the export is missing, rather than a generic TypeError. * fix(test): guard root lookup before .path dereference (CR finding 4) Added assert.ok() guards for claudeRoot and codexRoot after the .find() calls so that a missing root produces an explicit assertion failure rather than a TypeError on .path dereference. * fix(ci): fail-safe on transient API errors in approval dismissal (CR finding 6) resolveRole() returns 'unknown' for non-404 errors (rate limits, 5xx, network blips). shouldDismissReviewer() now treats 'unknown' as unresolvable and skips dismissal, preventing legitimate approvals from being dismissed due to a transient API failure. Only 'none' (true 404) is treated as a confirmed non-collaborator. * changeset: pr=3238 SDK package seam and runtime skills convergence * fix(sdk): harden resolveGlobalSkillDir against path traversal (CR finding 1) Use resolve+relative to validate that skillName cannot escape the global skills base directory. Values like "../../foo" or absolute paths now return null instead of joining directly. All imports (resolve, relative, isAbsolute) were already present in helpers.ts. Co-Authored-By: Claude Sonnet 4.6 * fix(sdk): split skill-dir-resolution and skill-not-found warnings (CR finding 2) After resolveGlobalSkillDir's hardening can return null for traversal attempts, the old single-branch warning "Global skill not found at ..." was misleading. Split into two distinct cases: - skillDir === null → "Could not resolve global skill directory for ..." - skillMd missing → "Global skill not found at ..." Co-Authored-By: Claude Sonnet 4.6 * test: lock skill path-traversal rejection in resolveGlobalSkillDir Regression test verifying that traversal segments (../../foo, ../escape), empty string, and absolute paths are all rejected (return null), while a legitimate skill name resolves correctly. Co-Authored-By: Claude Sonnet 4.6 * test(sdk): align display-path contract + traversal coverage for resolveGlobalSkillMarkdownPath (CR nitpicks) - renderGlobalSkillsBaseDisplayPath now returns a non-null string for unsupported runtimes (e.g. cline → "(cline does not use a skills directory)") matching the existing renderGlobalSkillDisplayPath contract; callers of both helpers no longer need null-checks for unsupported runtimes. - Remove now-redundant ! non-null assertion on renderGlobalSkillsBaseDisplayPath calls in skill-manifest.ts (return type is string, not string | null). - Extend the path-traversal test block to assert resolveGlobalSkillMarkdownPath also propagates null for ../../foo, ../escape, empty, and /abs/path inputs, locking the null-propagation contract against future refactors. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Sonnet 4.6 --- .changeset/gallant-ravens-travel.md | 5 + .../dismiss-unauthorized-pr-approvals.yml | 7 +- CONTEXT.md | 6 + docs/adr/0003-sdk-package-seam-module.md | 8 + get-shit-done/bin/lib/init.cjs | 5 +- get-shit-done/bin/lib/profile-output.cjs | 14 +- sdk/src/init-runner.ts | 4 +- sdk/src/phase-prompt.ts | 6 +- sdk/src/query-gsd-tools-path.ts | 25 +--- sdk/src/query/QUERY-HANDLERS.md | 5 +- sdk/src/query/helpers.test.ts | 54 +++++++ sdk/src/query/helpers.ts | 53 +++++++ sdk/src/query/profile-output.ts | 22 ++- sdk/src/query/profile.test.ts | 64 +++++++- sdk/src/query/roadmap.ts | 7 +- sdk/src/query/skill-manifest.test.ts | 62 ++++++++ sdk/src/query/skill-manifest.ts | 8 +- sdk/src/query/skills.test.ts | 36 +++++ sdk/src/query/skills.ts | 26 +++- sdk/src/query/state-project-load.ts | 33 +---- sdk/src/query/validate.ts | 5 +- sdk/src/query/verify.ts | 10 +- sdk/src/sdk-package-compatibility.test.ts | 95 ++++++++++++ sdk/src/sdk-package-compatibility.ts | 137 ++++++++++++++++++ sdk/src/types.ts | 2 +- tests/profile-output.test.cjs | 41 ++++++ tests/skill-manifest.test.cjs | 17 +++ 27 files changed, 658 insertions(+), 99 deletions(-) create mode 100644 .changeset/gallant-ravens-travel.md create mode 100644 docs/adr/0003-sdk-package-seam-module.md create mode 100644 sdk/src/query/skill-manifest.test.ts create mode 100644 sdk/src/sdk-package-compatibility.test.ts create mode 100644 sdk/src/sdk-package-compatibility.ts diff --git a/.changeset/gallant-ravens-travel.md b/.changeset/gallant-ravens-travel.md new file mode 100644 index 000000000..7bd4057f7 --- /dev/null +++ b/.changeset/gallant-ravens-travel.md @@ -0,0 +1,5 @@ +--- +type: Changed +pr: 3238 +--- +SDK package seam deepened and runtime skills policy converged on a single home-directory resolution path — install root is now consistent across workflows and agents directories diff --git a/.github/workflows/dismiss-unauthorized-pr-approvals.yml b/.github/workflows/dismiss-unauthorized-pr-approvals.yml index 366cb0f7d..439e37e68 100644 --- a/.github/workflows/dismiss-unauthorized-pr-approvals.yml +++ b/.github/workflows/dismiss-unauthorized-pr-approvals.yml @@ -54,18 +54,21 @@ jobs: function shouldDismissReviewer(login, roleName) { const reviewer = norm(login); const isBlocked = blockedReviewers.has(reviewer); + // 'unknown' means a transient API error — fail-safe: keep the approval. + // Only 'none' (true 404) is a confirmed non-collaborator. + const isKnownRole = roleName !== 'unknown'; const isAllowedRole = allowedRoles.has(roleName); const reasons = []; if (isBlocked) { reasons.push(`reviewer is blocked (${reviewer})`); } - if (!isAllowedRole) { + if (isKnownRole && !isAllowedRole) { reasons.push(`reviewer is not a collaborator or higher (detected role: ${roleName})`); } return { - dismiss: isBlocked || !isAllowedRole, + dismiss: isBlocked || (isKnownRole && !isAllowedRole), message: `Auto-dismissed approval: ${reasons.join('; ')}.`, }; } diff --git a/CONTEXT.md b/CONTEXT.md index 04340a437..7cbe0ce4b 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -40,6 +40,12 @@ Module owning command resolution, policy projection (`mutation`, `output_mode`), ### Query Pre-Project Config Policy Module Module policy that defines query-time behavior when `.planning/config.json` is absent: use built-in defaults for parity-sensitive query Interfaces, and emit parity-aligned empty model ids for pre-project model resolution surfaces. +### SDK Package Seam Module +Module owning SDK-to-`get-shit-done-cc` compatibility policy: legacy asset discovery, install-layout probing, transition-only error messaging, and thin Adapter access for CJS-era assets that native SDK Modules have not replaced yet. + +### Runtime-Global Skills Policy Module +Module owning runtime-aware global skills directory policy for SDK query surfaces. Resolves runtime-global skills bases/skill paths from runtime + env precedence, renders display paths for warnings/manifests, and reports unsupported runtimes with no skills directory. + ### MVP Mode Phase-level planning mode that frames work as a vertical slice (UI → API → DB) of one user-visible capability instead of horizontal layers. Resolved at workflow init via the precedence chain: `--mvp` CLI flag → ROADMAP.md `**Mode:** mvp` field → `workflow.mvp_mode` config → false. All-or-nothing per phase (PRD #2826 Q1). Surfaced as `MVP_MODE=true|false` to the planner, executor, verifier, and discovery surfaces (progress, stats, graphify). Canonical parser: `roadmap.cjs` `**Mode:**` field; canonical resolution chain documented in `workflows/plan-phase.md`. Concept index: `references/mvp-concepts.md`. diff --git a/docs/adr/0003-sdk-package-seam-module.md b/docs/adr/0003-sdk-package-seam-module.md new file mode 100644 index 000000000..ea40089f8 --- /dev/null +++ b/docs/adr/0003-sdk-package-seam-module.md @@ -0,0 +1,8 @@ +# SDK Package Seam Module owns SDK-to-get-shit-done-cc compatibility + +- **Status:** Accepted +- **Date:** 2026-05-07 + +We decided to define one explicit SDK Package Seam Module for the `@gsd-build/sdk` → `get-shit-done-cc` transition. During this transition, install-layout probing, legacy `gsd-tools.cjs` discovery, legacy `core.cjs` discovery, and compatibility-only missing-asset diagnostics must live behind one seam instead of leaking across SDK Modules. This keeps callers thin, raises leverage for standalone-SDK testing, and improves locality by making package-readiness bugs land in one place. First tracer-bullet slice: add one compatibility Adapter Module at this seam and migrate current legacy asset callers onto it before broader native replacement work. + +Runtime-global skills directory resolution is explicitly out of scope for this seam. That policy varies by runtime (`claude`, `codex`, `cline`, etc.) rather than by legacy package/install layout, so it now lives in a separate Runtime-Global Skills Policy Module consumed by `agent-skills` and `skill-manifest`. diff --git a/get-shit-done/bin/lib/init.cjs b/get-shit-done/bin/lib/init.cjs index 0848b4212..d79ce60e0 100644 --- a/get-shit-done/bin/lib/init.cjs +++ b/get-shit-done/bin/lib/init.cjs @@ -1770,6 +1770,7 @@ function cmdAgentSkills(cwd, agentType, raw) { */ function buildSkillManifest(cwd, skillsDir = null) { const { extractFrontmatter } = require('./frontmatter.cjs'); + const { getGlobalSkillsBase } = require('./runtime-homes.cjs'); const os = require('os'); const canonicalRoots = skillsDir ? [{ @@ -1811,13 +1812,13 @@ function buildSkillManifest(cwd, skillsDir = null) { }, { root: '~/.claude/skills', - path: path.join(os.homedir(), '.claude', 'skills'), + path: getGlobalSkillsBase('claude'), scope: 'global', kind: 'skills', }, { root: '~/.codex/skills', - path: path.join(os.homedir(), '.codex', 'skills'), + path: getGlobalSkillsBase('codex'), scope: 'global', kind: 'skills', }, diff --git a/get-shit-done/bin/lib/profile-output.cjs b/get-shit-done/bin/lib/profile-output.cjs index 914607f57..e24b4061b 100644 --- a/get-shit-done/bin/lib/profile-output.cjs +++ b/get-shit-done/bin/lib/profile-output.cjs @@ -13,6 +13,7 @@ const fs = require('fs'); const path = require('path'); const os = require('os'); const { output, error, safeReadFile, loadConfig } = require('./core.cjs'); +const { getGlobalSkillDir } = require('./runtime-homes.cjs'); // ─── Constants ──────────────────────────────────────────────────────────────── @@ -786,7 +787,18 @@ function cmdGenerateDevPreferences(cwd, options, raw) { // string) so the cline-install leaked-path lint does not flag it. let outputPath = options.output; if (!outputPath) { - outputPath = path.join(os.homedir(), '.claude', 'skills', 'gsd-dev-preferences', 'SKILL.md'); + let effectiveRuntime = 'claude'; + try { + const config = loadConfig(cwd); + effectiveRuntime = process.env.GSD_RUNTIME || config.runtime || 'claude'; + } catch { + effectiveRuntime = process.env.GSD_RUNTIME || 'claude'; + } + const skillDir = getGlobalSkillDir(effectiveRuntime, 'gsd-dev-preferences'); + if (!skillDir) { + error(`Runtime "${effectiveRuntime}" does not use a skills directory; pass --output to choose a path explicitly.`); + } + outputPath = path.join(skillDir, 'SKILL.md'); } else if (!path.isAbsolute(outputPath)) { outputPath = path.join(cwd, outputPath); } diff --git a/sdk/src/init-runner.ts b/sdk/src/init-runner.ts index 65dfcb362..79024c0c4 100644 --- a/sdk/src/init-runner.ts +++ b/sdk/src/init-runner.ts @@ -11,7 +11,6 @@ import { readFile, writeFile, mkdir } from 'node:fs/promises'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { homedir } from 'node:os'; import { execFile } from 'node:child_process'; import type { @@ -34,10 +33,11 @@ import { loadConfig } from './config.js'; import { runPhaseStepSession } from './session-runner.js'; import { sanitizePrompt } from './prompt-sanitizer.js'; import { resolveAgentsDir } from './query/helpers.js'; +import { resolveLegacyTemplatesDir } from './sdk-package-compatibility.js'; // ─── Constants ─────────────────────────────────────────────────────────────── -const GSD_TEMPLATES_DIR = join(homedir(), '.claude', 'get-shit-done', 'templates'); +const GSD_TEMPLATES_DIR = resolveLegacyTemplatesDir(); const GSD_AGENTS_DIR = resolveAgentsDir(); const RESEARCH_TYPES = ['STACK', 'FEATURES', 'ARCHITECTURE', 'PITFALLS'] as const; diff --git a/sdk/src/phase-prompt.ts b/sdk/src/phase-prompt.ts index cb6faa254..27ed1e933 100644 --- a/sdk/src/phase-prompt.ts +++ b/sdk/src/phase-prompt.ts @@ -9,13 +9,13 @@ import { readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { homedir } from 'node:os'; import type { ContextFiles, ParsedPlan } from './types.js'; import { PhaseType } from './types.js'; import { buildExecutorPrompt } from './prompt-builder.js'; import { PHASE_AGENT_MAP } from './tool-scoping.js'; import { sanitizePrompt } from './prompt-sanitizer.js'; +import { resolveLegacyInstallDir } from './sdk-package-compatibility.js'; // ─── Workflow file mapping ─────────────────────────────────────────────────── @@ -89,9 +89,9 @@ export class PromptFactory { sdkPromptsDir?: string; projectDir?: string; }) { - const gsdInstallDir = options?.gsdInstallDir ?? join(homedir(), '.claude', 'get-shit-done'); + const gsdInstallDir = options?.gsdInstallDir ?? resolveLegacyInstallDir(); this.workflowsDir = join(gsdInstallDir, 'workflows'); - this.agentsDir = options?.agentsDir ?? join(homedir(), '.claude', 'agents'); + this.agentsDir = options?.agentsDir ?? join(gsdInstallDir, '..', 'agents'); this.projectAgentsDir = options?.projectAgentsDir; this.projectDir = options?.projectDir; // SDK prompts dir: explicit override → package-relative default via import.meta.url diff --git a/sdk/src/query-gsd-tools-path.ts b/sdk/src/query-gsd-tools-path.ts index 18c22292a..d4f4e510e 100644 --- a/sdk/src/query-gsd-tools-path.ts +++ b/sdk/src/query-gsd-tools-path.ts @@ -1,24 +1 @@ -import { existsSync } from 'node:fs'; -import { join } from 'node:path'; -import { homedir } from 'node:os'; -import { fileURLToPath } from 'node:url'; - -const BUNDLED_GSD_TOOLS_PATH = fileURLToPath( - new URL('../../get-shit-done/bin/gsd-tools.cjs', import.meta.url), -); - -/** - * Resolve gsd-tools.cjs path. - * Probe order: SDK-bundled repo copy → project/.claude/get-shit-done → ~/.claude/get-shit-done - */ -export function resolveGsdToolsPath(projectDir: string): string { - const candidates = [ - BUNDLED_GSD_TOOLS_PATH, - join(projectDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), - join(homedir(), '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), - ]; - - return candidates.find(candidate => existsSync(candidate)) ?? candidates[candidates.length - 1]!; -} - -export { BUNDLED_GSD_TOOLS_PATH }; +export { resolveGsdToolsPath, BUNDLED_GSD_TOOLS_PATH } from './sdk-package-compatibility.js'; diff --git a/sdk/src/query/QUERY-HANDLERS.md b/sdk/src/query/QUERY-HANDLERS.md index afe6f283e..57c720881 100644 --- a/sdk/src/query/QUERY-HANDLERS.md +++ b/sdk/src/query/QUERY-HANDLERS.md @@ -107,7 +107,7 @@ Handlers for `**state.signal-waiting`**, `**state.signal-resume**`, `**state.val **`state.json` vs `state.load` (different CJS commands):** - **`state.json`** / `state json` — port of **`cmdStateJson`** (`state.ts` `stateJson`): rebuilt STATE.md frontmatter JSON. Read-only golden: `read-only-parity.integration.test.ts` compares to CJS `state json` with **`last_updated`** stripped. -- **`state.load`** / `state load` — port of **`cmdStateLoad`** (`state-project-load.ts` `stateProjectLoad`): `{ config, state_raw, state_exists, roadmap_exists, config_exists }`; **`config`** comes from **`get-shit-done/bin/lib/core.cjs`** `loadConfig` (resolved via the same candidate paths as a normal GSD install). Read-only golden: full `toEqual` vs `state load`. If `core.cjs` cannot be resolved, dispatch throws **`GSDError`** (document for minimal `@gsd-build/sdk`-only installs). +- **`state.load`** / `state load` — port of **`cmdStateLoad`** (`state-project-load.ts` `stateProjectLoad`): `{ config, state_raw, state_exists, roadmap_exists, config_exists }`; **`config`** comes from **`get-shit-done/bin/lib/core.cjs`** `loadConfig`, but discovery now routes through the **SDK Package Seam Module** (`sdk-package-compatibility.ts`) so install-layout probing stays behind one compatibility Adapter. Read-only golden: full `toEqual` vs `state load`. If `core.cjs` cannot be resolved, dispatch throws **`GSDError`** with the checked probe list (document for minimal `@gsd-build/sdk`-only installs). `stateExtractField` in `helpers.ts` uses **horizontal whitespace only** after `Field:` so YAML keys such as lowercase `progress:` in frontmatter are not mistaken for the body `Progress:` line (see `get-shit-done/bin/lib/state.cjs` — same rule). @@ -176,8 +176,9 @@ From `read-only-parity.integration.test.ts` (full `toEqual` on this repo): | `summary.extract` | Fixture `sdk/src/golden/fixtures/summary-extract-sample.md`; uses `extractFrontmatterLeading` (first `---` block) for parity with `frontmatter.cjs`. | | `history.digest` | No args; aggregate over `.planning/phases` + archived milestone phase dirs (`commands.cjs` `cmdHistoryDigest`). | | `audit-uat` | No args; full JSON parity with `uat.cjs` `cmdAuditUat` (`results`, `summary` with `by_category` / `by_phase`). | -| `skill-manifest` | No args; full manifest parity with `init.cjs` `buildSkillManifest` / `cmdSkillManifest`. Handler uses `extractFrontmatterLeading` (first `---` block) like CJS `frontmatter.cjs` `extractFrontmatter` — not TS `extractFrontmatter` (last block), so skills with multiple `---` sections match CJS. | +| `skill-manifest` | No args; full manifest parity with `init.cjs` `buildSkillManifest` / `cmdSkillManifest`. Handler uses `extractFrontmatterLeading` (first `---` block) like CJS `frontmatter.cjs` `extractFrontmatter` — not TS `extractFrontmatter` (last block), so skills with multiple `---` sections match CJS. Runtime-global skill roots now route through the **Runtime-Global Skills Policy Module**; legacy import-only skill root discovery (`~/.claude/get-shit-done/skills`) routes through the **SDK Package Seam Module**. | | `validate.agents` | No args; `agents_dir` matches `core.cjs` `getAgentsDir` (`GSD_AGENTS_DIR` or `sdk/dist/query/../../../agents` in this monorepo — same absolute path as CLI). `MODEL_PROFILES` / `expected` list stays aligned with `get-shit-done/bin/lib/model-profiles.cjs`. | +| `agent-skills` | Reads `config.agent_skills[agentType]` and emits raw `` XML. Project-relative entries stay project-root validated; `global:` resolves through the **Runtime-Global Skills Policy Module** instead of a Claude-only path. | | `state.get` | Dedicated tests: no args → full `{ content }` vs `state get`; one field (`milestone`) → `{ milestone: "…" }` vs `state get milestone` (frontmatter line match). | | `state.json` | `state json` vs SDK; **`last_updated`** stripped before `toEqual` (volatile). | | `state.load` | `state load` vs SDK; full **`cmdStateLoad`** object graph (`config`, `state_raw`, existence flags). | diff --git a/sdk/src/query/helpers.test.ts b/sdk/src/query/helpers.test.ts index 913bc4f44..7f40d5e87 100644 --- a/sdk/src/query/helpers.test.ts +++ b/sdk/src/query/helpers.test.ts @@ -21,6 +21,11 @@ import { resolveAgentsDir, getRuntimeConfigDir, detectRuntime, + resolveGlobalSkillsBase, + resolveGlobalSkillDir, + resolveGlobalSkillMarkdownPath, + renderGlobalSkillsBaseDisplayPath, + renderGlobalSkillDisplayPath, findProjectRoot, SUPPORTED_RUNTIMES, type Runtime, @@ -428,6 +433,55 @@ describe('resolveAgentsDir (runtime-aware)', () => { // ─── findProjectRoot (issue #2623) ───────────────────────────────────────── +describe('runtime-global skills directory helpers', () => { + const saved: Record = {}; + beforeEach(() => { + for (const k of RUNTIME_ENV_VARS) { saved[k] = process.env[k]; delete process.env[k]; } + }); + afterEach(() => { + for (const k of RUNTIME_ENV_VARS) { + if (saved[k] === undefined) delete process.env[k]; + else process.env[k] = saved[k]; + } + }); + + it('appends /skills for runtimes with a global skills directory', () => { + process.env.CODEX_HOME = '/codex'; + expect(resolveGlobalSkillsBase('codex')).toBe(join('/codex', 'skills')); + expect(resolveGlobalSkillDir('codex', 'demo')).toBe(join('/codex', 'skills', 'demo')); + expect(resolveGlobalSkillMarkdownPath('codex', 'demo')).toBe(join('/codex', 'skills', 'demo', 'SKILL.md')); + }); + + it('returns null for cline and renders unsupported display path', () => { + expect(resolveGlobalSkillsBase('cline')).toBeNull(); + expect(renderGlobalSkillsBaseDisplayPath('cline')).toBe('(cline does not use a skills directory)'); + expect(resolveGlobalSkillDir('cline', 'demo')).toBeNull(); + expect(resolveGlobalSkillMarkdownPath('cline', 'demo')).toBeNull(); + expect(renderGlobalSkillDisplayPath('cline', 'demo')).toBe('(cline does not use a skills directory)'); + }); + + it('renders home-relative display paths with ~ for warnings', () => { + expect(renderGlobalSkillsBaseDisplayPath('claude')).toBe('~/.claude/skills'); + expect(renderGlobalSkillDisplayPath('claude', 'demo')).toBe(join('~/.claude/skills', 'demo')); + }); + + it('rejects path-traversal segments — resolveGlobalSkillDir returns null for ../../foo', () => { + process.env.CODEX_HOME = '/codex'; + expect(resolveGlobalSkillDir('codex', '../../foo')).toBeNull(); + expect(resolveGlobalSkillDir('codex', '../escape')).toBeNull(); + expect(resolveGlobalSkillDir('codex', '')).toBeNull(); + // Absolute path as skillName is also rejected + expect(resolveGlobalSkillDir('codex', '/abs/path')).toBeNull(); + // Legitimate name still works + expect(resolveGlobalSkillDir('codex', 'demo')).toBe(join('/codex', 'skills', 'demo')); + // resolveGlobalSkillMarkdownPath must also propagate the null for unsafe inputs + expect(resolveGlobalSkillMarkdownPath('codex', '../../foo')).toBeNull(); + expect(resolveGlobalSkillMarkdownPath('codex', '../escape')).toBeNull(); + expect(resolveGlobalSkillMarkdownPath('codex', '')).toBeNull(); + expect(resolveGlobalSkillMarkdownPath('codex', '/abs/path')).toBeNull(); + }); +}); + describe('findProjectRoot (multi-repo .planning resolution)', () => { let workspace: string; diff --git a/sdk/src/query/helpers.ts b/sdk/src/query/helpers.ts index e82b00041..2aae268a4 100644 --- a/sdk/src/query/helpers.ts +++ b/sdk/src/query/helpers.ts @@ -121,6 +121,59 @@ export function resolveAgentsDir(runtime: Runtime = 'claude'): string { return join(getRuntimeConfigDir(runtime), 'agents'); } +/** + * Resolve the runtime-global skills base directory. + * + * Most runtimes store global skills under `/skills`. + * `cline` is rules-based and has no global skills directory. + */ +export function resolveGlobalSkillsBase(runtime: Runtime): string | null { + if (runtime === 'cline') return null; + return join(getRuntimeConfigDir(runtime), 'skills'); +} + +/** + * Render a human-readable runtime-global skills base path. + * Uses `~` when the path lives under the current home dir. + * Returns a displayable string for unsupported runtimes (never null). + */ +export function renderGlobalSkillsBaseDisplayPath(runtime: Runtime): string { + const base = resolveGlobalSkillsBase(runtime); + if (!base) return `(${runtime} does not use a skills directory)`; + const home = homedir(); + const homeWithSep = home.endsWith(pathSep) ? home : `${home}${pathSep}`; + return (base === home || base.startsWith(homeWithSep)) ? `~${base.slice(home.length)}` : base; +} + +/** Resolve one runtime-global skill directory, or `null` when unsupported. */ +export function resolveGlobalSkillDir(runtime: Runtime, skillName: string): string | null { + const base = resolveGlobalSkillsBase(runtime); + if (base === null) return null; + const candidate = resolve(base, skillName); + const rel = relative(base, candidate); + if (!skillName || rel.startsWith('..') || isAbsolute(rel)) return null; + return candidate; +} + +/** Resolve the canonical SKILL.md path for one runtime-global skill. */ +export function resolveGlobalSkillMarkdownPath(runtime: Runtime, skillName: string): string | null { + const dir = resolveGlobalSkillDir(runtime, skillName); + if (dir === null) return null; + return join(dir, 'SKILL.md'); +} + +/** + * Render a human-readable global skill path for warnings. + * Uses `~` when the path lives under the current home dir. + */ +export function renderGlobalSkillDisplayPath(runtime: Runtime, skillName: string): string { + const dir = resolveGlobalSkillDir(runtime, skillName); + if (!dir) return `(${runtime} does not use a skills directory)`; + const home = homedir(); + const homeWithSep = home.endsWith(pathSep) ? home : `${home}${pathSep}`; + return (dir === home || dir.startsWith(homeWithSep)) ? `~${dir.slice(home.length)}` : dir; +} + // ─── Types ────────────────────────────────────────────────────────────────── /** Paths to common .planning files. */ diff --git a/sdk/src/query/profile-output.ts b/sdk/src/query/profile-output.ts index d12ec3a10..20c7f5719 100644 --- a/sdk/src/query/profile-output.ts +++ b/sdk/src/query/profile-output.ts @@ -13,15 +13,15 @@ import { } from 'node:fs'; import { homedir } from 'node:os'; import { dirname, isAbsolute, join } from 'node:path'; -import { fileURLToPath } from 'node:url'; import { loadConfig } from '../config.js'; import { GSDError, ErrorClassification } from '../errors.js'; -import { detectRuntime } from './helpers.js'; +import { detectRuntime, resolveGlobalSkillMarkdownPath } from './helpers.js'; import { CLAUDE_INSTRUCTIONS } from './profile-questionnaire-data.js'; import type { QueryHandler } from './utils.js'; +import { resolveBundledTemplatesDir } from '../sdk-package-compatibility.js'; -const TEMPLATE_DIR = join(dirname(fileURLToPath(import.meta.url)), '../../../get-shit-done/templates'); +const TEMPLATE_DIR = resolveBundledTemplatesDir(); const DIMENSION_KEYS = [ 'communication_style', @@ -601,7 +601,21 @@ export const generateDevPreferences: QueryHandler = async (args, projectDir) => let outPath = outputPathOpt; if (!outPath) { - outPath = join(homedir(), '.claude', 'commands', 'gsd', 'dev-preferences.md'); + let runtime = detectRuntime(); + try { + const config = await loadConfig(projectDir); + runtime = detectRuntime(config as { runtime?: unknown }); + } catch { + /* default runtime */ + } + const defaultSkillPath = resolveGlobalSkillMarkdownPath(runtime, 'gsd-dev-preferences'); + if (!defaultSkillPath) { + throw new GSDError( + `Runtime "${runtime}" does not use a skills directory; pass --output to choose a path explicitly.`, + ErrorClassification.Validation, + ); + } + outPath = defaultSkillPath; } else if (!isAbsolute(outPath)) { outPath = join(projectDir, outPath); } diff --git a/sdk/src/query/profile.test.ts b/sdk/src/query/profile.test.ts index 26ad076bf..bce33c530 100644 --- a/sdk/src/query/profile.test.ts +++ b/sdk/src/query/profile.test.ts @@ -7,7 +7,7 @@ import { mkdtemp, writeFile, mkdir, rm, readFile } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; -import { writeProfile } from './profile-output.js'; +import { generateDevPreferences, writeProfile } from './profile-output.js'; import { learningsCopy } from './profile.js'; describe('writeProfile', () => { @@ -52,6 +52,68 @@ describe('writeProfile', () => { }); }); +describe('generateDevPreferences', () => { + let tmpDir: string; + + beforeEach(async () => { + tmpDir = await mkdtemp(join(tmpdir(), 'gsd-dev-preferences-')); + await mkdir(join(tmpDir, '.planning'), { recursive: true }); + }); + + afterEach(async () => { + await rm(tmpDir, { recursive: true, force: true }); + }); + + it('writes to runtime-global skills dir by default', async () => { + const analysisPath = join(tmpDir, 'analysis.json'); + const codexHome = join(tmpDir, 'codex-home'); + await writeFile( + analysisPath, + JSON.stringify({ + data_source: 'test', + dimensions: { + communication_style: { rating: 'terse', confidence: 'HIGH', claude_instruction: 'Keep it short.' }, + }, + }), + 'utf-8', + ); + await writeFile(join(tmpDir, '.planning', 'config.json'), JSON.stringify({ runtime: 'codex' }), 'utf-8'); + + const prevCodexHome = process.env.CODEX_HOME; + process.env.CODEX_HOME = codexHome; + try { + const result = await generateDevPreferences(['--analysis', analysisPath], tmpDir); + const data = result.data as Record; + const expectedPath = join(codexHome, 'skills', 'gsd-dev-preferences', 'SKILL.md'); + expect(data.command_path).toBe(expectedPath); + const md = await readFile(expectedPath, 'utf-8'); + expect(md).toContain('Behavioral Directives'); + } finally { + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + } + }); + + it('requires --output when runtime has no skills dir', async () => { + const analysisPath = join(tmpDir, 'analysis.json'); + await writeFile( + analysisPath, + JSON.stringify({ + data_source: 'test', + dimensions: { + communication_style: { rating: 'terse', confidence: 'HIGH', claude_instruction: 'Keep it short.' }, + }, + }), + 'utf-8', + ); + await writeFile(join(tmpDir, '.planning', 'config.json'), JSON.stringify({ runtime: 'cline' }), 'utf-8'); + + await expect(generateDevPreferences(['--analysis', analysisPath], tmpDir)).rejects.toThrow( + 'Runtime "cline" does not use a skills directory; pass --output to choose a path explicitly.', + ); + }); +}); + describe('learningsCopy', () => { let tmpDir: string; diff --git a/sdk/src/query/roadmap.ts b/sdk/src/query/roadmap.ts index 262e32c6b..d3f00a120 100644 --- a/sdk/src/query/roadmap.ts +++ b/sdk/src/query/roadmap.ts @@ -21,6 +21,7 @@ import { existsSync } from 'node:fs'; import { readFile, writeFile, readdir } from 'node:fs/promises'; import { join } from 'node:path'; import { GSDError, ErrorClassification } from '../errors.js'; +import { resolveGsdToolsPath } from '../sdk-package-compatibility.js'; import { escapeRegex, normalizePhaseName, @@ -737,11 +738,7 @@ export const roadmapAnnotateDependencies: QueryHandler = async (args, projectDir } const { spawnSync } = await import('node:child_process'); - const { fileURLToPath } = await import('node:url'); - - const toolsPath = fileURLToPath( - new URL('../../../get-shit-done/bin/gsd-tools.cjs', import.meta.url), - ); + const toolsPath = resolveGsdToolsPath(projectDir); const result = spawnSync(process.execPath, [toolsPath, 'roadmap', 'annotate-dependencies', phase], { cwd: projectDir, diff --git a/sdk/src/query/skill-manifest.test.ts b/sdk/src/query/skill-manifest.test.ts new file mode 100644 index 000000000..7910b05ac --- /dev/null +++ b/sdk/src/query/skill-manifest.test.ts @@ -0,0 +1,62 @@ +import { describe, expect, it } from 'vitest'; +import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir, homedir } from 'node:os'; +import { join } from 'node:path'; + +import { buildSkillManifest } from './skill-manifest.js'; +import { resolveGlobalSkillsBase, renderGlobalSkillsBaseDisplayPath } from './helpers.js'; +import { resolveLegacySkillsDir } from '../sdk-package-compatibility.js'; + +describe('skill-manifest', () => { + it('resolves runtime-global skills roots through shared runtime policy', () => { + const manifest = buildSkillManifest('/tmp/project-that-does-not-exist'); + const claudeRoot = manifest.roots.find(root => root.root === renderGlobalSkillsBaseDisplayPath('claude')); + const codexRoot = manifest.roots.find(root => root.root === renderGlobalSkillsBaseDisplayPath('codex')); + + expect(claudeRoot?.path).toBe(resolveGlobalSkillsBase('claude')); + expect(claudeRoot?.scope).toBe('global'); + expect(codexRoot?.path).toBe(resolveGlobalSkillsBase('codex')); + expect(codexRoot?.scope).toBe('global'); + }); + + it('resolves legacy import-only skills root through SDK Package Seam Module', () => { + const manifest = buildSkillManifest('/tmp/project-that-does-not-exist'); + const legacyRoot = manifest.roots.find(root => root.root === '.claude/get-shit-done/skills'); + + expect(legacyRoot).toBeDefined(); + expect(legacyRoot?.path).toBe(resolveLegacySkillsDir()); + expect(legacyRoot?.scope).toBe('import-only'); + expect(legacyRoot?.deprecated).toBe(true); + }); + + it('discovers skills from explicit --skills-dir without scanning global roots', async () => { + const tmpDir = await mkdtemp(join(tmpdir(), 'gsd-skill-manifest-')); + try { + const skillsDir = join(tmpDir, 'custom-skills'); + const alphaDir = join(skillsDir, 'alpha-skill'); + await mkdir(alphaDir, { recursive: true }); + await writeFile( + join(alphaDir, 'SKILL.md'), + '---\nname: alpha-skill\ndescription: alpha desc\n---\n\nTRIGGER when: need alpha\n', + ); + + const manifest = buildSkillManifest(tmpDir, skillsDir); + expect(manifest.counts.roots).toBe(1); + expect(manifest.roots[0]?.path).toBe(skillsDir); + expect(manifest.skills).toHaveLength(1); + expect(manifest.skills[0]).toMatchObject({ + name: 'alpha-skill', + description: 'alpha desc', + triggers: ['need alpha'], + root: skillsDir, + scope: 'custom', + }); + } finally { + await rm(tmpDir, { recursive: true, force: true }); + } + }); + + it('legacy skills dir helper points at ~/.claude/get-shit-done/skills', () => { + expect(resolveLegacySkillsDir()).toBe(join(homedir(), '.claude', 'get-shit-done', 'skills')); + }); +}); diff --git a/sdk/src/query/skill-manifest.ts b/sdk/src/query/skill-manifest.ts index 5482932a7..fd16b0662 100644 --- a/sdk/src/query/skill-manifest.ts +++ b/sdk/src/query/skill-manifest.ts @@ -12,7 +12,9 @@ import { join, resolve } from 'node:path'; import { homedir } from 'node:os'; import { extractFrontmatterLeading } from './frontmatter.js'; +import { resolveGlobalSkillsBase, renderGlobalSkillsBaseDisplayPath } from './helpers.js'; import type { QueryHandler } from './utils.js'; +import { resolveLegacySkillsDir } from '../sdk-package-compatibility.js'; export interface SkillManifestSkill { name: string; @@ -64,11 +66,11 @@ export function buildSkillManifest(cwd: string, skillsDir: string | null = null) { root: '.cursor/skills', path: join(cwd, '.cursor', 'skills'), scope: 'project', kind: 'skills' as const }, { root: '.github/skills', path: join(cwd, '.github', 'skills'), scope: 'project', kind: 'skills' as const }, { root: '.codex/skills', path: join(cwd, '.codex', 'skills'), scope: 'project', kind: 'skills' as const }, - { root: '~/.claude/skills', path: join(homedir(), '.claude', 'skills'), scope: 'global', kind: 'skills' as const }, - { root: '~/.codex/skills', path: join(homedir(), '.codex', 'skills'), scope: 'global', kind: 'skills' as const }, + { root: renderGlobalSkillsBaseDisplayPath('claude'), path: resolveGlobalSkillsBase('claude')!, scope: 'global', kind: 'skills' as const }, + { root: renderGlobalSkillsBaseDisplayPath('codex'), path: resolveGlobalSkillsBase('codex')!, scope: 'global', kind: 'skills' as const }, { root: '.claude/get-shit-done/skills', - path: join(homedir(), '.claude', 'get-shit-done', 'skills'), + path: resolveLegacySkillsDir(), scope: 'import-only', kind: 'skills' as const, deprecated: true, diff --git a/sdk/src/query/skills.test.ts b/sdk/src/query/skills.test.ts index d9f0d7b51..82de9e4d7 100644 --- a/sdk/src/query/skills.test.ts +++ b/sdk/src/query/skills.test.ts @@ -135,6 +135,42 @@ describe('agentSkills', () => { expect(r.format).toBe('text'); }); + it('resolves global: skills through runtime-aware global skills dir policy', async () => { + const codexHome = join(tmpDir, 'codex-home'); + await writeSkill(join(codexHome, 'skills'), 'global-skill'); + await writeConfig(tmpDir, { + runtime: 'codex', + agent_skills: { 'gsd-planner': ['global:global-skill'] }, + }); + + const prevCodexHome = process.env.CODEX_HOME; + process.env.CODEX_HOME = codexHome; + try { + const r = await agentSkills(['gsd-planner'], tmpDir); + expect(r.data).toBe( + '\n' + + 'Read these user-configured skills:\n' + + `- @${join(codexHome, 'skills', 'global-skill', 'SKILL.md')}\n` + + '', + ); + expect(r.format).toBe('text'); + } finally { + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + } + }); + + it('returns empty string for global: skills on runtimes without a skills dir', async () => { + await writeConfig(tmpDir, { + runtime: 'cline', + agent_skills: { 'gsd-planner': ['global:nope'] }, + }); + + const r = await agentSkills(['gsd-planner'], tmpDir); + expect(r.data).toBe(''); + expect(r.format).toBeUndefined(); + }); + it('does not signal format:"text" for empty result', async () => { const r = await agentSkills(['gsd-planner'], tmpDir); expect(r.format).toBeUndefined(); diff --git a/sdk/src/query/skills.ts b/sdk/src/query/skills.ts index 43168b0c3..0c1fdceba 100644 --- a/sdk/src/query/skills.ts +++ b/sdk/src/query/skills.ts @@ -5,7 +5,9 @@ * Ports `buildAgentSkillsBlock` semantics from * `get-shit-done/bin/lib/init.cjs` so the SDK path honors * `config.agent_skills[agentType]` the same way the legacy - * `gsd-tools.cjs agent-skills ` path does. Fixes #2555. + * `gsd-tools.cjs agent-skills ` path does. Project-relative skills stay + * project-root validated; `global:` now resolves through runtime-aware + * global skills dir policy rather than a Claude-only hardcoded path. Fixes #2555. * * @example * ```typescript @@ -23,9 +25,9 @@ import { existsSync, realpathSync } from 'node:fs'; import { join, resolve, sep } from 'node:path'; -import { homedir } from 'node:os'; import type { QueryHandler } from './utils.js'; +import { detectRuntime, renderGlobalSkillDisplayPath, resolveGlobalSkillDir, resolveGlobalSkillsBase } from './helpers.js'; import { loadConfig } from '../config.js'; const GLOBAL_SKILL_NAME_RE = /^[a-zA-Z0-9_-]+$/; @@ -76,13 +78,14 @@ export const agentSkills: QueryHandler = async (args, projectDir) => { } if (skillPaths.length === 0) return { data: '' }; - const globalSkillsBase = join(homedir(), '.claude', 'skills'); + const runtime = detectRuntime(config as { runtime?: unknown }); + const globalSkillsBase = resolveGlobalSkillsBase(runtime); const validEntries: Array<{ ref: string }> = []; for (const entry of skillPaths) { if (typeof entry !== 'string') continue; - // `global:` — skill installed under ~/.claude/skills// (#1992). + // `global:` — skill installed under the runtime-global skills dir (#1992, #3126). if (entry.startsWith('global:')) { const skillName = entry.slice(7); if (!skillName) { @@ -93,17 +96,26 @@ export const agentSkills: QueryHandler = async (args, projectDir) => { process.stderr.write(`[agent-skills] WARNING: Invalid global skill name "${skillName}" — skipping\n`); continue; } - const skillDir = join(globalSkillsBase, skillName); + if (globalSkillsBase === null) { + process.stderr.write(`[agent-skills] WARNING: Runtime "${runtime}" does not use a skills directory — "global:${skillName}" is not supported on this runtime\n`); + continue; + } + const skillDir = resolveGlobalSkillDir(runtime, skillName); + const displayPath = renderGlobalSkillDisplayPath(runtime, skillName); + if (!skillDir) { + process.stderr.write(`[agent-skills] WARNING: Could not resolve global skill directory for "${skillName}" on runtime "${runtime}" — skipping\n`); + continue; + } const skillMd = join(skillDir, 'SKILL.md'); if (!existsSync(skillMd)) { - process.stderr.write(`[agent-skills] WARNING: Global skill not found at "~/.claude/skills/${skillName}/SKILL.md" — skipping\n`); + process.stderr.write(`[agent-skills] WARNING: Global skill not found at "${displayPath}/SKILL.md" — skipping\n`); continue; } if (resolveWithinBase(skillMd, globalSkillsBase) === null) { process.stderr.write(`[agent-skills] WARNING: Global skill "${skillName}" failed path check (symlink escape?) — skipping\n`); continue; } - validEntries.push({ ref: `~/.claude/skills/${skillName}/SKILL.md` }); + validEntries.push({ ref: skillMd }); continue; } diff --git a/sdk/src/query/state-project-load.ts b/sdk/src/query/state-project-load.ts index 1ec29f7b7..f059c63d2 100644 --- a/sdk/src/query/state-project-load.ts +++ b/sdk/src/query/state-project-load.ts @@ -13,38 +13,9 @@ import { readFile } from 'node:fs/promises'; import { existsSync } from 'node:fs'; import { join } from 'node:path'; -import { homedir } from 'node:os'; -import { createRequire } from 'node:module'; -import { fileURLToPath } from 'node:url'; import { planningPaths } from './helpers.js'; import type { QueryHandler } from './utils.js'; -import { GSDError, ErrorClassification } from '../errors.js'; - -const BUNDLED_CORE_CJS = fileURLToPath( - new URL('../../../get-shit-done/bin/lib/core.cjs', import.meta.url), -); - -function resolveCoreCjsPath(projectDir: string): string | null { - const candidates = [ - BUNDLED_CORE_CJS, - join(projectDir, '.claude', 'get-shit-done', 'bin', 'lib', 'core.cjs'), - join(homedir(), '.claude', 'get-shit-done', 'bin', 'lib', 'core.cjs'), - ]; - return candidates.find(p => existsSync(p)) ?? null; -} - -function loadConfigCjs(projectDir: string): Record { - const corePath = resolveCoreCjsPath(projectDir); - if (!corePath) { - throw new GSDError( - 'state load: get-shit-done/bin/lib/core.cjs not found. Install GSD (e.g. npm i -g get-shit-done-cc) or clone with get-shit-done next to the SDK.', - ErrorClassification.Blocked, - ); - } - const req = createRequire(import.meta.url); - const { loadConfig } = req(corePath) as { loadConfig: (cwd: string) => Record }; - return loadConfig(projectDir); -} +import { loadLegacyCoreConfig } from '../sdk-package-compatibility.js'; /** * Query handler for `state load` / bare `state` (normalize → `state.load`). @@ -52,7 +23,7 @@ function loadConfigCjs(projectDir: string): Record { * Port of `cmdStateLoad` from `get-shit-done/bin/lib/state.cjs` lines 44–86. */ export const stateProjectLoad: QueryHandler = async (_args, projectDir, workstream) => { - const config = loadConfigCjs(projectDir); + const config = loadLegacyCoreConfig(projectDir); const planDir = planningPaths(projectDir, workstream).planning; let stateRaw = ''; diff --git a/sdk/src/query/validate.ts b/sdk/src/query/validate.ts index 65eef19af..c1c87d303 100644 --- a/sdk/src/query/validate.ts +++ b/sdk/src/query/validate.ts @@ -17,7 +17,6 @@ import { readFile, readdir, writeFile } from 'node:fs/promises'; import { existsSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; import { homedir } from 'node:os'; import { MODEL_PROFILES } from './config-query.js'; @@ -25,6 +24,7 @@ import { GSDError, ErrorClassification } from '../errors.js'; import { extractFrontmatter, parseMustHavesBlock } from './frontmatter.js'; import { escapeRegex, normalizePhaseName, planningPaths, resolvePathUnderProject } from './helpers.js'; import type { QueryHandler } from './utils.js'; +import { resolveBundledAgentsDir } from '../sdk-package-compatibility.js'; /** Max length for key_links regex patterns (ReDoS mitigation). */ const MAX_KEY_LINK_PATTERN_LEN = 512; @@ -790,8 +790,7 @@ export const validateHealth: QueryHandler = async (args, projectDir, workstream) */ function getAgentsDirForValidateAgents(): string { if (process.env.GSD_AGENTS_DIR) return process.env.GSD_AGENTS_DIR; - const here = dirname(fileURLToPath(import.meta.url)); - return resolve(here, '..', '..', '..', 'agents'); + return resolveBundledAgentsDir(); } /** diff --git a/sdk/src/query/verify.ts b/sdk/src/query/verify.ts index 8875d7f56..9eb55c945 100644 --- a/sdk/src/query/verify.ts +++ b/sdk/src/query/verify.ts @@ -26,6 +26,7 @@ import { planningPaths, } from './helpers.js'; import type { QueryHandler } from './utils.js'; +import { resolveGsdToolsPath } from '../sdk-package-compatibility.js'; // ─── verifyPlanStructure ─────────────────────────────────────────────────── @@ -658,14 +659,7 @@ export const verifySchemaDrift: QueryHandler = async (args, projectDir, workstre export const verifyCodebaseDrift: QueryHandler = async (_args, projectDir) => { try { const { execFileSync } = await import('node:child_process'); - const { fileURLToPath } = await import('node:url'); - const { dirname, resolve } = await import('node:path'); - const here = typeof __dirname === 'string' - ? __dirname - : dirname(fileURLToPath(import.meta.url)); - // sdk/src/query -> ../../../get-shit-done/bin/gsd-tools.cjs - // sdk/dist/query -> ../../../get-shit-done/bin/gsd-tools.cjs - const toolsPath = resolve(here, '..', '..', '..', 'get-shit-done', 'bin', 'gsd-tools.cjs'); + const toolsPath = resolveGsdToolsPath(projectDir); const out = execFileSync(process.execPath, [toolsPath, 'verify', 'codebase-drift'], { cwd: projectDir, encoding: 'utf-8', diff --git a/sdk/src/sdk-package-compatibility.test.ts b/sdk/src/sdk-package-compatibility.test.ts new file mode 100644 index 000000000..f8ad68460 --- /dev/null +++ b/sdk/src/sdk-package-compatibility.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it, vi } from 'vitest'; +import { join } from 'node:path'; + +import { + BUNDLED_CORE_CJS_PATH, + BUNDLED_GSD_AGENTS_DIR, + BUNDLED_GSD_TEMPLATES_DIR, + BUNDLED_GSD_TOOLS_PATH, + loadLegacyCoreConfig, + probeLegacySdkAsset, + resolveBundledAgentsDir, + resolveBundledTemplatesDir, + resolveGsdToolsPath, + resolveLegacyInstallDir, + resolveLegacyTemplatesDir, + resolveLegacyWorkflowsDir, +} from './sdk-package-compatibility.js'; +import { GSDError } from './errors.js'; + +describe('SDK Package Seam Module', () => { + const projectDir = '/work/project'; + const homeDir = '/users/tester'; + + it('resolves legacy install-relative directories through one seam', () => { + expect(resolveLegacyInstallDir(homeDir)).toBe(join(homeDir, '.claude', 'get-shit-done')); + expect(resolveLegacyTemplatesDir(homeDir)).toBe(join(homeDir, '.claude', 'get-shit-done', 'templates')); + expect(resolveLegacyWorkflowsDir(homeDir)).toBe(join(homeDir, '.claude', 'get-shit-done', 'workflows')); + expect(resolveBundledTemplatesDir()).toBe(BUNDLED_GSD_TEMPLATES_DIR); + expect(resolveBundledAgentsDir()).toBe(BUNDLED_GSD_AGENTS_DIR); + }); + + it('probes legacy gsd-tools locations in bundled -> project -> home order', () => { + const resolution = probeLegacySdkAsset('gsd-tools', projectDir, { + homeDir, + existsSync: path => path === join(projectDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), + }); + + expect(resolution.probes).toEqual([ + BUNDLED_GSD_TOOLS_PATH, + join(projectDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), + join(homeDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs'), + ]); + expect(resolution.path).toBe(join(projectDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs')); + expect(resolution.fallbackPath).toBe(join(homeDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs')); + }); + + it('returns concrete fallback gsd-tools path when no legacy probe exists', () => { + const path = resolveGsdToolsPath(projectDir, { + homeDir, + existsSync: () => false, + }); + + expect(path).toBe(join(homeDir, '.claude', 'get-shit-done', 'bin', 'gsd-tools.cjs')); + }); + + it('loads legacy core.cjs through one compatibility adapter', () => { + const loadConfig = vi.fn((cwd: string) => ({ cwd, source: 'legacy-core' })); + const requireFn = vi.fn(() => ({ loadConfig })); + const createRequire = vi.fn(() => requireFn as unknown as NodeJS.Require); + + const result = loadLegacyCoreConfig(projectDir, { + homeDir, + existsSync: path => path === BUNDLED_CORE_CJS_PATH, + createRequire, + }); + + expect(createRequire).toHaveBeenCalledOnce(); + expect(requireFn).toHaveBeenCalledWith(BUNDLED_CORE_CJS_PATH); + expect(loadConfig).toHaveBeenCalledWith(projectDir); + expect(result).toEqual({ cwd: projectDir, source: 'legacy-core' }); + }); + + it('reports checked core.cjs probes when legacy asset missing', () => { + expect(() => loadLegacyCoreConfig(projectDir, { + homeDir, + existsSync: () => false, + })).toThrow(GSDError); + + try { + loadLegacyCoreConfig(projectDir, { + homeDir, + existsSync: () => false, + }); + expect.fail('expected GSDError'); + } catch (error) { + expect(error).toBeInstanceOf(GSDError); + const message = (error as Error).message; + expect(message).toContain('state load: get-shit-done/bin/lib/core.cjs not found.'); + expect(message).toContain('Checked:'); + expect(message).toContain(BUNDLED_CORE_CJS_PATH); + expect(message).toContain(join(projectDir, '.claude', 'get-shit-done', 'bin', 'lib', 'core.cjs')); + expect(message).toContain(join(homeDir, '.claude', 'get-shit-done', 'bin', 'lib', 'core.cjs')); + } + }); +}); diff --git a/sdk/src/sdk-package-compatibility.ts b/sdk/src/sdk-package-compatibility.ts new file mode 100644 index 000000000..b72ece392 --- /dev/null +++ b/sdk/src/sdk-package-compatibility.ts @@ -0,0 +1,137 @@ +import { existsSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { GSDError, ErrorClassification } from './errors.js'; + +export type LegacySdkAsset = 'gsd-tools' | 'core-cjs'; + +export interface LegacySdkAssetResolution { + asset: LegacySdkAsset; + path: string | null; + fallbackPath: string; + probes: string[]; +} + +interface LegacySdkCompatibilityDeps { + existsSync?: (path: string) => boolean; + homeDir?: string; + createRequire?: typeof createRequire; +} + +export const BUNDLED_GSD_TOOLS_PATH = fileURLToPath( + new URL('../../get-shit-done/bin/gsd-tools.cjs', import.meta.url), +); + +export const BUNDLED_CORE_CJS_PATH = fileURLToPath( + new URL('../../get-shit-done/bin/lib/core.cjs', import.meta.url), +); + +export const BUNDLED_GSD_TEMPLATES_DIR = fileURLToPath( + new URL('../../get-shit-done/templates', import.meta.url), +); + +export const BUNDLED_GSD_AGENTS_DIR = fileURLToPath( + new URL('../../agents', import.meta.url), +); + +const LEGACY_ASSET_SUBPATH: Record = { + 'gsd-tools': 'gsd-tools.cjs', + 'core-cjs': join('lib', 'core.cjs'), +}; + +const BUNDLED_LEGACY_ASSET_PATH: Record = { + 'gsd-tools': BUNDLED_GSD_TOOLS_PATH, + 'core-cjs': BUNDLED_CORE_CJS_PATH, +}; + +export function resolveLegacyInstallDir(homeDir: string = homedir()): string { + return join(homeDir, '.claude', 'get-shit-done'); +} + +export function resolveLegacyTemplatesDir(homeDir: string = homedir()): string { + return join(resolveLegacyInstallDir(homeDir), 'templates'); +} + +export function resolveLegacyWorkflowsDir(homeDir: string = homedir()): string { + return join(resolveLegacyInstallDir(homeDir), 'workflows'); +} + +export function resolveLegacySkillsDir(homeDir: string = homedir()): string { + return join(resolveLegacyInstallDir(homeDir), 'skills'); +} + +export function resolveBundledTemplatesDir(): string { + return BUNDLED_GSD_TEMPLATES_DIR; +} + +export function resolveBundledAgentsDir(): string { + return BUNDLED_GSD_AGENTS_DIR; +} + +function legacyAssetProbes(asset: LegacySdkAsset, projectDir: string, homeDir: string): string[] { + const suffix = LEGACY_ASSET_SUBPATH[asset]; + return [ + BUNDLED_LEGACY_ASSET_PATH[asset], + join(projectDir, '.claude', 'get-shit-done', 'bin', suffix), + join(homeDir, '.claude', 'get-shit-done', 'bin', suffix), + ]; +} + +export function probeLegacySdkAsset( + asset: LegacySdkAsset, + projectDir: string, + deps: LegacySdkCompatibilityDeps = {}, +): LegacySdkAssetResolution { + const pathExists = deps.existsSync ?? existsSync; + const probes = legacyAssetProbes(asset, projectDir, deps.homeDir ?? homedir()); + return { + asset, + path: probes.find(candidate => pathExists(candidate)) ?? null, + fallbackPath: probes[probes.length - 1]!, + probes, + }; +} + +/** + * Resolve the legacy `gsd-tools.cjs` executable path through the SDK Package Seam Module. + * + * Preserves historical behavior: if no probe exists, return the final fallback path so + * downstream subprocess errors still show a concrete location. + */ +export function resolveGsdToolsPath(projectDir: string, deps: LegacySdkCompatibilityDeps = {}): string { + const resolution = probeLegacySdkAsset('gsd-tools', projectDir, deps); + return resolution.path ?? resolution.fallbackPath; +} + +function missingLegacyCoreMessage(resolution: LegacySdkAssetResolution): string { + return [ + 'state load: get-shit-done/bin/lib/core.cjs not found.', + `Checked: ${resolution.probes.join(', ')}`, + 'Install GSD (e.g. npm i -g get-shit-done-cc) or clone with get-shit-done next to the SDK.', + ].join(' '); +} + +/** + * Load `loadConfig(cwd)` from the legacy CJS install through one compatibility seam. + */ +export function loadLegacyCoreConfig(projectDir: string, deps: LegacySdkCompatibilityDeps = {}): Record { + const resolution = probeLegacySdkAsset('core-cjs', projectDir, deps); + if (!resolution.path) { + throw new GSDError( + missingLegacyCoreMessage(resolution), + ErrorClassification.Blocked, + ); + } + + const req = (deps.createRequire ?? createRequire)(import.meta.url); + const mod = req(resolution.path) as Partial<{ loadConfig: (cwd: string) => Record }>; + if (typeof mod.loadConfig !== 'function') { + throw new GSDError( + `state load: invalid core.cjs at ${resolution.path} (missing loadConfig(cwd)).`, + ErrorClassification.Blocked, + ); + } + return mod.loadConfig(projectDir); +} diff --git a/sdk/src/types.ts b/sdk/src/types.ts index f0958e648..24f14bbd2 100644 --- a/sdk/src/types.ts +++ b/sdk/src/types.ts @@ -197,7 +197,7 @@ export interface PlanResult { export interface GSDOptions { /** Root directory of the project. */ projectDir: string; - /** Path to gsd-tools.cjs. Falls back to /.claude/, then the bundled repo path, then ~/.claude/. */ + /** Path to gsd-tools.cjs. Falls back to the bundled repo path, then /.claude/, then ~/.claude/. */ gsdToolsPath?: string; /** * Optional session correlation id for query mutation events when using {@link GSD.createTools}. diff --git a/tests/profile-output.test.cjs b/tests/profile-output.test.cjs index 78e4f6739..edb1bc395 100644 --- a/tests/profile-output.test.cjs +++ b/tests/profile-output.test.cjs @@ -211,4 +211,45 @@ describe('generate-dev-preferences command', () => { const out = JSON.parse(result.output); assert.ok(out.command_path || out.command_name, 'should return command output'); }); + + test('uses runtime-aware skills dir for codex by default', () => { + const analysis = { + profile_version: '1.0', + dimensions: { + communication_style: { rating: 'terse-direct', confidence: 'HIGH' }, + }, + }; + const analysisPath = path.join(tmpDir, 'analysis.json'); + const codexHome = path.join(tmpDir, 'codex-home'); + fs.writeFileSync(analysisPath, JSON.stringify(analysis)); + + const result = runGsdTools( + ['generate-dev-preferences', '--analysis', analysisPath, '--raw'], + tmpDir, + { CODEX_HOME: codexHome, GSD_RUNTIME: 'codex' } + ); + assert.ok(result.success, `Failed: ${result.error}`); + const out = JSON.parse(result.output); + assert.strictEqual(out.command_path, path.join(codexHome, 'skills', 'gsd-dev-preferences', 'SKILL.md')); + assert.ok(fs.existsSync(out.command_path), 'runtime-aware output should be written'); + }); + + test('errors for cline unless --output is supplied', () => { + const analysis = { + profile_version: '1.0', + dimensions: { + communication_style: { rating: 'terse-direct', confidence: 'HIGH' }, + }, + }; + const analysisPath = path.join(tmpDir, 'analysis.json'); + fs.writeFileSync(analysisPath, JSON.stringify(analysis)); + + const result = runGsdTools( + ['generate-dev-preferences', '--analysis', analysisPath, '--raw'], + tmpDir, + { GSD_RUNTIME: 'cline' } + ); + assert.ok(!result.success, 'cline should require explicit --output'); + assert.ok(result.error.includes('does not use a skills directory'), 'should explain unsupported runtime'); + }); }); diff --git a/tests/skill-manifest.test.cjs b/tests/skill-manifest.test.cjs index 40844c439..1eca832a3 100644 --- a/tests/skill-manifest.test.cjs +++ b/tests/skill-manifest.test.cjs @@ -127,4 +127,21 @@ describe('skill-manifest', () => { assert.ok(Array.isArray(manifest.skills)); assert.ok(manifest.installation); }); + + test('global roots honor runtime-home env overrides instead of hardcoded home paths', () => { + const result = runGsdTools(['skill-manifest'], tmpDir, { + HOME: homeDir, + CLAUDE_CONFIG_DIR: path.join(homeDir, 'claude-custom'), + CODEX_HOME: path.join(homeDir, 'codex-custom'), + }); + assert.ok(result.success, `Command should succeed: ${result.error || result.output}`); + + const manifest = JSON.parse(result.output); + const claudeRoot = manifest.roots.find((root) => root.root === '~/.claude/skills'); + const codexRoot = manifest.roots.find((root) => root.root === '~/.codex/skills'); + assert.ok(claudeRoot, 'Expected ~/.claude/skills root to be present'); + assert.ok(codexRoot, 'Expected ~/.codex/skills root to be present'); + assert.strictEqual(claudeRoot.path, path.join(homeDir, 'claude-custom', 'skills')); + assert.strictEqual(codexRoot.path, path.join(homeDir, 'codex-custom', 'skills')); + }); });