From 4eeb30ddc848934764e7f3e02049fbe0e02a3585 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Tue, 11 Aug 2026 09:38:51 -0400 Subject: [PATCH] feat(skills): harden sharing and install lifecycle --- docs/agent-skill-sharing-installation-plan.md | 10 +- src/main/ipc/skill-cloud-ipc-handlers.ts | 220 ++++++--------- .../skill-install-management-ipc-handlers.ts | 133 +++++++++ src/main/runtime/orca-runtime.ts | 32 +++ src/main/runtime/rpc/errors.test.ts | 21 ++ src/main/runtime/rpc/errors.ts | 17 +- src/main/runtime/rpc/methods/skills.test.ts | 51 ++++ src/main/runtime/rpc/methods/skills.ts | 25 +- .../skill-install-discovery-verification.ts | 123 ++++++++ src/main/skills/skill-install-lock.ts | 4 +- .../skills/skill-install-operation-error.ts | 42 +++ .../skills/skill-install-recovery.test.ts | 169 +++++++++++ src/main/skills/skill-install-recovery.ts | 74 ++++- .../skill-install-request-service.test.ts | 21 +- .../skills/skill-install-request-service.ts | 23 +- src/main/skills/skill-install-service.test.ts | 18 ++ src/main/skills/skill-install-service.ts | 38 ++- .../skill-install-transaction-result.ts | 104 +++++++ .../skills/skill-install-transaction.test.ts | 89 +++++- src/main/skills/skill-install-transaction.ts | 144 +++++----- .../skills/skill-package-creation.test.ts | 156 +++++++++++ src/main/skills/skill-package-creation.ts | 27 +- src/main/skills/skill-package-download.ts | 17 +- src/main/skills/skill-package-extraction.ts | 25 +- .../skill-placement-alias-repair.test.ts | 100 +++++++ src/main/skills/skill-placement-copy.ts | 82 ++++++ .../skills/skill-placement-reconciliation.ts | 148 ++++------ .../skills/skill-remote-install-service.ts | 21 +- src/main/skills/skill-remove-recovery.ts | 194 +++++++++++++ .../skills/skill-remove-transaction.test.ts | 155 ++++++++++ src/main/skills/skill-remove-transaction.ts | 142 +++------- src/preload/api-types.ts | 13 + src/preload/index.ts | 18 ++ .../SkillCloudManagementActions.test.tsx | 130 +++++++++ .../skills/SkillCloudManagementActions.tsx | 264 ++++++++++++++++++ .../skills/SkillInstallDialog.test.tsx | 68 +++++ .../components/skills/SkillInstallDialog.tsx | 177 ++++-------- .../SkillInstallManagementDialog.test.tsx | 76 ++++- .../skills/SkillInstallManagementDialog.tsx | 76 ++++- .../skills/SkillInstallReviewContent.tsx | 172 ++++++++++++ .../skills/SkillShareDialog.test.tsx | 153 ++++++++++ .../components/skills/SkillShareDialog.tsx | 261 ++++++----------- .../skills/SkillShareReviewContent.tsx | 220 +++++++++++++++ .../src/components/skills/SkillsPage.tsx | 7 + .../skills/skill-install-result-label.ts | 5 +- .../skills/skill-share-package-selection.ts | 18 ++ src/renderer/src/web/web-preload-api.ts | 9 + src/shared/protocol-version.ts | 4 + src/shared/skill-cloud-contract.ts | 10 + src/shared/skill-install-capability.ts | 2 + src/shared/skill-install-contract.ts | 34 ++- src/shared/skill-install-failure.test.ts | 52 ++++ src/shared/skill-install-failure.ts | 105 +++++++ src/shared/skill-package-manifest.test.ts | 83 ++++++ src/shared/skill-package-manifest.ts | 11 +- src/shared/skill-sharing-contract.ts | 6 + 56 files changed, 3637 insertions(+), 762 deletions(-) create mode 100644 src/main/ipc/skill-install-management-ipc-handlers.ts create mode 100644 src/main/skills/skill-install-discovery-verification.ts create mode 100644 src/main/skills/skill-install-operation-error.ts create mode 100644 src/main/skills/skill-install-recovery.test.ts create mode 100644 src/main/skills/skill-install-transaction-result.ts create mode 100644 src/main/skills/skill-placement-alias-repair.test.ts create mode 100644 src/main/skills/skill-placement-copy.ts create mode 100644 src/main/skills/skill-remove-recovery.ts create mode 100644 src/main/skills/skill-remove-transaction.test.ts create mode 100644 src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx create mode 100644 src/renderer/src/components/skills/SkillCloudManagementActions.tsx create mode 100644 src/renderer/src/components/skills/SkillInstallReviewContent.tsx create mode 100644 src/renderer/src/components/skills/SkillShareDialog.test.tsx create mode 100644 src/renderer/src/components/skills/SkillShareReviewContent.tsx create mode 100644 src/renderer/src/components/skills/skill-share-package-selection.ts create mode 100644 src/shared/skill-install-failure.test.ts create mode 100644 src/shared/skill-install-failure.ts diff --git a/docs/agent-skill-sharing-installation-plan.md b/docs/agent-skill-sharing-installation-plan.md index f8e375c567c..ed706eec1cf 100644 --- a/docs/agent-skill-sharing-installation-plan.md +++ b/docs/agent-skill-sharing-installation-plan.md @@ -212,6 +212,9 @@ Rules: - `skill/SKILL.md` is required and must parse successfully. - The installed folder receives only the contents of `skill/`. - Paths use `/` in the manifest and are converted with the executing host's path API. +- LF and CRLF non-executable text share one normalized package identity, while each immutable + archive retains and hashes its exact source bytes; executable and binary files always use exact + bytes for identity. - Names are normalized once during publication. Installation never silently renames a conflict. - V1 rejects symlinks and special files. A later package-builder feature may safely dereference internal links after proving they remain within the source root. @@ -636,6 +639,8 @@ and test the same contract accurately. - Share creation, resolution, revocation, and access management. - Download grant creation. - Package catalog and version lookup. +- Manager-only package details include current user/organization access and active, unexpired + share records; non-managers never receive that management metadata. Exact files follow the repository that owns Orca Cloud APIs; desktop contracts stay provider neutral. @@ -652,6 +657,8 @@ neutral. - Share preview and access dialog. - Share completion dialog with copyable durable link. +- Access editing, active-link revocation, immutable-version deletion, and Cloud-package deletion + with explicit confirmation and copy explaining that installed copies remain local. - Install preview with destination, scope, coverage, and conflict state. - Install progress and structured outcome. - Installed version, update, removal, and incomplete-coverage actions on the Skills page. @@ -840,7 +847,8 @@ staging bytes. 4. If bytes match the receipt, use the normal installation transaction. 5. If bytes were modified, offer: - Keep local version. - - Export local bytes as a new package version when authorized. + - Publish local bytes as a new immutable version when exactly one non-missing Orca-managed + install matches the skill name and scope; Cloud rechecks ownership of that stable package ID. - Replace and discard local changes after explicit confirmation. 6. Reconcile all recorded aliases and fallback copies. 7. Preserve the previous package version in Cloud; rollback is a normal install of that immutable diff --git a/src/main/ipc/skill-cloud-ipc-handlers.ts b/src/main/ipc/skill-cloud-ipc-handlers.ts index 07cdcd6deda..e8dfe685a5c 100644 --- a/src/main/ipc/skill-cloud-ipc-handlers.ts +++ b/src/main/ipc/skill-cloud-ipc-handlers.ts @@ -7,11 +7,7 @@ import { SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } from '../../shared/skill-install-capability' import { - ManagedSkillInstallListSchema, SkillInstallDestinationSchema, - SkillInstallPreviewSchema, - SkillInstallResultSchema, - SkillPackageIdentitySchema, type SkillInstallRequest } from '../../shared/skill-install-contract' import { SkillDiscoveryTargetSchema, type SkillDiscoveryResult } from '../../shared/skills' @@ -20,15 +16,14 @@ import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { installSkillOnRemoteRuntime } from '../skills/skill-remote-install-service' import { classifySkillCloudInstallTarget } from '../skills/skill-cloud-install-target' import { SkillSharePreparationService } from '../skills/skill-share-preparation-service' -import { supportsSkillRuntimeManagement } from '../skills/skill-runtime-capability' -import { listWslDistrosAsync } from '../wsl' +import { skillInstallFailureFromError } from '../skills/skill-install-operation-error' import { callRuntimeEnvironment, getRuntimeEnvironmentStatus } from './runtime-environment-transport-routing' +import { registerSkillInstallManagementIpcHandlers } from './skill-install-management-ipc-handlers' const environmentIdSchema = z.string().min(1).max(128) -const skillNameSchema = z.string().regex(/^[a-z0-9][a-z0-9-]{0,63}$/) const sharePrepareSchema = z .object({ @@ -48,6 +43,7 @@ const sharePublishSchema = z .strict() const installDestinationFields = { + operationId: z.string().min(1).max(128).optional(), environmentId: environmentIdSchema.optional(), destination: SkillInstallDestinationSchema, conflictResolution: z @@ -71,21 +67,18 @@ const packageVersionInstallSchema = z }) .strict() -const installPreviewSchema = z +const replaceAccessSchema = z .object({ - environmentId: environmentIdSchema.optional(), - package: SkillPackageIdentitySchema, - name: skillNameSchema, - destination: SkillInstallDestinationSchema + packageId: z.string().min(1).max(128), + userIds: z.array(z.string().min(1).max(128)).max(100), + shareWithOrganization: z.boolean() }) .strict() -const removeSchema = z +const packageVersionSchema = z .object({ - environmentId: environmentIdSchema.optional(), - name: skillNameSchema, - destination: SkillInstallDestinationSchema, - conflictResolution: z.enum(['replace-and-discard-local', 'cancel']).optional() + packageId: z.string().min(1).max(128), + versionId: z.string().min(1).max(128) }) .strict() @@ -95,7 +88,7 @@ async function installGrant( input: z.infer | z.infer ) { const request: SkillInstallRequest = { - operationId: randomUUID(), + operationId: input.operationId ?? randomUUID(), package: { packageId: grant.version.packageId, versionId: grant.version.versionId, @@ -111,26 +104,45 @@ async function installGrant( destination: input.destination, conflictResolution: input.conflictResolution } - if (!input.environmentId) { - return { status: 'ok' as const, value: await runtime.installSharedSkillRequest(request) } - } - const userDataPath = app.getPath('userData') - const status = await getRuntimeEnvironmentStatus(userDataPath, input.environmentId, 15_000) - if ( - status.ok !== true || - status.result.capabilities?.includes(SKILL_INSTALL_CAPABILITY) !== true - ) { - return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } - } - return { - status: 'ok' as const, - value: await installSkillOnRemoteRuntime({ - userDataPath, - environmentId: input.environmentId, - request, - capabilities: status.result.capabilities ?? [], - requireHttps: app.isPackaged - }) + try { + if (!input.environmentId) { + return { status: 'ok' as const, value: await runtime.installSharedSkillRequest(request) } + } + const userDataPath = app.getPath('userData') + const status = await getRuntimeEnvironmentStatus(userDataPath, input.environmentId, 15_000) + if ( + status.ok !== true || + status.result.capabilities?.includes(SKILL_INSTALL_CAPABILITY) !== true + ) { + return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } + } + return { + status: 'ok' as const, + value: await installSkillOnRemoteRuntime({ + userDataPath, + environmentId: input.environmentId, + request, + capabilities: status.result.capabilities ?? [], + requireHttps: app.isPackaged + }) + } + } catch (error) { + const failure = skillInstallFailureFromError(error) + if (!failure) { + throw error + } + return { + status: 'ok' as const, + value: { + operationId: request.operationId, + status: failure.category === 'cancelled' ? ('cancelled' as const) : ('failed' as const), + name: grant.version.name, + packageDigest: request.package.packageDigest, + placements: [], + errorCategory: failure.code, + failure + } + } } } @@ -198,108 +210,52 @@ function registerCloudInstallHandlers(runtime: OrcaRuntimeService): void { ) return grant.status === 'ok' ? installGrant(runtime, grant.value, input) : grant }) - ipcMain.handle('skills:getPackage', (_event, packageId: unknown) => - runtime.getSkillPackage(z.string().min(1).max(128).parse(packageId), {}) - ) -} - -function registerInstallManagementHandlers(runtime: OrcaRuntimeService): void { - ipcMain.handle('skills:listWslDistros', async (_event, environmentIdValue: unknown) => { - const environmentId = environmentIdSchema.optional().parse(environmentIdValue) - if (!environmentId) { - return listWslDistrosAsync() + ipcMain.handle('skills:cancelInstall', async (_event, value: unknown) => { + const input = z + .object({ + operationId: z.string().min(1).max(128), + environmentId: environmentIdSchema.optional() + }) + .strict() + .parse(value) + if (!input.environmentId || input.environmentId.startsWith('ssh:')) { + return { cancelled: runtime.cancelSharedSkillInstall(input.operationId) } } const response = await callRuntimeEnvironment( app.getPath('userData'), - environmentId, - 'host.wsl.listDistros', - {}, + input.environmentId, + 'skills.cancelInstall', + { operationId: input.operationId }, 15_000 ) - return response.ok === true && Array.isArray(response.result) - ? response.result.filter((distro): distro is string => typeof distro === 'string') - : [] + return response.ok === true && response.result && typeof response.result === 'object' + ? { cancelled: (response.result as { cancelled?: unknown }).cancelled === true } + : { cancelled: false } }) - ipcMain.handle('skills:previewInstall', async (_event, value: unknown) => { - const input = installPreviewSchema.parse(value) - const request = { package: input.package, name: input.name, destination: input.destination } - if (!input.environmentId) { - return { - status: 'ok' as const, - value: await runtime.previewSharedSkillInstallRequest(request) - } - } - const userDataPath = app.getPath('userData') - if (!(await supportsSkillRuntimeManagement(userDataPath, input.environmentId))) { - return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } - } - const response = await callRuntimeEnvironment( - userDataPath, - input.environmentId, - 'skills.previewInstall', - request, - 30_000 + ipcMain.handle('skills:getPackage', (_event, packageId: unknown) => + runtime.getSkillPackage(z.string().min(1).max(128).parse(packageId), {}) + ) + ipcMain.handle('skills:replacePackageAccess', (_event, value: unknown) => { + const input = replaceAccessSchema.parse(value) + return runtime.replaceSkillPackageAccess( + input.packageId, + { + userIds: input.userIds, + shareWithOrganization: input.shareWithOrganization + }, + {} ) - if (response.ok !== true) { - throw new Error(`skill-preview-remote-${response.error.code}`) - } - return { status: 'ok' as const, value: SkillInstallPreviewSchema.parse(response.result) } }) - ipcMain.handle('skills:removeInstall', async (_event, value: unknown) => { - const input = removeSchema.parse(value) - const request = { - operationId: randomUUID(), - name: input.name, - destination: input.destination, - conflictResolution: input.conflictResolution - } - if (!input.environmentId) { - return { - status: 'ok' as const, - value: await runtime.removeSharedSkillInstallRequest(request) - } - } - const userDataPath = app.getPath('userData') - if (!(await supportsSkillRuntimeManagement(userDataPath, input.environmentId))) { - return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } - } - const response = await callRuntimeEnvironment( - userDataPath, - input.environmentId, - 'skills.removeInstall', - request, - 5 * 60_000 - ) - if (response.ok !== true) { - throw new Error(`skill-remove-remote-${response.error.code}`) - } - return { status: 'ok' as const, value: SkillInstallResultSchema.parse(response.result) } - }) - ipcMain.handle('skills:listManagedInstalls', async (_event, environmentIdValue: unknown) => { - const environmentId = environmentIdSchema.optional().parse(environmentIdValue) - if (!environmentId) { - return { status: 'ok' as const, value: await runtime.listManagedSkillInstalls() } - } - if (environmentId.startsWith('ssh:')) { - const value = await runtime.listManagedSkillInstalls(environmentId.slice('ssh:'.length)) - return { status: 'ok' as const, value } - } - const userDataPath = app.getPath('userData') - if (!(await supportsSkillRuntimeManagement(userDataPath, environmentId))) { - return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } - } - const response = await callRuntimeEnvironment( - userDataPath, - environmentId, - 'skills.listManagedInstalls', - {}, - 30_000 - ) - if (response.ok !== true) { - throw new Error(`skill-list-managed-remote-${response.error.code}`) - } - return { status: 'ok' as const, value: ManagedSkillInstallListSchema.parse(response.result) } + ipcMain.handle('skills:revokeShare', (_event, shareId: unknown) => + runtime.revokeSkillShare(z.string().min(1).max(128).parse(shareId), {}) + ) + ipcMain.handle('skills:deletePackageVersion', (_event, value: unknown) => { + const input = packageVersionSchema.parse(value) + return runtime.deleteSkillPackageVersion(input.packageId, input.versionId, {}) }) + ipcMain.handle('skills:deletePackage', (_event, packageId: unknown) => + runtime.deleteSkillPackage(z.string().min(1).max(128).parse(packageId), {}) + ) } export function registerSkillCloudIpcHandlers( @@ -308,5 +264,5 @@ export function registerSkillCloudIpcHandlers( ): void { registerSharingHandlers(runtime, discover) registerCloudInstallHandlers(runtime) - registerInstallManagementHandlers(runtime) + registerSkillInstallManagementIpcHandlers(runtime) } diff --git a/src/main/ipc/skill-install-management-ipc-handlers.ts b/src/main/ipc/skill-install-management-ipc-handlers.ts new file mode 100644 index 00000000000..17e8ce3fda1 --- /dev/null +++ b/src/main/ipc/skill-install-management-ipc-handlers.ts @@ -0,0 +1,133 @@ +import { randomUUID } from 'node:crypto' +import { app, ipcMain } from 'electron' +import { z } from 'zod' +import { SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } from '../../shared/skill-install-capability' +import { + ManagedSkillInstallListSchema, + SkillInstallDestinationSchema, + SkillInstallPreviewSchema, + SkillInstallResultSchema, + SkillPackageIdentitySchema +} from '../../shared/skill-install-contract' +import type { OrcaRuntimeService } from '../runtime/orca-runtime' +import { supportsSkillRuntimeManagement } from '../skills/skill-runtime-capability' +import { listWslDistrosAsync } from '../wsl' +import { callRuntimeEnvironment } from './runtime-environment-transport-routing' + +const environmentIdSchema = z.string().min(1).max(128) +const skillNameSchema = z.string().regex(/^[a-z0-9][a-z0-9-]{0,63}$/) +const installPreviewSchema = z + .object({ + environmentId: environmentIdSchema.optional(), + package: SkillPackageIdentitySchema, + name: skillNameSchema, + destination: SkillInstallDestinationSchema + }) + .strict() +const removeSchema = z + .object({ + environmentId: environmentIdSchema.optional(), + name: skillNameSchema, + destination: SkillInstallDestinationSchema, + conflictResolution: z.enum(['replace-and-discard-local', 'cancel']).optional() + }) + .strict() + +export function registerSkillInstallManagementIpcHandlers(runtime: OrcaRuntimeService): void { + ipcMain.handle('skills:listWslDistros', async (_event, environmentIdValue: unknown) => { + const environmentId = environmentIdSchema.optional().parse(environmentIdValue) + if (!environmentId) { + return listWslDistrosAsync() + } + const response = await callRuntimeEnvironment( + app.getPath('userData'), + environmentId, + 'host.wsl.listDistros', + {}, + 15_000 + ) + return response.ok === true && Array.isArray(response.result) + ? response.result.filter((distro): distro is string => typeof distro === 'string') + : [] + }) + ipcMain.handle('skills:previewInstall', async (_event, value: unknown) => { + const input = installPreviewSchema.parse(value) + const request = { package: input.package, name: input.name, destination: input.destination } + if (!input.environmentId) { + return { + status: 'ok' as const, + value: await runtime.previewSharedSkillInstallRequest(request) + } + } + const userDataPath = app.getPath('userData') + if (!(await supportsSkillRuntimeManagement(userDataPath, input.environmentId))) { + return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } + } + const response = await callRuntimeEnvironment( + userDataPath, + input.environmentId, + 'skills.previewInstall', + request, + 30_000 + ) + if (response.ok !== true) { + throw new Error(`skill-preview-remote-${response.error.code}`) + } + return { status: 'ok' as const, value: SkillInstallPreviewSchema.parse(response.result) } + }) + ipcMain.handle('skills:removeInstall', async (_event, value: unknown) => { + const input = removeSchema.parse(value) + const request = { + operationId: randomUUID(), + name: input.name, + destination: input.destination, + conflictResolution: input.conflictResolution + } + if (!input.environmentId) { + return { + status: 'ok' as const, + value: await runtime.removeSharedSkillInstallRequest(request) + } + } + const userDataPath = app.getPath('userData') + if (!(await supportsSkillRuntimeManagement(userDataPath, input.environmentId))) { + return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } + } + const response = await callRuntimeEnvironment( + userDataPath, + input.environmentId, + 'skills.removeInstall', + request, + 5 * 60_000 + ) + if (response.ok !== true) { + throw new Error(`skill-remove-remote-${response.error.code}`) + } + return { status: 'ok' as const, value: SkillInstallResultSchema.parse(response.result) } + }) + ipcMain.handle('skills:listManagedInstalls', async (_event, environmentIdValue: unknown) => { + const environmentId = environmentIdSchema.optional().parse(environmentIdValue) + if (!environmentId) { + return { status: 'ok' as const, value: await runtime.listManagedSkillInstalls() } + } + if (environmentId.startsWith('ssh:')) { + const value = await runtime.listManagedSkillInstalls(environmentId.slice('ssh:'.length)) + return { status: 'ok' as const, value } + } + const userDataPath = app.getPath('userData') + if (!(await supportsSkillRuntimeManagement(userDataPath, environmentId))) { + return { status: 'unsupported' as const, message: SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE } + } + const response = await callRuntimeEnvironment( + userDataPath, + environmentId, + 'skills.listManagedInstalls', + {}, + 30_000 + ) + if (response.ok !== true) { + throw new Error(`skill-list-managed-remote-${response.error.code}`) + } + return { status: 'ok' as const, value: ManagedSkillInstallListSchema.parse(response.result) } + }) +} diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 375f1f326a9..108458435c6 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -3291,6 +3291,7 @@ export class OrcaRuntimeService { private artifactService: ArtifactCloudService | null = null private skillCloudService: SkillCloudService | null = null private skillUploadSessions: SkillUploadSessionService | null = null + private readonly skillInstallOperations = new Map() private readonly claudeAgentTeams = new ClaudeAgentTeamsService() private mobileDictation: { id: string @@ -4649,6 +4650,37 @@ export class OrcaRuntimeService { async installSharedSkillRequest( request: SkillInstallRequest, signal?: AbortSignal + ): Promise { + if (this.skillInstallOperations.has(request.operationId)) { + throw new Error('skill-install-operation-in-progress') + } + const controller = new AbortController() + const abort = (): void => controller.abort() + if (signal?.aborted) { + abort() + } else { + signal?.addEventListener('abort', abort, { once: true }) + } + this.skillInstallOperations.set(request.operationId, controller) + try { + return await this.executeSharedSkillInstall(request, controller.signal) + } finally { + signal?.removeEventListener('abort', abort) + if (this.skillInstallOperations.get(request.operationId) === controller) { + this.skillInstallOperations.delete(request.operationId) + } + } + } + + cancelSharedSkillInstall(operationId: string): boolean { + const operation = this.skillInstallOperations.get(operationId) + operation?.abort() + return Boolean(operation) + } + + private async executeSharedSkillInstall( + request: SkillInstallRequest, + signal: AbortSignal ): Promise { const runtimeId = this.getStatus().runtimeId const sshTarget = await this.resolveSkillSshTarget(request.destination) diff --git a/src/main/runtime/rpc/errors.test.ts b/src/main/runtime/rpc/errors.test.ts index 1d9208647b9..9d0888846d1 100644 --- a/src/main/runtime/rpc/errors.test.ts +++ b/src/main/runtime/rpc/errors.test.ts @@ -14,6 +14,27 @@ class LineageError extends Error { } describe('mapRuntimeError', () => { + it('preserves the stable skill failure category and retryability across RPC', () => { + expect( + mapRuntimeError( + 'req_1', + { runtimeId: 'runtime-1' }, + new Error('skill-download-transport-failed') + ) + ).toMatchObject({ + ok: false, + error: { + code: 'skill_install_failure', + message: 'skill-download-transport-failed', + data: { + category: 'transport', + code: 'skill-download-transport-failed', + retryable: true + } + } + }) + }) + it.each(['terminal_tab_close_timeout', 'terminal_tab_not_found', 'terminal_tab_pinned'])( 'preserves the durable terminal tab close failure %s', (code) => { diff --git a/src/main/runtime/rpc/errors.ts b/src/main/runtime/rpc/errors.ts index db6396b8e0d..b912f2f0fb1 100644 --- a/src/main/runtime/rpc/errors.ts +++ b/src/main/runtime/rpc/errors.ts @@ -8,6 +8,10 @@ import { COMPUTER_ERROR_CODES } from '../../../shared/runtime-types' import { LINEAR_ERROR_CODES } from '../../../shared/linear-agent-access' import { AGENT_SESSION_RPC_ERROR_CODES } from '../../../shared/agent-session-host-authority' import { ARTIFACT_SHARING_DISABLED_CODE } from '../../../shared/artifact-sharing-gate' +import { + SKILL_INSTALL_RPC_ERROR_CODE, + classifySkillInstallFailureCode +} from '../../../shared/skill-install-failure' export function successResponse(id: string, meta: RpcEnvelopeMeta, result: unknown): RpcSuccess { return { @@ -98,7 +102,8 @@ const STRUCTURED_RUNTIME_PASSTHROUGH_CODES: ReadonlySet = new Set([ 'stale_delivery', 'waiter_exists', 'invalid_argument', - ARTIFACT_SHARING_DISABLED_CODE + ARTIFACT_SHARING_DISABLED_CODE, + SKILL_INSTALL_RPC_ERROR_CODE ]) export function mapRuntimeError(id: string, meta: RpcEnvelopeMeta, error: unknown): RpcFailure { @@ -157,6 +162,16 @@ export function mapRuntimeError(id: string, meta: RpcEnvelopeMeta, error: unknow if (RUNTIME_PASSTHROUGH_CODES.has(message)) { return errorResponse(id, meta, message, message) } + const skillInstallFailure = classifySkillInstallFailureCode(message) + if (skillInstallFailure) { + return errorResponse( + id, + meta, + SKILL_INSTALL_RPC_ERROR_CODE, + skillInstallFailure.code, + skillInstallFailure + ) + } if (message === 'invalid_terminal_send') { return errorResponse(id, meta, 'invalid_argument', 'Missing terminal send payload') } diff --git a/src/main/runtime/rpc/methods/skills.test.ts b/src/main/runtime/rpc/methods/skills.test.ts index 3a9c2a779ef..f0799160f6e 100644 --- a/src/main/runtime/rpc/methods/skills.test.ts +++ b/src/main/runtime/rpc/methods/skills.test.ts @@ -111,6 +111,57 @@ describe('skills.install RPC', () => { await installMethod().handler(request, { runtime } as unknown as RpcContext) expect(installSharedSkillRequest).toHaveBeenCalledWith(request, undefined) }) + + it('downgrades cancelled results for clients without the additive result capability', async () => { + const cancelled = { + operationId: 'operation_1', + status: 'cancelled' as const, + name: 'example', + packageDigest: 'a'.repeat(64), + placements: [], + failure: { + category: 'cancelled' as const, + code: 'skill-install-cancelled', + retryable: true + } + } + const runtime = { installSharedSkillRequest: vi.fn(async () => cancelled) } + const request = { + operationId: 'operation_1', + package: { + packageId: 'package_1', + versionId: 'version_1', + packageDigest: 'a'.repeat(64), + archiveSha256: 'b'.repeat(64), + compressedBytes: 100 + }, + ingress: { + kind: 'download-grant' as const, + url: 'https://storage.googleapis.com/package', + expiresAt: '2026-08-11T12:00:00.000Z' + }, + destination: { scope: 'global' as const } + } + + await expect( + installMethod().handler(request, { runtime, clientCapabilities: [] } as unknown as RpcContext) + ).resolves.toMatchObject({ status: 'failed', errorCategory: 'skill-install-cancelled' }) + await expect( + installMethod().handler(request, { + runtime, + clientCapabilities: ['skills.install-result.v2'] + } as unknown as RpcContext) + ).resolves.toMatchObject({ status: 'cancelled', failure: { category: 'cancelled' } }) + }) + + it('routes cancellation to the destination runtime by operation ID', async () => { + const cancelSharedSkillInstall = vi.fn(() => true) + const result = await method('skills.cancelInstall').handler({ operationId: 'operation_1' }, { + runtime: { cancelSharedSkillInstall } + } as unknown as RpcContext) + expect(result).toEqual({ cancelled: true }) + expect(cancelSharedSkillInstall).toHaveBeenCalledWith('operation_1') + }) }) describe('skill management RPC', () => { diff --git a/src/main/runtime/rpc/methods/skills.ts b/src/main/runtime/rpc/methods/skills.ts index de42ab7dff2..945c98d77da 100644 --- a/src/main/runtime/rpc/methods/skills.ts +++ b/src/main/runtime/rpc/methods/skills.ts @@ -1,4 +1,5 @@ import { defineMethod, type RpcMethod } from '../core' +import { z } from 'zod' import { SkillDiscoveryTargetSchema } from '../../../../shared/skills' import { SkillInstallPreviewRequestSchema, @@ -14,6 +15,7 @@ import { discoverSkillsOnTarget, resolveSkillDiscoveryTarget } from '../../../skills/skill-discovery-target' +import { SKILL_INSTALL_RESULT_V2_CAPABILITY } from '../../../../shared/skill-install-capability' export const SKILL_METHODS: RpcMethod[] = [ defineMethod({ @@ -35,7 +37,28 @@ export const SKILL_METHODS: RpcMethod[] = [ defineMethod({ name: 'skills.install', params: SkillInstallRequestSchema, - handler: (params, { runtime, signal }) => runtime.installSharedSkillRequest(params, signal) + handler: async (params, { runtime, signal, clientCapabilities }) => { + const result = await runtime.installSharedSkillRequest(params, signal) + if ( + result.status === 'cancelled' && + !clientCapabilities?.includes(SKILL_INSTALL_RESULT_V2_CAPABILITY) + ) { + return { + ...result, + status: 'failed' as const, + errorCategory: result.failure?.code ?? 'skill-install-cancelled', + failure: undefined + } + } + return result + } + }), + defineMethod({ + name: 'skills.cancelInstall', + params: z.object({ operationId: z.string().min(1).max(128) }).strict(), + handler: (params, { runtime }) => ({ + cancelled: runtime.cancelSharedSkillInstall(params.operationId) + }) }), defineMethod({ name: 'skills.previewInstall', diff --git a/src/main/skills/skill-install-discovery-verification.ts b/src/main/skills/skill-install-discovery-verification.ts new file mode 100644 index 00000000000..9276f1ba3be --- /dev/null +++ b/src/main/skills/skill-install-discovery-verification.ts @@ -0,0 +1,123 @@ +import { dirname, posix, resolve } from 'node:path' +import type { SkillInstallResult, SkillPlacementResult } from '../../shared/skill-install-contract' +import type { SkillDiscoveryResult } from '../../shared/skills' +import { discoverSkills } from './discovery' +import { discoverSkillsInWsl } from './skill-discovery-wsl' + +function normalizedPath(path: string, wslDistro?: string): string { + const normalized = wslDistro ? path.replace(/\/+$/, '') : resolve(path) + return !wslDistro && process.platform === 'win32' + ? normalized.toLocaleLowerCase('en-US') + : normalized +} + +function placementIsDiscovered( + discovery: SkillDiscoveryResult, + placement: SkillPlacementResult, + skillName: string, + wslDistro?: string +): boolean { + const placementPath = normalizedPath(placement.path, wslDistro) + const placementRoot = normalizedPath( + wslDistro ? posix.dirname(placement.path) : dirname(placement.path), + wslDistro + ) + return discovery.skills.some((skill) => { + if (skill.name !== skillName) { + return false + } + if (normalizedPath(skill.directoryPath, wslDistro) === placementPath) { + return true + } + return (skill.rootPaths ?? [skill.rootPath]).some( + (root) => normalizedPath(root, wslDistro) === placementRoot + ) + }) +} + +async function discoverInstalledSkill(input: { + scope: 'global' | 'workspace' + homeDirectory: string + workspaceDirectory?: string + wslDistro?: string +}): Promise { + if (input.wslDistro) { + return discoverSkillsInWsl({ + distro: input.wslDistro, + homeDir: input.homeDirectory, + cwd: input.workspaceDirectory ?? input.homeDirectory + }) + } + return discoverSkills({ + homeDir: input.homeDirectory, + repos: [], + ...(input.scope === 'workspace' && input.workspaceDirectory + ? { cwd: input.workspaceDirectory } + : { includeCwd: false }) + }) +} + +export async function verifySkillInstallDiscovery(input: { + result: SkillInstallResult + scope: 'global' | 'workspace' + homeDirectory: string + workspaceDirectory?: string + wslDistro?: string + discover?: () => Promise +}): Promise { + const discovery = await (input.discover?.() ?? discoverInstalledSkill(input)).catch(() => null) + if (!discovery) { + return { + ...input.result, + status: 'failed', + errorCategory: 'skill-discovery-verification-failed', + failure: { + category: 'provider-placement', + code: 'skill-discovery-verification-failed', + retryable: true + } + } + } + + const placements = input.result.placements.map((placement) => { + if ( + (placement.status !== 'installed' && placement.status !== 'unchanged') || + placementIsDiscovered(discovery, placement, input.result.name, input.wslDistro) + ) { + return placement + } + return { + ...placement, + status: 'failed' as const, + errorCategory: 'skill-discovery-placement-missing', + failure: { + category: 'provider-placement' as const, + code: 'skill-discovery-placement-missing', + retryable: true + } + } + }) + const canonical = placements.find((placement) => placement.topology === 'canonical-copy') + if (!canonical || canonical.status === 'failed') { + return { + ...input.result, + status: 'failed', + placements, + errorCategory: 'skill-discovery-canonical-missing', + failure: { + category: 'provider-placement', + code: 'skill-discovery-canonical-missing', + retryable: true + } + } + } + return { + ...input.result, + status: placements.some( + (placement) => placement.status === 'failed' || placement.status === 'skipped' + ) + ? 'partial' + : input.result.status, + placements + } +} diff --git a/src/main/skills/skill-install-lock.ts b/src/main/skills/skill-install-lock.ts index 8a77b8c7887..52f8b408eb1 100644 --- a/src/main/skills/skill-install-lock.ts +++ b/src/main/skills/skill-install-lock.ts @@ -1,6 +1,8 @@ import { randomUUID } from 'node:crypto' import { mkdir, open, readFile, rm, stat } from 'node:fs/promises' import { dirname } from 'node:path' +import { SKILL_INSTALL_BUSY_FAILURE } from '../../shared/skill-install-failure' +import { SkillInstallOperationError } from './skill-install-operation-error' const LOCK_RETRY_MS = 50 const LOCK_STALE_MS = 30 * 60 * 1000 @@ -74,7 +76,7 @@ export async function acquireSkillInstallLock(input: { } await removeStaleLock(input.path) if (Date.now() >= deadline) { - throw new Error('skill-install-busy') + throw new SkillInstallOperationError(SKILL_INSTALL_BUSY_FAILURE) } await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_MS)) } diff --git a/src/main/skills/skill-install-operation-error.ts b/src/main/skills/skill-install-operation-error.ts new file mode 100644 index 00000000000..66945544b91 --- /dev/null +++ b/src/main/skills/skill-install-operation-error.ts @@ -0,0 +1,42 @@ +import { + SKILL_INSTALL_RPC_ERROR_CODE, + SkillInstallFailureSchema, + classifySkillInstallFailureCode, + type SkillInstallFailure +} from '../../shared/skill-install-failure' + +export class SkillInstallOperationError extends Error { + readonly code = SKILL_INSTALL_RPC_ERROR_CODE + readonly data: SkillInstallFailure + + constructor(failure: SkillInstallFailure, options?: ErrorOptions) { + super(failure.code, options) + this.name = 'SkillInstallOperationError' + this.data = SkillInstallFailureSchema.parse(failure) + } +} + +export function skillInstallFailureFromError(error: unknown): SkillInstallFailure | null { + if (error instanceof SkillInstallOperationError) { + return error.data + } + if (error instanceof Error) { + const classified = classifySkillInstallFailureCode(error.message) + if (classified) { + return classified + } + const code = (error as NodeJS.ErrnoException).code + if (code) { + return { + category: 'filesystem', + code: 'skill-install-filesystem-failed', + retryable: code === 'EBUSY' || code === 'EACCES' || code === 'EPERM' + } + } + } + if (!error || typeof error !== 'object' || !('data' in error)) { + return null + } + const parsed = SkillInstallFailureSchema.safeParse((error as { data: unknown }).data) + return parsed.success ? parsed.data : null +} diff --git a/src/main/skills/skill-install-recovery.test.ts b/src/main/skills/skill-install-recovery.test.ts new file mode 100644 index 00000000000..7c1b5d8f384 --- /dev/null +++ b/src/main/skills/skill-install-recovery.test.ts @@ -0,0 +1,169 @@ +import { mkdir, mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { createSkillPackageArchive } from './skill-package-creation' +import { extractSkillPackageArchive } from './skill-package-extraction' +import { + readSkillInstallReceipt, + writeSkillInstallReceipt, + writeSkillStateFile, + type SkillInstallReceiptV1 +} from './skill-install-provenance' +import { + recoverSkillInstallTransaction, + skillInstallJournalPath, + type SkillInstallJournalV1 +} from './skill-install-recovery' +import { installLocalSkillPackage } from './skill-install-transaction' + +const roots: string[] = [] + +async function packageVersion(root: string, versionId: string, body: string) { + const source = join(root, `source-${versionId}`) + await mkdir(source) + await writeFile( + join(source, 'SKILL.md'), + `---\nname: recovery-skill\ndescription: Recovery\n---\n\n${body}\n` + ) + return createSkillPackageArchive({ + sourceDirectory: source, + archivePath: join(root, `${versionId}.tar.gz`), + packageId: 'package_1', + versionId + }) +} + +async function interruptedUpdate(phase: SkillInstallJournalV1['phase']): Promise<{ + root: string + canonicalPath: string + stateDirectory: string + journal: SkillInstallJournalV1 +}> { + const root = await mkdtemp(join(tmpdir(), 'orca-skill-recovery-test-')) + roots.push(root) + const first = await packageVersion(root, 'version_1', '# First') + const second = await packageVersion(root, 'version_2', '# Second') + const destinationRoot = join(root, 'skills') + const stateDirectory = join(root, 'state') + const input = { + operationId: 'install-first', + archivePath: first.archivePath, + destinationRoot, + stateDirectory, + scope: 'global' as const, + destinationIdentity: 'global:test', + hostIdentity: 'test', + expectedPackageDigest: first.manifest.packageDigest + } + await installLocalSkillPackage(input) + const canonicalPath = join(destinationRoot, 'recovery-skill') + const previous = await readSkillInstallReceipt(stateDirectory, canonicalPath) + if (!previous) { + throw new Error('fixture receipt missing') + } + const extractionPath = join(destinationRoot, '.orca-skill-extract-recovery') + const stagingPath = join(destinationRoot, '.recovery-skill.orca-staging-recovery') + const backupPath = join(destinationRoot, '.recovery-skill.orca-backup-recovery') + await extractSkillPackageArchive({ + archivePath: second.archivePath, + destinationDirectory: extractionPath, + expectedPackageDigest: second.manifest.packageDigest + }) + await rename(join(extractionPath, 'skill'), stagingPath) + await rename(canonicalPath, backupPath) + await rename(stagingPath, canonicalPath) + const receipt: SkillInstallReceiptV1 = { + ...previous, + versionId: second.manifest.versionId, + packageDigest: second.manifest.packageDigest, + archiveSha256: second.archiveSha256, + previousVersionId: previous.versionId, + installedAt: new Date().toISOString(), + fileModes: second.manifest.files.map((file) => ({ + path: file.path, + executable: file.executable + })) + } + const journal: SkillInstallJournalV1 = { + schemaVersion: 1, + operation: 'install', + phase, + canonicalPath, + extractionPath, + stagingPath, + backupPath, + backupDigest: first.manifest.packageDigest, + stagingFileModes: second.manifest.files, + backupFileModes: first.manifest.files, + receipt + } + await writeSkillStateFile(skillInstallJournalPath(stateDirectory, canonicalPath), journal) + return { root, canonicalPath, stateDirectory, journal } +} + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('skill install recovery', () => { + it('publishes provenance for a placed destination that has no new receipt', async () => { + const fixture = await interruptedUpdate('canonical-placed') + + await recoverSkillInstallTransaction(fixture.stateDirectory, fixture.canonicalPath) + + expect( + (await readSkillInstallReceipt(fixture.stateDirectory, fixture.canonicalPath))?.versionId + ).toBe('version_2') + expect(await readFile(join(fixture.canonicalPath, 'SKILL.md'), 'utf8')).toContain('# Second') + await expect(readFile(fixture.journal.backupPath)).rejects.toMatchObject({ code: 'ENOENT' }) + await expect( + readFile(skillInstallJournalPath(fixture.stateDirectory, fixture.canonicalPath)) + ).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('finalizes a published receipt with an incomplete journal', async () => { + const fixture = await interruptedUpdate('receipt-published') + await writeSkillInstallReceipt(fixture.stateDirectory, fixture.journal.receipt) + + await recoverSkillInstallTransaction(fixture.stateDirectory, fixture.canonicalPath) + + expect( + (await readSkillInstallReceipt(fixture.stateDirectory, fixture.canonicalPath))?.versionId + ).toBe('version_2') + await expect( + readFile(skillInstallJournalPath(fixture.stateDirectory, fixture.canonicalPath)) + ).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('rejects journal paths outside the canonical parent without removing them', async () => { + const fixture = await interruptedUpdate('canonical-placed') + const outside = join(fixture.root, 'outside') + await mkdir(outside) + await writeFile(join(outside, 'keep.txt'), 'keep') + await writeSkillStateFile( + skillInstallJournalPath(fixture.stateDirectory, fixture.canonicalPath), + { + ...fixture.journal, + backupPath: outside + } + ) + + await expect( + recoverSkillInstallTransaction(fixture.stateDirectory, fixture.canonicalPath) + ).rejects.toThrow('skill-install-journal-invalid') + expect(await readFile(join(outside, 'keep.txt'), 'utf8')).toBe('keep') + }) + + it('preserves a journal path whose bytes no longer match the recorded identity', async () => { + const fixture = await interruptedUpdate('canonical-placed') + await writeFile(join(fixture.journal.backupPath, 'local.md'), 'changed after crash') + + await expect( + recoverSkillInstallTransaction(fixture.stateDirectory, fixture.canonicalPath) + ).rejects.toThrow('skill-install-recovery-conflict') + expect(await readFile(join(fixture.journal.backupPath, 'local.md'), 'utf8')).toBe( + 'changed after crash' + ) + }) +}) diff --git a/src/main/skills/skill-install-recovery.ts b/src/main/skills/skill-install-recovery.ts index 0935e05db04..c2ce1046aff 100644 --- a/src/main/skills/skill-install-recovery.ts +++ b/src/main/skills/skill-install-recovery.ts @@ -1,5 +1,5 @@ import { lstat, readFile, rm } from 'node:fs/promises' -import { join } from 'node:path' +import { basename, dirname, join, resolve } from 'node:path' import { skillInstallStateKey, writeSkillInstallReceipt, @@ -26,6 +26,9 @@ export type SkillInstallJournalV1 = { extractionPath: string stagingPath: string backupPath: string + backupDigest: string | null + stagingFileModes: SkillInstalledFileMode[] + backupFileModes: SkillInstalledFileMode[] receipt: SkillInstallReceiptV1 } @@ -42,15 +45,33 @@ function isInstallJournal(value: unknown, canonicalPath: string): value is Skill return false } const journal = value as Partial + const parent = dirname(resolve(canonicalPath)) + const name = basename(canonicalPath) + const ownedPath = (path: unknown, prefix: string): path is string => + typeof path === 'string' && + dirname(resolve(path)) === parent && + basename(path).startsWith(prefix) return ( journal.schemaVersion === 1 && journal.operation === 'install' && typeof journal.phase === 'string' && journal.canonicalPath === canonicalPath && - typeof journal.extractionPath === 'string' && - typeof journal.stagingPath === 'string' && - typeof journal.backupPath === 'string' && - Boolean(journal.receipt) + ownedPath(journal.extractionPath, '.orca-skill-extract-') && + ownedPath(journal.stagingPath, `.${name}.orca-staging-`) && + ownedPath(journal.backupPath, `.${name}.orca-backup-`) && + journal.extractionPath !== journal.stagingPath && + journal.extractionPath !== journal.backupPath && + journal.stagingPath !== journal.backupPath && + (journal.backupDigest === null || + (typeof journal.backupDigest === 'string' && /^[a-f0-9]{64}$/.test(journal.backupDigest))) && + Array.isArray(journal.stagingFileModes) && + Array.isArray(journal.backupFileModes) && + [...journal.stagingFileModes, ...journal.backupFileModes].every( + (file) => typeof file.path === 'string' && typeof file.executable === 'boolean' + ) && + Boolean(journal.receipt) && + journal.receipt?.canonicalPath === canonicalPath && + typeof journal.receipt?.packageDigest === 'string' ) } @@ -92,8 +113,36 @@ export async function cleanSkillInstallJournalFiles( filesystem: SkillInstallFilesystem = nativeSkillInstallFilesystem ): Promise { await filesystem.remove(journal.extractionPath) - await filesystem.remove(journal.stagingPath) - await filesystem.remove(journal.backupPath) + await removeJournalOwnedSkill( + journal.stagingPath, + journal.receipt.packageDigest, + journal.stagingFileModes, + filesystem + ) + await removeJournalOwnedSkill( + journal.backupPath, + journal.backupDigest, + journal.backupFileModes, + filesystem + ) +} + +async function removeJournalOwnedSkill( + path: string, + expectedDigest: string | null, + files: readonly SkillInstalledFileMode[], + filesystem: SkillInstallFilesystem +): Promise { + if (!(await skillInstallPathExists(path))) { + return + } + if ( + !expectedDigest || + !(await skillInstallDestinationMatches(path, expectedDigest, filesystem, files)) + ) { + throw new Error('skill-install-recovery-conflict') + } + await filesystem.remove(path) } export async function recoverSkillInstallTransaction( @@ -123,6 +172,17 @@ export async function recoverSkillInstallTransaction( return } if (!destinationExists && backupExists) { + if ( + !journal.backupDigest || + !(await skillInstallDestinationMatches( + journal.backupPath, + journal.backupDigest, + filesystem, + journal.backupFileModes + )) + ) { + throw new Error('skill-install-recovery-conflict') + } await filesystem.rename(journal.backupPath, canonicalPath) await filesystem.remove(journal.extractionPath) await filesystem.remove(journal.stagingPath) diff --git a/src/main/skills/skill-install-request-service.test.ts b/src/main/skills/skill-install-request-service.test.ts index 984ff4f6f3b..aaf10885dbf 100644 --- a/src/main/skills/skill-install-request-service.test.ts +++ b/src/main/skills/skill-install-request-service.test.ts @@ -1,5 +1,5 @@ import { createHash } from 'node:crypto' -import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' @@ -90,4 +90,23 @@ describe('executeSkillInstallRequest', () => { ) ).rejects.toThrow('skill-install-local-ingress-rejected') }) + + it('returns a structured cancelled result without leaving partial ingress bytes', async () => { + const { request, dependencies } = await fixture() + const controller = new AbortController() + controller.abort() + + const result = await executeSkillInstallRequest(request, { + ...dependencies, + signal: controller.signal + }) + + expect(result).toMatchObject({ + status: 'cancelled', + errorCategory: 'skill-download-cancelled', + failure: { category: 'cancelled', retryable: true } + }) + const downloads = join(dependencies.stateDirectory, 'skill-installs', 'downloads') + expect(await readdir(downloads).catch(() => [])).toEqual([]) + }) }) diff --git a/src/main/skills/skill-install-request-service.ts b/src/main/skills/skill-install-request-service.ts index ee27b8446eb..82dd749aaf3 100644 --- a/src/main/skills/skill-install-request-service.ts +++ b/src/main/skills/skill-install-request-service.ts @@ -12,6 +12,7 @@ import { installSharedSkill } from './skill-install-service' import { downloadSkillPackageGrant } from './skill-package-download' import { detectSkillProvidersInWsl } from './skill-wsl-provider-detection' import { createWslSkillInstallFilesystem } from './skill-wsl-install-filesystem' +import { skillInstallFailureFromError } from './skill-install-operation-error' type StagedSkillPackage = { archivePath: string @@ -71,8 +72,9 @@ export async function executeSkillInstallRequest( request.destination, dependencies.authority ) - const ingress = await resolveIngress(request, dependencies) + let ingress: StagedSkillPackage | null = null try { + ingress = await resolveIngress(request, dependencies) const detectedProviders = destination.wslDistro ? await detectSkillProvidersInWsl(destination.wslDistro) : await dependencies.detectProviders() @@ -99,9 +101,24 @@ export async function executeSkillInstallRequest( expectedVersionId: request.package.versionId, conflictResolution: request.conflictResolution, filesystem, - wslDistro: destination.wslDistro + wslDistro: destination.wslDistro, + signal: dependencies.signal }) + } catch (error) { + const failure = skillInstallFailureFromError(error) + if (failure?.category !== 'cancelled') { + throw error + } + return { + operationId: request.operationId, + status: 'cancelled', + name: request.package.packageId, + packageDigest: request.package.packageDigest, + placements: [], + errorCategory: failure.code, + failure + } } finally { - await ingress.cleanup() + await ingress?.cleanup() } } diff --git a/src/main/skills/skill-install-service.test.ts b/src/main/skills/skill-install-service.test.ts index 729feea19fe..71a9d47052d 100644 --- a/src/main/skills/skill-install-service.test.ts +++ b/src/main/skills/skill-install-service.test.ts @@ -84,6 +84,24 @@ describe('skill install service', () => { expect(await readFile(join(claude, 'SKILL.md'), 'utf8')).toBe('unowned') }) + it('does not report success when normal skill discovery cannot observe the canonical copy', async () => { + const { root, input } = await fixture() + + const result = await installSharedSkill({ + ...input, + discover: async () => ({ skills: [], sources: [], scannedAt: Date.now() }) + }) + + expect(result).toMatchObject({ + status: 'failed', + errorCategory: 'skill-discovery-canonical-missing', + failure: { category: 'provider-placement', retryable: true } + }) + expect( + await readFile(join(root, 'home', '.agents', 'skills', 'test-skill', 'SKILL.md'), 'utf8') + ).toContain('# Test') + }) + it('removes the canonical skill and its owned provider alias', async () => { const { root, input } = await fixture() await installSharedSkill(input) diff --git a/src/main/skills/skill-install-service.ts b/src/main/skills/skill-install-service.ts index 562aa788e51..4f39ac274f6 100644 --- a/src/main/skills/skill-install-service.ts +++ b/src/main/skills/skill-install-service.ts @@ -10,6 +10,8 @@ import { reconcileSkillProviderPlacement } from './skill-placement-reconciliatio import { resolveSkillProviderDestinations } from './skill-provider-destinations' import { removeLocalSharedSkill } from './skill-remove-transaction' import type { SkillInstallFilesystem } from './skill-install-filesystem' +import { verifySkillInstallDiscovery } from './skill-install-discovery-verification' +import type { SkillDiscoveryResult } from '../../shared/skills' export type SkillInstallServiceInput = Omit< LocalSkillInstallInput, @@ -22,6 +24,8 @@ export type SkillInstallServiceInput = Omit< detectedProviders: readonly string[] filesystem?: SkillInstallFilesystem wslDistro?: string + discover?: () => Promise + signal?: AbortSignal } export type SkillRemoveServiceInput = { @@ -63,7 +67,8 @@ function localInput(input: SkillInstallServiceInput): LocalSkillInstallInput { expectedVersionId: input.expectedVersionId, conflictResolution: input.conflictResolution, filesystem: input.filesystem, - wslDistro: input.wslDistro + wslDistro: input.wslDistro, + signal: input.signal } } @@ -86,7 +91,12 @@ export async function installSharedSkill( preview.canonicalPath ) const result = await installLocalSkillPackage(request) - if (!result.canonicalPath || result.status === 'conflict' || result.status === 'failed') { + if ( + !result.canonicalPath || + result.status === 'conflict' || + result.status === 'failed' || + result.status === 'cancelled' + ) { return result } const placements: SkillPlacementResult[] = [canonicalPlacement(result)] @@ -97,6 +107,21 @@ export async function installSharedSkill( detectedProviders: input.detectedProviders }) for (const destination of destinations) { + if (input.signal?.aborted) { + placements.push({ + provider: destination.provider, + path: join(destination.rootPath, result.name), + topology: 'independent-copy', + status: 'skipped', + errorCategory: 'skill-placement-cancelled', + failure: { + category: 'cancelled', + code: 'skill-placement-cancelled', + retryable: true + } + }) + continue + } const placement = await reconcileSkillProviderPlacement({ canonicalPath: result.canonicalPath, skillName: result.name, @@ -118,7 +143,14 @@ export async function installSharedSkill( const incomplete = placements.some( (placement) => placement.status === 'failed' || placement.status === 'skipped' ) - return { ...result, status: incomplete ? 'partial' : result.status, placements } + return verifySkillInstallDiscovery({ + result: { ...result, status: incomplete ? 'partial' : result.status, placements }, + scope: input.scope, + homeDirectory: input.homeDirectory, + workspaceDirectory: input.workspaceDirectory, + wslDistro: input.wslDistro, + discover: input.discover + }) } export async function removeSharedSkill( diff --git a/src/main/skills/skill-install-transaction-result.ts b/src/main/skills/skill-install-transaction-result.ts new file mode 100644 index 00000000000..8c84746e592 --- /dev/null +++ b/src/main/skills/skill-install-transaction-result.ts @@ -0,0 +1,104 @@ +import type { SkillInstallResult } from '../../shared/skill-install-contract' +import type { SkillPackageManifestV1 } from '../../shared/skill-package-manifest' +import { skillInstallFailureFromError } from './skill-install-operation-error' +import type { SkillCanonicalState } from './skill-install-planner' +import type { SkillInstallReceiptV1 } from './skill-install-provenance' +import type { LocalSkillInstallInput } from './skill-install-transaction' + +export function skillInstallConflictResult( + operationId: string, + manifest: SkillPackageManifestV1, + state: SkillCanonicalState, + code = `skill-install-conflict-${state.kind}` +): SkillInstallResult { + const kind = + state.kind === 'modified' || + state.kind === 'unowned' || + state.kind === 'external-link' || + state.kind === 'name-collision' + ? state.kind + : 'modified' + return { + operationId, + status: 'conflict', + name: manifest.name, + packageDigest: manifest.packageDigest, + placements: [], + conflict: { + kind, + ...('digest' in state && state.digest ? { existingDigest: state.digest } : {}) + }, + errorCategory: code, + failure: { category: 'conflict', code, retryable: false } + } +} + +export function skillInstallFailureResult( + input: LocalSkillInstallInput, + manifest: SkillPackageManifestV1, + canonicalPath: string, + error: unknown +): SkillInstallResult | null { + const failure = skillInstallFailureFromError(error) + if (!failure || (failure.code !== 'skill-install-busy' && failure.category !== 'cancelled')) { + return null + } + return { + operationId: input.operationId, + status: failure.category === 'cancelled' ? 'cancelled' : 'failed', + name: manifest.name, + packageDigest: manifest.packageDigest, + canonicalPath, + placements: [], + errorCategory: failure.code, + failure + } +} + +export function skillInstallReplacementAllowed( + state: SkillCanonicalState, + input: LocalSkillInstallInput +): boolean { + if (state.kind === 'missing' || state.kind === 'clean-update') { + return input.conflictResolution !== 'cancel' + } + return ( + (state.kind === 'modified' || state.kind === 'unowned') && + input.conflictResolution === 'replace-and-discard-local' + ) +} + +export function createSkillInstallReceipt(input: { + request: LocalSkillInstallInput + manifest: SkillPackageManifestV1 + archiveSha256: string + canonicalPath: string + previous: SkillInstallReceiptV1 | null +}): SkillInstallReceiptV1 { + return { + schemaVersion: 1, + packageId: input.manifest.packageId, + versionId: input.manifest.versionId, + packageDigest: input.manifest.packageDigest, + archiveSha256: input.archiveSha256, + scope: input.request.scope, + destinationIdentity: input.request.destinationIdentity, + canonicalPath: input.canonicalPath, + placements: [ + { + provider: 'agent-skills', + path: input.canonicalPath, + topology: 'canonical-copy', + status: 'installed' + } + ], + ...(input.previous ? { previousVersionId: input.previous.versionId } : {}), + installedAt: new Date().toISOString(), + hostIdentity: input.request.hostIdentity, + fileModes: input.manifest.files.map((file) => ({ + path: file.path, + executable: file.executable + })), + ...(input.request.wslDistro ? { wslDistro: input.request.wslDistro } : {}) + } +} diff --git a/src/main/skills/skill-install-transaction.test.ts b/src/main/skills/skill-install-transaction.test.ts index e672677974d..6cdcb065b33 100644 --- a/src/main/skills/skill-install-transaction.test.ts +++ b/src/main/skills/skill-install-transaction.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { lstat, mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' @@ -9,6 +9,8 @@ import { type LocalSkillInstallInput } from './skill-install-transaction' import { nativeSkillInstallFilesystem } from './skill-install-filesystem' +import { acquireSkillInstallLock } from './skill-install-lock' +import { skillInstallStateKey } from './skill-install-provenance' const temporaryDirectories: string[] = [] @@ -147,4 +149,89 @@ describe('skill install transaction', () => { '# Second' ) }) + + it('returns a retryable failure when another process owns the destination lock', async () => { + const root = await temporaryDirectory() + const archive = await packageVersion(root, 'version_1', '# First') + const canonicalPath = join(root, 'skills', 'test-skill') + const lockPath = join(root, 'state', 'locks', `${skillInstallStateKey(canonicalPath)}.lock`) + const release = await acquireSkillInstallLock({ path: lockPath }) + try { + const result = await installLocalSkillPackage( + installInput(root, archive, { lockTimeoutMs: 1 }) + ) + expect(result).toMatchObject({ + status: 'failed', + errorCategory: 'skill-install-busy', + failure: { category: 'filesystem', retryable: true } + }) + expect( + (await readdir(join(root, 'skills'))).filter((name) => name.includes('extract')) + ).toEqual([]) + } finally { + await release() + } + }) + + it('invalidates a plan when destination state changes immediately before commit', async () => { + const root = await temporaryDirectory() + const archive = await packageVersion(root, 'version_1', '# Cloud') + const canonicalPath = join(root, 'skills', 'test-skill') + let injected = false + const changingFilesystem = { + ...nativeSkillInstallFilesystem, + rename: async (source: string, target: string): Promise => { + await nativeSkillInstallFilesystem.rename(source, target) + if (!injected && target.includes('.orca-staging-')) { + injected = true + await mkdir(canonicalPath) + await writeFile(join(canonicalPath, 'SKILL.md'), 'local content') + } + } + } + + const result = await installLocalSkillPackage( + installInput(root, archive, { filesystem: changingFilesystem }) + ) + + expect(result).toMatchObject({ + status: 'conflict', + errorCategory: 'skill-install-conflict-stale-preview' + }) + expect(await readFile(join(canonicalPath, 'SKILL.md'), 'utf8')).toBe('local content') + expect((await readdir(join(root, 'skills'))).filter((name) => name.includes('.orca-'))).toEqual( + [] + ) + }) + + it('restores the old version when cancellation arrives before canonical placement', async () => { + const root = await temporaryDirectory() + const first = await packageVersion(root, 'version_1', '# First') + const second = await packageVersion(root, 'version_2', '# Second') + await installLocalSkillPackage(installInput(root, first)) + const controller = new AbortController() + const cancellingFilesystem = { + ...nativeSkillInstallFilesystem, + rename: async (source: string, target: string): Promise => { + await nativeSkillInstallFilesystem.rename(source, target) + if (target.includes('.orca-backup-')) { + controller.abort() + } + } + } + + const result = await installLocalSkillPackage( + installInput(root, second, { filesystem: cancellingFilesystem, signal: controller.signal }) + ) + + expect(result).toMatchObject({ + status: 'cancelled', + failure: { category: 'cancelled', retryable: true } + }) + expect(await readFile(join(root, 'skills', 'test-skill', 'SKILL.md'), 'utf8')).toContain( + '# First' + ) + await expect(lstat(join(root, 'state', 'journals'))).resolves.toBeDefined() + expect(await readdir(join(root, 'state', 'journals'))).toEqual([]) + }) }) diff --git a/src/main/skills/skill-install-transaction.ts b/src/main/skills/skill-install-transaction.ts index d63eebe4be6..6fa603279af 100644 --- a/src/main/skills/skill-install-transaction.ts +++ b/src/main/skills/skill-install-transaction.ts @@ -9,8 +9,7 @@ import { readSkillInstallReceipt, skillInstallStateKey, writeSkillInstallReceipt, - writeSkillStateFile, - type SkillInstallReceiptV1 + writeSkillStateFile } from './skill-install-provenance' import { extractSkillPackageArchive } from './skill-package-extraction' import { recoverSkillRemovalTransaction } from './skill-remove-transaction' @@ -26,6 +25,14 @@ import { nativeSkillInstallFilesystem, type SkillInstallFilesystem } from './skill-install-filesystem' +import { SKILL_INSTALL_CANCELLED_FAILURE } from '../../shared/skill-install-failure' +import { SkillInstallOperationError } from './skill-install-operation-error' +import { + createSkillInstallReceipt, + skillInstallConflictResult, + skillInstallFailureResult, + skillInstallReplacementAllowed +} from './skill-install-transaction-result' export { recoverSkillInstallTransaction } from './skill-install-recovery' @@ -44,6 +51,8 @@ export type LocalSkillInstallInput = { conflictResolution?: 'replace-unmodified' | 'replace-and-discard-local' | 'cancel' filesystem?: SkillInstallFilesystem wslDistro?: string + signal?: AbortSignal + lockTimeoutMs?: number } export type LocalSkillInstallPreview = { @@ -56,31 +65,6 @@ function lockPath(stateDirectory: string, canonicalPath: string): string { return join(stateDirectory, 'locks', `${skillInstallStateKey(canonicalPath)}.lock`) } -function conflictResult( - operationId: string, - manifest: SkillPackageManifestV1, - state: SkillCanonicalState -): SkillInstallResult { - const kind = - state.kind === 'modified' || - state.kind === 'unowned' || - state.kind === 'external-link' || - state.kind === 'name-collision' - ? state.kind - : 'modified' - return { - operationId, - status: 'conflict', - name: manifest.name, - packageDigest: manifest.packageDigest, - placements: [], - conflict: { - kind, - ...('digest' in state && state.digest ? { existingDigest: state.digest } : {}) - } - } -} - async function inspectExtractedPackage(input: LocalSkillInstallInput): Promise<{ extractionPath: string manifest: SkillPackageManifestV1 @@ -95,11 +79,18 @@ async function inspectExtractedPackage(input: LocalSkillInstallInput): Promise<{ expectedPackageDigest: input.expectedPackageDigest, expectedPackageId: input.expectedPackageId, expectedVersionId: input.expectedVersionId, - filesystem: input.filesystem + filesystem: input.filesystem, + signal: input.signal }) return { extractionPath, manifest: extracted.manifest, archiveSha256: extracted.archiveSha256 } } +function throwIfCancelled(signal?: AbortSignal): void { + if (signal?.aborted) { + throw new SkillInstallOperationError(SKILL_INSTALL_CANCELLED_FAILURE) + } +} + export async function previewLocalSkillPackage( input: LocalSkillInstallInput ): Promise { @@ -121,64 +112,23 @@ export async function previewLocalSkillPackage( } } -function replacementAllowed(state: SkillCanonicalState, input: LocalSkillInstallInput): boolean { - if (state.kind === 'missing' || state.kind === 'clean-update') { - return input.conflictResolution !== 'cancel' - } - return ( - (state.kind === 'modified' || state.kind === 'unowned') && - input.conflictResolution === 'replace-and-discard-local' - ) -} - -function createReceipt(input: { - request: LocalSkillInstallInput - manifest: SkillPackageManifestV1 - archiveSha256: string - canonicalPath: string - previous: SkillInstallReceiptV1 | null -}): SkillInstallReceiptV1 { - return { - schemaVersion: 1, - packageId: input.manifest.packageId, - versionId: input.manifest.versionId, - packageDigest: input.manifest.packageDigest, - archiveSha256: input.archiveSha256, - scope: input.request.scope, - destinationIdentity: input.request.destinationIdentity, - canonicalPath: input.canonicalPath, - placements: [ - { - provider: 'agent-skills', - path: input.canonicalPath, - topology: 'canonical-copy', - status: 'installed' - } - ], - ...(input.previous ? { previousVersionId: input.previous.versionId } : {}), - installedAt: new Date().toISOString(), - hostIdentity: input.request.hostIdentity, - fileModes: input.manifest.files.map((file) => ({ - path: file.path, - executable: file.executable - })), - ...(input.request.wslDistro ? { wslDistro: input.request.wslDistro } : {}) - } -} - export async function installLocalSkillPackage( input: LocalSkillInstallInput ): Promise { const filesystem = input.filesystem ?? nativeSkillInstallFilesystem const extracted = await inspectExtractedPackage(input) const canonicalPath = join(input.destinationRoot, extracted.manifest.name) - const releaseLock = await acquireSkillInstallLock({ - path: lockPath(input.stateDirectory, canonicalPath) - }) + let releaseLock: (() => Promise) | null = null let journal: SkillInstallJournalV1 | null = null try { + throwIfCancelled(input.signal) + releaseLock = await acquireSkillInstallLock({ + path: lockPath(input.stateDirectory, canonicalPath), + timeoutMs: input.lockTimeoutMs + }) await recoverSkillRemovalTransaction(input.stateDirectory, canonicalPath, filesystem) await recoverSkillInstallTransaction(input.stateDirectory, canonicalPath, filesystem) + throwIfCancelled(input.signal) const previous = await readSkillInstallReceipt(input.stateDirectory, canonicalPath) const state = await inspectSkillCanonicalState({ canonicalPath, @@ -186,7 +136,7 @@ export async function installLocalSkillPackage( receipt: previous, filesystem }) - const receipt = createReceipt({ + const receipt = createSkillInstallReceipt({ request: input, manifest: extracted.manifest, archiveSha256: extracted.archiveSha256, @@ -194,6 +144,7 @@ export async function installLocalSkillPackage( previous }) if (state.kind === 'unchanged') { + throwIfCancelled(input.signal) await writeSkillInstallReceipt(input.stateDirectory, receipt) return { operationId: input.operationId, @@ -207,9 +158,10 @@ export async function installLocalSkillPackage( })) } } - if (!replacementAllowed(state, input)) { - return conflictResult(input.operationId, extracted.manifest, state) + if (!skillInstallReplacementAllowed(state, input)) { + return skillInstallConflictResult(input.operationId, extracted.manifest, state) } + throwIfCancelled(input.signal) const transactionId = randomUUID() const stagingPath = join( input.destinationRoot, @@ -219,6 +171,11 @@ export async function installLocalSkillPackage( input.destinationRoot, `.${extracted.manifest.name}.orca-backup-${transactionId}` ) + const destinationExists = await skillInstallPathExists(canonicalPath) + const backupDigest = 'digest' in state ? (state.digest ?? null) : null + if (destinationExists && !backupDigest) { + return skillInstallConflictResult(input.operationId, extracted.manifest, state) + } await filesystem.rename(join(extracted.extractionPath, 'skill'), stagingPath) journal = { schemaVersion: 1, @@ -228,14 +185,35 @@ export async function installLocalSkillPackage( extractionPath: extracted.extractionPath, stagingPath, backupPath, + backupDigest, + stagingFileModes: extracted.manifest.files, + backupFileModes: previous?.fileModes ?? extracted.manifest.files, receipt } const statePath = skillInstallJournalPath(input.stateDirectory, canonicalPath) await writeSkillStateFile(statePath, journal) - if (await skillInstallPathExists(canonicalPath)) { + throwIfCancelled(input.signal) + const commitState = await inspectSkillCanonicalState({ + canonicalPath, + manifest: extracted.manifest, + receipt: previous, + filesystem + }) + if (JSON.stringify(commitState) !== JSON.stringify(state)) { + await recoverSkillInstallTransaction(input.stateDirectory, canonicalPath, filesystem) + journal = null + return skillInstallConflictResult( + input.operationId, + extracted.manifest, + commitState, + 'skill-install-conflict-stale-preview' + ) + } + if (destinationExists) { await filesystem.rename(canonicalPath, backupPath) journal.phase = 'backup-created' await writeSkillStateFile(statePath, journal) + throwIfCancelled(input.signal) } await filesystem.rename(stagingPath, canonicalPath) journal.phase = 'canonical-placed' @@ -271,9 +249,13 @@ export async function installLocalSkillPackage( () => undefined ) } + const result = skillInstallFailureResult(input, extracted.manifest, canonicalPath, error) + if (result) { + return result + } throw error } finally { await filesystem.remove(extracted.extractionPath) - await releaseLock() + await releaseLock?.() } } diff --git a/src/main/skills/skill-package-creation.test.ts b/src/main/skills/skill-package-creation.test.ts index 37782efad8d..67ec5608a8d 100644 --- a/src/main/skills/skill-package-creation.test.ts +++ b/src/main/skills/skill-package-creation.test.ts @@ -1,6 +1,7 @@ import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { gunzipSync, gzipSync } from 'node:zlib' import { afterEach, describe, expect, it } from 'vitest' import { createSkillPackageArchive } from './skill-package-creation' import { extractSkillPackageArchive } from './skill-package-extraction' @@ -25,6 +26,16 @@ async function createSkill(root: string): Promise { return skill } +async function mutateArchive( + source: string, + target: string, + mutate: (tar: Buffer) => Buffer | void +): Promise { + const tar = gunzipSync(await readFile(source)) + const mutated = mutate(tar) ?? tar + await writeFile(target, gzipSync(mutated, { level: 9 })) +} + afterEach(async () => { await Promise.all( temporaryDirectories.splice(0).map((directory) => rm(directory, { recursive: true })) @@ -108,4 +119,149 @@ describe('skill package creation and extraction', () => { }) ).rejects.toThrow('skill-package-archive-digest-mismatch') }) + + it('rejects source changes after the package snapshot and removes temporary output', async () => { + const root = await temporaryDirectory() + const sourceDirectory = await createSkill(root) + const archivePath = join(root, 'drifted.tar.gz') + + await expect( + createSkillPackageArchive( + { + sourceDirectory, + archivePath, + packageId: 'package_1', + versionId: 'version_1' + }, + { + afterSourceObserved: async () => { + await writeFile(join(sourceDirectory, 'SKILL.md'), 'changed after observation') + } + } + ) + ).rejects.toThrow('skill-package-source-changed-during-staging') + await expect(readFile(archivePath)).rejects.toMatchObject({ code: 'ENOENT' }) + }) + + it('normalizes LF and CRLF text identity while preserving exact archive bytes', async () => { + const root = await temporaryDirectory() + const lf = join(root, 'lf') + const crlf = join(root, 'crlf') + await Promise.all([mkdir(lf), mkdir(crlf)]) + const markdown = '---\nname: line-skill\ndescription: Lines\n---\n\n# Lines\n' + await writeFile(join(lf, 'SKILL.md'), markdown) + await writeFile(join(crlf, 'SKILL.md'), markdown.replaceAll('\n', '\r\n')) + const publication = { + packageId: 'package_1', + versionId: 'version_1', + createdAt: '2026-08-11T12:00:00.000Z' + } + + const lfPackage = await createSkillPackageArchive({ + ...publication, + sourceDirectory: lf, + archivePath: join(root, 'lf.tar.gz') + }) + const crlfPackage = await createSkillPackageArchive({ + ...publication, + sourceDirectory: crlf, + archivePath: join(root, 'crlf.tar.gz') + }) + + expect(lfPackage.manifest.packageDigest).toBe(crlfPackage.manifest.packageDigest) + expect(lfPackage.manifest.files[0]?.identitySha256).toBe( + crlfPackage.manifest.files[0]?.identitySha256 + ) + expect(lfPackage.manifest.files[0]?.sha256).not.toBe(crlfPackage.manifest.files[0]?.sha256) + expect(lfPackage.archiveSha256).not.toBe(crlfPackage.archiveSha256) + }) + + it('rejects missing and malformed SKILL.md sources with stable archive errors', async () => { + const root = await temporaryDirectory() + const missing = join(root, 'missing') + const malformed = join(root, 'malformed') + await Promise.all([mkdir(missing), mkdir(malformed)]) + await writeFile(join(missing, 'README.md'), 'no skill') + await writeFile(join(malformed, 'SKILL.md'), '# No frontmatter identity') + const input = (sourceDirectory: string, name: string) => ({ + sourceDirectory, + archivePath: join(root, `${name}.tar.gz`), + packageId: 'package_1', + versionId: 'version_1' + }) + + await expect(createSkillPackageArchive(input(missing, 'missing'))).rejects.toThrow( + 'skill-package-skill-markdown-required' + ) + await expect(createSkillPackageArchive(input(malformed, 'malformed'))).rejects.toThrow( + 'skill-package-skill-name-invalid' + ) + }) + + it('rejects truncated tar data and invalid tar checksums before publishing extraction', async () => { + const root = await temporaryDirectory() + const created = await createSkillPackageArchive({ + sourceDirectory: await createSkill(root), + archivePath: join(root, 'package.tar.gz'), + packageId: 'package_1', + versionId: 'version_1' + }) + const invalidChecksum = join(root, 'invalid-checksum.tar.gz') + const truncated = join(root, 'truncated.tar.gz') + await mutateArchive(created.archivePath, invalidChecksum, (tar) => { + tar[0] ^= 1 + }) + await mutateArchive(created.archivePath, truncated, (tar) => tar.subarray(0, -600)) + + await expect( + extractSkillPackageArchive({ + archivePath: invalidChecksum, + destinationDirectory: join(root, 'invalid-checksum') + }) + ).rejects.toThrow('skill-package-tar-checksum-invalid') + await expect( + extractSkillPackageArchive({ + archivePath: truncated, + destinationDirectory: join(root, 'truncated') + }) + ).rejects.toThrow('skill-package-tar-truncated') + }) + + it('rejects content checksum and SKILL identity mismatches', async () => { + const root = await temporaryDirectory() + const created = await createSkillPackageArchive({ + sourceDirectory: await createSkill(root), + archivePath: join(root, 'package.tar.gz'), + packageId: 'package_1', + versionId: 'version_1' + }) + const invalidContent = join(root, 'invalid-content.tar.gz') + const invalidIdentity = join(root, 'invalid-identity.tar.gz') + await mutateArchive(created.archivePath, invalidContent, (tar) => { + const marker = Buffer.from('# Test\n') + const offset = tar.indexOf(marker) + expect(offset).toBeGreaterThan(0) + tar[offset + 2] ^= 1 + }) + await mutateArchive(created.archivePath, invalidIdentity, (tar) => { + const from = Buffer.from('"name":"test-skill"') + const to = Buffer.from('"name":"best-skill"') + const offset = tar.indexOf(from) + expect(offset).toBeGreaterThan(0) + to.copy(tar, offset) + }) + + await expect( + extractSkillPackageArchive({ + archivePath: invalidContent, + destinationDirectory: join(root, 'invalid-content') + }) + ).rejects.toThrow('skill-package-file-digest-mismatch') + await expect( + extractSkillPackageArchive({ + archivePath: invalidIdentity, + destinationDirectory: join(root, 'invalid-identity') + }) + ).rejects.toThrow('skill-package-skill-name-mismatch') + }) }) diff --git a/src/main/skills/skill-package-creation.ts b/src/main/skills/skill-package-creation.ts index b31cc5a687e..1042c04fff5 100644 --- a/src/main/skills/skill-package-creation.ts +++ b/src/main/skills/skill-package-creation.ts @@ -5,6 +5,7 @@ import { dirname, join } from 'node:path' import { computeSkillPackageDigest, parseSkillPackageManifest, + validateSkillPackageName, type SkillPackageManifestV1 } from '../../shared/skill-package-manifest' import { summarizeSkillMarkdown } from '../../shared/skill-metadata' @@ -20,6 +21,10 @@ export type CreatedSkillPackage = { compressedBytes: number } +export type SkillPackageCreationDependencies = { + afterSourceObserved?: () => Promise +} + function observationsMatch(left: ObservedSkillPackage, right: ObservedSkillPackage): boolean { return ( left.files.length === right.files.length && @@ -65,14 +70,21 @@ function packageManifest(input: { }) } -export async function createSkillPackageArchive(input: { - sourceDirectory: string - archivePath: string - packageId: string - versionId: string - createdAt?: string -}): Promise { +export async function createSkillPackageArchive( + input: { + sourceDirectory: string + archivePath: string + packageId: string + versionId: string + createdAt?: string + }, + dependencies: SkillPackageCreationDependencies = {} +): Promise { const sourceObservation = await observeSkillPackage(input.sourceDirectory) + if (!sourceObservation.files.some((file) => file.path === 'SKILL.md')) { + throw new Error('skill-package-skill-markdown-required') + } + await dependencies.afterSourceObserved?.() const workDirectory = await mkdtemp(join(tmpdir(), 'orca-skill-package-')) const stagedSkill = join(workDirectory, 'skill') const verificationDirectory = join(workDirectory, 'verification') @@ -92,6 +104,7 @@ export async function createSkillPackageArchive(input: { if (!summary.name) { throw new Error('skill-package-skill-name-required') } + validateSkillPackageName(summary.name) const manifest = packageManifest({ packageId: input.packageId, versionId: input.versionId, diff --git a/src/main/skills/skill-package-download.ts b/src/main/skills/skill-package-download.ts index bc30302636e..ba9d779f77d 100644 --- a/src/main/skills/skill-package-download.ts +++ b/src/main/skills/skill-package-download.ts @@ -5,6 +5,8 @@ import { SKILL_PACKAGE_CONTENT_TYPE, SKILL_PACKAGE_MAX_COMPRESSED_BYTES } from '../../shared/skill-package-manifest' +import { SKILL_INSTALL_CANCELLED_FAILURE } from '../../shared/skill-install-failure' +import { SkillInstallOperationError } from './skill-install-operation-error' const REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]) const MAX_REDIRECTS = 3 @@ -59,7 +61,10 @@ function validateUrl( function throwIfUnavailable(input: SkillPackageDownloadInput, expiresAt: number): void { if (input.signal?.aborted) { - throw new Error('skill-download-cancelled') + throw new SkillInstallOperationError({ + ...SKILL_INSTALL_CANCELLED_FAILURE, + code: 'skill-download-cancelled' + }) } if (input.now!() >= expiresAt) { throw new Error('skill-download-grant-expired') @@ -83,7 +88,10 @@ async function fetchWithoutCredentialRedirect( }) } catch { if (input.signal?.aborted) { - throw new Error('skill-download-cancelled') + throw new SkillInstallOperationError({ + ...SKILL_INSTALL_CANCELLED_FAILURE, + code: 'skill-download-cancelled' + }) } throw new Error('skill-download-transport-failed') } @@ -173,7 +181,10 @@ export async function downloadSkillPackageGrant( } catch (error) { await reader.cancel().catch(() => undefined) if (input.signal?.aborted) { - throw new Error('skill-download-cancelled') + throw new SkillInstallOperationError({ + ...SKILL_INSTALL_CANCELLED_FAILURE, + code: 'skill-download-cancelled' + }) } throw error } finally { diff --git a/src/main/skills/skill-package-extraction.ts b/src/main/skills/skill-package-extraction.ts index 127c926aec8..695998c564f 100644 --- a/src/main/skills/skill-package-extraction.ts +++ b/src/main/skills/skill-package-extraction.ts @@ -19,6 +19,8 @@ import { nativeSkillInstallFilesystem, type SkillInstallFilesystem } from './skill-install-filesystem' +import { SKILL_INSTALL_CANCELLED_FAILURE } from '../../shared/skill-install-failure' +import { SkillInstallOperationError } from './skill-install-operation-error' export type SkillPackageExtractionResult = { manifest: SkillPackageManifestV1 @@ -40,7 +42,8 @@ async function consumeZeroPadding(reader: TarByteReader, size: number): Promise< async function extractFile( reader: TarByteReader, destination: string, - expected: SkillPackageFile + expected: SkillPackageFile, + signal?: AbortSignal ): Promise { await mkdir(dirname(destination), { recursive: true, mode: 0o700 }) const handle = await open(destination, 'wx', expected.executable ? 0o700 : 0o600) @@ -48,6 +51,9 @@ async function extractFile( let offset = 0 try { while (offset < expected.size) { + if (signal?.aborted) { + throw new SkillInstallOperationError(SKILL_INSTALL_CANCELLED_FAILURE) + } const bytes = await reader.readExact(Math.min(64 * 1024, expected.size - offset)) hash.update(bytes) let chunkOffset = 0 @@ -143,11 +149,15 @@ export async function extractSkillPackageArchive(input: { expectedPackageId?: string expectedVersionId?: string filesystem?: SkillInstallFilesystem + signal?: AbortSignal }): Promise { const filesystem = input.filesystem ?? nativeSkillInstallFilesystem const archive = await openSkillTarGzip(input.archivePath) let destinationCreated = false try { + if (input.signal?.aborted) { + throw new SkillInstallOperationError(SKILL_INSTALL_CANCELLED_FAILURE) + } const manifest = await readManifest(archive.reader) if ( (input.expectedPackageDigest && manifest.packageDigest !== input.expectedPackageDigest) || @@ -161,6 +171,9 @@ export async function extractSkillPackageArchive(input: { const skillDirectory = join(input.destinationDirectory, 'skill') await mkdir(skillDirectory, { mode: 0o700 }) for (const expected of manifest.files) { + if (input.signal?.aborted) { + throw new SkillInstallOperationError(SKILL_INSTALL_CANCELLED_FAILURE) + } const header = parseSkillTarHeader(await archive.reader.readExact(SKILL_TAR_BLOCK_BYTES)) if ( !header || @@ -170,9 +183,17 @@ export async function extractSkillPackageArchive(input: { ) { throw new Error('skill-package-file-envelope-mismatch') } - await extractFile(archive.reader, join(skillDirectory, ...expected.path.split('/')), expected) + await extractFile( + archive.reader, + join(skillDirectory, ...expected.path.split('/')), + expected, + input.signal + ) } await requireArchiveEnd(archive.reader) + if (input.signal?.aborted) { + throw new SkillInstallOperationError(SKILL_INSTALL_CANCELLED_FAILURE) + } const archiveIdentity = await archive.archiveIdentity if ( input.expectedArchiveSha256 && diff --git a/src/main/skills/skill-placement-alias-repair.test.ts b/src/main/skills/skill-placement-alias-repair.test.ts new file mode 100644 index 00000000000..49470aa3d31 --- /dev/null +++ b/src/main/skills/skill-placement-alias-repair.test.ts @@ -0,0 +1,100 @@ +import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import type { SkillInstallReceiptV1 } from './skill-install-provenance' +import { reconcileSkillProviderPlacement } from './skill-placement-reconciliation' + +const roots: string[] = [] + +async function fixture(): Promise<{ + root: string + canonicalRoot: string + canonicalPath: string + packageDigest: string +}> { + const root = await mkdtemp(join(tmpdir(), 'orca-skill-alias-repair-test-')) + roots.push(root) + const canonicalRoot = join(root, 'canonical') + const canonicalPath = join(canonicalRoot, 'private-skill') + await mkdir(canonicalPath, { recursive: true }) + await writeFile(join(canonicalPath, 'SKILL.md'), 'private skill') + const { nativeSkillInstallFilesystem } = await import('./skill-install-filesystem') + const packageDigest = (await nativeSkillInstallFilesystem.observeSkill(canonicalPath)) + .observedDigest + return { root, canonicalRoot, canonicalPath, packageDigest } +} + +function receipt( + canonicalPath: string, + placementPath: string, + packageDigest: string +): SkillInstallReceiptV1 { + return { + schemaVersion: 1, + packageId: 'package_1', + versionId: 'version_1', + packageDigest, + archiveSha256: 'a'.repeat(64), + scope: 'global', + destinationIdentity: 'global:test', + canonicalPath, + placements: [ + { + provider: 'claude', + path: placementPath, + topology: 'provider-alias', + status: 'installed' + } + ], + installedAt: '2026-08-11T00:00:00.000Z', + hostIdentity: 'test' + } +} + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('skill provider alias reconciliation', () => { + it.runIf(process.platform !== 'win32')( + 'detects a provider parent already linked to canonical storage', + async () => { + const value = await fixture() + const providerRoot = join(value.root, 'provider') + await symlink(value.canonicalRoot, providerRoot, 'dir') + expect(await realpath(join(providerRoot, 'private-skill'))).toBe( + await realpath(value.canonicalPath) + ) + + const result = await reconcileSkillProviderPlacement({ + canonicalPath: value.canonicalPath, + skillName: 'private-skill', + destination: { provider: 'claude', rootPath: providerRoot, readsCanonicalRoot: false }, + previousReceipt: null, + packageDigest: value.packageDigest + }) + + expect(result).toMatchObject({ topology: 'provider-alias', status: 'unchanged' }) + } + ) + + it.runIf(process.platform !== 'win32')('repairs a broken Orca-owned alias', async () => { + const value = await fixture() + const providerRoot = join(value.root, 'provider') + const placementPath = join(providerRoot, 'private-skill') + await mkdir(providerRoot) + await symlink('../missing-skill', placementPath, 'dir') + + const result = await reconcileSkillProviderPlacement({ + canonicalPath: value.canonicalPath, + skillName: 'private-skill', + destination: { provider: 'claude', rootPath: providerRoot, readsCanonicalRoot: false }, + previousReceipt: receipt(value.canonicalPath, placementPath, value.packageDigest), + packageDigest: value.packageDigest + }) + + expect(result).toMatchObject({ topology: 'provider-alias', status: 'installed' }) + expect(await realpath(placementPath)).toBe(await realpath(value.canonicalPath)) + }) +}) diff --git a/src/main/skills/skill-placement-copy.ts b/src/main/skills/skill-placement-copy.ts new file mode 100644 index 00000000000..08ad2601537 --- /dev/null +++ b/src/main/skills/skill-placement-copy.ts @@ -0,0 +1,82 @@ +import { randomUUID } from 'node:crypto' +import { cp, mkdir, readdir } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import type { SkillInstalledFileMode, SkillInstallFilesystem } from './skill-install-filesystem' + +export async function createVerifiedSkillPlacementCopy( + canonicalPath: string, + destinationPath: string, + filesystem: SkillInstallFilesystem, + fileModes?: readonly SkillInstalledFileMode[] +): Promise { + const temporary = `${destinationPath}.orca-copy-${randomUUID()}` + try { + await cp(canonicalPath, temporary, { recursive: true, verbatimSymlinks: true }) + const [source, copied] = await Promise.all([ + filesystem.observeSkill(canonicalPath, fileModes), + filesystem.observeSkill(temporary, fileModes) + ]) + if (source.observedDigest !== copied.observedDigest) { + throw new Error('skill-placement-copy-digest-mismatch') + } + await filesystem.rename(temporary, destinationPath) + } finally { + await filesystem.remove(temporary) + } +} + +export async function replaceOwnedSkillPlacementCopy( + canonicalPath: string, + destinationPath: string, + filesystem: SkillInstallFilesystem, + fileModes?: readonly SkillInstalledFileMode[] +): Promise { + const replacement = `${destinationPath}.orca-copy-${randomUUID()}` + const backup = `${destinationPath}.orca-backup-${randomUUID()}` + try { + await createVerifiedSkillPlacementCopy(canonicalPath, replacement, filesystem, fileModes) + await filesystem.rename(destinationPath, backup) + try { + await filesystem.rename(replacement, destinationPath) + } catch (error) { + await filesystem.rename(backup, destinationPath) + throw error + } + await filesystem.remove(backup) + } finally { + await filesystem.remove(replacement) + } +} + +export async function createSkillPlacementCopyAtMissingDestination( + canonicalPath: string, + destinationPath: string, + filesystem: SkillInstallFilesystem, + fileModes?: readonly SkillInstalledFileMode[] +): Promise { + await mkdir(dirname(destinationPath), { recursive: true }) + await mkdir(destinationPath, { mode: 0o700 }) + let complete = false + try { + for (const entry of await readdir(canonicalPath)) { + await cp(join(canonicalPath, entry), join(destinationPath, entry), { + recursive: true, + verbatimSymlinks: true, + force: false, + errorOnExist: true + }) + } + const [source, copied] = await Promise.all([ + filesystem.observeSkill(canonicalPath, fileModes), + filesystem.observeSkill(destinationPath, fileModes) + ]) + if (source.observedDigest !== copied.observedDigest) { + throw new Error('skill-placement-copy-digest-mismatch') + } + complete = true + } finally { + if (!complete) { + await filesystem.remove(destinationPath) + } + } +} diff --git a/src/main/skills/skill-placement-reconciliation.ts b/src/main/skills/skill-placement-reconciliation.ts index ad2150637ef..8ec11d6e653 100644 --- a/src/main/skills/skill-placement-reconciliation.ts +++ b/src/main/skills/skill-placement-reconciliation.ts @@ -1,5 +1,4 @@ -import { randomUUID } from 'node:crypto' -import { cp, lstat, mkdir, readdir, realpath, symlink } from 'node:fs/promises' +import { lstat, mkdir, realpath, symlink } from 'node:fs/promises' import { dirname, join, relative, resolve } from 'node:path' import type { SkillPlacementResult } from '../../shared/skill-install-contract' import type { SkillInstallReceiptV1 } from './skill-install-provenance' @@ -9,12 +8,23 @@ import { type SkillInstalledFileMode, type SkillInstallFilesystem } from './skill-install-filesystem' +import { + createSkillPlacementCopyAtMissingDestination, + replaceOwnedSkillPlacementCopy +} from './skill-placement-copy' function normalizedPath(path: string): string { const normalized = resolve(path) return process.platform === 'win32' ? normalized.toLocaleLowerCase('en-US') : normalized } +function placementFailure(code: string, retryable = false) { + return { + errorCategory: code, + failure: { category: 'provider-placement' as const, code, retryable } + } +} + async function pathExists(path: string): Promise { return Boolean(await lstat(path).catch(() => null)) } @@ -52,51 +62,6 @@ async function createProviderAlias( await symlink(relative(realParent, realCanonical), destinationPath, 'dir') } -async function createVerifiedCopy( - canonicalPath: string, - destinationPath: string, - filesystem: SkillInstallFilesystem, - fileModes?: readonly SkillInstalledFileMode[] -): Promise { - const temporary = `${destinationPath}.orca-copy-${randomUUID()}` - try { - await cp(canonicalPath, temporary, { recursive: true, verbatimSymlinks: true }) - const [source, copied] = await Promise.all([ - filesystem.observeSkill(canonicalPath, fileModes), - filesystem.observeSkill(temporary, fileModes) - ]) - if (source.observedDigest !== copied.observedDigest) { - throw new Error('skill-placement-copy-digest-mismatch') - } - await filesystem.rename(temporary, destinationPath) - } finally { - await filesystem.remove(temporary) - } -} - -async function replaceOwnedCopy( - canonicalPath: string, - destinationPath: string, - filesystem: SkillInstallFilesystem, - fileModes?: readonly SkillInstalledFileMode[] -): Promise { - const replacement = `${destinationPath}.orca-copy-${randomUUID()}` - const backup = `${destinationPath}.orca-backup-${randomUUID()}` - try { - await createVerifiedCopy(canonicalPath, replacement, filesystem, fileModes) - await filesystem.rename(destinationPath, backup) - try { - await filesystem.rename(replacement, destinationPath) - } catch (error) { - await filesystem.rename(backup, destinationPath) - throw error - } - await filesystem.remove(backup) - } finally { - await filesystem.remove(replacement) - } -} - async function reconcileExistingPlacement(input: { canonicalPath: string destinationPath: string @@ -106,6 +71,11 @@ async function reconcileExistingPlacement(input: { filesystem: SkillInstallFilesystem fileModes?: readonly SkillInstalledFileMode[] }): Promise { + const previous = previousPlacement( + input.previousReceipt, + input.destination.provider, + input.destinationPath + ) if (input.filesystem.aliasTargets) { return (await input.filesystem.aliasTargets(input.canonicalPath, input.destinationPath)) ? { @@ -119,31 +89,40 @@ async function reconcileExistingPlacement(input: { path: input.destinationPath, topology: 'provider-alias', status: 'skipped', - errorCategory: 'skill-placement-unowned-link' + ...placementFailure('skill-placement-unowned-link') } } + const resolvedDestination = await realpath(input.destinationPath).catch(() => null) + const resolvedCanonical = await realpath(input.canonicalPath).catch(() => input.canonicalPath) + if ( + resolvedDestination && + normalizedPath(resolvedDestination) === normalizedPath(resolvedCanonical) + ) { + return { + provider: input.destination.provider, + path: input.destinationPath, + topology: 'provider-alias', + status: 'unchanged' + } + } const stat = await lstat(input.destinationPath) - const previous = previousPlacement( - input.previousReceipt, - input.destination.provider, - input.destinationPath - ) if (stat.isSymbolicLink()) { - const target = await realpath(input.destinationPath).catch(() => null) - if (target && normalizedPath(target) === normalizedPath(input.canonicalPath)) { - return { - provider: input.destination.provider, - path: input.destinationPath, - topology: 'provider-alias', - status: 'unchanged' - } + if (!resolvedDestination && previous?.topology === 'provider-alias') { + await input.filesystem.remove(input.destinationPath) + return createMissingPlacement({ + canonicalPath: input.canonicalPath, + destinationPath: input.destinationPath, + destination: input.destination, + filesystem: input.filesystem, + fileModes: input.fileModes + }) } return { provider: input.destination.provider, path: input.destinationPath, topology: 'provider-alias', status: 'skipped', - errorCategory: 'skill-placement-unowned-link' + ...placementFailure('skill-placement-unowned-link') } } if (stat.isDirectory() && previous?.topology === 'independent-copy') { @@ -156,7 +135,7 @@ async function reconcileExistingPlacement(input: { path: input.destinationPath, topology: 'independent-copy', status: 'skipped', - errorCategory: 'skill-placement-modified-copy' + ...placementFailure('skill-placement-modified-copy') } } if (observed.observedDigest === input.packageDigest) { @@ -167,7 +146,7 @@ async function reconcileExistingPlacement(input: { status: 'unchanged' } } - await replaceOwnedCopy( + await replaceOwnedSkillPlacementCopy( input.canonicalPath, input.destinationPath, input.filesystem, @@ -185,7 +164,7 @@ async function reconcileExistingPlacement(input: { path: input.destinationPath, topology: 'independent-copy', status: 'skipped', - errorCategory: 'skill-placement-unowned' + ...placementFailure('skill-placement-unowned') } } @@ -206,7 +185,7 @@ async function createMissingPlacement(input: { } } catch { try { - await createVerifiedCopyAtMissingDestination( + await createSkillPlacementCopyAtMissingDestination( input.canonicalPath, input.destinationPath, input.filesystem, @@ -224,45 +203,12 @@ async function createMissingPlacement(input: { path: input.destinationPath, topology: 'independent-copy', status: 'failed', - errorCategory: 'skill-placement-create-failed' + ...placementFailure('skill-placement-create-failed', true) } } } } -async function createVerifiedCopyAtMissingDestination( - canonicalPath: string, - destinationPath: string, - filesystem: SkillInstallFilesystem, - fileModes?: readonly SkillInstalledFileMode[] -): Promise { - await mkdir(dirname(destinationPath), { recursive: true }) - await mkdir(destinationPath, { mode: 0o700 }) - let complete = false - try { - for (const entry of await readdir(canonicalPath)) { - await cp(join(canonicalPath, entry), join(destinationPath, entry), { - recursive: true, - verbatimSymlinks: true, - force: false, - errorOnExist: true - }) - } - const [source, copied] = await Promise.all([ - filesystem.observeSkill(canonicalPath, fileModes), - filesystem.observeSkill(destinationPath, fileModes) - ]) - if (source.observedDigest !== copied.observedDigest) { - throw new Error('skill-placement-copy-digest-mismatch') - } - complete = true - } finally { - if (!complete) { - await filesystem.remove(destinationPath) - } - } -} - export async function reconcileSkillProviderPlacement(input: { canonicalPath: string skillName: string @@ -287,7 +233,7 @@ export async function reconcileSkillProviderPlacement(input: { path: destinationPath, topology: 'independent-copy', status: 'failed', - errorCategory: 'skill-placement-reconciliation-failed' + ...placementFailure('skill-placement-reconciliation-failed', true) } } } diff --git a/src/main/skills/skill-remote-install-service.ts b/src/main/skills/skill-remote-install-service.ts index a77f1828644..4bb752aa0a4 100644 --- a/src/main/skills/skill-remote-install-service.ts +++ b/src/main/skills/skill-remote-install-service.ts @@ -8,6 +8,8 @@ import { import { SKILL_UPLOAD_CAPABILITY } from '../../shared/skill-install-capability' import { callRuntimeEnvironment } from '../ipc/runtime-environment-transport-routing' import { transferSkillPackageToRuntime } from './skill-client-mediated-transfer' +import { SkillInstallFailureSchema } from '../../shared/skill-install-failure' +import { SkillInstallOperationError } from './skill-install-operation-error' const DIRECT_DOWNLOAD_FAILURE = 'skill-download-transport-failed' @@ -26,13 +28,26 @@ async function install( } function isDownloadTransportFailure(response: RuntimeRpcResponse): boolean { + const structured = + response.ok === false ? SkillInstallFailureSchema.safeParse(response.error.data) : null return ( response.ok === false && (response.error.code === DIRECT_DOWNLOAD_FAILURE || - response.error.message === DIRECT_DOWNLOAD_FAILURE) + response.error.message === DIRECT_DOWNLOAD_FAILURE || + (structured?.success === true && structured.data.code === DIRECT_DOWNLOAD_FAILURE)) ) } +function remoteFailure(response: RuntimeRpcResponse): Error { + if (response.ok === true) { + return new Error('skill-install-remote-response-invalid') + } + const failure = SkillInstallFailureSchema.safeParse(response.error.data) + return failure.success + ? new SkillInstallOperationError(failure.data) + : new Error(`skill-install-remote-${response.error.code}`) +} + export async function installSkillOnRemoteRuntime(input: { userDataPath: string environmentId: string @@ -46,7 +61,7 @@ export async function installSkillOnRemoteRuntime(input: { const direct = await install(input.userDataPath, input.environmentId, input.request) if (!isDownloadTransportFailure(direct)) { if (direct.ok !== true) { - throw new Error(`skill-install-remote-${direct.error.code}`) + throw remoteFailure(direct) } return SkillInstallResultSchema.parse(direct.result) } @@ -67,7 +82,7 @@ export async function installSkillOnRemoteRuntime(input: { ingress: { kind: 'staged-upload', uploadId: transfer.uploadId } }) if (staged.ok !== true) { - throw new Error(`skill-install-remote-${staged.error.code}`) + throw remoteFailure(staged) } return SkillInstallResultSchema.parse(staged.result) } finally { diff --git a/src/main/skills/skill-remove-recovery.ts b/src/main/skills/skill-remove-recovery.ts new file mode 100644 index 00000000000..0aeefd3b461 --- /dev/null +++ b/src/main/skills/skill-remove-recovery.ts @@ -0,0 +1,194 @@ +import { lstat, readFile, readlink, rm } from 'node:fs/promises' +import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path' +import type { SkillPlacementResult } from '../../shared/skill-install-contract' +import type { SkillInstallFilesystem } from './skill-install-filesystem' +import { nativeSkillInstallFilesystem } from './skill-install-filesystem' +import { skillInstallStateKey, type SkillInstallReceiptV1 } from './skill-install-provenance' + +export type RemovalMove = { + sourcePath: string + backupPath: string + placement: SkillPlacementResult + expectedDigest?: string + expectedAliasTarget?: string +} + +export type SkillRemovalJournalV1 = { + schemaVersion: 1 + operation: 'remove' + phase: 'prepared' | 'moving' | 'receipt-removed' + canonicalPath: string + movedCount: number + moves: RemovalMove[] + receipt: SkillInstallReceiptV1 + allowedProviderRoots: string[] +} + +export function skillRemovalJournalPath(stateDirectory: string, canonicalPath: string): string { + return join(stateDirectory, 'removal-journals', `${skillInstallStateKey(canonicalPath)}.json`) +} + +function validBackup(move: RemovalMove): boolean { + return ( + typeof move?.sourcePath === 'string' && + typeof move.backupPath === 'string' && + Boolean(move.placement) && + typeof move.placement.path === 'string' && + typeof move.placement.topology === 'string' && + dirname(move.sourcePath) === dirname(move.backupPath) && + basename(move.backupPath).startsWith(`.${basename(move.sourcePath)}.orca-remove-backup-`) + ) +} + +function normalizedPath(path: string): string { + const normalized = resolve(path) + return process.platform === 'win32' ? normalized.toLocaleLowerCase('en-US') : normalized +} + +function pathInside(root: string, path: string): boolean { + const child = relative(resolve(root), resolve(path)) + return ( + child !== '' && + child !== '..' && + !child.startsWith(`..${process.platform === 'win32' ? '\\' : '/'}`) && + !isAbsolute(child) + ) +} + +function moveOwnedByJournal(journal: Partial, move: RemovalMove): boolean { + if (!journal.receipt || !Array.isArray(journal.allowedProviderRoots)) { + return false + } + const canonical = normalizedPath(journal.canonicalPath!) + const source = normalizedPath(move.sourcePath) + if (source === canonical) { + return ( + move.placement.topology === 'canonical-copy' && + typeof move.expectedDigest === 'string' && + /^[a-f0-9]{64}$/.test(move.expectedDigest) + ) + } + const receiptPlacement = journal.receipt.placements.find( + (placement) => normalizedPath(placement.path) === source + ) + return Boolean( + receiptPlacement && + receiptPlacement.topology === move.placement.topology && + journal.allowedProviderRoots.some( + (root) => + pathInside(root, move.sourcePath) && + normalizedPath(move.sourcePath) === + normalizedPath(join(root, basename(journal.canonicalPath!))) + ) && + (move.placement.topology === 'provider-alias' + ? move.expectedAliasTarget === journal.canonicalPath + : move.expectedDigest === journal.receipt.packageDigest) + ) +} + +function isRemovalJournal(value: unknown, canonicalPath: string): value is SkillRemovalJournalV1 { + if (!value || typeof value !== 'object') { + return false + } + const journal = value as Partial + return ( + journal.schemaVersion === 1 && + journal.operation === 'remove' && + journal.canonicalPath === canonicalPath && + (journal.phase === 'prepared' || + journal.phase === 'moving' || + journal.phase === 'receipt-removed') && + Number.isInteger(journal.movedCount) && + Array.isArray(journal.moves) && + Boolean(journal.receipt) && + journal.receipt?.canonicalPath === canonicalPath && + Array.isArray(journal.allowedProviderRoots) && + journal.moves.every((move) => validBackup(move) && moveOwnedByJournal(journal, move)) && + new Set(journal.moves.map((move) => normalizedPath(move.sourcePath))).size === + journal.moves.length && + journal.movedCount! >= 0 && + journal.movedCount! <= journal.moves.length + ) +} + +async function movedBackupMatches( + move: RemovalMove, + receipt: SkillInstallReceiptV1, + filesystem: SkillInstallFilesystem +): Promise { + if (move.expectedAliasTarget) { + if (filesystem.aliasTargets) { + return filesystem.aliasTargets(move.expectedAliasTarget, move.backupPath) + } + const target = await readlink(move.backupPath).catch(() => null) + return Boolean( + target && + normalizedPath(resolve(dirname(move.backupPath), target)) === + normalizedPath(move.expectedAliasTarget) + ) + } + if (!move.expectedDigest) { + return false + } + const observed = await filesystem + .observeSkill(move.backupPath, receipt.fileModes) + .catch(() => null) + return observed?.observedDigest === move.expectedDigest +} + +async function readJournal( + stateDirectory: string, + canonicalPath: string +): Promise { + try { + const value: unknown = JSON.parse( + await readFile(skillRemovalJournalPath(stateDirectory, canonicalPath), 'utf8') + ) + if (!isRemovalJournal(value, canonicalPath)) { + throw new Error('skill-removal-journal-invalid') + } + return value + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { + return null + } + throw error + } +} + +export async function skillRemovalPathExists(path: string): Promise { + return Boolean(await lstat(path).catch(() => null)) +} + +export async function recoverSkillRemovalTransaction( + stateDirectory: string, + canonicalPath: string, + filesystem: SkillInstallFilesystem = nativeSkillInstallFilesystem +): Promise { + const journal = await readJournal(stateDirectory, canonicalPath) + if (!journal) { + return + } + if (journal.phase === 'receipt-removed') { + for (const move of journal.moves.slice(0, journal.movedCount)) { + if (!(await movedBackupMatches(move, journal.receipt, filesystem))) { + throw new Error('skill-removal-recovery-conflict') + } + await filesystem.remove(move.backupPath) + } + await rm(skillRemovalJournalPath(stateDirectory, canonicalPath), { force: true }) + return + } + for (const move of journal.moves.slice(0, journal.movedCount).toReversed()) { + if ( + (await skillRemovalPathExists(move.backupPath)) && + !(await skillRemovalPathExists(move.sourcePath)) + ) { + if (!(await movedBackupMatches(move, journal.receipt, filesystem))) { + throw new Error('skill-removal-recovery-conflict') + } + await filesystem.rename(move.backupPath, move.sourcePath) + } + } + await rm(skillRemovalJournalPath(stateDirectory, canonicalPath), { force: true }) +} diff --git a/src/main/skills/skill-remove-transaction.test.ts b/src/main/skills/skill-remove-transaction.test.ts new file mode 100644 index 00000000000..32bb28966f1 --- /dev/null +++ b/src/main/skills/skill-remove-transaction.test.ts @@ -0,0 +1,155 @@ +import { mkdir, mkdtemp, readFile, rename, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { basename, dirname, join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { createSkillPackageArchive } from './skill-package-creation' +import { + readSkillInstallReceipt, + writeSkillStateFile, + type SkillInstallReceiptV1 +} from './skill-install-provenance' +import { installLocalSkillPackage } from './skill-install-transaction' +import { + recoverSkillRemovalTransaction, + skillRemovalJournalPath, + type SkillRemovalJournalV1 +} from './skill-remove-transaction' + +const roots: string[] = [] + +async function installedFixture(): Promise<{ + root: string + canonicalPath: string + stateDirectory: string + receipt: SkillInstallReceiptV1 +}> { + const root = await mkdtemp(join(tmpdir(), 'orca-skill-remove-recovery-test-')) + roots.push(root) + const source = join(root, 'source') + await mkdir(source) + await writeFile( + join(source, 'SKILL.md'), + '---\nname: remove-skill\ndescription: Remove\n---\n\n# Remove\n' + ) + const archive = await createSkillPackageArchive({ + sourceDirectory: source, + archivePath: join(root, 'package.tar.gz'), + packageId: 'package_1', + versionId: 'version_1' + }) + const destinationRoot = join(root, 'skills') + const stateDirectory = join(root, 'state') + await installLocalSkillPackage({ + operationId: 'install', + archivePath: archive.archivePath, + destinationRoot, + stateDirectory, + scope: 'global', + destinationIdentity: 'global:test', + hostIdentity: 'test' + }) + const canonicalPath = join(destinationRoot, 'remove-skill') + const receipt = await readSkillInstallReceipt(stateDirectory, canonicalPath) + if (!receipt) { + throw new Error('fixture receipt missing') + } + return { root, canonicalPath, stateDirectory, receipt } +} + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +describe('skill removal recovery', () => { + it('rejects a journal-owned move outside its recorded provider roots', async () => { + const fixture = await installedFixture() + const outside = join(fixture.root, 'outside') + await mkdir(outside) + await writeFile(join(outside, 'keep.txt'), 'keep') + const sourcePath = outside + const backupPath = join(dirname(sourcePath), `.${basename(sourcePath)}.orca-remove-backup-x`) + const journal: SkillRemovalJournalV1 = { + schemaVersion: 1, + operation: 'remove', + phase: 'prepared', + canonicalPath: fixture.canonicalPath, + movedCount: 0, + moves: [ + { + sourcePath, + backupPath, + placement: { + provider: 'claude', + path: sourcePath, + topology: 'independent-copy', + status: 'installed' + }, + expectedDigest: fixture.receipt.packageDigest + } + ], + receipt: { + ...fixture.receipt, + placements: [ + ...fixture.receipt.placements, + { + provider: 'claude', + path: sourcePath, + topology: 'independent-copy', + status: 'installed' + } + ] + }, + allowedProviderRoots: [] + } + await writeSkillStateFile( + skillRemovalJournalPath(fixture.stateDirectory, fixture.canonicalPath), + journal + ) + + await expect( + recoverSkillRemovalTransaction(fixture.stateDirectory, fixture.canonicalPath) + ).rejects.toThrow('skill-removal-journal-invalid') + expect(await readFile(join(outside, 'keep.txt'), 'utf8')).toBe('keep') + }) + + it('preserves a moved backup whose bytes changed before recovery', async () => { + const fixture = await installedFixture() + const backupPath = join( + dirname(fixture.canonicalPath), + `.${basename(fixture.canonicalPath)}.orca-remove-backup-x` + ) + await rename(fixture.canonicalPath, backupPath) + await writeFile(join(backupPath, 'local.md'), 'changed') + const journal: SkillRemovalJournalV1 = { + schemaVersion: 1, + operation: 'remove', + phase: 'moving', + canonicalPath: fixture.canonicalPath, + movedCount: 1, + moves: [ + { + sourcePath: fixture.canonicalPath, + backupPath, + placement: { + provider: 'agent-skills', + path: fixture.canonicalPath, + topology: 'canonical-copy', + status: 'installed' + }, + expectedDigest: fixture.receipt.packageDigest + } + ], + receipt: fixture.receipt, + allowedProviderRoots: [] + } + await writeSkillStateFile( + skillRemovalJournalPath(fixture.stateDirectory, fixture.canonicalPath), + journal + ) + + await expect( + recoverSkillRemovalTransaction(fixture.stateDirectory, fixture.canonicalPath) + ).rejects.toThrow('skill-removal-recovery-conflict') + expect(await readFile(join(backupPath, 'local.md'), 'utf8')).toBe('changed') + }) +}) diff --git a/src/main/skills/skill-remove-transaction.ts b/src/main/skills/skill-remove-transaction.ts index 611dc84da69..b9be5c92061 100644 --- a/src/main/skills/skill-remove-transaction.ts +++ b/src/main/skills/skill-remove-transaction.ts @@ -1,5 +1,5 @@ import { randomUUID } from 'node:crypto' -import { lstat, readFile, rm } from 'node:fs/promises' +import { lstat } from 'node:fs/promises' import { basename, dirname, join } from 'node:path' import type { SkillInstallResult, SkillPlacementResult } from '../../shared/skill-install-contract' import { acquireSkillInstallLock } from './skill-install-lock' @@ -15,21 +15,19 @@ import { type SkillInstallFilesystem } from './skill-install-filesystem' import { isRemovableSkillPlacement } from './skill-removable-placement' +import { + recoverSkillRemovalTransaction, + skillRemovalJournalPath, + skillRemovalPathExists, + type RemovalMove, + type SkillRemovalJournalV1 +} from './skill-remove-recovery' -type RemovalMove = { - sourcePath: string - backupPath: string - placement: SkillPlacementResult -} - -type SkillRemovalJournalV1 = { - schemaVersion: 1 - operation: 'remove' - phase: 'prepared' | 'moving' | 'receipt-removed' - canonicalPath: string - movedCount: number - moves: RemovalMove[] -} +export { + recoverSkillRemovalTransaction, + skillRemovalJournalPath, + type SkillRemovalJournalV1 +} from './skill-remove-recovery' export type LocalSkillRemovalInput = { operationId: string @@ -40,94 +38,16 @@ export type LocalSkillRemovalInput = { filesystem?: SkillInstallFilesystem } -function journalPath(stateDirectory: string, canonicalPath: string): string { - return join(stateDirectory, 'removal-journals', `${skillInstallStateKey(canonicalPath)}.json`) -} - function lockPath(stateDirectory: string, canonicalPath: string): string { return join(stateDirectory, 'locks', `${skillInstallStateKey(canonicalPath)}.lock`) } -function validBackup(move: RemovalMove): boolean { - return ( - dirname(move.sourcePath) === dirname(move.backupPath) && - basename(move.backupPath).startsWith(`.${basename(move.sourcePath)}.orca-remove-backup-`) - ) -} - -function isRemovalJournal(value: unknown, canonicalPath: string): value is SkillRemovalJournalV1 { - if (!value || typeof value !== 'object') { - return false - } - const journal = value as Partial - return ( - journal.schemaVersion === 1 && - journal.operation === 'remove' && - journal.canonicalPath === canonicalPath && - (journal.phase === 'prepared' || - journal.phase === 'moving' || - journal.phase === 'receipt-removed') && - Number.isInteger(journal.movedCount) && - Array.isArray(journal.moves) && - journal.moves.every(validBackup) && - journal.movedCount! >= 0 && - journal.movedCount! <= journal.moves.length - ) -} - -async function readJournal( - stateDirectory: string, - canonicalPath: string -): Promise { - try { - const value: unknown = JSON.parse( - await readFile(journalPath(stateDirectory, canonicalPath), 'utf8') - ) - if (!isRemovalJournal(value, canonicalPath)) { - throw new Error('skill-removal-journal-invalid') - } - return value - } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - return null - } - throw error - } -} - -async function pathExists(path: string): Promise { - return Boolean(await lstat(path).catch(() => null)) -} - -export async function recoverSkillRemovalTransaction( - stateDirectory: string, - canonicalPath: string, - filesystem: SkillInstallFilesystem = nativeSkillInstallFilesystem -): Promise { - const journal = await readJournal(stateDirectory, canonicalPath) - if (!journal) { - return - } - if (journal.phase === 'receipt-removed') { - for (const move of journal.moves.slice(0, journal.movedCount)) { - await filesystem.remove(move.backupPath) - } - await rm(journalPath(stateDirectory, canonicalPath), { force: true }) - return - } - for (const move of journal.moves.slice(0, journal.movedCount).toReversed()) { - if ((await pathExists(move.backupPath)) && !(await pathExists(move.sourcePath))) { - await filesystem.rename(move.backupPath, move.sourcePath) - } - } - await rm(journalPath(stateDirectory, canonicalPath), { force: true }) -} - function conflictResult( input: LocalSkillRemovalInput, receipt: SkillInstallReceiptV1 | null, kind: 'modified' | 'unowned' | 'external-link' | 'name-collision' ): SkillInstallResult { + const code = `skill-removal-conflict-${kind}` return { operationId: input.operationId, status: 'conflict', @@ -135,7 +55,9 @@ function conflictResult( packageDigest: receipt?.packageDigest ?? '', canonicalPath: input.canonicalPath, placements: [], - conflict: { kind } + conflict: { kind }, + errorCategory: code, + failure: { category: 'conflict', code, retryable: false } } } @@ -204,17 +126,32 @@ export async function removeLocalSharedSkill( dirname(placement.path), `.${basename(placement.path)}.orca-remove-backup-${randomUUID()}` ), - placement + placement, + ...(placement.topology === 'provider-alias' + ? { expectedAliasTarget: receipt.canonicalPath } + : { expectedDigest: receipt.packageDigest }) }) - } else if (await pathExists(placement.path)) { + } else if (await skillRemovalPathExists(placement.path)) { skippedPlacements.push({ ...placement, status: 'skipped', - errorCategory: 'skill-removal-placement-modified-or-unowned' + errorCategory: 'skill-removal-placement-modified-or-unowned', + failure: { + category: 'conflict', + code: 'skill-removal-placement-modified-or-unowned', + retryable: false + } }) } } if (state !== 'missing') { + const canonicalDigest = await filesystem + .observeSkill(input.canonicalPath, receipt.fileModes) + .then((observed) => observed.observedDigest) + .catch(() => null) + if (!canonicalDigest) { + return conflictResult(input, receipt, 'modified') + } moves.push({ sourcePath: input.canonicalPath, backupPath: join( @@ -226,7 +163,8 @@ export async function removeLocalSharedSkill( path: input.canonicalPath, topology: 'canonical-copy', status: 'installed' - } + }, + expectedDigest: canonicalDigest }) } const journal: SkillRemovalJournalV1 = { @@ -235,9 +173,11 @@ export async function removeLocalSharedSkill( phase: 'prepared', canonicalPath: input.canonicalPath, movedCount: 0, - moves + moves, + receipt, + allowedProviderRoots: [...input.allowedProviderRoots] } - const statePath = journalPath(input.stateDirectory, input.canonicalPath) + const statePath = skillRemovalJournalPath(input.stateDirectory, input.canonicalPath) await writeSkillStateFile(statePath, journal) for (const move of moves) { journal.movedCount += 1 diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index f474d620a47..c2e06eaeda2 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -415,6 +415,7 @@ import type { SkillRemoveOperation, SkillShareInstallInput, SkillShareInstallOperation, + SkillInstallCancelInput, SkillSharePreview, SkillShareProgress, SkillSharePublishInput, @@ -2580,10 +2581,22 @@ export type PreloadApi = { installPackageVersion: ( input: SkillPackageVersionInstallInput ) => Promise + cancelInstall: (input: SkillInstallCancelInput) => Promise<{ cancelled: boolean }> previewInstall: (input: SkillInstallPreviewInput) => Promise removeInstall: (input: SkillRemoveInput) => Promise listManagedInstalls: (environmentId?: string) => Promise getPackage: (packageId: string) => Promise> + replacePackageAccess: (input: { + packageId: string + userIds: string[] + shareWithOrganization: boolean + }) => Promise> + revokeShare: (shareId: string) => Promise> + deletePackageVersion: (input: { + packageId: string + versionId: string + }) => Promise> + deletePackage: (packageId: string) => Promise> listWslDistros: (environmentId?: string) => Promise onShareProgress: (callback: (progress: SkillShareProgress) => void) => () => void onUpdateRun: (callback: (run: SkillUpdateRun) => void) => () => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 126f14aed87..f11caf00a03 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -143,6 +143,7 @@ import type { SkillRemoveOperation, SkillShareInstallInput, SkillShareInstallOperation, + SkillInstallCancelInput, SkillSharePreview, SkillShareProgress, SkillSharePublishInput, @@ -2537,6 +2538,8 @@ const api = { input: SkillPackageVersionInstallInput ): Promise => ipcRenderer.invoke('skills:installPackageVersion', input), + cancelInstall: (input: SkillInstallCancelInput): Promise<{ cancelled: boolean }> => + ipcRenderer.invoke('skills:cancelInstall', input), previewInstall: (input: SkillInstallPreviewInput): Promise => ipcRenderer.invoke('skills:previewInstall', input), removeInstall: (input: SkillRemoveInput): Promise => @@ -2545,6 +2548,21 @@ const api = { ipcRenderer.invoke('skills:listManagedInstalls', environmentId), getPackage: (packageId: string): Promise> => ipcRenderer.invoke('skills:getPackage', packageId), + replacePackageAccess: (input: { + packageId: string + userIds: string[] + shareWithOrganization: boolean + }): Promise> => + ipcRenderer.invoke('skills:replacePackageAccess', input), + revokeShare: (shareId: string): Promise> => + ipcRenderer.invoke('skills:revokeShare', shareId), + deletePackageVersion: (input: { + packageId: string + versionId: string + }): Promise> => + ipcRenderer.invoke('skills:deletePackageVersion', input), + deletePackage: (packageId: string): Promise> => + ipcRenderer.invoke('skills:deletePackage', packageId), listWslDistros: (environmentId?: string): Promise => ipcRenderer.invoke('skills:listWslDistros', environmentId), onShareProgress: (callback: (progress: SkillShareProgress) => void): (() => void) => { diff --git a/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx b/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx new file mode 100644 index 00000000000..c07bde26e48 --- /dev/null +++ b/src/renderer/src/components/skills/SkillCloudManagementActions.test.tsx @@ -0,0 +1,130 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { SkillCloudPackageDetails } from '../../../../shared/skill-cloud-contract' +import { SkillCloudManagementActions } from './SkillCloudManagementActions' + +const details: SkillCloudPackageDetails = { + id: 'pkg_1', + name: 'private-skill', + description: 'Private skill', + createdAt: '2026-08-11T00:00:00.000Z', + canManage: true, + versions: [], + management: { + userIds: ['legacy_user'], + shareWithOrganization: false, + shares: [ + { + id: 'share_1', + pinnedVersionId: 'ver_1', + createdAt: '2026-08-11T00:00:00.000Z' + } + ] + } +} + +function setup() { + const skills = { + replacePackageAccess: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }), + revokeShare: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }), + deletePackageVersion: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }), + deletePackage: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }) + } + const onChanged = vi.fn().mockResolvedValue(undefined) + const onPackageDeleted = vi.fn() + Object.defineProperty(window, 'api', { + configurable: true, + value: { + skills, + orcaProfiles: { + authStatus: vi.fn().mockResolvedValue({ + cloud: { userId: 'viewer', activeOrgId: 'org_1' } + }), + orgMembersList: vi.fn().mockResolvedValue({ + status: 'ok', + roster: { + members: [ + { + userId: 'user_2', + email: 'teammate@example.com', + displayName: 'Teammate', + role: 'member' + } + ] + } + }) + } + } + }) + render( + + ) + return { skills, onChanged, onPackageDeleted } +} + +afterEach(() => { + cleanup() + vi.restoreAllMocks() + Reflect.deleteProperty(window, 'api') +}) + +describe('SkillCloudManagementActions', () => { + it('saves organization and selected-user access while preserving unknown recipients', async () => { + const { skills, onChanged } = setup() + const organization = await screen.findByRole('checkbox', { name: 'Current organization' }) + await waitFor(() => expect((organization as HTMLButtonElement).disabled).toBe(false)) + fireEvent.click(organization) + fireEvent.click(screen.getByRole('checkbox', { name: 'Teammate' })) + fireEvent.click(screen.getByRole('button', { name: 'Save access' })) + + await waitFor(() => expect(skills.replacePackageAccess).toHaveBeenCalledOnce()) + expect(skills.replacePackageAccess).toHaveBeenCalledWith({ + packageId: 'pkg_1', + userIds: ['legacy_user', 'user_2'], + shareWithOrganization: true + }) + expect(onChanged).toHaveBeenCalledOnce() + }) + + it('requires confirmation before revoking an active link', async () => { + const { skills } = setup() + expect(screen.getByText(/Copies already installed on any machine remain there\./)).toBeTruthy() + fireEvent.click(screen.getByRole('button', { name: 'Unshare' })) + expect(skills.revokeShare).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'Confirm unshare' })) + + await waitFor(() => expect(skills.revokeShare).toHaveBeenCalledWith('share_1')) + }) + + it('requires confirmation before deleting the selected immutable version', async () => { + const { skills } = setup() + fireEvent.click(screen.getByRole('button', { name: 'Delete selected Cloud version' })) + expect(skills.deletePackageVersion).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'Confirm version deletion' })) + + await waitFor(() => + expect(skills.deletePackageVersion).toHaveBeenCalledWith({ + packageId: 'pkg_1', + versionId: 'ver_1' + }) + ) + }) + + it('requires confirmation before deleting the Cloud package', async () => { + const { skills, onChanged, onPackageDeleted } = setup() + fireEvent.click(screen.getByRole('button', { name: 'Delete Cloud package' })) + expect(skills.deletePackage).not.toHaveBeenCalled() + fireEvent.click(screen.getByRole('button', { name: 'Confirm package deletion' })) + + await waitFor(() => expect(skills.deletePackage).toHaveBeenCalledWith('pkg_1')) + expect(onPackageDeleted).toHaveBeenCalledOnce() + expect(onChanged).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/components/skills/SkillCloudManagementActions.tsx b/src/renderer/src/components/skills/SkillCloudManagementActions.tsx new file mode 100644 index 00000000000..c33a1ff2865 --- /dev/null +++ b/src/renderer/src/components/skills/SkillCloudManagementActions.tsx @@ -0,0 +1,264 @@ +import { useEffect, useMemo, useState } from 'react' +import { Loader2, Link2Off, Save, Trash2 } from 'lucide-react' +import { Badge } from '@/components/ui/badge' +import { Button } from '@/components/ui/button' +import { Checkbox } from '@/components/ui/checkbox' +import type { OrcaOrgMember } from '../../../../shared/orca-profiles' +import type { + SkillCloudOperation, + SkillCloudPackageDetails +} from '../../../../shared/skill-cloud-contract' + +type IdentifiedMember = OrcaOrgMember & { userId: string } +type SkillCloudManagement = NonNullable +type SkillCloudManagementActionsProps = { + details: SkillCloudPackageDetails + selectedVersionId: string + onChanged: () => Promise + onPackageDeleted: () => void +} + +function operationMessage(operation: SkillCloudOperation): string | null { + if (operation.status === 'ok') { + return null + } + return operation.status === 'reconnect-required' + ? 'Reconnect your Orca account and try again.' + : operation.message +} + +export function SkillCloudManagementActions( + props: SkillCloudManagementActionsProps +): React.JSX.Element | null { + const management = props.details.management + if (!props.details.canManage || !management) { + return null + } + const stateKey = JSON.stringify([ + props.details.id, + management.userIds, + management.shareWithOrganization, + management.shares + ]) + return +} + +function SkillCloudManagementForm({ + details, + selectedVersionId, + onChanged, + onPackageDeleted, + management +}: SkillCloudManagementActionsProps & { + management: SkillCloudManagement +}): React.JSX.Element { + const [members, setMembers] = useState([]) + const [organizationAvailable, setOrganizationAvailable] = useState(false) + const [userIds, setUserIds] = useState(management?.userIds ?? []) + const [shareWithOrganization, setShareWithOrganization] = useState( + management?.shareWithOrganization ?? false + ) + const [busyAction, setBusyAction] = useState(null) + const [confirmation, setConfirmation] = useState(null) + const [message, setMessage] = useState(null) + + useEffect(() => { + let active = true + setOrganizationAvailable(false) + setMembers([]) + void window.api.orcaProfiles + .authStatus() + .then(async (auth) => { + const orgId = auth.cloud?.activeOrgId + if (!active || !orgId) { + return + } + setOrganizationAvailable(true) + const result = await window.api.orcaProfiles.orgMembersList({ orgId }) + if (active && result.status === 'ok') { + setMembers( + result.roster.members.filter( + (member): member is IdentifiedMember => + Boolean(member.userId) && member.userId !== auth.cloud?.userId + ) + ) + } + }) + .catch(() => undefined) + return () => { + active = false + } + }, []) + + const unknownUserCount = useMemo( + () => userIds.filter((id) => !members.some((member) => member.userId === id)).length, + [members, userIds] + ) + + const run = async ( + action: string, + operation: () => Promise>, + after: () => Promise | void = onChanged + ): Promise => { + setBusyAction(action) + setMessage(null) + try { + const result = await operation() + const failure = operationMessage(result) + if (failure) { + setMessage(failure) + return + } + setConfirmation(null) + try { + await after() + } catch { + setMessage('Cloud change completed, but Orca could not refresh package details.') + } + } catch { + setMessage('Orca could not complete this Cloud change.') + } finally { + setBusyAction(null) + } + } + + return ( +
+
+

