diff --git a/docs/agent-skill-sharing-implementation-checklist.md b/docs/agent-skill-sharing-implementation-checklist.md index 91d78298c9c..8414d05fcc1 100644 --- a/docs/agent-skill-sharing-implementation-checklist.md +++ b/docs/agent-skill-sharing-implementation-checklist.md @@ -206,7 +206,7 @@ does not mean the surrounding phase is complete. user explicitly approves replacement. - [x] Package ingestion rejects unsafe archives before any destination mutation. - [x] Interrupted operations recover to a complete previous or requested version. -- [ ] Private package bytes remain access-controlled, published versions persist until deletion, +- [x] Private package bytes remain access-controlled, published versions persist until deletion, and incomplete uploads expire automatically. - [x] Mixed-version clients and remote hosts fail safely through capability negotiation. - [ ] Cloud resources are Terraform-owned, monitored, recoverable, and protected by independent @@ -305,8 +305,10 @@ does not mean the surrounding phase is complete. digest. - [x] Validate Agent Plugins 1.0.0 `plugin.json` fields from independently authored local rules; never fetch a schema during loading. -- [ ] Create fresh staging roots and reject unknown, malformed, or conflicting imported - `dev.orca.skill-sharing` namespaces without overwriting them. +- [x] Create fresh staging roots and reject unknown, malformed, or conflicting imported + `dev.orca.skill-sharing` namespaces without overwriting them. The extractor now exclusively + creates its staging root, preserves a pre-existing namespace on conflict, rejects unknown + top-level extension entries, and removes only staging it created. - [x] Keep the detailed manifest inside the archive for export, remote/SSH transport, and offline validation; keep GCS metadata compact and PostgreSQL limited to exact link/ownership lookup. - [x] Add selected skill IDs and per-skill conflict choices to preview/install requests. @@ -394,12 +396,13 @@ does not mean the surrounding phase is complete. - [x] Include only `skill/` contents in the eventual installed directory. - [x] Bind the user-visible share preview to the final staged digest. - [x] Clean staging files on success, cancellation, source drift, and error. -- [ ] Accept a non-empty ordered selection of skill directories and support at least 30 within the - global entry/file/byte limits. +- [x] Accept a non-empty ordered selection of skill directories and support at least 30 within the + global entry/file/byte limits. Focused local creation/extraction now covers one 30-skill + archive; staging load run `31585710645` finalized the same maximum selection through Cloud. - [ ] Reject duplicate normalized skill names and preserve selection across search/filter changes in the caller. - [x] Generate deterministic `plugin.json`, `skills/`, and Orca extension metadata. -- [ ] Bind the share review to every staged skill digest and the final bundle digest. +- [x] Bind the share review to every staged skill digest and the final bundle digest. ### Bounded extraction diff --git a/src/main/skills/skill-bundle-creation.test.ts b/src/main/skills/skill-bundle-creation.test.ts index c6ee801dc86..f4d0c5eece1 100644 --- a/src/main/skills/skill-bundle-creation.test.ts +++ b/src/main/skills/skill-bundle-creation.test.ts @@ -1,10 +1,14 @@ -import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' -import { AGENT_PLUGIN_SCHEMA_V1 } from '../../shared/skill-bundle-manifest' +import { + AGENT_PLUGIN_MANIFEST_PATH, + AGENT_PLUGIN_SCHEMA_V1 +} from '../../shared/skill-bundle-manifest' import { createSkillBundleArchive } from './skill-bundle-creation' import { extractSkillBundleArchive } from './skill-bundle-extraction' +import { writeSkillTarGzip } from './skill-package-tar' const temporaryDirectories: string[] = [] @@ -92,6 +96,91 @@ describe('skill bundle creation and extraction', () => { expect(await readFile(first.archivePath)).toEqual(await readFile(second.archivePath)) }) + it('packages and extracts thirty selected skills within the shared limits', async () => { + const root = await temporaryDirectory() + const names = Array.from( + { length: 30 }, + (_, index) => `skill-${String(index).padStart(2, '0')}` + ) + const sources = await Promise.all( + names.map(async (name) => ({ + sourceDirectory: await createSkill(root, name, `Description for ${name}`) + })) + ) + const created = await createSkillBundleArchive({ + sources: sources.toReversed(), + archivePath: join(root, 'thirty-skills.tar.gz'), + packageId: 'package_30', + versionId: 'version_30', + bundleName: 'thirty-skills' + }) + + const extracted = await extractSkillBundleArchive({ + archivePath: created.archivePath, + destinationDirectory: join(root, 'thirty-skills'), + expectedBundleDigest: created.manifest.bundleDigest + }) + + expect(extracted.manifest.skills.map((skill) => skill.name)).toEqual(names) + expect(extracted.manifest.skills).toHaveLength(30) + await expect( + readFile(join(extracted.skillsDirectory, 'skill-29', 'notes.txt'), 'utf8') + ).resolves.toBe('skill-29 notes\n') + }) + + it('rejects conflicting staging roots without changing their imported namespace', async () => { + const root = await temporaryDirectory() + const source = await createSkill(root, 'alpha-skill', 'Alpha') + const created = await createSkillBundleArchive({ + sources: [{ sourceDirectory: source }], + archivePath: join(root, 'bundle.tar.gz'), + packageId: 'package_1', + versionId: 'version_1', + bundleName: 'team-skills' + }) + const destination = join(root, 'conflicting-staging') + const importedManifest = join(destination, 'dev.orca.skill-sharing', 'manifest.json') + await mkdir(join(destination, 'dev.orca.skill-sharing'), { recursive: true }) + await writeFile(importedManifest, 'unowned\n') + + await expect( + extractSkillBundleArchive({ + archivePath: created.archivePath, + destinationDirectory: destination + }) + ).rejects.toMatchObject({ code: 'EEXIST' }) + await expect(readFile(importedManifest, 'utf8')).resolves.toBe('unowned\n') + }) + + it('rejects unknown top-level extension namespaces and removes fresh staging', async () => { + const root = await temporaryDirectory() + const plugin = Buffer.from( + JSON.stringify({ $schema: AGENT_PLUGIN_SCHEMA_V1, name: 'team-skills' }) + ) + const unknown = Buffer.from('{}') + const archivePath = join(root, 'unknown-extension.tar.gz') + await writeSkillTarGzip(archivePath, [ + { + path: AGENT_PLUGIN_MANIFEST_PATH, + size: plugin.length, + executable: false, + bytes: plugin + }, + { + path: 'dev.orca.unexpected/manifest.json', + size: unknown.length, + executable: false, + bytes: unknown + } + ]) + const destination = join(root, 'unknown-extension-staging') + + await expect( + extractSkillBundleArchive({ archivePath, destinationDirectory: destination }) + ).rejects.toThrow('skill-bundle-manifest-envelope-invalid') + await expect(stat(destination)).rejects.toMatchObject({ code: 'ENOENT' }) + }) + it('rejects duplicate names and source drift without publishing an archive', async () => { const root = await temporaryDirectory() const first = await createSkill(root, 'same-skill', 'First') diff --git a/src/main/skills/skill-bundle-extraction.ts b/src/main/skills/skill-bundle-extraction.ts index d8e8db28019..1c068d832cf 100644 --- a/src/main/skills/skill-bundle-extraction.ts +++ b/src/main/skills/skill-bundle-extraction.ts @@ -142,6 +142,8 @@ export async function extractSkillBundleArchive(input: { const archive = await openSkillTarGzip(input.archivePath) let destinationCreated = false try { + await mkdir(input.destinationDirectory, { mode: 0o700 }) + destinationCreated = true const pluginManifest = parseAgentPluginManifest( await readJsonEntry(archive.reader, AGENT_PLUGIN_MANIFEST_PATH) ) @@ -158,8 +160,7 @@ export async function extractSkillBundleArchive(input: { throw new Error('skill-bundle-identity-mismatch') } const skillsDirectory = join(input.destinationDirectory, 'skills') - await mkdir(skillsDirectory, { recursive: true, mode: 0o700 }) - destinationCreated = true + await mkdir(skillsDirectory, { mode: 0o700 }) for (const skill of manifest.skills) { for (const file of skill.files) { const archivePath = `skills/${skill.name}/${file.path}`