Add versioned build-manifest, lockfile, and archive-plan schemas with exact facetVersion/lockfileVersion dispatch and supplementary files support

openspec/changes/support-non-asset-files/tasks.md

@@ -16,25 +16,25 @@
 
 ## 1. Protocol Models and Archive Plan — Research
 
-- [ ] 1.1 Explore: Inspect the current facet, build-manifest, lockfile, and asset-name schemas and identify every current-versus-legacy validation call site
-- [ ] 1.2 Explore: Trace archive membership, path validation, collision detection, and per-entry hashing across protocol and engine build code
-- [ ] 1.3 Explore: Inspect protocol public exports, version constants, fixtures, and schema tests that constrain compatibility
-- [ ] 1.4 Propose: Define the protocol model for exact supplementary declarations, tagged archive-plan entries, version dispatch, and structured validation failures
+- [x] 1.1 Explore: Inspect the current facet, build-manifest, lockfile, and asset-name schemas and identify every current-versus-legacy validation call site
+- [x] 1.2 Explore: Trace archive membership, path validation, collision detection, and per-entry hashing across protocol and engine build code
+- [x] 1.3 Explore: Inspect protocol public exports, version constants, fixtures, and schema tests that constrain compatibility
+- [x] 1.4 Propose: Define the protocol model for exact supplementary declarations, tagged archive-plan entries, version dispatch, and structured validation failures
 
 ## 2. Protocol Models and Archive Plan — Implementation
 
-- [ ] 2.1 Implement: Add top-level and per-skill exact `files` declarations, current single-segment asset-name validation, and the shared skill/command namespace while isolating legacy `0.1` naming behavior
-- [ ] 2.2 Implement: Add one pure archive-plan operation that classifies manifest, primary-asset, skill-companion, and archive-only entries and enforces the complete path-safety and collision grammar, including Windows-portable component rules (reserved device names, forbidden characters, control bytes, trailing dot/space)
-- [ ] 2.3 Implement: Add separate archive-format and lockfile-format constants plus exact versioned build-manifest schemas for legacy `0.1` `assets` and current `0.2` `files`, pinning numeric `facetVersion: 0.2` and the exact `archive: "archive.tar.gz"` literal, and rejecting duplicate JSON object members in facet manifests, build manifests, and lockfiles before schema validation
-- [ ] 2.4 Implement: Add the lockfile `0.2` schema with deterministic per-asset file-integrity records and exact legacy-alpha-`1` versus current-`0.2` dispatch
-- [ ] 2.5 Implement: Curate protocol exports and add focused schema, name, archive-plan, collision, version-dispatch, and lockfile tests for all legal and illegal states
-- [ ] 2.6 Verify: Run the focused protocol typecheck and test suites for schemas and archive planning
+- [x] 2.1 Implement: Add top-level and per-skill exact `files` declarations, current single-segment asset-name validation, and the shared skill/command namespace while isolating legacy `0.1` naming behavior
+- [x] 2.2 Implement: Add one pure archive-plan operation that classifies manifest, primary-asset, skill-companion, and archive-only entries and enforces the complete path-safety and collision grammar, including Windows-portable component rules (reserved device names, forbidden characters, control bytes, trailing dot/space)
+- [x] 2.3 Implement: Add separate archive-format and lockfile-format constants plus exact versioned build-manifest schemas for legacy `0.1` `assets` and current `0.2` `files`, pinning numeric `facetVersion: 0.2` and the exact `archive: "archive.tar.gz"` literal, and rejecting duplicate JSON object members in facet manifests, build manifests, and lockfiles before schema validation
+- [x] 2.4 Implement: Add the lockfile `0.2` schema with deterministic per-asset file-integrity records and exact legacy-alpha-`1` versus current-`0.2` dispatch
+- [x] 2.5 Implement: Curate protocol exports and add focused schema, name, archive-plan, collision, version-dispatch, and lockfile tests for all legal and illegal states
+- [x] 2.6 Verify: Run the focused protocol typecheck and test suites for schemas and archive planning
 
 ## 3. Archive Verification and Consumer Bridge — Research
 