Access

+ + {members.length > 0 ? ( +
+ {members.map((member) => ( + + ))} +
+ ) : null} + {unknownUserCount > 0 ? ( +

+ {unknownUserCount} existing recipient{unknownUserCount === 1 ? '' : 's'} outside the + current roster will be preserved. +

+ ) : null} + +
+ +
+

Active links

+

+ Unsharing blocks future installs. Copies already installed on any machine remain there. +

+ {management.shares.length === 0 ? ( +

No active share links.

+ ) : ( + management.shares.map((share) => ( +
+ + {share.id} + + +
+ )) + )} +
+ +
+ + +
+ {message ? ( +

+ {message} +

+ ) : null} +
+ ) +} diff --git a/src/renderer/src/components/skills/SkillInstallDialog.test.tsx b/src/renderer/src/components/skills/SkillInstallDialog.test.tsx index 96cee577cb2..c804cee9ffa 100644 --- a/src/renderer/src/components/skills/SkillInstallDialog.test.tsx +++ b/src/renderer/src/components/skills/SkillInstallDialog.test.tsx @@ -59,6 +59,7 @@ function installApi(previewInstall: ReturnType) { placements: [] } }), + cancelInstall: vi.fn().mockResolvedValue({ cancelled: true }), listWslDistros: vi.fn().mockResolvedValue([]) } } @@ -134,4 +135,71 @@ describe('SkillInstallDialog', () => { ) expect(skills.installShare).not.toHaveBeenCalled() }) + + it('invalidates skill discovery after a verified install', async () => { + const previewInstall = vi.fn().mockResolvedValue({ + status: 'ok', + value: { + name: 'private-skill', + packageDigest: DIGEST, + destinationIdentity: 'local:global', + currentState: 'missing', + providers: [] + } + }) + const skills = installApi(previewInstall) + Object.defineProperty(window, 'api', { configurable: true, value: { skills } }) + const changed = vi.fn() + window.addEventListener('orca:installed-agent-skills-changed', changed) + render( undefined} />) + await inspectSkill() + + fireEvent.click(screen.getByRole('button', { name: 'Install skill' })) + + await screen.findByText('Installed and verified.') + expect(changed).toHaveBeenCalledOnce() + window.removeEventListener('orca:installed-agent-skills-changed', changed) + }) + + it('cancels an active destination-owned install and renders the structured result', async () => { + const previewInstall = vi.fn().mockResolvedValue({ + status: 'ok', + value: { + name: 'private-skill', + packageDigest: DIGEST, + destinationIdentity: 'local:global', + currentState: 'missing', + providers: [] + } + }) + let settleInstall: ((value: unknown) => void) | undefined + const skills = installApi(previewInstall) + skills.installShare.mockImplementation( + () => new Promise((resolve) => (settleInstall = resolve)) as never + ) + Object.defineProperty(window, 'api', { configurable: true, value: { skills } }) + render( undefined} />) + await inspectSkill() + + fireEvent.click(screen.getByRole('button', { name: 'Install skill' })) + fireEvent.click(await screen.findByRole('button', { name: 'Cancel installation' })) + + await waitFor(() => expect(skills.cancelInstall).toHaveBeenCalledOnce()) + const operationId = skills.installShare.mock.calls[0]?.[0].operationId + expect(skills.cancelInstall).toHaveBeenCalledWith({ operationId }) + settleInstall?.({ + status: 'ok', + value: { + operationId, + status: 'cancelled', + name: 'private-skill', + packageDigest: DIGEST, + placements: [], + errorCategory: 'skill-install-cancelled', + failure: { category: 'cancelled', code: 'skill-install-cancelled', retryable: true } + } + }) + await screen.findByText('Installation cancelled.') + expect(screen.getByRole('button', { name: 'Retry install' })).toBeDefined() + }) }) diff --git a/src/renderer/src/components/skills/SkillInstallDialog.tsx b/src/renderer/src/components/skills/SkillInstallDialog.tsx index b0c6dc66500..7d68cafafca 100644 --- a/src/renderer/src/components/skills/SkillInstallDialog.tsx +++ b/src/renderer/src/components/skills/SkillInstallDialog.tsx @@ -1,6 +1,5 @@ import { useMemo, useState } from 'react' -import { AlertTriangle, Check, Download, Loader2, ShieldCheck } from 'lucide-react' -import { Badge } from '@/components/ui/badge' +import { Download, Loader2 } from 'lucide-react' import { Button } from '@/components/ui/button' import { Dialog, @@ -10,8 +9,6 @@ import { DialogHeader, DialogTitle } from '@/components/ui/dialog' -import { Input } from '@/components/ui/input' -import { Label } from '@/components/ui/label' import { useAppStore } from '@/store' import type { SkillInstallDestination, @@ -19,10 +16,15 @@ import type { SkillInstallResult } from '../../../../shared/skill-install-contract' import { parseSkillShareId } from './skill-share-link' -import { skillInstallResultLabel } from './skill-install-result-label' import { skillInstallWorkspaceChoices } from './skill-install-workspace-choices' -import { summarizeSkillShareVersion, type ResolvedSkillShare } from './skill-share-version-summary' +import type { ResolvedSkillShare } from './skill-share-version-summary' import { SkillInstallTargetFields } from './SkillInstallTargetFields' +import { + SkillInstallOutcome, + SkillInstallReview, + SkillShareLinkInputForm +} from './SkillInstallReviewContent' +import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills' export function SkillInstallDialog({ open, @@ -52,6 +54,7 @@ export function SkillInstallDialog({ const [error, setError] = useState(null) const [result, setResult] = useState(null) const [destinationPreview, setDestinationPreview] = useState(null) + const [activeOperationId, setActiveOperationId] = useState(null) const workspaceChoices = useMemo( () => skillInstallWorkspaceChoices({ environmentId, folderWorkspaces, repos, worktreesByRepo }), @@ -149,8 +152,11 @@ export function SkillInstallDialog({ return } } + const operationId = crypto.randomUUID() + setActiveOperationId(operationId) const operation = await window.api.skills.installShare({ shareId: preview.shareId, + operationId, ...(environmentId === 'local' || environmentId.startsWith('ssh:') ? {} : { environmentId }), destination, ...(discardLocal ? { conflictResolution: 'replace-and-discard-local' } : {}) @@ -168,14 +174,31 @@ export function SkillInstallDialog({ return } setResult(operation.value) + if (!['conflict', 'failed', 'cancelled'].includes(operation.value.status)) { + notifyInstalledAgentSkillsChanged() + } } catch (cause) { console.warn('[skills] install failed:', cause) setError('Installation failed before Orca could verify the requested version.') } finally { + setActiveOperationId(null) setBusy(false) } } + const cancelInstall = async (): Promise => { + if (!activeOperationId) { + return + } + const cancelled = await window.api.skills.cancelInstall({ + operationId: activeOperationId, + ...(environmentId === 'local' || environmentId.startsWith('ssh:') ? {} : { environmentId }) + }) + if (!cancelled.cancelled) { + setError('The destination had already finished this installation.') + } + } + const close = (): void => { setPreview(null) setDestinationPreview(null) @@ -187,9 +210,6 @@ export function SkillInstallDialog({ onOpenChange(false) } - const version = preview?.version - const versionSummary = summarizeSkillShareVersion(version) - return ( !next && !busy && close()}> @@ -201,95 +221,22 @@ export function SkillInstallDialog({ {!preview ? ( -
{ - event.preventDefault() - void inspect() - }} - > -
- - setLink(event.target.value)} - placeholder="https://app.orca.dev/skills/share/…" - className="font-mono text-xs" - autoFocus - /> -

