chore: leftover local catalog/install work

backend/src/domain/artifact/install-plan.test.ts

@@ -73,6 +73,124 @@ describe('buildInstallPlan', () => {
     expect(plan.warningKeys).toEqual([])
   })
 
+  it('installs a GitHub-indexed package by its verified npm name, not a git spec', () => {
+    const published = Artifact.create({
+      id: 'dsh-context',
+      kind: 'bundle',
+      displayName: 'dsh-context',
+      summary: 'Context insight.',
+      source: githubSource({
+        owner: 'bowenliang123',
+        repo: 'dsh-context',
+        commit: 'a'.repeat(40),
+        npm: { packageName: 'dsh-context', latestVersion: '0.8.0' },
+      }),
+      payload: { kind: 'bundle', requiresBuild: true },
+    })
+
+    const plan = buildInstallPlan(published, installTarget('local-dsh'))
+    expect(plan.steps).toEqual([
+      {
+        type: 'add-package',
+        profile: 'local-dsh',
+        spec: 'dsh-context',
+        requiresBuildAllowance: false,
+      },
+    ])
+    expect(plan.manualCommands).toEqual([
+      'npx @dsh-fish/cli add dsh-context --profile local-dsh',
+      'dsh plugin --profile local-dsh add dsh-context',
+    ])
+    expect(plan.warningKeys).not.toContain('install.warning.buildAllowance')
+  })
+
+  it('does not treat a legal display name as an npm package', () => {
+    // The display name is a legal package.json name, but nothing verified it
+    // is published under this repository. Guessing would 404 and leave a
+    // ghost dependency that bricks later installs.
+    const unpublished = Artifact.create({
+      id: 'dsh-inline-comments',
+      kind: 'bundle',
+      displayName: 'dsh-inline-comments',
+      summary: 'Inline comments.',
+      source: githubSource({
+        owner: 'acme',
+        repo: 'dsh-inline-comments',
+        commit: 'a'.repeat(40),
+      }),
+      payload: { kind: 'bundle', requiresBuild: true },
+    })
+
+    const plan = buildInstallPlan(unpublished, target)
+    expect(plan.steps[0]).toMatchObject({
+      spec: `github:acme/dsh-inline-comments#${'a'.repeat(40)}`,
+      requiresBuildAllowance: true,
+    })
+    expect(plan.warningKeys).toContain('install.warning.buildAllowance')
+  })
+
+  it('prefers a same-repo Release tarball over a git checkout', () => {
+    const tarball =
+      'https://github.com/acme/thing/releases/download/v1.0.0/thing-1.0.0.tgz'
+    const prebuilt = Artifact.create({
+      id: 'thing',
+      kind: 'bundle',
+      displayName: 'thing',
+      summary: 'A prebuilt plugin.',
+      source: githubSource({
+        owner: 'acme',
+        repo: 'thing',
+        commit: 'a'.repeat(40),
+        releaseTarball: tarball,
+      }),
+      payload: { kind: 'bundle', requiresBuild: true },
+    })
+
+    const plan = buildInstallPlan(prebuilt, target)
+    expect(plan.steps[0]).toMatchObject({ spec: tarball, requiresBuildAllowance: false })
+    expect(plan.warningKeys).not.toContain('install.warning.buildAllowance')
+  })
+
+  it('prefers verified npm over a Release tarball', () => {
+    const tarball = 'https://github.com/acme/thing/releases/download/v1.0.0/thing-1.0.0.tgz'
+    const published = Artifact.create({
+      id: 'thing',
+      kind: 'bundle',
+      displayName: 'thing',
+      summary: 'Published and prebuilt.',
+      source: githubSource({
+        owner: 'acme',
+        repo: 'thing',
+        commit: 'a'.repeat(40),
+        npm: { packageName: 'thing', latestVersion: '1.0.0' },
+        releaseTarball: tarball,
+      }),
+      payload: { kind: 'bundle', requiresBuild: false },
+    })
+
+    expect(buildInstallPlan(published, target).steps[0]).toMatchObject({ spec: 'thing' })
+  })
+
+  it('pins a monorepo subdirectory onto the git spec', () => {
+    const nested = Artifact.create({
+      id: 'theme-gallery',
+      kind: 'bundle',
+      displayName: 'theme-gallery',
+      summary: 'A package inside a collection repo.',
+      source: githubSource({
+        owner: 'acme',
+        repo: 'dsh-plugins',
+        path: 'packages/theme-gallery',
+        commit: 'a'.repeat(40),
+      }),
+      payload: { kind: 'bundle', requiresBuild: false },
+    })
+
+    expect(buildInstallPlan(nested, target).steps[0]).toMatchObject({
+      spec: `github:acme/dsh-plugins#${'a'.repeat(40)}&path:/packages/theme-gallery`,
+    })
+  })
+
   it('pins a git bundle to a commit and warns when one is missing', () => {
     const pinned = buildInstallPlan(
       artifact(

backend/src/domain/artifact/install-plan.ts

@@ -1,7 +1,7 @@
 import { DomainError } from '../shared/error.js'
 import type { Artifact } from './artifact.js'
 import type { ArtifactKind } from './artifact-kind.js'
-import { packageSpec } from './source-ref.js'
+import { installTargetFor } from './source-ref.js'
 
 /**
  * Where an install writes. The hub never learns a machine's real paths: a step
@@ -93,23 +93,24 @@ export function buildInstallPlan(artifact: Artifact, target: InstallTarget): Ins
 
   switch (payload.kind) {
     case 'bundle': {
-      const spec = packageSpec(artifact.source)
+      const spec = bundleInstallSpec(artifact)
       if (spec === undefined) {
         throw DomainError.unsupported('This bundle has no installable package specifier.', {
           artifactId: artifact.id,
         })
       }
+      const gitSpec = spec.startsWith('github:')
       steps.push({
         type: 'add-package',
         profile: target.profile,
         spec,
-        requiresBuildAllowance: payload.requiresBuild,
+        requiresBuildAllowance: gitSpec && payload.requiresBuild,
       })
       manualCommands.push(`dsh plugin --profile ${target.profile} add ${spec}`)
-      if (payload.requiresBuild) {
+      if (gitSpec && payload.requiresBuild) {
         warningKeys.push('install.warning.buildAllowance')
       }
-      if (artifact.source.origin === 'github' && artifact.source.commit === undefined) {
+      if (gitSpec && artifact.source.origin === 'github' && artifact.source.commit === undefined) {
         warningKeys.push('install.warning.unpinnedGitSpec')
       }
       break
@@ -176,3 +177,15 @@ export function buildInstallPlan(artifact: Artifact, target: InstallTarget): Ins
       : { scannedAtCommit: artifact.sourceCommitSha }),
   }
 }
+
+/**
+ * Specifier `dsh plugin add` should receive for a catalog bundle.
+ *
+ * The choice is made on the source at index time (verified npm, then a
+ * same-repo Release tarball, then a pinned git spec). Display names are not
+ * consulted: a legal package.json name that is unpublished, or belongs to
+ * another repository, must not become the install spec.
+ */
+export function bundleInstallSpec(artifact: Artifact): string | undefined {
+  return installTargetFor(artifact.source)
+}

backend/src/domain/artifact/npm-binding.test.ts

@@ -0,0 +1,50 @@
+import { describe, expect, it } from 'vitest'
+import { npmBindingFromPackument } from './npm-binding.js'
+
+const packument = {
+  name: 'dsh-context',
+  'dist-tags': { latest: '0.8.0' },
+  versions: {
+    '0.8.0': {
+      repository: { url: 'git+https://github.com/bowenliang123/dsh-context.git' },
+    },
+  },
+}
+
+describe('npmBindingFromPackument', () => {
+  it('binds a packument whose repository is this owner/repo', () => {
+    expect(npmBindingFromPackument(packument, 'bowenliang123', 'dsh-context')).toEqual({
+      packageName: 'dsh-context',
+      latestVersion: '0.8.0',
+    })
+  })
+
+  it('ignores a packument that points at a different repository', () => {
+    expect(npmBindingFromPackument(packument, 'evil', 'dsh-context')).toBeUndefined()
+  })
+
+  it('ignores a packument with no repository field', () => {
+    expect(
+      npmBindingFromPackument(
+        { name: 'dsh-context', 'dist-tags': { latest: '0.8.0' }, versions: { '0.8.0': {} } },
+        'bowenliang123',
+        'dsh-context',
+      ),
+    ).toBeUndefined()
+  })
+
+  it('reads repository off the packument root when the version omits it', () => {
+    expect(
+      npmBindingFromPackument(
+        {
+          name: 'thing',
+          repository: 'github:acme/thing',
+          'dist-tags': { latest: '1.0.0' },
+          versions: { '1.0.0': {} },
+        },
+        'acme',
+        'thing',
+      ),
+    ).toEqual({ packageName: 'thing', latestVersion: '1.0.0' })
+  })
+})

backend/src/domain/artifact/npm-binding.ts

@@ -0,0 +1,47 @@
+import { githubRepoFromUrl, isNpmPackageName } from './source-ref.js'
+import type { VerifiedNpmBinding } from './source-ref.js'
+
+/**
+ * The fields of an npm packument that prove a package name belongs to a
+ * GitHub repository. Everything else on the document is ignored.
+ */
+export interface PackumentRepositoryHint {
+  readonly name?: string
+  readonly repository?: { readonly url?: string } | string
+  readonly 'dist-tags'?: { readonly latest?: string }
+  readonly versions?: Readonly<
+    Record<string, { readonly repository?: { readonly url?: string } | string }>
+  >
+}
+
+/**
+ * A published package whose packument `repository` is `owner/repo`.
+ *
+ * Undefined when the name is unpublished, malformed, or points at a different
+ * remote — that last case is name-squatting protection: a legal package.json
+ * name is not enough.
+ */
+export function npmBindingFromPackument(
+  packument: PackumentRepositoryHint,
+  owner: string,
+  repo: string,
+): VerifiedNpmBinding | undefined {
+  const packageName = packument.name
+  const latestVersion = packument['dist-tags']?.latest
+  if (packageName === undefined || latestVersion === undefined) return undefined
+  if (!isNpmPackageName(packageName) || latestVersion.trim() === '') return undefined
+
+  const version = packument.versions?.[latestVersion]
+  const github = githubRepoOf(version?.repository ?? packument.repository)
+  if (github === undefined) return undefined
+  if (github.owner.toLowerCase() !== owner.toLowerCase()) return undefined
+  if (github.repo.toLowerCase() !== repo.toLowerCase()) return undefined
+  return { packageName, latestVersion: latestVersion.trim() }
+}
+
+function githubRepoOf(
+  repository: { readonly url?: string } | string | undefined,
+): { owner: string; repo: string } | undefined {
+  const url = typeof repository === 'string' ? repository : repository?.url
+  return url === undefined ? undefined : githubRepoFromUrl(url)
+}

backend/src/domain/artifact/source-ref.test.ts

@@ -2,8 +2,10 @@ import { describe, expect, it } from 'vitest'
 import {
   githubRepoFromUrl,
   githubSource,
+  installTargetFor,
   mergeProvenance,
   npmSource,
+  releaseTarballTarget,
   sourceAssetBase,
   sourceDocBase,
   submissionSource,
@@ -102,4 +104,83 @@ describe('mergeProvenance', () => {
       latestVersion: '1.1.0',
     })
   })
+
+  it('keeps a verified npm binding a later crawl did not re-check', () => {
+    const listed = githubSource({
+      owner: 'acme',
+      repo: 'thing',
+      npm: { packageName: 'thing', latestVersion: '1.0.0' },
+      releaseTarball: 'https://github.com/acme/thing/releases/download/v1.0.0/thing.tgz',
+    })
+    const crawled = githubSource({ owner: 'acme', repo: 'thing', commit: 'abc1234' })
+
+    expect(mergeProvenance(listed, crawled)).toMatchObject({
+      commit: 'abc1234',
+      npm: { packageName: 'thing', latestVersion: '1.0.0' },
+      releaseTarball: 'https://github.com/acme/thing/releases/download/v1.0.0/thing.tgz',
+    })
+  })
+})
+
+describe('installTargetFor', () => {
+  it('pins an npm source to its latest version', () => {
+    expect(installTargetFor(npmSource('thing', '1.2.3'))).toBe('thing@1.2.3')
+  })
+
+  it('uses a verified npm name, then a bound tarball, then a pinned git spec', () => {
+    const tarball = 'https://github.com/acme/thing/releases/download/v1.0.0/thing.tgz'
+    expect(
+      installTargetFor(
+        githubSource({
+          owner: 'acme',
+          repo: 'thing',
+          commit: 'abc1234',
+          npm: { packageName: 'thing', latestVersion: '1.0.0' },
+          releaseTarball: tarball,
+        }),
+      ),
+    ).toBe('thing')
+    expect(
+      installTargetFor(
+        githubSource({ owner: 'acme', repo: 'thing', commit: 'abc1234', releaseTarball: tarball }),
+      ),
+    ).toBe(tarball)
+    expect(installTargetFor(githubSource({ owner: 'acme', repo: 'thing', commit: 'abc1234' }))).toBe(
+      'github:acme/thing#abc1234',
+    )
+  })
+})
+
+describe('releaseTarballTarget', () => {
+  const repo = 'acme/thing'
+
+  it('accepts an HTTPS GitHub Release archive for the same repository', () => {
+    const url = 'https://github.com/acme/thing/releases/download/v1.0.0/thing-1.0.0.tgz'
+    expect(releaseTarballTarget(url, repo)).toBe(url)
+  })
+
+  it('rejects an archive that is not this repository', () => {
+    expect(
+      releaseTarballTarget(
+        'https://github.com/evil/repo/releases/download/v1.0.0/p.tgz',
+        repo,
+      ),
+    ).toBeUndefined()
+  })
+
+  it('rejects a release CDN URL that cannot be bound to a repository', () => {
+    expect(
+      releaseTarballTarget('https://objects.githubusercontent.com/whatever/x.tgz', repo),
+    ).toBeUndefined()
+  })
+
+  it('refuses to construct a source whose tarball is not this repository', () => {
+    expect(() =>
+      githubSource({
+        owner: 'acme',
+        repo: 'thing',
+        releaseTarball: 'https://github.com/evil/repo/releases/download/v1/p.tgz',
+      }),
+    ).toThrow(/release tarball/i)
+  })
 })