-- [ ] 3.1 Explore: Trace outer/inner tar parsing and identify where duplicate, aliased, unsafe, and non-regular headers can be rejected before path-keyed maps are built
-- [ ] 3.2 Explore: Trace archive verification, cache extraction/auditing, registry download, and engine loading from verified bytes through resolved facet data
-- [ ] 3.3 Explore: Inspect integrity result types and CLI failure rendering for path-specific mismatches, decompression refusal, and unsupported versions
+- [x] 3.1 Explore: Trace outer/inner tar parsing and identify where duplicate, aliased, unsafe, and non-regular headers can be rejected before path-keyed maps are built
+- [x] 3.2 Explore: Trace archive verification, cache extraction/auditing, registry download, and engine loading from verified bytes through resolved facet data
+- [x] 3.3 Explore: Inspect integrity result types and CLI failure rendering for path-specific mismatches, decompression refusal, and unsupported versions
 - [ ] 3.4 Propose: Define the consumer-first bridge approach for strict `0.1`/`0.2` dispatch, tagged verified content, immutable fixtures, and actionable failures without enabling `0.2` production
 
 ## 4. Archive Verification and Consumer Bridge — Implementation

packages/engine/src/__tests__/build-pipeline.test.ts

@@ -358,8 +358,10 @@ describe('runBuildPipeline', () => {
     }
   })
 
