feat(#2873): detect cross-scope shadowing and reach the local spec tree
4a - the detection floor. A shadowed install now reports which triggers are shadowed and which scope wins, at install time and through a new W028 /gsd-health diagnostic. Exit codes are untouched: a shadowed install is a warning, not a failure. Only triggers whose stem exists at BOTH scopes are reported, so a global full profile beside a local core profile no longer names local artifacts the user does not have. 4b - spec-root reachability, claude runtime and global scope only. The winning global skill stops carrying a static workflow @-include and instead resolves its spec at runtime: prefer the project-local copy, fall back to the global one, stop if neither exists. Every other @-include stays static, and the local emission is byte-identical. It runs after the staged-skills rewrite pass, whose claude branch would otherwise mangle the literal tilde path into an undocumented $HOME form. Also fixed inline: readInstallManifest classified a top-level JSON array as an installed v1 manifest, because typeof [] is object. Refs #2873
This commit is contained in:
@@ -504,6 +504,88 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c
|
||||
return `${fm}\n${normalizedBody}`;
|
||||
}
|
||||
|
||||
// #2873 (4b) — spec-root reachability. Matches ONLY a line that is a real
|
||||
// `@~/.claude/gsd-core/workflows/<stem>.md` include: line-start `@`, exact
|
||||
// spec-root shape, nothing else on the line. This is deliberately narrower
|
||||
// than "any line mentioning gsd-core/workflows" so prose mentions and
|
||||
// `references/`/`templates/`/`@.planning/...` includes are never touched
|
||||
// (rows 24/25). CRLF-safe: an optional trailing `\r` is captured and
|
||||
// preserved rather than dropped.
|
||||
const WORKFLOW_SPEC_ROOT_INCLUDE_RE = /^@~\/\.claude\/gsd-core\/workflows\/([A-Za-z0-9._-]+)\.md[ \t]*(\r?)$/gm;
|
||||
|
||||
// Matches a fenced code-block delimiter line (``` or ~~~, any info string)
|
||||
// so occurrences of the include shape used as *documentation* inside a fence
|
||||
// are left untouched — Claude Code documents backticks as the way to
|
||||
// *prevent* an `@`-import, so rewriting a fenced example would corrupt
|
||||
// documentation-of-the-syntax.
|
||||
const FENCE_DELIMITER_RE = /^(```|~~~)[^\r\n]*$/gm;
|
||||
|
||||
/**
|
||||
* Rewrite a static global-scope Claude skill `@`-include of the command's own
|
||||
* workflow spec into an imperative two-step resolution the agent performs at
|
||||
* runtime: prefer the project-local spec (cwd-relative), fall back to the
|
||||
* global spec, and treat "neither exists" as a visible failure rather than a
|
||||
* silent no-spec proceed.
|
||||
*
|
||||
* WHY this can't stay a static `@`-include (even a relative one): Claude Code
|
||||
* documents relative `@`-paths as resolving against the file *containing* the
|
||||
* import, which for a global skill is `~/.claude/skills/gsd-<stem>/` — not
|
||||
* the project's working directory. `@./.claude/...` would therefore always
|
||||
* resolve inside the skill's own install directory, never the project, so
|
||||
* there is no static include syntax that can express "prefer local, fall
|
||||
* back to global". This function exists precisely so that resolution can be
|
||||
* performed by the agent, not the host's pre-expansion.
|
||||
*
|
||||
* Scope-free by design: this function does not know or care whether it is
|
||||
* being applied to a global or local artifact, or which runtime — that
|
||||
* judgment belongs to the caller (`skillsKind` in
|
||||
* `runtime-artifact-layout.cts`, the one site that knows install scope).
|
||||
* Applying it to a body with no workflow include is a no-op (row 26); a body
|
||||
* with two independent workflow includes has each rewritten independently
|
||||
* (row 27); an include inside a fenced code block or wrapped in inline
|
||||
* backticks is left untouched (the backtick case is already excluded by the
|
||||
* line-start anchor, since a backtick-wrapped line does not begin with `@`).
|
||||
* Idempotent: the replacement text never begins with `@` and never matches
|
||||
* `WORKFLOW_SPEC_ROOT_INCLUDE_RE`, so re-applying this function to its own
|
||||
* output is a no-op.
|
||||
*/
|
||||
function resolveSpecRootReference(body) {
|
||||
if (typeof body !== 'string' || body.length === 0) return body;
|
||||
if (!body.includes('@~/.claude/gsd-core/workflows/')) return body;
|
||||
|
||||
// Collect [start, end) offset ranges covered by fenced code blocks so
|
||||
// matches inside them are skipped. An unterminated trailing fence covers
|
||||
// to the end of the string (still "inside a fence").
|
||||
const fenceRanges = [];
|
||||
{
|
||||
let m;
|
||||
let openStart = null;
|
||||
FENCE_DELIMITER_RE.lastIndex = 0;
|
||||
while ((m = FENCE_DELIMITER_RE.exec(body)) !== null) {
|
||||
if (openStart === null) {
|
||||
openStart = m.index;
|
||||
} else {
|
||||
fenceRanges.push([openStart, m.index + m[0].length]);
|
||||
openStart = null;
|
||||
}
|
||||
}
|
||||
if (openStart !== null) fenceRanges.push([openStart, body.length]);
|
||||
}
|
||||
const isInsideFence = (offset) => fenceRanges.some(([start, end]) => offset >= start && offset < end);
|
||||
|
||||
return body.replace(WORKFLOW_SPEC_ROOT_INCLUDE_RE, (match, stem, cr, offset) => {
|
||||
if (isInsideFence(offset)) return match;
|
||||
return (
|
||||
`To load this command's workflow spec: check for ` +
|
||||
`\`.claude/gsd-core/workflows/${stem}.md\` relative to the current working ` +
|
||||
`directory first (project-local); if it is not there, fall back to ` +
|
||||
`\`~/.claude/gsd-core/workflows/${stem}.md\` (the global install). If ` +
|
||||
`neither file exists, stop — a workflow spec is required and none was found.` +
|
||||
cr
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
function normalizeKimiSkillName(skillName) {
|
||||
let text = String(skillName || '').trim().toLowerCase();
|
||||
if (text.startsWith('/')) text = text.slice(1);
|
||||
@@ -2995,6 +3077,38 @@ function applyRuntimeContentRewritesForCommandsInPlace(stagedDir, runtime, pathP
|
||||
return tempDir;
|
||||
}
|
||||
|
||||
/**
|
||||
* #2873 (4b) — second pass over a staged skills directory, run strictly AFTER
|
||||
* `applyRuntimeContentRewritesInPlace`. That pass's `case 'claude':` branch
|
||||
* unconditionally rewrites any bare (non-`@`-prefixed) `~/.claude/` substring
|
||||
* in the body to the computed pathPrefix (`$HOME/.claude/` for a global
|
||||
* install) and restores ONLY the `@`-prefixed form back to `~`
|
||||
* (`@$HOME/.claude/` → `@~/.claude/`). `resolveSpecRootReference`'s
|
||||
* replacement text is deliberately imperative prose containing a literal,
|
||||
* non-`@`-prefixed `~/.claude/gsd-core/workflows/<stem>.md` — running it
|
||||
* BEFORE the pass above would let that literal tilde text get silently
|
||||
* mangled into the undocumented `$HOME/` form the design explicitly rejects.
|
||||
* Running it here, after, means it only ever sees the FINAL
|
||||
* `@~/.claude/gsd-core/workflows/<stem>.md` include line (which survives the
|
||||
* pass above intact via its own `@`-guarded restore).
|
||||
*/
|
||||
function applySpecRootReferenceToStagedSkills(stagedDir) {
|
||||
if (!fs.existsSync(stagedDir)) return;
|
||||
const walk = (dir) => {
|
||||
for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
|
||||
const fullPath = path.join(dir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
walk(fullPath);
|
||||
} else if (entry.name === 'SKILL.md') {
|
||||
const content = fs.readFileSync(fullPath, 'utf8');
|
||||
const rewritten = resolveSpecRootReference(content);
|
||||
if (rewritten !== content) fs.writeFileSync(fullPath, rewritten);
|
||||
}
|
||||
}
|
||||
};
|
||||
walk(stagedDir);
|
||||
}
|
||||
|
||||
/**
|
||||
* HIGH-LEVEL: In-place fs walk: rewrite all .md files under stagedDir for the given runtime.
|
||||
*
|
||||
@@ -3032,6 +3146,17 @@ function rewriteStagedSkillBodies(stagedDir, opts) {
|
||||
const attribution = resolveAttribution ? resolveAttribution(runtime) : undefined;
|
||||
|
||||
applyRuntimeContentRewritesInPlace(stagedDir, runtime, pathPrefix, isGlobal, attribution);
|
||||
// #2873 (4b): claude, global scope only — see
|
||||
// applySpecRootReferenceToStagedSkills's doc comment for why this MUST run
|
||||
// after the rewrite pass above, not before. `rewriteStagedSkillBodies` is
|
||||
// the skills-kind seam (`kind.kind === 'skills'`), so this never touches a
|
||||
// 'commands' or 'agents' kind body (rows 24/25 unaffected), and claude has
|
||||
// no skills-kind entry at local scope, so this is already structurally
|
||||
// scoped to global (row 23) — the explicit isGlobal check is defense-in-depth
|
||||
// against that descriptor wiring ever changing.
|
||||
if (runtime === 'claude' && isGlobal) {
|
||||
applySpecRootReferenceToStagedSkills(stagedDir);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -3176,6 +3301,11 @@ export = {
|
||||
convertClaudeToAntigravityContent,
|
||||
convertClaudeCommandToAntigravitySkill,
|
||||
convertClaudeCommandToClaudeSkill,
|
||||
// #2873 (4b): pure, scope-free transform — applied by the one call site
|
||||
// that knows install scope (skillsKind's stage() in
|
||||
// runtime-artifact-layout.cts), never inside convertClaudeCommandToClaudeSkill
|
||||
// itself.
|
||||
resolveSpecRootReference,
|
||||
convertClaudeCommandToKimiSkill,
|
||||
convertClaudeCommandToKimiCodeSkill,
|
||||
buildKimiAgentArtifacts,
|
||||
|
||||
Reference in New Issue
Block a user