fix(#1383): resolve GSD version without a top-level require of the runtime-root package.json (#1409)
* fix(#1383): resolve GSD version without a top-level require of the runtime-root package.json The extracted runtime-artifact-conversion module sits in the gsd-tools loader chain and did a module-load `require('../../../package.json')`. On Codex (whose runtime root has no package.json) that threw `Cannot find module '../../../package.json'`, crashing every gsd-tools command before it did anything. Even on Claude the synthetic `{"type":"commonjs"}` has no `version`, so the sole consumer already emitted `version: undefined`. Resolve the version lazily and defensively instead: read the installed gsd-core/VERSION, else lazily require the runtime-root package.json, else degrade to '' so the caller omits the field. Both sources are validated against the repo's semver-prefix convention (mirrors update-context.cts) so a garbled VERSION is never emitted verbatim. install.js's dead duplicate converter is intentionally left untouched (scoped to the crash). Adds a #1383 regression block exercising resolveVersionFrom across VERSION-only / package.json-only / neither / malformed-VERSION layouts, asserting no-throw and the correct version string. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#1383): add changeset for the Codex gsd-tools crash fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(#1383): record resolveVersionFrom export in CONTEXT.md glossary Maintainer review gate on PR #1409: the lazy resolveVersionFrom seam added on the Runtime Artifact Conversion Module must be recorded in CONTEXT.md so the canonical glossary doesn't drift from the exported surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#1383): reword changeset to drop product-name parenthetical product-name-purity (#1777) rejects 'Codex (…)' parentheticals that render verbatim into CHANGELOG.md. Reword to a comma clause; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
committed by
GitHub
parent
dee40cd392
commit
0224f5bcf3
5
.changeset/sturdy-birds-climb.md
Normal file
5
.changeset/sturdy-birds-climb.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1409
|
||||
---
|
||||
**Codex runtime no longer crashes on startup** — every `gsd-tools` command previously aborted with `Cannot find module '../../../package.json'` on Codex, whose runtime root has no `package.json`, because a module in the loader chain did a top-level require of it. The version emitted into Hermes skill frontmatter is now sourced lazily from the installed `gsd-core/VERSION` (validated semver), so `gsd-tools` loads on every runtime and never emits `version: undefined`. (#1383)
|
||||
@@ -155,7 +155,7 @@ Module owning which skills and agents are written to runtime config directories
|
||||
Module owning the per-runtime mapping from artifact kind to filesystem placement. ADR-3660 defines the typed `kinds` per runtime (`commands`, `agents`, `skills`) with destination subpath, prefix, and stage adapter (with per-runtime converters in `bin/install.js`: `convertClaudeCommandToClaudeSkill`, `…CodexSkill`, `…CopilotSkill`, `…AntigravitySkill`). Owns the per-runtime `nested` skill-bundle decision (#69): a `skillsKind` flag in `src/runtime-artifact-layout.cts` drives whether a runtime receives the nested router layout (6 `gsd-ns-*` routers + concrete skills under `<router>/skills/<name>/`) or the flat `skills/gsd-<stem>/` layout; the evidence/doc-link matrix is recorded in a comment above `resolveRuntimeArtifactLayout`. Phase 1 applies this seam to the Runtime Surface Module (`surface.cjs:applySurface`); as of #813, `applySurface` applies the same per-runtime skill-body path rewrites as `installRuntimeArtifacts` for `skills` kinds — re-surfacing no longer overwrites installed SKILL.md bodies with converter-default `~/.claude` paths. Per ADR-1508 / #1511 the former `getInstallExports`/`loadInstallExports` relay (a `GSD_TEST_MODE`-guarded `require('bin/install.js')` by which `surface.cjs` reached `computePathPrefix`/`applyRuntimeContentRewritesInPlace`) was DELETED from this module; content rewriting now lives in the Runtime Artifact Conversion Module and `surface.cjs:applySurface` calls its `rewriteStagedSkillBodies` directly. The resolved `scope` is still carried on the `Layout` object so `applySurface` derives the same `pathPrefix` (global `$HOME` form vs. absolute) as a fresh install. Phase 2 is planned to migrate install/uninstall in `bin/install.js` so all lifecycle sites iterate one shared layout table instead of re-encoding runtime layout logic. This design is intended to remove the #3659 class of omissions. Migrations remain under the Installer Migration Module (ADR-0008). See ADR-3660.
|
||||
|
||||
### Runtime Artifact Conversion Module
|
||||
Sibling Module to Runtime Artifact Layout Module. Owns projection from canonical Claude-authored command/agent/skill markdown into runtime-specific artifact bodies, including converter selection, frontmatter/body normalization, runtime path rewrites, and staged artifact generation. Runtime Artifact Layout remains responsible for filesystem placement (`kind`, destination subpath, prefix, nesting); Runtime Artifact Conversion owns the content Implementation behind that placement seam so install, uninstall/surface parity, and future plugin/package projections stop reaching back through `bin/install.js` for converter functions or `GSD_TEST_MODE`-guarded installer exports. Chosen direction: sibling Module, not an expanded Layout Module, to preserve ADR-3660's narrow placement responsibility while deepening artifact content locality. First slice: relocate only the layout-reached conversion family (`convertClaudeCommandTo*Skill`, converted command-file emitters, `buildKimiAgentArtifacts`) plus the minimal helper closure they need; do not leave helper dependencies in `bin/install.js` because that would preserve the same shallow seam under a new filename. Installer integration decision: `bin/install.js` imports the conversion Module at top level and re-exports the moved names for compatibility; the conversion Module must not import `bin/install.js` or Runtime Artifact Layout, so the dependency direction becomes installer/layout Adapters -> conversion Module, never conversion -> installer. First-slice Interface decision: export the existing compatibility names only; do not introduce a grouped `convertRuntimeArtifact` Interface until after relocation proves byte-for-byte behavior. SHIPPED (ADR-1508): the converter family relocated in #1510 Phase 1 (`getDirName`→runtime-name-policy, `processAttribution` here); #1511 Phase 2 moved the content-rewrite engine here in full — `_applyRuntimeRewrites` (per-runtime switch, injected attribution), the staged-content walkers `applyRuntimeContentRewritesInPlace`/`applyRuntimeContentRewritesForCommandsInPlace`, `computePathPrefix` (private; `_computePathPrefix` for tests), and the deep public seam `rewriteStagedSkillBodies`/`rewriteStagedCommandBodies({runtime,configDir,scope,homedir?,platform?,resolveAttribution?})`. `bin/install.js` binds these back (single owner, exports preserved); `getCommitAttribution` stays in `bin/install.js` (impure install-time config I/O) and is injected. The `getInstallExports` relay in Runtime Artifact Layout Module was deleted; the dependency direction installer/layout → conversion (never upward) is now enforced. Exception: opencode and kilo path-prefix rewriting is a deliberate `bin/install.js`-owned pre-conversion step (`applyOpencodeFamilyPathPrefix`) per #784, not a violation of the single-owner rule. Source: `gsd-core/bin/lib/runtime-artifact-conversion.cjs` (generated from `src/runtime-artifact-conversion.cts`).
|
||||
Sibling Module to Runtime Artifact Layout Module. Owns projection from canonical Claude-authored command/agent/skill markdown into runtime-specific artifact bodies, including converter selection, frontmatter/body normalization, runtime path rewrites, and staged artifact generation. Runtime Artifact Layout remains responsible for filesystem placement (`kind`, destination subpath, prefix, nesting); Runtime Artifact Conversion owns the content Implementation behind that placement seam so install, uninstall/surface parity, and future plugin/package projections stop reaching back through `bin/install.js` for converter functions or `GSD_TEST_MODE`-guarded installer exports. Chosen direction: sibling Module, not an expanded Layout Module, to preserve ADR-3660's narrow placement responsibility while deepening artifact content locality. First slice: relocate only the layout-reached conversion family (`convertClaudeCommandTo*Skill`, converted command-file emitters, `buildKimiAgentArtifacts`) plus the minimal helper closure they need; do not leave helper dependencies in `bin/install.js` because that would preserve the same shallow seam under a new filename. Installer integration decision: `bin/install.js` imports the conversion Module at top level and re-exports the moved names for compatibility; the conversion Module must not import `bin/install.js` or Runtime Artifact Layout, so the dependency direction becomes installer/layout Adapters -> conversion Module, never conversion -> installer. First-slice Interface decision: export the existing compatibility names only; do not introduce a grouped `convertRuntimeArtifact` Interface until after relocation proves byte-for-byte behavior. SHIPPED (ADR-1508): the converter family relocated in #1510 Phase 1 (`getDirName`→runtime-name-policy, `processAttribution` here); #1511 Phase 2 moved the content-rewrite engine here in full — `_applyRuntimeRewrites` (per-runtime switch, injected attribution), the staged-content walkers `applyRuntimeContentRewritesInPlace`/`applyRuntimeContentRewritesForCommandsInPlace`, `computePathPrefix` (private; `_computePathPrefix` for tests), and the deep public seam `rewriteStagedSkillBodies`/`rewriteStagedCommandBodies({runtime,configDir,scope,homedir?,platform?,resolveAttribution?})`. `bin/install.js` binds these back (single owner, exports preserved); `getCommitAttribution` stays in `bin/install.js` (impure install-time config I/O) and is injected. The `getInstallExports` relay in Runtime Artifact Layout Module was deleted; the dependency direction installer/layout → conversion (never upward) is now enforced. Exception: opencode and kilo path-prefix rewriting is a deliberate `bin/install.js`-owned pre-conversion step (`applyOpencodeFamilyPathPrefix`) per #784, not a violation of the single-owner rule. Source: `gsd-core/bin/lib/runtime-artifact-conversion.cjs` (generated from `src/runtime-artifact-conversion.cts`). Also exports `resolveVersionFrom(libDir)` — a lazy, defensive GSD-version resolver (installed-tree `gsd-core/VERSION` first, then the source/npm `package.json` three dirs up, both validated against the repo's shared semver-prefix shape, degrading to `''` on failure) that replaced a module-load-time `require('../../../package.json')` which crashed on runtimes whose root carries no `package.json` (e.g. Codex) (#1383).
|
||||
|
||||
### Runtime Artifact Install Plan Module
|
||||
Module owning install-time staging and content-rewrite selection for a pre-resolved Runtime Artifact Layout. Interface: `createRuntimeArtifactInstallPlan({ layout, resolvedProfile, homedir?, platform?, resolveAttribution?, deps? }) -> { ok:true, plan:{ items, cleanupDirs } } | { ok:false, kind:'stage_failed'|'rewrite_failed', message, cleanupDirs, failedKind? }`. It iterates `layout.kinds` in order, calls each kind's `stage(resolvedProfile)`, delegates `commands` to Runtime Artifact Conversion `rewriteStagedCommandBodies`, delegates `skills` and `kimi-agents` to `rewriteStagedSkillBodies`, leaves non-rewritten kinds unchanged, and projects copy items as `{ kind, sourceDir, destDir }`. It deliberately does not prune, copy, run legacy migrations, print output, or execute cleanup; those remain Installer Module adapter responsibilities until later slices wire the plan into `bin/install.js`. Source: `gsd-core/bin/lib/runtime-artifact-install-plan.cjs` (generated from `src/runtime-artifact-install-plan.cts`). See Runtime Artifact Layout Module and Runtime Artifact Conversion Module.
|
||||
|
||||
@@ -21,10 +21,46 @@ import os from 'node:os';
|
||||
import fs from 'node:fs';
|
||||
import commandRoster = require('./command-roster.cjs');
|
||||
const { readGsdCommandNames, transformContentToHyphen } = commandRoster;
|
||||
const pkg = require('../../../package.json');
|
||||
import runtimeNamePolicy = require('./runtime-name-policy.cjs');
|
||||
const { getDirName } = runtimeNamePolicy;
|
||||
|
||||
// #1383: resolve GSD's version WITHOUT a top-level
|
||||
// `require('../../../package.json')`. That require ran at module load on every
|
||||
// gsd-tools invocation (this module sits in the gsd-tools loader chain) and
|
||||
// threw `Cannot find module '../../../package.json'` on runtimes whose root has
|
||||
// no package.json — notably Codex, where the installer omits the synthetic root
|
||||
// package.json — taking the entire CLI down before it did anything. And even
|
||||
// where it resolved (Claude's synthetic `{"type":"commonjs"}`), there is no
|
||||
// `version` field, so the single consumer below already emitted
|
||||
// `version: undefined`. Resolve lazily and defensively instead:
|
||||
// 1. Installed trees carry <root>/gsd-core/VERSION (written by the installer);
|
||||
// this module lives at <root>/gsd-core/bin/lib, so VERSION is two dirs up.
|
||||
// 2. The source / npm-package tree has no gsd-core/VERSION but carries a real
|
||||
// package.json three dirs up — read it lazily, never at module-load time.
|
||||
// A failed/invalid lookup degrades to '' (the caller omits the field) rather
|
||||
// than crashing or emitting `version: undefined`. Both sources are validated
|
||||
// against the same semver shape the repo's other VERSION reader enforces
|
||||
// (src/update-context.cts) so a garbled VERSION file is never emitted verbatim.
|
||||
// Exported for the #1383 regression.
|
||||
const SEMVER_PREFIX = /^\d+\.\d+\.\d+/; // mirrors src/update-context.cts SEMVER_PREFIX
|
||||
function resolveVersionFrom(libDir: string): string {
|
||||
try {
|
||||
const v = fs.readFileSync(path.join(libDir, '..', '..', 'VERSION'), 'utf8').trim();
|
||||
if (SEMVER_PREFIX.test(v)) return v;
|
||||
} catch { /* not an installed tree (no gsd-core/VERSION) */ }
|
||||
try {
|
||||
const pkg = require(path.join(libDir, '..', '..', '..', 'package.json'));
|
||||
if (pkg && typeof pkg.version === 'string' && SEMVER_PREFIX.test(pkg.version)) return pkg.version;
|
||||
} catch { /* runtime root has no package.json (e.g. Codex) */ }
|
||||
return '';
|
||||
}
|
||||
|
||||
let cachedVersion: string | undefined;
|
||||
function gsdVersion(): string {
|
||||
if (cachedVersion === undefined) cachedVersion = resolveVersionFrom(__dirname);
|
||||
return cachedVersion;
|
||||
}
|
||||
|
||||
|
||||
const colorNameToHex = {
|
||||
cyan: '#00FFFF',
|
||||
@@ -393,7 +429,10 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c
|
||||
// Hermes' SKILL.md spec lists `version` as a required frontmatter field.
|
||||
// Track GSD's package version so Hermes' skill_view() reports a stable
|
||||
// identifier per install.
|
||||
if (runtime === 'hermes') fm += `version: ${yamlQuote(pkg.version)}\n`;
|
||||
if (runtime === 'hermes') {
|
||||
const version = gsdVersion();
|
||||
if (version) fm += `version: ${yamlQuote(version)}\n`;
|
||||
}
|
||||
// #778 (b) — Qwen-only numeric priority for /skills ordering. Scoped to qwen
|
||||
// so Claude/Hermes skill frontmatter is unchanged (they ignore the field, but
|
||||
// we keep their output byte-stable). skillName is the `gsd-<stem>` dir name.
|
||||
@@ -2541,6 +2580,9 @@ export = {
|
||||
convertClaudeCommandToKiloSkill,
|
||||
readGsdCommandNames,
|
||||
transformContentToHyphen,
|
||||
// #1383: version resolver (exported for regression test of the Codex
|
||||
// missing-package.json crash + the VERSION-file source of truth).
|
||||
resolveVersionFrom,
|
||||
// #1182: agent converters + tool-name table dependency closure
|
||||
claudeToCopilotTools,
|
||||
convertCopilotToolName,
|
||||
|
||||
@@ -336,3 +336,61 @@ describe('Hermes Agent: SKILL.md format validation', () => {
|
||||
assert.strictEqual(fm.name, 'gsd-plan');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #1383 regression: version lookup must not require a runtime-root package.json ──
|
||||
// The extracted conversion module sits in the gsd-tools loader chain, so its old
|
||||
// top-level `require('../../../package.json')` crashed EVERY gsd-tools command on
|
||||
// Codex — whose runtime root has no package.json — with
|
||||
// `Cannot find module '../../../package.json'`. The Hermes `version:` field (the
|
||||
// require's only consumer) must instead be sourced from the installed
|
||||
// gsd-core/VERSION, lazily and defensively, so the module loads everywhere and
|
||||
// the emitted version is a real semver, never `undefined`.
|
||||
describe('#1383 regression: gsd-tools version lookup without a runtime-root package.json', () => {
|
||||
// Require the EXTRACTED module that the gsd-tools chain loads (not install.js's
|
||||
// in-process copy), to assert the crash path itself is gone.
|
||||
const conversion = require('../gsd-core/bin/lib/runtime-artifact-conversion.cjs');
|
||||
|
||||
let tmp;
|
||||
beforeEach(() => { tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-1383-')); });
|
||||
afterEach(() => { cleanup(tmp); });
|
||||
|
||||
// Build a fake install layout <tmp>/gsd-core/bin/lib and return that libDir.
|
||||
// `version` writes <tmp>/gsd-core/VERSION; `rootPkg` writes <tmp>/package.json.
|
||||
function layout({ version, rootPkg } = {}) {
|
||||
const libDir = path.join(tmp, 'gsd-core', 'bin', 'lib');
|
||||
fs.mkdirSync(libDir, { recursive: true });
|
||||
if (version !== undefined) fs.writeFileSync(path.join(tmp, 'gsd-core', 'VERSION'), version);
|
||||
if (rootPkg !== undefined) fs.writeFileSync(path.join(tmp, 'package.json'), JSON.stringify(rootPkg));
|
||||
return libDir;
|
||||
}
|
||||
|
||||
test('reads gsd-core/VERSION when the runtime root has no package.json (Codex layout)', () => {
|
||||
const libDir = layout({ version: '9.9.9\n' }); // deliberately NO root package.json
|
||||
assert.ok(!fs.existsSync(path.join(tmp, 'package.json')),
|
||||
'precondition: Codex layout has no runtime-root package.json');
|
||||
let v;
|
||||
assert.doesNotThrow(() => { v = conversion.resolveVersionFrom(libDir); },
|
||||
'version lookup must not throw on a layout without a runtime-root package.json');
|
||||
assert.strictEqual(v, '9.9.9', 'version is read (trimmed) from the installed VERSION file');
|
||||
});
|
||||
|
||||
test('falls back to the runtime-root package.json when no VERSION file exists (source/npm layout)', () => {
|
||||
const libDir = layout({ rootPkg: { version: '1.2.3' } }); // no VERSION file
|
||||
assert.strictEqual(conversion.resolveVersionFrom(libDir), '1.2.3',
|
||||
'source/npm tree has a real package.json three dirs up');
|
||||
});
|
||||
|
||||
test('degrades to "" (never throws, never emits undefined) when neither source exists', () => {
|
||||
const libDir = layout({}); // neither VERSION nor package.json
|
||||
let v;
|
||||
assert.doesNotThrow(() => { v = conversion.resolveVersionFrom(libDir); });
|
||||
assert.strictEqual(v, '', 'no source -> empty string, so the caller omits the version field');
|
||||
});
|
||||
|
||||
test('rejects a non-semver VERSION file rather than emitting it verbatim', () => {
|
||||
const libDir = layout({ version: 'not-a-version\n' }); // malformed, no package.json fallback
|
||||
let v;
|
||||
assert.doesNotThrow(() => { v = conversion.resolveVersionFrom(libDir); });
|
||||
assert.strictEqual(v, '', 'garbled VERSION is rejected, so the caller omits the field');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user