From c75d772e7f85a4b711612e9ffece487ee57bf2c6 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Wed, 12 Aug 2026 12:48:32 -0400 Subject: [PATCH] test(skills): cover placement conflict shapes --- ...-skill-sharing-implementation-checklist.md | 12 +++-- docs/agent-skill-sharing-installation-plan.md | 2 +- .../skills/skill-install-transaction.test.ts | 32 ++++++++++++++ .../skill-placement-reconciliation.test.ts | 44 ++++++++++++++++++- 4 files changed, 85 insertions(+), 5 deletions(-) diff --git a/docs/agent-skill-sharing-implementation-checklist.md b/docs/agent-skill-sharing-implementation-checklist.md index 801c478e8e0..0dbf8bf001b 100644 --- a/docs/agent-skill-sharing-implementation-checklist.md +++ b/docs/agent-skill-sharing-implementation-checklist.md @@ -1249,8 +1249,8 @@ OS, destination kind, transport, conflicts, placement topology/mechanism, copy f capability absence, recovered transactions, stale extraction/lock cleanup, and failure categories use fixed or bounded values; bundle error-category cardinality is capped at 32. The implementation adds no product telemetry event, RPC field, persisted schema, or remote opcode. The skill-sharing -release suite passes 423 tests with 29 expected platform skips, the focused affected matrix passes -98 tests with one expected skip, typecheck and the full lint pipeline pass, and the real-process +release suite passes 427 tests with 29 expected platform skips, the focused affected matrix passes +102 tests with one expected skip, typecheck and the full lint pipeline pass, and the real-process macOS recovery matrix passes all 17 crash boundaries. Physical Windows/WSL reruns remain separate platform gates below. @@ -1307,8 +1307,14 @@ platform gates below. safe removal. - [x] Test modified and unowned canonical and provider conflicts. - [ ] Test canonical/provider paths as regular files, directories, aliases, junctions, external - links, and broken links. + links, and broken links. Deterministic native coverage now preserves canonical regular-file + name collisions, unowned provider files/directories, external and broken unowned links, + Orca-owned broken-link repair, aliases, and verified copy fallback. Physical Windows + junction coverage remains open. - [ ] Test concurrent desktop, headless runtime, CLI, SSH, and recovery attempts. + The shared transaction core now serializes two simultaneous installs into one installed and + one unchanged result, one receipt, and no staging residue; cross-entry-point contention + remains open. - [ ] Test permission and read-only failures, disk exhaustion, cancellation, process termination, and host disconnect. Simulated `EACCES` and `ENOSPC` preservation plus lost-response retry are covered; physical failure and termination journeys remain open. diff --git a/docs/agent-skill-sharing-installation-plan.md b/docs/agent-skill-sharing-installation-plan.md index e368d72307e..b4f82e3c36c 100644 --- a/docs/agent-skill-sharing-installation-plan.md +++ b/docs/agent-skill-sharing-installation-plan.md @@ -99,7 +99,7 @@ conflict/topology/mechanism outcomes, copy fallback, capability absence, recover and bounded error categories without paths, skill names, contents, share URLs, grants, host IDs, or credentials. Bundle summaries are aggregate and cap error-category cardinality at 32. This is not product telemetry and changes no RPC, persisted state, or remote opcode, so old clients and -hosts remain compatible. The release suite passes 423 tests, full typecheck/lint pass, and the +hosts remain compatible. The release suite passes 427 tests, full typecheck/lint pass, and the real-process macOS crash-recovery matrix passes all 17 boundaries; physical platform reruns remain tracked in the checklist. diff --git a/src/main/skills/skill-install-transaction.test.ts b/src/main/skills/skill-install-transaction.test.ts index 2cd9a97c0b8..ba90c206de1 100644 --- a/src/main/skills/skill-install-transaction.test.ts +++ b/src/main/skills/skill-install-transaction.test.ts @@ -119,6 +119,19 @@ describe('skill install transaction', () => { expect(await readFile(join(destination, 'SKILL.md'), 'utf8')).toContain('# Cloud') }) + it('preserves a regular file at the canonical destination as a name collision', async () => { + const root = await temporaryDirectory() + const archive = await packageVersion(root, 'version_1', '# Cloud') + const destination = join(root, 'skills', 'test-skill') + await mkdir(join(root, 'skills'), { recursive: true }) + await writeFile(destination, 'unowned regular file') + + const result = await installLocalSkillPackage(installInput(root, archive)) + + expect(result).toMatchObject({ status: 'conflict', conflict: { kind: 'name-collision' } }) + expect(await readFile(destination, 'utf8')).toBe('unowned regular file') + }) + it('rejects a case-insensitive sibling collision before destination mutation', async () => { const root = await temporaryDirectory() const archive = await packageVersion(root, 'version_1', '# Cloud') @@ -272,6 +285,25 @@ describe('skill install transaction', () => { } }) + it('serializes simultaneous installs without duplicate provenance or staging bytes', async () => { + const root = await temporaryDirectory() + const archive = await packageVersion(root, 'version_1', '# Concurrent') + + const results = await Promise.all([ + installLocalSkillPackage(installInput(root, archive, { operationId: 'concurrent-a' })), + installLocalSkillPackage(installInput(root, archive, { operationId: 'concurrent-b' })) + ]) + + expect(results.map((result) => result.status).sort()).toEqual(['installed', 'unchanged']) + expect(await readFile(join(root, 'skills', 'test-skill', 'SKILL.md'), 'utf8')).toContain( + '# Concurrent' + ) + expect((await readdir(join(root, 'skills'))).filter((name) => name.includes('.orca-'))).toEqual( + [] + ) + expect(await readdir(join(root, 'state', 'receipts'))).toHaveLength(1) + }) + it('invalidates a plan when destination state changes immediately before commit', async () => { const root = await temporaryDirectory() const archive = await packageVersion(root, 'version_1', '# Cloud') diff --git a/src/main/skills/skill-placement-reconciliation.test.ts b/src/main/skills/skill-placement-reconciliation.test.ts index c6bd8ee329f..3cd730df0a6 100644 --- a/src/main/skills/skill-placement-reconciliation.test.ts +++ b/src/main/skills/skill-placement-reconciliation.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { lstat, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import type * as NodeFsPromises from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -23,6 +23,48 @@ afterEach(async () => { }) describe('skill provider placement reconciliation', () => { + it.each(['file', 'directory'] as const)( + 'preserves an unowned provider %s destination', + async (kind) => { + const root = await mkdtemp(join(tmpdir(), 'orca-skill-placement-test-')) + temporaryDirectories.push(root) + const canonicalPath = join(root, 'canonical', 'private-skill') + const providerRoot = join(root, 'provider') + const destinationPath = join(providerRoot, 'private-skill') + await mkdir(canonicalPath, { recursive: true }) + await writeFile(join(canonicalPath, 'SKILL.md'), 'private skill') + await mkdir(providerRoot) + if (kind === 'file') { + await writeFile(destinationPath, 'unowned file') + } else { + await mkdir(destinationPath) + await writeFile(join(destinationPath, 'SKILL.md'), 'unowned directory') + } + const observed = await nativeSkillInstallFilesystem.observeSkill(canonicalPath) + + const result = await reconcileSkillProviderPlacement({ + canonicalPath, + skillName: 'private-skill', + destination: { provider: 'claude', rootPath: providerRoot, readsCanonicalRoot: false }, + previousReceipt: null, + packageDigest: observed.observedDigest + }) + + expect(result).toMatchObject({ + topology: 'independent-copy', + status: 'skipped', + errorCategory: 'skill-placement-unowned' + }) + const destinationStat = await lstat(destinationPath) + expect(kind === 'file' ? destinationStat.isFile() : destinationStat.isDirectory()).toBe(true) + if (kind === 'file') { + expect(await readFile(destinationPath, 'utf8')).toBe('unowned file') + } else { + expect(await readFile(join(destinationPath, 'SKILL.md'), 'utf8')).toBe('unowned directory') + } + } + ) + it('creates a verified independent copy when native alias creation is denied', async () => { const root = await mkdtemp(join(tmpdir(), 'orca-skill-placement-test-')) temporaryDirectories.push(root)