Add scoped-name registry routing, fix cachePut parent-dir creation for slash-containing identities, and tighten scoped source parsing

Author: eXamadeus

openspec/changes/tighten-protocol-slug-facet-name-validation/.openspec.yaml

@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-06-15

packages/cli/src/commands/publish/__tests__/publish.test.ts

@@ -511,18 +511,17 @@ describe('publishCommand — registry interaction (preserved scenarios)', () =>
     expect(stdout).toContain('an admin will review your submission shortly')
   })
 
-  test('scoped facet: URL-encodes the scoped identity', async () => {
+  test('scoped facet: uploads via the two-segment scoped route (no %2F)', async () => {
     // The facet identity grammar (FacetManifestSchema) accepts an unscoped
     // slug (`cowsay`) or a scoped `@scope/name` (`@acme/cowsay`); the legacy
     // bare-slash form (`acme/cowsay`) is no longer a valid manifest name.
     //
-    // TODO(scoped-routes): the registry has moved to literal-slash scoped
-    // routes (`/v0/facets/@scope/name/...`), but the engine registry client
-    // still hits the single-`{name}` route and lets openapi-fetch percent-
-    // encode the whole identity (`@acme/cowsay` → `%40acme%2Fcowsay`). The
-    // scoped-route migration (publish/resolve/download) is tracked separately;
-    // this assertion documents the *current* client behavior and is expected
-    // to flip to the literal-slash form when that lands.
+    // The publish client routes scoped names through the registry's
+    // two-segment scoped route (`/v0/facets/{scope}/{name}/versions`) so the
+    // scope `/` stays a literal path separator. The scope marker is its own
+    // segment (`%40acme`) and the name is a separate segment (`cowsay`); the
+    // slash between them is NOT percent-encoded to `%2F` (which the registry
+    // rejects with E_INVALID_NAME).
     await buildFacetFixture(projectRoot, {
       name: '@acme/cowsay',
       version: '0.1.0',
@@ -541,7 +540,8 @@ describe('publishCommand — registry interaction (preserved scenarios)', () =>
     expect(spy.calls).toHaveLength(1)
     const call = spy.calls[0]
     if (call === undefined) expect.unreachable()
-    expect(call.url).toBe('https://api.test/v0/facets/%40acme%2Fcowsay/versions')
+    expect(call.url).toBe('https://api.test/v0/facets/%40acme/cowsay/versions')
+    expect(call.url).not.toContain('%2F')
   })
 
   test('413 (tarball too large): renders the registry error verbatim', async () => {

packages/engine/src/__tests__/cache.test.ts

@@ -198,6 +198,28 @@ describe('cachePut', () => {
     expect(cacheSlotIsDir(b)).toBe(true)
   })
 
+  // Slash-containing identities (scoped `@scope/name` and namespaced
+  // `acme/name`) render as nested slot paths under the cache root. cachePut
+  // must create the slot's parent directory before the rename, or the
+  // rename fails with ENOENT. Goes through cachePut (not seedSlot, which
+  // mkdir -p's the full path and would mask the defect).
+  test.each([
+    ['scoped', '@julian/cowsay'],
+    ['namespaced unscoped', 'acme/cowsay'],
+  ])('populates a %s slot whose identity contains a slash', (_label, name) => {
+    const id: CacheIdentity = { kind: 'registry', name, version: '1.0.0' }
+    const staging = cacheStagingDir()
+    writeFileSync(join(staging, 'facet.json'), `{"name":"${name}","version":"1.0.0"}`)
+    const result = cachePut(id, staging)
+    expect(result.ok).toBe(true)
+    if (!result.ok) expect.unreachable()
+    expect(result.path).toBe(cachePath(id))
+    expect(existsSync(staging)).toBe(false)
+    expect(cacheSlotIsDir(id)).toBe(true)
+    expect(cacheGet(id).hit).toBe(true)
+    expect(existsSync(join(result.path, 'facet.json'))).toBe(true)
+  })
+
   test('local-source slots disambiguate by absolute path', () => {
     const a: CacheIdentity = { kind: 'local', name: 'p', absolutePath: '/path/a' }
     const b: CacheIdentity = { kind: 'local', name: 'p', absolutePath: '/path/b' }

packages/engine/src/__tests__/registry.test.ts

@@ -100,6 +100,24 @@ describe('resolveRegistryMetadataBatch', () => {
     expect(calledUrls[0]).toBe('https://api.test/v0/facets/acme%2Fcowsay/latest')
   })
 
+  test('scoped names use the two-segment scoped route with an unencoded slash', async () => {
+    const calledUrls: string[] = []
+    globalThis.fetch = (async (input: string | URL | Request) => {
+      calledUrls.push(captureUrl(input))
+      return new Response(JSON.stringify(fixtures.versionMetadata({ name: '@julian/cowsay', version: '1.2.3' })), {
+        status: 200,
+      })
+    }) as unknown as typeof fetch
+    await resolveRegistryMetadataBatch([
+      { name: '@julian/cowsay', version: { kind: 'exact', major: 1, minor: 2, patch: 3 } },
+    ])
+    // Scope and name are independent path segments: the scope marker is a
+    // single segment (`%40julian`) and the slash between scope and name is a
+    // real path separator — never collapsed into `%2F`.
+    expect(calledUrls[0]).toBe('https://api.test/v0/facets/%40julian/cowsay/1.2.3')
+    expect(calledUrls[0]).not.toContain('%2F')
+  })
+
   test('404 maps to NOT_FOUND with the spec verbatim', async () => {
     globalThis.fetch = (async () =>
       new Response(JSON.stringify(fixtures.apiError({ error: 'gone', docs_url: 'x' })), {
@@ -251,6 +269,24 @@ describe('downloadAndExtractFacet', () => {
     expect(readFileSync(join(dest, 'commands/cowsay.md'), 'utf8')).toContain('# cowsay')
   })
 
+  test('scoped name uses the two-segment scoped archive route (no %2F)', async () => {
+    const facetJson = JSON.stringify(
+      { name: '@julian/cowsay', version: '0.1.0', commands: { cowsay: { description: 'Say moo' } } },
+      null,
+      2,
+    )
+    const { bytes, integrity } = buildArchive([
+      { path: 'facet.json', content: facetJson },
+      { path: 'commands/cowsay.md', content: '# cowsay\n' },
+    ])
+    const { urls } = stubArchiveDownload(new Response(bytes, { status: 200 }))
+    const result = await downloadAndExtractFacet({ ...META, name: '@julian/cowsay', transportHash: integrity }, dest)
+    expect(result.ok).toBe(true)
+    expect(urls[0]).toBe('https://api.test/v0/facets/%40julian/cowsay/0.1.0/archive')
+    expect(urls[0]).not.toContain('%2F')
+    expect(urls[1]).toBe(S3_URL)
+  })
+
   test('sha256 mismatch: returns NETWORK_ERROR with hash detail and writes nothing', async () => {
     const { bytes } = buildArchive([{ path: 'facet.json', content: JSON.stringify({ name: 'x', version: '0.1.0' }) }])
     stubArchiveDownload(new Response(bytes, { status: 200 }))

packages/engine/src/cache/operations.ts

@@ -9,7 +9,7 @@ import {
   statSync,
   writeFileSync,
 } from 'node:fs'
-import { join } from 'node:path'
+import { dirname, join } from 'node:path'
 import { validateAssetName } from '@agent-facets/common'
 import type {
   AssetIntegrityFailure,
@@ -145,8 +145,14 @@ export function cacheGet(identity: CacheIdentity): CacheLookup {
  */
 export function cachePut(identity: CacheIdentity, sourceDir: string): CachePutResult {
   const finalPath = cachePath(identity)
-  const root = resolveCacheRoot()
-  mkdirSync(root, { recursive: true })
+  // Create the slot's PARENT directory, not just the cache root. For a flat
+  // (unscoped, slashless) identity `dirname(finalPath)` is the cache root, so
+  // this also covers the common case. For a slash-containing identity —
+  // scoped `@scope/name@version` or namespaced `acme/name@version` — the slot
+  // renders as a nested path (`<root>/@scope/name@version`), whose parent
+  // (`<root>/@scope`) does not exist by default; without this the renameSync
+  // below fails with ENOENT.
+  mkdirSync(dirname(finalPath), { recursive: true })
 
   if (existsSync(finalPath)) {
     if (!isExistingDirectory(finalPath)) {

packages/engine/src/install/__tests__/run-add.test.ts

@@ -212,6 +212,30 @@ describe('runAdd — registry manifest-value rule', () => {
     await add('cowsay')
     expect(readFacets().cowsay).not.toBe('cowsay')
   })
+
+  test('scoped bare name pins the resolved exact version under the scoped key', async () => {
+    registryResolvedVersion = '0.1.1'
+    registryFixtureDir = buildFixture(fakeHome, '@julian/cowsay', '0.1.1')
+    const result = await add('@julian/cowsay')
+    expect(result.ok).toBe(true)
+    expect(readFacets()['@julian/cowsay']).toBe('0.1.1')
+  })
+
+  test('scoped explicit @latest is written verbatim and floats', async () => {
+    registryResolvedVersion = '0.1.1'
+    registryFixtureDir = buildFixture(fakeHome, '@julian/cowsay', '0.1.1')
+    const result = await add('@julian/cowsay@latest')
+    expect(result.ok).toBe(true)
+    expect(readFacets()['@julian/cowsay']).toBe('latest')
+  })
+
+  test('scoped explicit exact version is recorded as written', async () => {
+    registryResolvedVersion = '0.1.1'
+    registryFixtureDir = buildFixture(fakeHome, '@julian/cowsay', '0.1.1')
+    const result = await add('@julian/cowsay@0.1.1')
+    expect(result.ok).toBe(true)
+    expect(readFacets()['@julian/cowsay']).toBe('0.1.1')
+  })
 })
 
 describe('runAdd — git/local manifest-value rule', () => {

packages/engine/src/registry/__tests__/publish-route.test.ts

@@ -0,0 +1,55 @@
+/**
+ * Route-shape tests for `publishFacetVersion`.
+ *
+ * Scoped facet names MUST publish through the registry's two-segment scoped
+ * route (`/v0/facets/{scope}/{name}/versions`) so the scope `/` stays a
+ * literal path separator. Unscoped names keep the single-`{name}` route.
+ * These tests stub `fetch` and assert on the request URL shape.
+ */
+
+import { describe, expect, test } from 'bun:test'
+import { createRegistryClient } from '../client.ts'
+import { publishResponse } from '../fixtures.ts'
+import { publishFacetVersion } from '../publish.ts'
+
+function asFetch(
+  fn: (input: Request | string | URL, init?: RequestInit) => Promise<Response>,
+): typeof globalThis.fetch {
+  return fn as unknown as typeof globalThis.fetch
+}
+
+const BASE_URL = 'https://api.test'
+
+function stubPublish(): { urls: string[]; fetch: typeof globalThis.fetch } {
+  const urls: string[] = []
+  const fetch = asFetch(async (input) => {
+    urls.push(input instanceof Request ? input.url : String(input))
+    return new Response(JSON.stringify(publishResponse()), {
+      status: 201,
+      headers: { 'content-type': 'application/json' },
+    })
+  })
+  return { urls, fetch }
+}
+
+describe('publishFacetVersion — route selection', () => {
+  test('unscoped name posts to the single-{name} versions route', async () => {
+    const { urls, fetch } = stubPublish()
+    const client = createRegistryClient({ baseUrl: BASE_URL, fetch })
+    const result = await publishFacetVersion(client, { name: 'cowsay', tarball: new Uint8Array([1, 2, 3]) })
+    expect(result.ok).toBe(true)
+    expect(urls[0]).toBe('https://api.test/v0/facets/cowsay/versions')
+  })
+
+  test('scoped name posts to the two-segment scoped versions route (no %2F)', async () => {
+    const { urls, fetch } = stubPublish()
+    const client = createRegistryClient({ baseUrl: BASE_URL, fetch })
+    const result = await publishFacetVersion(client, {
+      name: '@julian/cowsay',
+      tarball: new Uint8Array([1, 2, 3]),
+    })
+    expect(result.ok).toBe(true)
+    expect(urls[0]).toBe('https://api.test/v0/facets/%40julian/cowsay/versions')
+    expect(urls[0]).not.toContain('%2F')
+  })
+})

packages/engine/src/registry/download.ts

@@ -7,6 +7,7 @@ import { createRegistryClient, translateWireError } from './client.ts'
 import { resolveCredential } from './credentials.ts'
 import type { paths } from './generated/registry-api.ts'
 import { uncappedGunzip } from './gunzip.ts'
+import { facetNameToRoute } from './http.ts'
 import type { RegistryMetadata, RegistryResult } from './types.ts'
 
 /**
@@ -178,13 +179,22 @@ export async function resolveArchiveUrl(
   // body stream is consumed in the process), so we capture it here and
   // render it below rather than re-reading `response.json()`.
   let wireError: unknown
+  // Scoped names use the two-segment scoped archive route so the scope `/`
+  // is never collapsed into `%2F` (which the registry rejects).
+  const route = facetNameToRoute(meta.name)
   try {
-    const result = await client.GET('/v0/facets/{name}/{version}/archive', {
-      params: { path: { name: meta.name, version: meta.version } },
-      // Do not follow the redirect inside the typed client; we want to
-      // read the presigned S3 URL off the `Location` header ourselves.
-      redirect: 'manual',
-    })
+    const result =
+      route.kind === 'scoped'
+        ? await client.GET('/v0/facets/{scope}/{name}/{version}/archive', {
+            params: { path: { scope: route.scope, name: route.name, version: meta.version } },
+            redirect: 'manual',
+          })
+        : await client.GET('/v0/facets/{name}/{version}/archive', {
+            params: { path: { name: route.name, version: meta.version } },
+            // Do not follow the redirect inside the typed client; we want to
+            // read the presigned S3 URL off the `Location` header ourselves.
+            redirect: 'manual',
+          })
     response = result.response
     wireError = result.error
   } catch (err) {