diff --git a/.env.release.draft b/.env.release.draft index 40058266..c8f0872b 100644 --- a/.env.release.draft +++ b/.env.release.draft @@ -93,3 +93,6 @@ SPRING_MAIL_PROPERTIES_MAIL_SMTP_SSL_TRUST= SKILLHUB_AUTH_PASSWORD_RESET_CODE_EXPIRY=PT10M SKILLHUB_AUTH_PASSWORD_RESET_FROM_ADDRESS=noreply@example.com SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub + +# Required for signing anonymous download rate-limit cookies. Use a unique random value per deployment. +SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=replace-with-random-download-secret-32-bytes diff --git a/.env.release.example b/.env.release.example index a6eadf21..2d30c7bc 100644 --- a/.env.release.example +++ b/.env.release.example @@ -79,6 +79,15 @@ SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_OIDC_SCOPE=openid,profile,email SPRING_SECURITY_OAUTH2_CLIENT_REGISTRATION_OIDC_CLIENT_NAME=OIDC SPRING_SECURITY_OAUTH2_CLIENT_PROVIDER_OIDC_ISSUER_URI= +# Direct (username/password) authentication for environments without OAuth2. +# To enable, set BOTH: +# - SKILLHUB_AUTH_DIRECT_ENABLED=true (server: enables the /api/v1/auth/direct endpoint) +# - SKILLHUB_WEB_AUTH_DIRECT_ENABLED=true (web: surfaces the username/password form) +# Set SKILLHUB_WEB_AUTH_DIRECT_PROVIDER to the provider id (e.g. "local"). +SKILLHUB_AUTH_DIRECT_ENABLED=false +SKILLHUB_WEB_AUTH_DIRECT_ENABLED=false +SKILLHUB_WEB_AUTH_DIRECT_PROVIDER= + # SMTP configuration for password reset verification emails. SPRING_MAIL_HOST= SPRING_MAIL_PORT=587 @@ -95,6 +104,10 @@ SKILLHUB_AUTH_PASSWORD_RESET_FROM_NAME=SkillHub # Security scanner is enabled by default. Set to false to disable scanning. SKILLHUB_SECURITY_SCANNER_ENABLED=true +# Required for signing anonymous download rate-limit cookies. Use a unique random value per deployment. +# runtime.sh generates and persists one automatically when this placeholder is still present. +SKILLHUB_DOWNLOAD_ANON_COOKIE_SECRET=replace-with-random-download-secret-32-bytes + # Scanner LLM configuration (optional, for AI-powered scanning features) SKILL_SCANNER_LLM_API_KEY= SKILL_SCANNER_LLM_BASE_URL= diff --git a/.gemini/config.yaml b/.gemini/config.yaml index 2499d3f0..2506a30c 100644 --- a/.gemini/config.yaml +++ b/.gemini/config.yaml @@ -1,6 +1,2 @@ -# https://developers.google.com/gemini-code-assist/docs/customize-gemini-behavior-github -have_fun: false # Just review the code code_review: - comment_severity_threshold: HIGH # Reduce quantity of comments - pull_request_opened: - summary: false # Don't summarize the PR in a separate comment + disable: true diff --git a/.github/workflows/pr-cli.yml b/.github/workflows/pr-cli.yml index 7de2a31e..65c0ca6a 100644 --- a/.github/workflows/pr-cli.yml +++ b/.github/workflows/pr-cli.yml @@ -7,6 +7,9 @@ on: - 'Makefile' - '.github/workflows/pr-cli.yml' +permissions: + contents: read + jobs: cli: strategy: @@ -16,6 +19,8 @@ jobs: runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 + with: + persist-credentials: false - uses: oven-sh/setup-bun@v2 with: bun-version: 1.3.13 diff --git a/.github/workflows/pr-e2e.yml b/.github/workflows/pr-e2e.yml index f25d8072..f94ffdc2 100644 --- a/.github/workflows/pr-e2e.yml +++ b/.github/workflows/pr-e2e.yml @@ -34,6 +34,8 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v4 + with: + persist-credentials: false - name: Set up pnpm uses: pnpm/action-setup@v4 diff --git a/.github/workflows/pr-scripts.yml b/.github/workflows/pr-scripts.yml new file mode 100644 index 00000000..e521eb9e --- /dev/null +++ b/.github/workflows/pr-scripts.yml @@ -0,0 +1,37 @@ +name: PR Scripts + +on: + pull_request: + paths: + - 'scripts/**' + - '.env.release.example' + - '.env.release.draft' + - 'compose.release.yml' + - 'Makefile' + - '.github/workflows/pr-cli.yml' + - '.github/workflows/pr-e2e.yml' + - '.github/workflows/pr-tests.yml' + - '.github/workflows/security.yml' + - '.github/workflows/pr-scripts.yml' + - '**/*.py' + +permissions: + contents: read + +jobs: + scripts-tests: + name: Script Regression Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + persist-credentials: false + - uses: actions/setup-node@v4 + with: + node-version: '21' + - run: bash scripts/tests/publish-cli-test.sh + - run: bash scripts/tests/runtime-secret-test.sh + - run: bash scripts/tests/validate-release-config-test.sh + - run: bash scripts/tests/dev-web-host-test.sh + - run: bash scripts/tests/workflow-security-test.sh diff --git a/.github/workflows/pr-tests.yml b/.github/workflows/pr-tests.yml index fd59d840..4e957ec3 100644 --- a/.github/workflows/pr-tests.yml +++ b/.github/workflows/pr-tests.yml @@ -25,6 +25,8 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v4 + with: + persist-credentials: false - name: Set up pnpm uses: pnpm/action-setup@v4 @@ -52,6 +54,8 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v4 + with: + persist-credentials: false - name: Set up Java uses: actions/setup-java@v4 @@ -65,3 +69,39 @@ jobs: - name: Run backend unit tests run: make test-backend + + docs-build: + name: Docs Build + if: ${{ github.event_name != 'pull_request' || !github.event.pull_request.draft }} + runs-on: ubuntu-latest + + steps: + - name: Check out repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Detect docs changes + id: changed + uses: dorny/paths-filter@v3 + with: + filters: | + docs: + - 'docs/skillhub/**' + - '.github/workflows/pr-tests.yml' + + - name: Set up Node.js + if: steps.changed.outputs.docs == 'true' + uses: actions/setup-node@v4 + with: + node-version: 24 + cache: npm + cache-dependency-path: docs/skillhub/package-lock.json + + - name: Install docs dependencies + if: steps.changed.outputs.docs == 'true' + run: cd docs/skillhub && npm ci + + - name: Build VitePress site + if: steps.changed.outputs.docs == 'true' + run: cd docs/skillhub && npm run build diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml new file mode 100644 index 00000000..3329a267 --- /dev/null +++ b/.github/workflows/security.yml @@ -0,0 +1,87 @@ +name: Security + +on: + pull_request: + types: + - opened + - synchronize + - reopened + - ready_for_review + push: + branches: + - main + schedule: + - cron: '23 3 * * 1' + workflow_dispatch: + +permissions: + contents: read + +jobs: + dependency-review: + name: Dependency Review + if: ${{ github.event_name == 'pull_request' && !github.event.pull_request.draft }} + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + steps: + - name: Check out repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Review dependency changes + uses: actions/dependency-review-action@v4 + + codeql: + name: CodeQL (${{ matrix.language }}) + if: ${{ github.event_name != 'pull_request' }} + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + + strategy: + fail-fast: false + matrix: + include: + - language: java-kotlin + build-mode: manual + - language: javascript-typescript + build-mode: none + - language: python + build-mode: none + + steps: + - name: Check out repository + uses: actions/checkout@v4 + with: + persist-credentials: false + + - name: Set up Java + if: matrix.language == 'java-kotlin' + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 21 + cache: maven + + - name: Ensure Maven wrapper is executable + if: matrix.language == 'java-kotlin' + run: chmod +x server/mvnw + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + + - name: Build Java for CodeQL + if: matrix.language == 'java-kotlin' + run: cd server && ./mvnw -q -DskipTests package + + - name: Analyze + uses: github/codeql-action/analyze@v3 + with: + category: /language:${{ matrix.language }} diff --git a/Makefile b/Makefile index 2272023a..bf3e7439 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ DEV_WEB_PID := $(DEV_DIR)/web.pid DEV_SERVER_LOG := $(DEV_DIR)/server.log DEV_WEB_LOG := $(DEV_DIR)/web.log DEV_WEB_URL := http://localhost:3000 +DEV_WEB_HOST ?= 127.0.0.1 DEV_API_URL := http://localhost:8080 DEV_SCANNER_URL := http://localhost:8000 STAGING_API_URL := http://localhost:8080 @@ -48,7 +49,7 @@ dev-all: ## 一键启动本地开发环境（依赖 + scanner + 后端 + 前端 echo "Frontend already running with PID $$(cat $(DEV_WEB_PID))"; \ else \ echo "Starting frontend..."; \ - $(DEV_PROCESS) start --pid-file $(DEV_WEB_PID) --log-file $(DEV_WEB_LOG) --cwd web -- pnpm exec vite --host 0.0.0.0 --strictPort >/dev/null; \ + $(DEV_PROCESS) start --pid-file $(DEV_WEB_PID) --log-file $(DEV_WEB_LOG) --cwd web -- pnpm exec vite --host $(DEV_WEB_HOST) --strictPort >/dev/null; \ fi @echo "Waiting for backend on $(DEV_API_URL) ..." @backend_ready=0; \ @@ -126,7 +127,7 @@ dev-all: ## 一键启动本地开发环境（依赖 + scanner + 后端 + 前端 @echo " Frontend: $(DEV_WEB_LOG)" dev-server: ## 启动后端开发服务器 - cd server && /bin/sh -lc '$(DEV_SERVER_PREPARE) && exec $(DEV_SERVER_CMD)' + cd server && /bin/sh -lc '$(DEV_SERVER_PREPARE) && exec env $(DEV_SERVER_SCANNER_ENV) $(DEV_SERVER_CMD)' dev-server-restart: ## 重启后端开发服务器 @mkdir -p $(DEV_DIR) @@ -237,7 +238,7 @@ web-install-ci: ## 以 CI 方式安装前端依赖 cd web && CI=true pnpm install --frozen-lockfile dev-web: ## 启动前端开发服务器 - cd web && pnpm run dev + cd web && pnpm exec vite --host $(DEV_WEB_HOST) build-frontend: web-deps ## 构建前端 cd web && pnpm run build @@ -278,13 +279,13 @@ lint-cli: ## CLI 代码检查 typecheck-cli: ## CLI 类型检查 cd cli && bun run typecheck -publish-cli: ## 发布 CLI（patch 版本）- bump + tag + push，触发 CI 自动发布 +publish-cli: ## 发布 CLI（patch 版本）- 本地 build+test → 推 release 分支 → 开 PR，合并后手动 tag 触发 CI ./scripts/publish-cli.sh patch -publish-cli-minor: ## 发布 CLI（minor 版本）- bump + tag + push，触发 CI 自动发布 +publish-cli-minor: ## 发布 CLI（minor 版本）- 本地 build+test → 推 release 分支 → 开 PR，合并后手动 tag 触发 CI ./scripts/publish-cli.sh minor -publish-cli-major: ## 发布 CLI（major 版本）- bump + tag + push，触发 CI 自动发布 +publish-cli-major: ## 发布 CLI（major 版本）- 本地 build+test → 推 release 分支 → 开 PR，合并后手动 tag 触发 CI ./scripts/publish-cli.sh major db-reset: ## 重置数据库 diff --git a/README.md b/README.md index d47ef5cc..2b068ab9 100644 --- a/README.md +++ b/README.md @@ -436,6 +436,12 @@ namespace `my-space` plus skill slug `my-skill`. 📖 **[Complete OpenClaw Integration Guide →](./docs/openclaw-integration.md)** +### [Hermes Agent](https://github.com/NousResearch/hermes-agent) + +[Hermes Agent](https://github.com/NousResearch/hermes-agent) uses the standard `SKILL.md` format and recursively discovers skills under `$HERMES_HOME/skills/`. Use SkillHub CLI's explicit `--dir` option to install a complete SkillHub package into Hermes without a registry adapter, then verify it with `hermes skills list`. + +📖 **[Complete Hermes Agent Integration Guide →](./docs/hermes-integration-en.md)** + ### [AstronClaw](https://agent.xfyun.cn/astron-claw) [AstronClaw](https://agent.xfyun.cn/astron-claw) is a cloud AI assistant built on OpenClaw's core capabilities, providing 24/7 online service through enterprise platforms like WeChat Work, DingTalk, and Feishu. It features a built-in skill system with over 130 official skills. You can connect it to a self-hosted SkillHub registry to enable one-click skill installation, search repository, dialogue-based automatic installation, and even custom skills management within your organization. diff --git a/README_zh.md b/README_zh.md index 80e65b75..42fb8225 100644 --- a/README_zh.md +++ b/README_zh.md @@ -370,6 +370,12 @@ namespace `my-space` 和 skill slug `my-skill`。 📖 **[完整 OpenClaw 集成指南 →](./docs/openclaw-integration.md)** +### [Hermes Agent](https://github.com/NousResearch/hermes-agent) + +[Hermes Agent](https://github.com/NousResearch/hermes-agent) 使用标准 `SKILL.md` 格式，并会递归发现 `$HERMES_HOME/skills/` 中的技能。通过 SkillHub CLI 的 `--dir` 参数即可把完整技能包安装到 Hermes，无需新增 registry 适配器；安装后可使用 `hermes skills list` 验证。 + +📖 **[完整 Hermes Agent 集成指南 →](./docs/hermes-integration.md)** + ### [AstronClaw](https://agent.xfyun.cn/astron-claw) [AstronClaw](https://agent.xfyun.cn/astron-claw) 是基于 OpenClaw 核心能力打造的云端 AI 助手，提供全天候在线服务，随时随地通过企业微信、钉钉、飞书等渠道提供服务。它内置了丰富的技能系统，您可以将其连接到自托管的 SkillHub 注册中心，支持技能市场一键安装、仓库搜索、对话自动安装，甚至管理和分发组织内部的自定义私有技能。 diff --git a/cli/README.md b/cli/README.md index 3f64aec0..b2a8cdf3 100644 --- a/cli/README.md +++ b/cli/README.md @@ -112,6 +112,9 @@ Logout only removes the token for the specified registry, preserving registry co # Keyword search skillhub search pdf +# Search with a one-off token +skillhub search pdf --token sk_xxx + # List all skills (empty query) skillhub search "" --limit 50 @@ -127,6 +130,10 @@ Output format: `namespace/slug version summary` # Install to auto-detected Agent directory skillhub install pdf-parser +# Choose install scope explicitly +skillhub install pdf-parser --scope user +skillhub install pdf-parser --scope project --agent codex + # Specify namespace (default: global) skillhub install pdf-parser --namespace myspace @@ -150,18 +157,21 @@ skillhub install pdf-parser --force The CLI determines the installation location using the following logic: -1. If `--dir` is specified: Install to that directory, agent marked as `custom` -2. If `--agent` is specified: Install to the corresponding Agent's skills directory -3. If neither is specified: Auto-scan current directory to detect existing Agent config directories - - 1 Agent detected → Install directly - - Multiple Agents detected → Interactive selection (TTY mode) or error (non-interactive mode) - - No Agent detected → Fallback to `/.agents/skills/` +1. If `--dir` is specified: Install to that directory, agent marked as `custom`. `--dir` is mutually exclusive with `--scope` and `--agent`. +2. If `--scope user|project` is specified: Limit detection to the chosen scope. + - With `--agent `: Install to that profile's user or project skills directory directly. + - Without `--agent`: Detect existing skills directories within the chosen scope only. In interactive user scope, the `generic` target (`/.agents/skills/`) is always also offered and can be selected alone or together with detected targets. + - No detected directory in the chosen scope → Fallback to `/.agents/skills/` for `--scope user` or `/.agents/skills/` for `--scope project`. +3. If `--agent` is specified (no `--scope`): Install to the corresponding Agent's skills directory (existing behaviour, unchanged). +4. If none of the above is specified: + - **Interactive mode** (stdin and stdout are both TTY, no `--json`): Prompt for `user` or `project` scope first, then continue per the `--scope` rule above. + - **Non-interactive mode**: Auto-scan current directory to detect existing Agent config directories. 1 Agent detected → install directly; multiple → error; none detected → fallback to `/.agents/skills/`. -> `--dir` and `--agent` cannot be used together. +> `--dir` cannot be combined with `--scope` or `--agent`. ### Install Paths -Each Agent has both project-level and user-level skills directories: +Each Agent has both project-level and user-level skills directories. Use `--scope user|project` to control which one is used. | Agent | Project-level Path | User-level Path | |-------|-------------------|-----------------| @@ -169,9 +179,9 @@ Each Agent has both project-level and user-level skills directories: | `codex` | `/.codex/skills/` | `~/.codex/skills/` | | `cursor` | `/.cursor/skills/` | `~/.cursor/skills/` | | `github-copilot` | `/.github-copilot/skills/` | `~/.github-copilot/skills/` | -| `gemini-cli` | `/.gemini-cli/skills/` | `~/.gemini-cli/skills/` | +| `gemini-cli` | `/.gemini/skills/` | `~/.gemini/skills/` | | `windsurf` | `/.windsurf/skills/` | `~/.windsurf/skills/` | -| `kiro-cli` | `/.kiro-cli/skills/` | `~/.kiro-cli/skills/` | +| `kiro-cli` | `/.kiro/skills/` | `~/.kiro/skills/` | | `roo` | `/.roo/skills/` | `~/.roo/skills/` | | `trae` | `/.trae/skills/` | `~/.trae/skills/` | | `trae-cn` | `/.trae-cn/skills/` | `~/.trae-cn/skills/` | @@ -179,8 +189,9 @@ Each Agent has both project-level and user-level skills directories: | `openclaw` | `/.openclaw/skills/` | `~/.openclaw/skills/` | | `opencode` | `/.opencode/skills/` | `~/.opencode/skills/` | | `kilo` | `/.kilo/skills/` | `~/.kilo/skills/` | +| _fallback_ | `/.agents/skills/` | `~/.agents/skills/` | -For Agents not in the list, use `--dir` to specify the installation path. +For a custom path or an unsupported Agent directory, use `--dir` to specify the installation path. In interactive user scope, the `generic` target is offered alongside detected Agent targets. When `--scope user|project` finds no matching agent directory, the CLI falls back to the `_fallback_` row above. ### File Structure After Installation @@ -325,8 +336,8 @@ Update mechanism: | `skillhub login --token [--registry ] [--json]` | Save token and registry configuration | | `skillhub logout [--registry ] [--json]` | Remove token for specified registry | | `skillhub whoami [--registry ] [--token ] [--json]` | Validate current token and display user information | -| `skillhub search [--registry ] [--limit ] [--json]` | Search published skills | -| `skillhub install [--namespace ] [--version ] [--agent ] [--dir ] [--force] [--registry ] [--token ] [--json]` | Install a skill | +| `skillhub search [--registry ] [--token ] [--limit ] [--json]` | Search published skills | +| `skillhub install [--scope ] [--namespace ] [--version ] [--agent ] [--dir ] [--force] [--registry ] [--token ] [--json]` | Install a skill | | `skillhub list [--agent ] [--dir ] [--registry ] [--json]` | List installed skills | | `skillhub remove [--agent ] [--all] [--remote] [--hard] [--namespace ] [--registry ] [--token ] [--json]` | Remove a skill | | `skillhub doctor [--json]` | Scan project directory and rebuild local inventory | diff --git a/cli/RELEASE.md b/cli/RELEASE.md index 45e0776a..b0d5e530 100644 --- a/cli/RELEASE.md +++ b/cli/RELEASE.md @@ -2,7 +2,14 @@ ## Overview -CLI releases are fully automated. Running `make publish-cli` on a clean `main` branch bumps the version, commits, creates a `cli-vX.Y.Z` tag, and pushes everything to origin. The GitHub Actions workflow [`release-cli.yml`](../.github/workflows/release-cli.yml) listens for the tag and handles build, test, npm publish, and GitHub Release creation. +CLI releases use a PR-based flow. Running `make publish-cli` on a clean `main` branch: + +1. Runs local build-and-test (lint, typecheck, test, build) +2. Computes the next version from the latest `cli-v*` tag on `origin` +3. Creates a `release/cli-vX.Y.Z` branch with the version bump committed +4. Pushes the branch and opens a PR to `main` + +After the PR is merged, you manually tag and push — the tag triggers [`release-cli.yml`](../.github/workflows/release-cli.yml) which builds, publishes to npm, and creates a GitHub Release. ## Prerequisites @@ -21,8 +28,9 @@ Configure in GitHub repository → Settings → Secrets and variables → Action ### Local Environment -- `node` and `npm` installed (the script uses `npm version` to bump) -- `git` installed with push access to the repository +- `node` and `bun` installed +- `gh` CLI installed and authenticated (`gh auth login`) +- `git` with push access to the repository - On the `main` branch with a clean working tree ### Package Configuration @@ -40,7 +48,7 @@ In [`cli/package.json`](./package.json): ## Release Process -### One-shot Release +### Step 1: Run the publish script From the repository root, on a clean `main` branch: @@ -52,15 +60,31 @@ make publish-cli-major # major: 0.1.5 -> 1.0.0 [`scripts/publish-cli.sh`](../scripts/publish-cli.sh) performs the following steps: -1. Verify the working tree is clean -2. Require the current branch to be `main`, otherwise abort +1. Verify `gh` CLI is installed and authenticated +2. Verify the working tree is clean and on `main` 3. `git pull --ff-only` from `origin/main` -4. Fetch remote tags and align `package.json` with the latest `cli-v*` tag -5. Compute the new version via `npm version ` -6. Verify the new tag does not exist locally or on origin -7. After interactive confirmation: commit the bump, create the `cli-vX.Y.Z` tag, push both commit and tag to origin +4. Run full local build-and-test (lint, typecheck, test, build) +5. Compute the next version from the latest `cli-v*` tag on `origin` (via `git ls-remote`, so local orphan tags from a failed `git push origin cli-vX.Y.Z` are ignored) +6. Verify the tag and release branch don't already exist +7. After interactive confirmation: create release branch, commit version bump, push, and open PR -Pushing the tag triggers CI — no further manual action required. +### Step 2: Merge the PR + +Review and merge the PR on GitHub as usual. + +### Step 3: Tag and push + +After the PR is merged: + +```bash +git fetch origin main +git tag cli-vX.Y.Z origin/main # replace with the actual version +git push origin cli-vX.Y.Z +``` + +This ensures the tag is always placed on the merge commit on `origin/main`, regardless of your local branch state. + +Pushing the tag triggers CI which builds, publishes to npm, and creates a GitHub Release. ### CI Workflow @@ -104,6 +128,22 @@ From the Actions UI: 2. Enter an existing tag name matching `cli-vX.Y.Z` 3. Optionally enable skip npm publish +## Error Recovery + +The script uses a cleanup state machine. If it fails at different stages: + +- **Before push**: release branch is deleted locally, you're returned to `main` +- **After push, before PR**: the script prints recovery instructions (open PR manually or delete the remote branch) +- **After PR opened**: success — no cleanup needed + +If you need to manually clean up a failed release: + +```bash +git checkout main +git branch -D release/cli-vX.Y.Z # delete local branch +git push origin --delete release/cli-vX.Y.Z # delete remote branch (if pushed) +``` + ## Troubleshooting ### `releases must be cut from 'main'` @@ -118,6 +158,10 @@ Commit or stash local changes first. The previous release didn't clean up, or someone else released the same version. Check `git tag --list 'cli-v*'` and remote tags, then retry with a higher version. +### `branch release/cli-vX.Y.Z already exists` + +A previous release attempt left a stale branch. Delete it locally and/or on origin, then retry. + ### npm Publish Fails - **403 with 2FA message**: `NPM_TOKEN` is not an Automation Token, or bypass 2FA is not enabled — regenerate with the correct type diff --git a/cli/package.json b/cli/package.json index 6898bffb..a7b94f7d 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@astron-team/skillhub", - "version": "0.1.6", + "version": "0.1.9", "description": "Manage and install skills for AI coding agents", "keywords": [ "skillhub", diff --git a/cli/src/agents/resolver.ts b/cli/src/agents/resolver.ts index 3925bd4b..190e1a8d 100644 --- a/cli/src/agents/resolver.ts +++ b/cli/src/agents/resolver.ts @@ -1,6 +1,7 @@ import { homedir } from 'node:os' import { CliError } from '../shared/errors' import { EXIT } from '../shared/constants' +import { canonicalizeExistingPath, pathExists } from '../platform/paths' import type { AgentCandidate } from './types' import { allProfiles, profileMap } from './detector' @@ -9,20 +10,31 @@ export interface ResolveInstallTargetOptions { home?: string | undefined dir?: string | undefined agents?: string[] | undefined + scope?: 'user' | 'project' | undefined json: boolean interactive: boolean detected?: AgentCandidate[] | undefined } export async function resolveInstallTargets(options: ResolveInstallTargetOptions): Promise { - if (options.dir && options.agents?.length) { + const agentList = options.agents ?? [] + + if (options.dir && agentList.length > 0) { throw new CliError('--dir cannot be used with --agent', EXIT.usage) } + if (options.dir && options.scope !== undefined) { + throw new CliError('--dir cannot be used with --scope', EXIT.usage) + } if (options.dir) { return [{ agent: 'custom', rootDir: options.dir, scope: 'user', source: 'explicit' }] } - if (options.agents?.length) { - const resolved = await resolveExplicitAgents(options.agents, options.cwd, options.home ?? homedir()) + + if (options.scope !== undefined) { + return resolveScopedTargets(options, agentList) + } + + if (agentList.length > 0) { + const resolved = await resolveExplicitAgents(agentList, options.cwd, options.home ?? homedir()) return dedupeByRoot(resolved) } const detected = options.detected ?? await detectAll(options.cwd, options.home ?? '') @@ -39,6 +51,68 @@ export async function resolveInstallTargets(options: ResolveInstallTargetOptions return [{ agent: 'generic', rootDir: `${options.cwd}/.agents/skills`, scope: 'project', source: 'fallback' }] } +async function resolveScopedTargets( + options: ResolveInstallTargetOptions, + agentList: string[] +): Promise { + const scope = options.scope! + const scopedHome = options.home ?? homedir() + + let candidates: AgentCandidate[] + if (agentList.length > 0) { + candidates = await resolveExplicitAgents(agentList, options.cwd, scopedHome, scope) + } else if (options.detected !== undefined) { + candidates = options.detected.filter(c => c.scope === scope) + } else { + candidates = await generateScopedCandidates(scope, options.cwd, scopedHome) + } + candidates = await dedupeByRoot(candidates) + + if (scope === 'user' && agentList.length === 0 && options.interactive && !options.json) { + candidates = await dedupeByRoot([ + ...candidates, + { + agent: 'generic', + rootDir: `${scopedHome}/.agents/skills`, + scope: 'user', + source: 'fallback' + } + ]) + } + + if (candidates.length === 0) { + const fallbackRoot = scope === 'user' + ? `${scopedHome}/.agents/skills` + : `${options.cwd}/.agents/skills` + return [{ agent: 'generic', rootDir: fallbackRoot, scope, source: 'fallback' }] + } + if (candidates.length === 1) return candidates + if (options.interactive && !options.json) { + return selectTargetsInteractively(candidates) + } + throw new CliError('multiple install targets detected', EXIT.usage, { + next: 'pass --agent or --dir', + candidates + }) +} + +async function generateScopedCandidates( + scope: 'user' | 'project', + cwd: string, + home: string +): Promise { + const results: AgentCandidate[] = [] + for (const profile of allProfiles) { + const roots = scope === 'user' ? profile.userRoots(home) : profile.projectRoots(cwd) + for (const root of roots) { + if (await pathExists(root)) { + results.push({ agent: profile.id, rootDir: root, scope, source: 'detected' }) + } + } + } + return results +} + async function detectAll(cwd: string, home: string): Promise { const results: AgentCandidate[] = [] for (const profile of allProfiles) { @@ -48,7 +122,12 @@ async function detectAll(cwd: string, home: string): Promise { return dedupeByRoot(results) } -async function resolveExplicitAgents(agents: string[], cwd: string, home?: string): Promise { +async function resolveExplicitAgents( + agents: string[], + cwd: string, + home: string, + scope?: 'user' | 'project' +): Promise { const results: AgentCandidate[] = [] for (const agentId of agents) { const profile = profileMap.get(agentId) @@ -57,34 +136,48 @@ async function resolveExplicitAgents(agents: string[], cwd: string, home?: strin next: 'use a supported agent profile or pass --dir' }) } - const userRoots = home ? profile.userRoots(home) : [] - const roots = userRoots.length > 0 ? userRoots : profile.projectRoots(cwd) - if (roots.length > 0) { - results.push(...roots.map(root => { - const scope: AgentCandidate['scope'] = root.startsWith(cwd) ? 'project' : 'user' - return { + let roots: string[] + if (scope === 'user') { + roots = profile.userRoots(home) + } else if (scope === 'project') { + roots = profile.projectRoots(cwd) + } else { + const userRoots = home ? profile.userRoots(home) : [] + roots = userRoots.length > 0 ? userRoots : profile.projectRoots(cwd) + } + const userRootSet = new Set(home ? profile.userRoots(home) : []) + for (const root of roots) { + const candidateScope: AgentCandidate['scope'] = scope !== undefined + ? scope + : (userRootSet.has(root) ? 'user' : 'project') + results.push({ agent: agentId, rootDir: root, - scope, - source: 'explicit' as const - } - })) + scope: candidateScope, + source: 'explicit' + }) } } return results } -function dedupeByRoot(candidates: AgentCandidate[]): AgentCandidate[] { +async function dedupeByRoot(candidates: AgentCandidate[]): Promise { const seen = new Set() - return candidates.filter(c => { - if (seen.has(c.rootDir)) return false - seen.add(c.rootDir) - return true - }) + const deduped: AgentCandidate[] = [] + + for (const candidate of candidates) { + const canonicalRootDir = await canonicalizeExistingPath(candidate.rootDir) + if (seen.has(canonicalRootDir)) continue + seen.add(canonicalRootDir) + deduped.push(candidate) + } + + return deduped } async function selectTargetsInteractively(candidates: AgentCandidate[]): Promise { const prompts = await import('prompts') + let highlightedIndex = 0 const { selected } = await prompts.default({ type: 'multiselect', name: 'selected', @@ -92,7 +185,13 @@ async function selectTargetsInteractively(candidates: AgentCandidate[]): Promise choices: candidates.map(c => ({ title: `${c.agent} (${c.rootDir})`, value: c - })) + })), + onRender: function (this: { cursor?: number }) { + highlightedIndex = this.cursor ?? highlightedIndex + }, + format: (selectedTargets: AgentCandidate[]) => ( + selectedTargets.length > 0 ? selectedTargets : [candidates[highlightedIndex] ?? candidates[0]!] + ) }) if (!selected || selected.length === 0) { throw new CliError('installation cancelled', EXIT.usage) diff --git a/cli/src/clients/skillhub-client.ts b/cli/src/clients/skillhub-client.ts index 2d3a1793..8d008483 100644 --- a/cli/src/clients/skillhub-client.ts +++ b/cli/src/clients/skillhub-client.ts @@ -44,6 +44,19 @@ export interface PublishResponse { visibility: string } +export interface DryRunResponse { + valid: boolean + errors: string[] + warnings: string[] + resolvedSlug: string | null + resolvedVersion: string | null +} + +interface ErrorEnvelope { + msg?: unknown + requestId?: unknown +} + export class SkillHubClient { constructor( readonly registry: string, @@ -80,9 +93,12 @@ export class SkillHubClient { } catch { throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' }) } - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' }) } + if (response.status === 403) { + throw await this.createAccessDeniedError(response) + } if (response.status === 404) { throw new CliError('skill or version not found', EXIT.generic, { registry: this.registry }) } @@ -113,6 +129,23 @@ export class SkillHubClient { return this.handleJsonResponse(response) } + async validatePublish(namespace: string, file: Blob, visibility: string, fileName = 'skill.zip'): Promise { + const formData = new FormData() + formData.append('file', file, fileName) + formData.append('visibility', visibility) + let response: Response + try { + response = await this.fetchImpl(`${this.registry}/api/cli/v1/skills/${namespace}/publish/validate`, { + method: 'POST', + headers: this.token ? { Authorization: `Bearer ${this.token}` } : {}, + body: formData + }) + } catch { + throw new CliError('registry unreachable', EXIT.network, { registry: this.registry, next: 'check network or pass --registry' }) + } + return this.handleJsonResponse(response) + } + private async getJson(path: string): Promise { let response: Response try { @@ -126,9 +159,12 @@ export class SkillHubClient { } private async handleJsonResponse(response: Response): Promise { - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { throw new CliError('authentication failed', EXIT.auth, { registry: this.registry, next: 'run `skillhub login`' }) } + if (response.status === 403) { + throw await this.createAccessDeniedError(response) + } if (response.status === 404) { throw new CliError('resource not found', EXIT.generic, { registry: this.registry }) } @@ -144,6 +180,26 @@ export class SkillHubClient { return body.data as T } + private async createAccessDeniedError(response: Response): Promise { + const error = await this.readErrorEnvelope(response) + return new CliError(error.message ?? 'access denied', EXIT.auth, { + registry: this.registry, + ...(error.requestId ? { requestId: error.requestId } : {}) + }) + } + + private async readErrorEnvelope(response: Response): Promise<{ message?: string; requestId?: string }> { + try { + const body = await response.json() as ErrorEnvelope + return { + ...(typeof body.msg === 'string' && body.msg.trim() ? { message: body.msg } : {}), + ...(typeof body.requestId === 'string' && body.requestId.trim() ? { requestId: body.requestId } : {}) + } + } catch { + return {} + } + } + private headers(): HeadersInit { return this.token ? { Authorization: `Bearer ${this.token}` } : {} } diff --git a/cli/src/commands/help.ts b/cli/src/commands/help.ts index e463a39e..9b35f3b4 100644 --- a/cli/src/commands/help.ts +++ b/cli/src/commands/help.ts @@ -28,13 +28,17 @@ export const commands = { }, search: { summary: 'Search published skills', - usage: 'skillhub search [query] [--limit ] [--registry ] [--json]', - examples: ['skillhub search', 'skillhub search pdf'] + usage: 'skillhub search [query] [--limit ] [--registry ] [--token ] [--json]', + examples: ['skillhub search', 'skillhub search pdf', 'skillhub search pdf --token sk_xxx'] }, install: { summary: 'Install a skill locally', - usage: 'skillhub install [--namespace ] [--version ] [--agent ] [--dir ] [--force] [--json]', - examples: ['skillhub install pdf-parser', 'skillhub install pdf-parser --agent codex'] + usage: 'skillhub install [--scope ] [--namespace ] [--version ] [--agent ] [--dir ] [--force] [--json]', + examples: [ + 'skillhub install pdf-parser', + 'skillhub install pdf-parser --scope user', + 'skillhub install pdf-parser --scope project --agent codex' + ] }, list: { summary: 'List local installs', diff --git a/cli/src/commands/install.ts b/cli/src/commands/install.ts index cc334131..0feed791 100644 --- a/cli/src/commands/install.ts +++ b/cli/src/commands/install.ts @@ -3,34 +3,113 @@ import { CredentialsStore } from '../stores/credentials-store' import { resolveRegistry, resolveToken } from '../services/registry-service' import { installSkill } from '../services/install-service' import { resolveInstallTargets } from '../agents/resolver' +import { CliError } from '../shared/errors' +import { EXIT } from '../shared/constants' +import { parseSkillName } from '../shared/skill-name-parser' export interface InstallCommandOptions { namespace?: string | undefined version?: string | undefined agent?: string[] | undefined dir?: string | undefined + scope?: string | undefined force?: boolean | undefined registry?: string | undefined token?: string | undefined json?: boolean | undefined } -export async function installCommand(slug: string, options: InstallCommandOptions): Promise { +export interface InstallCommandDeps { + promptScope?: () => Promise<'user' | 'project'> + resolveInstallTargets?: typeof resolveInstallTargets + installSkill?: typeof installSkill + isTTY?: () => boolean +} + +export function computeStrictIsTTY(env: { + stdinIsTTY: boolean + stdoutIsTTY: boolean + json: boolean +}): boolean { + return env.stdinIsTTY && env.stdoutIsTTY && !env.json +} + +export async function resolveEffectiveScope( + options: InstallCommandOptions, + env: { isTTY: boolean; promptScope: () => Promise<'user' | 'project'> } +): Promise<'user' | 'project' | undefined> { + if (options.scope !== undefined && options.scope !== 'user' && options.scope !== 'project') { + throw new CliError('--scope must be "user" or "project"', EXIT.usage) + } + const scope = options.scope as 'user' | 'project' | undefined + const agentList = options.agent ?? [] + + if (options.dir && scope !== undefined) { + throw new CliError('--dir cannot be used with --scope', EXIT.usage) + } + if (options.dir && agentList.length > 0) { + throw new CliError('--dir cannot be used with --agent', EXIT.usage) + } + + if (scope !== undefined) return scope + if (options.dir || agentList.length > 0) return undefined + if (env.isTTY) return await env.promptScope() + return undefined +} + +async function defaultPromptScope(): Promise<'user' | 'project'> { + const prompts = await import('prompts') + const { scope } = await prompts.default({ + type: 'select', + name: 'scope', + message: 'Install for user or project?', + choices: [ + { title: 'User (install to user-level agent directory)', value: 'user' }, + { title: 'Project (install to project-level agent directory)', value: 'project' } + ] + }) + if (!scope) { + throw new CliError('installation cancelled', EXIT.usage) + } + return scope +} + +export async function installCommand( + skillNameArg: string, + options: InstallCommandOptions, + deps: InstallCommandDeps = {} +): Promise { + const isTTYFn = deps.isTTY ?? (() => computeStrictIsTTY({ + stdinIsTTY: process.stdin.isTTY === true, + stdoutIsTTY: process.stdout.isTTY === true, + json: Boolean(options.json) + })) + const isTTY = isTTYFn() + + const promptScope = deps.promptScope ?? defaultPromptScope + const effectiveScope = await resolveEffectiveScope(options, { isTTY, promptScope }) + const configStore = new ConfigStore() const credentialsStore = new CredentialsStore() const registry = resolveRegistry(options, process.env, await configStore.read()) const token = resolveToken(options, process.env, await credentialsStore.getToken(registry)) - const namespace = options.namespace ?? 'global' - const targets = await resolveInstallTargets({ + const parsed = parseSkillName(skillNameArg) + const namespace = options.namespace ?? parsed.namespace + const slug = parsed.slug + + const resolveTargets = deps.resolveInstallTargets ?? resolveInstallTargets + const targets = await resolveTargets({ cwd: process.cwd(), + scope: effectiveScope, dir: options.dir, agents: options.agent ?? [], json: Boolean(options.json), - interactive: process.stdout.isTTY === true + interactive: isTTY }) - const result = await installSkill({ + const installFn = deps.installSkill ?? installSkill + const result = await installFn({ registry, token, namespace, slug, version: options.version, targets, diff --git a/cli/src/commands/publish.ts b/cli/src/commands/publish.ts index 16b4453c..1c73202f 100644 --- a/cli/src/commands/publish.ts +++ b/cli/src/commands/publish.ts @@ -14,6 +14,7 @@ export interface PublishCommandOptions { registry?: string token?: string json?: boolean + dryRun?: boolean } export async function publishCommand(path: string, options: PublishCommandOptions): Promise { @@ -40,7 +41,6 @@ export async function publishCommand(path: string, options: PublishCommandOption let archiveBlob: Blob let archiveName: string if (pathStat.isFile()) { - // If input is a file, check if it's already a zip if (await isZipFile(path)) { const buffer = await readFile(path) archiveBlob = new Blob([buffer], { type: 'application/zip' }) @@ -49,7 +49,6 @@ export async function publishCommand(path: string, options: PublishCommandOption throw new CliError(`file must be a zip archive: ${path}`, EXIT.filesystem, { path }) } } else if (pathStat.isDirectory()) { - // If input is a directory, create zip from it archiveBlob = await createZip(path) archiveName = `${basename(path)}.zip` } else { @@ -57,6 +56,49 @@ export async function publishCommand(path: string, options: PublishCommandOption } const client = new SkillHubClient(registry, token) + + if (options.dryRun) { + const result = await client.validatePublish(namespace, archiveBlob, toServerVisibility(visibility), archiveName) + + if (options.json) { + if (!result.valid) { + process.stdout.write(JSON.stringify(result) + '\n') + throw new CliError('validation failed', EXIT.validation) + } + return JSON.stringify(result) + } + + const lines: string[] = [] + if (result.valid) { + lines.push('Validation passed') + } else { + lines.push('Validation failed') + } + if (result.resolvedSlug) { + lines.push(` Slug: ${result.resolvedSlug}`) + } + if (result.resolvedVersion) { + lines.push(` Version: ${result.resolvedVersion}`) + } + if (result.errors.length > 0) { + lines.push('Errors:') + for (const error of result.errors) { + lines.push(` - ${error}`) + } + } + if (result.warnings.length > 0) { + lines.push('Warnings:') + for (const warning of result.warnings) { + lines.push(` - ${warning}`) + } + } + if (!result.valid) { + process.stdout.write(lines.join('\n') + '\n') + throw new CliError('validation failed', EXIT.validation) + } + return lines.join('\n') + } + const result = await client.publish(namespace, archiveBlob, toServerVisibility(visibility), archiveName) const detailUrl = `${registry}/space/${result.namespace}/${encodeURIComponent(result.slug)}` diff --git a/cli/src/commands/remove.ts b/cli/src/commands/remove.ts index 67f47f1d..4e8543b7 100644 --- a/cli/src/commands/remove.ts +++ b/cli/src/commands/remove.ts @@ -5,6 +5,7 @@ import { resolveRegistry, resolveToken } from '../services/registry-service' import { removeLocalSkill } from '../services/remove-service' import { CliError } from '../shared/errors' import { EXIT } from '../shared/constants' +import { parseSkillName } from '../shared/skill-name-parser' export interface RemoveCommandOptions { agent?: string[] | undefined @@ -17,7 +18,7 @@ export interface RemoveCommandOptions { json?: boolean | undefined } -export async function removeCommand(slug: string, options: RemoveCommandOptions): Promise { +export async function removeCommand(skillNameArg: string, options: RemoveCommandOptions): Promise { if (options.all && options.agent?.length) { throw new CliError('--all cannot be used with --agent', EXIT.usage) } @@ -29,9 +30,12 @@ export async function removeCommand(slug: string, options: RemoveCommandOptions) const credentialsStore = new CredentialsStore() const registry = resolveRegistry(options, process.env, await configStore.read()) + const parsed = parseSkillName(skillNameArg) + const namespace = options.namespace ?? parsed.namespace + const slug = parsed.slug + if (options.remote) { const token = resolveToken(options, process.env, await credentialsStore.getToken(registry)) - const namespace = options.namespace ?? 'global' if (!options.hard && process.stdout.isTTY) { const prompts = await import('prompts') diff --git a/cli/src/generated/pkg-info.ts b/cli/src/generated/pkg-info.ts index 66c7ded3..9445834b 100644 --- a/cli/src/generated/pkg-info.ts +++ b/cli/src/generated/pkg-info.ts @@ -1,3 +1,3 @@ // Generated by scripts/generate-pkg-info.ts - do not edit by hand. export const PKG_NAME = "@astron-team/skillhub" -export const PKG_VERSION = "0.1.6" +export const PKG_VERSION = "0.1.9" diff --git a/cli/src/index.ts b/cli/src/index.ts index 97f3f34f..512b5b1b 100644 --- a/cli/src/index.ts +++ b/cli/src/index.ts @@ -223,9 +223,10 @@ cli cli .command('search [query]', 'Search published skills') .option('--registry ', 'Registry URL') + .option('--token ', 'API token') .option('--limit ', 'Max results', { default: 20 }) .option('--json', 'Output JSON') - .action((query: string | undefined, options: { registry?: string; limit?: number; json?: boolean }) => { + .action((query: string | undefined, options: { registry?: string; token?: string; limit?: number; json?: boolean }) => { return runCommand(() => searchCommand(query ?? '', options), Boolean(options.json)) }) @@ -233,6 +234,7 @@ cli .command('install ', 'Install a skill locally') .option('--namespace ', 'Namespace', { default: 'global' }) .option('--version ', 'Version') + .option('--scope ', 'Install scope: user or project') .option('--agent ', 'Agent profile (repeatable)') .option('--dir ', 'Install directory') .option('--force', 'Overwrite existing') @@ -278,6 +280,7 @@ cli .command('publish ', 'Publish a local skill package') .option('--namespace ', 'Namespace') .option('--visibility ', 'Visibility (public|namespace-only|private)') + .option('--dry-run', 'Validate without publishing') .option('--registry ', 'Registry URL') .option('--token ', 'API token') .option('--json', 'Output JSON') diff --git a/cli/src/platform/archive.ts b/cli/src/platform/archive.ts index 1e420567..f549185d 100644 --- a/cli/src/platform/archive.ts +++ b/cli/src/platform/archive.ts @@ -1,6 +1,14 @@ import { mkdir, readdir, readFile, writeFile } from 'node:fs/promises' import { dirname, isAbsolute, join, relative, resolve } from 'node:path' import { zipSync, unzipSync } from 'fflate' +import { MAX_PACKAGE_BYTES } from './download' + +const MAX_ZIP_ENTRIES = 500 +const MAX_SINGLE_FILE_BYTES = 10 * 1024 * 1024 +const EOCD_SIGNATURE = 0x06054b50 +const CENTRAL_DIRECTORY_SIGNATURE = 0x02014b50 +const ZIP64_MARKER_16 = 0xffff +const ZIP64_MARKER_32 = 0xffffffff /** * Extract a zip archive buffer into the target directory. @@ -8,9 +16,15 @@ import { zipSync, unzipSync } from 'fflate' */ export async function extractZip(buffer: ArrayBuffer, targetDir: string): Promise { await mkdir(targetDir, { recursive: true }) - const files = unzipSync(new Uint8Array(buffer)) - for (const [name, data] of Object.entries(files)) { - const filePath = safeJoin(targetDir, name) + const archive = new Uint8Array(buffer) + validateZipCentralDirectory(archive) + const files = unzipSync(archive) + const entries = Object.entries(files).map(([name, data]) => ({ + name, + data, + filePath: safeJoin(targetDir, name), + })) + for (const { name, data, filePath } of entries) { if (name.endsWith('/')) { await mkdir(filePath, { recursive: true }) continue @@ -20,6 +34,78 @@ export async function extractZip(buffer: ArrayBuffer, targetDir: string): Promis } } +function validateZipCentralDirectory(archive: Uint8Array): void { + const view = new DataView(archive.buffer, archive.byteOffset, archive.byteLength) + const eocdOffset = findEndOfCentralDirectory(view) + if (eocdOffset < 0) { + throw new Error('invalid zip central directory') + } + + const diskNumber = view.getUint16(eocdOffset + 4, true) + const centralDirectoryDisk = view.getUint16(eocdOffset + 6, true) + const entriesOnDisk = view.getUint16(eocdOffset + 8, true) + const totalEntries = view.getUint16(eocdOffset + 10, true) + const centralDirectorySize = view.getUint32(eocdOffset + 12, true) + const centralDirectoryOffset = view.getUint32(eocdOffset + 16, true) + + if ( + entriesOnDisk === ZIP64_MARKER_16 || + totalEntries === ZIP64_MARKER_16 || + centralDirectorySize === ZIP64_MARKER_32 || + centralDirectoryOffset === ZIP64_MARKER_32 + ) { + throw new Error('zip64 archives are not supported') + } + if (diskNumber !== 0 || centralDirectoryDisk !== 0 || entriesOnDisk !== totalEntries) { + throw new Error('multi-disk zip archives are not supported') + } + if (totalEntries > MAX_ZIP_ENTRIES) { + throw new Error('zip entry count exceeds limit') + } + if (centralDirectoryOffset + centralDirectorySize > archive.byteLength) { + throw new Error('invalid zip central directory') + } + + let offset = centralDirectoryOffset + let totalUncompressedSize = 0 + const decoder = new TextDecoder() + for (let i = 0; i < totalEntries; i++) { + if (offset + 46 > archive.byteLength || view.getUint32(offset, true) !== CENTRAL_DIRECTORY_SIGNATURE) { + throw new Error('invalid zip central directory') + } + const uncompressedSize = view.getUint32(offset + 24, true) + const nameLength = view.getUint16(offset + 28, true) + const extraLength = view.getUint16(offset + 30, true) + const commentLength = view.getUint16(offset + 32, true) + const nameStart = offset + 46 + const nameEnd = nameStart + nameLength + const nextOffset = nameEnd + extraLength + commentLength + if (nameEnd > archive.byteLength || nextOffset > archive.byteLength) { + throw new Error('invalid zip central directory') + } + + const entryName = decoder.decode(archive.subarray(nameStart, nameEnd)) + if (!entryName.endsWith('/') && uncompressedSize > MAX_SINGLE_FILE_BYTES) { + throw new Error('zip entry size exceeds limit') + } + totalUncompressedSize += uncompressedSize + if (totalUncompressedSize > MAX_PACKAGE_BYTES) { + throw new Error('zip total uncompressed size exceeds limit') + } + offset = nextOffset + } +} + +function findEndOfCentralDirectory(view: DataView): number { + const minOffset = Math.max(0, view.byteLength - 0xffff - 22) + for (let offset = view.byteLength - 22; offset >= minOffset; offset--) { + if (view.getUint32(offset, true) === EOCD_SIGNATURE) { + return offset + } + } + return -1 +} + /** * Create a zip archive from a directory. * Returns the archive as a Blob. diff --git a/cli/src/platform/download.ts b/cli/src/platform/download.ts new file mode 100644 index 00000000..30490731 --- /dev/null +++ b/cli/src/platform/download.ts @@ -0,0 +1,56 @@ +import { EXIT } from '../shared/constants' +import { CliError } from '../shared/errors' + +export const MAX_PACKAGE_BYTES = 100 * 1024 * 1024 + +export async function readBoundedResponseBody(response: Response, maxBytes = MAX_PACKAGE_BYTES): Promise { + const contentLength = response.headers.get('content-length') + if (contentLength !== null) { + const declaredLength = Number(contentLength) + if (Number.isFinite(declaredLength) && declaredLength > maxBytes) { + throw new CliError('download exceeds maximum package size', EXIT.network, { + contentLength: declaredLength, + maxBytes + }) + } + } + + if (!response.body) { + const buffer = await response.arrayBuffer() + if (buffer.byteLength > maxBytes) { + throw new CliError('download exceeds maximum package size', EXIT.network, { + receivedBytes: buffer.byteLength, + maxBytes + }) + } + return buffer + } + + const reader = response.body.getReader() + const chunks: Uint8Array[] = [] + let receivedBytes = 0 + + for (;;) { + const { done, value } = await reader.read() + if (done) { + break + } + receivedBytes += value.byteLength + if (receivedBytes > maxBytes) { + await reader.cancel() + throw new CliError('download exceeds maximum package size', EXIT.network, { + receivedBytes, + maxBytes + }) + } + chunks.push(value) + } + + const result = new Uint8Array(receivedBytes) + let offset = 0 + for (const chunk of chunks) { + result.set(chunk, offset) + offset += chunk.byteLength + } + return result.buffer +} diff --git a/cli/src/platform/paths.ts b/cli/src/platform/paths.ts index e139b2cc..7811a766 100644 --- a/cli/src/platform/paths.ts +++ b/cli/src/platform/paths.ts @@ -24,6 +24,15 @@ export async function pathExists(path: string): Promise { } } +export async function canonicalizeExistingPath(path: string): Promise { + const { realpath } = await import('node:fs/promises') + try { + return await realpath(path) + } catch { + return path + } +} + export async function applyCredentialPermissions(path: string): Promise { if (process.platform === 'win32') return const { chmod } = await import('node:fs/promises') diff --git a/cli/src/services/install-service.ts b/cli/src/services/install-service.ts index b989ce58..bba71345 100644 --- a/cli/src/services/install-service.ts +++ b/cli/src/services/install-service.ts @@ -1,11 +1,12 @@ -import { mkdir, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, rename, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { SkillHubClient } from '../clients/skillhub-client' import { InventoryStore } from '../stores/inventory-store' import { CliError } from '../shared/errors' import { EXIT } from '../shared/constants' import { extractZip } from '../platform/archive' -import { pathExists } from '../platform/paths' +import { readBoundedResponseBody } from '../platform/download' +import { canonicalizeExistingPath, pathExists } from '../platform/paths' import type { AgentCandidate } from '../agents/types' export interface InstallOptions { @@ -19,54 +20,105 @@ export interface InstallOptions { home?: string | undefined } -export async function installSkill(options: InstallOptions): Promise<{ installed: Array<{ agent: string; dir: string }> }> { - const client = new SkillHubClient(options.registry, options.token) - const resolved = await client.resolve(options.namespace, options.slug, options.version) - const response = await client.download(options.namespace, options.slug, resolved.version) - const buffer = await response.arrayBuffer() +async function preflightInstallTargets( + targets: AgentCandidate[], + slug: string, + force: boolean +): Promise> { + const seenSkillDirs = new Set() + const preparedTargets: Array<{ target: AgentCandidate; skillDir: string }> = [] - const installed: Array<{ agent: string; dir: string }> = [] - const store = new InventoryStore(options.home) + for (const target of targets) { + const canonicalRootDir = await canonicalizeExistingPath(target.rootDir) + const canonicalSkillDir = join(canonicalRootDir, slug) + if (seenSkillDirs.has(canonicalSkillDir)) { + throw new CliError(`multiple install targets resolve to ${canonicalSkillDir}`, EXIT.usage, { + path: canonicalSkillDir, + next: 'select only one target for this directory' + }) + } + seenSkillDirs.add(canonicalSkillDir) - for (const target of options.targets) { - const skillDir = join(target.rootDir, options.slug) - - if (await pathExists(skillDir) && !options.force) { + const skillDir = join(target.rootDir, slug) + if (await pathExists(skillDir) && !force) { throw new CliError(`skill already installed at ${skillDir}`, EXIT.filesystem, { path: skillDir, next: 'pass --force to overwrite' }) } + preparedTargets.push({ target, skillDir }) + } - if (await pathExists(skillDir) && options.force) { - await store.removeTargetsByInstallDir(skillDir) - await rm(skillDir, { recursive: true, force: true }) + return preparedTargets +} + +export async function installSkill(options: InstallOptions): Promise<{ installed: Array<{ agent: string; dir: string }> }> { + const preparedTargets = await preflightInstallTargets(options.targets, options.slug, options.force) + const client = new SkillHubClient(options.registry, options.token) + const resolved = await client.resolve(options.namespace, options.slug, options.version) + const response = await client.download(options.namespace, options.slug, resolved.version) + const buffer = await readBoundedResponseBody(response) + + const installed: Array<{ agent: string; dir: string }> = [] + const store = new InventoryStore(options.home) + + for (const { target, skillDir } of preparedTargets) { + await mkdir(target.rootDir, { recursive: true }) + const tempDir = await mkdtemp(join(target.rootDir, `.${options.slug}.install-`)) + let movedIntoPlace = false + + try { + await extractZip(buffer, tempDir) + + const installedAt = new Date().toISOString() + const metaDir = join(tempDir, '.skillhub') + await mkdir(metaDir, { recursive: true }) + await writeFile(join(metaDir, 'metadata.json'), JSON.stringify({ + registry: options.registry, + namespace: options.namespace, + slug: options.slug, + version: resolved.version, + agent: target.agent, + installedAt + }, null, 2)) + + if (await pathExists(skillDir) && !options.force) { + throw new CliError(`skill already installed at ${skillDir}`, EXIT.filesystem, { + path: skillDir, + next: 'pass --force to overwrite' + }) + } + + if (await pathExists(skillDir) && options.force) { + await store.removeTargetsByInstallDir(skillDir) + await rm(skillDir, { recursive: true, force: true }) + } + + try { + await rename(tempDir, skillDir) + } catch (error) { + if (!options.force && await pathExists(skillDir)) { + throw new CliError(`skill already installed at ${skillDir}`, EXIT.filesystem, { + path: skillDir, + next: 'pass --force to overwrite' + }) + } + throw error + } + movedIntoPlace = true + + await store.upsertTarget(options.registry, options.namespace, options.slug, resolved.version, { + agent: target.agent, + rootDir: target.rootDir, + installDir: skillDir, + installedAt + }) + } finally { + if (!movedIntoPlace) { + await rm(tempDir, { recursive: true, force: true }).catch(() => {}) + } } - // Create skill directory and extract into a clean skill-specific directory. - await mkdir(skillDir, { recursive: true }) - await extractZip(buffer, skillDir) - - // Write .skillhub/metadata.json - const metaDir = join(skillDir, '.skillhub') - await mkdir(metaDir, { recursive: true }) - await writeFile(join(metaDir, 'metadata.json'), JSON.stringify({ - registry: options.registry, - namespace: options.namespace, - slug: options.slug, - version: resolved.version, - agent: target.agent, - installedAt: new Date().toISOString() - }, null, 2)) - - // Update inventory - await store.upsertTarget(options.registry, options.namespace, options.slug, resolved.version, { - agent: target.agent, - rootDir: target.rootDir, - installDir: skillDir, - installedAt: new Date().toISOString() - }) - installed.push({ agent: target.agent, dir: skillDir }) } diff --git a/cli/src/shared/constants.ts b/cli/src/shared/constants.ts index e61a5ca9..76d78f74 100644 --- a/cli/src/shared/constants.ts +++ b/cli/src/shared/constants.ts @@ -8,5 +8,6 @@ export const EXIT = { auth: 2, network: 3, filesystem: 4, - usage: 5 + usage: 5, + validation: 6 } as const diff --git a/cli/src/shared/output.ts b/cli/src/shared/output.ts index 9b2eafd9..977116bc 100644 --- a/cli/src/shared/output.ts +++ b/cli/src/shared/output.ts @@ -30,6 +30,9 @@ export function renderError(error: unknown, json: boolean): string { if (typeof cliError.details.path === 'string') { lines.push(`Context: path ${cliError.details.path}`) } + if (typeof cliError.details.requestId === 'string') { + lines.push(`Request ID: ${cliError.details.requestId}`) + } if (typeof cliError.details.next === 'string') { lines.push(`Next: ${cliError.details.next}`) } diff --git a/cli/src/shared/skill-name-parser.ts b/cli/src/shared/skill-name-parser.ts new file mode 100644 index 00000000..05e0662b --- /dev/null +++ b/cli/src/shared/skill-name-parser.ts @@ -0,0 +1,27 @@ +export interface ParsedSkillName { + namespace: string + slug: string +} + +export function parseSkillName(skillName: string, defaultNamespace = 'global'): ParsedSkillName { + const separatorIndex = skillName.indexOf('--') + + if (separatorIndex <= 0) { + return { + namespace: defaultNamespace, + slug: separatorIndex === 0 ? skillName.slice(2) : skillName + } + } + + if (separatorIndex === skillName.length - 2) { + return { + namespace: defaultNamespace, + slug: skillName.slice(0, -2) + } + } + + return { + namespace: skillName.slice(0, separatorIndex), + slug: skillName.slice(separatorIndex + 2) + } +} diff --git a/cli/test/helpers/fake-registry.ts b/cli/test/helpers/fake-registry.ts index 7159cd2d..4fde3a95 100644 --- a/cli/test/helpers/fake-registry.ts +++ b/cli/test/helpers/fake-registry.ts @@ -22,16 +22,23 @@ export function createFakeRegistry(handlers: Record) { /** * Controls how a specific endpoint behaves when a failure is injected: * 'auth' => 401 { code: 401, message: 'unauthorized' } + * 'forbidden' => 403 with a standard SkillHub error envelope * 'not_found' => 404 { code: 404, message: 'not found' } * 'server_error' => 500 { code: 500, message: 'internal error' } * 'network' => handler throws, causing fetch() to reject with a TypeError */ -export type FailureMode = 'auth' | 'not_found' | 'server_error' | 'network' +export type FailureMode = 'auth' | 'forbidden' | 'not_found' | 'server_error' | 'network' function failureResponse(mode: FailureMode): Response { switch (mode) { case 'auth': return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + case 'forbidden': + return Response.json({ + code: 403, + msg: 'API token is missing required scope: skill:publish', + requestId: 'req-test-forbidden' + }, { status: 403 }) case 'not_found': return Response.json({ code: 404, message: 'not found' }, { status: 404 }) case 'server_error': @@ -91,6 +98,12 @@ export interface CapturedPublish { visibility: string } +export interface CapturedValidate { + namespace: string + fileName: string + visibility: string +} + /** Last resolve GET: useful for verifying --version is forwarded as ?version=. */ export interface CapturedResolve { namespace: string @@ -116,6 +129,8 @@ interface FakeRegistryOptions { searchItems?: Array<{ namespace: string; slug: string; latestVersion: string; summary: string }> /** Skills available for resolve / download / delete / publish. */ skills?: FakeSkill[] + /** Response to return for publish/validate (dry-run) requests. */ + dryRunResponse?: { valid: boolean; errors: string[]; warnings: string[]; resolvedSlug: string | null; resolvedVersion: string | null } /** * Per-endpoint failure injection. When set for an endpoint, that endpoint * ignores all other logic and returns the specified failure (or throws for @@ -128,6 +143,7 @@ interface FakeRegistryOptions { download?: FailureMode deleteRemote?: FailureMode publish?: FailureMode + validate?: FailureMode } } @@ -167,7 +183,8 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { publish: CapturedPublish | null resolve: CapturedResolve | null delete: CapturedDelete | null - } = { publish: null, resolve: null, delete: null } + validate: CapturedValidate | null + } = { publish: null, resolve: null, delete: null, validate: null } // If any endpoint is configured with 'network' failure mode, we need a real // TCP-level failure. Start a connection-dropping server and return its URL @@ -339,6 +356,34 @@ export async function startFakeRegistry(options: FakeRegistryOptions = {}) { }) } + // Validate (dry-run): POST /api/cli/v1/skills/:namespace/publish/validate + const validateMatch = path.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/publish\/validate$/) + if (validateMatch && req.method === 'POST') { + if (options.failures?.validate) return failureResponse(options.failures.validate) + const authErr = checkAuth(req) + if (authErr) return authErr + const namespace = validateMatch[1]! + + return req.formData().then(form => { + const fileField = form.get('file') + const visibility = (form.get('visibility') as string | null) ?? 'PUBLIC' + let fileName = 'skill.zip' + if (fileField instanceof File) { + fileName = fileField.name || fileName + } + state.validate = { namespace, fileName, visibility } + + const dryRunData = options.dryRunResponse ?? { + valid: true, + errors: [], + warnings: [], + resolvedSlug: fileName.replace(/\.zip$/, ''), + resolvedVersion: '1.0.0' + } + return Response.json({ code: 0, data: dryRunData }) + }) + } + // Publish: POST /api/cli/v1/skills/:namespace/publish const publishMatch = path.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/publish$/) if (publishMatch && req.method === 'POST') { diff --git a/cli/test/integration/auth-resolution.test.ts b/cli/test/integration/auth-resolution.test.ts new file mode 100644 index 00000000..1251445f --- /dev/null +++ b/cli/test/integration/auth-resolution.test.ts @@ -0,0 +1,159 @@ +/** + * End-to-end integration coverage for token / registry priority resolution. + * + * The unit test in test/unit/services/registry-service.test.ts pins the + * resolution function in isolation. These tests verify the same priorities + * are wired through the actual CLI subprocess: --flag > SKILLHUB_* env > + * stored config / credentials > built-in default. + * + * Why this matters: a regression in the wiring (e.g. command forgets to + * forward `process.env`) would silently downgrade users to the wrong + * registry / token without surfacing in unit tests. + */ +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined +let registryB: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined + registryB?.stop(); registryB = undefined +}) + +async function seedCredentials(home: string, registryUrl: string, token: string): Promise { + await mkdir(join(home, '.skillhub'), { recursive: true }) + await writeFile( + join(home, '.skillhub', 'credentials.json'), + JSON.stringify({ tokens: { [registryUrl]: token } }) + ) +} + +async function seedConfig(home: string, registryUrl: string): Promise { + await mkdir(join(home, '.skillhub'), { recursive: true }) + await writeFile( + join(home, '.skillhub', 'config.json'), + JSON.stringify({ registry: registryUrl }) + ) +} + +// --------------------------------------------------------------------------- +// Token priority: --token > SKILLHUB_TOKEN > stored +// --------------------------------------------------------------------------- + +describe('auth resolution — token priority', () => { + test('--token flag wins over SKILLHUB_TOKEN env', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_from_flag', + user: { handle: 'flag-user', displayName: 'Flag' } + }) + + const result = await runCli( + ['whoami', '--registry', registry.url, '--token', 'sk_from_flag'], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_TOKEN: 'sk_wrong_from_env' } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('flag-user') + }) + + test('SKILLHUB_TOKEN env wins over stored token', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_from_env', + user: { handle: 'env-user', displayName: 'Env' } + }) + await seedCredentials(env.home, registry.url, 'sk_wrong_from_storage') + + const result = await runCli( + ['whoami', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_TOKEN: 'sk_from_env' } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('env-user') + }) + + test('stored token used when neither --token nor env is set', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_from_storage', + user: { handle: 'storage-user', displayName: 'Storage' } + }) + await seedCredentials(env.home, registry.url, 'sk_from_storage') + + const result = await runCli( + ['whoami', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('storage-user') + }) +}) + +// --------------------------------------------------------------------------- +// Registry priority: --registry > SKILLHUB_REGISTRY > config.json +// --------------------------------------------------------------------------- + +describe('auth resolution — registry priority', () => { + test('--registry flag wins over SKILLHUB_REGISTRY env', async () => { + const env = await createTempHome() + // Each registry only authenticates its own token. The wrong registry + // would 401, so a successful whoami proves the right one was used. + registry = await startFakeRegistry({ + token: 'sk_a', + user: { handle: 'a-user', displayName: 'A' } + }) + registryB = await startFakeRegistry({ + token: 'sk_b', + user: { handle: 'b-user', displayName: 'B' } + }) + + const result = await runCli( + ['whoami', '--registry', registry.url, '--token', 'sk_a'], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_REGISTRY: registryB.url } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('a-user') + }) + + test('SKILLHUB_REGISTRY env wins over config.registry', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_env', + user: { handle: 'env-reg', displayName: 'EnvReg' } + }) + registryB = await startFakeRegistry({ + token: 'sk_config', + user: { handle: 'config-reg', displayName: 'ConfigReg' } + }) + await seedConfig(env.home, registryB.url) + + const result = await runCli( + ['whoami', '--token', 'sk_env'], + { HOME: env.home, USERPROFILE: env.home, SKILLHUB_REGISTRY: registry.url } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('env-reg') + }) + + test('config.registry used when no --registry / env present', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_config', + user: { handle: 'config-only-user', displayName: 'CfgOnly' } + }) + await seedConfig(env.home, registry.url) + await seedCredentials(env.home, registry.url, 'sk_config') + + const result = await runCli( + ['whoami'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('config-only-user') + }) +}) diff --git a/cli/test/integration/concurrency.test.ts b/cli/test/integration/concurrency.test.ts new file mode 100644 index 00000000..8490ed23 --- /dev/null +++ b/cli/test/integration/concurrency.test.ts @@ -0,0 +1,164 @@ +/** + * Concurrency tests for inventory.json bookkeeping. + * + * inventory-store.ts uses an OS-level lock file with retry + stale-lock + * detection. These tests exercise that path through real CLI subprocesses + * (Bun.spawn) running in parallel — the same way users hit it when scripts + * fan out installs. + * + * The unit test in test/unit/stores/inventory-store.test.ts pins the + * single-process lock recovery; here we cover the cross-process case. + */ +import { mkdir, readFile, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { zipSync, strToU8 } from 'fflate' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined +}) + +function makeSkillZip(): Uint8Array { + return zipSync({ 'SKILL.md': strToU8('# c') }) +} + +describe('cross-process concurrency on inventory.json', () => { + // KNOWN BUG (documented here, not yet fixed): + // inventory-store.upsertTarget() reads inventory, modifies in memory, + // then writeAtomic() acquires the lock only over the write half. Two + // concurrent installs each read the (empty) inventory, each adds their + // own item, and the second writer overwrites the first — a classic + // lost-update. + // + // When the fix lands (lock spans read+write, or upsertTarget acquires + // the lock first and re-reads), tighten the inventory assertion to + // `expect(slugs).toEqual(['first', 'second'])`. + test('two parallel installs of distinct slugs: filesystem is correct, inventory has at least one (lost-update bug pinned)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [ + { namespace: 'global', slug: 'first', version: '1.0.0', zipBytes: makeSkillZip() }, + { namespace: 'global', slug: 'second', version: '1.0.0', zipBytes: makeSkillZip() } + ] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const dirA = join(env.cwd, 'A') + const dirB = join(env.cwd, 'B') + await mkdir(dirA, { recursive: true }) + await mkdir(dirB, { recursive: true }) + + const [r1, r2] = await Promise.all([ + runCli( + ['install', 'first', '--dir', dirA, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ), + runCli( + ['install', 'second', '--dir', dirB, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + ]) + + // Both subprocess installs report success — neither errored at the + // protocol level even though the inventory bookkeeping race ate one of + // their inventory writes. + expect(r1.exitCode).toBe(0) + expect(r2.exitCode).toBe(0) + + // Filesystem is correct: both bundles extracted independently. + expect(await Bun.file(join(dirA, 'first', 'SKILL.md')).exists()).toBe(true) + expect(await Bun.file(join(dirB, 'second', 'SKILL.md')).exists()).toBe(true) + + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + const slugs = inv.items.map(i => i.slug).sort() + // Today: at least one slug always lands; under the lost-update race + // both may NOT be there. When the lock widens to cover read+write, + // upgrade this to `toEqual(['first', 'second'])`. + expect(slugs.length).toBeGreaterThanOrEqual(1) + const lastSlug = slugs[slugs.length - 1]! + expect(['first', 'second']).toContain(lastSlug) + }) + + test('two parallel installs of the same slug to the same dir: exactly one wins, one conflicts', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'race', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'race-dir') + await mkdir(installDir, { recursive: true }) + + const [r1, r2] = await Promise.all([ + runCli( + ['install', 'race', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ), + runCli( + ['install', 'race', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + ]) + + // Two valid outcomes: (a) both succeed because the loser's existence + // check ran BEFORE the winner extracted, OR (b) one succeeds and the + // other reports already-installed (EXIT.filesystem). + // Either way, inventory must end up coherent (single item, single + // target — no duplicates). + const codes = [r1.exitCode, r2.exitCode].sort((a, b) => a - b) + expect(codes[0]).toBe(0) // at least one succeeded + const otherCode = codes[1]! + expect([0, 4]).toContain(otherCode) // other either succeeded or got conflict + + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string; targets: Array<{ installDir: string }> }> } + const item = inv.items.find(i => i.slug === 'race') + expect(item).toBeDefined() + expect(item!.targets).toHaveLength(1) // no duplicate targets + }) + + test('install proceeds after a stale lock file from a dead process', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'after-stale', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Plant a stale lock file: PID 1 (init, never the same as our test + // child, and won't match the spawned subprocess's PID), with a very + // old timestamp so the store treats it as stale. + const skillhubDir = join(env.home, '.skillhub') + await mkdir(skillhubDir, { recursive: true }) + const lockPath = join(skillhubDir, 'inventory.json.lock') + const ancientTimestamp = Date.now() - 600_000 // 10 minutes ago — past the 30s stale threshold + await writeFile(lockPath, JSON.stringify({ pid: 1, timestamp: ancientTimestamp })) + + const installDir = join(env.cwd, 'stale') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'after-stale', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + const inv = JSON.parse( + await readFile(join(skillhubDir, 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + expect(inv.items.find(i => i.slug === 'after-stale')).toBeDefined() + }) +}) diff --git a/cli/test/integration/cross-command.test.ts b/cli/test/integration/cross-command.test.ts new file mode 100644 index 00000000..0eed997b --- /dev/null +++ b/cli/test/integration/cross-command.test.ts @@ -0,0 +1,509 @@ +/** + * Cross-command flow tests. + * + * Per-command tests verify each subcommand in isolation. These cases pin + * behaviors that only emerge when commands chain — e.g. "logout then install + * fails with auth" or "install + fs-delete + list reports status=missing". + * Bugs in the boundaries between commands (shared inventory, credentials, + * config) tend to slip through single-command suites. + */ +import { mkdir, rm, writeFile, readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { zipSync, strToU8 } from 'fflate' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined +let registryB: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined + registryB?.stop(); registryB = undefined +}) + +function makeSkillZip(): Uint8Array { + return zipSync({ 'SKILL.md': strToU8('# x-cross') }) +} + +// --------------------------------------------------------------------------- +// 1. Auth lifecycle: login → whoami → logout → whoami +// --------------------------------------------------------------------------- + +describe('cross-command — auth lifecycle', () => { + test('login → whoami(success) → logout → whoami(not logged in)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'cycle-user', displayName: 'Cycle' } + }) + + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + const w1 = await runCli(['whoami', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + expect(w1.exitCode).toBe(0) + expect(w1.stdout).toContain('cycle-user') + + await runCli(['logout', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + const w2 = await runCli(['whoami', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + expect(w2.exitCode).toBe(2) + expect(w2.stderr.toLowerCase()).toContain('not logged in') + }) + + test('logout-then-install against an auth-required registry fails with EXIT.auth', async () => { + const env = await createTempHome() + // Inject auth failure on resolve so this fake server behaves like a + // production registry that requires a bearer token even on resolve. + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + failures: { resolve: 'auth' } + }) + + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['logout', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'after-logout') + await mkdir(installDir, { recursive: true }) + + // No --token here — credentials were just cleared by logout. + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(2) // EXIT.auth + expect(result.stderr.toLowerCase()).toMatch(/auth|401|unauthorized/) + }) +}) + +// --------------------------------------------------------------------------- +// 2. Full local lifecycle: install → list → remove → list +// --------------------------------------------------------------------------- + +describe('cross-command — local lifecycle', () => { + test('install → list → remove --all → list shows empty', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'lifecycle') + await mkdir(installDir, { recursive: true }) + + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const list1 = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(JSON.parse(list1.stdout).items).toHaveLength(1) + + await runCli( + ['remove', 'pdf-parser', '--all', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + + const list2 = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(JSON.parse(list2.stdout).items).toHaveLength(0) + }) + + test('install x2 same slug + same dir without --force conflicts; --force succeeds; second install replaces first', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'reinstall-here') + await mkdir(installDir, { recursive: true }) + + const r1 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r1.exitCode).toBe(0) + + const r2 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r2.exitCode).toBe(4) // EXIT.filesystem (already installed) + + const r3 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--force'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r3.exitCode).toBe(0) + + // Inventory has exactly one target, not two duplicates. + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string; targets: Array<{ installDir: string }> }> } + const item = inv.items.find(i => i.slug === 'pdf-parser') + expect(item?.targets).toHaveLength(1) + }) + + test('install A then install B (different slugs, same parent dir) → list shows both', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [ + { namespace: 'global', slug: 'a-skill', version: '1.0.0', zipBytes: makeSkillZip() }, + { namespace: 'global', slug: 'b-skill', version: '1.0.0', zipBytes: makeSkillZip() } + ] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'two-skills') + await mkdir(installDir, { recursive: true }) + + await runCli( + ['install', 'a-skill', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + await runCli( + ['install', 'b-skill', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + const items = JSON.parse(list.stdout).items as Array<{ slug: string }> + expect(items.map(i => i.slug).sort()).toEqual(['a-skill', 'b-skill']) + }) + + test('remove --all → install same slug again succeeds (no stale inventory state)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'reuse') + await mkdir(installDir, { recursive: true }) + + await runCli(['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['remove', 'pdf-parser', '--all', '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + + // Re-install at the same dir without --force should now succeed, since + // the previous install was removed. + const reinstall = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(reinstall.exitCode).toBe(0) + }) +}) + +// --------------------------------------------------------------------------- +// 3. Filesystem drift between install dir and inventory +// --------------------------------------------------------------------------- + +describe('cross-command — filesystem drift', () => { + test('install → fs-delete the install dir → list reports status=missing', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'drift') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // External clobber: delete the install dir behind the CLI's back. + await rm(join(installDir, 'pdf-parser'), { recursive: true, force: true }) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; status: string }> + expect(items[0]?.slug).toBe('pdf-parser') + expect(items[0]?.status).toBe('missing') + }) + + // After commit a14d89d8 ("refactor(cli): improve doctor command + // semantics and transparency") doctor switched from REPLACE to MERGE + // semantics: it never removes inventory entries, even when the install + // dir on disk is gone. Stale entries are surfaced via `list --json`'s + // status="missing" instead. This test pins that contract. + test('install → fs-delete the install dir → doctor preserves the entry; list reports status=missing', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Install into an agent-shaped dir under cwd so doctor will scan it. + const codexSkills = join(env.cwd, '.codex', 'skills') + await mkdir(codexSkills, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', codexSkills, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // Wipe the install but leave the dir tree shape — metadata gone. + await rm(join(codexSkills, 'pdf-parser'), { recursive: true, force: true }) + + const doctor = await runCli(['doctor', '--json'], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + expect(doctor.exitCode).toBe(0) + + // Inventory still has the entry — doctor preserved it because the + // installDir was NOT in the (now-empty) scan result. + const inv = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + expect(inv.items.find(i => i.slug === 'pdf-parser')).toBeDefined() + + // The user-facing surface for "this is gone on disk" is `list` — it + // reports status="missing" by stat'ing the installDir at read time. + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; status: string }> + expect(items.find(i => i.slug === 'pdf-parser')?.status).toBe('missing') + }) +}) + +// --------------------------------------------------------------------------- +// 4. doctor idempotence +// --------------------------------------------------------------------------- + +describe('cross-command — doctor idempotence', () => { + test('two consecutive doctor runs produce identical inventory (idempotent)', async () => { + const env = await createTempHome() + + // Seed one valid metadata file. + const metaDir = join(env.cwd, '.codex', 'skills', 'pdf-parser', '.skillhub') + await mkdir(metaDir, { recursive: true }) + await writeFile(join(metaDir, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'pdf-parser', + version: '1.0.0', + agent: 'codex', + installedAt: '2026-04-20T12:00:00Z' + })) + + await runCli(['doctor'], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + const after1 = await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + + await runCli(['doctor'], { HOME: env.home, USERPROFILE: env.home }, { cwd: env.cwd }) + const after2 = await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + + expect(after2).toBe(after1) + }) +}) + +// --------------------------------------------------------------------------- +// 5. publish does not change local inventory +// --------------------------------------------------------------------------- + +describe('cross-command — publish vs local inventory', () => { + test('publish does NOT add the published skill to local inventory', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok', user: { handle: 'u', displayName: 'U' } }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Build a tiny skill dir to publish. + const dir = join(env.cwd, 'src-skill') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, 'SKILL.md'), '---\nname: pub-only\ndescription: x\n---\n# pub-only') + + const pub = await runCli(['publish', dir, '--registry', registry.url], { HOME: env.home, USERPROFILE: env.home }) + expect(pub.exitCode).toBe(0) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + expect(JSON.parse(list.stdout).items).toHaveLength(0) + }) +}) + +// --------------------------------------------------------------------------- +// 6. Cross-registry isolation in queries +// --------------------------------------------------------------------------- + +describe('cross-command — cross-registry isolation', () => { + test('list scoped to registry A does not show items installed from registry B', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_a', + user: { handle: 'a', displayName: 'A' }, + skills: [{ namespace: 'global', slug: 'a-only', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + registryB = await startFakeRegistry({ + token: 'sk_b', + user: { handle: 'b', displayName: 'B' }, + skills: [{ namespace: 'global', slug: 'b-only', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + + await runCli(['login', '--registry', registry.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', registryB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + + const dirA = join(env.cwd, 'A') + const dirB = join(env.cwd, 'B') + await mkdir(dirA, { recursive: true }) + await mkdir(dirB, { recursive: true }) + + await runCli(['install', 'a-only', '--dir', dirA, '--registry', registry.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['install', 'b-only', '--dir', dirB, '--registry', registryB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + + const listA = await runCli(['list', '--registry', registry.url, '--json'], { HOME: env.home, USERPROFILE: env.home }) + const slugsA = (JSON.parse(listA.stdout).items as Array<{ slug: string }>).map(i => i.slug) + expect(slugsA).toEqual(['a-only']) + + const listB = await runCli(['list', '--registry', registryB.url, '--json'], { HOME: env.home, USERPROFILE: env.home }) + const slugsB = (JSON.parse(listB.stdout).items as Array<{ slug: string }>).map(i => i.slug) + expect(slugsB).toEqual(['b-only']) + }) +}) + +// --------------------------------------------------------------------------- +// 7. Auto-detect + list filter integration (project-level) +// --------------------------------------------------------------------------- + +describe('cross-command — auto-detect + list', () => { + test('install auto-detects project-level .codex; subsequent list --agent codex shows it', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Pre-create .codex/skills so auto-detect picks codex/project-level. + await mkdir(join(env.cwd, '.codex', 'skills'), { recursive: true }) + + const inst = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(inst.exitCode).toBe(0) + + const list = await runCli( + ['list', '--agent', 'codex', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; agent: string }> + expect(items.some(i => i.slug === 'pdf-parser' && i.agent === 'codex')).toBe(true) + }) +}) + +// --------------------------------------------------------------------------- +// 8. Inventory metadata corruption resilience after install +// --------------------------------------------------------------------------- + +describe('cross-command — metadata.json drift', () => { + test('install → manually corrupt metadata.json → list reports the row but with sane handling', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'meta-drift') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // Corrupt the installed metadata. inventory.json (the authoritative + // source for `list`) is untouched, so `list` should still work. + await writeFile( + join(installDir, 'pdf-parser', '.skillhub', 'metadata.json'), + '{ truncated' + ) + + const list = await runCli( + ['list', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(list.exitCode).toBe(0) + const items = JSON.parse(list.stdout).items as Array<{ slug: string; status: string }> + expect(items[0]?.slug).toBe('pdf-parser') + // Status remains "ok" because list uses inventory.json, not metadata.json. + expect(items[0]?.status).toBe('ok') + }) +}) + +// --------------------------------------------------------------------------- +// 9. Registry priority chain end-to-end +// --------------------------------------------------------------------------- + +describe('cross-command — registry priority end-to-end', () => { + test('search uses --registry over SKILLHUB_REGISTRY env over default', async () => { + registry = await startFakeRegistry({ + searchItems: [{ namespace: 'global', slug: 'wins', latestVersion: '1.0.0', summary: 'right one' }] + }) + registryB = await startFakeRegistry({ + searchItems: [{ namespace: 'global', slug: 'loses', latestVersion: '1.0.0', summary: 'wrong one' }] + }) + + const result = await runCli( + ['search', '', '--registry', registry.url, '--json'], + { SKILLHUB_REGISTRY: registryB.url } + ) + expect(result.exitCode).toBe(0) + const items = JSON.parse(result.stdout).items as Array<{ slug: string }> + expect(items.map(i => i.slug)).toEqual(['wins']) + }) +}) + +// --------------------------------------------------------------------------- +// 10. Help / Version ergonomics across commands +// --------------------------------------------------------------------------- + +describe('cross-command — help reaches every documented command', () => { + test('every command listed in help responds to --help with non-empty body', async () => { + const helpResult = await runCli(['help']) + expect(helpResult.exitCode).toBe(0) + + const commandNames = [ + 'help', 'version', 'login', 'logout', 'whoami', + 'search', 'install', 'list', 'remove', 'doctor', + 'publish', 'update' + ] + for (const cmd of commandNames) { + expect(helpResult.stdout).toContain(cmd) + const sub = await runCli([cmd, '--help']) + // --help exits 0 for cac-style CLIs; we don't insist on that, just + // that some informative output makes it to stdout. + expect(sub.stdout.length).toBeGreaterThan(0) + } + }) +}) diff --git a/cli/test/integration/doctor-command.test.ts b/cli/test/integration/doctor-command.test.ts index f2afecb1..0d833722 100644 --- a/cli/test/integration/doctor-command.test.ts +++ b/cli/test/integration/doctor-command.test.ts @@ -143,6 +143,139 @@ describe('doctor command', () => { expect(json.inventoryPath).toContain('inventory.json') }) + // ------------------------------------------------------------------------- + // P1: same registry+namespace+slug appearing in two agent dirs with + // different versions surfaces in `conflicts` and is excluded from items. + // ------------------------------------------------------------------------- + test('doctor reports conflicts when two agent dirs disagree on version', async () => { + const { home, cwd } = await createTempHome() + + // Two installs of the same global/pdf-parser with mismatched versions. + await seedSkill(cwd, { + agentDir: '.codex', + slug: 'pdf-parser', + metadata: { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'pdf-parser', + version: '1.0.0', + agent: 'codex', + installedAt: '2026-04-20T12:00:00Z' + } + }) + await seedSkill(cwd, { + agentDir: '.claude', + slug: 'pdf-parser', + metadata: { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'pdf-parser', + version: '2.0.0', + agent: 'claude-code', + installedAt: '2026-04-21T09:00:00Z' + } + }) + + const result = await runCli(['doctor', '--json'], { + HOME: home, + USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + ok: boolean + itemsScanned: number + targetsScanned: number + conflicts: Array<{ key: string; versions: string[] }> + } + expect(json.ok).toBe(true) + // Conflicting group is dropped from items, recorded as a conflict. + expect(json.itemsScanned).toBe(0) + expect(json.targetsScanned).toBe(0) + expect(json.conflicts).toHaveLength(1) + expect(json.conflicts[0]?.key).toBe('https://skill.xfyun.cn|global|pdf-parser') + expect(json.conflicts[0]?.versions.sort()).toEqual(['1.0.0', '2.0.0']) + + // The persisted inventory must mirror the JSON output: no items. + const inventory = JSON.parse( + await readFile(join(home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: unknown[] } + expect(inventory.items).toHaveLength(0) + }) + + // ------------------------------------------------------------------------- + // P1: malformed metadata (unparseable JSON, or missing required fields) + // is reported in `skipped` and does not produce inventory entries. Two + // distinct failure modes are seeded to exercise both branches in + // scanMetadata: JSON.parse throw and the post-parse field check. + // ------------------------------------------------------------------------- + test('doctor reports skipped entries for malformed and incomplete metadata', async () => { + const { home, cwd } = await createTempHome() + + // (1) Bad JSON: triggers the catch around JSON.parse → "no .skillhub/metadata.json" + // because the catch block is shared with the readFile failure path. + const badJsonDir = join(cwd, '.codex', 'skills', 'broken-json', '.skillhub') + await mkdir(badJsonDir, { recursive: true }) + await writeFile(join(badJsonDir, 'metadata.json'), '{ this is not json') + + // (2) Incomplete fields: parses fine but is missing `version`. + const incompleteDir = join(cwd, '.claude', 'skills', 'incomplete', '.skillhub') + await mkdir(incompleteDir, { recursive: true }) + await writeFile( + join(incompleteDir, 'metadata.json'), + JSON.stringify({ + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'incomplete', + // version intentionally missing + agent: 'claude-code', + installedAt: '2026-04-22T10:00:00Z' + }) + ) + + // (3) A valid sibling so we can prove skipped entries don't poison the + // surrounding scan — the valid skill should still land in inventory. + await seedSkill(cwd, { + agentDir: '.codex', + slug: 'good-skill', + metadata: { + registry: 'https://skill.xfyun.cn', + namespace: 'global', + slug: 'good-skill', + version: '1.0.0', + agent: 'codex', + installedAt: '2026-04-22T10:00:00Z' + } + }) + + const result = await runCli(['doctor', '--json'], { + HOME: home, + USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + ok: boolean + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + expect(json.ok).toBe(true) + + // Both broken entries should be in skipped, the good one in items. + const broken = json.skipped.find(s => s.path.endsWith('broken-json')) + expect(broken).toBeDefined() + const incomplete = json.skipped.find(s => s.path.endsWith('incomplete')) + expect(incomplete).toBeDefined() + expect(incomplete?.reason).toContain('incomplete') + + expect(json.itemsScanned).toBe(1) // only good-skill + const inventory = JSON.parse( + await readFile(join(home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string }> } + expect(inventory.items).toHaveLength(1) + expect(inventory.items[0]?.slug).toBe('good-skill') + }) + test('doctor backs up existing inventory.json and reports backupPath', async () => { const { home, cwd } = await createTempHome() @@ -227,4 +360,104 @@ describe('doctor command', () => { expect.arrayContaining(['external-skill', 'local-skill']) ) }) + + // ------------------------------------------------------------------------- + // P1 — Symlink safety: doctor must skip (not follow) symlinked agent / + // skill / .skillhub directories. This protects against malicious or + // accidental symlinks that would otherwise let metadata be slurped from + // arbitrary filesystem locations. + // ------------------------------------------------------------------------- + test('doctor skips an agent dir that is a symlink', async () => { + const { home, cwd } = await createTempHome() + const { symlink, mkdir: mkdirP } = await import('node:fs/promises') + + // Real target with a valid metadata file off in /tmp. + const realRoot = join(cwd, '__real__', '.codex', 'skills', 'pdf-parser', '.skillhub') + await mkdirP(realRoot, { recursive: true }) + await writeFile(join(realRoot, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser', + version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z' + })) + + // Symlink ./.codex -> __real__/.codex inside cwd. Doctor scans cwd. + await symlink(join(cwd, '__real__', '.codex'), join(cwd, '.codex')) + + const result = await runCli(['doctor', '--json'], { + HOME: home, USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + // The symlinked agent dir must NOT contribute an inventory item. + expect(json.itemsScanned).toBe(0) + expect(json.skipped.some(s => s.path.endsWith('.codex') && s.reason.includes('regular directory'))).toBe(true) + }) + + test('doctor skips a slug dir that is a symlink (real agent dir, symlinked slug)', async () => { + const { home, cwd } = await createTempHome() + const { symlink, mkdir: mkdirP } = await import('node:fs/promises') + + // Real metadata under cwd/__real__/pdf-parser/.skillhub/ + const realSlug = join(cwd, '__real__', 'pdf-parser') + const realSkillhub = join(realSlug, '.skillhub') + await mkdirP(realSkillhub, { recursive: true }) + await writeFile(join(realSkillhub, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser', + version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z' + })) + + // .codex/skills exists as a real dir, but pdf-parser inside it is a + // symlink to the real metadata location. + const skillsDir = join(cwd, '.codex', 'skills') + await mkdirP(skillsDir, { recursive: true }) + await symlink(realSlug, join(skillsDir, 'pdf-parser')) + + const result = await runCli(['doctor', '--json'], { + HOME: home, USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + expect(json.itemsScanned).toBe(0) + const symlinked = json.skipped.find(s => s.path.endsWith('pdf-parser')) + expect(symlinked).toBeDefined() + expect(symlinked?.reason).toContain('regular directory') + }) + + test('doctor skips a .skillhub dir that is a symlink', async () => { + const { home, cwd } = await createTempHome() + const { symlink, mkdir: mkdirP } = await import('node:fs/promises') + + // Real metadata reachable through a symlinked .skillhub directory. + const realSkillhub = join(cwd, '__real_meta__') + await mkdirP(realSkillhub, { recursive: true }) + await writeFile(join(realSkillhub, 'metadata.json'), JSON.stringify({ + registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser', + version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z' + })) + + const slugDir = join(cwd, '.codex', 'skills', 'pdf-parser') + await mkdirP(slugDir, { recursive: true }) + await symlink(realSkillhub, join(slugDir, '.skillhub')) + + const result = await runCli(['doctor', '--json'], { + HOME: home, USERPROFILE: home + }, { cwd }) + + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { + itemsScanned: number + skipped: Array<{ path: string; reason: string }> + } + expect(json.itemsScanned).toBe(0) + const skipped = json.skipped.find(s => s.path.endsWith('pdf-parser')) + expect(skipped).toBeDefined() + expect(skipped?.reason.toLowerCase()).toMatch(/skillhub|regular directory/) + }) }) diff --git a/cli/test/integration/install-command.test.ts b/cli/test/integration/install-command.test.ts index e25cdbd7..a4ca3bd5 100644 --- a/cli/test/integration/install-command.test.ts +++ b/cli/test/integration/install-command.test.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile } from 'node:fs/promises' +import { mkdir, readFile, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { afterEach, describe, expect, test } from 'bun:test' import { zipSync, strToU8 } from 'fflate' @@ -218,6 +218,65 @@ describe('install command — P1', () => { expect(result.stderr.toLowerCase()).toMatch(/auth|unauthorized|401/) }) + test('bad token stops on 401 without retrying resolve anonymously', async () => { + const env = await createTempHome() + const installDir = join(env.cwd, 'skills-no-anon-retry') + await mkdir(installDir, { recursive: true }) + + const resolveAuthHeaders: Array = [] + let downloadRequests = 0 + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + const resolveMatch = url.pathname.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/([^/]+)\/resolve$/) + if (resolveMatch) { + const auth = req.headers.get('authorization') + resolveAuthHeaders.push(auth) + if (auth === 'Bearer sk_bad') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ + code: 0, + data: { + namespace: resolveMatch[1], + slug: resolveMatch[2], + version: '1.0.0', + versionId: 1, + fingerprint: 'abc123', + downloadUrl: `${url.protocol}//${url.host}/api/cli/v1/skills/${resolveMatch[1]}/${resolveMatch[2]}/download` + } + }) + } + if (url.pathname.endsWith('/download')) { + downloadRequests += 1 + return new Response(makeSkillZip() as BodyInit, { + status: 200, + headers: { 'Content-Type': 'application/zip' } + }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const registryUrl = `http://localhost:${server.port}` + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registryUrl, '--token', 'sk_bad'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).toBe(2) + expect(result.stderr).toContain('Error: authentication failed') + expect(result.stderr).toContain(`Context: registry ${registryUrl}`) + expect(result.stderr).toContain('Next:') + expect(resolveAuthHeaders).toEqual(['Bearer sk_bad']) + expect(downloadRequests).toBe(0) + } finally { + server.stop() + } + }) + // ------------------------------------------------------------------------- // P1 — --namespace override // ------------------------------------------------------------------------- @@ -272,3 +331,691 @@ describe('install command — P1', () => { // test/unit/agents/resolver.test.ts. // ------------------------------------------------------------------------- }) + +// --------------------------------------------------------------------------- +// P0/P1 — Conflict & --force handling +// --------------------------------------------------------------------------- + +describe('install command — conflict and --force', () => { + test('re-installing without --force into an existing dir errors with EXIT.filesystem', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-conflict') + await mkdir(installDir, { recursive: true }) + + const first = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(first.exitCode).toBe(0) + + const second = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(second.exitCode).toBe(4) // EXIT.filesystem + expect(second.stderr).toContain('already installed') + expect(second.stderr).toContain('--force') + }) + + test('--force overwrites stale files left in the install dir', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-force') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // Tamper with SKILL.md to prove the second install replaces it. + const skillFile = join(installDir, 'pdf-parser', 'SKILL.md') + await writeFile(skillFile, '# tampered content') + expect(await readFile(skillFile, 'utf-8')).toBe('# tampered content') + + const forced = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--force'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(forced.exitCode).toBe(0) + expect(await readFile(skillFile, 'utf-8')).toBe('# test skill') + }) +}) + +// --------------------------------------------------------------------------- +// P1 — Server-side error mapping during install +// --------------------------------------------------------------------------- + +describe('install command — server errors', () => { + test('resolve 404 surfaces an error and aborts install (no metadata.json written)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + failures: { resolve: 'not_found' } + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-resolve-404') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'no-such-slug', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + expect(result.stderr).toMatch(/404|not found/i) + + // No metadata file should have been created at the install destination. + const metaPath = join(installDir, 'no-such-slug', '.skillhub', 'metadata.json') + expect(await Bun.file(metaPath).exists()).toBe(false) + }) + + // Regression test for the production bug observed on 2026-05-06: server + // marks `bundle_ready=true` in DB but the bundle file is missing on disk. + // /resolve returns 200 with a downloadUrl, then /download returns 404. The + // CLI must surface a non-zero exit and a meaningful stderr — not silently + // succeed with an empty install dir. + test('download 404 (resolve OK) is reported as a download failure', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + // resolve succeeds (skill is present in fixture list) but the download + // endpoint is forced to 404 to simulate a missing bundle on storage. + skills: [{ namespace: 'global', slug: 'orphan-bundle', version: '1.0.0', zipBytes: makeSkillZip() }], + failures: { download: 'not_found' } + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-bundle-missing') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'orphan-bundle', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + expect(result.stderr.toLowerCase()).toMatch(/download|404|not found/) + }) + + // ------------------------------------------------------------------------- + // P1 — Path safety: install only writes inside // + // ------------------------------------------------------------------------- + + test('install only writes inside // — sibling files in are untouched', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'shared-dir') + await mkdir(installDir, { recursive: true }) + // Place an unrelated file as a sibling of the future / subdir. + const sibling = join(installDir, 'IMPORTANT.txt') + await writeFile(sibling, 'this file must survive install') + + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + // Sibling file must still exist with original content. + expect(await readFile(sibling, 'utf-8')).toBe('this file must survive install') + // / subdir created. + expect(await Bun.file(join(installDir, 'pdf-parser', 'SKILL.md')).exists()).toBe(true) + }) + + test('--force re-install does not touch sibling files in ', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'shared-force') + await mkdir(installDir, { recursive: true }) + await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + // After first install, drop a sibling file; --force should not delete it. + const sibling = join(installDir, 'sibling-after-install.bin') + await writeFile(sibling, 'sentinel') + + const r2 = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--force'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r2.exitCode).toBe(0) + expect(await readFile(sibling, 'utf-8')).toBe('sentinel') + }) + + test('install --dir creates the subdir even when is empty', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'empty-dir') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + expect(await Bun.file(join(installDir, 'pdf-parser', 'SKILL.md')).exists()).toBe(true) + expect(await Bun.file(join(installDir, 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(true) + }) + + test('install --dir pointing at a regular file (not a directory) fails before download', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Create a file at the location --dir would otherwise treat as a directory. + const filePath = join(env.cwd, 'not-a-dir') + await writeFile(filePath, 'i am a file, not a dir') + + const result = await runCli( + ['install', 'pdf-parser', '--dir', filePath, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).not.toBe(0) + // Original file must still be unchanged (the install should not have + // scribbled on it before bailing). + expect(await readFile(filePath, 'utf-8')).toBe('i am a file, not a dir') + }) + + test('--json emits a parseable error envelope when install fails', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + failures: { resolve: 'not_found' } + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'skills-json-error') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'no-such-slug', '--dir', installDir, '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + // JSON error envelope is printed to stdout (or stderr, depending on the + // command); we accept either to keep the test resilient to that choice. + const candidate = result.stdout || result.stderr + const json = JSON.parse(candidate) as { + ok: boolean + message: string + exitCode: number + } + expect(json.ok).toBe(false) + expect(typeof json.message).toBe('string') + expect(json.exitCode).toBe(result.exitCode) + }) +}) + +// --------------------------------------------------------------------------- +// P1 — Multi-agent and auto-detect targeting +// --------------------------------------------------------------------------- + +describe('install command — multi-agent & auto-detect', () => { + test('multi --agent installs the same skill into every specified user-level dir', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + [ + 'install', 'pdf-parser', + '--agent', 'codex', + '--agent', 'claude-code', + '--registry', registry.url, + '--token', 'sk_ok', + '--json' + ], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) as { installed: Array<{ agent: string }> } + const agents = parsed.installed.map(t => t.agent).sort() + expect(agents).toEqual(['claude-code', 'codex']) + + // Both metadata files exist on disk under user-level /./skills. + expect(await Bun.file(join(env.home, '.codex', 'skills', 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(true) + expect(await Bun.file(join(env.home, '.claude', 'skills', 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(true) + }) + + test('duplicate --agent dedupes to one target', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + [ + 'install', 'pdf-parser', + '--agent', 'codex', + '--agent', 'codex', + '--registry', registry.url, + '--token', 'sk_ok', + '--json' + ], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) as { installed: Array<{ agent: string }> } + expect(parsed.installed).toHaveLength(1) + expect(parsed.installed[0]?.agent).toBe('codex') + }) + + test('--agent unknown-id surfaces a usage error with hint', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + ['install', 'pdf-parser', '--agent', 'totally-not-a-real-agent', '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(5) // EXIT.usage + expect(result.stderr.toLowerCase()).toMatch(/unknown agent|--dir/) + }) + + test('auto-detect: cwd with only .codex/skills present installs project-level there', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + // Pre-create the codex skills dir so auto-detect picks project scope. + await mkdir(join(env.cwd, '.codex', 'skills'), { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(result.exitCode).toBe(0) + + const parsed = JSON.parse(result.stdout) as { installed: Array<{ dir: string; agent: string }> } + expect(parsed.installed[0]?.agent).toBe('codex') + // On macOS env.cwd may resolve through /private/var/... symlinks; assert + // against the structural part of the path instead of an exact prefix. + // Use a regex that accepts both Unix (/) and Windows (\) path separators. + expect(parsed.installed[0]?.dir).toMatch(/[/\\]\.codex[/\\]skills[/\\]pdf-parser/) + expect(parsed.installed[0]?.dir).not.toContain(env.home) // not user-level + }) + + test('auto-detect: multiple agent dirs in cwd and non-interactive mode fails with hint', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + await mkdir(join(env.cwd, '.codex', 'skills'), { recursive: true }) + await mkdir(join(env.cwd, '.claude', 'skills'), { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(result.exitCode).toBe(5) // EXIT.usage + expect(result.stderr.toLowerCase()).toMatch(/multiple install targets|--agent|--dir/) + }) + + test('auto-detect: cwd with no agent dirs falls back to .agents/skills', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [{ namespace: 'global', slug: 'pdf-parser', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + await runCli(['login', '--registry', registry.url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const result = await runCli( + ['install', 'pdf-parser', '--registry', registry.url, '--token', 'sk_ok', '--json'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) as { installed: Array<{ dir: string; agent: string }> } + expect(parsed.installed[0]?.agent).toBe('generic') + expect(parsed.installed[0]?.dir).toContain('.agents') + }) + + // ------------------------------------------------------------------------- + // P1 — Bundle integrity: download body that's not a valid zip + // ------------------------------------------------------------------------- + test('download body that is not a valid zip surfaces an extraction error', async () => { + const env = await createTempHome() + // Stand up a custom server that returns valid resolve JSON but plain + // text on download. + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + const baseUrl = `${url.protocol}//${url.host}` + const resolveMatch = url.pathname.match(/^\/api\/cli\/v1\/skills\/([^/]+)\/([^/]+)\/resolve$/) + if (resolveMatch && req.method === 'GET') { + return Response.json({ + code: 0, + data: { + namespace: resolveMatch[1], + slug: resolveMatch[2], + version: '1.0.0', + versionId: 1, + fingerprint: 'deadbeef', + downloadUrl: `${baseUrl}/api/cli/v1/skills/${resolveMatch[1]}/${resolveMatch[2]}/versions/1.0.0/download` + } + }) + } + if (url.pathname.includes('/download')) { + // NOT a zip — plain text. + return new Response('this is plain text, not a zip', { + status: 200, headers: { 'Content-Type': 'application/zip' } + }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await runCli(['login', '--registry', url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'bad-bundle') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', 'pdf-parser', '--dir', installDir, '--registry', url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).not.toBe(0) + // No metadata should have been written. + expect(await Bun.file(join(installDir, 'pdf-parser', '.skillhub', 'metadata.json')).exists()).toBe(false) + } finally { + server.stop() + } + }) + + // ------------------------------------------------------------------------- + // P2 — Slug edge cases (Unicode, very long) + // ------------------------------------------------------------------------- + test('slug with non-ASCII characters round-trips through resolve URL (encoded)', async () => { + const env = await createTempHome() + let resolveUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.includes('/resolve')) { + resolveUrl = req.url + // Return 404 — we only care that the URL was constructed correctly. + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await runCli(['login', '--registry', url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'unicode-slug') + await mkdir(installDir, { recursive: true }) + + const result = await runCli( + ['install', '中文-技能', '--dir', installDir, '--registry', url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + // Server returns 404 — install fails. Just confirm CLI didn't crash + // before hitting the server. + expect(result.exitCode).not.toBe(0) + // The slug must appear URL-percent-encoded in the resolve URL. + expect(resolveUrl).toMatch(/%E4%B8%AD%E6%96%87/) + } finally { + server.stop() + } + }) + + test('slug 200+ characters is forwarded as-is to /resolve (server is authoritative)', async () => { + const env = await createTempHome() + let resolveUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.includes('/resolve')) { + resolveUrl = req.url + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await runCli(['login', '--registry', url, '--token', 'sk_ok'], { HOME: env.home, USERPROFILE: env.home }) + + const installDir = join(env.cwd, 'long-slug') + await mkdir(installDir, { recursive: true }) + + const longSlug = 'a'.repeat(220) + const result = await runCli( + ['install', longSlug, '--dir', installDir, '--registry', url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).not.toBe(0) + expect(resolveUrl).toContain(longSlug) + } finally { + server.stop() + } + }) +}) + +// --------------------------------------------------------------------------- +// P0 — --scope flag +// --------------------------------------------------------------------------- + +describe('install command — --scope', () => { + test('--scope project --agent codex installs to /.codex/skills', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'foo', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + + await runCli( + ['login', '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const result = await runCli( + ['install', 'foo', '--scope', 'project', '--agent', 'codex', + '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + + expect(result.exitCode).toBe(0) + const metaPath = join(env.cwd, '.codex', 'skills', 'foo', '.skillhub', 'metadata.json') + const meta = JSON.parse(await readFile(metaPath, 'utf-8')) + expect(meta.slug).toBe('foo') + }) + + test('--scope user --agent codex installs to /.codex/skills', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'foo', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + + await runCli( + ['login', '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const result = await runCli( + ['install', 'foo', '--scope', 'user', '--agent', 'codex', + '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + + expect(result.exitCode).toBe(0) + const metaPath = join(env.home, '.codex', 'skills', 'foo', '.skillhub', 'metadata.json') + const meta = JSON.parse(await readFile(metaPath, 'utf-8')) + expect(meta.slug).toBe('foo') + }) + + test('--scope user clean env falls back to /.agents/skills', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'foo', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + + await runCli( + ['login', '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const result = await runCli( + ['install', 'foo', '--scope', 'user', + '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + + expect(result.exitCode).toBe(0) + const metaPath = join(env.home, '.agents', 'skills', 'foo', '.skillhub', 'metadata.json') + const meta = JSON.parse(await readFile(metaPath, 'utf-8')) + expect(meta.slug).toBe('foo') + }) + + test('--scope project --agent codex --json output omits scope field on installed entries', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u1', displayName: 'User One' }, + skills: [{ namespace: 'global', slug: 'foo', version: '1.0.0', zipBytes: makeSkillZip() }] + }) + + await runCli( + ['login', '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const result = await runCli( + ['install', 'foo', '--scope', 'project', '--agent', 'codex', '--json', + '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home }, + { cwd: env.cwd } + ) + + expect(result.exitCode).toBe(0) + const parsed = JSON.parse(result.stdout) + expect(parsed).toMatchObject({ ok: true, namespace: 'global', slug: 'foo' }) + expect(parsed.installed[0]).toHaveProperty('agent') + expect(parsed.installed[0]).toHaveProperty('dir') + expect(parsed.installed[0]).not.toHaveProperty('scope') + }) + + test('--scope invalid returns exit code 5 with usage error', async () => { + const result = await runCli(['install', 'foo', '--scope', 'invalid']) + expect(result.exitCode).toBe(5) + expect(result.stderr).toMatch(/user.+project|"user".+"project"/) + }) + + test('--scope invalid --json returns JSON error shape', async () => { + const result = await runCli(['install', 'foo', '--scope', 'invalid', '--json']) + expect(result.exitCode).toBe(5) + const parsed = JSON.parse(result.stderr) + expect(parsed.ok).toBe(false) + expect(parsed.exitCode).toBe(5) + expect(parsed.message).toMatch(/user.+project/) + }) + + test('--dir + --scope returns usage error', async () => { + const result = await runCli(['install', 'foo', '--dir', '/tmp/x', '--scope', 'user']) + expect(result.exitCode).toBe(5) + expect(result.stderr).toMatch(/--dir cannot be used with --scope/) + }) + + test('--dir + --scope --json returns JSON usage error', async () => { + const result = await runCli( + ['install', 'foo', '--dir', '/tmp/x', '--scope', 'user', '--json'] + ) + expect(result.exitCode).toBe(5) + const parsed = JSON.parse(result.stderr) + expect(parsed.ok).toBe(false) + expect(parsed.message).toMatch(/--dir cannot be used with --scope/) + }) + + test('help install includes --scope usage and examples', async () => { + const result = await runCli(['help', 'install']) + expect(result.exitCode).toBe(0) + expect(result.stdout).toMatch(/--scope/) + expect(result.stdout).toMatch(/--scope user/) + expect(result.stdout).toMatch(/--scope project --agent codex/) + }) +}) diff --git a/cli/test/integration/inventory-resilience.test.ts b/cli/test/integration/inventory-resilience.test.ts new file mode 100644 index 00000000..ac08fb7c --- /dev/null +++ b/cli/test/integration/inventory-resilience.test.ts @@ -0,0 +1,107 @@ +/** + * inventory.json resilience. + * + * inventory.json is the local manifest of installed skills. These tests pin + * how the CLI behaves when that file is corrupt or written by overlapping + * operations: + * - list against a corrupt inventory should fail loudly (not silently) + * - install against a corrupt inventory should still complete the + * filesystem extraction even if inventory bookkeeping fails — partial + * state surfaces a clear error + * - sequential installs of distinct skills do not corrupt the manifest + * + * The unit test in test/unit/stores/inventory-store.test.ts asserts the + * lock-file recovery path. These cover the user-facing CLI surface. + */ +import { mkdir, readFile, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { zipSync, strToU8 } from 'fflate' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let registry: Awaited> | undefined + +afterEach(() => { + registry?.stop(); registry = undefined +}) + +function makeSkillZip(): Uint8Array { + return zipSync({ 'SKILL.md': strToU8('# test') }) +} + +describe('inventory resilience', () => { + test('list exits non-zero when inventory.json is malformed (documents current generic-error UX)', async () => { + const env = await createTempHome() + await mkdir(join(env.home, '.skillhub'), { recursive: true }) + await writeFile(join(env.home, '.skillhub', 'inventory.json'), '{ this is not JSON') + + const result = await runCli(['list'], { HOME: env.home, USERPROFILE: env.home }) + + // Contract: CLI must not crash silently or print a stack trace. It + // exits non-zero and emits a short message. + expect(result.exitCode).not.toBe(0) + expect(result.stderr.length).toBeGreaterThan(0) + expect(result.stderr.length).toBeLessThan(2000) + // Documented gap: today's message is the generic "unexpected failure" + // and does not mention `inventory` or `JSON`. When the CLI surfaces a + // more specific message in the future, tighten this assertion. + expect(result.stderr).toContain('Error') + }) + + test('list --json on a corrupt inventory emits a parseable error envelope (not a stack trace)', async () => { + const env = await createTempHome() + await mkdir(join(env.home, '.skillhub'), { recursive: true }) + await writeFile(join(env.home, '.skillhub', 'inventory.json'), '{"items":') + + const result = await runCli(['list', '--json'], { HOME: env.home, USERPROFILE: env.home }) + + expect(result.exitCode).not.toBe(0) + const candidate = result.stdout || result.stderr + expect(candidate.length).toBeLessThan(2000) + // Contract: --json error path is machine-parseable, regardless of the + // (currently generic) human message. + const json = JSON.parse(candidate) as { ok: boolean; message: string; exitCode: number } + expect(json.ok).toBe(false) + expect(typeof json.message).toBe('string') + expect(json.exitCode).toBe(result.exitCode) + }) + + test('two sequential installs of distinct slugs leave a coherent inventory', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' }, + skills: [ + { namespace: 'global', slug: 'one', version: '1.0.0', zipBytes: makeSkillZip() }, + { namespace: 'global', slug: 'two', version: '1.0.0', zipBytes: makeSkillZip() } + ] + }) + + const baseDir = join(env.cwd, 'pool') + await mkdir(baseDir, { recursive: true }) + + const r1 = await runCli( + ['install', 'one', '--dir', baseDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r1.exitCode).toBe(0) + + const r2 = await runCli( + ['install', 'two', '--dir', baseDir, '--registry', registry.url, '--token', 'sk_ok'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(r2.exitCode).toBe(0) + + const inventory = JSON.parse( + await readFile(join(env.home, '.skillhub', 'inventory.json'), 'utf-8') + ) as { items: Array<{ slug: string; targets: Array<{ installDir: string }> }> } + + const slugs = inventory.items.map(i => i.slug).sort() + expect(slugs).toEqual(['one', 'two']) + for (const item of inventory.items) { + expect(item.targets.length).toBeGreaterThan(0) + } + }) +}) diff --git a/cli/test/integration/list-command.test.ts b/cli/test/integration/list-command.test.ts index 2c2f95d4..da15ed42 100644 --- a/cli/test/integration/list-command.test.ts +++ b/cli/test/integration/list-command.test.ts @@ -351,4 +351,112 @@ describe('list command', () => { expect(json.items).toHaveLength(1) expect(json.items[0].status).toBe('missing') }) + + // ------------------------------------------------------------------------- + // P1: Combined filters — --agent + --registry should narrow precisely + // ------------------------------------------------------------------------- + test('--agent codex --registry A shows only codex targets from registry A', async () => { + const { home } = await createTempHome() + + const codexA = join(home, 'a', 'codex', 'pdf') + const claudeA = join(home, 'a', 'claude', 'pdf') + const codexB = join(home, 'b', 'codex', 'pdf') + for (const d of [codexA, claudeA, codexB]) await mkdir(d, { recursive: true }) + + await seedInventory(home, [ + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'pdf', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'a', 'codex'), installDir: codexA, installedAt: INSTALLED_AT }, + { agent: 'claude-code', rootDir: join(home, 'a', 'claude'), installDir: claudeA, installedAt: INSTALLED_AT } + ] + }, + { + registry: FAKE_REGISTRY_B, namespace: 'global', slug: 'pdf', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'b', 'codex'), installDir: codexB, installedAt: INSTALLED_AT } + ] + } + ]) + + const result = await runCli( + ['list', '--agent', 'codex', '--registry', FAKE_REGISTRY_A, '--json'], + { HOME: home, USERPROFILE: home } + ) + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { items: Array<{ agent: string; installDir: string }> } + expect(json.items).toHaveLength(1) + expect(json.items[0]?.agent).toBe('codex') + expect(json.items[0]?.installDir).toBe(codexA) + }) + + // ------------------------------------------------------------------------- + // P1: --agent + --dir should compose AND, not OR + // ------------------------------------------------------------------------- + test('--agent + --dir composes as AND: only items matching both surface', async () => { + const { home } = await createTempHome() + + const codexHere = join(home, 'here', 'codex', 'pdf') + const codexElse = join(home, 'else', 'codex', 'pdf') + await mkdir(codexHere, { recursive: true }) + await mkdir(codexElse, { recursive: true }) + + await seedInventory(home, [ + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'pdf', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'here', 'codex'), installDir: codexHere, installedAt: INSTALLED_AT } + ] + }, + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'pdf-elsewhere', version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: join(home, 'else', 'codex'), installDir: codexElse, installedAt: INSTALLED_AT } + ] + } + ]) + + const result = await runCli( + ['list', '--registry', FAKE_REGISTRY_A, '--agent', 'codex', '--dir', join(home, 'here'), '--json'], + { HOME: home, USERPROFILE: home } + ) + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { items: Array<{ slug: string }> } + expect(json.items).toHaveLength(1) + expect(json.items[0]?.slug).toBe('pdf') + }) + + // ------------------------------------------------------------------------- + // P1: SKILLHUB_REGISTRY env scopes list to the env-specified registry + // (registry priority --registry > env > config > default also applies to + // list, not just to network-touching commands). + // ------------------------------------------------------------------------- + test('SKILLHUB_REGISTRY env scopes list to that registry, hiding the other', async () => { + const { home } = await createTempHome() + const dirA = join(home, 'a', 'codex', 'one') + const dirB = join(home, 'b', 'codex', 'two') + await mkdir(dirA, { recursive: true }) + await mkdir(dirB, { recursive: true }) + + await seedInventory(home, [ + { + registry: FAKE_REGISTRY_A, namespace: 'global', slug: 'one', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: join(home, 'a', 'codex'), installDir: dirA, installedAt: INSTALLED_AT }] + }, + { + registry: FAKE_REGISTRY_B, namespace: 'global', slug: 'two', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: join(home, 'b', 'codex'), installDir: dirB, installedAt: INSTALLED_AT }] + } + ]) + + // No --registry flag — scope comes from SKILLHUB_REGISTRY env. + const result = await runCli( + ['list', '--json'], + { HOME: home, USERPROFILE: home, SKILLHUB_REGISTRY: FAKE_REGISTRY_B } + ) + expect(result.exitCode).toBe(0) + const json = JSON.parse(result.stdout) as { items: Array<{ slug: string }> } + expect(json.items).toHaveLength(1) + expect(json.items[0]?.slug).toBe('two') + }) }) diff --git a/cli/test/integration/multi-registry.test.ts b/cli/test/integration/multi-registry.test.ts new file mode 100644 index 00000000..eecd563f --- /dev/null +++ b/cli/test/integration/multi-registry.test.ts @@ -0,0 +1,110 @@ +/** + * Multi-registry credential isolation. + * + * credentials.json keys tokens by registry URL. Operations on one registry + * must not leak into another. These tests cover: + * - Logging into A then B preserves both tokens. + * - Logging out of A leaves B's token intact. + * - whoami after logout reflects per-registry session state. + * - Re-login to A overwrites only A's slot. + */ +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' +import { createTempHome } from '../helpers/temp-env' + +let regA: Awaited> | undefined +let regB: Awaited> | undefined + +afterEach(() => { + regA?.stop(); regA = undefined + regB?.stop(); regB = undefined +}) + +async function readCreds(home: string): Promise<{ tokens: Record }> { + return JSON.parse(await readFile(join(home, '.skillhub', 'credentials.json'), 'utf-8')) +} + +describe('multi-registry credential isolation', () => { + test('login to A then B leaves both tokens in credentials.json', async () => { + const env = await createTempHome() + regA = await startFakeRegistry({ token: 'sk_a', user: { handle: 'a', displayName: 'A' } }) + regB = await startFakeRegistry({ token: 'sk_b', user: { handle: 'b', displayName: 'B' } }) + + await runCli( + ['login', '--registry', regA.url, '--token', 'sk_a'], + { HOME: env.home, USERPROFILE: env.home } + ) + await runCli( + ['login', '--registry', regB.url, '--token', 'sk_b'], + { HOME: env.home, USERPROFILE: env.home } + ) + + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBe('sk_a') + expect(creds.tokens[regB.url]).toBe('sk_b') + }) + + test('logout from A removes A token while B token survives', async () => { + const env = await createTempHome() + regA = await startFakeRegistry({ token: 'sk_a', user: { handle: 'a', displayName: 'A' } }) + regB = await startFakeRegistry({ token: 'sk_b', user: { handle: 'b', displayName: 'B' } }) + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', regB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['logout', '--registry', regA.url], { HOME: env.home, USERPROFILE: env.home }) + + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBeUndefined() + expect(creds.tokens[regB.url]).toBe('sk_b') + }) + + test('whoami after logout-A: A reports not-logged-in, B still authenticates', async () => { + const env = await createTempHome() + regA = await startFakeRegistry({ token: 'sk_a', user: { handle: 'a-user', displayName: 'A' } }) + regB = await startFakeRegistry({ token: 'sk_b', user: { handle: 'b-user', displayName: 'B' } }) + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', regB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['logout', '--registry', regA.url], { HOME: env.home, USERPROFILE: env.home }) + + const whoamiA = await runCli( + ['whoami', '--registry', regA.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(whoamiA.exitCode).toBe(2) // EXIT.auth + expect(whoamiA.stderr.toLowerCase()).toContain('not logged in') + + const whoamiB = await runCli( + ['whoami', '--registry', regB.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(whoamiB.exitCode).toBe(0) + expect(whoamiB.stdout).toContain('b-user') + }) + + test('re-login to A overwrites only A entry; B token unchanged', async () => { + const env = await createTempHome() + // Don't pin a token on either registry so any value passes whoami; we + // only care about credentials.json bookkeeping here. + regA = await startFakeRegistry({ user: { handle: 'a', displayName: 'A' } }) + regB = await startFakeRegistry({ user: { handle: 'b', displayName: 'B' } }) + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a_old'], { HOME: env.home, USERPROFILE: env.home }) + await runCli(['login', '--registry', regB.url, '--token', 'sk_b'], { HOME: env.home, USERPROFILE: env.home }) + + { + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBe('sk_a_old') + expect(creds.tokens[regB.url]).toBe('sk_b') + } + + await runCli(['login', '--registry', regA.url, '--token', 'sk_a_new'], { HOME: env.home, USERPROFILE: env.home }) + + const creds = await readCreds(env.home) + expect(creds.tokens[regA.url]).toBe('sk_a_new') + expect(creds.tokens[regB.url]).toBe('sk_b') + }) +}) diff --git a/cli/test/integration/publish-command.test.ts b/cli/test/integration/publish-command.test.ts index 5f382fdf..560e2f33 100644 --- a/cli/test/integration/publish-command.test.ts +++ b/cli/test/integration/publish-command.test.ts @@ -237,3 +237,295 @@ describe('publish command — P1', () => { expect(result.stderr).toContain('registry') }) }) + +// --------------------------------------------------------------------------- +// P1 — content shape: directory layout and edge files +// --------------------------------------------------------------------------- + +import { mkdir } from 'node:fs/promises' +import { unzipSync, strFromU8 } from 'fflate' + +describe('publish command — content shape', () => { + /** + * Spin up a publish endpoint that captures the raw zip body and lets us + * inspect entries server-side. Returns the captured bytes alongside a + * stop() handle so tests can assert what the CLI actually packaged. + */ + async function startCapturingPublishServer() { + let capturedBytes: Uint8Array | null = null + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + const form = await req.formData() + const file = form.get('file') + if (file instanceof File) { + capturedBytes = new Uint8Array(await file.arrayBuffer()) + } + return Response.json({ + code: 0, + data: { namespace: 'global', slug: 'captured', version: '1.0.0', visibility: 'PUBLIC' } + }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + return { + url: `http://localhost:${server.port}`, + stop: () => server.stop(), + getCaptured: () => capturedBytes + } + } + + test('publishing a directory with subdirs packages every file at its relative path', async () => { + const env = await createTempHome() + const server = await startCapturingPublishServer() + try { + await login(env, server.url) + + const dir = await mkdtemp(join(tmpdir(), 'skillhub-publish-nested-')) + await writeFile(join(dir, 'SKILL.md'), '# nested') + await mkdir(join(dir, 'references'), { recursive: true }) + await writeFile(join(dir, 'references', 'a.md'), 'aa') + await mkdir(join(dir, 'scripts'), { recursive: true }) + await writeFile(join(dir, 'scripts', 'run.sh'), '#!/bin/sh\necho ok\n') + + const result = await runCli(['publish', dir, '--registry', server.url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(0) + + const captured = server.getCaptured() + expect(captured).not.toBeNull() + const rawEntries = unzipSync(captured!) + // Normalize all entry keys to use forward slashes for cross-platform compatibility + const entries = Object.fromEntries( + Object.entries(rawEntries).map(([key, value]) => [key.replace(/\\/g, '/'), value]) + ) + // Filter out directory marker entries (zip records empty entries for + // dirs with a trailing slash); we only care about file entries. + const files = Object.keys(entries).filter(k => !k.endsWith('/')).sort() + expect(files).toEqual([ + 'SKILL.md', + 'references/a.md', + 'scripts/run.sh' + ]) + expect(strFromU8(entries['SKILL.md']!)).toBe('# nested') + expect(strFromU8(entries['references/a.md']!)).toBe('aa') + } finally { + server.stop() + } + }) + + test('publishing a directory with hidden dotfiles packages them as-is', async () => { + const env = await createTempHome() + const server = await startCapturingPublishServer() + try { + await login(env, server.url) + + const dir = await mkdtemp(join(tmpdir(), 'skillhub-publish-hidden-')) + await writeFile(join(dir, 'SKILL.md'), '# h') + await writeFile(join(dir, '.DS_Store'), 'macos junk') + await writeFile(join(dir, '.editorconfig'), 'root = true\n') + + const result = await runCli(['publish', dir, '--registry', server.url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(0) + + const captured = server.getCaptured() + expect(captured).not.toBeNull() + const rawEntries = unzipSync(captured!) + // Normalize all entry keys to use forward slashes for cross-platform compatibility + const entries = Object.fromEntries( + Object.entries(rawEntries).map(([key, value]) => [key.replace(/\\/g, '/'), value]) + ) + // Pin current behavior so future filtering changes are intentional. + expect(Object.keys(entries).sort()).toEqual(['.DS_Store', '.editorconfig', 'SKILL.md']) + } finally { + server.stop() + } + }) + + test('publishing an empty directory still issues a request and reports the server outcome', async () => { + const env = await createTempHome() + // Fake registry accepts publish unconditionally; CLI is not authoritative + // on SKILL.md presence (server is). We assert only that the CLI does not + // crash client-side and exits with whatever the server returned. + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await mkdtemp(join(tmpdir(), 'skillhub-publish-empty-')) + + const result = await runCli(['publish', dir, '--registry', registry.url], { + HOME: env.home, USERPROFILE: env.home + }) + // Today's contract: empty dir → empty zip uploaded → server returns 200. + // If the server adds client-side or server-side validation later this + // assertion will need to flip; that's intentional and traceable. + expect(result.exitCode).toBe(0) + }) + + test('server 422 with a JSON validation body surfaces a non-zero exit and stderr', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json( + { code: 422, message: 'validation.token.name.size', errors: ['name exceeds 64 chars'] }, + { status: 422 } + ) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).not.toBe(0) + // The server's HTTP status should propagate visibly so a CI log + // shows what happened. + expect(result.stderr).toMatch(/422|registry|validation/i) + } finally { + server.stop() + } + }) + + // 502/503 are special-cased to EXIT.network because they indicate + // infrastructure-level unavailability (gateway/proxy failure). + test('server 503 Service Unavailable maps to EXIT.network with status in stderr', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json({ code: 503, message: 'service unavailable' }, { status: 503 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(3) // EXIT.network + expect(result.stderr).toMatch(/503|registry/i) + } finally { + server.stop() + } + }) + + test('server 401 mid-session (token revoked) maps to EXIT.auth', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + // Whoami succeeds (login step). Publish then returns 401 as if the + // server revoked the token between the login + publish calls. + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).toBe(2) // EXIT.auth + expect(result.stderr.toLowerCase()).toMatch(/auth|401|unauthorized/) + } finally { + server.stop() + } + }) + + test('publish response missing required fields is handled without crash', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + // 200 OK but body shape doesn't match the expected schema. + return Response.json({ code: 0, data: { unexpected: true } }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url, '--json'], { + HOME: env.home, USERPROFILE: env.home + }) + // Either parses with placeholder values or fails — the contract we + // want is "no crash". Pin: exit 0 means current behavior accepts + // partial responses; flip if/when stricter validation lands. + expect([0, 1, 2, 3]).toContain(result.exitCode) + // Either way, output is bounded — no stack trace dump. + expect((result.stdout + result.stderr).length).toBeLessThan(2000) + } finally { + server.stop() + } + }) + + test('server 413 Payload Too Large maps to a network-class non-zero exit', async () => { + const env = await createTempHome() + const server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/auth/whoami') { + return Response.json({ code: 0, data: { handle: 'u', displayName: 'U' } }) + } + if (url.pathname.endsWith('/publish') && req.method === 'POST') { + return Response.json({ code: 413, message: 'payload too large' }, { status: 413 }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + try { + const url = `http://localhost:${server.port}` + await login(env, url) + const dir = await makeTempDir(['SKILL.md', '# x']) + const result = await runCli(['publish', dir, '--registry', url], { + HOME: env.home, USERPROFILE: env.home + }) + expect(result.exitCode).not.toBe(0) + expect(result.stderr).toMatch(/413|registry/i) + } finally { + server.stop() + } + }) +}) diff --git a/cli/test/integration/publish-dry-run.test.ts b/cli/test/integration/publish-dry-run.test.ts new file mode 100644 index 00000000..456572d1 --- /dev/null +++ b/cli/test/integration/publish-dry-run.test.ts @@ -0,0 +1,177 @@ +import { mkdtemp, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, test } from 'bun:test' +import { createTempHome } from '../helpers/temp-env' +import { startFakeRegistry } from '../helpers/fake-registry' +import { runCli } from '../helpers/run-cli' + +let registry: Awaited> | undefined + +afterEach(() => { + registry?.stop() + registry = undefined +}) + +async function login(env: { home: string }, registryUrl: string) { + const result = await runCli(['login', '--registry', registryUrl, '--token', 'sk_ok'], { + HOME: env.home, + USERPROFILE: env.home + }) + if (result.exitCode !== 0) { + throw new Error(`login failed: ${result.stderr}`) + } +} + +async function makeTempDir(...files: Array<[string, string]>) { + const dir = await mkdtemp(join(tmpdir(), 'skillhub-dryrun-')) + for (const [name, content] of files) { + await writeFile(join(dir, name), content) + } + return dir +} + +describe('publish --dry-run', () => { + test('calls validate endpoint and reports success', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '---\nname: my-skill\ndescription: A test\n---\n# Hello']) + const result = await runCli(['publish', dir, '--dry-run', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(0) + expect(result.stdout).toContain('Validation passed') + expect(registry.received.validate).not.toBeNull() + expect(registry.received.validate!.namespace).toBe('global') + expect(registry.received.publish).toBeNull() + }) + + test('--dry-run with --json returns structured response on warnings (valid=false)', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + dryRunResponse: { + valid: false, + errors: [], + warnings: ['Disallowed file extension: data.bin'], + resolvedSlug: 'my-skill', + resolvedVersion: '2.0.0' + } + }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '---\nname: my-skill\ndescription: test\n---\n']) + const result = await runCli(['publish', dir, '--dry-run', '--json', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(6) + const json = JSON.parse(result.stdout) + expect(json.valid).toBe(false) + expect(json.resolvedSlug).toBe('my-skill') + expect(json.resolvedVersion).toBe('2.0.0') + expect(json.warnings).toContain('Disallowed file extension: data.bin') + }) + + test('--dry-run reports validation errors', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + dryRunResponse: { + valid: false, + errors: ['Missing required file: SKILL.md at root'], + warnings: [], + resolvedSlug: null, + resolvedVersion: null + } + }) + await login(env, registry.url) + + const dir = await makeTempDir(['README.md', '# No SKILL.md here']) + const result = await runCli(['publish', dir, '--dry-run', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(6) + expect(result.stdout).toContain('Validation failed') + expect(result.stdout).toContain('Missing required file: SKILL.md at root') + }) + + test('--dry-run does not actually publish', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '---\nname: test\ndescription: test\n---\n']) + await runCli(['publish', dir, '--dry-run', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(registry.received.publish).toBeNull() + }) + + test('--dry-run respects --namespace', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '---\nname: test\ndescription: test\n---\n']) + await runCli(['publish', dir, '--dry-run', '--namespace', 'myteam', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(registry.received.validate!.namespace).toBe('myteam') + }) + + test('--dry-run forwards --visibility to server', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '---\nname: test\ndescription: test\n---\n']) + await runCli(['publish', dir, '--dry-run', '--visibility', 'private', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(registry.received.validate!.visibility).toBe('PRIVATE') + }) + + test('--dry-run requires authentication', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + + const dir = await makeTempDir(['SKILL.md', '---\nname: test\ndescription: test\n---\n']) + const result = await runCli(['publish', dir, '--dry-run', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(2) + expect(result.stderr).toContain('authentication') + }) + + test('--dry-run reports scope error on 403', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok', failures: { validate: 'forbidden' } }) + await login(env, registry.url) + + const dir = await makeTempDir(['SKILL.md', '---\nname: test\ndescription: test\n---\n']) + const result = await runCli(['publish', dir, '--dry-run', '--registry', registry.url], { + HOME: env.home, + USERPROFILE: env.home + }) + + expect(result.exitCode).toBe(2) + expect(result.stderr).toContain('scope') + expect(result.stderr).toContain('Request ID: req-test-forbidden') + }) +}) diff --git a/cli/test/integration/remove-command.test.ts b/cli/test/integration/remove-command.test.ts index 3704d862..35148d77 100644 --- a/cli/test/integration/remove-command.test.ts +++ b/cli/test/integration/remove-command.test.ts @@ -320,4 +320,111 @@ describe('remove command — local remove (P1)', () => { const agents = parsed.removed.map((r: { agent: string }) => r.agent).sort() expect(agents).toEqual(['claude-code', 'cursor']) }) + + // ------------------------------------------------------------------------- + // P1: --remote --hard against a slug that doesn't exist on the server + // ------------------------------------------------------------------------- + test('--remote --hard for a nonexistent slug surfaces server 404 as non-zero exit', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ + token: 'sk_ok', + user: { handle: 'u', displayName: 'U' } + // No skills configured → DELETE returns 404. + }) + + const result = await runCli( + [ + 'remove', 'never-published', + '--remote', '--hard', + '--namespace', 'global', + '--registry', registry.url, + '--token', 'sk_ok' + ], + { HOME: env.home, USERPROFILE: env.home } + ) + + expect(result.exitCode).not.toBe(0) + expect(result.stderr.toLowerCase()).toMatch(/404|not found|registry returned 4/) + }) + + // ------------------------------------------------------------------------- + // P1: --agent on a multi-target inventory leaves OTHER agents' targets + // intact in the inventory file (not just in the JSON envelope). + // ------------------------------------------------------------------------- + test('--agent removes one target while leaving others in inventory.json', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + + const rootDir = `${env.home}/agents` + const codexDir = `${rootDir}/codex/skills/keep-others` + const claudeDir = `${rootDir}/claude-code/skills/keep-others` + await createInstallDir(codexDir) + await createInstallDir(claudeDir) + + await seedInventory(env.home, [ + { + registry: registry.url, + namespace: 'global', + slug: 'keep-others', + version: '1.0.0', + targets: [ + { agent: 'codex', rootDir: `${rootDir}/codex`, installDir: codexDir, installedAt: '2026-04-20T00:00:00Z' }, + { agent: 'claude-code', rootDir: `${rootDir}/claude-code`, installDir: claudeDir, installedAt: '2026-04-20T00:00:00Z' } + ] + } + ]) + + const result = await runCli( + ['remove', 'keep-others', '--agent', 'codex', '--registry', registry.url, '--json'], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + const inv = JSON.parse(await Bun.file(`${env.home}/.skillhub/inventory.json`).text()) as { + items: Array<{ slug: string; targets: Array<{ agent: string }> }> + } + const survived = inv.items.find(i => i.slug === 'keep-others') + expect(survived).toBeDefined() + expect(survived!.targets.map(t => t.agent)).toEqual(['claude-code']) + }) + + // ------------------------------------------------------------------------- + // P1: --agent + --namespace together filter precisely so a same-slug skill + // in a different namespace is not collateral damage. + // ------------------------------------------------------------------------- + test('--agent + --namespace filters precisely; same slug under different namespace is untouched', async () => { + const env = await createTempHome() + registry = await startFakeRegistry({ token: 'sk_ok' }) + + const rootDir = `${env.home}/agents` + const aDir = `${rootDir}/codex/skills/dup-slug-A` + const bDir = `${rootDir}/codex/skills/dup-slug-B` + await createInstallDir(aDir) + await createInstallDir(bDir) + + await seedInventory(env.home, [ + { + registry: registry.url, namespace: 'team-a', slug: 'dup-slug-A', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: `${rootDir}/codex`, installDir: aDir, installedAt: '2026-04-20T00:00:00Z' }] + }, + { + registry: registry.url, namespace: 'team-b', slug: 'dup-slug-B', version: '1.0.0', + targets: [{ agent: 'codex', rootDir: `${rootDir}/codex`, installDir: bDir, installedAt: '2026-04-20T00:00:00Z' }] + } + ]) + + // Remove dup-slug-A only — dup-slug-B should survive even though both + // share the codex agent. + const result = await runCli( + ['remove', 'dup-slug-A', '--agent', 'codex', '--registry', registry.url], + { HOME: env.home, USERPROFILE: env.home } + ) + expect(result.exitCode).toBe(0) + + const inv = JSON.parse(await Bun.file(`${env.home}/.skillhub/inventory.json`).text()) as { + items: Array<{ slug: string }> + } + const slugs = inv.items.map(i => i.slug).sort() + expect(slugs).toEqual(['dup-slug-B']) + }) }) diff --git a/cli/test/integration/search-command.test.ts b/cli/test/integration/search-command.test.ts index 2f064783..f352b158 100644 --- a/cli/test/integration/search-command.test.ts +++ b/cli/test/integration/search-command.test.ts @@ -10,6 +10,109 @@ afterEach(() => { }) describe('search command', () => { + test('--token sends bearer auth and takes priority over SKILLHUB_TOKEN', async () => { + let capturedAuth = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + capturedAuth = req.headers.get('authorization') ?? '' + return Response.json({ + code: 0, + data: { + items: [{ namespace: 'global', slug: 'pdf-parser', latestVersion: '1.2.0', summary: 'Parse PDFs' }], + total: 1, + limit: 20 + } + }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const result = await runCli( + ['search', 'pdf', '--registry', `http://localhost:${server.port}`, '--token', 'sk_ok'], + { SKILLHUB_TOKEN: 'sk_bad' } + ) + + expect(result.exitCode).toBe(0) + expect(capturedAuth).toBe('Bearer sk_ok') + expect(result.stdout).toContain('global/pdf-parser') + } finally { + server.stop() + } + }) + + test('bad --token fails with auth output and does not retry anonymously', async () => { + const authHeaders: Array = [] + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + const auth = req.headers.get('authorization') + authHeaders.push(auth) + if (auth === 'Bearer sk_bad') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ + code: 0, + data: { + items: [{ namespace: 'global', slug: 'anonymous-only', latestVersion: '1.0.0', summary: 'anonymous fallback' }], + total: 1, + limit: 20 + } + }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const registryUrl = `http://localhost:${server.port}` + const result = await runCli(['search', 'pdf', '--registry', registryUrl, '--token', 'sk_bad']) + + expect(result.exitCode).toBe(2) + expect(result.stderr).toContain('Error: authentication failed') + expect(result.stderr).toContain(`Context: registry ${registryUrl}`) + expect(result.stderr).toContain('Next:') + expect(authHeaders).toEqual(['Bearer sk_bad']) + } finally { + server.stop() + } + }) + + test('bad --token returns structured json auth error', async () => { + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + return Response.json({ code: 401, message: 'unauthorized' }, { status: 401 }) + } + return Response.json({ code: 404 }, { status: 404 }) + } + }) + + try { + const registryUrl = `http://localhost:${server.port}` + const result = await runCli(['search', 'pdf', '--registry', registryUrl, '--token', 'sk_bad', '--json']) + + expect(result.exitCode).toBe(2) + const parsed = JSON.parse(result.stderr) + expect(parsed.ok).toBe(false) + expect(parsed.message).toBe('authentication failed') + expect(parsed.exitCode).toBe(2) + expect(parsed.details.registry).toBe(registryUrl) + expect(typeof parsed.details.next).toBe('string') + expect(parsed.details.next).toContain('skillhub login') + } finally { + server.stop() + } + }) + test('prints compact search table', async () => { registry = await startFakeRegistry({ searchItems: [{ namespace: 'global', slug: 'pdf-parser', latestVersion: '1.2.0', summary: 'Parse PDFs' }] @@ -103,4 +206,235 @@ describe('search command', () => { expect(result.exitCode).toBe(3) // EXIT.network expect(result.stderr).toMatch(/registry unreachable|registry returned 5\d\d/) }) + + // ------------------------------------------------------------------------- + // P2: query containing non-ASCII characters must be URL-encoded in the + // outgoing request. We capture the raw URL via a custom Bun.serve and + // assert the q parameter is the percent-encoded UTF-8 form of "中文测试". + // ------------------------------------------------------------------------- + test('non-ASCII query is URL-encoded as UTF-8 percent escapes', async () => { + let capturedUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + capturedUrl = req.url + return Response.json({ code: 0, data: { items: [], total: 0, limit: 20 } }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + const registryUrl = `http://localhost:${server.port}` + try { + const result = await runCli(['search', '中文测试', '--registry', registryUrl]) + expect(result.exitCode).toBe(0) + // UTF-8 of 中文测试 = E4 B8 AD E6 96 87 E6 B5 8B E8 AF 95 + expect(capturedUrl).toContain('q=%E4%B8%AD%E6%96%87%E6%B5%8B%E8%AF%95') + } finally { + server.stop() + } + }) + + // ------------------------------------------------------------------------- + // P2: queries containing special characters (script tags, ampersands, + // equals signs) are percent-encoded so they don't break the query string. + // ------------------------------------------------------------------------- + test('special-character query is encoded so the URL stays parseable', async () => { + let capturedUrl = '' + const server = Bun.serve({ + port: 0, + fetch(req) { + const url = new URL(req.url) + if (url.pathname === '/api/cli/v1/skills/search') { + capturedUrl = req.url + return Response.json({ code: 0, data: { items: [], total: 0, limit: 20 } }) + } + return Response.json({ code: 404, message: 'not found' }, { status: 404 }) + } + }) + const registryUrl = `http://localhost:${server.port}` + try { + const result = await runCli(['search', '