From 90a0704c8c4a4783eddad16426e32933c7a52bb3 Mon Sep 17 00:00:00 2001 From: Julian Coy Date: Mon, 20 Jul 2026 14:28:24 -0400 Subject: [PATCH] Add versioned build-manifest, lockfile, and archive-plan schemas with exact `facetVersion`/`lockfileVersion` dispatch and supplementary `files` support --- .../changes/support-non-asset-files/tasks.md | 26 +- .../src/__tests__/build-pipeline.test.ts | 28 +- .../src/__tests__/archive-plan.test.ts | 168 ++++++++ .../__tests__/build-manifest-versions.test.ts | 145 +++++++ .../__tests__/duplicate-json-members.test.ts | 55 +++ .../src/__tests__/facet-manifest.test.ts | 141 +++++- .../src/__tests__/lockfile-versions.test.ts | 190 ++++++++ packages/protocol/src/build/archive-plan.ts | 404 ++++++++++++++++++ packages/protocol/src/build/content-hash.ts | 17 +- packages/protocol/src/index.ts | 71 ++- .../src/integrity/validate-archive.ts | 9 +- .../protocol/src/loaders/build-manifest.ts | 107 +++++ packages/protocol/src/loaders/facet.ts | 41 +- packages/protocol/src/loaders/lockfile.ts | 111 +++++ packages/protocol/src/loaders/validate.ts | 101 +++++ .../protocol/src/schemas/build-manifest.ts | 91 +++- .../src/schemas/facet-manifest-legacy.ts | 113 +++++ .../protocol/src/schemas/facet-manifest.ts | 74 +++- packages/protocol/src/schemas/lockfile.ts | 136 +++++- 19 files changed, 1976 insertions(+), 52 deletions(-) create mode 100644 packages/protocol/src/__tests__/archive-plan.test.ts create mode 100644 packages/protocol/src/__tests__/build-manifest-versions.test.ts create mode 100644 packages/protocol/src/__tests__/duplicate-json-members.test.ts create mode 100644 packages/protocol/src/__tests__/lockfile-versions.test.ts create mode 100644 packages/protocol/src/build/archive-plan.ts create mode 100644 packages/protocol/src/loaders/build-manifest.ts create mode 100644 packages/protocol/src/loaders/lockfile.ts create mode 100644 packages/protocol/src/schemas/facet-manifest-legacy.ts diff --git a/openspec/changes/support-non-asset-files/tasks.md b/openspec/changes/support-non-asset-files/tasks.md index e1507ddb..4ea4c7c3 100644 --- a/openspec/changes/support-non-asset-files/tasks.md +++ b/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 diff --git a/packages/engine/src/__tests__/build-pipeline.test.ts b/packages/engine/src/__tests__/build-pipeline.test.ts index be8c2278..ea62d6f0 100644 --- a/packages/engine/src/__tests__/build-pipeline.test.ts +++ b/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) }) diff --git a/packages/protocol/src/__tests__/archive-plan.test.ts b/packages/protocol/src/__tests__/archive-plan.test.ts new file mode 100644 index 00000000..f4a3e3d5 --- /dev/null +++ b/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 { + 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') + }) +}) diff --git a/packages/protocol/src/__tests__/build-manifest-versions.test.ts b/packages/protocol/src/__tests__/build-manifest-versions.test.ts new file mode 100644 index 00000000..0879505a --- /dev/null +++ b/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') + }) +}) diff --git a/packages/protocol/src/__tests__/duplicate-json-members.test.ts b/packages/protocol/src/__tests__/duplicate-json-members.test.ts new file mode 100644 index 00000000..ceb16986 --- /dev/null +++ b/packages/protocol/src/__tests__/duplicate-json-members.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, test } from 'bun:test' +import { findDuplicateJsonMembers, validateFacetManifest, validateLegacyFacetManifest } from '@agent-facets/protocol' + +describe('findDuplicateJsonMembers', () => { + test('clean document has no duplicates', () => { + expect(findDuplicateJsonMembers('{"a":1,"b":{"c":2},"d":[{"e":3}]}')).toEqual([]) + }) + + test('top-level duplicate is detected', () => { + const errors = findDuplicateJsonMembers('{"files":{},"files":{}}') + expect(errors).toHaveLength(1) + expect(errors[0]?.message).toContain('"files"') + }) + + test('nested duplicate reports the enclosing path', () => { + const errors = findDuplicateJsonMembers('{"facets":{"cowsay":{"version":"1.0.0","version":"2.0.0"}}}') + expect(errors).toHaveLength(1) + expect(errors[0]?.path).toBe('facets.cowsay') + }) + + test('duplicate inside an array element object is detected', () => { + const errors = findDuplicateJsonMembers('{"assets":[{"name":"a","name":"b"}]}') + expect(errors).toHaveLength(1) + }) + + test('escaped keys are decoded before comparison', () => { + // "\u0066iles" decodes to "files". + const errors = findDuplicateJsonMembers('{"files":{},"\\u0066iles":{}}') + expect(errors).toHaveLength(1) + }) + + test('same key in sibling objects is not a duplicate', () => { + expect(findDuplicateJsonMembers('{"a":{"x":1},"b":{"x":2}}')).toEqual([]) + }) + + test('string values containing braces and quotes do not confuse the scanner', () => { + expect(findDuplicateJsonMembers('{"a":"{\\"a\\":1,\\"a\\":2}","b":"}{"}')).toEqual([]) + }) +}) + +describe('facet-manifest validators reject duplicate members', () => { + test('current validator rejects duplicate top-level members', () => { + const text = '{"name":"ok","version":"1.0.0","skills":{"a":{"description":"x"}},"skills":{"b":{"description":"y"}}}' + const result = validateFacetManifest(text) + if (result.ok) expect.unreachable() + expect(result.errors[0]?.message).toContain('Duplicate JSON object member') + }) + + test('legacy validator rejects duplicate members too', () => { + const text = '{"name":"ok","version":"1.0.0","agents":{"a":{"description":"x"}},"agents":{"a":{"description":"x"}}}' + const result = validateLegacyFacetManifest(text) + if (result.ok) expect.unreachable() + expect(result.errors[0]?.message).toContain('Duplicate JSON object member') + }) +}) diff --git a/packages/protocol/src/__tests__/facet-manifest.test.ts b/packages/protocol/src/__tests__/facet-manifest.test.ts index b4891288..dd06d6ea 100644 --- a/packages/protocol/src/__tests__/facet-manifest.test.ts +++ b/packages/protocol/src/__tests__/facet-manifest.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'bun:test' -import { type FacetManifest, FacetManifestSchema } from '@agent-facets/protocol' +import { type FacetManifest, FacetManifestSchema, LegacyFacetManifestSchema } from '@agent-facets/protocol' import { type } from 'arktype' // --- Valid manifests --- @@ -245,16 +245,30 @@ describe('FacetManifestSchema — invalid manifests', () => { expect(FacetManifestSchema(input)).not.toBeInstanceOf(type.errors) }) - test('deep-nested namespaced asset names (no traversal) stay valid', () => { + // Current-format names are single-segment only (design D9): slash-namespaced + // names are a legacy-0.1 concept, isolated to LegacyFacetManifestSchema. + test('slash-namespaced asset names are rejected in the current schema', () => { const input = { name: 'ok', version: '1.0.0', skills: { 'viper-plans/planning': { description: 'plan things' }, - 'viper-plans/review/deep': { description: 'deeper' }, }, } const result = FacetManifestSchema(input) + expect(result).toBeInstanceOf(type.errors) + }) + + test('slash-namespaced asset names remain valid under the legacy schema', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { + 'viper-plans/planning': { description: 'plan things' }, + 'viper-plans/review/deep': { description: 'deeper' }, + }, + } + const result = LegacyFacetManifestSchema(input) expect(result).not.toBeInstanceOf(type.errors) }) @@ -395,3 +409,124 @@ describe('FacetManifestSchema — unknown field tolerance', () => { expect(agents?.reviewer?.model).toBe('claude-sonnet') }) }) + +// --- Current-format additions: shared namespace + supplementary files --- + +describe('FacetManifestSchema — shared skill/command namespace', () => { + test('skill and command with the same name are rejected, identifying both', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { review: { description: 'x' } }, + commands: { review: { description: 'y' } }, + } + const result = FacetManifestSchema(input) + expect(result).toBeInstanceOf(type.errors) + const errors = result as InstanceType + expect(errors.some((e) => e.message.includes('skills.review') && e.message.includes('commands.review'))).toBe(true) + }) + + test('agent may share a name with a skill', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { review: { description: 'x' } }, + agents: { review: { description: 'y' } }, + } + expect(FacetManifestSchema(input)).not.toBeInstanceOf(type.errors) + }) + + test('agent may share a name with a command', () => { + const input = { + name: 'ok', + version: '1.0.0', + commands: { review: { description: 'x' } }, + agents: { review: { description: 'y' } }, + } + expect(FacetManifestSchema(input)).not.toBeInstanceOf(type.errors) + }) + + test('legacy schema permits skill/command name sharing', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { review: { description: 'x' } }, + commands: { review: { description: 'y' } }, + } + expect(LegacyFacetManifestSchema(input)).not.toBeInstanceOf(type.errors) + }) +}) + +describe('FacetManifestSchema — supplementary file declarations', () => { + test('top-level and per-skill files declarations are accepted', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { review: { description: 'x', files: ['references/api.md', 'scripts/run.ts'] } }, + files: ['README.md', 'LICENSE', 'docs/notes.md'], + } + expect(FacetManifestSchema(input)).not.toBeInstanceOf(type.errors) + }) + + test.each([ + ['../secret'], + ['/absolute'], + ['C:/secret'], + ['docs//guide.md'], + ['docs\\guide.md'], + ['aux.txt'], + ['report.'], + ])('unsafe top-level path %j is rejected', (path) => { + const input = { name: 'ok', version: '1.0.0', agents: { a: { description: 'x' } }, files: [path] } + expect(FacetManifestSchema(input)).toBeInstanceOf(type.errors) + }) + + test('top-level path under skills/ is rejected toward the owning skill', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { review: { description: 'x' } }, + files: ['skills/review/references/api.md'], + } + const result = FacetManifestSchema(input) + expect(result).toBeInstanceOf(type.errors) + const errors = result as InstanceType + expect(errors.some((e) => e.message.includes("owning skill's files"))).toBe(true) + }) + + test('skill companion SKILL.md is rejected', () => { + const input = { + name: 'ok', + version: '1.0.0', + skills: { review: { description: 'x', files: ['SKILL.md'] } }, + } + expect(FacetManifestSchema(input)).toBeInstanceOf(type.errors) + }) + + test('root facet.json declaration is rejected; nested basename is fine', () => { + const bad = { name: 'ok', version: '1.0.0', agents: { a: { description: 'x' } }, files: ['facet.json'] } + expect(FacetManifestSchema(bad)).toBeInstanceOf(type.errors) + const good = { name: 'ok', version: '1.0.0', agents: { a: { description: 'x' } }, files: ['fixtures/facet.json'] } + expect(FacetManifestSchema(good)).not.toBeInstanceOf(type.errors) + }) + + test('portable collision between declared paths is rejected', () => { + const input = { + name: 'ok', + version: '1.0.0', + agents: { a: { description: 'x' } }, + files: ['Docs/guide.md', 'docs/guide.md'], + } + expect(FacetManifestSchema(input)).toBeInstanceOf(type.errors) + }) + + test('declared path colliding with a primary asset path is rejected', () => { + const input = { + name: 'ok', + version: '1.0.0', + agents: { reviewer: { description: 'x' } }, + files: ['agents/reviewer.md'], + } + expect(FacetManifestSchema(input)).toBeInstanceOf(type.errors) + }) +}) diff --git a/packages/protocol/src/__tests__/lockfile-versions.test.ts b/packages/protocol/src/__tests__/lockfile-versions.test.ts new file mode 100644 index 00000000..84600bee --- /dev/null +++ b/packages/protocol/src/__tests__/lockfile-versions.test.ts @@ -0,0 +1,190 @@ +import { describe, expect, test } from 'bun:test' +import { + CURRENT_LOCKFILE_VERSION, + CurrentLockfileSchema, + LEGACY_LOCKFILE_VERSION, + LegacyLockfileSchema, + parseLockfileDocument, + SUPPORTED_LOCKFILE_VERSIONS, +} from '@agent-facets/protocol' +import { type } from 'arktype' + +const HASH = `sha256:${'b'.repeat(64)}` + +const legacyLockfile = { + lockfileVersion: 1, + facets: { + cowsay: { + source: { kind: 'local', path: '../cowsay' }, + version: '1.0.0', + integrity: HASH, + assets: [{ scope: 'project', type: 'skill', name: 'cowsay' }], + }, + }, +} + +const currentSkillAsset = { + scope: 'project', + type: 'skill', + name: 'review', + files: [ + { path: 'skills/review/SKILL.md', integrity: HASH }, + { path: 'skills/review/references/api.md', integrity: HASH }, + { path: 'skills/review/scripts/run.ts', integrity: HASH }, + ], +} + +const currentLockfile = { + lockfileVersion: 0.2, + facets: { + cowsay: { + source: { kind: 'registry', registry: 'https://cafe.example' }, + version: '1.0.0', + integrity: HASH, + assets: [ + currentSkillAsset, + { scope: 'project', type: 'agent', name: 'reviewer', files: [{ path: 'agents/reviewer.md', integrity: HASH }] }, + ], + }, + }, +} + +describe('lockfile version constants', () => { + test('constants are pinned and dispatch is exact, not ordered', () => { + expect(LEGACY_LOCKFILE_VERSION).toBe(1) + expect(CURRENT_LOCKFILE_VERSION).toBe(0.2) + expect(SUPPORTED_LOCKFILE_VERSIONS).toEqual([1, 0.2]) + // 0.2 < 1 numerically — exact-equality dispatch must not treat the + // current version as "older" than legacy alpha. + expect(CURRENT_LOCKFILE_VERSION < LEGACY_LOCKFILE_VERSION).toBe(true) + }) +}) + +describe('LegacyLockfileSchema', () => { + test('accepts the legacy alpha shape pinned to version 1', () => { + expect(LegacyLockfileSchema(legacyLockfile)).not.toBeInstanceOf(type.errors) + }) + + test('rejects any other version number', () => { + expect(LegacyLockfileSchema({ ...legacyLockfile, lockfileVersion: 0.2 })).toBeInstanceOf(type.errors) + expect(LegacyLockfileSchema({ ...legacyLockfile, lockfileVersion: 2 })).toBeInstanceOf(type.errors) + }) +}) + +describe('CurrentLockfileSchema', () => { + test('accepts a 0.2 lockfile with sorted per-file integrity records', () => { + expect(CurrentLockfileSchema(currentLockfile)).not.toBeInstanceOf(type.errors) + }) + + function withAssets(assets: unknown[]): unknown { + return { + lockfileVersion: 0.2, + facets: { + cowsay: { + source: { kind: 'local', path: '../cowsay' }, + version: '1.0.0', + integrity: HASH, + assets, + }, + }, + } + } + + test('rejects a missing files array', () => { + expect(CurrentLockfileSchema(withAssets([{ scope: 'project', type: 'agent', name: 'a' }]))).toBeInstanceOf( + type.errors, + ) + }) + + test('rejects an empty files array', () => { + expect( + CurrentLockfileSchema(withAssets([{ scope: 'project', type: 'agent', name: 'a', files: [] }])), + ).toBeInstanceOf(type.errors) + }) + + test('rejects unsorted file records', () => { + const unsorted = { + ...currentSkillAsset, + files: [ + { path: 'skills/review/scripts/run.ts', integrity: HASH }, + { path: 'skills/review/SKILL.md', integrity: HASH }, + ], + } + expect(CurrentLockfileSchema(withAssets([unsorted]))).toBeInstanceOf(type.errors) + }) + + test('rejects duplicate file paths', () => { + const duplicated = { + ...currentSkillAsset, + files: [ + { path: 'skills/review/SKILL.md', integrity: HASH }, + { path: 'skills/review/SKILL.md', integrity: HASH }, + ], + } + expect(CurrentLockfileSchema(withAssets([duplicated]))).toBeInstanceOf(type.errors) + }) + + test('rejects traversal in file paths', () => { + const evil = { ...currentSkillAsset, files: [{ path: '../escape.md', integrity: HASH }] } + expect(CurrentLockfileSchema(withAssets([evil]))).toBeInstanceOf(type.errors) + }) + + test('rejects malformed integrity values', () => { + const bad = { ...currentSkillAsset, files: [{ path: 'skills/review/SKILL.md', integrity: 'md5:nope' }] } + expect(CurrentLockfileSchema(withAssets([bad]))).toBeInstanceOf(type.errors) + }) +}) + +describe('parseLockfileDocument — exact version dispatch', () => { + test('parses a legacy alpha 1 document', () => { + const result = parseLockfileDocument(JSON.stringify(legacyLockfile)) + if (!result.ok) expect.unreachable() + expect(result.data.lockfileVersion).toBe(1) + }) + + test('parses a current 0.2 document', () => { + const result = parseLockfileDocument(JSON.stringify(currentLockfile)) + if (!result.ok) expect.unreachable() + expect(result.data.lockfileVersion).toBe(0.2) + if (result.data.lockfileVersion !== 0.2) expect.unreachable() + const asset = result.data.lockfile.facets.cowsay?.assets[0] + expect(asset?.files[0]?.path).toBe('skills/review/SKILL.md') + }) + + test('legacy shape claiming 0.2 fails as 0.2 — no shape-sniffing', () => { + const result = parseLockfileDocument(JSON.stringify({ ...legacyLockfile, lockfileVersion: 0.2 })) + if (result.ok) expect.unreachable() + if (result.failure.code !== 'schema-violation') expect.unreachable() + expect(result.failure.lockfileVersion).toBe(0.2) + }) + + test('current shape claiming 1 is interpreted only under legacy rules', () => { + // Legacy tolerates unknown fields (spec: unrecognized keys are allowed), + // so per-file records ride along as unrecognized extension data — the + // document is a legacy lockfile, never a current one. + const result = parseLockfileDocument(JSON.stringify({ ...currentLockfile, lockfileVersion: 1 })) + if (!result.ok) expect.unreachable() + expect(result.data.lockfileVersion).toBe(1) + }) + + test('unsupported version is a structured failure', () => { + const result = parseLockfileDocument(JSON.stringify({ ...legacyLockfile, lockfileVersion: 3 })) + if (result.ok) expect.unreachable() + if (result.failure.code !== 'unsupported-lockfile-version') expect.unreachable() + expect(result.failure.observed).toBe(3) + expect(result.failure.supported).toEqual([1, 0.2]) + }) + + test('duplicate JSON members are rejected before schema validation', () => { + const text = '{"lockfileVersion":1,"lockfileVersion":1,"facets":{}}' + const result = parseLockfileDocument(text) + if (result.ok) expect.unreachable() + expect(result.failure.code).toBe('duplicate-members') + }) + + test('invalid JSON is a structured failure', () => { + const result = parseLockfileDocument('not json') + if (result.ok) expect.unreachable() + expect(result.failure.code).toBe('invalid-json') + }) +}) diff --git a/packages/protocol/src/build/archive-plan.ts b/packages/protocol/src/build/archive-plan.ts new file mode 100644 index 00000000..ea150ddc --- /dev/null +++ b/packages/protocol/src/build/archive-plan.ts @@ -0,0 +1,404 @@ +import type { AssetType, ValidationError } from '@agent-facets/common' + +/** + * Archive plan — the single shared derivation of archive membership and + * entry classification from a facet manifest (design D3). + * + * The embedded `facet.json` is the sole source of truth for which paths an + * archive contains and what each path *is*. Build collection, per-entry + * hashing, archive verification, parsed-archive results, and installation + * all consume this one operation so their membership sets cannot drift + * apart. No stage maintains its own allowlist. + * + * The operation is pure: it takes manifest-shaped data and returns either a + * deterministic (lexicographically sorted) tagged entry list or structured + * validation failures. It never touches disk — build-time source checks + * (regular files only, resolved link identity) are an engine concern layered + * on top. + */ + +/** Distinct failure classes for archive-plan validation (design D7). */ +export type ArchivePlanErrorCode = + // Per-path grammar + | 'path-empty' + | 'path-traversal' + | 'path-absolute' + | 'path-backslash' + | 'path-control-byte' + | 'path-empty-segment' + | 'path-forbidden-character' + | 'path-reserved-device-name' + | 'path-trailing-dot-or-space' + // Declaration-site rules + | 'site-top-level-under-skills' + | 'site-skill-companion-is-primary' + | 'reserved-root-manifest' + // Whole-set collision rules + | 'collision-duplicate' + | 'collision-unicode-alias' + | 'collision-case-fold' + | 'collision-prefix' + | 'collision-primary-path' + +/** + * A structured archive-plan failure. Extends the project-wide + * `ValidationError` with a machine-readable failure-class discriminator so + * every D7 failure class is distinguishable without parsing messages. + */ +export interface ArchivePlanError extends ValidationError { + code: ArchivePlanErrorCode +} + +/** + * One planned inner-archive entry, tagged with its classification. Every + * entry is exactly one of these — classification via optional fields is + * prohibited (design D6). + */ +export type ArchivePlanEntry = + | { kind: 'manifest'; path: 'facet.json' } + | { kind: 'primary-asset'; path: string; assetType: AssetType; name: string } + | { kind: 'skill-companion'; path: string; skill: string } + | { kind: 'archive-only'; path: string } + +export type ArchivePlanResult = { ok: true; data: ArchivePlanEntry[] } | { ok: false; errors: ArchivePlanError[] } + +/** + * The minimal manifest shape the plan derivation needs. Structural (rather + * than importing `FacetManifest`) so the facet-manifest schema's narrow can + * call into this module without an import cycle, and so any + * manifest-version's validated data can be planned. + */ +export interface ArchivePlanInput { + skills?: Record | undefined + agents?: Record | undefined + commands?: Record | undefined + files?: string[] | undefined +} + +/** The reserved root path of the embedded manifest. */ +const MANIFEST_PATH = 'facet.json' + +/** Characters forbidden in any path segment for filesystem portability. */ +const FORBIDDEN_CHARS_RE = /[<>:"|?*]/ + +/** Control bytes (0x00–0x1F) are never valid in portable paths. NUL included. */ +// biome-ignore lint/suspicious/noControlCharactersInRegex: rejecting control bytes is the point +const CONTROL_BYTES_RE = /[\u0000-\u001f]/ + +/** Windows drive prefix (`C:`) or URL-like prefix (`scheme://`). */ +const DRIVE_PREFIX_RE = /^[A-Za-z]:/ +const URL_PREFIX_RE = /^[A-Za-z][A-Za-z0-9+.-]*:\/\// + +/** Windows-reserved device names, matched case-insensitively per segment. */ +const RESERVED_DEVICE_NAMES = new Set([ + 'con', + 'prn', + 'aux', + 'nul', + 'com1', + 'com2', + 'com3', + 'com4', + 'com5', + 'com6', + 'com7', + 'com8', + 'com9', + 'lpt1', + 'lpt2', + 'lpt3', + 'lpt4', + 'lpt5', + 'lpt6', + 'lpt7', + 'lpt8', + 'lpt9', +]) + +function planError( + code: ArchivePlanErrorCode, + declarationSite: string, + path: string, + message: string, + expected: string, +): ArchivePlanError { + return { + code, + path: declarationSite, + message, + expected, + actual: path === '' ? 'empty path' : `"${path}"`, + } +} + +/** + * Validate one declared supplementary path against the portable path grammar + * (design D7). Returns every failure class the path violates, attributed to + * `declarationSite` (e.g. `files` or `skills.review.files`). + */ +export function validateSupplementaryPath(declared: string, declarationSite: string): ArchivePlanError[] { + const errors: ArchivePlanError[] = [] + const fail = (code: ArchivePlanErrorCode, message: string, expected: string) => { + errors.push(planError(code, declarationSite, declared, message, expected)) + } + + if (declared === '') { + fail('path-empty', 'Declared path must not be empty.', 'a non-empty relative path') + return errors + } + if (CONTROL_BYTES_RE.test(declared)) { + fail('path-control-byte', `Declared path "${declared}" contains control bytes.`, 'no control bytes (0x00-0x1F)') + // Control bytes make further reporting unreliable; stop here. + return errors + } + if (declared.includes('\\')) { + fail( + 'path-backslash', + `Declared path "${declared}" contains a backslash. Use forward slashes.`, + 'forward-slash separated relative path', + ) + } + if (declared.startsWith('/')) { + fail('path-absolute', `Declared path "${declared}" is absolute. Paths must be relative.`, 'a relative path') + } else if (DRIVE_PREFIX_RE.test(declared) || URL_PREFIX_RE.test(declared)) { + fail( + 'path-absolute', + `Declared path "${declared}" has a drive or URL-like prefix. Paths must be relative.`, + 'a relative path', + ) + } + + const segments = declared.split('/') + for (let i = 0; i < segments.length; i++) { + const segment = segments[i] as string + if (segment === '..') { + fail('path-traversal', `Declared path "${declared}" contains a ".." segment.`, 'no parent-directory traversal') + continue + } + if (segment === '' || segment === '.') { + // A leading slash was already reported as absolute; don't double-report + // its empty first segment. + if (segment === '' && i === 0 && declared.startsWith('/')) continue + fail( + 'path-empty-segment', + `Declared path "${declared}" contains an empty or "." segment.`, + 'canonical path segments', + ) + continue + } + if (FORBIDDEN_CHARS_RE.test(segment)) { + fail( + 'path-forbidden-character', + `Declared path "${declared}" contains a character that is not portable across filesystems (< > : " | ? *).`, + 'segments without < > : " | ? *', + ) + } + const base = segment.includes('.') ? segment.slice(0, segment.indexOf('.')) : segment + if (RESERVED_DEVICE_NAMES.has(segment.toLowerCase()) || RESERVED_DEVICE_NAMES.has(base.toLowerCase())) { + fail( + 'path-reserved-device-name', + `Declared path "${declared}" contains segment "${segment}", a reserved device name on Windows.`, + 'no Windows-reserved device-name segments', + ) + } + if (segment.endsWith('.') || segment.endsWith(' ')) { + fail( + 'path-trailing-dot-or-space', + `Declared path "${declared}" contains segment "${segment}" ending in a dot or space, which is not portable.`, + 'no segments ending in a dot or space', + ) + } + } + + return errors +} + +/** Segment-wise collision key: canonical Unicode form + portable case fold. */ +function collisionKey(path: string): string { + return path.normalize('NFC').toLowerCase() +} + +interface PlannedPath { + entry: ArchivePlanEntry + /** Where this path was declared, for error attribution. */ + declarationSite: string +} + +/** + * Derive the tagged archive plan from manifest data (design D3), enforcing + * the complete D7 path-safety and collision grammar over the whole planned + * entry set. + * + * Preconditions: asset names are assumed to have passed their manifest + * schema's name grammar. Declared supplementary paths are validated here in + * full, so callers get identical results whether the manifest came from an + * authoring tree or an embedded archive entry. + */ +export function planArchiveEntries(manifest: ArchivePlanInput): ArchivePlanResult { + const errors: ArchivePlanError[] = [] + const planned: PlannedPath[] = [{ entry: { kind: 'manifest', path: MANIFEST_PATH }, declarationSite: '' }] + + // 1. Conventional primary-asset paths, derived from manifest keys. + if (manifest.skills) { + for (const name of Object.keys(manifest.skills)) { + planned.push({ + entry: { kind: 'primary-asset', path: `skills/${name}/SKILL.md`, assetType: 'skill', name }, + declarationSite: `skills.${name}`, + }) + } + } + if (manifest.agents) { + for (const name of Object.keys(manifest.agents)) { + planned.push({ + entry: { kind: 'primary-asset', path: `agents/${name}.md`, assetType: 'agent', name }, + declarationSite: `agents.${name}`, + }) + } + } + if (manifest.commands) { + for (const name of Object.keys(manifest.commands)) { + planned.push({ + entry: { kind: 'primary-asset', path: `commands/${name}.md`, assetType: 'command', name }, + declarationSite: `commands.${name}`, + }) + } + } + + // 2. Skill companions: declared per skill, relative to the skill directory. + if (manifest.skills) { + for (const [skillName, skill] of Object.entries(manifest.skills)) { + if (!skill.files) continue + const site = `skills.${skillName}.files` + for (const declared of skill.files) { + const pathErrors = validateSupplementaryPath(declared, site) + if (pathErrors.length > 0) { + errors.push(...pathErrors) + continue + } + if (declared === 'SKILL.md') { + errors.push( + planError( + 'site-skill-companion-is-primary', + site, + declared, + `Skill "${skillName}" declares "SKILL.md" as a companion file. SKILL.md is the skill's primary file and is always included.`, + 'companion paths other than SKILL.md', + ), + ) + continue + } + planned.push({ + entry: { kind: 'skill-companion', path: `skills/${skillName}/${declared}`, skill: skillName }, + declarationSite: site, + }) + } + } + } + + // 3. Archive-only supplementary files: top-level, repo-relative. + if (manifest.files) { + const site = 'files' + for (const declared of manifest.files) { + const pathErrors = validateSupplementaryPath(declared, site) + if (pathErrors.length > 0) { + errors.push(...pathErrors) + continue + } + if (declared === MANIFEST_PATH) { + errors.push( + planError( + 'reserved-root-manifest', + site, + declared, + `The root path "${MANIFEST_PATH}" is the embedded manifest itself and cannot be declared. The basename may be used below another directory.`, + `paths other than root ${MANIFEST_PATH}`, + ), + ) + continue + } + if (declared.split('/')[0] === 'skills') { + errors.push( + planError( + 'site-top-level-under-skills', + site, + declared, + `Top-level files must not resolve under skills/. Declare "${declared}" in the owning skill's files array instead.`, + 'top-level paths outside skills/', + ), + ) + continue + } + planned.push({ entry: { kind: 'archive-only', path: declared }, declarationSite: site }) + } + } + + // 4. Whole-set collision detection: exact duplicates, Unicode-normalization + // aliases, portable case-fold aliases, and file/directory prefix conflicts. + // Primary paths participate so supplementary-vs-primary collisions are + // caught and reported with their own class. + const byKey = new Map() + const accepted: PlannedPath[] = [] + for (const candidate of planned) { + const key = collisionKey(candidate.entry.path) + const existing = byKey.get(key) + if (!existing) { + byKey.set(key, candidate) + accepted.push(candidate) + continue + } + const supplementaryVsPrimary = + existing.entry.kind === 'manifest' || + existing.entry.kind === 'primary-asset' || + candidate.entry.kind === 'manifest' || + candidate.entry.kind === 'primary-asset' + const code: ArchivePlanErrorCode = + supplementaryVsPrimary && candidate.entry.kind !== existing.entry.kind + ? 'collision-primary-path' + : candidate.entry.path === existing.entry.path + ? 'collision-duplicate' + : candidate.entry.path.normalize('NFC') === existing.entry.path.normalize('NFC') + ? 'collision-unicode-alias' + : 'collision-case-fold' + errors.push( + planError( + code, + candidate.declarationSite, + candidate.entry.path, + `Path "${candidate.entry.path}" collides with "${existing.entry.path}" (declared at ${existing.declarationSite || 'the manifest root'}) on supported filesystems.`, + 'collision-free archive paths', + ), + ) + } + + // File/directory prefix conflicts: a planned file path that is also a + // directory prefix of another planned path cannot coexist on disk. + const directoryKeys = new Map() + for (const candidate of accepted) { + const segments = candidate.entry.path.split('/') + for (let i = 1; i < segments.length; i++) { + directoryKeys.set(collisionKey(segments.slice(0, i).join('/')), candidate) + } + } + for (const candidate of accepted) { + const conflict = directoryKeys.get(collisionKey(candidate.entry.path)) + if (conflict) { + errors.push( + planError( + 'collision-prefix', + candidate.declarationSite, + candidate.entry.path, + `Path "${candidate.entry.path}" is a file but also a parent directory of "${conflict.entry.path}". A path cannot be both.`, + 'no file/directory prefix conflicts', + ), + ) + } + } + + if (errors.length > 0) { + return { ok: false, errors } + } + + const entries = accepted.map((p) => p.entry) + entries.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)) + return { ok: true, data: entries } +} diff --git a/packages/protocol/src/build/content-hash.ts b/packages/protocol/src/build/content-hash.ts index 1edc8f2a..4a21d1e5 100644 --- a/packages/protocol/src/build/content-hash.ts +++ b/packages/protocol/src/build/content-hash.ts @@ -4,7 +4,16 @@ import { type } from 'arktype' import { createTar, parseTar, type TarFileInput, type TarFileItem } from 'nanotar' import { FACET_MANIFEST_FILE, type ResolvedFacetManifest } from '../loaders/facet.ts' import { mapArkErrors, parseJson } from '../loaders/validate.ts' -import { type BuildManifest, BuildManifestSchema } from '../schemas/build-manifest.ts' +import { + BUILD_MANIFEST_NAME, + type BuildManifest, + BuildManifestSchema, + INNER_ARCHIVE_NAME, +} from '../schemas/build-manifest.ts' + +// Outer-tar layout constants are defined beside the build-manifest schemas +// (which pin them) and re-exported here for assembly/parsing consumers. +export { BUILD_MANIFEST_NAME, INNER_ARCHIVE_NAME } export interface ArchiveEntry { path: string @@ -100,12 +109,6 @@ export function assembleTar(entries: ArchiveEntry[]): Uint8Array { return createTar(files, { attrs: DETERMINISTIC_ATTRS }) } -/** Fixed name for the inner archive within the outer `.facet` tar. */ -export const INNER_ARCHIVE_NAME = 'archive.tar.gz' - -/** Fixed name for the build manifest within the outer `.facet` tar. */ -export const BUILD_MANIFEST_NAME = 'build-manifest.json' - /** * Assembles the outer uncompressed tar that forms the `.facet` file. * diff --git a/packages/protocol/src/index.ts b/packages/protocol/src/index.ts index e2eefebf..54e3bd94 100644 --- a/packages/protocol/src/index.ts +++ b/packages/protocol/src/index.ts @@ -9,6 +9,17 @@ // without taking a separate runtime dep on `common`. `common` is bundled // into protocol's published tarball via tsdown's `alwaysBundle`. export { splitFrontMatter } from '@agent-facets/common' +// archive plan — the single shared derivation of archive membership and +// entry classification from a facet manifest (design D3). Build collection, +// hashing, verification, and installation all consume this one operation. +export type { + ArchivePlanEntry, + ArchivePlanError, + ArchivePlanErrorCode, + ArchivePlanInput, + ArchivePlanResult, +} from './build/archive-plan.ts' +export { planArchiveEntries, validateSupplementaryPath } from './build/archive-plan.ts' // content hashing + archive format (deterministic tar layout, hash format, // constants — all part of the integrity contract) export type { ArchiveEntry } from './build/content-hash.ts' @@ -49,11 +60,28 @@ export { verifyLockfileOneCheck, verifyRegistryThreeCheck, } from './integrity/index.ts' +// versioned build-manifest parsing (exact facetVersion dispatch, no +// cross-version fallback) +export type { + BuildManifestParseFailure, + ParseBuildManifestResult, + ParsedBuildManifest, +} from './loaders/build-manifest.ts' +export { parseBuildManifestDocument } from './loaders/build-manifest.ts' // loaders (pure bytes-validators — no I/O) export type { ResolvedFacetManifest } from './loaders/facet.ts' -export { FACET_MANIFEST_FILE, resolvePromptsFromMap, validateFacetManifest } from './loaders/facet.ts' +export { + FACET_MANIFEST_FILE, + resolvePromptsFromMap, + validateFacetManifest, + validateLegacyFacetManifest, +} from './loaders/facet.ts' +// versioned lockfile parsing (exact lockfileVersion dispatch — legacy alpha +// `1` vs current `0.2`, no numeric ordering, no shape-sniffing) +export type { LockfileParseFailure, ParsedLockfile, ParseLockfileResult } from './loaders/lockfile.ts' +export { parseLockfileDocument } from './loaders/lockfile.ts' export { SERVER_MANIFEST_FILE, validateServerManifest } from './loaders/server.ts' -export { mapArkErrors, parseJson } from './loaders/validate.ts' +export { findDuplicateJsonMembers, mapArkErrors, parseJson } from './loaders/validate.ts' // asset-name grammar (Agent Skills spec) — exported so build validators, the // CLI, and the engine's edit/scaffold machinery all validate skill/command/ // agent names against one canonical grammar. Distinct from facet identity @@ -66,17 +94,48 @@ export { validateAssetNameSegment, } from './schemas/asset-name.ts' // schemas -export type { BuildManifest } from './schemas/build-manifest.ts' -export { BuildManifestSchema } from './schemas/build-manifest.ts' +export type { BuildManifest, CurrentBuildManifest, LegacyBuildManifest } from './schemas/build-manifest.ts' +export { + BuildManifestSchema, + CurrentBuildManifestSchema, + FACET_ARCHIVE_VERSION, + LEGACY_FACET_ARCHIVE_VERSION, + LegacyBuildManifestSchema, + SUPPORTED_FACET_VERSIONS, +} from './schemas/build-manifest.ts' export type { FacetManifest } from './schemas/facet-manifest.ts' export { FacetManifestSchema } from './schemas/facet-manifest.ts' +// legacy 0.1 facet-manifest schema — frozen pre-0.2 rules (multi-segment +// asset names, no shared skill/command namespace, no supplementary files), +// consumed only by legacy archive verification during the compatibility +// window. +export type { LegacyFacetManifest } from './schemas/facet-manifest-legacy.ts' +export { LegacyFacetManifestSchema } from './schemas/facet-manifest-legacy.ts' // facet identity grammar (slugs + scoped/unscoped facet names) — exported so // other facet-spec implementations (e.g. the registry enforcing scope // ownership) validate scopes with the same grammar. export type { FacetName, FacetNameResult, SlugResult } from './schemas/facet-name.ts' export { parseFacetName, parseSlug, validateFacetName } from './schemas/facet-name.ts' -export type { Lockfile, LockfileAssetEntry, LockfileFacet, LockfileSource } from './schemas/lockfile.ts' -export { LOCKFILE_VERSION, LockfileSchema } from './schemas/lockfile.ts' +export type { + CurrentLockfile, + CurrentLockfileAssetEntry, + CurrentLockfileFacet, + LegacyLockfile, + Lockfile, + LockfileAssetEntry, + LockfileFacet, + LockfileFileRecord, + LockfileSource, +} from './schemas/lockfile.ts' +export { + CURRENT_LOCKFILE_VERSION, + CurrentLockfileSchema, + LEGACY_LOCKFILE_VERSION, + LegacyLockfileSchema, + LOCKFILE_VERSION, + LockfileSchema, + SUPPORTED_LOCKFILE_VERSIONS, +} from './schemas/lockfile.ts' export type { FacetsJson } from './schemas/project-manifest.ts' export { FacetsJsonSchema } from './schemas/project-manifest.ts' export type { ServerManifest } from './schemas/server-manifest.ts' diff --git a/packages/protocol/src/integrity/validate-archive.ts b/packages/protocol/src/integrity/validate-archive.ts index 82e34098..c3f8c057 100644 --- a/packages/protocol/src/integrity/validate-archive.ts +++ b/packages/protocol/src/integrity/validate-archive.ts @@ -3,7 +3,7 @@ import { computeContentHash, INNER_ARCHIVE_NAME, parseFacetArchive, parseInnerAr import { detectNamingCollisions } from '../build/detect-collisions.ts' import { validateContentFiles } from '../build/validate-content.ts' import { validateCompactFacets } from '../build/validate-facets.ts' -import { FACET_MANIFEST_FILE, resolvePromptsFromMap, validateFacetManifest } from '../loaders/facet.ts' +import { FACET_MANIFEST_FILE, resolvePromptsFromMap, validateLegacyFacetManifest } from '../loaders/facet.ts' import type { BuildManifest } from '../schemas/build-manifest.ts' import type { FacetManifest } from '../schemas/facet-manifest.ts' import { verifyHash } from './verify.ts' @@ -94,7 +94,10 @@ export interface VerifiedArchive { * Failures surface as one `ValidationError` per asset, rooted at * the in-archive path. * 6. Locate the inner archive's `facet.json` entry and validate it - * against the facet-manifest schema (via `validateFacetManifest`). + * against the legacy `0.1` facet-manifest schema (via + * `validateLegacyFacetManifest`) — this verifier currently handles + * only legacy `0.1` archives, which retain legacy asset-name and + * namespace rules during the compatibility window (design D9). * Failures surface unchanged, with their `path` re-rooted at * `FACET_MANIFEST_FILE`. * 7. Reconstruct a `ResolvedFacetManifest` from the inner-tar entries @@ -272,7 +275,7 @@ export async function validateFacetArchive( ], } } - const facetResult = validateFacetManifest(facetManifestAsset.bytes) + const facetResult = validateLegacyFacetManifest(facetManifestAsset.bytes) if (!facetResult.ok) { return { ok: false, diff --git a/packages/protocol/src/loaders/build-manifest.ts b/packages/protocol/src/loaders/build-manifest.ts new file mode 100644 index 00000000..e0b6c234 --- /dev/null +++ b/packages/protocol/src/loaders/build-manifest.ts @@ -0,0 +1,107 @@ +import type { ValidationError } from '@agent-facets/common' +import { type } from 'arktype' +import { + type CurrentBuildManifest, + CurrentBuildManifestSchema, + FACET_ARCHIVE_VERSION, + LEGACY_FACET_ARCHIVE_VERSION, + type LegacyBuildManifest, + LegacyBuildManifestSchema, + SUPPORTED_FACET_VERSIONS, +} from '../schemas/build-manifest.ts' +import { findDuplicateJsonMembers, mapArkErrors, parseJson } from './validate.ts' + +/** + * Structured failure data for build-manifest parsing. Every expected failure + * mode is a tagged variant — no thrown errors, no message parsing. + */ +export type BuildManifestParseFailure = + /** The document is not valid JSON. */ + | { code: 'invalid-json'; errors: ValidationError[] } + /** The document contains duplicate object member names (rejected before schema validation). */ + | { code: 'duplicate-members'; errors: ValidationError[] } + /** The declared `facetVersion` is not a supported archive format. */ + | { code: 'unsupported-facet-version'; observed: number | undefined; supported: readonly number[] } + /** The document declared a supported version but violates that version's schema. */ + | { code: 'schema-violation'; facetVersion: number; errors: ValidationError[] } + +/** + * A successfully parsed build manifest, tagged by its exact archive format + * version so downstream consumers dispatch exhaustively and never treat one + * format's fields as the other's. + */ +export type ParsedBuildManifest = + | { facetVersion: typeof LEGACY_FACET_ARCHIVE_VERSION; manifest: LegacyBuildManifest } + | { facetVersion: typeof FACET_ARCHIVE_VERSION; manifest: CurrentBuildManifest } + +export type ParseBuildManifestResult = + | { ok: true; data: ParsedBuildManifest } + | { ok: false; failure: BuildManifestParseFailure } + +/** + * Parses and validates a `build-manifest.json` document with exact + * `facetVersion` dispatch (design D4): + * + * 1. JSON parse (syntax errors are structured failures). + * 2. Reject duplicate object member names before schema validation. + * 3. Dispatch on `facetVersion` by exact equality — `0.1` selects the + * legacy schema, `0.2` the current schema, anything else is a + * structured unsupported-version failure carrying the observed and + * supported versions. + * + * There is NO fallback between versions: a malformed `0.2` manifest fails + * as a `0.2` schema violation and is never reinterpreted as `0.1`. + */ +export function parseBuildManifestDocument(bytes: Uint8Array | string): ParseBuildManifestResult { + const text = typeof bytes === 'string' ? bytes : new TextDecoder().decode(bytes) + + const jsonResult = parseJson(text) + if (!jsonResult.ok) { + return { ok: false, failure: { code: 'invalid-json', errors: jsonResult.errors } } + } + + const duplicates = findDuplicateJsonMembers(text) + if (duplicates.length > 0) { + return { ok: false, failure: { code: 'duplicate-members', errors: duplicates } } + } + + const observedVersion = + typeof jsonResult.data === 'object' && jsonResult.data !== null && 'facetVersion' in jsonResult.data + ? (jsonResult.data as { facetVersion?: unknown }).facetVersion + : undefined + + if (observedVersion === LEGACY_FACET_ARCHIVE_VERSION) { + const validated = LegacyBuildManifestSchema(jsonResult.data) + if (validated instanceof type.errors) { + return { + ok: false, + failure: { + code: 'schema-violation', + facetVersion: LEGACY_FACET_ARCHIVE_VERSION, + errors: mapArkErrors(validated), + }, + } + } + return { ok: true, data: { facetVersion: LEGACY_FACET_ARCHIVE_VERSION, manifest: validated } } + } + + if (observedVersion === FACET_ARCHIVE_VERSION) { + const validated = CurrentBuildManifestSchema(jsonResult.data) + if (validated instanceof type.errors) { + return { + ok: false, + failure: { code: 'schema-violation', facetVersion: FACET_ARCHIVE_VERSION, errors: mapArkErrors(validated) }, + } + } + return { ok: true, data: { facetVersion: FACET_ARCHIVE_VERSION, manifest: validated } } + } + + return { + ok: false, + failure: { + code: 'unsupported-facet-version', + observed: typeof observedVersion === 'number' ? observedVersion : undefined, + supported: SUPPORTED_FACET_VERSIONS, + }, + } +} diff --git a/packages/protocol/src/loaders/facet.ts b/packages/protocol/src/loaders/facet.ts index 03667c6d..fb7f4610 100644 --- a/packages/protocol/src/loaders/facet.ts +++ b/packages/protocol/src/loaders/facet.ts @@ -1,7 +1,8 @@ import type { Validated, ValidationError } from '@agent-facets/common' import { type } from 'arktype' import { type FacetManifest, FacetManifestSchema } from '../schemas/facet-manifest.ts' -import { mapArkErrors, parseJson } from './validate.ts' +import { type LegacyFacetManifest, LegacyFacetManifestSchema } from '../schemas/facet-manifest-legacy.ts' +import { findDuplicateJsonMembers, mapArkErrors, parseJson } from './validate.ts' export const FACET_MANIFEST_FILE = 'facet.json' @@ -59,6 +60,14 @@ export function validateFacetManifest(bytes: Uint8Array | string): Validated 0) { + return { ok: false, errors: duplicates } + } + const validated = FacetManifestSchema(jsonResult.data) if (validated instanceof type.errors) { return { ok: false, errors: mapArkErrors(validated) } @@ -67,6 +76,36 @@ export function validateFacetManifest(bytes: Uint8Array | string): Validated { + const text = typeof bytes === 'string' ? bytes : new TextDecoder().decode(bytes) + + const jsonResult = parseJson(text) + if (!jsonResult.ok) { + return jsonResult + } + + // Same duplicate-member rejection as the current validator — parser + // collapse is a smuggling vector regardless of format version. + const duplicates = findDuplicateJsonMembers(text) + if (duplicates.length > 0) { + return { ok: false, errors: duplicates } + } + + const validated = LegacyFacetManifestSchema(jsonResult.data) + if (validated instanceof type.errors) { + return { ok: false, errors: mapArkErrors(validated) } + } + + return { ok: true, data: validated } +} + /** * Resolves prompt content for all skills, agents, and commands using a * caller-supplied map of relative path → file content. The map MUST contain diff --git a/packages/protocol/src/loaders/lockfile.ts b/packages/protocol/src/loaders/lockfile.ts new file mode 100644 index 00000000..4e0df3b5 --- /dev/null +++ b/packages/protocol/src/loaders/lockfile.ts @@ -0,0 +1,111 @@ +import type { ValidationError } from '@agent-facets/common' +import { type } from 'arktype' +import { + CURRENT_LOCKFILE_VERSION, + type CurrentLockfile, + CurrentLockfileSchema, + LEGACY_LOCKFILE_VERSION, + type LegacyLockfile, + LegacyLockfileSchema, + SUPPORTED_LOCKFILE_VERSIONS, +} from '../schemas/lockfile.ts' +import { findDuplicateJsonMembers, mapArkErrors, parseJson } from './validate.ts' + +/** + * Structured failure data for lockfile parsing. Every expected failure mode + * is a tagged variant — no thrown errors, no message parsing. + */ +export type LockfileParseFailure = + /** The document is not valid JSON. */ + | { code: 'invalid-json'; errors: ValidationError[] } + /** The document contains duplicate object member names (rejected before schema validation). */ + | { code: 'duplicate-members'; errors: ValidationError[] } + /** The declared `lockfileVersion` is not a supported lockfile schema version. */ + | { code: 'unsupported-lockfile-version'; observed: number | undefined; supported: readonly number[] } + /** The document declared a supported version but violates that version's schema. */ + | { code: 'schema-violation'; lockfileVersion: number; errors: ValidationError[] } + +/** + * A successfully parsed lockfile, tagged by its exact schema version so + * downstream consumers dispatch exhaustively — legacy identity-only asset + * entries can never be mistaken for current per-file-integrity entries. + */ +export type ParsedLockfile = + | { lockfileVersion: typeof LEGACY_LOCKFILE_VERSION; lockfile: LegacyLockfile } + | { lockfileVersion: typeof CURRENT_LOCKFILE_VERSION; lockfile: CurrentLockfile } + +export type ParseLockfileResult = { ok: true; data: ParsedLockfile } | { ok: false; failure: LockfileParseFailure } + +/** + * Parses and validates a `facets.lock` document with exact version dispatch + * (design D10): + * + * 1. JSON parse (syntax errors are structured failures). + * 2. Reject duplicate object member names before schema validation. + * 3. Dispatch on `lockfileVersion` by EXACT equality, never numeric + * ordering — legacy numeric `1` selects only the previous alpha + * schema, numeric `0.2` selects the current schema, anything else is a + * structured unsupported-version failure carrying the observed and + * supported versions. + * + * There is NO fallback or shape-sniffing between versions: a malformed + * `0.2` lockfile fails as a `0.2` schema violation and is never + * reinterpreted as legacy alpha `1`. + */ +export function parseLockfileDocument(bytes: Uint8Array | string): ParseLockfileResult { + const text = typeof bytes === 'string' ? bytes : new TextDecoder().decode(bytes) + + const jsonResult = parseJson(text) + if (!jsonResult.ok) { + return { ok: false, failure: { code: 'invalid-json', errors: jsonResult.errors } } + } + + const duplicates = findDuplicateJsonMembers(text) + if (duplicates.length > 0) { + return { ok: false, failure: { code: 'duplicate-members', errors: duplicates } } + } + + const observedVersion = + typeof jsonResult.data === 'object' && jsonResult.data !== null && 'lockfileVersion' in jsonResult.data + ? (jsonResult.data as { lockfileVersion?: unknown }).lockfileVersion + : undefined + + if (observedVersion === LEGACY_LOCKFILE_VERSION) { + const validated = LegacyLockfileSchema(jsonResult.data) + if (validated instanceof type.errors) { + return { + ok: false, + failure: { + code: 'schema-violation', + lockfileVersion: LEGACY_LOCKFILE_VERSION, + errors: mapArkErrors(validated), + }, + } + } + return { ok: true, data: { lockfileVersion: LEGACY_LOCKFILE_VERSION, lockfile: validated } } + } + + if (observedVersion === CURRENT_LOCKFILE_VERSION) { + const validated = CurrentLockfileSchema(jsonResult.data) + if (validated instanceof type.errors) { + return { + ok: false, + failure: { + code: 'schema-violation', + lockfileVersion: CURRENT_LOCKFILE_VERSION, + errors: mapArkErrors(validated), + }, + } + } + return { ok: true, data: { lockfileVersion: CURRENT_LOCKFILE_VERSION, lockfile: validated } } + } + + return { + ok: false, + failure: { + code: 'unsupported-lockfile-version', + observed: typeof observedVersion === 'number' ? observedVersion : undefined, + supported: SUPPORTED_LOCKFILE_VERSIONS, + }, + } +} diff --git a/packages/protocol/src/loaders/validate.ts b/packages/protocol/src/loaders/validate.ts index acf94366..bd30f9a8 100644 --- a/packages/protocol/src/loaders/validate.ts +++ b/packages/protocol/src/loaders/validate.ts @@ -16,6 +16,107 @@ export function mapArkErrors(errors: InstanceType): Validati })) } +/** + * Scans a KNOWN-VALID JSON document for duplicate object member names. + * `JSON.parse` silently collapses duplicates (last member wins), which lets + * two parsers see different data in one document — a smuggling vector for + * security-relevant artifacts (facet manifests, build manifests, lockfiles). + * + * Precondition: `text` has already been accepted by `JSON.parse`. The scan + * assumes well-formed input and only tracks strings, object/array nesting, + * and member-key positions. Escaped keys are decoded before comparison so + * `"\u0066iles"` and `"files"` are detected as duplicates. + * + * Returns one ValidationError per duplicated member, with `path` pointing at + * the enclosing object's location (dot-separated, array indices numeric). + */ +export function findDuplicateJsonMembers(text: string): ValidationError[] { + const errors: ValidationError[] = [] + + type Frame = + | { kind: 'object'; keys: Set; pathSegment: string } + | { kind: 'array'; index: number; pathSegment: string } + + const stack: Frame[] = [] + let expectKey = false + let pendingSegment = '' + let i = 0 + + const currentPath = (): string => + stack + .map((f) => f.pathSegment) + .filter((s) => s !== '') + .join('.') + + while (i < text.length) { + const ch = text[i] as string + if (ch === '"') { + // Scan the string token, honoring escapes. + let j = i + 1 + while (j < text.length) { + const c = text[j] + if (c === '\\') { + j += 2 + continue + } + if (c === '"') break + j++ + } + const raw = text.slice(i, j + 1) + const top = stack[stack.length - 1] + if (top?.kind === 'object' && expectKey) { + const key = JSON.parse(raw) as string + if (top.keys.has(key)) { + errors.push({ + path: currentPath(), + message: `Duplicate JSON object member "${key}". Documents with duplicate members are rejected because parsers disagree on which member wins.`, + expected: 'unique object member names', + actual: `member "${key}" declared more than once`, + }) + } + top.keys.add(key) + pendingSegment = key + expectKey = false + } + i = j + 1 + continue + } + if (ch === '{') { + stack.push({ kind: 'object', keys: new Set(), pathSegment: pendingSegment }) + pendingSegment = '' + expectKey = true + i++ + continue + } + if (ch === '[') { + stack.push({ kind: 'array', index: 0, pathSegment: pendingSegment }) + pendingSegment = '0' + i++ + continue + } + if (ch === '}' || ch === ']') { + stack.pop() + pendingSegment = '' + i++ + continue + } + if (ch === ',') { + const top = stack[stack.length - 1] + if (top?.kind === 'object') { + expectKey = true + } else if (top?.kind === 'array') { + top.index++ + pendingSegment = String(top.index) + } + i++ + continue + } + i++ + } + + return errors +} + /** * Parses a JSON string. Returns the parsed data or a ValidationError array. */ diff --git a/packages/protocol/src/schemas/build-manifest.ts b/packages/protocol/src/schemas/build-manifest.ts index e590675d..b7053c66 100644 --- a/packages/protocol/src/schemas/build-manifest.ts +++ b/packages/protocol/src/schemas/build-manifest.ts @@ -1,13 +1,98 @@ import { type } from 'arktype' +// --- Archive-format constants --- + +/** + * The current archive format version written into every new build manifest. + * Distinct from `LOCKFILE_VERSION` (see ./lockfile.ts): both currently equal + * `0.2`, but that is release alignment, not a permanent invariant — archive + * and resolution formats may evolve independently (design D10). + */ +export const FACET_ARCHIVE_VERSION = 0.2 + +/** The legacy archive format version, accepted as consumer input during the compatibility window. */ +export const LEGACY_FACET_ARCHIVE_VERSION = 0.1 + +/** Every archive format version this implementation can verify. */ +export const SUPPORTED_FACET_VERSIONS: readonly number[] = [LEGACY_FACET_ARCHIVE_VERSION, FACET_ARCHIVE_VERSION] + +/** + * Outer-tar layout constants. Defined here (with the schemas that pin them) + * and re-exported from `../build/content-hash.ts`, which consumes them for + * assembly/parsing — a single source for the wire contract without an + * import cycle. + */ +/** Fixed name of the inner compressed archive inside the outer tar. */ +export const INNER_ARCHIVE_NAME = 'archive.tar.gz' +/** Fixed name of the build manifest inside the outer tar. */ +export const BUILD_MANIFEST_NAME = 'build-manifest.json' + +const INTEGRITY_RE = /^sha256:[a-f0-9]{64}$/ + +// --- Versioned schemas (exact facetVersion dispatch, design D4) --- + +/** + * Legacy `0.1` build-manifest schema — frozen at the pre-supplementary-file + * shape: a per-asset `assets` hash map. `facetVersion` is pinned to the + * exact numeric literal `0.1`; a `files` key is rejected so the two format + * shapes are unrepresentable in one validated document. + */ +export const LegacyBuildManifestSchema = type({ + facetVersion: type.unit(LEGACY_FACET_ARCHIVE_VERSION), + archive: 'string', + integrity: INTEGRITY_RE, + assets: type.Record('string', 'string'), +}).narrow((data, ctx) => { + if (Object.hasOwn(data, 'files')) { + return ctx.mustBe('a legacy 0.1 build manifest without a current-format "files" map') + } + return true +}) + +/** Inferred TypeScript type for a validated legacy (`0.1`) build manifest */ +export type LegacyBuildManifest = typeof LegacyBuildManifestSchema.infer + +/** + * Current `0.2` build-manifest schema. `facetVersion` is pinned to the exact + * numeric literal `0.2` and `archive` to the exact canonical entry name, so + * producers and consumers cannot disagree about which outer-tar entry is + * authoritative. The `files` map carries one `sha256:` hash per + * canonical inner-archive path — hashes only; entry classification is NEVER + * read from the build manifest, it is derived from the embedded `facet.json` + * via the archive plan (design D3/D4). A legacy `assets` key is rejected. + */ +export const CurrentBuildManifestSchema = type({ + facetVersion: type.unit(FACET_ARCHIVE_VERSION), + archive: type.unit(INNER_ARCHIVE_NAME), + integrity: INTEGRITY_RE, + files: type.Record('string', type(INTEGRITY_RE)), +}).narrow((data, ctx) => { + if (Object.hasOwn(data, 'assets')) { + return ctx.mustBe('a current 0.2 build manifest without a legacy "assets" map') + } + return true +}) + +/** Inferred TypeScript type for a validated current (`0.2`) build manifest */ +export type CurrentBuildManifest = typeof CurrentBuildManifestSchema.infer + +// --- Transitional permissive schema --- + /** - * Schema for the build manifest (build-manifest.json). - * Written by `facet build` alongside the .facet archive. + * Schema for the build manifest (build-manifest.json), written by + * `facet build` alongside the .facet archive. + * + * @deprecated Transitional: this permissive shape (unpinned `facetVersion`, + * legacy `assets` map) predates exact version dispatch. Verification and + * engine call sites migrate to `parseBuildManifestDocument` / + * `LegacyBuildManifestSchema` / `CurrentBuildManifestSchema` during the + * consumer-bridge and producer blocks of the `0.2` rollout, after which this + * export is removed. */ export const BuildManifestSchema = type({ facetVersion: 'number', archive: 'string', - integrity: /^sha256:[a-f0-9]{64}$/, + integrity: INTEGRITY_RE, assets: type.Record('string', 'string'), }) diff --git a/packages/protocol/src/schemas/facet-manifest-legacy.ts b/packages/protocol/src/schemas/facet-manifest-legacy.ts new file mode 100644 index 00000000..b734d397 --- /dev/null +++ b/packages/protocol/src/schemas/facet-manifest-legacy.ts @@ -0,0 +1,113 @@ +import { type } from 'arktype' +import { validateAssetName } from './asset-name.ts' +import { validateFacetName } from './facet-name.ts' + +/** + * Legacy `0.1` facet-manifest schema — frozen at the pre-supplementary-file + * rules so archives produced before archive format `0.2` remain consumable + * during the compatibility window. + * + * Differences from the current `FacetManifestSchema`: + * - Asset names use the legacy multi-segment grammar (`validateAssetName`, + * allowing namespaced names like `viper-plans/planning`). + * - Skills and commands do NOT share a namespace; only same-type duplicates + * are invalid (and those cannot appear in JSON-parsed records anyway). + * - No supplementary `files` declarations are recognized. A `files` key in a + * legacy manifest is tolerated as unknown extension data, matching the + * legacy consumers' unknown-field tolerance. + * + * This schema is consumed only by legacy `0.1` archive verification. An + * invalid current-format manifest MUST NOT be reinterpreted under these + * rules (design D4/D9: no cross-version fallback). + */ + +const LegacySkillDescriptor = type({ + description: 'string', + 'adapters?': type.Record('string', 'unknown'), +}) + +const LegacyAgentDescriptor = type({ + description: 'string', + 'adapters?': type.Record('string', 'unknown'), +}) + +const LegacyCommandDescriptor = type({ + description: 'string', + 'adapters?': type.Record('string', 'unknown'), +}) + +const LegacySelectiveFacetsEntry = type({ + name: 'string', + version: 'string', + 'skills?': 'string[]', + 'agents?': 'string[]', + 'commands?': 'string[]', +}) + +const LegacyFacetsEntry = type('string').or(LegacySelectiveFacetsEntry) + +const LegacyServerReference = type('string').or({ image: 'string' }) + +export const LegacyFacetManifestSchema = type({ + name: 'string', + version: 'string', + 'description?': 'string', + 'author?': 'string', + 'private?': 'boolean', + 'skills?': type.Record('string', LegacySkillDescriptor), + 'agents?': type.Record('string', LegacyAgentDescriptor), + 'commands?': type.Record('string', LegacyCommandDescriptor), + 'facets?': LegacyFacetsEntry.array(), + 'servers?': type.Record('string', LegacyServerReference), +}).narrow((data, ctx) => { + const facetName = validateFacetName(data.name) + if (!facetName.ok) { + ctx.mustBe(`a valid facet name: ${facetName.reason}`) + } + + const hasSkills = data.skills && Object.keys(data.skills).length > 0 + const hasAgents = data.agents && Object.keys(data.agents).length > 0 + const hasCommands = data.commands && Object.keys(data.commands).length > 0 + const hasFacets = data.facets && data.facets.length > 0 + + if (!hasSkills && !hasAgents && !hasCommands && !hasFacets) { + ctx.mustBe('Manifest must include at least one text asset (skills, agents, commands, or facets)') + } + + if (data.facets) { + for (let i = 0; i < data.facets.length; i++) { + const entry = data.facets[i] + if (typeof entry === 'object') { + const hasSelectedSkills = entry.skills && entry.skills.length > 0 + const hasSelectedAgents = entry.agents && entry.agents.length > 0 + const hasSelectedCommands = entry.commands && entry.commands.length > 0 + + if (!hasSelectedSkills && !hasSelectedAgents && !hasSelectedCommands) { + ctx.mustBe('Selective facets entry must include at least one asset type (skills, agents, or commands)') + } + } + } + } + + // Legacy asset-name rules: multi-segment names permitted, every segment + // validated against the Agent Skills grammar. + const assetNameGroups: [string, Record | undefined][] = [ + ['skills', data.skills], + ['agents', data.agents], + ['commands', data.commands], + ] + for (const [group, record] of assetNameGroups) { + if (!record) continue + for (const key of Object.keys(record)) { + const check = validateAssetName(key) + if (!check.ok) { + ctx.mustBe(`${group} name "${key}" ${check.reason}`) + } + } + } + + return true +}) + +/** Inferred TypeScript type for a validated legacy (`0.1`) facet manifest */ +export type LegacyFacetManifest = typeof LegacyFacetManifestSchema.infer diff --git a/packages/protocol/src/schemas/facet-manifest.ts b/packages/protocol/src/schemas/facet-manifest.ts index c3fe4097..c30cb4c8 100644 --- a/packages/protocol/src/schemas/facet-manifest.ts +++ b/packages/protocol/src/schemas/facet-manifest.ts @@ -1,13 +1,20 @@ import { type } from 'arktype' -import { validateAssetName } from './asset-name.ts' +import { planArchiveEntries } from '../build/archive-plan.ts' +import { validateAssetNameSegment } from './asset-name.ts' import { validateFacetName } from './facet-name.ts' // --- Sub-schemas --- -/** Skill descriptor — description is required, prompt resolved from skills//SKILL.md */ +/** + * Skill descriptor — description is required, prompt resolved from + * skills//SKILL.md. `files` declares exact companion paths relative to + * the skill directory (design D1); the path grammar, site rules, and + * collision freedom are enforced by the archive-plan narrow below. + */ const SkillDescriptor = type({ description: 'string', 'adapters?': type.Record('string', 'unknown'), + 'files?': 'string[]', }) /** Agent descriptor — description is required, prompt resolved from agents/.md */ @@ -52,16 +59,26 @@ const ServerReference = type('string').or({ image: 'string' }) * Structural validation covers field types and shapes. Narrow constraints enforce: * 1. At least one text asset (skills, agents, commands, or facets) must be present * 2. Selective facets entries must include at least one asset type selection - * 3. Asset names must satisfy the Agent Skills grammar (validateAssetName from - * ./asset-name.ts): per `/`-separated segment, 1-64 chars, lowercase - * letters/digits/hyphens, no leading/trailing/consecutive hyphens. This - * grammar subsumes path safety (empty, `.`, `..`, and backslash segments - * all fail), so it replaces the weaker path-only guard for manifest keys. + * 3. Asset names must satisfy the Agent Skills grammar as a SINGLE segment + * (validateAssetNameSegment from ./asset-name.ts): 1-64 chars, lowercase + * letters/digits/hyphens, no leading/trailing/consecutive hyphens, no `/`. + * Slash-namespaced names are legacy-`0.1`-only (LegacyFacetManifestSchema). + * The grammar subsumes path safety (empty, `.`, `..`, and backslash + * segments all fail), so it replaces the weaker path-only guard for + * manifest keys. * 4. The facet identity `name` must be a valid facet name — an unscoped slug * or a scoped `@scope/name` (validateFacetName). Asset names and facet * identities intentionally diverge: asset names stay local kebab segments * (digit-start allowed, never scoped); facet identities may carry a * registry scope. + * 5. Skills and commands share one logical namespace (design D9): a skill and + * a command must not use the same name. Agents remain separate and may + * share a name with a skill or command. + * 6. Supplementary `files` declarations (top-level and per-skill) must satisfy + * the portable path grammar and be collision-free across the whole planned + * archive-entry set — enforced by the shared archive-plan derivation + * (design D3/D7), so a manifest that validates here always yields a valid + * archive plan. */ export const FacetManifestSchema = type({ name: 'string', @@ -79,6 +96,11 @@ export const FacetManifestSchema = type({ 'commands?': type.Record('string', CommandDescriptor), 'facets?': FacetsEntry.array(), 'servers?': type.Record('string', ServerReference), + // Top-level supplementary files: exact repo-relative paths for archive-only + // files (README.md, LICENSE, ...). Shipped and hashed, never materialized. + // Must not resolve under skills/ — skill companions have exactly one + // declaration site (design D1). + 'files?': 'string[]', }).narrow((data, ctx) => { // Constraint 0: the facet identity name must be a valid facet name. Either // an unscoped slug (`cowsay`) or a scoped `@scope/name` (`@julian/cowsay`). @@ -117,11 +139,11 @@ export const FacetManifestSchema = type({ } } - // Constraint 3: asset names must satisfy the Agent Skills grammar (see - // ./asset-name.ts). This tightens the previous path-safety-only check: a - // manifest declaring `MySkill` or `foo_bar` now fails at build AND install - // (this schema validates fetched manifests too). Because the grammar rejects - // empty, `.`, `..`, and backslash segments, it also subsumes the filesystem + // Constraint 3: asset names must satisfy the Agent Skills grammar as a + // single segment (see ./asset-name.ts). Current-format names are never + // slash-namespaced — multi-segment parsing is isolated to the legacy `0.1` + // schema (facet-manifest-legacy.ts). Because the grammar rejects empty, + // `.`, `..`, and backslash segments, it also subsumes the filesystem // safety the install pipeline needs when writing join(baseDir, // relativePathFor(type, name)). LockfileSchema intentionally keeps the // weaker `@agent-facets/common` path-safety guard so legacy installs with @@ -134,13 +156,39 @@ export const FacetManifestSchema = type({ for (const [group, record] of assetNameGroups) { if (!record) continue for (const key of Object.keys(record)) { - const check = validateAssetName(key) + const check = validateAssetNameSegment(key) if (!check.ok) { ctx.mustBe(`${group} name "${key}" ${check.reason}`) } } } + // Constraint 5: skills and commands share one logical namespace (design + // D9). A facet declaring both skill `review` and command `review` is + // invalid; the error identifies both declarations. Agents are a separate + // namespace. + if (data.skills && data.commands) { + for (const name of Object.keys(data.skills)) { + if (Object.hasOwn(data.commands, name)) { + ctx.mustBe( + `skills and commands share one namespace: "${name}" is declared as both skills.${name} and commands.${name}`, + ) + } + } + } + + // Constraint 6: supplementary declarations must yield a valid archive plan + // (design D3/D7): portable path grammar, declaration-site rules, and + // collision freedom across the whole planned entry set. Delegating to the + // shared derivation keeps this schema and every downstream consumer + // (build collection, hashing, verification) agreeing on one grammar. + const plan = planArchiveEntries(data) + if (!plan.ok) { + for (const error of plan.errors) { + ctx.mustBe(error.path ? `${error.path}: ${error.message}` : error.message) + } + } + return true }) diff --git a/packages/protocol/src/schemas/lockfile.ts b/packages/protocol/src/schemas/lockfile.ts index 62ecc44a..80021328 100644 --- a/packages/protocol/src/schemas/lockfile.ts +++ b/packages/protocol/src/schemas/lockfile.ts @@ -1,11 +1,37 @@ import { validateAssetName } from '@agent-facets/common' import { type } from 'arktype' +/** + * The legacy alpha lockfile schema version. Numeric `1` identifies ONLY the + * previous alpha schema (asset entries without per-file integrity records). + * Version dispatch uses exact equality, never numeric ordering (design D10). + * When the stable lockfile v1 schema is eventually released, support for + * this legacy-alpha `1` is removed rather than reinterpreted. + */ +export const LEGACY_LOCKFILE_VERSION = 1 + +/** + * The current lockfile schema version. Distinct constant from + * `FACET_ARCHIVE_VERSION` (see ./build-manifest.ts): both currently equal + * `0.2`, but that is release alignment, not a permanent invariant — archive + * and resolution formats may evolve independently (design D10). + */ +export const CURRENT_LOCKFILE_VERSION = 0.2 + +/** Every lockfile schema version this implementation can read. */ +export const SUPPORTED_LOCKFILE_VERSIONS: readonly number[] = [LEGACY_LOCKFILE_VERSION, CURRENT_LOCKFILE_VERSION] + /** * Current lockfile schema version. Bump on breaking shape changes. * Forward-compat migrations key off this field. + * + * @deprecated Transitional: engine's pre-`0.2` loader still keys its + * newer-version guard and empty-lockfile bootstrap off this constant. It + * migrates to exact dispatch via `parseLockfileDocument` (with + * `LEGACY_LOCKFILE_VERSION` / `CURRENT_LOCKFILE_VERSION`) in the lockfile + * migration block of the `0.2` rollout, after which this export is removed. */ -export const LOCKFILE_VERSION = 1 +export const LOCKFILE_VERSION = LEGACY_LOCKFILE_VERSION /** * A single asset contributed by a facet at this resolved version. @@ -106,6 +132,108 @@ const LockfileFacetEntry = type({ assets: LockfileAsset.array(), }) +// --- Current (0.2) asset shape: per-materialized-file integrity records --- + +const FILE_INTEGRITY_RE = /^sha256:[a-f0-9]{64}$/ + +/** + * One materialized-file integrity record inside its owning asset entry + * (design D10). `path` is the canonical inner-archive path; `integrity` is + * the hash of that archive entry's exact canonical bytes. Companion files + * are subordinate records here — they never become independent assets. + */ +const LockfileAssetFileRecord = type({ + path: 'string', + integrity: FILE_INTEGRITY_RE, +}) + +/** + * A current (`0.2`) asset entry: adapter-agnostic identity plus a required, + * deterministically sorted `files` array covering every materialized file — + * `skills//SKILL.md` plus declared companions for skills, exactly the + * conventional primary path for agents and commands. Archive-only + * supplementary files never appear here (facet-level integrity pins them). + * + * The narrow enforces: the shared asset-name guard (as in the legacy asset + * shape), path safety for every file record, and strict lexicographic + * ordering by path (which also forbids duplicate paths) so lockfile diffs + * stay stable and reviewable. + */ +const CurrentLockfileAsset = type({ + scope: "'system' | 'user' | 'project'", + type: "'skill' | 'agent' | 'command'", + name: 'string', + files: LockfileAssetFileRecord.array(), +}).narrow((data, ctx) => { + const check = validateAssetName(data.name) + if (!check.ok) { + return ctx.mustBe(`asset name "${data.name}" ${check.reason}`) + } + if (data.files.length === 0) { + return ctx.mustBe('an asset entry with at least one materialized file record') + } + for (let i = 0; i < data.files.length; i++) { + const record = data.files[i] as (typeof data.files)[number] + const pathCheck = validateAssetName(record.path) + if (!pathCheck.ok) { + return ctx.mustBe(`file path "${record.path}" ${pathCheck.reason}`) + } + if (i > 0) { + const previous = (data.files[i - 1] as (typeof data.files)[number]).path + if (!(record.path > previous)) { + return ctx.mustBe( + `file records sorted by path: "${record.path}" must sort after "${previous}" with no duplicates`, + ) + } + } + } + return true +}) + +const CurrentLockfileFacetEntry = type({ + source: LockfileSource, + version: LockedVersion, + integrity: 'string', + assets: CurrentLockfileAsset.array(), +}) + +// --- Versioned lockfile schemas (exact version dispatch, design D10) --- + +/** + * Legacy alpha (`1`) lockfile schema — the previous alpha shape with + * asset entries carrying identity only, pinned to exact numeric + * `lockfileVersion: 1`. Read-only during the compatibility window; normal + * installs migrate it to `0.2`, frozen installs retain it without rewriting. + */ +export const LegacyLockfileSchema = type({ + lockfileVersion: type.unit(LEGACY_LOCKFILE_VERSION), + facets: type.Record('string', LockfileFacetEntry), +}) + +/** Inferred TypeScript type for a validated legacy (`1`) lockfile */ +export type LegacyLockfile = typeof LegacyLockfileSchema.infer + +/** + * Current (`0.2`) lockfile schema: exact numeric `lockfileVersion: 0.2` and + * per-materialized-file integrity records inside every asset entry. + */ +export const CurrentLockfileSchema = type({ + lockfileVersion: type.unit(CURRENT_LOCKFILE_VERSION), + facets: type.Record('string', CurrentLockfileFacetEntry), +}) + +/** Inferred TypeScript type for a validated current (`0.2`) lockfile */ +export type CurrentLockfile = typeof CurrentLockfileSchema.infer + +/** Inferred type for a current facet entry inside a `0.2` lockfile */ +export type CurrentLockfileFacet = typeof CurrentLockfileFacetEntry.infer + +/** Inferred type for a current asset entry with its file-integrity records */ +export type CurrentLockfileAssetEntry = typeof CurrentLockfileAsset.infer + +/** Inferred type for one materialized-file integrity record */ +export type LockfileFileRecord = typeof LockfileAssetFileRecord.infer + /** * Schema for facets.lock — the adapter-agnostic lockfile recording * resolved facet installation state. @@ -113,6 +241,12 @@ const LockfileFacetEntry = type({ * Drift-proof deletion: OLD asset set comes from `facets[name].assets`; * NEW comes from the freshly-extracted artifact's build-manifest; * `to-delete` = OLD \ NEW. No separate cache required. + * + * @deprecated Transitional: this permissive shape (unpinned + * `lockfileVersion`, identity-only assets) predates exact version dispatch. + * Engine's loader migrates to `parseLockfileDocument` / + * `LegacyLockfileSchema` / `CurrentLockfileSchema` in the lockfile + * migration block of the `0.2` rollout, after which this export is removed. */ export const LockfileSchema = type({ lockfileVersion: 'number',