From 85d53d4eb6ce132a7451d90e019e65a1dcb81293 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Mon, 19 Oct 2026 13:12:52 -0400 Subject: [PATCH 1/4] Add configurable skill prefixes with safe install reconciliation --- .agents/skills/manage-e-stack/SKILL.md | 8 +- .../manage-e-stack/scripts/preflight.sh | 51 +- .github/workflows/installer.yml | 20 + .github/workflows/publish.yml | 7 +- CHANGELOG.md | 12 + README.md | 5 +- bin/install.cjs | 486 +++++++++++++++--- docs/publishing.md | 4 +- docs/skill-authoring.md | 34 +- scripts/test-install-edge-cases.cjs | 84 ++- scripts/test-prefix-safety.cjs | 110 ++++ 11 files changed, 690 insertions(+), 131 deletions(-) create mode 100644 .github/workflows/installer.yml create mode 100644 scripts/test-prefix-safety.cjs diff --git a/.agents/skills/manage-e-stack/SKILL.md b/.agents/skills/manage-e-stack/SKILL.md index 6298ba7..1f138b0 100644 --- a/.agents/skills/manage-e-stack/SKILL.md +++ b/.agents/skills/manage-e-stack/SKILL.md @@ -1,5 +1,6 @@ --- name: manage-e-stack +version: 1.1.0 description: "MUST USE for any work on the e-stack repo — adding, editing, prepping, or publishing skills. Triggers: 'add a skill to e-stack', 'edit an estack skill', 'publish e-stack', 'release to npm', 'prep for release', 'get it ready to publish but don't publish', 'put this in e-stack', 'fix this skill', 'ship it', or any change to files under skills/ or package.json in this repo. Routes to the right step file based on intent." --- @@ -33,11 +34,12 @@ These apply to every route. Violating them breaks the install or publish. - **NEVER push a `v*` tag without intent to publish.** Any `v*` tag push runs a real npm release. - **Prep ≠ publish.** `steps/prep.md` leaves work release-ready (gates green, versions bumped, `[Unreleased]` entries written, committed and pushed) without releasing anything; the next `steps/publish.md` run sweeps up everything prepped. When the user says "don't publish yet" or other work is in flight, route to prep. - **Only the repo owner can push to `main`.** Branch protection requires PRs from everyone else. -- **Live install location:** `~/.agents/skills/estack-*/` (symlinked from `~/.claude/skills/estack-*/`; the installer copies from `skills/` here) +- **Live install location:** `~/.agents/skills//` (symlinked from `~/.claude/skills//`; the installer copies from `skills/` here) - **Renaming/removing a skill** needs no installer edit. Deleting the folder from `skills/` is enough: the installer compares its checksum manifest against what the package ships and retires anything it recorded installing that is gone, both on disk and in the manifest. `DEPRECATED_SKILLS` in `bin/install.cjs` is legacy-only — do NOT add to it. It covers two names removed before that existed, where an old install may have no manifest entry to prune. - **Hooks** live in `hooks/.js` at the repo root (flat — no subfolders). They are Claude Code-only: unlike skills, hooks cannot be installed into `~/.agents/` and discovered from there. The installer copies them to `~/.claude/hooks/` and registers each via a dedicated `setupHook(dryRun)` function in `bin/install.cjs` that idempotently patches `~/.claude/settings.json` (and honors `dryRun` by returning before writing). Hook scripts MUST wrap their body in `try { ... } catch { /* never break the tool */ }` and exit 0 — a hook crash must never break the underlying tool call. See `docs/hook-authoring.md`. -- **Installer:** `node bin/install.cjs` from repo root **dry-runs by default** (previews changes, writes nothing); add `--install` to actually sync. `--dry-run` forces a preview even under `npx`. Always pass `--yes` alongside `--install` when running it from an agent or a script: locally modified items trigger an interactive prompt that reads EOF on a non-TTY stdin and aborts the entire install. `--yes` answers it with "back up local changes, install the latest"; `--skip-modified` answers it with "keep local versions". Add `--no-statusline` to remove the statusline (`--statusline` restores it, `ESTACK_NO_STATUSLINE=1` skips it for one run). -- **Installer settings:** there is exactly ONE settings file, `~/.e-stack/.env`, the same one skills read API keys from. Installer settings are env vars in it: `ESTACK_SKILLS_DIR`, `ESTACK_HOOKS_DIR`, `ESTACK_BACKUP_DIR`, `ESTACK_NO_STATUSLINE`, plus `ESTACK_HOME` for the file's own location. Resolution is live environment first, then the file, matching every skill. Never add a second config file, a JSON sidecar, or a marker file — put the new setting in `.env` under an `ESTACK_*` name and write it with `writeSetting()`, which replaces or appends only that one line so shared credentials survive. Installer *state* is a different thing from settings: derive it from the existing checksum manifest (`~/.claude/.estack-checksums.json`) the way the statusline claim does, rather than persisting a new field. +- **Installer:** `node bin/install.cjs` from repo root **dry-runs by default** (previews changes, writes nothing); add `--install` to actually sync. `--dry-run` forces a preview even under `npx`. Always pass `--yes` alongside `--install` when running it from an agent or a script: locally modified items trigger an interactive prompt that reads EOF on a non-TTY stdin and aborts the entire install. `--yes` answers it with "back up local changes, install the latest"; `--skip-modified` answers it with "keep local versions". Add `--no-statusline` to remove the statusline (`--statusline` restores it, `ESTACK_NO_STATUSLINE=1` skips it for one run). Add `--no-prefix` to install skills under their short names (`--prefix` restores the `estack-` prefix, `ESTACK_SKILL_PREFIX=none` does the same from the settings file). +- **Skill name prefix:** skills always ship prefixed in `skills/`, and prefixed is the default install. `ESTACK_SKILL_PREFIX=none` makes the installer rewrite `estack-` to `` in every packaged file as it copies, so the folder, the `name:` line, cross-skill references, and self-referencing install paths all move together. The setting is read on every run including `--silent`/`--startup`, so changing it in `~/.e-stack/.env` reconciles an existing install at the next session start. That works because the checksum manifest is keyed by *installed* name — anything touching the destination or the manifest goes through `installedName()`, and only reads from `PACKAGE_SKILLS_DIR` use the package name. A prefix switch backs up old files and carries per-skill `.env` files, then retires the old name only after successful installation. Silent mode defers edited sources and foreign destinations; manual/startup installs back up foreign skills before replacement. Existing backup directories are retained with unique suffixes for new copies. Quoted standalone storage identifiers and variable-based state paths remain canonical. Package-side hashes are computed on the rewritten content (`computeSkillHash(dir, true)`), or every skill reads as locally modified forever. `scripts/check-skill-name.cjs` rule 5 is what makes the rewrite complete; do not loosen it. +- **Installer settings:** there is exactly ONE settings file, `~/.e-stack/.env`, the same one skills read API keys from. Installer settings are env vars in it: `ESTACK_SKILLS_DIR`, `ESTACK_HOOKS_DIR`, `ESTACK_BACKUP_DIR`, `ESTACK_NO_STATUSLINE`, `ESTACK_SKILL_PREFIX`, plus `ESTACK_HOME` for the file's own location. Resolution is live environment first, then the file, matching every skill. Never add a second config file, a JSON sidecar, or a marker file — put the new setting in `.env` under an `ESTACK_*` name and write it with `writeSetting()`, which replaces or appends only that one line so shared credentials survive. Installer *state* is a different thing from settings: derive it from the existing checksum manifest (`~/.claude/.estack-checksums.json`) the way the statusline claim does, rather than persisting a new field. - **Testing the installer:** `os.homedir()` follows `USERPROFILE`, so the whole installer runs against a throwaway home — `USERPROFILE= node bin/install.cjs --install --yes`. Use it for anything touching settings, paths, or `settings.json` wiring instead of experimenting on a real setup. - **Prefer deterministic changes.** Any time a change can be made exactly and reproducibly — via a shell command, an existing repo script, or a targeted diff (Edit tool) — use that instead of reading content and regenerating it. AI-generated output is never byte-for-byte identical to the source; transcription errors accumulate even on "trivial" tasks. Apply this to everything: - **File copies**: `cp -r` / `xcopy /E /I` / `robocopy /E`, not Read+Write diff --git a/.agents/skills/manage-e-stack/scripts/preflight.sh b/.agents/skills/manage-e-stack/scripts/preflight.sh index 5cf3378..13cac46 100644 --- a/.agents/skills/manage-e-stack/scripts/preflight.sh +++ b/.agents/skills/manage-e-stack/scripts/preflight.sh @@ -2,51 +2,10 @@ # Pre-flight diagnostics for e-stack publishing # Read-only — does not modify anything -# Real skill files live in ~/.agents/skills/ (symlinked from ~/.claude/skills/). -# Fall back to ~/.claude/skills/ for pre-migration installs. -INSTALL_ROOT="$HOME/.agents/skills" -[ -d "$INSTALL_ROOT" ] || INSTALL_ROOT="$HOME/.claude/skills" - -echo "=== Current installed skills (in $INSTALL_ROOT) ===" -for skill in "$INSTALL_ROOT"/estack-*/; do - [ -d "$skill" ] || continue - echo "$(basename "$skill"): $(find "$skill" -type f | wc -l) files" -done - -echo "" -echo "=== Repo skills ===" -for skill in skills/estack-*/; do - echo "$(basename "$skill")" -done - -echo "" -echo "=== Diffs between repo and installed ===" -diffs=0 -for skill in skills/estack-*/; do - name=$(basename "$skill") - if [ ! -d "$INSTALL_ROOT/$name" ]; then - echo "NEW: $name (not yet installed)" - diffs=$((diffs + 1)) - else - diff_output=$(diff -rq "$skill" "$INSTALL_ROOT/$name" 2>&1) - if [ -n "$diff_output" ]; then - echo "CHANGED: $name" - echo "$diff_output" - diffs=$((diffs + 1)) - fi - fi -done -for skill in "$INSTALL_ROOT"/estack-*/; do - [ -d "$skill" ] || continue - name=$(basename "$skill") - if [ ! -d "skills/$name" ]; then - echo "STALE: $name (installed but not in repo)" - diffs=$((diffs + 1)) - fi -done -if [ "$diffs" -eq 0 ]; then - echo "(all match)" -fi +# The installer resolves custom roots and prefix settings and compares the +# transformed package hashes, so unprefixed installs get accurate diagnostics. +echo "=== Installed skills and pending changes ===" +node bin/install.cjs --dry-run echo "" echo "=== Current installed hooks ===" @@ -142,7 +101,7 @@ echo "=== Version bump check (vs last release tag) ===" node scripts/check-versions.cjs || true echo "" -echo "=== Docs sync check (README.md / CLAUDE.md lists) ===" +echo "=== Docs sync check (README.md / AGENTS.md lists) ===" node scripts/check-docs.cjs || true echo "" diff --git a/.github/workflows/installer.yml b/.github/workflows/installer.yml new file mode 100644 index 0000000..1d735f7 --- /dev/null +++ b/.github/workflows/installer.yml @@ -0,0 +1,20 @@ +name: Installer regression tests +on: + pull_request: + push: + branches: [main] +permissions: + contents: read +jobs: + installer: + strategy: + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v6 + - uses: actions/setup-node@v6 + with: + node-version: "24" + - run: node scripts/test-install-edge-cases.cjs + - run: node scripts/test-prefix-safety.cjs diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index a3635b8..318842a 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -46,7 +46,7 @@ jobs: - name: Verify every skill's feedback section matches the template run: node scripts/update-skill-feedback.cjs --check - - name: Verify README and CLAUDE.md list every skill and hook + - name: Verify README and AGENTS.md list every skill and hook run: node scripts/check-docs.cjs - name: Verify skill names are converted correctly (estack- prefix, description, self-references) @@ -55,6 +55,11 @@ jobs: - name: Verify skills store state in ~/.e-stack and credentials in the shared .env run: node scripts/check-paths.cjs + - name: Verify installer regressions + run: | + node scripts/test-install-edge-cases.cjs + node scripts/test-prefix-safety.cjs + - name: Setup Python uses: actions/setup-python@v6 with: diff --git a/CHANGELOG.md b/CHANGELOG.md index 20e22ae..5d5bd78 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- Skill names can drop the `estack-` prefix. `ESTACK_SKILL_PREFIX=none` in `~/.e-stack/.env`, or installing with `--no-prefix`, installs every skill under its short name: `/email-writer` instead of `/estack-email-writer`. Prefixed stays the default and `--prefix` puts it back. The installer rewrites `estack-` to `` in every file as it copies, so the folder name, the `name:` line, cross-skill references in descriptions and bodies, and the self-referencing paths some skills use to run their own scripts all move together. Filenames inside a skill keep their own names — `estack-notify.ps1` still lives in `skills/notify/scripts/`. +- The setting is read on every install, including the `--silent` and `--startup` auto-updates. Change it in `~/.e-stack/.env` and the next session start reconciles an existing install to it: the names from the old setting are retired from `~/.agents/skills/` and `~/.claude/skills/`, the same skills are reinstalled under the new ones, and the `estack-notify` Stop hook in `settings.json` is repointed. Each completed switch backs up the old files and preserves credentials; silent mode defers switches requiring review. +- Short names like `notify` and `repo-search` are ordinary enough that a user may already have a skill on one. Installing unprefixed over a directory the installer never put there names it in a warning and copies it to `~/.estack-backup/` before replacing it, during manual or startup installs; silent updates preserve the collision for review. +- State directories keep their prefixed names whichever way the setting is set: a skill installed at `~/.agents/skills/flight-planner/` still reads and writes `~/.e-stack/estack-flight-planner/config.json`. `~/.e-stack/` is storage rather than a skill reference, so saved configs, caches, and credentials survive changing the setting, and the statusline keeps finding the flag file the notify skill writes. +- The installer distinguishes a skill leaving the pack from a skill changing its name. Retiring 24 skills to rename them now reads `Retired: estack-notify (reinstalling as notify)` followed by a line saying the prefix setting is what moved them, instead of 24 lines of `(no longer in the package)`. + +### Fixed +- Prefix switches preserve saved-state paths, back up the old skill without replacing earlier backups, and carry per-skill `.env` files forward. Old names remain available until the replacement installs successfully. +- Silent updates defer conflicting or locally edited prefix switches; startup updates identify foreign skill takeovers explicitly. Unowned deprecated short names are left alone. +- Dry runs no longer migrate legacy settings or change permissions. Conflicting prefix/statusline flags fail before writing anything. + --- ## [1.0.77] - 2026-09-05 diff --git a/README.md b/README.md index c38f357..9a82ba4 100644 --- a/README.md +++ b/README.md @@ -91,13 +91,16 @@ ESTACK_NO_STATUSLINE=1 | `ESTACK_HOOKS_DIR` | `~/.claude/hooks` | Where Claude hook scripts are installed | | `ESTACK_BACKUP_DIR` | `~/.estack-backup` | Where your local changes are backed up | | `ESTACK_NO_STATUSLINE` | unset | Set to `1` to remove the statusline | +| `ESTACK_SKILL_PREFIX` | `estack-` | Set to `none` to install skills under their short names | | `ESTACK_HOME` | `~/.e-stack` | Where this file itself lives (env-only) | Resolution order is the one every skill uses: the live process environment first, then the file. So exporting a variable overrides it for a single run without editing anything. Path values must be absolute; a relative one is ignored and the default applies. The installer only ever replaces or appends its own line, so your API keys are never disturbed. +**Skill names.** Skills install prefixed by default: `/estack-email-writer`, `/estack-repo-search`. Set `ESTACK_SKILL_PREFIX=none` (or install with `--no-prefix`) and they install as `/email-writer` and `/repo-search` instead, with every cross-reference inside the skills rewritten to match. Change the value later and the next session start reconciles an existing install to it — the old names are retired and the same skills are reinstalled under the new ones, after backing up the previous files and preserving per-skill `.env` files. The old name is retired only after its replacement installs successfully. Anything a skill saved stays put: state directories keep their prefixed names either way, so `/flight-planner` still reads the `~/.e-stack/estack-flight-planner/config.json` you already had. Two things to know before turning the prefix off: the short names are ordinary ones like `notify` and `repo-search`, so if you already have a skill of your own on one of them it gets copied to `~/.estack-backup/` and replaced (the installer names it in a warning first); and every doc here writes the prefixed form. Silent updates defer a switch when the source is edited or the destination belongs to another skill, preserving both and printing a diagnostic; run the manual installer to review it. Startup updates back up foreign skills before replacement and identify them separately from local edits. Existing backup directories are retained; newer copies get unique suffixes. + **Statusline.** E-Stack ships a statusline showing the model, tokens used in the current context window, the project folder, rate limits, and an `estack-notify` bell. A first install claims the `statusLine` slot even if you already have one configured, so you can see what it does; your previous value is saved to `~/.estack-backup/statusLine.json`. From then on it is only updated while it is still E-Stack's. Switch back to your own or clear it, and no later update touches it again. `--no-statusline` removes it outright and `--statusline` brings it back. -**Installer flags.** `--install` applies changes (the installer previews by default when run from a clone). `--yes` answers the locally-modified prompt with "back up my changes, install the latest" and `--skip-modified` answers it with "keep my versions" — pass one of them from any script or CI job, since the prompt otherwise reads EOF on a non-interactive stdin and aborts. `--statusline` / `--no-statusline` turn the statusline on or off and remember the choice. +**Installer flags.** Opposing flags such as `--prefix --no-prefix` are rejected before any changes. `--dry-run` never migrates settings or writes files, even when combined with `--startup` or `--silent`. `--install` applies changes (the installer previews by default when run from a clone). `--yes` answers the locally-modified prompt with "back up my changes, install the latest" and `--skip-modified` answers it with "keep my versions" — pass one of them from any script or CI job, since the prompt otherwise reads EOF on a non-interactive stdin and aborts. `--statusline` / `--no-statusline` turn the statusline on or off and remember the choice, and `--prefix` / `--no-prefix` do the same for the `estack-` prefix on skill names. ## Requirements diff --git a/bin/install.cjs b/bin/install.cjs index 85805dc..accafcb 100644 --- a/bin/install.cjs +++ b/bin/install.cjs @@ -6,6 +6,20 @@ const path = require('path'); const crypto = require('crypto'); const os = require('os'); const readline = require('readline'); +const { isUtf8 } = require('buffer'); + +const IS_LOCAL = !__dirname.includes('node_modules'); +const DRY_RUN = process.argv.includes('--dry-run') || + (IS_LOCAL && !process.argv.includes('--install')); +const SILENT = process.argv.includes('--silent') && !DRY_RUN; +const STARTUP = process.argv.includes('--startup') && !DRY_RUN; +for (const [positive, negative] of [['--prefix', '--no-prefix'], ['--statusline', '--no-statusline']]) { + if (process.argv.includes(positive) && process.argv.includes(negative)) { + console.error('Conflicting flags: ' + positive + ' and ' + negative); + process.exit(1); + } +} + // ── Settings ─────────────────────────────────────────────────────────────── // The pack already has exactly one place a user configures things: @@ -69,6 +83,11 @@ function secureEnvFile() { } function writeSetting(name, value) { + if (DRY_RUN) { + if (value === null) delete ENV_FILE_VALUES[name]; + else ENV_FILE_VALUES[name] = value; + return; + } fs.mkdirSync(ESTACK_DIR, { recursive: true, mode: 0o700 }); let lines = []; try { @@ -94,7 +113,7 @@ function writeSetting(name, value) { // Existing installs created this file before it was owner-only, and most runs // never write a setting, so tighten it on every run rather than only on write. -if (fs.existsSync(ENV_FILE)) secureEnvFile(); +if (!DRY_RUN && fs.existsSync(ENV_FILE)) secureEnvFile(); // Fold the two short-lived predecessors of this file into it. (function migrateLegacySettings() { @@ -103,7 +122,7 @@ if (fs.existsSync(ENV_FILE)) secureEnvFile(); try { if (fs.existsSync(marker)) { writeSetting('ESTACK_NO_STATUSLINE', '1'); - fs.rmSync(marker, { force: true }); + if (!DRY_RUN) fs.rmSync(marker, { force: true }); } if (fs.existsSync(configFile)) { const cfg = JSON.parse(fs.readFileSync(configFile, 'utf8')); @@ -116,7 +135,7 @@ if (fs.existsSync(ENV_FILE)) secureEnvFile(); const value = cfg.paths && cfg.paths[key]; if (value) writeSetting(name, value); } - fs.rmSync(configFile, { force: true }); + if (!DRY_RUN) fs.rmSync(configFile, { force: true }); } } catch (_) { /* not worth failing an install over */ } })(); @@ -335,15 +354,120 @@ function ensureSymlink(target, linkPath) { fs.symlinkSync(target, linkPath, type); } -// ── Flags ────────────────────────────────────────────────────────────────── -const SILENT = process.argv.includes('--silent'); -const STARTUP = process.argv.includes('--startup'); -// When run directly from the repo (not via npx/node_modules), default to dry-run -// so local testing never silently clobbers the live ~/.claude/skills install. -// Pass --install to actually write files, or --dry-run to force preview mode. -const IS_LOCAL = !__dirname.includes('node_modules'); -const DRY_RUN = process.argv.includes('--dry-run') || - (IS_LOCAL && !process.argv.includes('--install')); +// ── Skill name prefix ────────────────────────────────────────────────────── +// The package always ships skills prefixed (skills/estack-/), and that +// is what installs by default. ESTACK_SKILL_PREFIX=none installs them under +// their short names instead: ~/.agents/skills/notify/, /notify, and every +// cross-reference inside the skills rewritten to match. +// +// --no-prefix install short names, and write ESTACK_SKILL_PREFIX=none +// --prefix install prefixed, and clear the setting +// ESTACK_SKILL_PREFIX=none the same, for one run or permanently via the file +// +// Read fresh on every run — including the --silent and --startup auto-update +// paths — so editing ~/.e-stack/.env and starting a new session reconciles an +// existing install to whichever naming the file now asks for. The rename is +// not a one-way door: the checksum manifest is keyed by installed name, so the +// names from the previous mode are migration sources. They are backed up and +// retired only after the replacement installs successfully. +const SKILL_PREFIX = 'estack-'; + +function prefixEnabled() { + if (process.argv.includes('--prefix')) return true; + if (process.argv.includes('--no-prefix')) return false; + return String(setting('ESTACK_SKILL_PREFIX') || '').trim().toLowerCase() !== 'none'; +} + +const PREFIXED = prefixEnabled(); + +// The name a package skill installs under. +function installedName(packageName) { + if (PREFIXED || !packageName.startsWith(SKILL_PREFIX)) return packageName; + return packageName.slice(SKILL_PREFIX.length); +} + +// Persists an explicit --no-prefix / --prefix choice so later auto-updates keep +// it. A bare ESTACK_SKILL_PREFIX in the environment is honored for the run but +// deliberately not written to the file, matching --no-statusline. +function persistPrefixChoice(dryRun) { + const optOut = process.argv.includes('--no-prefix'); + const optIn = process.argv.includes('--prefix'); + if (!optOut && !optIn) return; + if (dryRun) return; + writeSetting('ESTACK_SKILL_PREFIX', optOut ? 'none' : null); +} + +// Matches a reference to one of this package's skills by its full prefixed +// name, so unprefixed mode can rewrite `estack-notify` to `notify` everywhere +// it appears: the `name:` line in frontmatter, cross-skill mentions in prose, +// slash commands in descriptions, and the self-referencing install paths +// several skills use to run their own scripts (estack-book-extractor's +// SKILL.md shells out to ~/.claude/skills/estack-book-extractor/scripts/). +// +// Three exclusions keep it off things that are not skill references: +// (? e.isDirectory() && e.name.startsWith(SKILL_PREFIX)) + .map((e) => e.name.slice(SKILL_PREFIX.length)) + // Longest first so estack-prompt-builder-coach never matches as + // estack-prompt-builder with a stray "-coach" left behind. + .sort((a, b) => b.length - a.length); + } catch (_) { /* handled by the caller's own missing-skills-dir check */ } + RENAME_PATTERN = shortNames.length === 0 ? null : new RegExp( + '(? n.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')).join('|') + ')' + + '(?![-\\w])(?!\\.[A-Za-z0-9]+)', + 'g' + ); + return RENAME_PATTERN; +} + +// Rewrites skill references in one packaged file. A no-op in prefixed mode, so +// a default install runs byte-for-byte the code path it always has. Binary +// files are passed through untouched. +function renameReferences(buffer) { + if (PREFIXED) return buffer; + const pattern = renamePattern(); + if (pattern === null) return buffer; + if (buffer.includes(0) || !isUtf8(buffer)) return buffer; + const text = buffer.toString('utf8'); + const renamed = text.replace(pattern, (match, short, offset) => { + const before = text.slice(0, offset); + const after = text.slice(offset + match.length); + // Quoted bare names in executable code are storage keys or identifiers. + // Rewrite only when the preceding path segment explicitly names skills. + if (/['"]$/.test(before) && /^['"]/.test(after) && + !/skills['"]?\s*(?:,|\/)\s*['"]$/.test(before)) return match; + // Variable-based state paths ($ESTACK_ROOT/estack-...) also stay stable. + if (/[A-Za-z0-9_$}.][\\/]$/.test(before) && !/skills[\\/]$/.test(before)) return match; + return short; + }); + return renamed === text ? buffer : Buffer.from(renamed, 'utf8'); +} // ── Deprecated skills ────────────────────────────────────────────────────── // Skills that were renamed or removed. The installer removes these on every @@ -378,17 +502,23 @@ function walkDir(dir, base) { const full = path.join(dir, entry.name); if (entry.isDirectory()) { if (!HASH_IGNORE_DIRS.has(entry.name)) files.push(...walkDir(full, base)); - } else if (!HASH_IGNORE_EXTS.has(path.extname(entry.name))) { + } else if (!USER_DATA_FILENAMES.has(entry.name) && !HASH_IGNORE_EXTS.has(path.extname(entry.name))) { files.push(path.relative(base, full)); } } return files; } -function computeFileHash(filePath) { +// `fromPackage` hashes what the file will look like once installed rather than +// as it sits in the package, so an unprefixed install's on-disk content still +// matches its stored checksum and does not read as locally modified forever. +// Installed files are hashed as-is: a user who hand-wrote a prefixed name back +// into an unprefixed skill has modified it, and should be told so. +function computeFileHash(filePath, fromPackage) { if (!fs.existsSync(filePath)) return null; const hash = crypto.createHash('sha256'); - const raw = fs.readFileSync(filePath); + let raw = fs.readFileSync(filePath); + if (fromPackage) raw = renameReferences(raw); hash.update(Buffer.from(raw.toString('utf8').replace(/\r\n/g, '\n'))); return hash.digest('hex'); } @@ -402,7 +532,7 @@ function syncStartupHookSources(dryRun) { const source = path.join(PACKAGE_HOOKS_DIR, filename); const destination = path.join(AGENTS_HOOKS_DIR, filename); if (!fs.existsSync(source)) continue; - const sourceHash = computeFileHash(source); + const sourceHash = computeFileHash(source, true); const destinationHash = computeFileHash(destination); if (sourceHash === destinationHash) continue; if (!dryRun) { @@ -412,14 +542,14 @@ function syncStartupHookSources(dryRun) { fs.mkdirSync(path.dirname(backupDestination), { recursive: true }); fs.copyFileSync(destination, backupDestination); } - fs.copyFileSync(source, destination); + copyPackageFile(source, destination); } updated.push(filename); } return updated; } -function computeSkillHash(skillDir) { +function computeSkillHash(skillDir, fromPackage) { // statSync (not lstat) so symlinked dirs hash their contents; plain files → null try { if (!fs.statSync(skillDir).isDirectory()) return null; @@ -430,7 +560,8 @@ function computeSkillHash(skillDir) { const files = walkDir(skillDir, skillDir); for (const relPath of files) { const fullPath = path.join(skillDir, relPath); - const raw = fs.readFileSync(fullPath); + let raw = fs.readFileSync(fullPath); + if (fromPackage) raw = renameReferences(raw); hash.update(relPath.replace(/\\/g, '/')); hash.update(Buffer.from(raw.toString('utf8').replace(/\r\n/g, '\n'))); } @@ -444,12 +575,50 @@ function copyDir(src, dest) { fs.cpSync(src, dest, { recursive: true }); } +// Copies one packaged file, rewriting skill references on the way through. +// The mode is carried over by hand because the content is rewritten rather +// than copied, and several skills ship executable .sh scripts. +function copyPackageFile(src, dest) { + if (PREFIXED) { + fs.copyFileSync(src, dest); + return; + } + fs.writeFileSync(dest, renameReferences(fs.readFileSync(src))); + try { fs.chmodSync(dest, fs.statSync(src).mode); } catch (_) {} +} + +// copyDir for a packaged skill. In prefixed mode this is copyDir exactly. +function copyPackageDir(src, dest) { + if (PREFIXED) { + copyDir(src, dest); + return; + } + if (fs.existsSync(dest)) { + fs.rmSync(dest, { recursive: true, force: true }); + } + fs.mkdirSync(dest, { recursive: true }); + for (const entry of fs.readdirSync(src, { withFileTypes: true })) { + const from = path.join(src, entry.name); + const to = path.join(dest, entry.name); + if (entry.isDirectory()) copyPackageDir(from, to); + else copyPackageFile(from, to); + } +} + +// Takes an installed name — the backup mirrors what is actually on disk. function backupSkill(name) { - const agentsDir = path.join(AGENTS_DIR, name); - const installedDir = fs.existsSync(agentsDir) ? agentsDir : path.join(SKILLS_DIR, name); - if (!fs.existsSync(installedDir)) return; - fs.mkdirSync(BACKUP_DIR, { recursive: true }); - copyDir(installedDir, path.join(BACKUP_DIR, name)); + const seen = new Set(); + for (const root of [AGENTS_DIR, SKILLS_DIR]) { + const installedDir = path.join(root, name); + if (!fs.existsSync(installedDir)) continue; + const real = fs.realpathSync(installedDir); + if (seen.has(real)) continue; + seen.add(real); + fs.mkdirSync(BACKUP_DIR, { recursive: true }); + const base = path.join(BACKUP_DIR, name); + const destination = fs.existsSync(base) ? base + '-' + crypto.randomUUID() : base; + fs.cpSync(installedDir, destination, { recursive: true, dereference: true }); + } } function backupHook(filename) { @@ -527,6 +696,7 @@ function getSkillVersion(skillDir) { // Version of the currently installed copy of a skill (agents dir, falling // back to the legacy skills dir for pre-migration installs). +// Takes an installed name. function getInstalledSkillVersion(name) { const agentsDir = path.join(AGENTS_DIR, name); if (fs.existsSync(agentsDir)) return getSkillVersion(agentsDir); @@ -569,20 +739,55 @@ function collectUserDataFiles(dir) { // Copies a skill to ~/.agents/skills/ and creates/updates the symlink at ~/.claude/skills/. // If a real (non-symlink) directory already exists at the skills path, it is removed first. // User-owned files (e.g. .env) present in the installed copy are preserved across the update. +const prefixMigrations = new Map(); +const installedThisRun = new Set(); +let manifestBeforeInstall = {}; + function installSkillFiles(name) { - const agentsSkillDir = path.join(AGENTS_DIR, name); - const skillsLinkDir = path.join(SKILLS_DIR, name); + const installed = installedName(name); + const agentsSkillDir = path.join(AGENTS_DIR, installed); + const skillsLinkDir = path.join(SKILLS_DIR, installed); + const migration = prefixMigrations.get(installed); + const sourceDir = migration && migration.directory; + const legacyDir = isRealDir(skillsLinkDir) ? skillsLinkDir : null; + const userDataFiles = collectUserDataFiles(sourceDir || agentsSkillDir); + const oldLegacyDir = migration && path.join(SKILLS_DIR, migration.oldName); + for (const dir of [oldLegacyDir, legacyDir, agentsSkillDir]) { + if (!dir || !isRealDir(dir)) continue; + for (const [rel, content] of collectUserDataFiles(dir)) { + if (userDataFiles.has(rel) && !userDataFiles.get(rel).equals(content)) { + throw new Error('Conflicting user data at ' + rel + '; both skill copies were preserved'); + } + userDataFiles.set(rel, content); + } + } + if (migration) backupSkill(migration.oldName); + if (claimsForeignName(name, manifestBeforeInstall)) backupSkill(installed); if (!isSymlink(skillsLinkDir) && fs.existsSync(skillsLinkDir)) { fs.rmSync(skillsLinkDir, { recursive: true, force: true }); } - const userDataFiles = collectUserDataFiles(agentsSkillDir); - copyDir(path.join(PACKAGE_SKILLS_DIR, name), agentsSkillDir); + copyPackageDir(path.join(PACKAGE_SKILLS_DIR, name), agentsSkillDir); for (const [rel, content] of userDataFiles) { const dest = path.join(agentsSkillDir, rel); fs.mkdirSync(path.dirname(dest), { recursive: true }); fs.writeFileSync(dest, content); } ensureSymlink(agentsSkillDir, skillsLinkDir); + installedThisRun.add(installed); +} + +// Unprefixed mode claims short names — `notify`, `repo-search`, `better-title` +// — that a user may already be using for a skill of their own. The manifest +// records only what this installer put on disk, so an existing directory with +// no entry in it belongs to somebody else. It is copied into the backup dir +// before being replaced, and the name is returned so the caller can say out +// loud whose skill just got taken over. +function claimsForeignName(name, storedChecksums) { + const installed = installedName(name); + if (storedChecksums[installed]) return null; // ours already + const target = path.join(AGENTS_DIR, installed); + if (!isRealDir(target) && !isRealDir(path.join(SKILLS_DIR, installed))) return null; + return installed; } // ── Hook setup ───────────────────────────────────────────────────────────── @@ -773,7 +978,10 @@ function setupRepoSearchNudgeHook(dryRun) { // (it is PowerShell, and hooks/ is flat .js only), so it is wired from the // skill's installed location rather than from HOOKS_DIR. function setupNotifyStopHook(dryRun) { - const script = path.join(AGENTS_DIR, 'estack-notify', 'scripts', 'estack-notify-stop.ps1') + // The script's own filename never changes; only the skill folder around it + // does, when ESTACK_SKILL_PREFIX=none. Flipping the setting repoints this + // entry in place via the mentionsScript match below. + const script = path.join(AGENTS_DIR, installedName('estack-notify'), 'scripts', 'estack-notify-stop.ps1') .replace(/\\/g, '/'); // The args form avoids quoting a Windows path inside a command string, and it // is the shape Claude Code writes itself. @@ -898,10 +1106,19 @@ async function main() { ? (() => { try { return JSON.parse(fs.readFileSync(CHECKSUMS_FILE, 'utf8')); } catch (_) { return {}; } })() : {}; let changed = false; + // Both spellings: an install that has been flipped to unprefixed carries + // these under their short names. + const deprecated = []; for (const name of DEPRECATED_SKILLS) { + deprecated.push(name); + const short = installedName(name); + if (short !== name && newChecksums0[short]) deprecated.push(short); + } + for (const name of deprecated) { const agentsDir = path.join(AGENTS_DIR, name); const skillsDir = path.join(SKILLS_DIR, name); let found = false; + if (!DRY_RUN && (fs.existsSync(agentsDir) || isRealDir(skillsDir))) backupSkill(name); if (fs.existsSync(agentsDir)) { if (!DRY_RUN) fs.rmSync(agentsDir, { recursive: true, force: true }); found = true; @@ -947,14 +1164,21 @@ async function main() { process.exit(0); } - // 2. Compute hashes for package skills + // The name each skill installs under. Everything past this point that + // touches disk or the checksum manifest goes through installedName(); only + // reads from PACKAGE_SKILLS_DIR use the package name. + const installNames = skillNames.map(installedName); + + // 2. Compute hashes for package skills — of the installed form, so an + // unprefixed install's on-disk content matches its stored checksum. const packageHashes = {}; for (const name of skillNames) { - packageHashes[name] = computeSkillHash(path.join(PACKAGE_SKILLS_DIR, name)); + packageHashes[name] = computeSkillHash(path.join(PACKAGE_SKILLS_DIR, name), true); } // 2b. Scan package hooks persistStatuslineChoice(DRY_RUN); + persistPrefixChoice(DRY_RUN); const skipStatusline = statuslineOptedOut(); const hookFilenames = (fs.existsSync(PACKAGE_HOOKS_DIR) ? fs.readdirSync(PACKAGE_HOOKS_DIR).filter((f) => f.endsWith('.js')).sort() @@ -969,7 +1193,7 @@ async function main() { const packageHookHashes = {}; for (const filename of hookFilenames) { - packageHookHashes[filename] = computeFileHash(path.join(PACKAGE_HOOKS_DIR, filename)); + packageHookHashes[filename] = computeFileHash(path.join(PACKAGE_HOOKS_DIR, filename), true); } const startupSourcesUpdated = syncStartupHookSources(DRY_RUN); @@ -1003,29 +1227,81 @@ async function main() { ? fs.readdirSync(PACKAGE_HOOKS_DIR) .filter((f) => f.endsWith('.js') && !STARTUP_HOOK_FILENAMES.includes(f)) : []; + // + // This is also what reconciles a change to ESTACK_SKILL_PREFIX. The manifest + // is keyed by installed name, so after a flip the 24 entries from the old + // naming become migration sources. Installation carries their user data and + // backs them up; finishPrefixMigrations retires them only after a successful + // replacement. Silent updates defer migrations that require user review. const orphans = Object.keys(storedChecksums).filter((key) => ( key.startsWith('hook:') ? !packagedHooks.includes(key.slice(5)) - : !skillNames.includes(key) + : !installNames.includes(key) )); + // A skill leaves the manifest for two very different reasons, and saying + // "no longer in the package" for both is a lie the user acts on. If the same + // skill is being reinstalled under the other naming, name where it went — + // 24 lines of "no longer in the package" reads like the pack was gutted. + const renamedTo = (key) => { + const counterpart = key.startsWith(SKILL_PREFIX) + ? key.slice(SKILL_PREFIX.length) + : SKILL_PREFIX + key; + return installNames.includes(counterpart) ? counterpart : null; + }; + + manifestBeforeInstall = Object.assign({}, storedChecksums); + const foreignNames = skillNames.map((name) => claimsForeignName(name, storedChecksums)).filter(Boolean); + for (const key of orphans) { + const next = key.startsWith('hook:') ? null : renamedTo(key); + if (!next) continue; + const directory = fs.existsSync(path.join(AGENTS_DIR, key)) + ? path.join(AGENTS_DIR, key) : path.join(SKILLS_DIR, key); + prefixMigrations.set(next, { oldName: key, directory }); + } + + // Commit the rename only after the destination has been installed successfully. + function finishPrefixMigrations(checksums) { + for (const [next, migration] of prefixMigrations) { + if (!installedThisRun.has(next)) continue; + for (const root of [SKILLS_DIR, AGENTS_DIR]) { + const target = path.join(root, migration.oldName); + if (isSymlink(target)) fs.unlinkSync(target); + else if (fs.existsSync(target)) fs.rmSync(target, { recursive: true, force: true }); + } + delete checksums[migration.oldName]; + } + } + + let renamedCount = 0; for (const key of orphans) { const isHook = key.startsWith('hook:'); const targets = isHook ? [path.join(HOOKS_DIR, key.slice(5))] : [path.join(AGENTS_DIR, key), path.join(SKILLS_DIR, key)]; - if (!DRY_RUN) { + const moving = !isHook && renamedTo(key); + if (!DRY_RUN && !moving) { + if (!isHook) backupSkill(key); for (const target of targets) { if (!fs.existsSync(target) && !isSymlink(target)) continue; try { fs.rmSync(target, { recursive: true, force: true }); } catch (_) {} } delete storedChecksums[key]; } + const newName = isHook ? null : renamedTo(key); + if (newName) renamedCount++; if (!SILENT && !STARTUP) { - console.log((DRY_RUN ? ' [dry run] Would retire: ' : ' Retired: ') + - (isHook ? 'hook ' + key.slice(5) : key) + ' (no longer in the package)'); + console.log((newName ? (DRY_RUN ? ' [dry run] Would retire after replacement: ' : ' Pending retirement after replacement: ') + : (DRY_RUN ? ' [dry run] Would retire: ' : ' Retired: ')) + + (isHook ? 'hook ' + key.slice(5) : key) + + (newName ? ' (reinstalling as ' + newName + ')' : ' (no longer in the package)')); } } + if (renamedCount > 0 && !SILENT && !STARTUP) { + console.log('\n ESTACK_SKILL_PREFIX is ' + (PREFIXED ? "'" + SKILL_PREFIX + "'" : 'none') + + ', so ' + renamedCount + ' skill' + (renamedCount !== 1 ? 's are' : ' is') + + ' moving to ' + (PREFIXED ? 'prefixed' : 'short') + ' names. Nothing was removed from the pack.\n'); + } if (orphans.length > 0 && !DRY_RUN) { fs.mkdirSync(CLAUDE_DIR, { recursive: true }); fs.writeFileSync(CHECKSUMS_FILE, JSON.stringify(storedChecksums, null, 2)); @@ -1036,12 +1312,13 @@ async function main() { const modifiedSkills = []; const needsUpdate = []; for (const name of skillNames) { - const agentsSkillDir = path.join(AGENTS_DIR, name); + const installed = installedName(name); + const agentsSkillDir = path.join(AGENTS_DIR, installed); let installedDir = null; if (fs.existsSync(agentsSkillDir)) { installedDir = agentsSkillDir; } else { - const legacyDir = path.join(SKILLS_DIR, name); + const legacyDir = path.join(SKILLS_DIR, installed); if (isRealDir(legacyDir)) { installedDir = legacyDir; // old-style install, will be migrated on next write } @@ -1050,15 +1327,19 @@ async function main() { needsUpdate.push(name); continue; } + if (foreignNames.includes(installed)) { + needsUpdate.push(name); + continue; + } const currentHash = computeSkillHash(installedDir); - if (!storedChecksums[name]) { + if (!storedChecksums[installed]) { // No stored checksum — skill exists but wasn't installed by us. // Treat as locally modified if it differs from the package version. if (currentHash !== packageHashes[name]) { modifiedSkills.push(name); needsUpdate.push(name); } - } else if (currentHash !== storedChecksums[name]) { + } else if (currentHash !== storedChecksums[installed]) { // Stored checksum exists but current doesn't match — user modified it modifiedSkills.push(name); if (currentHash !== packageHashes[name]) { @@ -1107,18 +1388,30 @@ async function main() { fs.mkdirSync(SKILLS_DIR, { recursive: true }); const newChecksums = Object.assign({}, storedChecksums); for (const name of skillNames) { + const installed = installedName(name); + const migration = prefixMigrations.get(installed); + if (foreignNames.includes(installed) || (migration && + computeSkillHash(migration.directory) !== storedChecksums[migration.oldName])) { + console.error('estack: kept ' + (migration ? migration.oldName : installed) + + '; run the installer without --silent to review local changes or a name collision at ' + installed); + continue; + } if (modifiedSkills.includes(name)) continue; - if (!needsUpdate.includes(name) && fs.existsSync(path.join(AGENTS_DIR, name))) continue; + if (!needsUpdate.includes(name) && fs.existsSync(path.join(AGENTS_DIR, installedName(name)))) continue; installSkillFiles(name); - newChecksums[name] = packageHashes[name]; + newChecksums[installedName(name)] = packageHashes[name]; } fs.mkdirSync(HOOKS_DIR, { recursive: true }); for (const filename of hookFilenames) { if (modifiedHooks.includes(filename)) continue; if (!hooksNeedingUpdate.includes(filename) && fs.existsSync(path.join(HOOKS_DIR, filename))) continue; - fs.copyFileSync(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); + copyPackageFile(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); newChecksums['hook:' + filename] = packageHookHashes[filename]; } + finishPrefixMigrations(newChecksums); + setupRepoSearchNudgeHook(); + setupNotifyStopHook(); + setupStatuslineHook(); fs.writeFileSync(CHECKSUMS_FILE, JSON.stringify(newChecksums, null, 2)); process.exit(0); } @@ -1139,22 +1432,29 @@ async function main() { const mergeNeededLabels = []; // display labels with version transitions for (const name of skillNames) { + const installed = installedName(name); const newV = getSkillVersion(path.join(PACKAGE_SKILLS_DIR, name)); + const migration = prefixMigrations.get(installed); + if (migration && computeSkillHash(migration.directory) !== storedChecksums[migration.oldName]) { + mergeNeeded.push(installed); + mergeNeededLabels.push(migration.oldName + ' → ' + installed); + } + if (modifiedSkills.includes(name)) { // Backup local version, install new version - const oldV = getInstalledSkillVersion(name); - backupSkill(name); + const oldV = getInstalledSkillVersion(installed); + backupSkill(installed); installSkillFiles(name); - newChecksums[name] = packageHashes[name]; - mergeNeeded.push(name); - mergeNeededLabels.push(withVersion(name, oldV, newV)); + newChecksums[installed] = packageHashes[name]; + mergeNeeded.push(installed); + mergeNeededLabels.push(withVersion(installed, oldV, newV)); continue; } - if (!needsUpdate.includes(name) && fs.existsSync(path.join(AGENTS_DIR, name))) continue; - const oldV = getInstalledSkillVersion(name); + if (!needsUpdate.includes(name) && fs.existsSync(path.join(AGENTS_DIR, installed))) continue; + const oldV = getInstalledSkillVersion(installed); installSkillFiles(name); - newChecksums[name] = packageHashes[name]; - updated.push(withVersion(name, oldV, newV)); + newChecksums[installed] = packageHashes[name]; + updated.push(withVersion(installed, oldV, newV)); } // Install hooks @@ -1168,7 +1468,7 @@ async function main() { if (modifiedHooks.includes(filename)) { const oldV = getHookVersion(path.join(HOOKS_DIR, filename)); backupHook(filename); - fs.copyFileSync(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); + copyPackageFile(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); newChecksums['hook:' + filename] = packageHookHashes[filename]; mergeNeededHooks.push(filename); mergeNeededHookLabels.push(withVersion(filename, oldV, newV)); @@ -1176,7 +1476,7 @@ async function main() { } if (!hooksNeedingUpdate.includes(filename) && fs.existsSync(path.join(HOOKS_DIR, filename))) continue; const oldV = getHookVersion(path.join(HOOKS_DIR, filename)); - fs.copyFileSync(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); + copyPackageFile(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); newChecksums['hook:' + filename] = packageHookHashes[filename]; updatedHooks.push(withVersion(filename, oldV, newV)); } @@ -1185,11 +1485,17 @@ async function main() { setupNotifyStopHook(); setupStatuslineHook(); + finishPrefixMigrations(newChecksums); fs.writeFileSync(CHECKSUMS_FILE, JSON.stringify(newChecksums, null, 2)); // Build output for Claude Code const output = {}; const msgParts = []; + if (foreignNames.length) msgParts.push('estack: foreign skills backed up and replaced: ' + foreignNames.join(', ')); + for (const [next, migration] of prefixMigrations) { + if (installedThisRun.has(next)) msgParts.push('estack: renamed ' + migration.oldName + ' to ' + next + + '; previous files backed up under ' + BACKUP_DIR); + } if (updated.length > 0) { msgParts.push('estack: updated ' + updated.join(', ')); @@ -1244,12 +1550,24 @@ async function main() { // 7. Interactive mode — prompt if modifications detected let modifiedAction = null; // 'overwrite', 'skip', or 'merge' + // Unprefixed mode installs over short names. Anything already sitting on one + // of them that this installer did not put there is somebody else's skill, so + // say whose name is being taken rather than folding it silently into the + // "modified locally" list. It is still backed up and replaced from here. + if (foreignNames.length > 0) { + console.log('\nWARNING — ESTACK_SKILL_PREFIX=none installs these skills under names'); + console.log('that already exist and were not installed by estack:'); + for (const name of foreignNames) console.log(' - ' + name); + console.log('They will be backed up to ' + BACKUP_DIR.replace(HOME, '~') + + ' and replaced. Re-run with --prefix to keep the estack- names instead.'); + } + if (modifiedSkills.length > 0 || modifiedHooks.length > 0) { console.log('\nThe following items have been modified locally:'); if (modifiedSkills.length > 0) { console.log(' Skills:'); for (const name of modifiedSkills) { - console.log(' - ' + name); + console.log(' - ' + installedName(name)); } } if (modifiedHooks.length > 0) { @@ -1307,32 +1625,47 @@ async function main() { const mergedSkills = []; for (const name of skillNames) { + const installed = installedName(name); + const isForeign = foreignNames.includes(installed); + const pending = prefixMigrations.get(installed); + if (pending && NON_INTERACTIVE_ACTION === 'skip' && process.argv.includes('--skip-modified') && + computeSkillHash(pending.directory) !== storedChecksums[pending.oldName]) { + console.log(' Skipped ' + pending.oldName + ' (local modifications preserved; prefix switch pending)'); + continue; + } + if (modifiedSkills.includes(name)) { if (modifiedAction === 'skip') { - console.log(' Skipped ' + name + ' (local modifications preserved)'); - const currentHash = computeSkillHash(path.join(AGENTS_DIR, name)) || - computeSkillHash(path.join(SKILLS_DIR, name)); - if (currentHash) newChecksums[name] = currentHash; + console.log(' Skipped ' + installed + ' (local modifications preserved)'); + const currentHash = computeSkillHash(path.join(AGENTS_DIR, installed)) || + computeSkillHash(path.join(SKILLS_DIR, installed)); + if (currentHash && !isForeign) newChecksums[installed] = currentHash; continue; } - if (modifiedAction === 'merge') { - if (!DRY_RUN) backupSkill(name); - mergedSkills.push(name); - console.log((DRY_RUN ? ' [dry run] Would back up ' : ' Backed up ') + name + ' → ~/.estack-backup/' + name); + // A name claimed from someone else is backed up whichever action was + // chosen — "overwrite" is a choice about estack's own skills, and it + // should not be what silently destroys a skill estack never installed. + if (modifiedAction === 'merge' || isForeign) { + if (!DRY_RUN) backupSkill(installed); + mergedSkills.push(installed); + console.log((DRY_RUN ? ' [dry run] Would back up ' : ' Backed up ') + installed + ' → ~/.estack-backup/' + installed); } // overwrite or merge — fall through to install - } else if (!needsUpdate.includes(name) && fs.existsSync(path.join(AGENTS_DIR, name))) { + } else if (!needsUpdate.includes(name) && fs.existsSync(path.join(AGENTS_DIR, installed))) { // Already installed and up-to-date - if (DRY_RUN) console.log(' [dry run] Up to date (no change): ' + name); + if (DRY_RUN) console.log(' [dry run] Up to date (no change): ' + installed); continue; } - const isUpdate = fs.existsSync(path.join(AGENTS_DIR, name)) || - isRealDir(path.join(SKILLS_DIR, name)); - const label = withVersion(name, - isUpdate ? getInstalledSkillVersion(name) : null, + const migration = prefixMigrations.get(installed); + if (migration) console.log((DRY_RUN ? ' [dry run] Would back up ' : ' Backed up ') + migration.oldName + + ' before switching to ' + installed); + const isUpdate = fs.existsSync(path.join(AGENTS_DIR, installed)) || + isRealDir(path.join(SKILLS_DIR, installed)); + const label = withVersion(installed, + isUpdate ? getInstalledSkillVersion(installed) : null, getSkillVersion(path.join(PACKAGE_SKILLS_DIR, name))); if (!DRY_RUN) installSkillFiles(name); - newChecksums[name] = packageHashes[name]; + newChecksums[installed] = packageHashes[name]; installedCount++; if (DRY_RUN) { console.log(' [dry run] Would ' + (isUpdate ? 'update ' : 'install ') + label); @@ -1369,7 +1702,7 @@ async function main() { const hookLabel = withVersion(filename, isHookUpdate ? getHookVersion(path.join(HOOKS_DIR, filename)) : null, getHookVersion(path.join(PACKAGE_HOOKS_DIR, filename))); - if (!DRY_RUN) fs.copyFileSync(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); + if (!DRY_RUN) copyPackageFile(path.join(PACKAGE_HOOKS_DIR, filename), path.join(HOOKS_DIR, filename)); newChecksums['hook:' + filename] = packageHookHashes[filename]; installedHookCount++; if (DRY_RUN) { @@ -1380,7 +1713,10 @@ async function main() { } // 9. Write checksums - if (!DRY_RUN) fs.writeFileSync(CHECKSUMS_FILE, JSON.stringify(newChecksums, null, 2)); + if (!DRY_RUN) { + finishPrefixMigrations(newChecksums); + fs.writeFileSync(CHECKSUMS_FILE, JSON.stringify(newChecksums, null, 2)); + } // 10. Setup Claude and Codex startup hooks plus the Claude repo-search nudge. // In dry-run these inspect settings.json read-only and report would-be action. @@ -1416,7 +1752,11 @@ async function main() { for (const name of skillNames) { const desc = getSkillDescription(path.join(PACKAGE_SKILLS_DIR, name)); const ver = getSkillVersion(path.join(PACKAGE_SKILLS_DIR, name)); - console.log(' /' + name + (ver ? ' v' + ver : '') + (desc ? ' — ' + desc : '')); + // The description names neighbouring skills, so it gets the same rewrite + // the installed copy did — otherwise the listing advertises /estack-notify + // on an install where the command is /notify. + const shown = PREFIXED ? desc : renameReferences(Buffer.from(desc, 'utf8')).toString('utf8'); + console.log(' /' + installedName(name) + (ver ? ' v' + ver : '') + (shown ? ' — ' + shown : '')); } if (mergedSkills.length > 0) { diff --git a/docs/publishing.md b/docs/publishing.md index 614b4b2..2a719f7 100644 --- a/docs/publishing.md +++ b/docs/publishing.md @@ -4,7 +4,7 @@ e-stack publishes to npm via GitHub Actions when a git tag matching `v*` is push The shared startup updater is the exception to the Claude-only hook location: its core and Claude/Codex adapters install to `~/.agents/hooks/`, while `bin/install.cjs` registers the appropriate adapter in `~/.claude/settings.json` and `~/.codex/hooks.json`. Neither host auto-discovers hooks from `~/.agents/`; the explicit host registrations remain required. -The package ships both `skills/` and `hooks/` (whitelisted in `package.json`'s `files` field). Skills install to `~/.agents/skills/estack-*/` (symlinked from `~/.claude/skills/estack-*/`). Any agent that reads `~/.agents/skills/` — including OpenClaw, Codex, ChatGPT, and Cursor — will pick up skills automatically with no extra config. Hooks are Claude Code-only: they install to `~/.claude/hooks/` and get registered in `~/.claude/settings.json` by `bin/install.cjs`. Do not try to route hooks through `~/.agents/`. +The package ships both `skills/` and `hooks/` (whitelisted in `package.json`'s `files` field). Skills install to `~/.agents/skills//` (symlinked from `~/.claude/skills//`), prefixed by default or short names when `ESTACK_SKILL_PREFIX=none`. Any agent that reads `~/.agents/skills/` — including OpenClaw, Codex, ChatGPT, and Cursor — will pick up skills automatically with no extra config. Hooks are Claude Code-only: they install to `~/.claude/hooks/` and get registered in `~/.claude/settings.json` by `bin/install.cjs`. Do not try to route hooks through `~/.agents/`. ### How to publish @@ -84,3 +84,5 @@ These are the settings that make the model above hold. If something here drifts, - Re-enabling Actions does not replay missed tag events. If tags were pushed while Actions was disabled, delete and re-push only the latest tag after re-enabling: `git push origin :refs/tags/vX.Y.Z && git push origin refs/tags/vX.Y.Z` - Fastest Actions health check: run `gh workflow run publish.yml --repo ElliotDrel/e-stack`. HTTP 422 = disabled at account level. A run appearing = Actions is working. - **Manual CLI publish (when Actions are disabled):** there is a full fallback flow for publishing directly from your machine with `npm publish`. It requires temporarily enabling a package setting and creating a 2FA-bypass token, both of which are reverted after each release. See `docs/manual-publish.md`. + +Installer regressions run on Windows and Ubuntu for every PR (`installer.yml`). Both `node scripts/test-install-edge-cases.cjs` and `node scripts/test-prefix-safety.cjs` also gate the tag-triggered publish workflow. diff --git a/docs/skill-authoring.md b/docs/skill-authoring.md index 3624892..2089abc 100644 --- a/docs/skill-authoring.md +++ b/docs/skill-authoring.md @@ -48,8 +48,9 @@ Anything the skill persists for its own use between runs: config, credentials, caches, history, registries, cloned repos, internal artifacts. Use the skill's full folder name — `estack-doc-review-viewer` owns `~/.e-stack/estack-doc-review-viewer/`. The name is deliberately not shortened: -the installed skill, its frontmatter `name:`, and its state directory all answer -to one identity, which is also what keeps `check-skill-name.cjs` honest. +the state directory always uses the packaged `estack-` identity, even when the +installed folder and frontmatter use the short name. Prefix switches never move +saved state or credentials. One folder holds everything every e-stack skill has ever written. The user has a single directory to find, back up, inspect, or delete, instead of a dotfile per @@ -119,8 +120,8 @@ SERPAPI_KEY=def456 `~/.e-stack/.env`. The installer follows the same rule for its own settings — `ESTACK_SKILLS_DIR`, -`ESTACK_HOOKS_DIR`, `ESTACK_BACKUP_DIR`, `ESTACK_NO_STATUSLINE`, and `ESTACK_HOME` -all live in this file too. It is the pack's single settings file, not just its +`ESTACK_HOOKS_DIR`, `ESTACK_BACKUP_DIR`, `ESTACK_NO_STATUSLINE`, +`ESTACK_SKILL_PREFIX`, and `ESTACK_HOME` all live in this file too. It is the pack's single settings file, not just its credential file, so a new persistent setting belongs here under an `ESTACK_*` name rather than in a new JSON sidecar or marker file. A real environment variable wins so a one-off override works without editing the file. @@ -173,7 +174,7 @@ worked example. --- -### Doc Listings (README.md + CLAUDE.md) +### Doc Listings (README.md + AGENTS.md) Every skill and hook must be listed in two places: @@ -198,6 +199,18 @@ Allowed exceptions: mentions inside `references/` docs (prose about the skill's The publish workflow runs `--all` as a hard gate. Run it locally before tagging if you've renamed a skill or edited its frontmatter. +**Rule 5 is load-bearing, not just tidiness.** `ESTACK_SKILL_PREFIX=none` installs +skills under their short names, and the installer produces that by rewriting +`estack-` to `` in every packaged file on its way to disk — the +`name:` line, cross-skill mentions in prose, slash commands in descriptions, and +the self-referencing paths some skills use to run their own scripts +(`~/.claude/skills/estack-book-extractor/scripts/extract_text.py`). One pattern +finds all of them only because rule 5 guarantees every reference carries the +prefix. Write a skill reference bare and it survives the rewrite as a name that +does not exist on that install. The rewrite deliberately leaves +alone: stable state identifiers, a longer name a shorter one is a prefix of, and a filename like +`estack-notify.ps1`, which keeps its name inside the renamed folder. + --- ### Skill Feedback Section @@ -222,3 +235,14 @@ Use this for setup tasks, environment checks, or gathering context that the skil **Example:** - `skills/estack-repo-search/SKILL.md` — clones and indexes a repo on load + +### Prefix transformations and stored state + +The installer rewrites skill names and explicit `skills/` paths when +`ESTACK_SKILL_PREFIX=none`. Quoted standalone identifiers in executable code +stay canonical unless they are a segment immediately following `skills`. +This preserves storage expressions such as `resolve(ESTACK_ROOT, +'estack-doc-review-viewer')` and `Path.home() / ".e-stack" / +"estack-pdf-to-md"`. Keep state paths canonical; never derive storage from the +installed folder's basename. Run `node scripts/test-prefix-safety.cjs` when +changing references or state paths. diff --git a/scripts/test-install-edge-cases.cjs b/scripts/test-install-edge-cases.cjs index ec9b434..17f3315 100644 --- a/scripts/test-install-edge-cases.cjs +++ b/scripts/test-install-edge-cases.cjs @@ -24,7 +24,7 @@ function freshSandbox() { function runInstaller(args) { return execFileSync('node', [INSTALLER, ...args], { encoding: 'utf8', - env: Object.assign({}, process.env, { USERPROFILE: SANDBOX, HOME: SANDBOX }), + env: Object.assign(Object.fromEntries(Object.entries(process.env).filter(([key]) => !/^ESTACK_/.test(key))), { USERPROFILE: SANDBOX, HOME: SANDBOX }), stdio: ['pipe', 'pipe', 'pipe'], }); } @@ -162,6 +162,88 @@ const backupCore = path.join(SANDBOX, '.estack-backup', 'hooks', 'estack-startup check('local shared-adapter edit backed up', fs.existsSync(backupCore) && fs.readFileSync(backupCore, 'utf8').includes('// local change')); check('shared adapter restored from the package', !fs.readFileSync(sharedCore, 'utf8').includes('// local change')); +// ── Scenario L: ESTACK_SKILL_PREFIX=none installs short names ──────────────── +console.log('\nScenario L: unprefixed install'); +freshSandbox(); +runInstaller(['--install', '--yes', '--no-prefix']); +const SHORT = SAMPLE.replace(/^estack-/, ''); +check('short name is the real dir', isRealDir(agentsSkill(SHORT))); +check('prefixed name is absent', !fs.existsSync(agentsSkill(SAMPLE))); +check('symlink uses the short name', isLink(claudeSkill(SHORT))); +check('choice persisted to ~/.e-stack/.env', + /^ESTACK_SKILL_PREFIX=none$/m.test(fs.readFileSync(path.join(SANDBOX, '.e-stack', '.env'), 'utf8'))); + +const notifyMd = fs.readFileSync(path.join(SANDBOX, '.agents', 'skills', 'notify', 'SKILL.md'), 'utf8'); +check('frontmatter name rewritten', /^name: notify$/m.test(notifyMd)); +// Scoped to the description line: elsewhere in the file `/estack-notify` +// legitimately appears as part of the scripts/estack-notify.ps1 filename. +const notifyDesc = (notifyMd.match(/^description:.*$/m) || [''])[0]; +check('slash commands in the description rewritten', + notifyDesc.includes('/notify ') && !notifyDesc.includes('/estack-notify')); +check('skill folder in a self-referencing path rewritten', notifyMd.includes('.agents/skills/notify/scripts/')); +// estack-notify.ps1 is a real filename inside the skill, not a skill reference. +check('script filenames left alone', notifyMd.includes('scripts/estack-notify.ps1') && + fs.existsSync(path.join(SANDBOX, '.agents', 'skills', 'notify', 'scripts', 'estack-notify-lib.ps1'))); + +const bookMd = fs.readFileSync(path.join(SANDBOX, '.agents', 'skills', 'pdf-to-md', 'SKILL.md'), 'utf8'); +check('cross-skill install paths rewritten', !/skills\/estack-pdf-to-md/.test(bookMd)); + +// State under ~/.e-stack/ is storage, not a skill reference. Renaming it would +// orphan every saved config the moment the setting is flipped, and would split +// the statusline from the notify flag file it reads. +const plannerMd = fs.readFileSync(path.join(SANDBOX, '.agents', 'skills', 'flight-planner', 'SKILL.md'), 'utf8'); +check('state dir under ~/.e-stack keeps its prefixed name', + plannerMd.includes('.e-stack/estack-flight-planner') && !/\.e-stack\/flight-planner/.test(plannerMd)); +const statusline = fs.readFileSync(path.join(SANDBOX, '.claude', 'hooks', 'estack-statusline.js'), 'utf8'); +check('statusline still reads the prefixed notify flag dir', + statusline.includes("'.e-stack', 'estack-notify'")); +const notifyLib = fs.readFileSync(path.join(SANDBOX, '.agents', 'skills', 'notify', 'scripts', 'estack-notify-lib.ps1'), 'utf8'); +check('notify writes the flag where the statusline reads it', + notifyLib.includes('estack-notify') && !/\.e-stack[\\/]notify\b/.test(notifyLib)); + +const notifyStop = JSON.parse(fs.readFileSync(path.join(SANDBOX, '.claude', 'settings.json'), 'utf8')) + .hooks.Stop[0].hooks[0].args[2]; +check('notify Stop hook points at the short folder', + notifyStop.includes('/notify/scripts/estack-notify-stop.ps1')); + +// ── Scenario M: an unprefixed install is stable across runs ────────────────── +// The package hash has to be computed on the rewritten content, or every skill +// reads as locally modified on the very next session start. +console.log('\nScenario M: unprefixed install is idempotent'); +const secondRun = runInstaller(['--install', '--yes']); +check('nothing reinstalls', /0 skills installed/.test(secondRun)); +check('nothing reads as locally modified', !/modified locally/.test(secondRun)); + +// ── Scenario N: editing the .env flips an existing install on next update ──── +console.log('\nScenario N: flip back to prefixed by editing ~/.e-stack/.env'); +const envFile = path.join(SANDBOX, '.e-stack', '.env'); +fs.writeFileSync(envFile, fs.readFileSync(envFile, 'utf8').replace(/^ESTACK_SKILL_PREFIX=none$/m, 'ESTACK_SKILL_PREFIX=estack-')); +runInstaller(['--install', '--startup']); // the auto-update path +check('prefixed name installed', isRealDir(agentsSkill(SAMPLE))); +check('short name retired from ~/.agents/skills/', !fs.existsSync(agentsSkill(SHORT))); +check('short symlink retired from ~/.claude/skills/', !fs.existsSync(claudeSkill(SHORT)) && !isLink(claudeSkill(SHORT))); +const manifest = JSON.parse(fs.readFileSync(path.join(SANDBOX, '.claude', '.estack-checksums.json'), 'utf8')); +check('manifest carries no stale short-name entries', manifest[SAMPLE] && !manifest[SHORT]); +check('notify Stop hook repointed at the prefixed folder', + JSON.parse(fs.readFileSync(path.join(SANDBOX, '.claude', 'settings.json'), 'utf8')) + .hooks.Stop[0].hooks[0].args[2].includes('/estack-notify/scripts/')); +check('flip settles', /0 skills installed/.test(runInstaller(['--install', '--yes']))); + +// ── Scenario O: claiming a short name someone else is using ────────────────── +// Overwriting is the chosen behavior, but never without a copy and a warning. +console.log('\nScenario O: unprefixed install over a foreign skill of the same name'); +const foreign = path.join(SANDBOX, '.agents', 'skills', 'notify'); +fs.rmSync(foreign, { recursive: true, force: true }); +fs.mkdirSync(foreign, { recursive: true }); +fs.writeFileSync(path.join(foreign, 'SKILL.md'), '---\nname: notify\n---\nnot ours\n'); +const claimed = runInstaller(['--install', '--yes', '--no-prefix']); +check('collision is called out by name', /WARNING/.test(claimed) && /\n - notify\n/.test(claimed)); +const foreignBackups = fs.readdirSync(path.join(SANDBOX, '.estack-backup')).filter(n => n === 'notify' || n.startsWith('notify-')).map(n => path.join(SANDBOX, '.estack-backup', n, 'SKILL.md')); +check('foreign skill backed up before replacement', + foreignBackups.some(file => fs.existsSync(file) && fs.readFileSync(file, 'utf8').includes('not ours'))); +check('estack skill now occupies the name', + fs.readFileSync(path.join(foreign, 'SKILL.md'), 'utf8').includes('desktop notifications')); + // ── Cleanup ────────────────────────────────────────────────────────────────── fs.rmSync(SANDBOX, { recursive: true, force: true }); console.log('\n' + (failures === 0 ? 'ALL EDGE CASES PASSED.' : failures + ' EDGE-CASE FAILURE(S).')); diff --git a/scripts/test-prefix-safety.cjs b/scripts/test-prefix-safety.cjs new file mode 100644 index 0000000..0cf7291 --- /dev/null +++ b/scripts/test-prefix-safety.cjs @@ -0,0 +1,110 @@ +// Real installer regressions: every subprocess uses a disposable home. +'use strict'; +const { execFileSync } = require('child_process'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const crypto = require('crypto'); +const { pathToFileURL } = require('url'); +const installer = process.env.PREFIX_TEST_INSTALLER || path.resolve(__dirname, '../bin/install.cjs'); +const home = fs.mkdtempSync(path.join(os.tmpdir(), 'estack-prefix-safety-')); +const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !/^ESTACK_/.test(key))); +Object.assign(env, { HOME: home, USERPROFILE: home }); +const at = (...parts) => path.join(home, ...parts); +const skill = name => at('.agents', 'skills', name); +const sample = 'estack-better-title'; +const short = 'better-title'; +let failures = 0; +function check(label, value) { console.log((value ? 'OK ' : 'FAIL ') + label); if (!value) failures++; } +function run(args, extra = {}) { return execFileSync(process.execPath, [installer, ...args], { env: { ...env, ...extra }, encoding: 'utf8' }); } +function read(file) { return fs.readFileSync(file, 'utf8'); } +function fresh() { fs.rmSync(home, { recursive: true, force: true }); fs.mkdirSync(home); } +function snapshot(dir = home) { + const entries = []; + for (const name of fs.readdirSync(dir).sort()) { + const file = path.join(dir, name), stat = fs.lstatSync(file); + const value = stat.isSymbolicLink() ? fs.readlinkSync(file) : stat.isDirectory() ? snapshot(file) : crypto.createHash('sha256').update(fs.readFileSync(file)).digest('hex'); + entries.push([name, stat.mode, value]); + } + return JSON.stringify(entries); +} +try { + run(['--install', '--yes', '--no-prefix']); + const registry = skill('doc-review-viewer') + '/registry.mjs'; + const output = execFileSync(process.execPath, ['--input-type=module', '-e', `const m = await import(${JSON.stringify(pathToFileURL(registry).href)}); console.log(m.ROOT);`], { env, encoding: 'utf8' }).trim(); + check('executed registry keeps the canonical state directory', output === at('.e-stack', 'estack-doc-review-viewer')); + check('Python legacy credential path keeps canonical state name', read(skill('pdf-to-md') + '/scripts/pdf_to_md.py').includes('".e-stack" / "estack-pdf-to-md"')); + check('Python legacy installed-skill path follows prefix choice', read(skill('pdf-to-md') + '/scripts/pdf_to_md.py').includes('"skills" / "pdf-to-md"')); + // Verify all quoted identifiers used in state-owning source files remain canonical. + for (const [name, relative] of [['doc-review-viewer', 'registry.mjs'], ['pdf-to-md', 'scripts/pdf_to_md.py']]) { + check('state identifier preserved in ' + relative, read(skill(name) + '/' + relative).includes('estack-' + name)); + } + // Flip twice with modified content, nested credentials, and an existing backup. + fs.appendFileSync(skill(short) + '/SKILL.md', '\nLOCAL EDIT ONE\n'); + fs.mkdirSync(skill(short) + '/nested', { recursive: true }); + fs.writeFileSync(skill(short) + '/nested/.env', 'TEST_KEY=fixture\n'); + fs.mkdirSync(at('.estack-backup', short), { recursive: true }); + fs.writeFileSync(at('.estack-backup', short, 'keep.txt'), 'older backup'); + const beforePreview = snapshot(); + const preview = run(['--dry-run', '--prefix', '--yes']); + check('prefix preview is write-free', snapshot() === beforePreview); + check('preview describes backup and rename', preview.includes('Would back up ' + short) && preview.includes('reinstalling as ' + sample)); + run(['--install', '--yes', '--prefix']); + check('flip carries nested credentials', read(skill(sample) + '/nested/.env') === 'TEST_KEY=fixture\n'); + check('existing backup survives', read(at('.estack-backup', short, 'keep.txt')) === 'older backup'); + const backups = fs.readdirSync(at('.estack-backup')).filter(n => n.startsWith(short + '-')); + check('local edits backed up separately', backups.some(n => read(at('.estack-backup', n, 'SKILL.md')).includes('LOCAL EDIT ONE'))); + run(['--install', '--yes', '--no-prefix']); + check('reverse flip carries credentials', read(skill(short) + '/nested/.env').includes('TEST_KEY=fixture')); + check('credentials do not cause perpetual modified installs', run(['--install', '--yes']).includes('0 skills installed')); + check('old name retired only after replacement', !fs.existsSync(skill(sample)) && fs.existsSync(skill(short))); + // Silent conflict keeps both the original install and foreign destination. + fresh(); run(['--install', '--yes']); + fs.mkdirSync(skill(short), { recursive: true }); fs.writeFileSync(skill(short) + '/SKILL.md', 'FOREIGN SKILL'); + run(['--install', '--silent', '--no-prefix']); + check('silent collision preserves old install', fs.existsSync(skill(sample))); + check('silent collision preserves foreign skill', read(skill(short) + '/SKILL.md') === 'FOREIGN SKILL'); + const startup = JSON.parse(run(['--install', '--startup'])); + check('startup identifies foreign takeover', startup.systemMessage.includes('foreign skills backed up and replaced: ' + short)); + check('startup does not call foreign skill a local modification', !(startup.additionalContext || '').includes(short)); + check('foreign skill backed up', read(at('.estack-backup', short, 'SKILL.md')) === 'FOREIGN SKILL'); + check('startup completes pending flip', !fs.existsSync(skill(sample)) && read(skill(short) + '/SKILL.md').includes('name: ' + short)); + const manifest = JSON.parse(read(at('.claude', '.estack-checksums.json'))); + check('manifest records completed names only', manifest[short] && !manifest[sample]); + // Failed replacement must leave the old skill and ownership record usable. + fresh(); run(['--install', '--yes']); + const injector = at('fail-write.cjs'); + fs.writeFileSync(injector, `const fs = require('fs'); const write = fs.writeFileSync; fs.writeFileSync = function(file, ...args) { if (String(file).replace(/\\/g, '/').endsWith('/skills/better-title/SKILL.md')) throw new Error('injected destination failure'); return write.call(this, file, ...args); };`); + let rejected = false; + try { execFileSync(process.execPath, ['--require', injector, installer, '--install', '--yes', '--no-prefix'], { env, encoding: 'utf8', stdio: 'pipe' }); } catch (_) { rejected = true; } + check('failed replacement preserves original skill', rejected && fs.existsSync(skill(sample) + '/SKILL.md')); + check('failed replacement preserves original manifest entry', JSON.parse(read(at('.claude', '.estack-checksums.json')))[sample]); + // Skip means a modified source stays available until the user reviews it. + fresh(); run(['--install', '--yes']); fs.appendFileSync(skill(sample) + '/SKILL.md', '\nLOCAL KEEP\n'); + run(['--install', '--skip-modified', '--no-prefix']); + check('skip-modified retains edited source across a flip', read(skill(sample) + '/SKILL.md').includes('LOCAL KEEP') && !fs.existsSync(skill(short))); + // Credentials in old real Claude directories must be collected before unlink. + fresh(); const legacy = at('.claude', 'skills', sample); + fs.cpSync(path.resolve(path.dirname(installer), '../skills', sample), legacy, { recursive: true }); + fs.writeFileSync(path.join(legacy, '.env'), 'LEGACY_KEY=fixture\n'); + run(['--install', '--yes']); + check('legacy real directory credentials survive migration', read(skill(sample) + '/.env') === 'LEGACY_KEY=fixture\n'); + // Deprecated short name is not evidence of ownership. + fresh(); fs.mkdirSync(skill('prompt-builder'), { recursive: true }); fs.writeFileSync(skill('prompt-builder') + '/SKILL.md', 'USER OWNED'); + run(['--install', '--yes', '--no-prefix']); + check('foreign deprecated short name survives', read(skill('prompt-builder') + '/SKILL.md') === 'USER OWNED'); + // Dry-run must not migrate settings, create directories, or chmod existing files. + fresh(); fs.mkdirSync(at('.e-stack')); fs.writeFileSync(at('.e-stack', 'config.json'), JSON.stringify({ statusline: { enabled: false } })); + fs.writeFileSync(at('.e-stack', 'no-statusline'), ''); fs.writeFileSync(at('.e-stack', '.env'), 'TEST_API_KEY=unchanged\n'); + for (const flags of [[], ['--silent'], ['--startup']]) { + const before = snapshot(); run(['--dry-run', '--no-prefix', ...flags]); + check('dry-run leaves legacy configuration intact ' + flags.join(' '), snapshot() === before); + } + for (const flags of [['--prefix', '--no-prefix'], ['--statusline', '--no-statusline']]) { + const before = snapshot(); let failed = false; + try { run(['--install', '--yes', ...flags]); } catch (e) { failed = e.status !== 0 && String(e.stderr).includes('Conflicting flags'); } + check('conflicting flags rejected before writes ' + flags.join(' '), failed && snapshot() === before); + } + console.log(failures ? `${failures} PREFIX SAFETY FAILURES` : 'ALL PREFIX SAFETY CHECKS PASSED'); +} finally { fs.rmSync(home, { recursive: true, force: true }); } +process.exitCode = failures ? 1 : 0; From 405a4de7769a3c79a8f1a42cedd83347950d84e7 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Mon, 19 Oct 2026 13:17:09 -0400 Subject: [PATCH 2/4] Keep hooks valid when prefix switches are deferred --- bin/install.cjs | 12 +++++++++++- scripts/test-prefix-safety.cjs | 3 +++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/bin/install.cjs b/bin/install.cjs index accafcb..6c05c5c 100644 --- a/bin/install.cjs +++ b/bin/install.cjs @@ -981,7 +981,10 @@ function setupNotifyStopHook(dryRun) { // The script's own filename never changes; only the skill folder around it // does, when ESTACK_SKILL_PREFIX=none. Flipping the setting repoints this // entry in place via the mentionsScript match below. - const script = path.join(AGENTS_DIR, installedName('estack-notify'), 'scripts', 'estack-notify-stop.ps1') + const requested = installedName('estack-notify'); + const pending = prefixMigrations.get(requested); + const active = pending && !installedThisRun.has(requested) && !dryRun ? pending.oldName : requested; + const script = path.join(AGENTS_DIR, active, 'scripts', 'estack-notify-stop.ps1') .replace(/\\/g, '/'); // The args form avoids quoting a Windows path inside a command string, and it // is the shape Claude Code writes itself. @@ -1379,6 +1382,13 @@ async function main() { // 5. Silent mode — no output at all if (SILENT) { + const blocked = [...prefixMigrations].filter(([next, migration]) => + foreignNames.includes(next) || computeSkillHash(migration.directory) !== storedChecksums[migration.oldName]); + if (blocked.length) { + console.error('estack: prefix switch deferred; original skills and hooks kept. Run the manual installer to review: ' + + blocked.map(([next]) => next).join(', ')); + process.exit(0); + } if (needsUpdate.length === 0 && modifiedSkills.length === 0 && hooksNeedingUpdate.length === 0 && modifiedHooks.length === 0 && startupSourcesUpdated.length === 0) { diff --git a/scripts/test-prefix-safety.cjs b/scripts/test-prefix-safety.cjs index 0cf7291..5fdadcb 100644 --- a/scripts/test-prefix-safety.cjs +++ b/scripts/test-prefix-safety.cjs @@ -64,6 +64,7 @@ try { run(['--install', '--silent', '--no-prefix']); check('silent collision preserves old install', fs.existsSync(skill(sample))); check('silent collision preserves foreign skill', read(skill(short) + '/SKILL.md') === 'FOREIGN SKILL'); + check('silent deferral keeps notify hook on original skill', JSON.parse(read(at('.claude', 'settings.json'))).hooks.Stop[0].hooks[0].args[2].includes('/estack-notify/scripts/')); const startup = JSON.parse(run(['--install', '--startup'])); check('startup identifies foreign takeover', startup.systemMessage.includes('foreign skills backed up and replaced: ' + short)); check('startup does not call foreign skill a local modification', !(startup.additionalContext || '').includes(short)); @@ -81,8 +82,10 @@ try { check('failed replacement preserves original manifest entry', JSON.parse(read(at('.claude', '.estack-checksums.json')))[sample]); // Skip means a modified source stays available until the user reviews it. fresh(); run(['--install', '--yes']); fs.appendFileSync(skill(sample) + '/SKILL.md', '\nLOCAL KEEP\n'); + fs.appendFileSync(skill('estack-notify') + '/SKILL.md', '\nLOCAL NOTIFY KEEP\n'); run(['--install', '--skip-modified', '--no-prefix']); check('skip-modified retains edited source across a flip', read(skill(sample) + '/SKILL.md').includes('LOCAL KEEP') && !fs.existsSync(skill(short))); + check('skipped notify keeps a valid Stop hook', JSON.parse(read(at('.claude', 'settings.json'))).hooks.Stop[0].hooks[0].args[2].includes('/estack-notify/scripts/')); // Credentials in old real Claude directories must be collected before unlink. fresh(); const legacy = at('.claude', 'skills', sample); fs.cpSync(path.resolve(path.dirname(installer), '../skills', sample), legacy, { recursive: true }); From 1043345c2a4cbb70b80a9e8577f1482722e17bbd Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Mon, 19 Oct 2026 13:20:37 -0400 Subject: [PATCH 3/4] Fix quoted hook references and early Node 18 compatibility --- .github/workflows/installer.yml | 3 ++- bin/install.cjs | 11 ++++++++--- docs/skill-authoring.md | 3 ++- scripts/test-prefix-safety.cjs | 11 +++++++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/.github/workflows/installer.yml b/.github/workflows/installer.yml index 1d735f7..8dbf82f 100644 --- a/.github/workflows/installer.yml +++ b/.github/workflows/installer.yml @@ -10,11 +10,12 @@ jobs: strategy: matrix: os: [ubuntu-latest, windows-latest] + node: ["18.0.0", "24"] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v6 - uses: actions/setup-node@v6 with: - node-version: "24" + node-version: ${{ matrix.node }} - run: node scripts/test-install-edge-cases.cjs - run: node scripts/test-prefix-safety.cjs diff --git a/bin/install.cjs b/bin/install.cjs index 6c05c5c..8989e2a 100644 --- a/bin/install.cjs +++ b/bin/install.cjs @@ -6,7 +6,11 @@ const path = require('path'); const crypto = require('crypto'); const os = require('os'); const readline = require('readline'); -const { isUtf8 } = require('buffer'); +const { TextDecoder } = require('util'); +const UTF8_DECODER = new TextDecoder('utf-8', { fatal: true }); +function isUtf8(buffer) { + try { UTF8_DECODER.decode(buffer); return true; } catch (_) { return false; } +} const IS_LOCAL = !__dirname.includes('node_modules'); const DRY_RUN = process.argv.includes('--dry-run') || @@ -459,8 +463,9 @@ function renameReferences(buffer) { const before = text.slice(0, offset); const after = text.slice(offset + match.length); // Quoted bare names in executable code are storage keys or identifiers. - // Rewrite only when the preceding path segment explicitly names skills. - if (/['"]$/.test(before) && /^['"]/.test(after) && + // Explicit skill-name constants are references; storage keys stay canonical. + const skillConstant = /\b(?:SKILL_NAME|skillName)\s*=\s*['"]$/.test(before); + if (/['"]$/.test(before) && /^['"]/.test(after) && !skillConstant && !/skills['"]?\s*(?:,|\/)\s*['"]$/.test(before)) return match; // Variable-based state paths ($ESTACK_ROOT/estack-...) also stay stable. if (/[A-Za-z0-9_$}.][\\/]$/.test(before) && !/skills[\\/]$/.test(before)) return match; diff --git a/docs/skill-authoring.md b/docs/skill-authoring.md index 2089abc..211bf23 100644 --- a/docs/skill-authoring.md +++ b/docs/skill-authoring.md @@ -240,7 +240,8 @@ Use this for setup tasks, environment checks, or gathering context that the skil The installer rewrites skill names and explicit `skills/` paths when `ESTACK_SKILL_PREFIX=none`. Quoted standalone identifiers in executable code -stay canonical unless they are a segment immediately following `skills`. +stay canonical unless they are a segment immediately following `skills` or +assigned to an explicit skill identifier (`SKILL_NAME` or `skillName`). This preserves storage expressions such as `resolve(ESTACK_ROOT, 'estack-doc-review-viewer')` and `Path.home() / ".e-stack" / "estack-pdf-to-md"`. Keep state paths canonical; never derive storage from the diff --git a/scripts/test-prefix-safety.cjs b/scripts/test-prefix-safety.cjs index 5fdadcb..8757edf 100644 --- a/scripts/test-prefix-safety.cjs +++ b/scripts/test-prefix-safety.cjs @@ -30,6 +30,12 @@ function snapshot(dir = home) { } try { run(['--install', '--yes', '--no-prefix']); + const nudge = JSON.parse(execFileSync(process.execPath, [at('.claude', 'hooks', 'repo-search-nudge.js')], { + env, encoding: 'utf8', input: JSON.stringify({ tool_name: 'WebFetch', tool_input: { url: 'https://github.com/example/repo' } }), + })); + check('executed nudge recommends the installed short name', + nudge.hookSpecificOutput.additionalContext.includes('the repo-search skill') && + !nudge.hookSpecificOutput.additionalContext.includes('estack-repo-search')); const registry = skill('doc-review-viewer') + '/registry.mjs'; const output = execFileSync(process.execPath, ['--input-type=module', '-e', `const m = await import(${JSON.stringify(pathToFileURL(registry).href)}); console.log(m.ROOT);`], { env, encoding: 'utf8' }).trim(); check('executed registry keeps the canonical state directory', output === at('.e-stack', 'estack-doc-review-viewer')); @@ -92,6 +98,11 @@ try { fs.writeFileSync(path.join(legacy, '.env'), 'LEGACY_KEY=fixture\n'); run(['--install', '--yes']); check('legacy real directory credentials survive migration', read(skill(sample) + '/.env') === 'LEGACY_KEY=fixture\n'); + fresh(); + const earlyNode = at('without-buffer-isutf8.cjs'); + fs.writeFileSync(earlyNode, "require('buffer').isUtf8 = undefined;\n"); + execFileSync(process.execPath, ['--require', earlyNode, installer, '--install', '--yes', '--no-prefix'], { env, encoding: 'utf8' }); + check('unprefixed install works without buffer.isUtf8', fs.existsSync(skill(short) + '/SKILL.md')); // Deprecated short name is not evidence of ownership. fresh(); fs.mkdirSync(skill('prompt-builder'), { recursive: true }); fs.writeFileSync(skill('prompt-builder') + '/SKILL.md', 'USER OWNED'); run(['--install', '--yes', '--no-prefix']); From d55f526ec9eb83d096893ef76d2459354284ecb0 Mon Sep 17 00:00:00 2001 From: Elliot Drel Date: Mon, 19 Oct 2026 14:54:27 -0400 Subject: [PATCH 4/4] Require the intended installer failure in preservation regression --- scripts/test-prefix-safety.cjs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/scripts/test-prefix-safety.cjs b/scripts/test-prefix-safety.cjs index 8757edf..95a39af 100644 --- a/scripts/test-prefix-safety.cjs +++ b/scripts/test-prefix-safety.cjs @@ -6,6 +6,7 @@ const os = require('os'); const path = require('path'); const crypto = require('crypto'); const { pathToFileURL } = require('url'); +const { Script } = require('vm'); const installer = process.env.PREFIX_TEST_INSTALLER || path.resolve(__dirname, '../bin/install.cjs'); const home = fs.mkdtempSync(path.join(os.tmpdir(), 'estack-prefix-safety-')); const env = Object.fromEntries(Object.entries(process.env).filter(([key]) => !/^ESTACK_/.test(key))); @@ -81,11 +82,17 @@ try { // Failed replacement must leave the old skill and ownership record usable. fresh(); run(['--install', '--yes']); const injector = at('fail-write.cjs'); - fs.writeFileSync(injector, `const fs = require('fs'); const write = fs.writeFileSync; fs.writeFileSync = function(file, ...args) { if (String(file).replace(/\\/g, '/').endsWith('/skills/better-title/SKILL.md')) throw new Error('injected destination failure'); return write.call(this, file, ...args); };`); + const originalSkill = read(skill(sample) + '/SKILL.md'); + const injectorSource = String.raw`const fs = require('fs'); const write = fs.writeFileSync; fs.writeFileSync = function(file, ...args) { if (String(file).replace(/\\/g, '/').endsWith('/skills/better-title/SKILL.md')) throw new Error('injected destination failure'); return write.call(this, file, ...args); };`; + new Script(injectorSource, { filename: injector }); + fs.writeFileSync(injector, injectorSource); let rejected = false; - try { execFileSync(process.execPath, ['--require', injector, installer, '--install', '--yes', '--no-prefix'], { env, encoding: 'utf8', stdio: 'pipe' }); } catch (_) { rejected = true; } - check('failed replacement preserves original skill', rejected && fs.existsSync(skill(sample) + '/SKILL.md')); - check('failed replacement preserves original manifest entry', JSON.parse(read(at('.claude', '.estack-checksums.json')))[sample]); + try { execFileSync(process.execPath, ['--require', injector, installer, '--install', '--yes', '--no-prefix'], { env, encoding: 'utf8', stdio: 'pipe' }); } catch (error) { + rejected = error.status !== 0 && /^Error during installation: injected destination failure\r?$/m.test(String(error.stderr)); + } + check('installer reaches the intended destination write failure', rejected); + check('failed replacement preserves original skill unchanged', rejected && read(skill(sample) + '/SKILL.md') === originalSkill); + check('failed replacement preserves original manifest entry', rejected && JSON.parse(read(at('.claude', '.estack-checksums.json')))[sample]); // Skip means a modified source stays available until the user reviews it. fresh(); run(['--install', '--yes']); fs.appendFileSync(skill(sample) + '/SKILL.md', '\nLOCAL KEEP\n'); fs.appendFileSync(skill('estack-notify') + '/SKILL.md', '\nLOCAL NOTIFY KEEP\n');