- Opening the link does not install anything. Review the immutable version first. -

-
- -
+ void inspect()} + /> ) : result && result.status !== 'conflict' ? ( -
-
-
- -
-
-

{skillInstallResultLabel(result)}

-

- {result.placements.length} placement{result.placements.length === 1 ? '' : 's'}{' '} - checked. -

-
-
- {result.status === 'partial' ? ( -
- {result.placements - .filter((item) => item.status === 'failed' || item.status === 'skipped') - .map((item) => ( -

- {item.provider}: {item.errorCategory || item.status} -

- ))} -
- ) : null} -
+ ) : ( -
-
-
-
-

{version?.name}

-

{version?.description}

-
- Immutable version -
-
- {version?.manifest.files.length} files - {versionSummary.scriptCount} scripts - {versionSummary.executableCount} executable -
-

- SHA-256 {version?.packageDigest} -

- {version?.publisher ? ( -

- Published by Orca user {version.publisher.userId} - {version.publisher.organizationId - ? ` in organization ${version.publisher.organizationId}` - : ''} - . -

- ) : null} -

- A skill contains instructions and may include scripts. Treat it as code from its - author. -

-
- + void install(true)} + > { @@ -316,34 +263,7 @@ export function SkillInstallDialog({ sshConnections={sshConnections} workspaceChoices={workspaceChoices} /> - - {result?.status === 'conflict' || - (destinationPreview && - ['modified', 'unowned', 'external-link', 'name-collision'].includes( - destinationPreview.currentState - )) ? ( -
-
- Local copy needs a decision -
-

- Orca found{' '} - {result?.conflict?.kind || destinationPreview?.currentState || 'changed'} content - and left it untouched. Keep it, or explicitly discard and replace it with this - version. -

- -
- ) : null} -
+ )} {error ? ( @@ -355,7 +275,12 @@ export function SkillInstallDialog({ - {preview && (!result || result.status === 'conflict') ? ( + {busy && activeOperationId ? ( + + ) : null} + {preview && (!result || ['conflict', 'failed', 'cancelled'].includes(result.status)) ? ( ) : null} diff --git a/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx b/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx index aecee003ea7..86453bc28cd 100644 --- a/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx +++ b/src/renderer/src/components/skills/SkillInstallManagementDialog.test.tsx @@ -8,6 +8,7 @@ import type { SkillCloudVersion } from '../../../../shared/skill-cloud-contract' import { useAppStore } from '@/store' +import { INSTALLED_AGENT_SKILLS_CHANGED_EVENT } from '@/hooks/installed-agent-skills-change-event' import { SkillInstallManagementDialog } from './SkillInstallManagementDialog' const DIGEST = 'a'.repeat(64) @@ -62,10 +63,14 @@ function packageDetails(versions: SkillCloudVersion[]): SkillCloudPackageDetails } } -function skillsApi(installed: ManagedSkillInstall, versions: SkillCloudVersion[]) { +function skillsApi( + installed: ManagedSkillInstall, + versions: SkillCloudVersion[], + details = packageDetails(versions) +) { return { listManagedInstalls: vi.fn().mockResolvedValue({ status: 'ok', value: [installed] }), - getPackage: vi.fn().mockResolvedValue({ status: 'ok', value: packageDetails(versions) }), + getPackage: vi.fn().mockResolvedValue({ status: 'ok', value: details }), installPackageVersion: vi.fn().mockResolvedValue({ status: 'ok', value: { @@ -85,7 +90,11 @@ function skillsApi(installed: ManagedSkillInstall, versions: SkillCloudVersion[] packageDigest: DIGEST, placements: [] } - }) + }), + replacePackageAccess: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }), + revokeShare: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }), + deletePackageVersion: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }), + deletePackage: vi.fn().mockResolvedValue({ status: 'ok', value: undefined }) } } @@ -115,6 +124,8 @@ describe('SkillInstallManagementDialog', () => { version('ver_2', '2026-08-12T00:00:00.000Z'), version('ver_1', '2026-08-11T00:00:00.000Z') ]) + const changed = vi.fn() + window.addEventListener(INSTALLED_AGENT_SKILLS_CHANGED_EVENT, changed) Object.defineProperty(window, 'api', { configurable: true, value: { skills } }) render( undefined} />) await selectInstall('ver_1') @@ -125,6 +136,36 @@ describe('SkillInstallManagementDialog', () => { expect(skills.installPackageVersion).toHaveBeenCalledWith( expect.objectContaining({ packageId: 'pkg_1', versionId: 'ver_2' }) ) + expect(changed).toHaveBeenCalledOnce() + window.removeEventListener(INSTALLED_AGENT_SKILLS_CHANGED_EVENT, changed) + }) + + it('does not invalidate discovery after a failed local mutation', async () => { + const skills = skillsApi(install('ver_1'), [ + version('ver_2', '2026-08-12T00:00:00.000Z'), + version('ver_1', '2026-08-11T00:00:00.000Z') + ]) + skills.installPackageVersion.mockResolvedValue({ + status: 'ok', + value: { + operationId: 'operation_failed', + status: 'failed', + name: 'private-skill', + packageDigest: DIGEST, + placements: [] + } + }) + const changed = vi.fn() + window.addEventListener(INSTALLED_AGENT_SKILLS_CHANGED_EVENT, changed) + Object.defineProperty(window, 'api', { configurable: true, value: { skills } }) + render( undefined} />) + await selectInstall('ver_1') + + fireEvent.click(screen.getByRole('button', { name: 'Install selected version' })) + + await waitFor(() => expect(skills.installPackageVersion).toHaveBeenCalledOnce()) + expect(changed).not.toHaveBeenCalled() + window.removeEventListener(INSTALLED_AGENT_SKILLS_CHANGED_EVENT, changed) }) it('rolls an installed skill back to an older immutable version', async () => { @@ -159,4 +200,33 @@ describe('SkillInstallManagementDialog', () => { expect.objectContaining({ name: 'private-skill', destination: { scope: 'global' } }) ) }) + + it('keeps the local install visible after deleting its Cloud package', async () => { + const versions = [version('ver_2', '2026-08-12T00:00:00.000Z')] + const details: SkillCloudPackageDetails = { + ...packageDetails(versions), + management: { + userIds: [], + shareWithOrganization: false, + shares: [] + } + } + const skills = skillsApi(install('ver_2'), versions, details) + Object.defineProperty(window, 'api', { + configurable: true, + value: { + skills, + orcaProfiles: { authStatus: vi.fn().mockResolvedValue({ cloud: undefined }) } + } + }) + render( undefined} />) + await selectInstall('ver_2') + + fireEvent.click(screen.getByRole('button', { name: 'Delete Cloud package' })) + fireEvent.click(screen.getByRole('button', { name: 'Confirm package deletion' })) + + await screen.findByText('Cloud package deleted. The installed copy remains on this machine.') + expect(skills.deletePackage).toHaveBeenCalledWith('pkg_1') + expect(screen.getByText('global · ver_2')).toBeTruthy() + }) }) diff --git a/src/renderer/src/components/skills/SkillInstallManagementDialog.tsx b/src/renderer/src/components/skills/SkillInstallManagementDialog.tsx index f30f7be1d0f..17e80045b1b 100644 --- a/src/renderer/src/components/skills/SkillInstallManagementDialog.tsx +++ b/src/renderer/src/components/skills/SkillInstallManagementDialog.tsx @@ -23,6 +23,9 @@ import type { SkillInstallResult } from '../../../../shared/skill-install-contract' import type { SkillCloudPackageDetails } from '../../../../shared/skill-cloud-contract' +import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills' +import { SkillCloudManagementActions } from './SkillCloudManagementActions' +import { skillInstallResultLabel } from './skill-install-result-label' function installKey(install: ManagedSkillInstall): string { return `${install.destinationIdentity}:${install.name}:${install.packageId}` @@ -47,6 +50,8 @@ export function SkillInstallManagementDialog({ const [confirmRemove, setConfirmRemove] = useState(false) const [result, setResult] = useState(null) const [error, setError] = useState(null) + const [notice, setNotice] = useState(null) + const [activeOperationId, setActiveOperationId] = useState(null) const selected = useMemo( () => installs.find((install) => installKey(install) === selectedKey) ?? null, @@ -70,6 +75,7 @@ export function SkillInstallManagementDialog({ setInstalls(operation.value) setSelectedKey('') setDetails(null) + setNotice(null) } catch (cause) { console.warn('[skills] managed install listing failed:', cause) setError('Orca could not inspect managed installs on this machine.') @@ -86,6 +92,7 @@ export function SkillInstallManagementDialog({ setSelectedKey(installKey(install)) setBusy(true) setError(null) + setNotice(null) setResult(null) setConfirmRemove(false) try { @@ -108,16 +115,42 @@ export function SkillInstallManagementDialog({ } } + const refreshPackageDetails = async (): Promise => { + if (!selected) { + return + } + const operation = await window.api.skills.getPackage(selected.packageId) + if (operation.status !== 'ok') { + throw new Error(operation.status) + } + setDetails(operation.value) + setVersionId((current) => + operation.value.versions.some((version) => version.versionId === current) + ? current + : (operation.value.versions[0]?.versionId ?? '') + ) + } + + const packageDeleted = (): void => { + setDetails(null) + setVersionId('') + setNotice('Cloud package deleted. The installed copy remains on this machine.') + } + const installVersion = async (discardLocal = false): Promise => { if (!selected || !versionId) { return } setBusy(true) setError(null) + setNotice(null) + const operationId = crypto.randomUUID() + setActiveOperationId(operationId) try { const operation = await window.api.skills.installPackageVersion({ packageId: selected.packageId, versionId, + operationId, ...(environmentId === 'local' || environmentId.startsWith('ssh:') ? {} : { environmentId }), destination: selected.destination, ...(discardLocal ? { conflictResolution: 'replace-and-discard-local' } : {}) @@ -131,17 +164,32 @@ export function SkillInstallManagementDialog({ return } setResult(operation.value) - if (operation.value.status !== 'conflict') { + if (!['conflict', 'failed', 'cancelled'].includes(operation.value.status)) { + notifyInstalledAgentSkillsChanged() await load() } } catch (cause) { console.warn('[skills] version installation failed:', cause) setError('Orca could not verify the requested version.') } finally { + setActiveOperationId(null) setBusy(false) } } + const cancelInstall = async (): Promise => { + if (!activeOperationId) { + return + } + const cancelled = await window.api.skills.cancelInstall({ + operationId: activeOperationId, + ...(environmentId === 'local' || environmentId.startsWith('ssh:') ? {} : { environmentId }) + }) + if (!cancelled.cancelled) { + setError('The destination had already finished this installation.') + } + } + const remove = async (discardLocal = false): Promise => { if (!selected) { return @@ -152,6 +200,7 @@ export function SkillInstallManagementDialog({ } setBusy(true) setError(null) + setNotice(null) try { const operation = await window.api.skills.removeInstall({ ...(environmentId === 'local' || environmentId.startsWith('ssh:') ? {} : { environmentId }), @@ -164,7 +213,8 @@ export function SkillInstallManagementDialog({ return } setResult(operation.value) - if (operation.value.status !== 'conflict') { + if (!['conflict', 'failed', 'cancelled'].includes(operation.value.status)) { + notifyInstalledAgentSkillsChanged() await load() } } catch (cause) { @@ -179,6 +229,7 @@ export function SkillInstallManagementDialog({ setSelectedKey('') setDetails(null) setError(null) + setNotice(null) setResult(null) setConfirmRemove(false) onOpenChange(false) @@ -299,6 +350,11 @@ export function SkillInstallManagementDialog({ )} Install selected version + {activeOperationId ? ( + + ) : null} + ) : null} + {result ? ( +

