diff --git a/openspec/changes/support-non-asset-files/tasks.md b/openspec/changes/support-non-asset-files/tasks.md index f584f20a..540bc829 100644 --- a/openspec/changes/support-non-asset-files/tasks.md +++ b/openspec/changes/support-non-asset-files/tasks.md @@ -87,15 +87,15 @@ - [x] 9.1 Implement: Replace numeric-order lockfile handling with exact legacy-alpha-`1` and current-`0.2` loading, normal-mode migration, and frozen-mode no-rewrite behavior - [x] 9.2 Implement: Derive sorted lockfile asset file records from the verified materialization subset and recomputed entry hashes rather than copying self-declared hash values -- [ ] 9.3 Implement: Enforce pre-materialization agreement among facet integrity, asset identities, complete owned path sets, recomputed entry hashes, and verified build-manifest hashes with path-specific result variants, running the adapter-compatibility preflight (positional `0.0` rejected by a `{0.1}` CLI) ahead of archive-version dispatch and per-file reconciliation -- [ ] 9.4 Implement: Introduce receipt `0.2` asset/file ownership, safe legacy refinement, project-isolated bootstrap, and containment validation that treats receipt data as untrusted -- [ ] 9.5 Implement: Commit lockfile, receipt, and adapter state transactionally and ensure frozen consistency gates complete before receipt-driven cleanup begins -- [ ] 9.6 Implement: Materialize only primary assets and owned skill companions through tagged adapter requests carrying validated ownership sets from the lockfile and receipt, with per-file skip/repair behavior and rollback journal preimages -- [ ] 9.7 Implement: Make drift and removal path-specific, preserve unowned files, and support offline multi-file cleanup from receipts without cache or network access -- [ ] 9.8 Implement: Render lockfile, archive-version, per-file mismatch, adapter-bundle, and receipt failures exhaustively in CLI install output using one compatibility table for known format transitions -- [ ] 9.9 Implement: Add engine and CLI tests for migration, frozen failures, receipt corruption/isolation, pulled-lockfile cleanup, per-file drift, integrity mismatch, rollback, archive-only withholding, and exact diagnostics -- [ ] 9.10 Implement: Add a full-cycle end-to-end test that builds and verifies a facet with skill companions and archive-only files, installs it, detects and repairs single-file drift, exercises interrupted-install convergence on re-run without deleting unowned files, and removes it offline from the receipt, then exercises the same install path with an immutable legacy `0.1` archive -- [ ] 9.11 Verify: Run focused install, materialization, receipt, lockfile, cache, registry, and CLI install tests +- [x] 9.3 Implement: Enforce pre-materialization agreement among facet integrity, asset identities, complete owned path sets, recomputed entry hashes, and verified build-manifest hashes with path-specific result variants, running the adapter-compatibility preflight (positional `0.0` rejected by a `{0.1}` CLI) ahead of archive-version dispatch and per-file reconciliation +- [x] 9.4 Implement: Introduce receipt `0.2` asset/file ownership, safe legacy refinement, project-isolated bootstrap, and containment validation that treats receipt data as untrusted +- [x] 9.5 Implement: Commit lockfile, receipt, and adapter state transactionally and ensure frozen consistency gates complete before receipt-driven cleanup begins +- [x] 9.6 Implement: Materialize only primary assets and owned skill companions through tagged adapter requests carrying validated ownership sets from the lockfile and receipt, with per-file skip/repair behavior and rollback journal preimages +- [x] 9.7 Implement: Make drift and removal path-specific, preserve unowned files, and support offline multi-file cleanup from receipts without cache or network access +- [x] 9.8 Implement: Render lockfile, archive-version, per-file mismatch, adapter-bundle, and receipt failures exhaustively in CLI install output using one compatibility table for known format transitions +- [x] 9.9 Implement: Add engine and CLI tests for migration, frozen failures, receipt corruption/isolation, pulled-lockfile cleanup, per-file drift, integrity mismatch, rollback, archive-only withholding, and exact diagnostics +- [x] 9.10 Implement: Add a full-cycle end-to-end test that builds and verifies a facet with skill companions and archive-only files, installs it, detects and repairs single-file drift, exercises interrupted-install convergence on re-run without deleting unowned files, and removes it offline from the receipt, then exercises the same install path with an immutable legacy `0.1` archive +- [x] 9.11 Verify: Run focused install, materialization, receipt, lockfile, cache, registry, and CLI install tests ## 10. Current Producer and Build Pipeline — Research diff --git a/packages/cli/src/__tests__/install-view.test.tsx b/packages/cli/src/__tests__/install-view.test.tsx index e12cd1b3..f7485b68 100644 --- a/packages/cli/src/__tests__/install-view.test.tsx +++ b/packages/cli/src/__tests__/install-view.test.tsx @@ -519,6 +519,36 @@ describe('InstallView — integrity failure', () => { expect(frame).toContain('No assets were written') instance.unmount() }) + + // 9.8: pre-materialization per-file reconciliation failures render with the + // exact drifting path and both hashes. + test('renders a per-file reconcile failure with the exact path', async () => { + const failure: RunInstallFailure = { + code: 'RECONCILE_PER_FILE_INTEGRITY', + facet: 'viper-plans', + asset: 'skill:planning', + path: 'skills/planning/references/api.md', + expected: `sha256:${'a'.repeat(64)}`, + actual: `sha256:${'b'.repeat(64)}`, + } + const events: StageEvent[] = [ + { kind: 'install-start', totalFacets: 1 }, + { kind: 'facet-start', facet: 'viper-plans', specifier: 'github:a/v' }, + { kind: 'facet-failure', facet: 'viper-plans', failure }, + ] + const result: RunInstallResult = { + ok: false, + failure, + rollback: { kind: 'not-needed', reason: 'test fixture' }, + } + const instance = render(createElement(InstallView, { mode: 'add', run: makeFakeRun(events, result) })) + await settle() + const frame = findContentFrame(instance.frames) + expect(frame).toContain('file integrity mismatch') + expect(frame).toContain('skills/planning/references/api.md') + expect(frame).toContain(failure.code === 'RECONCILE_PER_FILE_INTEGRITY' ? failure.expected : '') + instance.unmount() + }) }) describe('InstallView — parse error failure', () => { diff --git a/packages/cli/src/tui/views/install/facet-row.tsx b/packages/cli/src/tui/views/install/facet-row.tsx index 1a5c84e7..7e0c2d3d 100644 --- a/packages/cli/src/tui/views/install/facet-row.tsx +++ b/packages/cli/src/tui/views/install/facet-row.tsx @@ -170,6 +170,14 @@ function oneLineFailureSummary(failure: RunInstallFailure): string { return 'failed to load facet.json' case 'ADAPTER_INSTALL_FAILED': return `adapter ${failure.adapter} failed during materialization` + case 'RECONCILE_FACET_INTEGRITY': + return 'lockfile integrity mismatch' + case 'RECONCILE_ASSET_IDENTITY': + return 'lockfile asset set mismatch' + case 'RECONCILE_OWNED_PATH_SET': + return `lockfile file set mismatch (${failure.asset})` + case 'RECONCILE_PER_FILE_INTEGRITY': + return `file integrity mismatch: ${failure.path}` default: return 'install failed' } diff --git a/packages/cli/src/tui/views/install/failure-block.tsx b/packages/cli/src/tui/views/install/failure-block.tsx index 591f7f82..343b7e9f 100644 --- a/packages/cli/src/tui/views/install/failure-block.tsx +++ b/packages/cli/src/tui/views/install/failure-block.tsx @@ -365,6 +365,65 @@ export function FailureBlock({ failure }: { failure: RunInstallFailure }): React ) + case 'RECONCILE_FACET_INTEGRITY': + return ( + + + ✕ lockfile integrity mismatch for {failure.facet} + + expected {failure.expected} + actual {failure.actual} + + {' '} + The lockfile disagrees with the resolved content. Delete facets.lock and re-run. + + + ) + case 'RECONCILE_ASSET_IDENTITY': + return ( + + + ✕ lockfile asset set does not match resolved content for {failure.facet} + + {failure.missing.length > 0 && ( + locked but not resolved: {failure.missing.join(', ')} + )} + {failure.unexpected.length > 0 && ( + resolved but not locked: {failure.unexpected.join(', ')} + )} + Delete facets.lock and re-run, or `facet add` to update it. + + ) + case 'RECONCILE_OWNED_PATH_SET': + return ( + + + ✕ lockfile file set does not match resolved content for {failure.facet} ({failure.asset}) + + {failure.missing.length > 0 && ( + locked but not resolved: {failure.missing.join(', ')} + )} + {failure.unexpected.length > 0 && ( + resolved but not locked: {failure.unexpected.join(', ')} + )} + Delete facets.lock and re-run, or `facet add` to update it. + + ) + case 'RECONCILE_PER_FILE_INTEGRITY': + return ( + + + ✕ file integrity mismatch: {failure.path} + + + {' '} + in {failure.facet} ({failure.asset}) + + expected {failure.expected} + actual {failure.actual} + Delete facets.lock and re-run, or `facet add` to update it. + + ) default: { // Exhaustiveness guard: any new `RunInstallFailure` variant must // get a `case` arm above. Without this, an un-rendered failure diff --git a/packages/cli/src/util/__tests__/archive-compatibility.test.ts b/packages/cli/src/util/__tests__/archive-compatibility.test.ts new file mode 100644 index 00000000..b6e6be6b --- /dev/null +++ b/packages/cli/src/util/__tests__/archive-compatibility.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, test } from 'bun:test' +import { archiveCompatibilityGuidance } from '../archive-compatibility.ts' + +describe('archiveCompatibilityGuidance', () => { + test('names the minimum supporting release for a known newer format', () => { + const g = archiveCompatibilityGuidance('0.2', ['0.1']) + expect(g.what).toContain('archive format 0.2') + expect(g.detail).toContain('supported archive formats: 0.1') + // Known format → concrete minimum release, not a bare "update to latest". + expect(g.fix).toContain('0.2.0 or later') + }) + + test('advises updating to latest for an unknown future format without inventing a minimum', () => { + const g = archiveCompatibilityGuidance('0.9', ['0.1', '0.2']) + expect(g.what).toContain('archive format 0.9') + expect(g.fix).toContain('latest release') + // No fabricated minimum version for a format this CLI cannot know about. + expect(g.fix).not.toMatch(/\d+\.\d+\.\d+ or later/) + }) + + test('handles a missing/unparseable declared format', () => { + const g = archiveCompatibilityGuidance(undefined, ['0.1', '0.2']) + expect(g.what).toContain('does not recognize') + expect(g.detail).toContain('0.1, 0.2') + expect(g.fix).toContain('latest release') + }) +}) diff --git a/packages/cli/src/util/archive-compatibility.ts b/packages/cli/src/util/archive-compatibility.ts new file mode 100644 index 00000000..60439f45 --- /dev/null +++ b/packages/cli/src/util/archive-compatibility.ts @@ -0,0 +1,81 @@ +/** + * The single CLI-side compatibility table mapping a known archive format to + * the minimum `agent-facets` release that supports it (design D4, task 9.8). + * + * This is the ONE authoritative place the CLI turns an unsupported + * archive-format failure into actionable upgrade guidance. For a KNOWN newer + * format, it names the minimum supporting release ("update agent-facets to + * or later"). For an UNKNOWN future format, it advises updating to + * the latest release without inventing a minimum version — an already-shipped + * CLI cannot know the minimum release for a format defined after it was built. + * + * The table intentionally lives in the CLI, not the protocol: it maps a spec + * artifact version to a specific npm package release, which is a distribution + * fact this CLI owns, not a normative part of the protocol. + */ + +/** + * Known archive-format → minimum supporting `agent-facets` release. Keys are + * the exact numeric `facetVersion` values rendered as strings (the form the + * verifier reports as `observed`). Extend this map when a future format ships + * with a known minimum CLI release. + * + * `0.1` and `0.2` are the formats THIS CLI already supports, so they never + * reach the unsupported-format path; the table is for formats NEWER than what + * a given installed CLI understands. It is seeded with the `0.2` boundary so + * an older pre-`0.2` CLI (which lacks this table entirely) is the only build + * that shows a generic message — every `0.2`-aware release maps known newer + * formats precisely as they are added. + */ +const MINIMUM_RELEASE_FOR_FORMAT: Readonly> = { + // The first release that emits/consumes the `0.2` archive format. Present so + // the mapping mechanism is exercised and documented; a CLI that supports + // `0.2` will not itself render `0.2` as unsupported. + '0.2': '0.2.0', +} + +export interface ArchiveCompatibilityGuidance { + /** The one-line "what went wrong" summary. */ + what: string + /** Supporting detail (supported formats, and a minimum release when known). */ + detail: string + /** The actionable fix line. */ + fix: string +} + +/** + * Render actionable upgrade guidance for an unsupported archive format. + * + * @param observed the archive's declared `facetVersion` (string), or + * `undefined` when the archive did not declare a parseable + * version. + * @param supported the archive formats this CLI supports. + */ +export function archiveCompatibilityGuidance( + observed: string | undefined, + supported: readonly string[], +): ArchiveCompatibilityGuidance { + if (observed === undefined) { + return { + what: 'this facet uses an archive format this CLI does not recognize', + detail: `supported archive formats: ${supported.join(', ')}`, + fix: 'update agent-facets to the latest release with `facet self-update` and try again', + } + } + + const minimumRelease = MINIMUM_RELEASE_FOR_FORMAT[observed] + if (minimumRelease !== undefined) { + return { + what: `this facet uses archive format ${observed}, which this CLI does not support`, + detail: `supported archive formats: ${supported.join(', ')}`, + fix: `update agent-facets to ${minimumRelease} or later (e.g. \`facet self-update\`) and try again`, + } + } + + // Unknown future format: no minimum is invented. + return { + what: `this facet uses archive format ${observed}, which this CLI does not support`, + detail: `supported archive formats: ${supported.join(', ')}`, + fix: 'update agent-facets to the latest release with `facet self-update` and try again', + } +} diff --git a/packages/cli/src/util/registry-errors.ts b/packages/cli/src/util/registry-errors.ts index 019de22f..d5bed2a2 100644 --- a/packages/cli/src/util/registry-errors.ts +++ b/packages/cli/src/util/registry-errors.ts @@ -1,4 +1,5 @@ import type { RegistryError } from '@agent-facets/engine' +import { archiveCompatibilityGuidance } from './archive-compatibility.ts' import type { CliError } from './errors.ts' /** @@ -56,16 +57,12 @@ export function translateEngineRegistryError(err: RegistryError): CliError { fix: 'try again; if persistent, file a bug', } case 'UNSUPPORTED_ARCHIVE': - // Basic upgrade guidance for now; the full facet-format → - // minimum-CLI-release compatibility table lands with the install - // failure-rendering work. - return { - what: - err.observed === undefined - ? 'this facet uses an archive format this CLI does not recognize' - : `this facet uses archive format ${err.observed}, which this CLI does not support`, - detail: `supported archive formats: ${err.supported.join(', ')}`, - fix: 'update agent-facets with `facet self-update` and try again', - } + // The single compatibility table names the minimum supporting release + // for a known newer format, or advises updating to latest for an + // unknown future one (design D4, task 9.8). + return archiveCompatibilityGuidance( + err.observed === undefined ? undefined : String(err.observed), + err.supported.map(String), + ) } } diff --git a/packages/engine/src/__tests__/run-install.test.ts b/packages/engine/src/__tests__/run-install.test.ts index cc713bbe..d02c156d 100644 --- a/packages/engine/src/__tests__/run-install.test.ts +++ b/packages/engine/src/__tests__/run-install.test.ts @@ -3,7 +3,15 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, import { tmpdir } from 'node:os' import { join } from 'node:path' import type { Adapter } from '@agent-facets/adapter' -import { ADAPTER_API_VERSION, deleteAssetFile, installAssetFile, readAssetFile } from '@agent-facets/adapter' +import { + ADAPTER_API_VERSION, + deleteAssetFile, + deleteSkillBundle, + installAssetFile, + installSkillBundle, + readAssetFile, + readSkillBundle, +} from '@agent-facets/adapter' import type { BuildManifest, Lockfile } from '@agent-facets/protocol' import { CURRENT_LOCKFILE_VERSION, CurrentLockfileSchema, computeContentHash } from '@agent-facets/protocol' import { type } from 'arktype' @@ -328,6 +336,43 @@ describe('runInstall — local source success path', () => { { path: 'skills/planning/SKILL.md', integrity: expect.stringMatching(/^sha256:[a-f0-9]{64}$/) }, ]) }) + + // 9.3: pre-materialization reconciliation. A locked per-file hash that no + // longer matches the freshly-derived plan aborts before any adapter write + // with a path-specific failure. + test('a tampered locked per-file hash aborts with RECONCILE_PER_FILE_INTEGRITY', async () => { + const local = buildLocalFixture('viper-plans') + const relPath = `./${local.split('/').pop()}` + writeFileSync(join(projectRoot, 'facets.json'), JSON.stringify({ facets: { 'viper-plans': relPath } })) + + // First install writes a valid 0.2 lockfile. + const first = await runInstall({ projectRoot, adapters: [buildFakeAdapter('test')] }) + if (!first.ok) expect.unreachable() + + // Tamper the locked per-file integrity for the skill's SKILL.md so the + // freshly-derived plan disagrees on re-install. + const lockPath = join(projectRoot, 'facets.lock') + const lock = JSON.parse(readFileSync(lockPath, 'utf8')) + const wrong = `sha256:${'1'.repeat(64)}` + lock.facets['viper-plans'].assets[0].files[0].integrity = wrong + writeFileSync(lockPath, JSON.stringify(lock)) + + const second = await runInstall({ projectRoot, adapters: [buildFakeAdapter('test')] }) + if (second.ok) expect.unreachable() + if (second.failure.code !== 'RECONCILE_PER_FILE_INTEGRITY') expect.unreachable() + expect(second.failure.facet).toBe('viper-plans') + expect(second.failure.asset).toBe('skill:planning') + expect(second.failure.path).toBe('skills/planning/SKILL.md') + expect(second.failure.expected).toBe(wrong) + expect(second.failure.actual).toMatch(/^sha256:[a-f0-9]{64}$/) + // Reconciliation runs before materialize, so nothing was written for + // this facet: rollback replays an empty journal (zero entries undone). + if (second.rollback.kind === 'not-needed') expect.unreachable() + expect(second.rollback.entriesUndone).toBe(0) + // The tampered lockfile is left unchanged on disk (no tri-write ran). + const after = JSON.parse(readFileSync(join(projectRoot, 'facets.lock'), 'utf8')) + expect(after.facets['viper-plans'].assets[0].files[0].integrity).toBe(wrong) + }) }) describe('runInstall — registry source surfaces REGISTRY_ERROR on resolution failure', () => { @@ -974,3 +1019,263 @@ describe('runInstall — git cache hit short-circuits clone', () => { expect(result.failure.lockedIntegrity).toBe(wrongIntegrity) }) }) + +// 9.6: real skill-bundle materialization — companions install, skip when +// identical, and repair per-file drift. Uses the SDK bundle helpers so the +// companion round-trip goes through the same code every real adapter uses. +describe('runInstall — multi-file skill materialization', () => { + function buildBundleFixture(name: string, version = '0.1.0'): string { + const repo = realpathSync(mkdtempSync(join(projectRoot, 'bundle-fixture-'))) + writeFileSync( + join(repo, 'facet.json'), + JSON.stringify({ + name, + version, + skills: { planning: { description: 'planning skill', files: ['references/api.md', 'assets/logo.bin'] } }, + }), + ) + mkdirSync(join(repo, 'skills/planning/references'), { recursive: true }) + mkdirSync(join(repo, 'skills/planning/assets'), { recursive: true }) + writeFileSync(join(repo, 'skills/planning/SKILL.md'), `# planning ${version}\n`) + writeFileSync(join(repo, 'skills/planning/references/api.md'), '# api reference\n') + // A binary companion — must survive byte-for-byte. + writeFileSync(join(repo, 'skills/planning/assets/logo.bin'), Buffer.from([0, 1, 2, 253, 254, 255])) + return repo + } + + /** Fake adapter that stores skills as bundles via the SDK helpers. */ + function buildBundleAdapter(name: string): Adapter { + const baseDir = join(projectRoot, `.${name}`) + const skillPaths = (n: string) => ({ + root: join(baseDir, 'skills', n), + primaryFile: join(baseDir, 'skills', n, 'SKILL.md'), + pruneBoundary: baseDir, + }) + const flatPath = (type: string, n: string) => ({ file: join(baseDir, `${type}s`, `${n}.md`) }) + return { + name, + apiVersion: ADAPTER_API_VERSION, + supportsInstall: true, + buildAssetMetadata: (data) => ({ ok: true, data: (data ?? {}) as Record }), + async installAsset(request) { + if (request.assetType === 'skill') { + return installSkillBundle(skillPaths(request.name), { + content: request.content, + metadata: request.metadata as Record | undefined, + companions: request.companions, + ownedCompanionPaths: request.ownedCompanionPaths, + }) + } + const p = flatPath(request.assetType, request.name) + await installAssetFile(p, request.content, request.metadata as Record | undefined) + return { ok: true, primaryPath: p.file } + }, + async readAsset(request) { + if (request.assetType === 'skill') { + return readSkillBundle(skillPaths(request.name), request.ownedCompanionPaths) + } + try { + const { content, metadata } = await readAssetFile(flatPath(request.assetType, request.name)) + return { ok: true, asset: { assetType: request.assetType, content, metadata } } + } catch { + return { ok: false, failure: { code: 'not-found' } } + } + }, + async deleteAsset(request) { + if (request.assetType === 'skill') { + return deleteSkillBundle(skillPaths(request.name), request.ownedCompanionPaths) + } + const p = flatPath(request.assetType, request.name) + await deleteAssetFile(p) + return { ok: true, existed: true, deletedPaths: [p.file] } + }, + } + } + + test('installs a skill with companions and records their owned paths', async () => { + const local = buildBundleFixture('viper-plans') + writeFileSync( + join(projectRoot, 'facets.json'), + JSON.stringify({ facets: { 'viper-plans': `./${local.split('/').pop()}` } }), + ) + + const result = await runInstall({ projectRoot, adapters: [buildBundleAdapter('bundle')] }) + if (!result.ok) expect.unreachable() + + // Both companions materialized, binary preserved byte-for-byte. + const companionDir = join(projectRoot, '.bundle/skills/planning') + expect(readFileSync(join(companionDir, 'references/api.md'), 'utf8')).toBe('# api reference\n') + expect([...readFileSync(join(companionDir, 'assets/logo.bin'))]).toEqual([0, 1, 2, 253, 254, 255]) + + // The lockfile records all three owned files with recomputed hashes. + const written = CurrentLockfileSchema(JSON.parse(readFileSync(join(projectRoot, 'facets.lock'), 'utf8'))) + if (written instanceof type.errors) expect.unreachable() + const asset = written.facets['viper-plans']?.assets[0] + expect(asset?.files.map((f) => f.path)).toEqual([ + 'skills/planning/SKILL.md', + 'skills/planning/assets/logo.bin', + 'skills/planning/references/api.md', + ]) + }) + + test('reinstall skips an unchanged bundle, then repairs a single drifted companion', async () => { + const local = buildBundleFixture('viper-plans') + writeFileSync( + join(projectRoot, 'facets.json'), + JSON.stringify({ facets: { 'viper-plans': `./${local.split('/').pop()}` } }), + ) + const adapters = [buildBundleAdapter('bundle')] + + const first = await runInstall({ projectRoot, adapters }) + if (!first.ok) expect.unreachable() + expect(first.summary.totalAssets).toBe(1) + + // Second install with no changes: the whole bundle is identical → skipped. + const second = await runInstall({ projectRoot, adapters }) + if (!second.ok) expect.unreachable() + expect(second.summary.totalAssets).toBe(0) + + // Drift a single companion on disk, then reinstall: the bundle is + // repaired (one write) and the drifted file is restored from source. The + // verbose log names the exact drifted path (path-specific drift, 9.7). + const apiPath = join(projectRoot, '.bundle/skills/planning/references/api.md') + writeFileSync(apiPath, '# TAMPERED\n') + const logs: string[] = [] + const third = await runInstall({ projectRoot, adapters, onLog: (b) => logs.push(b()) }) + if (!third.ok) expect.unreachable() + expect(third.summary.totalAssets).toBe(1) + expect(readFileSync(apiPath, 'utf8')).toBe('# api reference\n') + expect(logs.some((l) => l.includes('drift: skills/planning/references/api.md'))).toBe(true) + }) + + test('an unowned file in the skill directory survives an update', async () => { + const local = buildBundleFixture('viper-plans') + writeFileSync( + join(projectRoot, 'facets.json'), + JSON.stringify({ facets: { 'viper-plans': `./${local.split('/').pop()}` } }), + ) + const adapters = [buildBundleAdapter('bundle')] + + const first = await runInstall({ projectRoot, adapters }) + if (!first.ok) expect.unreachable() + + // A user drops an unowned note into the skill dir. + const notePath = join(projectRoot, '.bundle/skills/planning/notes.txt') + writeFileSync(notePath, 'my notes\n') + + // Change the primary so a reinstall re-writes the bundle. + writeFileSync(join(local, 'skills/planning/SKILL.md'), '# planning edited\n') + const second = await runInstall({ projectRoot, adapters }) + if (!second.ok) expect.unreachable() + + // The unowned note is untouched by the owned-set replacement. + expect(readFileSync(notePath, 'utf8')).toBe('my notes\n') + }) + + // 9.7: offline multi-file cleanup from the receipt. After install, removing + // the facet (empty manifest) deletes the primary AND every owned companion + // using only the receipt's recorded owned paths — no cache, no network — + // while preserving unowned files. + test('removal deletes owned companions offline from the receipt, preserving unowned files', async () => { + const local = buildBundleFixture('viper-plans') + writeFileSync( + join(projectRoot, 'facets.json'), + JSON.stringify({ facets: { 'viper-plans': `./${local.split('/').pop()}` } }), + ) + const adapters = [buildBundleAdapter('bundle')] + + const first = await runInstall({ projectRoot, adapters }) + if (!first.ok) expect.unreachable() + + const skillDir = join(projectRoot, '.bundle/skills/planning') + const apiPath = join(skillDir, 'references/api.md') + const logoPath = join(skillDir, 'assets/logo.bin') + const notePath = join(skillDir, 'notes.txt') + expect(existsSync(apiPath)).toBe(true) + expect(existsSync(logoPath)).toBe(true) + // A user file that the receipt does not own. + writeFileSync(notePath, 'keep me\n') + + // Delete the source fixture so nothing can be re-derived from it, and + // empty the manifest to trigger receipt-driven removal. + rmSync(local, { recursive: true, force: true }) + writeFileSync(join(projectRoot, 'facets.json'), JSON.stringify({ facets: {} })) + + const removed = await runInstall({ projectRoot, adapters }) + if (!removed.ok) expect.unreachable() + + // Primary + both owned companions removed; the unowned note survives. + expect(existsSync(join(skillDir, 'SKILL.md'))).toBe(false) + expect(existsSync(apiPath)).toBe(false) + expect(existsSync(logoPath)).toBe(false) + expect(readFileSync(notePath, 'utf8')).toBe('keep me\n') + }) + + // 9.9: archive-only supplementary files (e.g. a root README.md) ship in the + // verified archive and are pinned by facet integrity, but are NEVER + // materialized to an adapter and NEVER recorded as a lockfile asset. + test('an archive-only README is verified but never materialized or locked', async () => { + const repo = realpathSync(mkdtempSync(join(projectRoot, 'archiveonly-fixture-'))) + writeFileSync( + join(repo, 'facet.json'), + JSON.stringify({ + name: 'viper-plans', + version: '0.1.0', + skills: { planning: { description: 'planning skill' } }, + files: ['README.md'], + }), + ) + mkdirSync(join(repo, 'skills/planning'), { recursive: true }) + writeFileSync(join(repo, 'skills/planning/SKILL.md'), '# planning\n') + writeFileSync(join(repo, 'README.md'), '# my facet\n') + writeFileSync( + join(projectRoot, 'facets.json'), + JSON.stringify({ facets: { 'viper-plans': `./${repo.split('/').pop()}` } }), + ) + + const result = await runInstall({ projectRoot, adapters: [buildBundleAdapter('bundle')] }) + if (!result.ok) expect.unreachable() + + // README is NOT written into the adapter tree anywhere. + expect(existsSync(join(projectRoot, '.bundle/README.md'))).toBe(false) + expect(existsSync(join(projectRoot, '.bundle/skills/planning/README.md'))).toBe(false) + + // No lockfile asset lists README.md among its files. + const written = CurrentLockfileSchema(JSON.parse(readFileSync(join(projectRoot, 'facets.lock'), 'utf8'))) + if (written instanceof type.errors) expect.unreachable() + for (const asset of written.facets['viper-plans']?.assets ?? []) { + expect(asset.files.some((f) => f.path === 'README.md')).toBe(false) + } + }) + + // 9.10: interrupted-install convergence. A crash can leave a partial bundle + // (primary present, a companion missing). Re-running install compares the + // on-disk bundle to the source and repairs it, converging without deleting + // any unowned file. + test('re-running install converges a partially-materialized bundle without deleting unowned files', async () => { + const local = buildBundleFixture('viper-plans') + writeFileSync( + join(projectRoot, 'facets.json'), + JSON.stringify({ facets: { 'viper-plans': `./${local.split('/').pop()}` } }), + ) + const adapters = [buildBundleAdapter('bundle')] + + const first = await runInstall({ projectRoot, adapters }) + if (!first.ok) expect.unreachable() + + const skillDir = join(projectRoot, '.bundle/skills/planning') + // Simulate an interrupted install: a companion is missing on disk, and an + // unowned user file is present. + rmSync(join(skillDir, 'references/api.md'), { force: true }) + const notePath = join(skillDir, 'notes.txt') + writeFileSync(notePath, 'keep me\n') + + // Re-run converges: the missing companion is restored, the unowned file + // survives, and the install reports the bundle as repaired (one write). + const second = await runInstall({ projectRoot, adapters }) + if (!second.ok) expect.unreachable() + expect(second.summary.totalAssets).toBe(1) + expect(readFileSync(join(skillDir, 'references/api.md'), 'utf8')).toBe('# api reference\n') + expect(readFileSync(notePath, 'utf8')).toBe('keep me\n') + }) +}) diff --git a/packages/engine/src/install/__tests__/receipt.test.ts b/packages/engine/src/install/__tests__/receipt.test.ts index 08fc44f0..4faf13e2 100644 --- a/packages/engine/src/install/__tests__/receipt.test.ts +++ b/packages/engine/src/install/__tests__/receipt.test.ts @@ -3,8 +3,30 @@ import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'nod import { tmpdir } from 'node:os' import { join } from 'node:path' import type { Lockfile } from '@agent-facets/protocol' -import { LOCKFILE_VERSION } from '@agent-facets/protocol' -import { bootstrapReceipt, loadReceipt, type Receipt, receiptPath, writeReceipt } from '../receipt.ts' +import { CURRENT_LOCKFILE_VERSION, LEGACY_LOCKFILE_VERSION } from '@agent-facets/protocol' +import { + bootstrapReceipt, + CURRENT_RECEIPT_VERSION, + loadReceipt, + type Receipt, + receiptPath, + writeReceipt, +} from '../receipt.ts' + +/** A current (0.2) receipt skill asset owning exactly its SKILL.md. */ +function skillAsset(name: string): { scope: 'project'; type: 'skill'; name: string; files: string[] } { + return { scope: 'project', type: 'skill', name, files: [`skills/${name}/SKILL.md`] } +} + +/** A current (0.2) receipt agent asset owning exactly its primary file. */ +function agentAsset(name: string): { scope: 'project'; type: 'agent'; name: string; files: string[] } { + return { scope: 'project', type: 'agent', name, files: [`agents/${name}.md`] } +} + +/** A current (0.2) receipt command asset owning exactly its primary file. */ +function commandAsset(name: string): { scope: 'project'; type: 'command'; name: string; files: string[] } { + return { scope: 'project', type: 'command', name, files: [`commands/${name}.md`] } +} let facetDir: string let projectDir: string @@ -86,7 +108,7 @@ describe('loadReceipt', () => { test('returns path-mismatch when embedded path differs', () => { const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: '/some/other/project', facets: {}, } @@ -105,19 +127,16 @@ describe('loadReceipt', () => { // reported — never acted on — while everything else loads normally // (W2: per-entry extraction, not whole-receipt rejection). const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: realpathSync(projectDir), facets: { cowsay: { version: '0.0.1', - assets: [ - { scope: 'project', type: 'skill', name: '../escape' }, - { scope: 'project', type: 'skill', name: 'cowsay' }, - ], + assets: [{ ...skillAsset('escape'), name: '../escape' }, skillAsset('cowsay')], }, hello: { version: '1.0.0', - assets: [{ scope: 'project', type: 'agent', name: 'greeter' }], + assets: [agentAsset('greeter')], }, }, } @@ -132,18 +151,72 @@ describe('loadReceipt', () => { expect(result.invalidEntries[0]?.asset).toBe('../escape') expect(result.invalidEntries[0]?.reason.length).toBeGreaterThan(0) // The valid sibling asset and the untouched facet still load. - expect(result.receipt.facets.cowsay?.assets).toEqual([{ scope: 'project', type: 'skill', name: 'cowsay' }]) + expect(result.receipt.facets.cowsay?.assets).toEqual([skillAsset('cowsay')]) expect(result.receipt.facets.hello?.assets).toHaveLength(1) }) + test('drops an asset whose owned file path escapes, reporting it', () => { + // Untrusted-input containment: a crafted owned PATH (not name) must drop + // the whole asset record — never delete an escaping path — while valid + // siblings still load. + const receipt: Receipt = { + version: CURRENT_RECEIPT_VERSION, + path: realpathSync(projectDir), + facets: { + cowsay: { + version: '0.0.1', + assets: [ + { scope: 'project', type: 'skill', name: 'cowsay', files: ['skills/cowsay/../../escape.md'] }, + skillAsset('safe'), + ], + }, + }, + } + const path = receiptPath(projectDir) + mkdirSync(join(facetDir, 'receipts'), { recursive: true }) + writeFileSync(path, JSON.stringify(receipt)) + const result = loadReceipt(projectDir) + if (!result.ok) expect.unreachable() + expect(result.invalidEntries).toHaveLength(1) + expect(result.invalidEntries[0]?.asset).toBe('cowsay') + expect(result.invalidEntries[0]?.reason).toContain('owned path') + expect(result.receipt.facets.cowsay?.assets).toEqual([skillAsset('safe')]) + }) + + test('refines a legacy (1) receipt to primary-only ownership', () => { + // Legacy installs could not materialize companions, so a legacy + // identity-only receipt is refined to the single conventional primary + // path per asset and loads as current. + const legacy = { + version: LEGACY_LOCKFILE_VERSION, + path: realpathSync(projectDir), + facets: { + cowsay: { + version: '0.0.1', + assets: [ + { scope: 'project', type: 'skill', name: 'cowsay' }, + { scope: 'project', type: 'command', name: 'moo' }, + ], + }, + }, + } + const path = receiptPath(projectDir) + mkdirSync(join(facetDir, 'receipts'), { recursive: true }) + writeFileSync(path, JSON.stringify(legacy)) + const result = loadReceipt(projectDir) + if (!result.ok) expect.unreachable() + expect(result.receipt.version).toBe(CURRENT_RECEIPT_VERSION) + expect(result.receipt.facets.cowsay?.assets).toEqual([skillAsset('cowsay'), commandAsset('moo')]) + }) + test('a fully valid receipt reports no invalid entries', () => { const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: realpathSync(projectDir), facets: { cowsay: { version: '0.0.1', - assets: [{ scope: 'project', type: 'skill', name: 'cowsay' }], + assets: [skillAsset('cowsay')], }, }, } @@ -155,12 +228,12 @@ describe('loadReceipt', () => { test('loads a valid receipt successfully', () => { const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: realpathSync(projectDir), facets: { cowsay: { version: '0.0.1', - assets: [{ scope: 'project', type: 'skill', name: 'cowsay' }], + assets: [skillAsset('cowsay')], }, }, } @@ -188,7 +261,7 @@ describe('loadReceipt', () => { describe('writeReceipt', () => { test('creates the receipts directory if it does not exist', () => { const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: realpathSync(projectDir), facets: {}, } @@ -200,19 +273,16 @@ describe('writeReceipt', () => { test('round-trips a receipt with multiple facets', () => { const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: realpathSync(projectDir), facets: { cowsay: { version: '0.0.1', - assets: [ - { scope: 'project', type: 'skill', name: 'cowsay' }, - { scope: 'project', type: 'command', name: 'moo' }, - ], + assets: [skillAsset('cowsay'), commandAsset('moo')], }, hello: { version: '1.0.0', - assets: [{ scope: 'project', type: 'agent', name: 'greeter' }], + assets: [agentAsset('greeter')], }, }, } @@ -228,12 +298,12 @@ describe('writeReceipt', () => { test('normalizes receipt.path so a stale path does not cause path-mismatch (#20)', () => { const canonical = realpathSync(projectDir) const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: '/some/other/path', // intentionally wrong facets: { cowsay: { version: '0.0.1', - assets: [{ scope: 'project', type: 'skill', name: 'cowsay' }], + assets: [skillAsset('cowsay')], }, }, } @@ -250,9 +320,9 @@ describe('writeReceipt', () => { // --------------------------------------------------------------------------- describe('bootstrapReceipt', () => { - test('creates a receipt from a lockfile', () => { + test('creates a current (0.2) receipt from a legacy lockfile, primary-only', () => { const lockfile: Lockfile = { - lockfileVersion: LOCKFILE_VERSION, + lockfileVersion: LEGACY_LOCKFILE_VERSION, facets: { cowsay: { source: { kind: 'registry', registry: 'https://api.agentfacets.io' }, @@ -266,15 +336,46 @@ describe('bootstrapReceipt', () => { }, } const receipt = bootstrapReceipt(projectDir, lockfile) - expect(receipt.version).toBe(1) + expect(receipt.version).toBe(CURRENT_RECEIPT_VERSION) expect(receipt.path).toBe(realpathSync(projectDir)) expect(receipt.facets.cowsay?.version).toBe('0.0.1') - expect(receipt.facets.cowsay?.assets).toHaveLength(2) + // Legacy identity-only assets seed their single conventional primary path. + expect(receipt.facets.cowsay?.assets).toEqual([skillAsset('cowsay'), commandAsset('moo')]) + }) + + test('mirrors owned companion paths from a 0.2 lockfile', () => { + const lockfile: Lockfile = { + lockfileVersion: CURRENT_LOCKFILE_VERSION, + facets: { + cowsay: { + source: { kind: 'registry', registry: 'https://api.agentfacets.io' }, + version: '0.0.1', + integrity: 'sha256:abc', + assets: [ + { + scope: 'project', + type: 'skill', + name: 'cowsay', + files: [ + { path: 'skills/cowsay/SKILL.md', integrity: `sha256:${'0'.repeat(64)}` }, + { path: 'skills/cowsay/references/art.md', integrity: `sha256:${'1'.repeat(64)}` }, + ], + }, + ] as unknown as Lockfile['facets'][string]['assets'], + }, + }, + } + const receipt = bootstrapReceipt(projectDir, lockfile) + // Receipt mirrors the owned PATHS (no hashes). + expect(receipt.facets.cowsay?.assets[0]?.files).toEqual([ + 'skills/cowsay/SKILL.md', + 'skills/cowsay/references/art.md', + ]) }) test('empty lockfile produces empty receipt', () => { const lockfile: Lockfile = { - lockfileVersion: LOCKFILE_VERSION, + lockfileVersion: CURRENT_LOCKFILE_VERSION, facets: {}, } const receipt = bootstrapReceipt(projectDir, lockfile) @@ -283,7 +384,7 @@ describe('bootstrapReceipt', () => { test('strips source and integrity from lockfile entries', () => { const lockfile: Lockfile = { - lockfileVersion: LOCKFILE_VERSION, + lockfileVersion: LEGACY_LOCKFILE_VERSION, facets: { cowsay: { source: { kind: 'git', url: 'https://github.com/test/cowsay', commit: 'abc123' }, diff --git a/packages/engine/src/install/__tests__/run-install.receipt.test.ts b/packages/engine/src/install/__tests__/run-install.receipt.test.ts index 7bc32833..b61e46f5 100644 --- a/packages/engine/src/install/__tests__/run-install.receipt.test.ts +++ b/packages/engine/src/install/__tests__/run-install.receipt.test.ts @@ -4,7 +4,7 @@ import { tmpdir } from 'node:os' import { join } from 'node:path' import { ADAPTER_API_VERSION } from '@agent-facets/adapter/api-version' import type { BuildManifest } from '@agent-facets/protocol' -import type { Receipt } from '../receipt.ts' +import { CURRENT_RECEIPT_VERSION, type Receipt } from '../receipt.ts' import type { Addition, StageEvent } from '../types.ts' /** @@ -212,7 +212,7 @@ describe('runInstall — frozen orphan-on-pull cleanup', () => { const receipt = JSON.parse(readFileSync(receiptFile, 'utf8')) as Receipt receipt.facets.ghost = { version: '1.0.0', - assets: [{ scope: 'project', type: 'skill', name: 'ghostly' }], + assets: [{ scope: 'project', type: 'skill', name: 'ghostly', files: ['skills/ghostly/SKILL.md'] }], } writeReceipt(projectRoot, receipt) mkdirSync(join(projectRoot, '.test-adapter/skills'), { recursive: true }) @@ -236,6 +236,35 @@ describe('runInstall — frozen orphan-on-pull cleanup', () => { expect(readFileSync(join(projectRoot, 'facets.json'), 'utf8')).toBe(facetsBefore) expect(readFileSync(join(projectRoot, 'facets.lock'), 'utf8')).toBe(lockBefore) }) + + // 9.5: the frozen consistency gate completes before receipt-driven cleanup. + // A lockfile ORPHAN (pinned in the lockfile, absent from facets.json) is + // surfaced as drift and rejected BEFORE any materialized asset is deleted. + test('a frozen lockfile orphan fails before cleanup deletes anything', async () => { + await seedInstalledProject() // cowsay in all three files, asset on disk + + // Simulate an orphan: drop cowsay from facets.json only. The lockfile + // still pins it — a frozen install must reject this as LOCKFILE_DRIFT + // (reason 'orphaned') before the drift-removal loop runs. + writeFileSync(join(projectRoot, 'facets.json'), `${JSON.stringify({ facets: {} }, null, 2)}\n`) + const assetPath = join(projectRoot, '.test-adapter/skills/planning.md') + expect(existsSync(assetPath)).toBe(true) + const lockBefore = readFileSync(join(projectRoot, 'facets.lock'), 'utf8') + const receiptFile = receiptPath(projectRoot) + const receiptBefore = readFileSync(receiptFile, 'utf8') + + metadataOffline = true + const result = await install({ frozen: true }) + + if (result.ok) expect.unreachable() + if (result.failure.code !== 'LOCKFILE_DRIFT') expect.unreachable() + expect(result.failure.facets.some((f) => f.name === 'cowsay' && f.reason === 'orphaned')).toBe(true) + // No cleanup ran: the materialized asset and both files are untouched. + expect(existsSync(assetPath)).toBe(true) + expect(readFileSync(join(projectRoot, 'facets.lock'), 'utf8')).toBe(lockBefore) + expect(readFileSync(receiptFile, 'utf8')).toBe(receiptBefore) + expect(result.rollback.kind).toBe('not-needed') + }) }) // --- W2: per-entry escape handling ---------------------------------------------- @@ -246,14 +275,14 @@ describe('runInstall — receipt escape entries (W2)', () => { // crafted escape asset alongside a valid one. The escape is reported // and skipped; the valid asset is still cleaned up. const receipt: Receipt = { - version: 1, + version: CURRENT_RECEIPT_VERSION, path: realpathSync(projectRoot), facets: { ghost: { version: '1.0.0', assets: [ - { scope: 'project', type: 'skill', name: '../escape' }, - { scope: 'project', type: 'skill', name: 'ghostly' }, + { scope: 'project', type: 'skill', name: '../escape', files: ['skills/escape/SKILL.md'] }, + { scope: 'project', type: 'skill', name: 'ghostly', files: ['skills/ghostly/SKILL.md'] }, ], }, }, diff --git a/packages/engine/src/install/commit/install-loop.ts b/packages/engine/src/install/commit/install-loop.ts index 3a6d2e97..b13cf929 100644 --- a/packages/engine/src/install/commit/install-loop.ts +++ b/packages/engine/src/install/commit/install-loop.ts @@ -5,6 +5,7 @@ import type { InstallJournal } from '../journal.ts' import { materialize } from '../materialize.ts' import { materializeFailureToRunInstall } from '../materialize-failure.ts' import type { FacetOutcome, OnLog, RunInstallFailure, StageEvent } from '../types.ts' +import { reconcileLockedAgainstPlan } from './reconcile.ts' import { resolveFacet } from './resolve-facet.ts' export interface InstallLoopSuccess { @@ -69,7 +70,7 @@ export async function installFacets(args: InstallLoopArgs): Promise 0) { serverWarnings.push({ facet: facetName, servers: serversDeclared }) @@ -80,6 +81,20 @@ export async function installFacets(args: InstallLoopArgs): Promise filesOf(a) !== undefined) + if (!anyLockedFiles) return undefined + + // Asset-identity agreement: the locked identity set vs the plan's. + const lockedIdentities = new Set(previous.assets.map(identityKey)) + const plannedIdentities = new Set(plan.assets.map(identityKey)) + const missing = [...lockedIdentities].filter((k) => !plannedIdentities.has(k)).sort() + const unexpected = [...plannedIdentities].filter((k) => !lockedIdentities.has(k)).sort() + if (missing.length > 0 || unexpected.length > 0) { + return { code: 'RECONCILE_ASSET_IDENTITY', facet, missing, unexpected } + } + + // Per-asset owned-path sets and per-file integrity. Only a `0.2` locked + // asset carries `files`; a legacy identity-only asset has none, so per-file + // reconciliation is skipped for it (migration). + const plannedByIdentity = new Map(plan.assets.map((a) => [identityKey(a), a])) + for (const lockedAsset of previous.assets) { + const lockedFiles = filesOf(lockedAsset) + if (lockedFiles === undefined) continue // legacy identity-only entry + + const planned = plannedByIdentity.get(identityKey(lockedAsset)) + // Identity agreement above guarantees a planned counterpart exists. + if (planned === undefined) continue + + const assetLabel = `${lockedAsset.type}:${lockedAsset.name}` + const plannedByPath = new Map(planned.files.map((f) => [f.path, f.integrity])) + const lockedPaths = new Set(lockedFiles.map((f) => f.path)) + const plannedPaths = new Set(planned.files.map((f) => f.path)) + + const missingPaths = [...lockedPaths].filter((p) => !plannedPaths.has(p)).sort() + const unexpectedPaths = [...plannedPaths].filter((p) => !lockedPaths.has(p)).sort() + if (missingPaths.length > 0 || unexpectedPaths.length > 0) { + return { + code: 'RECONCILE_OWNED_PATH_SET', + facet, + asset: assetLabel, + missing: missingPaths, + unexpected: unexpectedPaths, + } + } + + for (const record of lockedFiles) { + const recomputed = plannedByPath.get(record.path) + // Path-set agreement above guarantees a recomputed hash exists. + if (recomputed === undefined) continue + if (record.integrity !== recomputed) { + return { + code: 'RECONCILE_PER_FILE_INTEGRITY', + facet, + asset: assetLabel, + path: record.path, + expected: record.integrity, + actual: recomputed, + } + } + } + } + + return undefined +} + +function identityKey(asset: { scope: string; type: string; name: string }): string { + return `${asset.scope}:${asset.type}:${asset.name}` +} + +/** + * Extract the per-file records from a locked asset. A `0.2` asset carries a + * `files` array; a legacy (`1`) identity-only asset does not. The permissive + * `LockfileAssetEntry` type does not statically expose `files`, so this reads + * it structurally and narrows the shape at the boundary. + */ +function filesOf(asset: unknown): ReadonlyArray<{ path: string; integrity: string }> | undefined { + if (typeof asset !== 'object' || asset === null || !('files' in asset)) return undefined + const files = (asset as { files: unknown }).files + if (!Array.isArray(files)) return undefined + return files as ReadonlyArray<{ path: string; integrity: string }> +} diff --git a/packages/engine/src/install/commit/resolve-git.ts b/packages/engine/src/install/commit/resolve-git.ts index c0ac368f..3aa6650a 100644 --- a/packages/engine/src/install/commit/resolve-git.ts +++ b/packages/engine/src/install/commit/resolve-git.ts @@ -9,7 +9,12 @@ import type { Source } from '../../sources/facet/types.ts' import { cloneFailureToRunInstall } from '../clone-failure.ts' import { resolveCloneRef } from '../resolve-clone-ref.ts' import type { OnLog, StageEvent } from '../types.ts' -import { buildVerifiedAssetPlan } from '../verified-asset-plan.ts' +import { + buildVerifiedAssetPlan, + readSkillCompanionBytes, + type SkillCompanionBytes, + type VerifiedAssetPlan, +} from '../verified-asset-plan.ts' import { buildLockfileSource, loadFacetContent } from './finalize-facet.ts' import { auditedGitCacheLookup } from './git-cache.ts' import type { ResolveFacetResult } from './types.ts' @@ -97,10 +102,13 @@ export async function resolveGitFacet(args: ResolveGitFacetArgs): Promise | undefined if (auditedCacheHit && effectiveLocked !== undefined) { if (frozenLockfile) { // Frozen audited hit matching the lockfile: inherit verbatim — a - // legacy `1` entry stays legacy and is never rewritten. + // legacy `1` entry stays legacy and is never rewritten. No plan is + // derived, so reconciliation is skipped for this path. entry = { source: effectiveLocked.source, version: effectiveLocked.version, @@ -111,15 +119,17 @@ export async function resolveGitFacet(args: ResolveGitFacetArgs): Promise | undefined if (frozenLockfile && effectiveLocked !== undefined) { // Frozen reproduction guard. The verifier labels the failure // `lockfile` (built-vs-lockfile divergence) — reporting `git` here @@ -92,17 +99,22 @@ export async function resolveLocalFacet(args: ResolveLocalFacetArgs): Promise | undefined if (effectiveLocked !== undefined && frozenLockfile) { // Frozen reproduction: inherit the entry verbatim. The chain just // proved the content reproduces the locked integrity, and frozen mode - // never rewrites the lockfile — a legacy `1` entry stays legacy. + // never rewrites the lockfile — a legacy `1` entry stays legacy. No + // plan is derived, so reconciliation is skipped for this path. entry = { source: effectiveLocked.source, version: effectiveLocked.version, @@ -210,25 +218,30 @@ export async function resolveRegistryFacet(args: ResolveRegistryFacetArgs): Prom // `0.2`. Identity (source/version/integrity) comes from the locked entry // when reproducing, or from the resolved version + chain integrity when // confirming. - const plan = buildVerifiedAssetPlan(content.manifest, result.slotPath) - if (!plan.ok) { - return { ok: false, failure: { code: 'BUILD_FAILED', facet: facetName, errors: plan.errors } } + const built = buildVerifiedAssetPlan(content.manifest, result.slotPath) + if (!built.ok) { + return { ok: false, failure: { code: 'BUILD_FAILED', facet: facetName, errors: built.errors } } } + plan = built.plan + companionBytes = readSkillCompanionBytes(built.plan, result.slotPath) entry = effectiveLocked !== undefined ? { source: effectiveLocked.source, version: effectiveLocked.version, integrity: effectiveLocked.integrity, - assets: plan.plan.assets, + assets: built.plan.assets, } : { source: { kind: 'registry', registry: getRegistryBaseUrl() }, version: exactVersion, integrity: result.integrity, - assets: plan.plan.assets, + assets: built.plan.assets, } } - return { ok: true, value: { entry, resolved: content.resolved, serversDeclared: content.serversDeclared } } + return { + ok: true, + value: { entry, resolved: content.resolved, plan, companionBytes, serversDeclared: content.serversDeclared }, + } } diff --git a/packages/engine/src/install/commit/tri-write.ts b/packages/engine/src/install/commit/tri-write.ts index 07374c87..f4da575d 100644 --- a/packages/engine/src/install/commit/tri-write.ts +++ b/packages/engine/src/install/commit/tri-write.ts @@ -3,13 +3,22 @@ import { join } from 'node:path' import type { FacetsJson, Lockfile, LockfileFacet } from '@agent-facets/protocol' import { writeFacetsJson } from '../../manifest/project-files.ts' import { FACETS_LOCK_FILE, writeLockfile } from '../lockfile-io.ts' -import { type Receipt, type ReceiptFacetEntry, receiptPath, writeReceipt } from '../receipt.ts' +import { + ownedPathsForLockedAsset, + type Receipt, + type ReceiptFacetEntry, + receiptPath, + writeReceipt, +} from '../receipt.ts' import type { OnLog, RunInstallFailure } from '../types.ts' /** - * Derive the new receipt from the entries this run resolved. The - * receipt records `{ version, assets[] }` per facet — a self-sufficient - * deletion record for future drift removal. + * Derive the new receipt from the entries this run resolved. The receipt + * records `{ version, assets[] }` per facet, each asset carrying the owned + * inner-archive file paths mirrored from the lockfile — a self-sufficient, + * offline-capable deletion record for future drift removal. Paths come from + * the `0.2` lockfile asset `files[]`; a legacy identity-only asset seeds its + * single conventional primary path. */ export function buildUpdatedReceipt( receipt: Receipt, @@ -19,7 +28,12 @@ export function buildUpdatedReceipt( for (const [name, entry] of Object.entries(newFacetEntries)) { facets[name] = { version: entry.version, - assets: entry.assets.map((a) => ({ scope: a.scope, type: a.type, name: a.name })), + assets: entry.assets.map((a) => ({ + scope: a.scope, + type: a.type, + name: a.name, + files: ownedPathsForLockedAsset(a), + })), } } return { ...receipt, facets } diff --git a/packages/engine/src/install/commit/types.ts b/packages/engine/src/install/commit/types.ts index 37879f90..d9aec3b3 100644 --- a/packages/engine/src/install/commit/types.ts +++ b/packages/engine/src/install/commit/types.ts @@ -1,15 +1,27 @@ import type { LockfileFacet, ResolvedFacetManifest } from '@agent-facets/protocol' import type { RunInstallFailure } from '../types.ts' +import type { SkillCompanionBytes, VerifiedAssetPlan } from '../verified-asset-plan.ts' /** * What a per-source-kind resolver hands back to the commit * orchestrator: the lockfile entry to record, the resolved manifest - * (prompt bodies loaded) for materialization, and any server - * declarations to warn about. + * (prompt bodies loaded) for materialization, the verified asset plan + * (per-file paths + recomputed hashes + ownership) that pre-materialization + * reconciliation and materialization consume, the skill companion bytes read + * from the verified directory (for atomic skill-bundle install), and any + * server declarations to warn about. + * + * `plan` and `companionBytes` are `undefined` on the frozen-reproduction + * path, where the entry is inherited verbatim (possibly legacy `1`) and no + * fresh plan is derived. Reconciliation and companion materialization only + * run when a plan is present. */ export interface ResolvedFacet { entry: LockfileFacet resolved: ResolvedFacetManifest + plan: VerifiedAssetPlan | undefined + /** Skill companion bytes keyed by `skill:`; empty map for a companion-less skill. */ + companionBytes: Map | undefined serversDeclared: ReadonlyArray } diff --git a/packages/engine/src/install/materialize.ts b/packages/engine/src/install/materialize.ts index 41d5e2b1..8c13d0e9 100644 --- a/packages/engine/src/install/materialize.ts +++ b/packages/engine/src/install/materialize.ts @@ -1,6 +1,7 @@ import type { Adapter, AdapterAssetFailure, + CompanionMap, DeleteAssetRequest, InstallAssetRequest, ReadAssetRequest, @@ -10,6 +11,7 @@ import type { LockfileAssetEntry, ResolvedFacetManifest } from '@agent-facets/pr import { type AdapterCompatibilityFailure, compatibilityFailureFor } from '../adapters/api-compatibility.ts' import type { InstallJournal } from './journal.ts' import type { OnLog, StageEvent } from './types.ts' +import type { SkillCompanionBytes } from './verified-asset-plan.ts' /** * Compute the NEW asset set a facet contributes at this version. Derived @@ -65,6 +67,13 @@ export interface MaterializeOptions { /** Previous lockfile assets (OLD set); empty array when absent. */ oldAssets: readonly LockfileAssetEntry[] newAssets: readonly LockfileAssetEntry[] + /** + * Skill companion bytes for the assets being installed, keyed by + * `skill:` (from the resolver's verified asset plan). Absent on the + * frozen-reproduction path (no fresh plan) and for delete-only calls; a + * missing entry or map means "no companions" (single-file behavior). + */ + companionBytes?: Map journal: InstallJournal onLog?: OnLog /** Structured progress events for view layers. */ @@ -162,16 +171,28 @@ export async function materialize(opts: MaterializeOptions): Promise `[verbose] installing ${opts.facetName}@${opts.manifest.version} → ${adapter.name}`) + // Previous owned companion paths per asset key — the engine-verified set + // from the OLD lockfile entry, used both to read the prior bundle and to + // tell the adapter which owned paths a replacement may remove. + const oldOwnedByKey = new Map() + for (const old of opts.oldAssets) { + oldOwnedByKey.set(assetKey(old), ownedCompanionPathsOf(old)) + } + for (const asset of opts.newAssets) { const content = contentFor(opts.manifest, asset) const metadata = buildAssetMetadata(opts.manifest, asset, adapter.name) + // NEW companion bytes for this skill (empty for companion-less skills + // and non-skill assets); PREVIOUS owned paths for safe replacement. + const companions = opts.companionBytes?.get(assetKey(asset)) ?? {} + const ownedCompanionPaths = oldOwnedByKey.get(assetKey(asset)) ?? [] // Capture original state for rollback (F14). Treating any failure // as "didn't exist" would let the journal's delete-undo silently // delete a pre-existing asset we never read successfully. Narrow to // the structured `not-found` only and surface everything else as // `read-failed` — install fails loud before we write anything. - const readOutcome = await readPrevious(adapter, asset) + const readOutcome = await readPrevious(adapter, asset, ownedCompanionPaths) if (!readOutcome.ok) { return { ok: false, @@ -204,10 +225,14 @@ export async function materialize(opts: MaterializeOptions): Promise `[verbose] =${asset.type}:${asset.name} (skipped)`) skipped++ @@ -216,9 +241,21 @@ export async function materialize(opts: MaterializeOptions): Promise `[verbose] ~${asset.type}:${asset.name} drift: skills/${asset.name}/${path}`) + } + } + let writtenPath: string | undefined try { - const result = await adapter.installAsset(installRequestFor(asset, content, metadata)) + const result = await adapter.installAsset( + installRequestFor(asset, content, metadata, companions, ownedCompanionPaths), + ) if (!result.ok) { return { ok: false, @@ -245,21 +282,40 @@ export async function materialize(opts: MaterializeOptions): Promise `[verbose] ${sigil}${asset.type}:${asset.name}${writtenPath ? ` → ${writtenPath}` : ''}`) written++ + // Rollback preimage: restore the COMPLETE prior bundle (primary + + // previously-owned companion bytes), or delete a freshly-created asset. + // The owned-path set handed to the restore install is the union of the + // paths this operation could have written or removed, so the restore + // converges the bundle back to its prior state without touching unowned + // files. Companion-less skills and single-file assets restore exactly + // as before. + const restoreOwned = previous ? Object.keys(previous.companions) : ownedCompanionPaths opts.journal.record({ label: `install ${adapter.name}:${asset.type}:${asset.name}`, undo: async () => { if (previous) { - await runUndoInstall(adapter, asset, previous.content, previous.metadata ?? {}) + await runUndoInstall( + adapter, + asset, + previous.content, + previous.metadata ?? {}, + previous.companions, + restoreOwned, + ) } else { - await runUndoDelete(adapter, asset) + await runUndoDelete(adapter, asset, ownedCompanionPaths) } }, }) } for (const asset of toDelete) { + // The owned companion paths to delete come from the OLD entry — the set + // this machine materialized for the asset being removed. + const ownedCompanionPaths = oldOwnedByKey.get(assetKey(asset)) ?? ownedCompanionPathsOf(asset) + // Same F14 guard as the install branch above. - const readOutcome = await readPrevious(adapter, asset) + const readOutcome = await readPrevious(adapter, asset, ownedCompanionPaths) if (!readOutcome.ok) { return { ok: false, @@ -270,7 +326,7 @@ export async function materialize(opts: MaterializeOptions): Promise { - await runUndoInstall(adapter, asset, previous.content, previous.metadata ?? {}) + await runUndoInstall( + adapter, + asset, + previous.content, + previous.metadata ?? {}, + previous.companions, + Object.keys(previous.companions), + ) }, }) } @@ -314,15 +377,45 @@ export async function materialize(opts: MaterializeOptions): Promise }).files + if (!Array.isArray(rawFiles)) return [] + const skillRoot = `skills/${asset.name}/` + const primary = `skills/${asset.name}/SKILL.md` + const owned: string[] = [] + for (const f of rawFiles) { + const path = typeof f === 'string' ? f : f.path + if (path === primary) continue + owned.push(path.startsWith(skillRoot) ? path.slice(skillRoot.length) : path) + } + return owned +} + +/** + * Bridge from the engine's asset entry to the adapter's tagged install + * request. A skill request carries the new companion bundle (verbatim bytes + * keyed skill-root-relative) plus the engine-verified set of previously-owned + * companion paths, so the adapter replaces exactly the owned paths absent from + * the new bundle and never touches unowned files. An empty companion map and + * empty owned set reproduce the single-file behavior. + */ +function installRequestFor( + asset: LockfileAssetEntry, + content: string, + metadata: unknown, + companions: CompanionMap, + ownedCompanionPaths: readonly string[], +): InstallAssetRequest { if (asset.type === 'skill') { return { assetType: 'skill', @@ -330,43 +423,57 @@ function installRequestFor(asset: LockfileAssetEntry, content: string, metadata: name: asset.name, content, metadata, - companions: {}, - ownedCompanionPaths: [], + companions, + ownedCompanionPaths, } } return { assetType: asset.type, scope: asset.scope, name: asset.name, content, metadata } } -function readRequestFor(asset: LockfileAssetEntry): ReadAssetRequest { +function readRequestFor(asset: LockfileAssetEntry, ownedCompanionPaths: readonly string[]): ReadAssetRequest { if (asset.type === 'skill') { - return { assetType: 'skill', scope: asset.scope, name: asset.name, ownedCompanionPaths: [] } + return { assetType: 'skill', scope: asset.scope, name: asset.name, ownedCompanionPaths } } return { assetType: asset.type, scope: asset.scope, name: asset.name } } -function deleteRequestFor(asset: LockfileAssetEntry): DeleteAssetRequest { +function deleteRequestFor(asset: LockfileAssetEntry, ownedCompanionPaths: readonly string[]): DeleteAssetRequest { if (asset.type === 'skill') { - return { assetType: 'skill', scope: asset.scope, name: asset.name, ownedCompanionPaths: [] } + return { assetType: 'skill', scope: asset.scope, name: asset.name, ownedCompanionPaths } } return { assetType: asset.type, scope: asset.scope, name: asset.name } } +/** The captured prior state of an asset — its primary content, metadata, and + * (for skills) the bytes of its previously-owned companions. Used for + * skip-if-identical comparison and full-bundle rollback preimages. */ +interface PreviousAsset { + content: string + metadata?: Record + companions: CompanionMap +} + /** - * Read an asset's previous state for rollback capture. The structured - * `not-found` is the one "asset didn't exist" signal we trust — every - * other failure (and any throw, which is an adapter bug) means `previous` - * is unknown, so the caller must fail loud instead of assuming absence. + * Read an asset's previous state for rollback capture, including its + * previously-owned companion bytes for skills. The structured `not-found` is + * the one "asset didn't exist" signal we trust — every other failure (and any + * throw, which is an adapter bug) means `previous` is unknown, so the caller + * must fail loud instead of assuming absence. + * + * `ownedCompanionPaths` is the engine-verified previously-owned set (from the + * lockfile/receipt); the read returns exactly those companions that exist, so + * unowned files are never swept into the preimage. */ async function readPrevious( adapter: Adapter, asset: LockfileAssetEntry, -): Promise< - { ok: true; previous: { content: string; metadata?: Record } | null } | { ok: false; cause: string } -> { + ownedCompanionPaths: readonly string[], +): Promise<{ ok: true; previous: PreviousAsset | null } | { ok: false; cause: string }> { try { - const result = await adapter.readAsset(readRequestFor(asset)) + const result = await adapter.readAsset(readRequestFor(asset, ownedCompanionPaths)) if (result.ok) { - return { ok: true, previous: { content: result.asset.content, metadata: result.asset.metadata } } + const companions = result.asset.assetType === 'skill' ? result.asset.companions : {} + return { ok: true, previous: { content: result.asset.content, metadata: result.asset.metadata, companions } } } if (result.failure.code === 'not-found') return { ok: true, previous: null } return { ok: false, cause: describeAssetFailure(result.failure) } @@ -388,8 +495,12 @@ async function runUndoInstall( asset: LockfileAssetEntry, content: string, metadata: Record, + companions: CompanionMap, + ownedCompanionPaths: readonly string[], ): Promise { - const result = await adapter.installAsset(installRequestFor(asset, content, metadata)) + const result = await adapter.installAsset( + installRequestFor(asset, content, metadata, companions, ownedCompanionPaths), + ) if (!result.ok) { throw new Error( `undo install ${adapter.name}:${asset.type}:${asset.name} failed: ${describeAssetFailure(result.failure)}`, @@ -398,8 +509,12 @@ async function runUndoInstall( } /** Inverse delete during rollback. Same throw-on-`{ ok: false }` rule as {@link runUndoInstall}. */ -async function runUndoDelete(adapter: Adapter, asset: LockfileAssetEntry): Promise { - const result = await adapter.deleteAsset(deleteRequestFor(asset)) +async function runUndoDelete( + adapter: Adapter, + asset: LockfileAssetEntry, + ownedCompanionPaths: readonly string[], +): Promise { + const result = await adapter.deleteAsset(deleteRequestFor(asset, ownedCompanionPaths)) if (!result.ok) { throw new Error( `undo delete ${adapter.name}:${asset.type}:${asset.name} failed: ${describeAssetFailure(result.failure)}`, @@ -407,6 +522,44 @@ async function runUndoDelete(adapter: Adapter, asset: LockfileAssetEntry): Promi } } +/** + * Compare two companion maps for byte-exact equality. Used by + * skip-if-identical so a drifted or added/removed companion forces a repair + * through the atomic bundle replacement. + */ +function companionsIdentical(a: CompanionMap, b: CompanionMap): boolean { + const aKeys = Object.keys(a) + const bKeys = Object.keys(b) + if (aKeys.length !== bKeys.length) return false + for (const key of aKeys) { + if (!bytesEqual(a[key], b[key])) return false + } + return true +} + +function bytesEqual(a: Uint8Array | undefined, b: Uint8Array | undefined): boolean { + if (a === undefined || b === undefined || a.length !== b.length) return false + for (let i = 0; i < a.length; i++) { + if (a[i] !== b[i]) return false + } + return true +} + +/** + * The skill-root-relative companion paths that differ between the previously + * installed bundle and the new one — drifted (bytes changed), added, or + * removed. Sorted for stable, reviewable output. Used for path-specific drift + * reporting. + */ +function driftedCompanionPaths(previous: CompanionMap, next: CompanionMap): string[] { + const paths = new Set([...Object.keys(previous), ...Object.keys(next)]) + const drifted: string[] = [] + for (const path of paths) { + if (!bytesEqual(previous[path], next[path])) drifted.push(path) + } + return drifted.sort() +} + /** Render a structured adapter failure as a one-line cause string. */ function describeAssetFailure(failure: AdapterAssetFailure): string { switch (failure.code) { diff --git a/packages/engine/src/install/receipt.ts b/packages/engine/src/install/receipt.ts index 9ce83fd6..18330488 100644 --- a/packages/engine/src/install/receipt.ts +++ b/packages/engine/src/install/receipt.ts @@ -27,48 +27,108 @@ import { type } from 'arktype' import { facetReceiptsDir } from '../facet-dir.ts' import { jsonFileText } from '../json-file-text.ts' +// --------------------------------------------------------------------------- +// Versions (exact dispatch, mirroring the lockfile — design D10) +// --------------------------------------------------------------------------- + +/** + * The legacy receipt schema version. Numeric `1` identifies ONLY the previous + * alpha shape: asset entries with no owned-file records. Legacy installs + * could not materialize companions, so a legacy receipt is safely refined to + * primary-only ownership on load. Version dispatch is exact, never ordered. + */ +export const LEGACY_RECEIPT_VERSION = 1 + +/** + * The current receipt schema version. Current receipts mirror each committed + * lockfile asset/file ownership set: every asset records the exact inner- + * archive paths this machine materialized for it, so offline removal deletes + * exactly the owned files. The receipt stores paths only — never adapter- + * encoded hashes (design D10). + */ +export const CURRENT_RECEIPT_VERSION = 0.2 + // --------------------------------------------------------------------------- // Schema // --------------------------------------------------------------------------- -const ReceiptAssetSchema = type({ +/** + * A current (`0.2`) receipt asset record: adapter-agnostic identity plus the + * complete set of owned inner-archive file paths. A skill owns + * `skills//SKILL.md` plus every materialized companion; an agent or + * command owns exactly its single conventional primary file. Companion paths + * are the engine-supplied `ownedCompanionPaths` handed to the adapter delete + * request, so offline multi-file cleanup is exact. + */ +const CurrentReceiptAssetSchema = type({ + scope: "'system' | 'user' | 'project'", + type: "'skill' | 'agent' | 'command'", + name: 'string', + files: 'string[]', +}) + +const CurrentReceiptFacetEntrySchema = type({ + version: 'string', + assets: CurrentReceiptAssetSchema.array(), +}) + +const CurrentReceiptSchema = type({ + version: type.unit(CURRENT_RECEIPT_VERSION), + path: 'string', + facets: type.Record('string', CurrentReceiptFacetEntrySchema), +}) + +/** Legacy (`1`) receipt: identity-only asset tuples, no owned-file records. */ +const LegacyReceiptAssetSchema = type({ scope: "'system' | 'user' | 'project'", type: "'skill' | 'agent' | 'command'", name: 'string', }) -const ReceiptFacetEntrySchema = type({ +const LegacyReceiptFacetEntrySchema = type({ version: 'string', - assets: ReceiptAssetSchema.array(), + assets: LegacyReceiptAssetSchema.array(), }) -const ReceiptSchema = type({ - version: '1', +const LegacyReceiptSchema = type({ + version: type.unit(LEGACY_RECEIPT_VERSION), path: 'string', - facets: type.Record('string', ReceiptFacetEntrySchema), + facets: type.Record('string', LegacyReceiptFacetEntrySchema), }) // --------------------------------------------------------------------------- // Types // --------------------------------------------------------------------------- +/** + * A current receipt asset record with its owned inner-archive file paths. + * Extends the adapter-agnostic identity so existing identity-only consumers + * (which read `scope`/`type`/`name`) keep working while offline removal reads + * `files` for exact owned-path deletion. + */ +export interface ReceiptAsset extends LockfileAssetEntry { + files: string[] +} + export interface ReceiptFacetEntry { version: string - assets: LockfileAssetEntry[] + assets: ReceiptAsset[] } export interface Receipt { - version: 1 + version: typeof CURRENT_RECEIPT_VERSION path: string facets: Record } /** - * A receipt asset entry rejected during load because its name failed - * validation (path traversal, backslashes, empty segments). Reported — - * never acted on — while the facet's remaining valid entries still - * load (D6: a corrupted receipt may cause a skipped cleanup; it must - * never poison the rest of the record). + * A receipt asset entry rejected during load because its name or one of its + * owned file paths failed validation (path traversal, backslashes, empty + * segments). Reported — never acted on — while the facet's remaining valid + * entries still load (D6: a corrupted receipt may cause a skipped cleanup; it + * must never poison the rest of the record). Because an invalid path could + * escape the adapter's storage root, the whole asset record is dropped rather + * than partially trusted — an unowned or escaping path is never deleted. */ export interface InvalidReceiptAsset { facet: string @@ -168,38 +228,106 @@ export function loadReceipt(projectDir: string): LoadReceiptResult { return { ok: false, reason: 'corrupt' } } - const validated = ReceiptSchema(parsed) - if (validated instanceof type.errors) { + // Exact version dispatch (design D10): a current `0.2` receipt carries + // owned-file records; a legacy `1` receipt is identity-only and is refined + // to primary-only ownership (legacy installs could not materialize + // companions). Any other/absent version is corrupt — the shape is never + // sniffed to guess a schema. + const observedVersion = + typeof parsed === 'object' && parsed !== null && 'version' in parsed + ? (parsed as { version?: unknown }).version + : undefined + + let embeddedPath: string + let rawFacets: Record }> + if (observedVersion === CURRENT_RECEIPT_VERSION) { + const validated = CurrentReceiptSchema(parsed) + if (validated instanceof type.errors) return { ok: false, reason: 'corrupt' } + embeddedPath = validated.path + rawFacets = validated.facets + } else if (observedVersion === LEGACY_RECEIPT_VERSION) { + const validated = LegacyReceiptSchema(parsed) + if (validated instanceof type.errors) return { ok: false, reason: 'corrupt' } + embeddedPath = validated.path + // Refine legacy identity-only assets to primary-only owned-file sets: + // the single conventional primary path per asset, no companions. + rawFacets = {} + for (const [name, entry] of Object.entries(validated.facets)) { + rawFacets[name] = { + version: entry.version, + assets: entry.assets.map((a) => ({ ...a, files: [primaryPathFor(a)] })), + } + } + } else { return { ok: false, reason: 'corrupt' } } - const receipt = validated as Receipt - // Self-identification check: the embedded path must match the // project being operated on. - if (receipt.path !== canonical) { + if (embeddedPath !== canonical) { return { ok: false, reason: 'path-mismatch' } } - // Validate all asset names — extract crafted names that could cause - // path traversal when passed to adapters. Invalid entries are - // reported per-entry; valid entries are still processed. + // Validate every asset name AND every owned file path — receipt data is + // untrusted. A crafted name or a path that could traverse outside the + // adapter's storage drops the whole asset record (reported, never + // deleted), while the facet's remaining valid entries still load (D6). const invalidEntries: InvalidReceiptAsset[] = [] const facets: Record = {} - for (const [facetName, entry] of Object.entries(receipt.facets)) { - const validAssets: LockfileAssetEntry[] = [] + for (const [facetName, entry] of Object.entries(rawFacets)) { + const validAssets: ReceiptAsset[] = [] for (const asset of entry.assets) { - const check = validateAssetName(asset.name) - if (check.ok) { - validAssets.push(asset) - } else { - invalidEntries.push({ facet: facetName, asset: asset.name, reason: check.reason }) + const nameCheck = validateAssetName(asset.name) + if (!nameCheck.ok) { + invalidEntries.push({ facet: facetName, asset: asset.name, reason: nameCheck.reason }) + continue + } + let badPath: { path: string; reason: string } | undefined + for (const p of asset.files) { + const check = validateAssetName(p) + if (!check.ok) { + badPath = { path: p, reason: check.reason } + break + } } + if (badPath !== undefined) { + invalidEntries.push({ + facet: facetName, + asset: asset.name, + reason: `owned path "${badPath.path}" ${badPath.reason}`, + }) + continue + } + validAssets.push({ scope: asset.scope, type: asset.type, name: asset.name, files: [...asset.files] }) } facets[facetName] = { version: entry.version, assets: validAssets } } - return { ok: true, receipt: { ...receipt, facets }, invalidEntries } + return { + ok: true, + receipt: { version: CURRENT_RECEIPT_VERSION, path: canonical, facets }, + invalidEntries, + } +} + +/** The raw asset shape produced by version dispatch before path validation. */ +interface RawReceiptAsset { + scope: 'system' | 'user' | 'project' + type: 'skill' | 'agent' | 'command' + name: string + files: ReadonlyArray +} + +/** The conventional primary inner-archive path for an asset identity. */ +function primaryPathFor(asset: { type: 'skill' | 'agent' | 'command'; name: string }): string { + switch (asset.type) { + case 'skill': + return `skills/${asset.name}/SKILL.md` + case 'agent': + return `agents/${asset.name}.md` + case 'command': + return `commands/${asset.name}.md` + } } // --------------------------------------------------------------------------- @@ -230,13 +358,15 @@ export function writeReceipt(projectDir: string, receipt: Receipt): void { // --------------------------------------------------------------------------- /** - * Create a fresh receipt from a lockfile. Used when no receipt exists - * (first operation on the project) or when the existing receipt fails - * validation (path mismatch, corruption). + * Create a fresh current (`0.2`) receipt from a lockfile. Used when no + * receipt exists (first operation on the project) or when the existing + * receipt fails validation (path mismatch, corruption). * - * Seeds from the lockfile's entries — records what *should* be on - * disk. Assets orphaned before this change shipped are unrecoverable - * (explicit non-goal in the proposal). + * Seeds from the lockfile's entries — records what *should* be on disk. + * Owned file paths come from a `0.2` lockfile asset's `files[]`; a legacy + * (identity-only) lockfile asset seeds the single conventional primary path + * (legacy could not materialize companions). Assets orphaned before this + * change shipped are unrecoverable (explicit non-goal in the proposal). */ export function bootstrapReceipt(projectDir: string, lockfile: Lockfile): Receipt { const canonical = realpathSync(projectDir) @@ -245,9 +375,28 @@ export function bootstrapReceipt(projectDir: string, lockfile: Lockfile): Receip for (const [name, entry] of Object.entries(lockfile.facets)) { facets[name] = { version: entry.version, - assets: entry.assets.map((a) => ({ scope: a.scope, type: a.type, name: a.name })), + assets: entry.assets.map((a) => ({ + scope: a.scope, + type: a.type, + name: a.name, + files: ownedPathsForLockedAsset(a), + })), } } - return { version: 1, path: canonical, facets } + return { version: CURRENT_RECEIPT_VERSION, path: canonical, facets } +} + +/** + * Owned inner-archive paths for a locked asset. A `0.2` asset carries a + * `files[]` array of `{ path, integrity }`; the receipt mirrors the paths + * (no hashes). A legacy identity-only asset has no `files`, so the single + * conventional primary path is used. + */ +export function ownedPathsForLockedAsset(asset: LockfileAssetEntry): string[] { + const files = (asset as { files?: ReadonlyArray<{ path: string }> }).files + if (Array.isArray(files) && files.length > 0) { + return files.map((f) => f.path) + } + return [primaryPathFor(asset)] } diff --git a/packages/engine/src/install/run-install.ts b/packages/engine/src/install/run-install.ts index 3e865660..7b67feb9 100644 --- a/packages/engine/src/install/run-install.ts +++ b/packages/engine/src/install/run-install.ts @@ -119,6 +119,18 @@ export async function runInstall(opts: RunInstallOptions): Promise + unexpected: ReadonlyArray + } + | { + code: 'RECONCILE_OWNED_PATH_SET' + facet: string + asset: string + missing: ReadonlyArray + unexpected: ReadonlyArray + } + | { + code: 'RECONCILE_PER_FILE_INTEGRITY' + facet: string + asset: string + path: string + expected: string + actual: string + } | { code: 'ABORTED' } /** diff --git a/packages/engine/src/install/verified-asset-plan.ts b/packages/engine/src/install/verified-asset-plan.ts index c3c21041..7e40c514 100644 --- a/packages/engine/src/install/verified-asset-plan.ts +++ b/packages/engine/src/install/verified-asset-plan.ts @@ -1,3 +1,5 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' import type { ValidationError } from '@agent-facets/common' import { type FacetManifest, planArchiveEntries } from '@agent-facets/protocol' import { computeDirIntegrity } from '../cache/index.ts' @@ -178,3 +180,49 @@ function assetOrder(a: VerifiedAsset, b: VerifiedAsset): number { if (a.type !== b.type) return TYPE_ORDER[a.type] - TYPE_ORDER[b.type] return a.name < b.name ? -1 : a.name > b.name ? 1 : 0 } + +/** + * A canonical map of skill companion paths (relative to the skill root) to + * their exact bytes — the shape the adapter skill-install contract's + * `companions` field expects. `SKILL.md` (the primary) is NOT included; it is + * carried as the request's `content`. + */ +export type SkillCompanionBytes = Record + +/** + * Read the companion bytes for every skill in a verified asset plan from the + * verified directory, keyed by `type:name` asset identity. + * + * Companion bytes are read verbatim (opaque `Uint8Array`) — never decoded or + * front-matter processed. Paths are converted from the plan's full inner- + * archive form (`skills//references/api.md`) to the skill-root-relative + * form the adapter contract uses (`references/api.md`). Only skill assets with + * companions beyond `SKILL.md` appear in the result; a companion-less skill + * maps to an empty companion map. + * + * `verifiedDir` MUST be the same verified directory the plan was derived from. + */ +export function readSkillCompanionBytes( + plan: VerifiedAssetPlan, + verifiedDir: string, +): Map { + const byAsset = new Map() + for (const asset of plan.assets) { + if (asset.type !== 'skill') continue + const skillRoot = `skills/${asset.name}/` + const primary = `skills/${asset.name}/SKILL.md` + const companions: SkillCompanionBytes = {} + for (const file of asset.files) { + if (file.path === primary) continue + // Every skill companion path is derived by `planArchiveEntries` to live + // below the skill root, so this prefix strip is total. + const relative = file.path.startsWith(skillRoot) ? file.path.slice(skillRoot.length) : file.path + companions[relative] = new Uint8Array(readFileSync(join(verifiedDir, file.path))) + } + // Keyed by the `scope:type:name` identity `materialize` uses to look this + // up. Companion bytes only exist for `project`-scoped skills today (the + // only scope the plan mints), so the scope prefix is fixed. + byAsset.set(`${asset.scope}:skill:${asset.name}`, companions) + } + return byAsset +}