mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-10 06:58:59 +02:00
fix(setup): alias skills install as rewritten copies, never symlinks
The two back-compat alias dirs — _gstack-command (root router) and connect-chrome (→ open-gstack-browser) — symlinked the canonical SKILL.md verbatim, so each alias re-served the canonical frontmatter name:. Claude Code keys skills on that name and requires global uniqueness: the connect-chrome duplicate silently shadowed /open-gstack-browser (whichever readdir returned first won), and the _gstack-command duplicate could drop the ENTIRE personal-skills set — every /gstack command vanished until the user hand-deleted the alias dirs, and the next setup re-broke it. Fix: copy-then-rewrite. A shared _install_alias_skill_md helper reads the SOURCE SKILL.md and writes a fresh copy with name: rewritten to the alias dir's own name (_gstack-command / connect-chrome / gstack-connect-chrome). sed never edits in place: on Unix the old install was a symlink into the repo, and an in-place rewrite through it would have corrupted the generated source (eng review E2). bin/gstack-relink gets the same treatment for its root-alias helper, and its discovery loop now skips symlinked source dirs so the connect-chrome repo symlink can't re-mint the duplicate. Tests assert: installed aliases are NOT symlinks, carry their own unique names, all installed frontmatter names are globally unique, re-runs refresh cleanly, legacy symlinked aliases are replaced not written through, and the source files stay byte-intact. Fixes #2511 Fixes #2201 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan
co-authored by
Claude Fable 5
Claude Fable 5
parent
0f38feee78
commit
663aca3b05
+12
-1
@@ -52,7 +52,13 @@ _link_root_skill_alias() {
|
||||
[ -f "$INSTALL_DIR/SKILL.md" ] || return 0
|
||||
[ -L "$target" ] && rm -f "$target"
|
||||
mkdir -p "$target"
|
||||
ln -snf "$INSTALL_DIR/SKILL.md" "$target/SKILL.md"
|
||||
# Copy-then-rewrite, never a symlink (#2511): a symlinked alias re-serves
|
||||
# the canonical `name: gstack`, Claude Code sees a duplicate skill name,
|
||||
# and drops the ENTIRE personal-skills set. sed reads the source and writes
|
||||
# a fresh copy — remove any prior symlink first so the redirect can never
|
||||
# write through it into the generated source.
|
||||
rm -f "$target/SKILL.md"
|
||||
sed "1,/^---\$/ s/^name:[[:space:]].*/name: _gstack-command/" "$INSTALL_DIR/SKILL.md" > "$target/SKILL.md"
|
||||
}
|
||||
|
||||
_link_root_skill_alias
|
||||
@@ -61,6 +67,11 @@ _link_root_skill_alias
|
||||
SKILL_COUNT=0
|
||||
for skill_dir in "$INSTALL_DIR"/*/; do
|
||||
[ -d "$skill_dir" ] || continue
|
||||
# Skip symlinked skill dirs (connect-chrome → open-gstack-browser): linking
|
||||
# one under the symlink's basename would duplicate the canonical frontmatter
|
||||
# name and collide in Claude Code's skill registry (#2201). setup owns the
|
||||
# rewritten-copy alias for those.
|
||||
[ -L "${skill_dir%/}" ] && continue
|
||||
skill=$(basename "$skill_dir")
|
||||
# Skip non-skill directories
|
||||
case "$skill" in bin|browse|design|docs|extension|lib|node_modules|scripts|test|.git|.github) continue ;; esac
|
||||
|
||||
@@ -784,24 +784,40 @@ link_claude_skill_dirs() {
|
||||
fi
|
||||
}
|
||||
|
||||
# ─── Helper: install an alias SKILL.md as a rewritten COPY ───────────────────
|
||||
# Alias dirs (_gstack-command, connect-chrome) must NOT symlink the canonical
|
||||
# SKILL.md: the alias then carries the canonical frontmatter name:, Claude Code
|
||||
# sees two skills with the same name, and drops the ENTIRE personal-skills set
|
||||
# (#2511, #2201). Copy-then-rewrite instead: sed reads the SOURCE and writes a
|
||||
# fresh copy with name: set to the alias. It must never edit through an
|
||||
# existing symlink — that would rewrite the generated source file itself.
|
||||
_install_alias_skill_md() {
|
||||
local src_skill_md="$1"
|
||||
local dst_dir="$2"
|
||||
local alias_name="$3"
|
||||
[ -f "$src_skill_md" ] || return 0
|
||||
# Old installs left the alias as a whole-dir symlink — replace it.
|
||||
if [ -L "$dst_dir" ]; then rm -f "$dst_dir"; fi
|
||||
mkdir -p "$dst_dir"
|
||||
# Remove any prior symlinked SKILL.md so the redirect below cannot write
|
||||
# through it into the generated source.
|
||||
rm -f "$dst_dir/SKILL.md"
|
||||
sed "1,/^---\$/ s/^name:[[:space:]].*/name: $alias_name/" "$src_skill_md" > "$dst_dir/SKILL.md"
|
||||
}
|
||||
|
||||
# Claude Code skips the repo-shaped ~/.claude/skills/gstack directory when
|
||||
# building the user-facing slash-command list. Keep the repo path for runtime
|
||||
# assets, and add a separate thin wrapper whose frontmatter name remains
|
||||
# `gstack` so `/gstack` can autocomplete.
|
||||
# assets, and add a separate thin wrapper. Its frontmatter name is rewritten to
|
||||
# `_gstack-command` (the dir name) so it never collides with the canonical
|
||||
# `gstack` name (#2511).
|
||||
link_claude_root_skill_alias() {
|
||||
local gstack_dir="$1"
|
||||
local skills_dir="$2"
|
||||
local target="$skills_dir/_gstack-command"
|
||||
|
||||
[ -f "$gstack_dir/SKILL.md" ] || return 0
|
||||
if [ -L "$target" ]; then
|
||||
rm -f "$target"
|
||||
fi
|
||||
mkdir -p "$target"
|
||||
if [ -L "$target/SKILL.md" ]; then rm "$target/SKILL.md"; fi
|
||||
_link_or_copy "$gstack_dir/SKILL.md" "$target/SKILL.md"
|
||||
_install_alias_skill_md "$gstack_dir/SKILL.md" "$target" "_gstack-command"
|
||||
echo " linked root skill alias: gstack"
|
||||
_print_windows_copy_note_once
|
||||
}
|
||||
|
||||
# ─── Helper: remove old unprefixed Claude skill entries ───────────────────────
|
||||
@@ -1231,13 +1247,16 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then
|
||||
GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" >/dev/null 2>&1 || true
|
||||
fi
|
||||
# Backwards-compat alias: /connect-chrome → /open-gstack-browser
|
||||
# Rewritten copy, not a symlink: a symlinked alias re-serves the canonical
|
||||
# name: open-gstack-browser, so one of the two silently shadows the other
|
||||
# (#2201) — and duplicate names can drop the whole skill set (#2511).
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/connect-chrome"
|
||||
_OGB_ALIAS_NAME="connect-chrome"
|
||||
if [ "$SKILL_PREFIX" -eq 1 ]; then
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/gstack-connect-chrome"
|
||||
_OGB_ALIAS_NAME="gstack-connect-chrome"
|
||||
fi
|
||||
if [ -L "$_OGB_LINK" ] || [ ! -e "$_OGB_LINK" ]; then
|
||||
_link_or_copy "gstack/open-gstack-browser" "$_OGB_LINK"
|
||||
fi
|
||||
_install_alias_skill_md "$SOURCE_GSTACK_DIR/open-gstack-browser/SKILL.md" "$_OGB_LINK" "$_OGB_ALIAS_NAME"
|
||||
if [ "$LOCAL_INSTALL" -eq 1 ]; then
|
||||
log "gstack ready (project-local)."
|
||||
log " skills: $INSTALL_SKILLS_DIR"
|
||||
@@ -1299,13 +1318,17 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then
|
||||
if [ -x "$GSTACK_RELINK" ]; then
|
||||
GSTACK_SKILLS_DIR="$INSTALL_SKILLS_DIR" GSTACK_INSTALL_DIR="$SOURCE_GSTACK_DIR" "$GSTACK_RELINK" >/dev/null 2>&1 || true
|
||||
fi
|
||||
# Rewritten copy, not a symlink: a symlinked alias re-serves the
|
||||
# canonical name: open-gstack-browser, so one of the two silently
|
||||
# shadows the other (#2201) — and duplicate names can drop the whole
|
||||
# skill set (#2511).
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/connect-chrome"
|
||||
_OGB_ALIAS_NAME="connect-chrome"
|
||||
if [ "$SKILL_PREFIX" -eq 1 ]; then
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/gstack-connect-chrome"
|
||||
_OGB_ALIAS_NAME="gstack-connect-chrome"
|
||||
fi
|
||||
if [ -L "$_OGB_LINK" ] || [ ! -e "$_OGB_LINK" ]; then
|
||||
_link_or_copy "gstack/open-gstack-browser" "$_OGB_LINK"
|
||||
fi
|
||||
_install_alias_skill_md "$SOURCE_GSTACK_DIR/open-gstack-browser/SKILL.md" "$_OGB_LINK" "$_OGB_ALIAS_NAME"
|
||||
log "gstack ready (claude)."
|
||||
log " browse: $BROWSE_BIN"
|
||||
fi
|
||||
|
||||
@@ -2435,7 +2435,10 @@ describe('setup script validation', () => {
|
||||
const fnEnd = setupContent.indexOf('# ─── Helper: remove old unprefixed Claude skill entries', fnStart);
|
||||
const fnBody = setupContent.slice(fnStart, fnEnd);
|
||||
expect(fnBody).toContain('_gstack-command');
|
||||
expect(fnBody).toContain('_link_or_copy "$gstack_dir/SKILL.md" "$target/SKILL.md"');
|
||||
// #2511: the alias must be a rewritten COPY (unique frontmatter name),
|
||||
// never a verbatim symlink of the canonical SKILL.md.
|
||||
expect(fnBody).toContain('_install_alias_skill_md "$gstack_dir/SKILL.md" "$target" "_gstack-command"');
|
||||
expect(fnBody).not.toContain('_link_or_copy "$gstack_dir/SKILL.md"');
|
||||
|
||||
const claudeSection = setupContent.slice(
|
||||
setupContent.indexOf('# 4. Install for Claude'),
|
||||
|
||||
+43
-3
@@ -215,9 +215,15 @@ describe('gstack-relink (#578)', () => {
|
||||
const aliasSkill = path.join(aliasDir, 'SKILL.md');
|
||||
expect(fs.lstatSync(aliasDir).isDirectory()).toBe(true);
|
||||
expect(fs.lstatSync(aliasDir).isSymbolicLink()).toBe(false);
|
||||
expect(fs.lstatSync(aliasSkill).isSymbolicLink()).toBe(true);
|
||||
expect(fs.readlinkSync(aliasSkill)).toBe(path.join(installDir, 'SKILL.md'));
|
||||
expect(fs.readFileSync(aliasSkill, 'utf-8')).toContain('name: gstack');
|
||||
// #2511: the alias is a rewritten COPY, never a symlink. A symlinked
|
||||
// alias re-serves the canonical `name: gstack`; Claude Code refuses
|
||||
// duplicate skill names and drops the entire personal-skills set.
|
||||
expect(fs.lstatSync(aliasSkill).isSymbolicLink()).toBe(false);
|
||||
const aliasContent = fs.readFileSync(aliasSkill, 'utf-8');
|
||||
expect(aliasContent).toContain('name: _gstack-command');
|
||||
expect(aliasContent).not.toContain('name: gstack\n');
|
||||
// The rewrite happened on the COPY: the canonical source keeps its name.
|
||||
expect(fs.readFileSync(path.join(installDir, 'SKILL.md'), 'utf-8')).toContain('name: gstack');
|
||||
|
||||
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix true`, {
|
||||
GSTACK_INSTALL_DIR: installDir,
|
||||
@@ -226,6 +232,40 @@ describe('gstack-relink (#578)', () => {
|
||||
expect(fs.existsSync(aliasSkill)).toBe(true);
|
||||
});
|
||||
|
||||
// #2201: connect-chrome ships as a dir SYMLINK to open-gstack-browser. The
|
||||
// discovery loop used to link it under its own basename while its SKILL.md
|
||||
// carried `name: open-gstack-browser` — a duplicate name that silently
|
||||
// shadows the real skill (readdir-order roulette). Symlinked source dirs
|
||||
// must be skipped; setup owns the rewritten-copy alias.
|
||||
test('symlinked skill dirs are skipped, so no duplicate frontmatter names (#2201)', () => {
|
||||
setupMockInstall(['open-gstack-browser', 'qa']);
|
||||
fs.symlinkSync(
|
||||
path.join(installDir, 'open-gstack-browser'),
|
||||
path.join(installDir, 'connect-chrome'),
|
||||
);
|
||||
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix false`, {
|
||||
GSTACK_INSTALL_DIR: installDir,
|
||||
GSTACK_SKILLS_DIR: skillsDir,
|
||||
});
|
||||
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
||||
GSTACK_INSTALL_DIR: installDir,
|
||||
GSTACK_SKILLS_DIR: skillsDir,
|
||||
});
|
||||
|
||||
expect(fs.existsSync(path.join(skillsDir, 'open-gstack-browser'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(skillsDir, 'connect-chrome'))).toBe(false);
|
||||
|
||||
// No two installed SKILL.md files may share a frontmatter name.
|
||||
const names: string[] = [];
|
||||
for (const entry of fs.readdirSync(skillsDir)) {
|
||||
const skillMd = path.join(skillsDir, entry, 'SKILL.md');
|
||||
if (!fs.existsSync(skillMd)) continue;
|
||||
const m = fs.readFileSync(skillMd, 'utf-8').match(/^name:\s*(\S+)/m);
|
||||
if (m) names.push(m[1]);
|
||||
}
|
||||
expect(new Set(names).size).toBe(names.length);
|
||||
});
|
||||
|
||||
// FIRST INSTALL: --no-prefix must create ONLY flat names, zero gstack-* pollution
|
||||
test('first install --no-prefix: only flat names exist, zero gstack-* entries', () => {
|
||||
setupMockInstall(['qa', 'ship', 'review', 'plan-ceo-review', 'gstack-upgrade']);
|
||||
|
||||
@@ -0,0 +1,164 @@
|
||||
/**
|
||||
* Alias name uniqueness (#2511 / #2201).
|
||||
*
|
||||
* setup installs two back-compat alias dirs — `_gstack-command` (root router)
|
||||
* and `connect-chrome` (→ open-gstack-browser). Both used to symlink the
|
||||
* canonical SKILL.md verbatim, so the alias carried the canonical frontmatter
|
||||
* `name:`. Claude Code keys skills on that name and requires global
|
||||
* uniqueness: the `connect-chrome` duplicate silently shadowed
|
||||
* /open-gstack-browser (readdir-order roulette), and the `_gstack-command`
|
||||
* duplicate could drop the ENTIRE personal-skills set.
|
||||
*
|
||||
* The fix is copy-then-rewrite: sed reads the SOURCE and writes a fresh copy
|
||||
* with `name:` set to the alias dir's own name. Eng review E2 pinned the
|
||||
* hazard this suite guards hardest: on Unix the old install path was a
|
||||
* SYMLINK to the repo source, so an in-place sed through it would have
|
||||
* corrupted the generated SKILL.md — the source files must stay byte-intact.
|
||||
*/
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { spawnSync } from 'child_process';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const SETUP_SRC = fs.readFileSync(path.join(ROOT, 'setup'), 'utf-8');
|
||||
|
||||
function extractFn(name: string): string {
|
||||
const start = SETUP_SRC.indexOf(`${name}() {`);
|
||||
const end = SETUP_SRC.indexOf('\n}\n', start);
|
||||
if (start < 0 || end < 0) throw new Error(`Could not locate ${name}() in setup`);
|
||||
return SETUP_SRC.slice(start, end + 2);
|
||||
}
|
||||
|
||||
const installDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-alias-install-'));
|
||||
|
||||
const sourceRootSkill = fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8');
|
||||
const sourceOgbSkill = fs.readFileSync(
|
||||
path.join(ROOT, 'open-gstack-browser', 'SKILL.md'),
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
beforeAll(() => {
|
||||
const installOnce = [
|
||||
`link_claude_skill_dirs "${ROOT}" "${installDir}"`,
|
||||
`link_claude_root_skill_alias "${ROOT}" "${installDir}"`,
|
||||
// The connect-chrome back-compat alias, exactly as the install section does it.
|
||||
`_install_alias_skill_md "${ROOT}/open-gstack-browser/SKILL.md" "${installDir}/connect-chrome" "connect-chrome"`,
|
||||
].join('\n');
|
||||
const script = [
|
||||
'set -e',
|
||||
'IS_WINDOWS=0',
|
||||
'SKILL_PREFIX=0',
|
||||
'QUIET=1',
|
||||
'_WINDOWS_COPY_NOTE_PRINTED=1',
|
||||
extractFn('_link_or_copy'),
|
||||
extractFn('_print_windows_copy_note_once'),
|
||||
extractFn('_link_skill_runtime_assets'),
|
||||
extractFn('link_claude_skill_dirs'),
|
||||
extractFn('_install_alias_skill_md'),
|
||||
extractFn('link_claude_root_skill_alias'),
|
||||
// Run TWICE: the second pass proves re-runs refresh instead of corrupting
|
||||
// (the historical failure mode was sed'ing through a symlink on re-run).
|
||||
installOnce,
|
||||
installOnce,
|
||||
].join('\n');
|
||||
const result = spawnSync('bash', ['-c', script], { encoding: 'utf-8', timeout: 60_000 });
|
||||
if (result.status !== 0) {
|
||||
throw new Error(`alias install failed: ${result.stderr}\n${result.stdout}`);
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
afterAll(() => {
|
||||
fs.rmSync(installDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
function frontmatterName(skillMdPath: string): string | null {
|
||||
const m = fs.readFileSync(skillMdPath, 'utf-8').match(/^name:\s*(\S+)/m);
|
||||
return m ? m[1] : null;
|
||||
}
|
||||
|
||||
describe('alias installs are rewritten copies (#2511, #2201)', () => {
|
||||
test('_gstack-command alias is NOT a symlink and carries its own name', () => {
|
||||
const aliasDir = path.join(installDir, '_gstack-command');
|
||||
const aliasSkill = path.join(aliasDir, 'SKILL.md');
|
||||
expect(fs.lstatSync(aliasDir).isSymbolicLink()).toBe(false);
|
||||
expect(fs.lstatSync(aliasSkill).isSymbolicLink()).toBe(false);
|
||||
expect(frontmatterName(aliasSkill)).toBe('_gstack-command');
|
||||
});
|
||||
|
||||
test('connect-chrome alias is NOT a symlink and carries its own name', () => {
|
||||
const aliasDir = path.join(installDir, 'connect-chrome');
|
||||
const aliasSkill = path.join(aliasDir, 'SKILL.md');
|
||||
expect(fs.lstatSync(aliasDir).isSymbolicLink()).toBe(false);
|
||||
expect(fs.lstatSync(aliasSkill).isSymbolicLink()).toBe(false);
|
||||
expect(frontmatterName(aliasSkill)).toBe('connect-chrome');
|
||||
});
|
||||
|
||||
test('alias body is the canonical content — only the name: line differs', () => {
|
||||
const alias = fs.readFileSync(
|
||||
path.join(installDir, '_gstack-command', 'SKILL.md'),
|
||||
'utf-8',
|
||||
);
|
||||
expect(alias.replace(/^name:.*$/m, 'name: gstack')).toBe(sourceRootSkill);
|
||||
|
||||
const ogbAlias = fs.readFileSync(
|
||||
path.join(installDir, 'connect-chrome', 'SKILL.md'),
|
||||
'utf-8',
|
||||
);
|
||||
expect(ogbAlias.replace(/^name:.*$/m, 'name: open-gstack-browser')).toBe(sourceOgbSkill);
|
||||
});
|
||||
|
||||
test('the SOURCE files are byte-intact (E2: sed never wrote through a symlink)', () => {
|
||||
expect(fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8')).toBe(sourceRootSkill);
|
||||
expect(
|
||||
fs.readFileSync(path.join(ROOT, 'open-gstack-browser', 'SKILL.md'), 'utf-8'),
|
||||
).toBe(sourceOgbSkill);
|
||||
expect(frontmatterName(path.join(ROOT, 'SKILL.md'))).toBe('gstack');
|
||||
expect(frontmatterName(path.join(ROOT, 'open-gstack-browser', 'SKILL.md'))).toBe(
|
||||
'open-gstack-browser',
|
||||
);
|
||||
});
|
||||
|
||||
test('every installed skill name is globally unique', () => {
|
||||
const names: string[] = [];
|
||||
for (const entry of fs.readdirSync(installDir)) {
|
||||
const skillMd = path.join(installDir, entry, 'SKILL.md');
|
||||
if (!fs.existsSync(skillMd)) continue;
|
||||
const name = frontmatterName(skillMd);
|
||||
if (name) names.push(name);
|
||||
}
|
||||
expect(names.length).toBeGreaterThan(10);
|
||||
const dupes = names.filter((n, i) => names.indexOf(n) !== i);
|
||||
expect(dupes).toEqual([]);
|
||||
});
|
||||
|
||||
test('a legacy symlinked alias is replaced, not written through', () => {
|
||||
// Simulate a pre-fix install: alias SKILL.md is a symlink to the source.
|
||||
const legacyDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-alias-legacy-'));
|
||||
try {
|
||||
const aliasDir = path.join(legacyDir, '_gstack-command');
|
||||
fs.mkdirSync(aliasDir);
|
||||
fs.symlinkSync(path.join(ROOT, 'SKILL.md'), path.join(aliasDir, 'SKILL.md'));
|
||||
|
||||
const script = [
|
||||
'set -e',
|
||||
'IS_WINDOWS=0',
|
||||
extractFn('_link_or_copy'),
|
||||
extractFn('_install_alias_skill_md'),
|
||||
extractFn('link_claude_root_skill_alias'),
|
||||
`link_claude_root_skill_alias "${ROOT}" "${legacyDir}"`,
|
||||
].join('\n');
|
||||
const result = spawnSync('bash', ['-c', script], { encoding: 'utf-8', timeout: 30_000 });
|
||||
expect(result.status).toBe(0);
|
||||
|
||||
const aliasSkill = path.join(aliasDir, 'SKILL.md');
|
||||
expect(fs.lstatSync(aliasSkill).isSymbolicLink()).toBe(false);
|
||||
expect(frontmatterName(aliasSkill)).toBe('_gstack-command');
|
||||
// The source the legacy symlink pointed at is untouched.
|
||||
expect(fs.readFileSync(path.join(ROOT, 'SKILL.md'), 'utf-8')).toBe(sourceRootSkill);
|
||||
} finally {
|
||||
fs.rmSync(legacyDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user