+ {skillInstallResultLabel(result)} +

+ ) : null} {error ? (

{error}

) : null} + {notice ? ( +

+ {notice} +

+ ) : null} + + ) +} + +export function SkillInstallOutcome({ result }: { result: SkillInstallResult }): React.JSX.Element { + return ( +
+
+
+ {result.status === 'failed' || result.status === 'cancelled' ? ( + + ) : ( + + )} +
+
+

{skillInstallResultLabel(result)}

+

+ {result.placements.length} placement{result.placements.length === 1 ? '' : 's'} checked. +

+
+
+ {result.status === 'partial' ? ( +
+ {result.placements + .filter((item) => item.status === 'failed' || item.status === 'skipped') + .map((item) => ( +

+ {item.provider}: {item.errorCategory || item.status} +

+ ))} +
+ ) : null} + {result.failure ? ( +

+ {result.failure.code} + {result.failure.retryable ? ' · You can retry safely.' : ''} +

+ ) : null} +
+ ) +} + +export function SkillInstallReview({ + preview, + destinationPreview, + result, + busy, + onDiscard, + children +}: { + preview: ResolvedSkillShare + destinationPreview: SkillInstallPreview | null + result: SkillInstallResult | null + busy: boolean + onDiscard: () => void + children: ReactNode +}): React.JSX.Element { + const version = preview.version + const versionSummary = summarizeSkillShareVersion(version) + const hasConflict = + result?.status === 'conflict' || + (destinationPreview && + ['modified', 'unowned', 'external-link', 'name-collision'].includes( + destinationPreview.currentState + )) + return ( +
+
+
+
+

{version.name}

+

{version.description}

+
+ Immutable version +
+
+ {version.manifest.files.length} files + {versionSummary.scriptCount} scripts + {versionSummary.executableCount} executable +
+

+ SHA-256 {version.packageDigest} +

+ {version.publisher ? ( +

+ Published by Orca user {version.publisher.userId} + {version.publisher.organizationId + ? ` in organization ${version.publisher.organizationId}` + : ''} + . +

+ ) : null} +

+ A skill contains instructions and may include scripts. Treat it as code from its author. +

+
+ + {children} + + {hasConflict ? ( +
+
+ Local copy needs a decision +
+

+ Orca found {result?.conflict?.kind || destinationPreview?.currentState || 'changed'}{' '} + content and left it untouched. Keep it, or explicitly discard and replace it with this + version. +

+ +
+ ) : null} +
+ ) +} diff --git a/src/renderer/src/components/skills/SkillShareDialog.test.tsx b/src/renderer/src/components/skills/SkillShareDialog.test.tsx new file mode 100644 index 00000000000..2d967a01b90 --- /dev/null +++ b/src/renderer/src/components/skills/SkillShareDialog.test.tsx @@ -0,0 +1,153 @@ +// @vitest-environment happy-dom + +import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ManagedSkillInstall } from '../../../../shared/skill-install-contract' +import type { + SkillSharePreview, + SkillShareProgress +} from '../../../../shared/skill-sharing-contract' +import type { DiscoveredSkill } from '../../../../shared/skills' +import { SkillShareDialog } from './SkillShareDialog' + +const skill: DiscoveredSkill = { + id: 'home:private-skill', + name: 'private-skill', + description: 'Private skill', + providers: ['codex'], + sourceKind: 'home', + sourceLabel: 'Home', + rootPath: '/home/skills', + directoryPath: '/home/skills/private-skill', + skillFilePath: '/home/skills/private-skill/SKILL.md', + installed: true, + fileCount: 1, + updatedAt: null +} + +const preview: SkillSharePreview = { + preparationId: '11111111-1111-4111-8111-111111111111', + packageId: 'pkg_1', + versionId: 'ver_2', + name: 'private-skill', + description: 'Private skill', + packageDigest: 'a'.repeat(64), + archiveSha256: 'b'.repeat(64), + fileCount: 1, + totalBytes: 128, + compressedBytes: 96, + scriptPaths: [], + executablePaths: [], + expiresAt: '2026-08-11T01:00:00.000Z' +} + +function managedInstall(destinationIdentity: string): ManagedSkillInstall { + return { + name: 'private-skill', + packageId: 'pkg_1', + versionId: 'ver_1', + packageDigest: 'c'.repeat(64), + scope: 'global', + destinationIdentity, + destination: { scope: 'global' }, + installedAt: '2026-08-11T00:00:00.000Z', + state: 'unchanged' + } +} + +function setup(installs: ManagedSkillInstall[], organization = false) { + let progressListener: ((progress: SkillShareProgress) => void) | null = null + const skills = { + listManagedInstalls: vi.fn().mockResolvedValue({ status: 'ok', value: installs }), + prepareShare: vi.fn().mockResolvedValue(preview), + publishShare: vi.fn(), + cancelShare: vi.fn().mockResolvedValue(undefined), + releaseShare: vi.fn().mockResolvedValue(undefined), + onShareProgress: vi.fn((listener) => { + progressListener = listener + return () => undefined + }) + } + Object.defineProperty(window, 'api', { + configurable: true, + value: { + skills, + orcaProfiles: { + authStatus: vi.fn().mockResolvedValue({ + cloud: { + email: 'owner@example.com', + ...(organization ? { activeOrgId: 'org_1', activeOrgName: 'Orca' } : {}) + } + }), + orgMembersList: vi.fn().mockResolvedValue({ + status: 'ok', + roster: { members: [] } + }) + } + } + }) + render( undefined} />) + return { + skills, + emitProgress: (progress: SkillShareProgress) => progressListener?.(progress) + } +} + +afterEach(() => { + cleanup() + vi.restoreAllMocks() + Reflect.deleteProperty(window, 'api') +}) + +describe('SkillShareDialog', () => { + it('publishes a new immutable version for one exact managed-install match', async () => { + const { skills } = setup([managedInstall('global:local')]) + + await screen.findByRole('heading', { name: 'Publish new skill version' }) + expect(screen.getByRole('button', { name: 'Publish new version' })).toBeTruthy() + expect(skills.prepareShare).toHaveBeenCalledWith({ + skillId: skill.id, + packageId: 'pkg_1' + }) + }) + + it('does not choose a package when managed-install matching is ambiguous', async () => { + const { skills } = setup([ + managedInstall('global:local'), + managedInstall('global:other-environment') + ]) + + await screen.findByRole('heading', { name: 'Share skill' }) + await waitFor(() => + expect(skills.prepareShare).toHaveBeenCalledWith({ + skillId: skill.id + }) + ) + }) + + it('shows bounded upload progress and supports cancellation', async () => { + let rejectPublish: (error: Error) => void = () => undefined + const { skills, emitProgress } = setup([], true) + skills.publishShare.mockReturnValue( + new Promise((_resolve, reject) => { + rejectPublish = reject + }) + ) + await screen.findByRole('heading', { name: 'Share skill' }) + + fireEvent.click(screen.getByRole('button', { name: 'Publish skill' })) + await screen.findByRole('button', { name: 'Cancel upload' }) + emitProgress({ + preparationId: preview.preparationId, + phase: 'uploading', + bytesSent: 48, + totalBytes: 96 + }) + await screen.findByText('50%') + fireEvent.click(screen.getByRole('button', { name: 'Cancel upload' })) + await waitFor(() => expect(skills.cancelShare).toHaveBeenCalledWith(preview.preparationId)) + rejectPublish(new Error('aborted')) + + await screen.findByText('Upload cancelled. The prepared copy is still available to retry.') + }) +}) diff --git a/src/renderer/src/components/skills/SkillShareDialog.tsx b/src/renderer/src/components/skills/SkillShareDialog.tsx index f5f913b0fc4..56dda69f5b8 100644 --- a/src/renderer/src/components/skills/SkillShareDialog.tsx +++ b/src/renderer/src/components/skills/SkillShareDialog.tsx @@ -1,27 +1,22 @@ import { useEffect, useMemo, useRef, useState } from 'react' -import { Check, Clipboard, FileCode2, Loader2, Share2 } from 'lucide-react' +import { Loader2, Share2 } from 'lucide-react' import { toast } from 'sonner' -import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' -import { Checkbox } from '@/components/ui/checkbox' -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle -} from '@/components/ui/dialog' -import { Label } from '@/components/ui/label' -import { Progress } from '@/components/ui/progress' -import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs' +import { Dialog, DialogContent, DialogFooter } from '@/components/ui/dialog' import { translate } from '@/i18n/i18n' -import type { OrcaOrgMember } from '../../../../shared/orca-profiles' import type { SkillSharePreview, SkillShareProgress } from '../../../../shared/skill-sharing-contract' +import type { ManagedSkillInstall } from '../../../../shared/skill-install-contract' import type { DiscoveredSkill } from '../../../../shared/skills' +import { + SkillShareDialogHeader, + SkillSharePreparationReview, + SkillSharePublishedLink, + type SelectableOrgMember +} from './SkillShareReviewContent' +import { matchingManagedSkillInstall } from './skill-share-package-selection' type SkillShareDialogProps = { skill: DiscoveredSkill | null @@ -29,18 +24,6 @@ type SkillShareDialogProps = { onOpenChange: (open: boolean) => void } -type SelectableOrgMember = OrcaOrgMember & { userId: string } - -function byteLabel(bytes: number): string { - if (bytes < 1024) { - return `${bytes} B` - } - if (bytes < 1024 * 1024) { - return `${(bytes / 1024).toFixed(1)} KB` - } - return `${(bytes / (1024 * 1024)).toFixed(1)} MB` -} - function operationError(status: string): string { return status === 'reconnect-required' ? translate( @@ -70,7 +53,10 @@ export function SkillShareDialog({ const [error, setError] = useState(null) const [preparing, setPreparing] = useState(false) const [publishing, setPublishing] = useState(false) + const [cancelling, setCancelling] = useState(false) + const [publishingNewVersion, setPublishingNewVersion] = useState(false) const generation = useRef(0) + const cancellationRequested = useRef(false) useEffect(() => { if (!open || !skill) { @@ -81,17 +67,34 @@ export function SkillShareDialog({ setShareUrl(null) setProgress(null) setError(null) + setPublishingNewVersion(false) setPreparing(true) - void Promise.all([ - window.api.skills.prepareShare({ skillId: skill.id }), - window.api.orcaProfiles.authStatus() - ]) - .then(async ([nextPreview, auth]) => { + void (async () => { + let managedInstall: ManagedSkillInstall | null = null + try { + const operation = await window.api.skills.listManagedInstalls() + if (operation.status === 'ok') { + managedInstall = matchingManagedSkillInstall(skill, operation.value) + } + } catch (cause) { + console.warn('[skills] managed install lookup failed during share:', cause) + } + const [nextPreview, auth] = await Promise.all([ + window.api.skills.prepareShare({ + skillId: skill.id, + ...(managedInstall ? { packageId: managedInstall.packageId } : {}) + }), + window.api.orcaProfiles.authStatus() + ]) + return { nextPreview, auth, managedInstall } + })() + .then(async ({ nextPreview, auth, managedInstall }) => { if (generation.current !== current) { await window.api.skills.releaseShare(nextPreview.preparationId) return } setPreview(nextPreview) + setPublishingNewVersion(managedInstall !== null) const cloud = auth.cloud setAuthor(cloud?.displayName || cloud?.email || '') setOrganization(cloud?.activeOrgName || '') @@ -168,6 +171,7 @@ export function SkillShareDialog({ return } setPublishing(true) + cancellationRequested.current = false setError(null) try { const result = await window.api.skills.publishShare({ @@ -184,16 +188,38 @@ export function SkillShareDialog({ } catch (cause) { console.warn('[skills] publish failed:', cause) setError( - translate( - 'auto.components.skills.SkillShareDialog.publishFailed', - 'Could not publish this skill. The prepared copy is still available to retry.' - ) + cancellationRequested.current + ? translate( + 'auto.components.skills.SkillShareDialog.publishCancelled', + 'Upload cancelled. The prepared copy is still available to retry.' + ) + : translate( + 'auto.components.skills.SkillShareDialog.publishFailed', + 'Could not publish this skill. The prepared copy is still available to retry.' + ) ) } finally { + cancellationRequested.current = false + setCancelling(false) setPublishing(false) } } + const cancelPublish = async (): Promise => { + if (!preview || cancelling) { + return + } + cancellationRequested.current = true + setCancelling(true) + try { + await window.api.skills.cancelShare(preview.preparationId) + } catch { + cancellationRequested.current = false + setCancelling(false) + setError('Orca could not send the cancellation request. The upload may still finish.') + } + } + const copyLink = async (): Promise => { if (!shareUrl) { return @@ -205,24 +231,10 @@ export function SkillShareDialog({ return ( !next && !publishing && void close()}> - - - {shareUrl - ? translate('auto.components.skills.SkillShareDialog.ready', 'Skill link ready') - : translate('auto.components.skills.SkillShareDialog.title', 'Share skill')} - - - {shareUrl - ? translate( - 'auto.components.skills.SkillShareDialog.readyDescription', - 'Recipients authenticate with Orca before they can inspect or install it.' - ) - : translate( - 'auto.components.skills.SkillShareDialog.description', - 'Review the exact files, choose who can access them, then publish an immutable version.' - )} - - + {preparing ? (
@@ -230,124 +242,23 @@ export function SkillShareDialog({ {translate('auto.components.skills.SkillShareDialog.preparing', 'Preparing preview…')}
) : preview && !shareUrl ? ( -
-
-
-
- -
-
-

{preview.name}

-

{preview.description}

-
-
-
- {preview.fileCount} files - {byteLabel(preview.totalBytes)} - {preview.scriptPaths.length} scripts - {preview.executablePaths.length} executable -
-

- SHA-256 {preview.packageDigest} -

-
- -
-
- -

- {author - ? `Publishing as ${author}${organization ? ` in ${organization}` : ''}.` - : 'A connected Orca Cloud account is required.'} -

-
- setAudience(value as typeof audience)} - > - - - Organization - - Selected people - - - Everyone currently in {organization || 'the organization'} can access the link. - - - {members.length > 0 ? ( -
- {members.map((member) => { - const checked = selectedUserIds.includes(member.userId) - return ( - - ) - })} -
- ) : ( -

No teammates are available.

- )} - - -
- -
- -