mirror of https://github.com/garrytan/gstack.git
fix: stop the connect-chrome alias from shadowing open-gstack-browser
connect-chrome is a repo-level symlink to open-gstack-browser, but both got installed into ~/.claude/skills as separate directories carrying the same frontmatter `name: open-gstack-browser`. Claude Code keys skills on that name, so one shadowed the other and the survivor followed unsorted readdir order — leaving /open-gstack-browser unregistered on an arbitrary subset of machines, even though README.md, AGENTS.md, the router SKILL.md and the sidebar extension all point users at it. Two paths minted the collision: gstack-relink's discovery loop names entries from the directory basename (so the symlink became its own entry), and setup separately symlinked the alias directory at the canonical skill directory. Install the alias as a thin skill with its own name that delegates to the canonical skill. gstack-relink now owns it, so the duplicated blocks in setup are gone. A marker comment lets a later relink recognize and clean up an alias it generated, which keeps skill_prefix flips from leaving two aliases behind. Fixes #2201 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
notjbg
parent
11de390be1
commit
728466c0e8
|
|
@ -36,6 +36,10 @@ SKILLS_DIR="${GSTACK_SKILLS_DIR:-$(dirname "$INSTALL_DIR")}"
|
|||
# Read prefix setting
|
||||
PREFIX=$("$GSTACK_CONFIG" get skill_prefix 2>/dev/null || echo "false")
|
||||
|
||||
# Marker written into generated alias skills so a later relink can recognize
|
||||
# (and clean up) an alias it created earlier, even after a prefix flip.
|
||||
ALIAS_MARKER='<!-- gstack-generated-alias -->'
|
||||
|
||||
# Helper: remove old skill entry (symlink or real directory with symlinked SKILL.md)
|
||||
_cleanup_skill_entry() {
|
||||
local entry="$1"
|
||||
|
|
@ -43,6 +47,11 @@ _cleanup_skill_entry() {
|
|||
rm -f "$entry"
|
||||
elif [ -d "$entry" ] && [ -L "$entry/SKILL.md" ]; then
|
||||
rm -rf "$entry"
|
||||
elif [ -d "$entry" ] && [ -f "$entry/SKILL.md" ] && grep -qF "$ALIAS_MARKER" "$entry/SKILL.md" 2>/dev/null; then
|
||||
# A generated alias holds a real SKILL.md, not a symlink, so the branch
|
||||
# above never matches it. Without this, a prefix flip leaves the old alias
|
||||
# behind and both names stay registered.
|
||||
rm -rf "$entry"
|
||||
fi
|
||||
}
|
||||
|
||||
|
|
@ -55,6 +64,55 @@ _link_root_skill_alias() {
|
|||
ln -snf "$INSTALL_DIR/SKILL.md" "$target/SKILL.md"
|
||||
}
|
||||
|
||||
# Backwards-compat alias: /connect-chrome → /open-gstack-browser.
|
||||
#
|
||||
# The alias must NOT be a symlink to the canonical skill directory. Claude Code
|
||||
# keys skills on the frontmatter `name:`, so two directories sharing a name
|
||||
# shadow each other and the survivor follows unsorted readdir order — meaning
|
||||
# `/open-gstack-browser` (the name README, AGENTS.md, the router SKILL.md and
|
||||
# the sidebar extension all tell users to run) would be registered on some
|
||||
# machines and silently missing on others. Generate a thin alias skill with its
|
||||
# own `name:` instead, so both commands resolve, on every machine.
|
||||
_link_alias_skill() {
|
||||
local canonical="$1"
|
||||
local alias_name="$2"
|
||||
|
||||
[ -f "$INSTALL_DIR/$canonical/SKILL.md" ] || return 0
|
||||
|
||||
local description
|
||||
description=$(grep -m1 '^description:' "$INSTALL_DIR/$canonical/SKILL.md" 2>/dev/null |
|
||||
sed 's/^description:[[:space:]]*//')
|
||||
[ -n "$description" ] || description="Alias for /$canonical."
|
||||
|
||||
# Drop the alias entry belonging to the other prefix mode, so flipping
|
||||
# skill_prefix never leaves two alias skills registered at once.
|
||||
if [ "$PREFIX" = "true" ]; then
|
||||
_cleanup_skill_entry "$SKILLS_DIR/$alias_name"
|
||||
canonical="gstack-$canonical"
|
||||
alias_name="gstack-$alias_name"
|
||||
else
|
||||
_cleanup_skill_entry "$SKILLS_DIR/gstack-$alias_name"
|
||||
fi
|
||||
|
||||
local target="$SKILLS_DIR/$alias_name"
|
||||
if [ -L "$target" ]; then rm -f "$target"; fi
|
||||
mkdir -p "$target"
|
||||
if [ -L "$target/SKILL.md" ]; then rm -f "$target/SKILL.md"; fi
|
||||
cat > "$target/SKILL.md" <<EOF
|
||||
---
|
||||
name: $alias_name
|
||||
description: Deprecated alias for /$canonical. $description
|
||||
---
|
||||
$ALIAS_MARKER
|
||||
|
||||
# /$alias_name (deprecated alias)
|
||||
|
||||
Kept for backwards compatibility. The skill itself lives at \`/$canonical\`.
|
||||
|
||||
Invoke the \`$canonical\` skill and follow it exactly. Do not restate its steps here.
|
||||
EOF
|
||||
}
|
||||
|
||||
_link_root_skill_alias
|
||||
|
||||
# Discover skills (directories with SKILL.md, excluding meta dirs)
|
||||
|
|
@ -62,8 +120,10 @@ SKILL_COUNT=0
|
|||
for skill_dir in "$INSTALL_DIR"/*/; do
|
||||
[ -d "$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
|
||||
# Skip non-skill directories. `connect-chrome` is a repo-level symlink to
|
||||
# open-gstack-browser; it is installed by _link_alias_skill below, not here,
|
||||
# so that it gets its own frontmatter name instead of the canonical one.
|
||||
case "$skill" in bin|browse|connect-chrome|design|docs|extension|lib|node_modules|scripts|test|.git|.github) continue ;; esac
|
||||
[ -f "$skill_dir/SKILL.md" ] || continue
|
||||
|
||||
if [ "$PREFIX" = "true" ]; then
|
||||
|
|
@ -91,6 +151,10 @@ for skill_dir in "$INSTALL_DIR"/*/; do
|
|||
SKILL_COUNT=$((SKILL_COUNT + 1))
|
||||
done
|
||||
|
||||
# Alias skills, installed after the loop so they overwrite any stale entry a
|
||||
# previous relink symlinked into place.
|
||||
_link_alias_skill "open-gstack-browser" "connect-chrome"
|
||||
|
||||
# Patch SKILL.md name: fields to match prefix setting
|
||||
"$INSTALL_DIR/bin/gstack-patch-names" "$INSTALL_DIR" "$PREFIX"
|
||||
|
||||
|
|
|
|||
22
setup
22
setup
|
|
@ -546,6 +546,10 @@ link_claude_skill_dirs() {
|
|||
dir_name="$(basename "$skill_dir")"
|
||||
# Skip node_modules
|
||||
[ "$dir_name" = "node_modules" ] && continue
|
||||
# Skip the connect-chrome alias symlink — gstack-relink installs it as a
|
||||
# thin skill with its own name. Linking it here would resolve its
|
||||
# frontmatter to open-gstack-browser and collide with the canonical skill.
|
||||
[ "$dir_name" = "connect-chrome" ] && continue
|
||||
# Use frontmatter name: if present (e.g., run-tests/ with name: test → symlink as "test")
|
||||
skill_name=$(grep -m1 '^name:' "$skill_dir/SKILL.md" 2>/dev/null | sed 's/^name:[[:space:]]*//' | tr -d '[:space:]')
|
||||
[ -z "$skill_name" ] && skill_name="$dir_name"
|
||||
|
|
@ -996,18 +1000,12 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then
|
|||
# Self-healing: re-run gstack-relink to ensure name: fields and directory
|
||||
# names are consistent with the config. This catches cases where an interrupted
|
||||
# setup, stale git state, or gen:skill-docs left name: fields out of sync.
|
||||
# gstack-relink also installs the /connect-chrome → /open-gstack-browser
|
||||
# backwards-compat alias as a thin skill with its own frontmatter name.
|
||||
GSTACK_RELINK="$SOURCE_GSTACK_DIR/bin/gstack-relink"
|
||||
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
|
||||
# Backwards-compat alias: /connect-chrome → /open-gstack-browser
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/connect-chrome"
|
||||
if [ "$SKILL_PREFIX" -eq 1 ]; then
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/gstack-connect-chrome"
|
||||
fi
|
||||
if [ -L "$_OGB_LINK" ] || [ ! -e "$_OGB_LINK" ]; then
|
||||
_link_or_copy "gstack/open-gstack-browser" "$_OGB_LINK"
|
||||
fi
|
||||
if [ "$LOCAL_INSTALL" -eq 1 ]; then
|
||||
log "gstack ready (project-local)."
|
||||
log " skills: $INSTALL_SKILLS_DIR"
|
||||
|
|
@ -1065,17 +1063,11 @@ if [ "$INSTALL_CLAUDE" -eq 1 ]; then
|
|||
"$SOURCE_GSTACK_DIR/bin/gstack-patch-names" "$SOURCE_GSTACK_DIR" "$SKILL_PREFIX"
|
||||
link_claude_skill_dirs "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR"
|
||||
link_claude_root_skill_alias "$SOURCE_GSTACK_DIR" "$INSTALL_SKILLS_DIR"
|
||||
# As above: gstack-relink installs the /connect-chrome alias.
|
||||
GSTACK_RELINK="$SOURCE_GSTACK_DIR/bin/gstack-relink"
|
||||
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
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/connect-chrome"
|
||||
if [ "$SKILL_PREFIX" -eq 1 ]; then
|
||||
_OGB_LINK="$INSTALL_SKILLS_DIR/gstack-connect-chrome"
|
||||
fi
|
||||
if [ -L "$_OGB_LINK" ] || [ ! -e "$_OGB_LINK" ]; then
|
||||
_link_or_copy "gstack/open-gstack-browser" "$_OGB_LINK"
|
||||
fi
|
||||
log "gstack ready (claude)."
|
||||
log " browse: $BROWSE_BIN"
|
||||
fi
|
||||
|
|
|
|||
|
|
@ -470,6 +470,119 @@ describe('upgrade migrations', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// The connect-chrome alias used to be a symlink to the open-gstack-browser
|
||||
// skill directory, so both entries carried `name: open-gstack-browser`. Claude
|
||||
// Code keys skills on that frontmatter name, so one shadowed the other and the
|
||||
// survivor followed unsorted readdir order — `/open-gstack-browser` was missing
|
||||
// on some machines even though README/AGENTS.md/the extension all reference it.
|
||||
describe('connect-chrome alias does not collide with open-gstack-browser', () => {
|
||||
// Recreate the repo layout: a canonical skill plus the tracked symlink.
|
||||
function setupWithAlias(): void {
|
||||
setupMockInstall(['qa', 'open-gstack-browser']);
|
||||
fs.symlinkSync('open-gstack-browser', path.join(installDir, 'connect-chrome'));
|
||||
}
|
||||
|
||||
function relink(): void {
|
||||
run(`${path.join(installDir, 'bin', 'gstack-relink')}`, {
|
||||
GSTACK_INSTALL_DIR: installDir,
|
||||
GSTACK_SKILLS_DIR: skillsDir,
|
||||
});
|
||||
}
|
||||
|
||||
function setPrefix(value: 'true' | 'false'): void {
|
||||
run(`${path.join(installDir, 'bin', 'gstack-config')} set skill_prefix ${value}`, {
|
||||
GSTACK_INSTALL_DIR: installDir,
|
||||
GSTACK_SKILLS_DIR: skillsDir,
|
||||
});
|
||||
}
|
||||
|
||||
function frontmatterName(dir: string): string | null {
|
||||
const m = fs
|
||||
.readFileSync(path.join(skillsDir, dir, 'SKILL.md'), 'utf-8')
|
||||
.match(/^name:\s*(.+)$/m);
|
||||
return m ? m[1].trim() : null;
|
||||
}
|
||||
|
||||
// The core invariant: no two installed skills may declare the same name.
|
||||
function expectNoDuplicateNames(): void {
|
||||
const names = fs
|
||||
.readdirSync(skillsDir)
|
||||
.filter((d) => fs.existsSync(path.join(skillsDir, d, 'SKILL.md')))
|
||||
.map(frontmatterName);
|
||||
expect(names.length).toBe(new Set(names).size);
|
||||
}
|
||||
|
||||
test('each installed skill declares a unique frontmatter name', () => {
|
||||
setupWithAlias();
|
||||
setPrefix('false');
|
||||
relink();
|
||||
|
||||
expect(frontmatterName('open-gstack-browser')).toBe('open-gstack-browser');
|
||||
expect(frontmatterName('connect-chrome')).toBe('connect-chrome');
|
||||
expectNoDuplicateNames();
|
||||
});
|
||||
|
||||
test('the alias is a real file, not a symlink into the canonical skill', () => {
|
||||
setupWithAlias();
|
||||
setPrefix('false');
|
||||
relink();
|
||||
|
||||
const aliasSkill = path.join(skillsDir, 'connect-chrome', 'SKILL.md');
|
||||
expect(fs.lstatSync(aliasSkill).isSymbolicLink()).toBe(false);
|
||||
// It still points the agent at the canonical skill rather than duplicating it.
|
||||
expect(fs.readFileSync(aliasSkill, 'utf-8')).toContain('open-gstack-browser');
|
||||
});
|
||||
|
||||
test('flipping skill_prefix leaves exactly one alias behind', () => {
|
||||
setupWithAlias();
|
||||
setPrefix('false');
|
||||
relink();
|
||||
setPrefix('true');
|
||||
relink();
|
||||
|
||||
expect(fs.existsSync(path.join(skillsDir, 'gstack-connect-chrome'))).toBe(true);
|
||||
expect(fs.existsSync(path.join(skillsDir, 'connect-chrome'))).toBe(false);
|
||||
expect(frontmatterName('gstack-connect-chrome')).toBe('gstack-connect-chrome');
|
||||
expectNoDuplicateNames();
|
||||
|
||||
// ...and flipping back restores the flat names with no leftovers.
|
||||
setPrefix('false');
|
||||
relink();
|
||||
expect(fs.existsSync(path.join(skillsDir, 'gstack-connect-chrome'))).toBe(false);
|
||||
expect(frontmatterName('connect-chrome')).toBe('connect-chrome');
|
||||
expectNoDuplicateNames();
|
||||
});
|
||||
|
||||
test('relink is idempotent for the alias', () => {
|
||||
setupWithAlias();
|
||||
setPrefix('false');
|
||||
relink();
|
||||
relink();
|
||||
relink();
|
||||
|
||||
expect(frontmatterName('connect-chrome')).toBe('connect-chrome');
|
||||
expectNoDuplicateNames();
|
||||
});
|
||||
|
||||
// Upgrade path: installs created before this fix have the alias as a symlink.
|
||||
test('upgrades a pre-existing symlinked alias in place', () => {
|
||||
setupWithAlias();
|
||||
setPrefix('false');
|
||||
// `gstack-config set` auto-relinks, so roll the alias back to the old
|
||||
// shape: a bare symlink pointing at the canonical skill directory.
|
||||
const alias = path.join(skillsDir, 'connect-chrome');
|
||||
fs.rmSync(alias, { recursive: true, force: true });
|
||||
fs.symlinkSync(path.join(installDir, 'open-gstack-browser'), alias);
|
||||
expect(fs.lstatSync(alias).isSymbolicLink()).toBe(true);
|
||||
|
||||
relink();
|
||||
|
||||
expect(fs.lstatSync(path.join(skillsDir, 'connect-chrome')).isSymbolicLink()).toBe(false);
|
||||
expect(frontmatterName('connect-chrome')).toBe('connect-chrome');
|
||||
expectNoDuplicateNames();
|
||||
});
|
||||
});
|
||||
|
||||
describe('gstack-patch-names (#620/#578)', () => {
|
||||
// Helper to read name: from SKILL.md frontmatter
|
||||
function readSkillName(skillDir: string): string | null {
|
||||
|
|
|
|||
Loading…
Reference in New Issue