From 728466c0e8404d384aef3b349d258f9f51cb5aa1 Mon Sep 17 00:00:00 2001 From: notjbg Date: Wed, 8 Jul 2026 17:08:19 -0700 Subject: [PATCH] fix: stop the connect-chrome alias from shadowing open-gstack-browser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- bin/gstack-relink | 68 +++++++++++++++++++++++++- setup | 22 +++------ test/relink.test.ts | 113 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 186 insertions(+), 17 deletions(-) diff --git a/bin/gstack-relink b/bin/gstack-relink index dd2a681fa..ff1c8ea0a 100755 --- a/bin/gstack-relink +++ b/bin/gstack-relink @@ -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='' + # 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" </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 diff --git a/test/relink.test.ts b/test/relink.test.ts index d83c4cd37..226af5907 100644 --- a/test/relink.test.ts +++ b/test/relink.test.ts @@ -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 {