From 836267fd453741fdf5f6e716f758343f9337d7c0 Mon Sep 17 00:00:00 2001 From: dongmucat <1127093059@qq.com> Date: Mon, 11 May 2026 14:39:06 +0800 Subject: [PATCH] fix(cli): respect configured npm registry --- cli/src/clients/npm-registry-client.ts | 69 ++++++++- .../unit/clients/npm-registry-client.test.ts | 143 +++++++++++++++++- 2 files changed, 202 insertions(+), 10 deletions(-) diff --git a/cli/src/clients/npm-registry-client.ts b/cli/src/clients/npm-registry-client.ts index 826598ea..4f4912a3 100644 --- a/cli/src/clients/npm-registry-client.ts +++ b/cli/src/clients/npm-registry-client.ts @@ -1,32 +1,85 @@ import { EXIT, CLI_PACKAGE_NAME } from '../shared/constants' import { CliError } from '../shared/errors' +const DEFAULT_NPM_REGISTRY = 'https://registry.npmjs.org' + +function readEnv(env: NodeJS.ProcessEnv, name: string): string | undefined { + const exactValue = env[name]?.trim() + if (exactValue) { + return exactValue + } + + const lowerName = name.toLowerCase() + for (const [key, value] of Object.entries(env)) { + const normalizedValue = value?.trim() + if (key.toLowerCase() === lowerName && normalizedValue) { + return normalizedValue + } + } + return undefined +} + +function resolveRegistry(env: NodeJS.ProcessEnv): string { + return readEnv(env, 'SKILLHUB_NPM_REGISTRY') + ?? readEnv(env, 'npm_config_registry') + ?? readEnv(env, 'NPM_CONFIG_REGISTRY') + ?? DEFAULT_NPM_REGISTRY +} + +function buildLatestUrl(registry: string, packageName: string): string { + try { + const base = registry.endsWith('/') ? registry : `${registry}/` + return new URL(`${encodeURIComponent(packageName)}/latest`, base).toString() + } catch { + throw new CliError('invalid npm registry URL', EXIT.usage, { + registry, + next: 'check npm registry configuration and retry' + }) + } +} + export class NpmRegistryClient { constructor( private readonly fetchImpl: typeof fetch = fetch, - private readonly timeoutMs = 10_000 + private readonly timeoutMs = 10_000, + private readonly env: NodeJS.ProcessEnv = process.env ) {} async latestVersion(packageName = CLI_PACKAGE_NAME): Promise { + const registry = resolveRegistry(this.env) + const url = buildLatestUrl(registry, packageName) const controller = new AbortController() const timer = setTimeout(() => controller.abort(), this.timeoutMs) try { let response: Response try { - response = await this.fetchImpl(`https://registry.npmjs.org/${packageName}/latest`, { + response = await this.fetchImpl(url, { signal: controller.signal }) - } catch { + } catch (error) { + const isTimeout = error instanceof Error && error.name === 'AbortError' throw new CliError('npm registry unreachable', EXIT.network, { - next: 'check network connectivity and retry' + registry, + cause: error instanceof Error ? error.message : String(error), + next: isTimeout + ? 'check npm registry connectivity or proxy settings and retry' + : 'check npm registry/proxy configuration and retry' }) } if (!response.ok) { - throw new CliError(`npm registry returned ${response.status}`, EXIT.network) + throw new CliError(`npm registry returned ${response.status}`, EXIT.network, { registry }) } - const body = await response.json() - if (typeof body.version !== 'string') { - throw new CliError('npm registry response missing version', EXIT.network) + let body: unknown + try { + body = await response.json() + } catch (error) { + throw new CliError('npm registry response invalid', EXIT.network, { + registry, + cause: error instanceof Error ? error.message : String(error) + }) + } + if (typeof body !== 'object' || body === null || !('version' in body) || typeof body.version !== 'string') { + throw new CliError('npm registry response missing version', EXIT.network, { registry }) } return body.version } finally { diff --git a/cli/test/unit/clients/npm-registry-client.test.ts b/cli/test/unit/clients/npm-registry-client.test.ts index 32583bb7..d348238c 100644 --- a/cli/test/unit/clients/npm-registry-client.test.ts +++ b/cli/test/unit/clients/npm-registry-client.test.ts @@ -2,15 +2,154 @@ import { describe, expect, test } from 'bun:test' import { NpmRegistryClient } from '../../../src/clients/npm-registry-client' describe('NpmRegistryClient', () => { + test('uses npm_config_registry when checking the latest version', async () => { + let requestedUrl = '' + const successfulFetch = (async (input: RequestInfo | URL) => { + requestedUrl = String(input) + return Response.json({ version: '1.2.3' }) + }) as typeof fetch + const client = new NpmRegistryClient(successfulFetch, 10_000, { + npm_config_registry: 'https://registry.npmmirror.com' + }) + + await expect(client.latestVersion()).resolves.toBe('1.2.3') + expect(requestedUrl).toBe('https://registry.npmmirror.com/%40astron-team%2Fskillhub/latest') + }) + + test('uses SkillHub registry override before npm registry env vars', async () => { + let requestedUrl = '' + const successfulFetch = (async (input: RequestInfo | URL) => { + requestedUrl = String(input) + return Response.json({ version: '1.2.3' }) + }) as typeof fetch + const client = new NpmRegistryClient(successfulFetch, 10_000, { + SKILLHUB_NPM_REGISTRY: 'https://skillhub-registry.example.test/npm/', + npm_config_registry: 'https://lower-priority.example.test', + NPM_CONFIG_REGISTRY: 'https://lowest-priority.example.test' + }) + + await expect(client.latestVersion()).resolves.toBe('1.2.3') + expect(requestedUrl).toBe('https://skillhub-registry.example.test/npm/%40astron-team%2Fskillhub/latest') + }) + + test('resolves registry env names case-insensitively for Windows compatibility', async () => { + let requestedUrl = '' + const successfulFetch = (async (input: RequestInfo | URL) => { + requestedUrl = String(input) + return Response.json({ version: '1.2.3' }) + }) as typeof fetch + const client = new NpmRegistryClient(successfulFetch, 10_000, { + skillhub_npm_registry: 'https://windows-env.example.test' + }) + + await expect(client.latestVersion()).resolves.toBe('1.2.3') + expect(requestedUrl).toBe('https://windows-env.example.test/%40astron-team%2Fskillhub/latest') + }) + + test('ignores empty registry env values before falling back', async () => { + let requestedUrl = '' + const successfulFetch = (async (input: RequestInfo | URL) => { + requestedUrl = String(input) + return Response.json({ version: '1.2.3' }) + }) as typeof fetch + const client = new NpmRegistryClient(successfulFetch, 10_000, { + SKILLHUB_NPM_REGISTRY: ' ', + npm_config_registry: '', + NPM_CONFIG_REGISTRY: 'https://registry.example.test' + }) + + await expect(client.latestVersion()).resolves.toBe('1.2.3') + expect(requestedUrl).toBe('https://registry.example.test/%40astron-team%2Fskillhub/latest') + }) + + test('uses the default npm registry when no registry is configured', async () => { + let requestedUrl = '' + const successfulFetch = (async (input: RequestInfo | URL) => { + requestedUrl = String(input) + return Response.json({ version: '1.2.3' }) + }) as typeof fetch + const client = new NpmRegistryClient(successfulFetch, 10_000, {}) + + await expect(client.latestVersion()).resolves.toBe('1.2.3') + expect(requestedUrl).toBe('https://registry.npmjs.org/%40astron-team%2Fskillhub/latest') + }) + test('classifies network failures as CLI errors', async () => { const failingFetch = (async () => { throw new TypeError('fetch failed') }) as unknown as typeof fetch - const client = new NpmRegistryClient(failingFetch) + const client = new NpmRegistryClient(failingFetch, 10_000, { + NPM_CONFIG_REGISTRY: 'https://registry.example.test' + }) await expect(client.latestVersion()).rejects.toMatchObject({ message: 'npm registry unreachable', - exitCode: 3 + exitCode: 3, + details: { + registry: 'https://registry.example.test', + cause: 'fetch failed', + next: 'check npm registry/proxy configuration and retry' + } + }) + }) + + test('reports registry context for non-2xx responses', async () => { + const failingFetch = (async () => new Response('{}', { status: 503 })) as unknown as typeof fetch + const client = new NpmRegistryClient(failingFetch, 10_000, { + SKILLHUB_NPM_REGISTRY: 'https://registry.example.test' + }) + + await expect(client.latestVersion()).rejects.toMatchObject({ + message: 'npm registry returned 503', + exitCode: 3, + details: { + registry: 'https://registry.example.test' + } + }) + }) + + test('rejects registry responses without a version', async () => { + const failingFetch = (async () => Response.json({ name: '@astron-team/skillhub' })) as unknown as typeof fetch + const client = new NpmRegistryClient(failingFetch, 10_000, { + npm_config_registry: 'https://registry.example.test' + }) + + await expect(client.latestVersion()).rejects.toMatchObject({ + message: 'npm registry response missing version', + exitCode: 3, + details: { + registry: 'https://registry.example.test' + } + }) + }) + + test('rejects invalid JSON registry responses', async () => { + const failingFetch = (async () => new Response('not json')) as unknown as typeof fetch + const client = new NpmRegistryClient(failingFetch, 10_000, { + npm_config_registry: 'https://registry.example.test' + }) + + await expect(client.latestVersion()).rejects.toMatchObject({ + message: 'npm registry response invalid', + exitCode: 3, + details: { + registry: 'https://registry.example.test' + } + }) + }) + + test('rejects invalid registry configuration', async () => { + const client = new NpmRegistryClient(fetch, 10_000, { + npm_config_registry: 'https://[' + }) + + await expect(client.latestVersion()).rejects.toMatchObject({ + message: 'invalid npm registry URL', + exitCode: 5, + details: { + registry: 'https://[', + next: 'check npm registry configuration and retry' + } }) }) })