-  test('build succeeds with cross-type name sharing', async () => {
-    const dir = await createFixtureDir('cross-type')
+  // Skills and commands share one logical namespace in the current manifest
+  // format (design D9); agents remain a separate namespace.
+  test('build fails when a skill and command share a name', async () => {
+    const dir = await createFixtureDir('cross-type-collision')
     await Bun.write(join(dir, 'skills/review/SKILL.md'), '# Review skill')
     await Bun.write(join(dir, 'commands/review.md'), '# Review command')
     await Bun.write(
@@ -376,6 +378,28 @@ describe('runBuildPipeline', () => {
       }),
     )
 
+    const result = await runBuildPipeline(dir)
+    expect(result.ok).toBe(false)
+  })
+
+  test('build succeeds when an agent shares a name with a skill', async () => {
+    const dir = await createFixtureDir('cross-type-agent')
+    await Bun.write(join(dir, 'skills/review/SKILL.md'), '# Review skill')
+    await Bun.write(join(dir, 'agents/review.md'), '# Review agent')
+    await Bun.write(
+      join(dir, 'facet.json'),
+      JSON.stringify({
+        name: 'test-facet',
+        version: '1.0.0',
+        skills: {
+          review: { description: 'A review skill' },
+        },
+        agents: {
+          review: { description: 'A review agent' },
+        },
+      }),
+    )
+
     const result = await runBuildPipeline(dir)
     expect(result.ok).toBe(true)
   })

packages/protocol/src/__tests__/archive-plan.test.ts

@@ -0,0 +1,168 @@
+import { describe, expect, test } from 'bun:test'
+import { type ArchivePlanError, type ArchivePlanInput, planArchiveEntries } from '@agent-facets/protocol'
+
+function planErrors(manifest: ArchivePlanInput): ArchivePlanError[] {
+  const result = planArchiveEntries(manifest)
+  if (result.ok) expect.unreachable()
+  return result.errors
+}
+
+function expectCode(manifest: ArchivePlanInput, code: ArchivePlanError['code']) {
+  const errors = planErrors(manifest)
+  expect(errors.map((e) => e.code)).toContain(code)
+}
+
+describe('planArchiveEntries — legal plans', () => {
+  test('asset-only manifest plans manifest + conventional primary paths, sorted', () => {
+    const result = planArchiveEntries({
+      skills: { review: {} },
+      agents: { reviewer: {} },
+      commands: { ship: {} },
+    })
+    if (!result.ok) expect.unreachable()
+    expect(result.data).toEqual([
+      { kind: 'primary-asset', path: 'agents/reviewer.md', assetType: 'agent', name: 'reviewer' },
+      { kind: 'primary-asset', path: 'commands/ship.md', assetType: 'command', name: 'ship' },
+      { kind: 'manifest', path: 'facet.json' },
+      { kind: 'primary-asset', path: 'skills/review/SKILL.md', assetType: 'skill', name: 'review' },
+    ])
+  })
+
+  test('skill companions and top-level files are classified and resolved', () => {
+    const result = planArchiveEntries({
+      skills: { review: { files: ['references/api.md', 'scripts/run.ts'] } },
+      files: ['README.md', 'docs/notes.md'],
+    })
+    if (!result.ok) expect.unreachable()
+    expect(result.data).toEqual([
+      { kind: 'archive-only', path: 'README.md' },
+      { kind: 'archive-only', path: 'docs/notes.md' },
+      { kind: 'manifest', path: 'facet.json' },
+      { kind: 'primary-asset', path: 'skills/review/SKILL.md', assetType: 'skill', name: 'review' },
+      { kind: 'skill-companion', path: 'skills/review/references/api.md', skill: 'review' },
+      { kind: 'skill-companion', path: 'skills/review/scripts/run.ts', skill: 'review' },
+    ])
+  })
+
+  test('basename facet.json is permitted below another directory', () => {
+    const result = planArchiveEntries({
+      skills: { example: { files: ['examples/facet.json'] } },
+      files: ['fixtures/facet.json'],
+    })
+    expect(result.ok).toBe(true)
+  })
+
+  test('names owned by the outer archive may appear as inner supplementary paths', () => {
+    const result = planArchiveEntries({
+      agents: { a: {} },
+      files: ['build-manifest.json', 'archive.tar.gz'],
+    })
+    expect(result.ok).toBe(true)
+  })
+
+  test('empty files arrays are legal', () => {
+    const result = planArchiveEntries({ skills: { review: { files: [] } }, files: [] })
+    expect(result.ok).toBe(true)
+  })
+})
+
+describe('planArchiveEntries — per-path grammar failures', () => {
+  const base: ArchivePlanInput = { agents: { a: {} } }
+
+  test.each([
+    ['../secret', 'path-traversal'],
+    ['docs/../secret', 'path-traversal'],
+    ['/absolute', 'path-absolute'],
+    ['C:/secret', 'path-absolute'],
+    ['file://etc/passwd', 'path-absolute'],
+    ['docs\\guide.md', 'path-backslash'],
+    ['docs/\u0000name', 'path-control-byte'],
+    ['docs/\u0007bell', 'path-control-byte'],
+    ['docs//guide.md', 'path-empty-segment'],
+    ['./docs/guide.md', 'path-empty-segment'],
+    ['docs/./guide.md', 'path-empty-segment'],
+    ['docs/', 'path-empty-segment'],
+    ['notes:draft.md', 'path-forbidden-character'],
+    ['what?.md', 'path-forbidden-character'],
+    ['a<b.md', 'path-forbidden-character'],
+    ['a|b.md', 'path-forbidden-character'],
+    ['references/con', 'path-reserved-device-name'],
+    ['aux.txt', 'path-reserved-device-name'],
+    ['docs/COM1.md', 'path-reserved-device-name'],
+    ['docs/LpT9', 'path-reserved-device-name'],
+    ['report.', 'path-trailing-dot-or-space'],
+    ['draft ', 'path-trailing-dot-or-space'],
+    ['dir. /file.md', 'path-trailing-dot-or-space'],
+  ] as const)('top-level %j fails with %s', (path, code) => {
+    expectCode({ ...base, files: [path] }, code)
+  })
+
+  test('empty declared path fails', () => {
+    expectCode({ ...base, files: [''] }, 'path-empty')
+  })
+
+  test('per-skill companion paths run the same grammar', () => {
+    expectCode({ skills: { review: { files: ['../escape.md'] } } }, 'path-traversal')
+    expectCode({ skills: { review: { files: ['refs\\a.md'] } } }, 'path-backslash')
+    expectCode({ skills: { review: { files: ['refs/aux'] } } }, 'path-reserved-device-name')
+  })
+
+  test('errors carry the declaration site', () => {
+    const errors = planErrors({ skills: { review: { files: ['../up'] } } })
+    expect(errors[0]?.path).toBe('skills.review.files')
+    const topErrors = planErrors({ agents: { a: {} }, files: ['../up'] })
+    expect(topErrors[0]?.path).toBe('files')
+  })
+})
+
+describe('planArchiveEntries — declaration-site rules', () => {
+  test('top-level path under skills/ is redirected to the owning skill', () => {
+    const errors = planErrors({
+      skills: { review: {} },
+      files: ['skills/review/references/api.md'],
+    })
+    expect(errors.map((e) => e.code)).toContain('site-top-level-under-skills')
+  })
+
+  test('skill companion SKILL.md is rejected', () => {
+    expectCode({ skills: { review: { files: ['SKILL.md'] } } }, 'site-skill-companion-is-primary')
+  })
+
+  test('root facet.json declaration is rejected', () => {
+    expectCode({ agents: { a: {} }, files: ['facet.json'] }, 'reserved-root-manifest')
+  })
+})
+
+describe('planArchiveEntries — collision failures', () => {
+  test('exact duplicate supplementary paths', () => {
+    expectCode({ agents: { a: {} }, files: ['docs/guide.md', 'docs/guide.md'] }, 'collision-duplicate')
+  })
+
+  test('portable case-fold alias', () => {
+    expectCode({ agents: { a: {} }, files: ['Docs/guide.md', 'docs/guide.md'] }, 'collision-case-fold')
+  })
+
+  test('Unicode normalization alias (NFC vs NFD)', () => {
+    expectCode({ agents: { a: {} }, files: ['docs/caf\u00e9.md', 'docs/cafe\u0301.md'] }, 'collision-unicode-alias')
+  })
+
+  test('file/directory prefix conflict', () => {
+    expectCode({ agents: { a: {} }, files: ['docs', 'docs/guide.md'] }, 'collision-prefix')
+  })
+
+  test('supplementary path colliding with a conventional primary path', () => {
+    expectCode({ agents: { reviewer: {} }, files: ['agents/reviewer.md'] }, 'collision-primary-path')
+  })
+
+  test('case-fold collision with a primary path is a primary-path collision', () => {
+    expectCode({ agents: { reviewer: {} }, files: ['agents/Reviewer.md'] }, 'collision-primary-path')
+  })
+
+  test('skill companion colliding with its own primary via case fold', () => {
+    expectCode({ skills: { review: { files: ['skill.md'] } } }, 'collision-primary-path')
+  })
+
+  test('companion duplicated across two declaration entries', () => {
+    expectCode({ skills: { review: { files: ['refs/a.md', 'refs/a.md'] } } }, 'collision-duplicate')
+  })
+})

packages/protocol/src/__tests__/build-manifest-versions.test.ts

@@ -0,0 +1,145 @@
+import { describe, expect, test } from 'bun:test'
+import {
+  CurrentBuildManifestSchema,
+  FACET_ARCHIVE_VERSION,
+  LEGACY_FACET_ARCHIVE_VERSION,
+  LegacyBuildManifestSchema,
+  parseBuildManifestDocument,
+  SUPPORTED_FACET_VERSIONS,
+} from '@agent-facets/protocol'
+import { type } from 'arktype'
+
+const HASH = `sha256:${'a'.repeat(64)}`
+
+const legacyManifest = {
+  facetVersion: 0.1,
+  archive: 'archive.tar.gz',
+  integrity: HASH,
+  assets: { 'skills/review/SKILL.md': HASH },
+}
+
+const currentManifest = {
+  facetVersion: 0.2,
+  archive: 'archive.tar.gz',
+  integrity: HASH,
+  files: { 'facet.json': HASH, 'skills/review/SKILL.md': HASH, 'README.md': HASH },
+}
+
+describe('version constants', () => {
+  test('archive format constants are pinned', () => {
+    expect(LEGACY_FACET_ARCHIVE_VERSION).toBe(0.1)
+    expect(FACET_ARCHIVE_VERSION).toBe(0.2)
+    expect(SUPPORTED_FACET_VERSIONS).toEqual([0.1, 0.2])
+  })
+})
+
+describe('LegacyBuildManifestSchema', () => {
+  test('accepts the legacy 0.1 shape', () => {
+    expect(LegacyBuildManifestSchema(legacyManifest)).not.toBeInstanceOf(type.errors)
+  })
+
+  test('rejects a non-0.1 facetVersion', () => {
+    expect(LegacyBuildManifestSchema({ ...legacyManifest, facetVersion: 0.2 })).toBeInstanceOf(type.errors)
+  })
+
+  test('rejects a current-format files map', () => {
+    expect(LegacyBuildManifestSchema({ ...legacyManifest, files: { 'facet.json': HASH } })).toBeInstanceOf(type.errors)
+  })
+})
+
+describe('CurrentBuildManifestSchema', () => {
+  test('accepts the current 0.2 shape', () => {
+    expect(CurrentBuildManifestSchema(currentManifest)).not.toBeInstanceOf(type.errors)
+  })
+
+  test('rejects a non-0.2 facetVersion', () => {
+    expect(CurrentBuildManifestSchema({ ...currentManifest, facetVersion: 0.1 })).toBeInstanceOf(type.errors)
+  })
+
+  test('rejects a non-canonical archive entry name', () => {
+    expect(CurrentBuildManifestSchema({ ...currentManifest, archive: 'payload.tar.gz' })).toBeInstanceOf(type.errors)
+  })
+
+  test('rejects a legacy assets map', () => {
+    expect(CurrentBuildManifestSchema({ ...currentManifest, assets: { 'facet.json': HASH } })).toBeInstanceOf(
+      type.errors,
+    )
+  })
+
+  test('rejects malformed file-hash values', () => {
+    expect(CurrentBuildManifestSchema({ ...currentManifest, files: { 'facet.json': 'md5:abc' } })).toBeInstanceOf(
+      type.errors,
+    )
+  })
+
+  test('rejects a missing files map', () => {
+    const { files: _files, ...withoutFiles } = currentManifest
+    expect(CurrentBuildManifestSchema(withoutFiles)).toBeInstanceOf(type.errors)
+  })
+})
+
+describe('parseBuildManifestDocument — exact version dispatch', () => {
+  test('parses a legacy 0.1 document', () => {
+    const result = parseBuildManifestDocument(JSON.stringify(legacyManifest))
+    if (!result.ok) expect.unreachable()
+    expect(result.data.facetVersion).toBe(0.1)
+    if (result.data.facetVersion !== 0.1) expect.unreachable()
+    expect(result.data.manifest.assets['skills/review/SKILL.md']).toBe(HASH)
+  })
+
+  test('parses a current 0.2 document', () => {
+    const result = parseBuildManifestDocument(JSON.stringify(currentManifest))
+    if (!result.ok) expect.unreachable()
+    expect(result.data.facetVersion).toBe(0.2)
+    if (result.data.facetVersion !== 0.2) expect.unreachable()
+    expect(result.data.manifest.files['README.md']).toBe(HASH)
+  })
+
+  test('accepts bytes input', () => {
+    const result = parseBuildManifestDocument(new TextEncoder().encode(JSON.stringify(currentManifest)))
+    expect(result.ok).toBe(true)
+  })
+
+  test('unsupported version is a structured failure with observed and supported versions', () => {
+    const result = parseBuildManifestDocument(JSON.stringify({ ...currentManifest, facetVersion: 0.3 }))
+    if (result.ok) expect.unreachable()
+    if (result.failure.code !== 'unsupported-facet-version') expect.unreachable()
+    expect(result.failure.observed).toBe(0.3)
+    expect(result.failure.supported).toEqual([0.1, 0.2])
+  })
+
+  test('missing facetVersion is unsupported with observed undefined', () => {
+    const result = parseBuildManifestDocument(JSON.stringify({ archive: 'archive.tar.gz', integrity: HASH }))
+    if (result.ok) expect.unreachable()
+    if (result.failure.code !== 'unsupported-facet-version') expect.unreachable()
+    expect(result.failure.observed).toBeUndefined()
+  })
+
+  test('malformed 0.2 document fails as 0.2 — never reinterpreted as 0.1', () => {
+    // Valid legacy shape except it claims 0.2: must fail the 0.2 schema.
+    const result = parseBuildManifestDocument(JSON.stringify({ ...legacyManifest, facetVersion: 0.2 }))
+    if (result.ok) expect.unreachable()
+    if (result.failure.code !== 'schema-violation') expect.unreachable()
+    expect(result.failure.facetVersion).toBe(0.2)
+  })
+
+  test('malformed 0.1 document fails as 0.1 — never reinterpreted as 0.2', () => {
+    const result = parseBuildManifestDocument(JSON.stringify({ ...currentManifest, facetVersion: 0.1 }))
+    if (result.ok) expect.unreachable()
+    if (result.failure.code !== 'schema-violation') expect.unreachable()
+    expect(result.failure.facetVersion).toBe(0.1)
+  })
+
+  test('duplicate JSON members are rejected before schema validation', () => {
+    const text = `{"facetVersion":0.2,"archive":"archive.tar.gz","integrity":"${HASH}","files":{"facet.json":"${HASH}"},"files":{"evil.txt":"${HASH}"}}`
+    const result = parseBuildManifestDocument(text)
+    if (result.ok) expect.unreachable()
+    expect(result.failure.code).toBe('duplicate-members')
+  })
+
+  test('invalid JSON is a structured failure', () => {
+    const result = parseBuildManifestDocument('{not json')
+    if (result.ok) expect.unreachable()
+    expect(result.failure.code).toBe('invalid-json')
+  })
+})