Merge remote-tracking branch 'upstream/next' into kimi-runtime-support
This commit is contained in:
5
.changeset/silly-zebras-forage.md
Normal file
5
.changeset/silly-zebras-forage.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 817
|
||||
---
|
||||
**`/gsd:surface` no longer corrupts installed skill paths** — re-surfacing (profile/enable/disable/reset) now applies the same per-runtime path rewrites as install, so SKILL.md bodies keep the correct install target instead of reverting to the converter's default `~/.claude` paths.
|
||||
@@ -116,7 +116,7 @@ Module owning install detection for `/gsd:update`. `resolveUpdateContext({ home,
|
||||
Module owning which skills and agents are written to runtime config directories at install time (Phase 1) and at runtime via cluster-level toggles (Phase 2). Phase 1: `gsd-core/bin/lib/install-profiles.cjs` defines named profiles (`core`, `standard`, `full`), computes transitive closure over `requires:` frontmatter, stages skills/agents to runtime config dirs, and persists the chosen profile in a `.gsd-profile` marker. Profile resolution precedence: explicit `--profile=` flag > `.gsd-profile` marker > `full`. `--minimal`/`--core-only` are back-compat aliases for `--profile=core`. Phase 2: `gsd-core/bin/lib/surface.cjs` implements the `/gsd:surface` slash command for cluster-level enable/disable without reinstall; cluster definitions live in `gsd-core/bin/lib/clusters.cjs`; per-runtime state persists in `<runtimeConfigDir>/.gsd-surface.json` independent from the `.gsd-profile` marker. See ADR-0011.
|
||||
|
||||
### Runtime Artifact Layout Module
|
||||
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`). Phase 1 applies this seam to the Runtime Surface Module (`surface.cjs:applySurface`). 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.
|
||||
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`). 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. The shared accessor `getInstallExports` (exported from `runtime-artifact-layout.cjs`) is the single-source seam through which `surface.cjs` reaches `computePathPrefix` and `applyRuntimeContentRewritesInPlace`; the resolved `scope` (`'local'`|`'global'`) is now carried on the `Layout` object returned by `resolveRuntimeArtifactLayout` 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 Install Policy Module
|
||||
Projects a pure, typed install plan for a given runtime by composing artifact placements (Runtime Artifact Layout Module), command text (Shell Command Projection Module), and per-runtime config intentions — with no filesystem IO or format-specific serialization. Runtime-specific adapters consume the plan and execute concrete file mutations and config rendering. See ADR-58.
|
||||
|
||||
@@ -12006,6 +12006,7 @@ module.exports = {
|
||||
normalizeAgentBodyForRuntime,
|
||||
yamlIdentifier,
|
||||
computePathPrefix,
|
||||
applyRuntimeContentRewritesInPlace,
|
||||
getCodexSkillAdapterHeader,
|
||||
convertClaudeCommandToCursorSkill,
|
||||
convertClaudeCommandToCursorCommand,
|
||||
|
||||
@@ -34,6 +34,8 @@ const _require: NodeRequire = require;
|
||||
|
||||
interface InstallExports {
|
||||
readGsdCommandNames: () => string[];
|
||||
computePathPrefix: (opts: { isGlobal: boolean; isOpencode: boolean; isWindowsHost: boolean; resolvedTarget: string; homeDir: string }) => string;
|
||||
applyRuntimeContentRewritesInPlace: (stagedDir: string, runtime: string, pathPrefix: string) => void;
|
||||
[converterName: string]: unknown;
|
||||
}
|
||||
|
||||
@@ -86,6 +88,7 @@ interface ArtifactKind {
|
||||
interface Layout {
|
||||
runtime: string;
|
||||
configDir: string;
|
||||
scope?: 'local' | 'global';
|
||||
kinds: ArtifactKind[];
|
||||
}
|
||||
|
||||
@@ -426,7 +429,7 @@ function resolveRuntimeArtifactLayout(runtime: string, configDir: string, scope:
|
||||
throw new TypeError(`Unknown runtime: '${runtime}' — add to runtime-artifact-layout.cjs table`);
|
||||
}
|
||||
|
||||
return { runtime, configDir, kinds };
|
||||
return { runtime, configDir, scope, kinds };
|
||||
}
|
||||
|
||||
export = { resolveRuntimeArtifactLayout, findInstallSourceRoot };
|
||||
export = { resolveRuntimeArtifactLayout, findInstallSourceRoot, getInstallExports };
|
||||
|
||||
@@ -23,6 +23,7 @@
|
||||
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import os from 'node:os';
|
||||
import { platformWriteSync } from './shell-command-projection.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import installProfiles = require('./install-profiles.cjs');
|
||||
@@ -35,7 +36,7 @@ import { CLUSTERS } from './clusters.cjs';
|
||||
import type { ClusterMap } from './clusters.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import runtimeArtifactLayout = require('./runtime-artifact-layout.cjs');
|
||||
const { findInstallSourceRoot } = runtimeArtifactLayout;
|
||||
const { findInstallSourceRoot, getInstallExports } = runtimeArtifactLayout;
|
||||
|
||||
const SURFACE_FILE_NAME = '.gsd-surface.json';
|
||||
|
||||
@@ -53,6 +54,7 @@ interface ArtifactKind {
|
||||
interface Layout {
|
||||
runtime: string;
|
||||
configDir: string;
|
||||
scope?: 'local' | 'global';
|
||||
kinds: ArtifactKind[];
|
||||
}
|
||||
|
||||
@@ -256,8 +258,29 @@ function applySurface(runtimeConfigDir: string, layout: Layout, manifest: Map<st
|
||||
}
|
||||
const skillManifest = normalizeSkillManifest(layout.configDir, manifest);
|
||||
const resolved = resolveSurface(layout.configDir, skillManifest, clusterMap);
|
||||
// Mirror installRuntimeArtifacts: skills kinds get per-runtime path rewrites
|
||||
// so SKILL.md bodies reference the install target (pathPrefix), not the
|
||||
// converter's default ~/.claude paths (#813). Computed lazily so command-only
|
||||
// runtimes do not trigger the install.js require.
|
||||
let pathPrefix: string | null = null;
|
||||
for (const kind of layout.kinds) {
|
||||
const staged = kind.stage(resolved);
|
||||
if (kind.kind === 'skills') {
|
||||
const installExports = getInstallExports();
|
||||
if (pathPrefix === null) {
|
||||
const scope = layout.scope ?? 'global';
|
||||
const resolvedTarget = path.resolve(layout.configDir).replace(/\\/g, '/');
|
||||
const homeDir = os.homedir().replace(/\\/g, '/');
|
||||
pathPrefix = installExports.computePathPrefix({
|
||||
isGlobal: scope === 'global',
|
||||
isOpencode: layout.runtime === 'opencode',
|
||||
isWindowsHost: process.platform === 'win32',
|
||||
resolvedTarget,
|
||||
homeDir,
|
||||
});
|
||||
}
|
||||
installExports.applyRuntimeContentRewritesInPlace(staged, layout.runtime, pathPrefix);
|
||||
}
|
||||
const dest = path.join(layout.configDir, kind.destSubpath);
|
||||
_syncGsdDir(staged, dest, kind, skillManifest);
|
||||
}
|
||||
|
||||
@@ -302,6 +302,89 @@ describe('applySurface', () => {
|
||||
assert.ok(fs.existsSync(userDir), 'user-custom-skill dir must be preserved when kindPrefix is empty (Hermes)');
|
||||
assert.ok(fs.existsSync(path.join(destDir, stem1, 'SKILL.md')), 'GSD help/SKILL.md must be copied');
|
||||
});
|
||||
|
||||
// Regression test for #813: applySurface must apply per-runtime path rewrites
|
||||
// (applyRuntimeContentRewritesInPlace) just as installRuntimeArtifacts does.
|
||||
// Without the fix, skill bodies retain the converter's default ~/.claude/ paths
|
||||
// instead of being rewritten to the install target (pathPrefix).
|
||||
//
|
||||
// Both 'cursor' and 'codex' use skillsKind AND have a path-rewrite case in
|
||||
// _applyRuntimeRewrites — so the regression guard covers both.
|
||||
for (const runtime of ['cursor', 'codex']) {
|
||||
test(`applySurface rewrites ${runtime} skill bodies to the install pathPrefix, not the converter default ~/.claude path (#813)`, (t) => {
|
||||
// Use mkdtempSync under os.tmpdir() — NOT under the user's home dir — so that
|
||||
// computePathPrefix returns an ABSOLUTE prefix `${configDir}/` for local installs,
|
||||
// clearly distinguishable from the `~/.claude/` converter default.
|
||||
const configDir = fs.mkdtempSync(path.join(os.tmpdir(), `gsd-surface-813-${runtime}-`));
|
||||
t.after(() => cleanup(configDir));
|
||||
|
||||
writeActiveProfile(configDir, 'standard');
|
||||
writeSurface(configDir, {
|
||||
baseProfile: 'standard',
|
||||
disabledClusters: [],
|
||||
explicitAdds: [],
|
||||
explicitRemoves: [],
|
||||
});
|
||||
|
||||
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||
// skills/gsd-<name>/SKILL.md (destSubpath 'skills', prefix 'gsd-')
|
||||
// scope='local' ensures computePathPrefix returns an absolute configDir prefix
|
||||
// rather than the home-relative default (e.g. ~/.cursor/ or ~/.codex/).
|
||||
const layout = resolveRuntimeArtifactLayout(runtime, configDir, 'local');
|
||||
applySurface(configDir, layout, manifest, CLUSTERS);
|
||||
|
||||
// Collect every SKILL.md body under ${configDir}/skills/
|
||||
const skillsRoot = path.join(configDir, 'skills');
|
||||
const skillBodies = [];
|
||||
if (fs.existsSync(skillsRoot)) {
|
||||
for (const dirEntry of fs.readdirSync(skillsRoot)) {
|
||||
const skillMd = path.join(skillsRoot, dirEntry, 'SKILL.md');
|
||||
if (fs.existsSync(skillMd)) {
|
||||
skillBodies.push(fs.readFileSync(skillMd, 'utf8'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// (a) Sanity: at least one SKILL.md must have been staged
|
||||
assert.ok(
|
||||
skillBodies.length > 0,
|
||||
`applySurface must stage at least one ${runtime} SKILL.md under ${skillsRoot}/ but found none`
|
||||
);
|
||||
|
||||
// (b) BUG SYMPTOM (#813): after the rewrite, no body should contain '~/.claude/'
|
||||
// or '$HOME/.claude/' — both are converter-default forms that must be eliminated.
|
||||
// This assertion FAILS on unpatched code — applySurface does not call
|
||||
// applyRuntimeContentRewritesInPlace, so the converter's default ~/.claude/
|
||||
// paths are left verbatim in the staged files.
|
||||
const bodiesWithTildeClaude = skillBodies.filter(b => b.includes('~/.claude/') || b.includes('$HOME/.claude/'));
|
||||
assert.strictEqual(
|
||||
bodiesWithTildeClaude.length,
|
||||
0,
|
||||
`#813 regression: ${bodiesWithTildeClaude.length} ${runtime} SKILL.md(s) still contain '~/.claude/' or '$HOME/.claude/' after applySurface — ` +
|
||||
`applyRuntimeContentRewritesInPlace was not applied (mirrors installRuntimeArtifacts' rewrite step)`
|
||||
);
|
||||
|
||||
// (c) The rewrite must inject the real install target path, not just remove the tilde.
|
||||
// This also fails on unpatched code for the same reason.
|
||||
// NOTE: this assertion depends on the command corpus emitting rewritable
|
||||
// '~/.claude/'-style paths in at least one skill body. If a future corpus
|
||||
// change removes all such paths, this assertion will become vacuously true
|
||||
// (no body will contain configDirPrefix either) — update the test rather than
|
||||
// treating a silent zero-match as a pass.
|
||||
// Production derives pathPrefix as `path.resolve(configDir).replace(/\\/g, '/')`
|
||||
// (mirrors installRuntimeArtifacts), so on Windows the rewritten body uses
|
||||
// forward slashes. Normalize the expected prefix the same way so this assertion
|
||||
// is cross-platform (Windows CI leg is not covered by local gsd-test) (#813).
|
||||
const configDirPrefix = `${path.resolve(configDir).replace(/\\/g, '/')}/`;
|
||||
const bodiesWithAbsolutePrefix = skillBodies.filter(b => b.includes(configDirPrefix));
|
||||
assert.ok(
|
||||
bodiesWithAbsolutePrefix.length > 0,
|
||||
`#813 regression: no ${runtime} SKILL.md contains the absolute configDir prefix '${configDirPrefix}' — ` +
|
||||
`the path rewrite was not applied by applySurface. ` +
|
||||
`(If the command corpus no longer emits any '~/.claude/'-style paths, update this test.)`
|
||||
);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
// ─── resolveSurface ──────────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user