mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 08:03:39 +00:00
Add skill deletion with cross-platform transaction safety
Implements end-to-end skill removal with placement enumeration, dependency guards, and transactional recovery. Covers native, WSL, and remote hosts; users can delete canonical directories and alias placements (symlinked directories or files) in a single atomic batch. Includes UI selection flow, preview, confirmation, and results band. Block reasons (bundled, plugin, unowned, stale) gate deletions that would fail or contradict user intent.
This commit is contained in:
@@ -59,6 +59,7 @@ const {
|
||||
registerTerminalPreviewHandlersMock,
|
||||
registerSpeechHandlersMock,
|
||||
registerSkillsHandlersMock,
|
||||
registerSkillDeleteIpcHandlersMock,
|
||||
registerWorkspaceSpaceHandlersMock,
|
||||
registerWorkspacePortHandlersMock,
|
||||
registerLocalhostWorktreeLabelHandlersMock,
|
||||
@@ -124,6 +125,7 @@ const {
|
||||
registerTerminalPreviewHandlersMock: vi.fn(),
|
||||
registerSpeechHandlersMock: vi.fn(),
|
||||
registerSkillsHandlersMock: vi.fn(),
|
||||
registerSkillDeleteIpcHandlersMock: vi.fn(),
|
||||
registerWorkspaceSpaceHandlersMock: vi.fn(),
|
||||
registerWorkspacePortHandlersMock: vi.fn(),
|
||||
registerLocalhostWorktreeLabelHandlersMock: vi.fn(),
|
||||
@@ -226,6 +228,10 @@ vi.mock('./skills', () => ({
|
||||
registerSkillsHandlers: registerSkillsHandlersMock
|
||||
}))
|
||||
|
||||
vi.mock('./skill-delete-ipc-handlers', () => ({
|
||||
registerSkillDeleteIpcHandlers: registerSkillDeleteIpcHandlersMock
|
||||
}))
|
||||
|
||||
vi.mock('./workspace-space', () => ({
|
||||
registerWorkspaceSpaceHandlers: registerWorkspaceSpaceHandlersMock
|
||||
}))
|
||||
@@ -441,6 +447,7 @@ describe('registerCoreHandlers', () => {
|
||||
registerTerminalPreviewHandlersMock.mockReset()
|
||||
registerSpeechHandlersMock.mockReset()
|
||||
registerSkillsHandlersMock.mockReset()
|
||||
registerSkillDeleteIpcHandlersMock.mockReset()
|
||||
registerWorkspaceSpaceHandlersMock.mockReset()
|
||||
registerWorkspacePortHandlersMock.mockReset()
|
||||
registerLocalhostWorktreeLabelHandlersMock.mockReset()
|
||||
@@ -528,6 +535,7 @@ describe('registerCoreHandlers', () => {
|
||||
expect(registerTerminalPreviewHandlersMock).toHaveBeenCalledWith(runtime)
|
||||
expect(registerSettingsHandlersMock).toHaveBeenCalledWith(store, agentAwakeService)
|
||||
expect(registerSkillsHandlersMock).toHaveBeenCalledWith(store, runtime)
|
||||
expect(registerSkillDeleteIpcHandlersMock).toHaveBeenCalledWith(store, runtime)
|
||||
expect(registerWorkspaceSpaceHandlersMock).toHaveBeenCalledWith(store)
|
||||
expect(registerWorkspacePortHandlersMock).toHaveBeenCalledWith(store)
|
||||
expect(registerLocalhostWorktreeLabelHandlersMock).toHaveBeenCalledWith(store)
|
||||
|
||||
@@ -39,6 +39,7 @@ import { registerSessionHandlers } from './session'
|
||||
import { registerSettingsHandlers } from './settings'
|
||||
import { registerDiagnosticsHandlers } from './diagnostics'
|
||||
import { registerSkillsHandlers } from './skills'
|
||||
import { registerSkillDeleteIpcHandlers } from './skill-delete-ipc-handlers'
|
||||
import { registerWorkspaceSpaceHandlers } from './workspace-space'
|
||||
import { registerWorkspacePortHandlers } from './workspace-ports'
|
||||
import { registerLocalhostWorktreeLabelHandlers } from './localhost-worktree-labels'
|
||||
@@ -176,6 +177,7 @@ export function registerCoreHandlers(
|
||||
registerComputerUsePermissionHandlers()
|
||||
registerSettingsHandlers(store, agentAwakeService)
|
||||
registerSkillsHandlers(store, runtime)
|
||||
registerSkillDeleteIpcHandlers(store, runtime)
|
||||
if (automations) {
|
||||
registerAutomationHandlers(store, automations)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
import { app } from 'electron'
|
||||
import type { Store } from '../persistence'
|
||||
import type { OrcaRuntimeService } from '../runtime/orca-runtime'
|
||||
import {
|
||||
SkillDeleteRequestSchema,
|
||||
type SkillDeletePlan,
|
||||
type SkillDeleteResult
|
||||
} from '../../shared/skill-delete-contract'
|
||||
import { SkillDiscoveryTargetSchema } from '../../shared/skills'
|
||||
import {
|
||||
previewSkillDeleteRequest,
|
||||
runSkillDeleteRequest,
|
||||
type SkillDeleteRequestDependencies
|
||||
} from '../skills/skill-delete-request-service'
|
||||
import { resolveSkillDiscoveryTarget } from '../skills/skill-discovery-target'
|
||||
import { handleMainWindowSkillIpc } from './skill-ipc-main-window'
|
||||
|
||||
/**
|
||||
* Local only. A remote runtime is reached straight from the renderer through
|
||||
* `callRuntimeRpc`, mirroring how the scan is routed — there is deliberately no
|
||||
* main-process remote branch here, unlike install and remove, because a
|
||||
* main-side capability check on one transport never runs on the other.
|
||||
*/
|
||||
export function registerSkillDeleteIpcHandlers(store: Store, runtime?: OrcaRuntimeService): void {
|
||||
const dependencies: SkillDeleteRequestDependencies = {
|
||||
repos: () => store.getRepos(),
|
||||
...(runtime
|
||||
? {
|
||||
resolveProviderRootOverrides: (target) =>
|
||||
runtime.resolveSkillDiscoveryProviderRoots(target)
|
||||
}
|
||||
: {}),
|
||||
userDataPath: app.getPath('userData')
|
||||
}
|
||||
const resolve = (value: unknown) => {
|
||||
const request = SkillDeleteRequestSchema.parse(value)
|
||||
// Send exactly what the scan sent: the nested target keeps discovery's own
|
||||
// stripping schema, while the request itself is strict.
|
||||
const target = resolveSkillDiscoveryTarget(
|
||||
request.target ? SkillDiscoveryTargetSchema.parse(request.target) : undefined
|
||||
)
|
||||
return { request, target }
|
||||
}
|
||||
|
||||
handleMainWindowSkillIpc(
|
||||
'skills:previewDelete',
|
||||
async (_event, value: unknown): Promise<SkillDeletePlan> => {
|
||||
const { request, target } = resolve(value)
|
||||
return previewSkillDeleteRequest(request, target, dependencies)
|
||||
}
|
||||
)
|
||||
|
||||
handleMainWindowSkillIpc(
|
||||
'skills:delete',
|
||||
async (_event, value: unknown): Promise<SkillDeleteResult> => {
|
||||
const { request, target } = resolve(value)
|
||||
return runSkillDeleteRequest(request, target, dependencies)
|
||||
}
|
||||
)
|
||||
}
|
||||
@@ -1,5 +1,12 @@
|
||||
import { app } from 'electron'
|
||||
import { defineMethod, type RpcMethod } from '../core'
|
||||
import { z } from 'zod'
|
||||
import { SkillDeleteRequestSchema } from '../../../../shared/skill-delete-contract'
|
||||
import {
|
||||
previewSkillDeleteRequest,
|
||||
runSkillDeleteRequest,
|
||||
type SkillDeleteRequestDependencies
|
||||
} from '../../../skills/skill-delete-request-service'
|
||||
import { SkillDiscoveryTargetSchema } from '../../../../shared/skills'
|
||||
import {
|
||||
SkillInstallPreviewRequestSchema,
|
||||
@@ -27,7 +34,9 @@ import {
|
||||
AgentSkillSharingError
|
||||
} from '../../../../shared/agent-skill-sharing-contract'
|
||||
|
||||
function resolveDiscoveryTarget(
|
||||
/** Exported so the delete plan's root rebuild resolves its target exactly the
|
||||
* way `skills.discover` resolved the scan's — including WSL. */
|
||||
export function resolveDiscoveryTarget(
|
||||
params: z.infer<typeof SkillDiscoveryTargetSchema>,
|
||||
runtime: Pick<OrcaRuntimeService, 'resolveProjectRuntimeForWorktree'>
|
||||
) {
|
||||
@@ -40,6 +49,16 @@ function resolveDiscoveryTarget(
|
||||
return resolveSkillDiscoveryTarget(target)
|
||||
}
|
||||
|
||||
function skillDeleteDependencies(
|
||||
runtime: Pick<OrcaRuntimeService, 'listRepos' | 'resolveSkillDiscoveryProviderRoots'>
|
||||
): SkillDeleteRequestDependencies {
|
||||
return {
|
||||
repos: () => runtime.listRepos(),
|
||||
resolveProviderRootOverrides: (target) => runtime.resolveSkillDiscoveryProviderRoots(target),
|
||||
userDataPath: app.getPath('userData')
|
||||
}
|
||||
}
|
||||
|
||||
export const SKILL_METHODS: RpcMethod[] = [
|
||||
defineMethod({
|
||||
name: 'skills.discover',
|
||||
@@ -55,6 +74,26 @@ export const SKILL_METHODS: RpcMethod[] = [
|
||||
})
|
||||
}
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'skills.previewDelete',
|
||||
params: SkillDeleteRequestSchema,
|
||||
handler: async (params, { runtime }) =>
|
||||
previewSkillDeleteRequest(
|
||||
params,
|
||||
resolveDiscoveryTarget(params.target ?? {}, runtime),
|
||||
skillDeleteDependencies(runtime)
|
||||
)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'skills.delete',
|
||||
params: SkillDeleteRequestSchema,
|
||||
handler: async (params, { runtime }) =>
|
||||
runSkillDeleteRequest(
|
||||
params,
|
||||
resolveDiscoveryTarget(params.target ?? {}, runtime),
|
||||
skillDeleteDependencies(runtime)
|
||||
)
|
||||
}),
|
||||
defineMethod({
|
||||
name: 'skills.share',
|
||||
params: AgentSkillShareRequestSchema,
|
||||
|
||||
@@ -25,6 +25,7 @@ import {
|
||||
type SkillScanOutcome
|
||||
} from './skill-scan-coalescer'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
import { skillDirectoryMaxDepth } from '../../shared/skill-discovery-depth'
|
||||
|
||||
export { buildSkillDiscoverySources } from './skill-discovery-sources'
|
||||
|
||||
@@ -136,7 +137,7 @@ async function readSkillSummary(skillFilePath: string): Promise<{
|
||||
type ScannedSkill = DiscoveredSkill & { canonicalSkillFilePath: string }
|
||||
|
||||
async function scanRoot(root: SkillScanRoot, signal: AbortSignal): Promise<ScannedSkill[]> {
|
||||
const maxDepth = root.sourceKind === 'plugin' ? 9 : 4
|
||||
const maxDepth = skillDirectoryMaxDepth(root.sourceKind)
|
||||
const skillFiles = await findSkillFiles(root.path, maxDepth, signal)
|
||||
// Why: a root can hold many packages and each one costs a summary read plus a
|
||||
// package walk. Unbounded fan-out here is what turned one scan into a burst of
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
import { posix as pathPosix, win32 as pathWin32 } from 'node:path'
|
||||
import { skillDirectoryMaxDepth } from '../../shared/skill-discovery-depth'
|
||||
import type { SkillPathSemantics } from '../../shared/skill-path-containment'
|
||||
import type { SkillScanRoot } from './skill-discovery-sources'
|
||||
import type { SkillInstallFilesystem, SkillPathInspection } from './skill-install-filesystem'
|
||||
import { SKILL_FILE_NAME } from './skill-root-file-walk'
|
||||
import { isSkillStagingEntryName } from './skill-staging-names'
|
||||
|
||||
/** One directory in a discovery root that holds a `SKILL.md`, with everything
|
||||
* the classifier needs to decide which of the three placement kinds it is. */
|
||||
export type SkillPlacementCandidate = {
|
||||
path: string
|
||||
root: SkillScanRoot
|
||||
/** `lstat` kind of the directory entry itself — a symlink here is an alias. */
|
||||
entryKind: 'directory' | 'symlink'
|
||||
/** `lstat` kind of the `SKILL.md` inside it. */
|
||||
skillFileKind: 'file' | 'symlink'
|
||||
directoryRealpath: string | null
|
||||
skillFileRealpath: string | null
|
||||
}
|
||||
|
||||
export class SkillDeleteEnumerationUnsupportedError extends Error {
|
||||
constructor() {
|
||||
super('skill-delete-filesystem-cannot-enumerate')
|
||||
this.name = 'SkillDeleteEnumerationUnsupportedError'
|
||||
}
|
||||
}
|
||||
|
||||
type Enumerable = Required<Pick<SkillInstallFilesystem, 'listEntries' | 'inspectPaths'>>
|
||||
|
||||
export function requireEnumerableFilesystem(filesystem: SkillInstallFilesystem): Enumerable {
|
||||
if (!filesystem.listEntries || !filesystem.inspectPaths) {
|
||||
throw new SkillDeleteEnumerationUnsupportedError()
|
||||
}
|
||||
return { listEntries: filesystem.listEntries, inspectPaths: filesystem.inspectPaths }
|
||||
}
|
||||
|
||||
/**
|
||||
* Breadth-first so every level costs one batched round trip rather than one per
|
||||
* directory — the bound that keeps a WSL plan to a handful of `wsl.exe` boots
|
||||
* instead of hundreds.
|
||||
*
|
||||
* Plugin roots are deliberately not walked: they nest up to ten levels, and a
|
||||
* plugin skill is blocked from the canonical path alone (see the guards), so
|
||||
* paying for that walk would buy nothing.
|
||||
*/
|
||||
export async function enumerateSkillPlacementCandidates(input: {
|
||||
roots: readonly SkillScanRoot[]
|
||||
filesystem: SkillInstallFilesystem
|
||||
semantics: SkillPathSemantics
|
||||
toFilesystemPath: (path: string) => string
|
||||
}): Promise<SkillPlacementCandidate[]> {
|
||||
const { listEntries, inspectPaths } = requireEnumerableFilesystem(input.filesystem)
|
||||
const api = input.semantics.sep === '\\' ? pathWin32 : pathPosix
|
||||
const walkable = input.roots.filter((root) => root.sourceKind !== 'plugin')
|
||||
const candidates: SkillPlacementCandidate[] = []
|
||||
const inspectionTargets = new Set<string>()
|
||||
let frontier = dedupeByPath(walkable.map((root) => ({ root, path: root.path, depth: 0 })))
|
||||
|
||||
while (frontier.length > 0) {
|
||||
const listings = await listEntries(frontier.map((entry) => input.toFilesystemPath(entry.path)))
|
||||
const next: typeof frontier = []
|
||||
for (const current of frontier) {
|
||||
const entries = listings.get(input.toFilesystemPath(current.path)) ?? []
|
||||
const skillFile = entries.find((entry) => entry.name === SKILL_FILE_NAME)
|
||||
// The root itself is never a placement, only the directories below it.
|
||||
if (current.depth > 0 && (skillFile?.kind === 'file' || skillFile?.kind === 'symlink')) {
|
||||
candidates.push({
|
||||
path: current.path,
|
||||
root: current.root,
|
||||
entryKind: 'directory',
|
||||
skillFileKind: skillFile.kind,
|
||||
directoryRealpath: null,
|
||||
skillFileRealpath: null
|
||||
})
|
||||
inspectionTargets.add(current.path)
|
||||
inspectionTargets.add(api.join(current.path, SKILL_FILE_NAME))
|
||||
}
|
||||
for (const entry of entries) {
|
||||
if (entry.kind !== 'directory' && entry.kind !== 'symlink') {
|
||||
continue
|
||||
}
|
||||
// The SKILL.md of this directory is already accounted for above; as a
|
||||
// child entry it can only ever be a file, never a placement.
|
||||
if (entry.name === SKILL_FILE_NAME || isSkillStagingEntryName(entry.name)) {
|
||||
continue
|
||||
}
|
||||
const childPath = api.join(current.path, entry.name)
|
||||
if (entry.kind === 'symlink') {
|
||||
// A symlinked directory is an alias candidate on its own, and is not
|
||||
// descended into: its contents belong to whatever it points at.
|
||||
candidates.push({
|
||||
path: childPath,
|
||||
root: current.root,
|
||||
entryKind: 'symlink',
|
||||
skillFileKind: 'file',
|
||||
directoryRealpath: null,
|
||||
skillFileRealpath: null
|
||||
})
|
||||
inspectionTargets.add(childPath)
|
||||
inspectionTargets.add(api.join(childPath, SKILL_FILE_NAME))
|
||||
continue
|
||||
}
|
||||
if (current.depth + 1 <= skillDirectoryMaxDepth(current.root.sourceKind)) {
|
||||
next.push({ root: current.root, path: childPath, depth: current.depth + 1 })
|
||||
}
|
||||
}
|
||||
}
|
||||
// Two roots can share a path — `~/.claude/skills` is both a home root and a
|
||||
// repo root when the home directory is the workspace — so without this the
|
||||
// same directory is walked (and later staged) twice.
|
||||
frontier = dedupeByPath(next)
|
||||
}
|
||||
|
||||
const inspections = await resolveInspections(inspectPaths, inspectionTargets, input)
|
||||
return candidates
|
||||
.map((candidate) => {
|
||||
const directory = inspections.get(candidate.path)
|
||||
const skillFile = inspections.get(api.join(candidate.path, SKILL_FILE_NAME))
|
||||
return {
|
||||
...candidate,
|
||||
entryKind: directory?.kind === 'symlink' ? ('symlink' as const) : candidate.entryKind,
|
||||
skillFileKind:
|
||||
skillFile?.kind === 'symlink' ? ('symlink' as const) : candidate.skillFileKind,
|
||||
directoryRealpath: directory?.realpath ?? null,
|
||||
skillFileRealpath: skillFile?.realpath ?? null
|
||||
}
|
||||
})
|
||||
.filter(
|
||||
(candidate) => candidate.skillFileRealpath !== null || candidate.entryKind === 'symlink'
|
||||
)
|
||||
}
|
||||
|
||||
function dedupeByPath<T extends { path: string }>(entries: readonly T[]): T[] {
|
||||
const seen = new Set<string>()
|
||||
return entries.filter((entry) => {
|
||||
if (seen.has(entry.path)) {
|
||||
return false
|
||||
}
|
||||
seen.add(entry.path)
|
||||
return true
|
||||
})
|
||||
}
|
||||
|
||||
async function resolveInspections(
|
||||
inspectPaths: Enumerable['inspectPaths'],
|
||||
targets: ReadonlySet<string>,
|
||||
input: { toFilesystemPath: (path: string) => string }
|
||||
): Promise<Map<string, SkillPathInspection>> {
|
||||
const ordered = [...targets]
|
||||
const raw = await inspectPaths(ordered.map((path) => input.toFilesystemPath(path)))
|
||||
return new Map(
|
||||
ordered.flatMap((path) => {
|
||||
const inspection = raw.get(input.toFilesystemPath(path))
|
||||
return inspection ? [[path, inspection] as const] : []
|
||||
})
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,137 @@
|
||||
import { posix as pathPosix, win32 as pathWin32 } from 'node:path'
|
||||
import { skillFileMaxDepth } from '../../shared/skill-discovery-depth'
|
||||
import type {
|
||||
SkillDeleteBlockReason,
|
||||
SkillDeletePlacement
|
||||
} from '../../shared/skill-delete-contract'
|
||||
import {
|
||||
skillPathDepthBelow,
|
||||
skillPathsEqual,
|
||||
type SkillPathSemantics
|
||||
} from '../../shared/skill-path-containment'
|
||||
import type { SkillScanRoot } from './skill-discovery-sources'
|
||||
import type { SkillPlacementCandidate } from './skill-delete-enumeration'
|
||||
|
||||
/**
|
||||
* Headroom against ordinary clock and read skew, not against WSL's
|
||||
* second-granularity mtime: the displayed value and the host's re-read come
|
||||
* from the same `stat` on the same host, so they already agree exactly.
|
||||
*/
|
||||
export const SKILL_DELETE_FRESHNESS_TOLERANCE_MS = 1_000
|
||||
|
||||
export type SkillDeleteGuardContext = {
|
||||
semantics: SkillPathSemantics
|
||||
roots: readonly SkillScanRoot[]
|
||||
}
|
||||
|
||||
function pathApi(semantics: SkillPathSemantics): typeof pathPosix {
|
||||
return semantics.sep === '\\' ? pathWin32 : pathPosix
|
||||
}
|
||||
|
||||
/**
|
||||
* A request carrying no `updatedAt` fails closed. It is reachable on WSL when
|
||||
* the guest `stat` fails, and "unknown" is not "unchanged" for a delete.
|
||||
*/
|
||||
export function isSkillDeleteFresh(
|
||||
displayedUpdatedAt: number | null,
|
||||
observedMtimeMs: number | null
|
||||
): boolean {
|
||||
if (displayedUpdatedAt === null || observedMtimeMs === null) {
|
||||
return false
|
||||
}
|
||||
return Math.abs(displayedUpdatedAt - observedMtimeMs) <= SKILL_DELETE_FRESHNESS_TOLERANCE_MS
|
||||
}
|
||||
|
||||
/** The root a path belongs to, honouring depth; null when no root owns it. */
|
||||
export function owningSkillRoot(
|
||||
path: string,
|
||||
context: SkillDeleteGuardContext
|
||||
): SkillScanRoot | null {
|
||||
const api = pathApi(context.semantics)
|
||||
const skillFilePath = api.join(path, 'SKILL.md')
|
||||
for (const root of context.roots) {
|
||||
const depth = skillPathDepthBelow(root.path, skillFilePath, context.semantics)
|
||||
if (depth !== null && depth <= skillFileMaxDepth(root.sourceKind)) {
|
||||
return root
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* Host-side eligibility, re-derived from where the skill actually lives rather
|
||||
* than read off the row: `sourceKind` is first-scanned-root-wins, so a
|
||||
* plugin-cache or `.system/` skill symlinked into a plain home root reports
|
||||
* `home` on the client. The host verdict may be stricter than the badge.
|
||||
*
|
||||
* A canonical directory *outside* every root is deliberately not blocked here.
|
||||
* The rule is "remove every placement Orca owns, and never touch anything
|
||||
* outside its roots" — so a tool-managed skill linked into a repo stays
|
||||
* deletable by its links, while its content is left alone. When no placement
|
||||
* sits inside a root either, the plan reports `unowned` on that basis instead.
|
||||
*/
|
||||
export function blockedCanonicalReason(
|
||||
canonicalDirectoryPath: string,
|
||||
context: SkillDeleteGuardContext
|
||||
): SkillDeleteBlockReason | null {
|
||||
const api = pathApi(context.semantics)
|
||||
const canonicalSkillFile = api.join(canonicalDirectoryPath, 'SKILL.md')
|
||||
for (const root of context.roots) {
|
||||
const depth = skillPathDepthBelow(root.path, canonicalSkillFile, context.semantics)
|
||||
if (depth === null || depth > skillFileMaxDepth(root.sourceKind)) {
|
||||
continue
|
||||
}
|
||||
if (root.sourceKind === 'plugin') {
|
||||
return 'plugin'
|
||||
}
|
||||
if (
|
||||
root.sourceKind === 'home' &&
|
||||
api.relative(root.path, canonicalSkillFile).split(context.semantics.sep)[0] === '.system'
|
||||
) {
|
||||
return 'bundled'
|
||||
}
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
export type ClassifiedPlacement = SkillDeletePlacement & { root: SkillScanRoot }
|
||||
|
||||
/**
|
||||
* Each kind compares against a different realpath. Comparing every entry's own
|
||||
* realpath against the file identity alone finds every `canonical` placement
|
||||
* and silently zero `alias-dir` ones — the exact shape this exists to remove.
|
||||
*/
|
||||
export function classifySkillPlacement(
|
||||
candidate: SkillPlacementCandidate,
|
||||
canonicalSkillFilePath: string,
|
||||
context: SkillDeleteGuardContext
|
||||
): ClassifiedPlacement | null {
|
||||
const api = pathApi(context.semantics)
|
||||
const canonicalDirectoryPath = api.dirname(canonicalSkillFilePath)
|
||||
const owningRoot = owningSkillRoot(candidate.path, context)
|
||||
if (!owningRoot || skillPathsEqual(owningRoot.path, candidate.path, context.semantics)) {
|
||||
return null
|
||||
}
|
||||
const placement = (kind: SkillDeletePlacement['kind']): ClassifiedPlacement => ({
|
||||
path: candidate.path,
|
||||
kind,
|
||||
rootLabel: owningRoot.label,
|
||||
root: owningRoot
|
||||
})
|
||||
|
||||
if (candidate.entryKind === 'symlink') {
|
||||
// A symlinked directory's realpath can never equal a file path, so this
|
||||
// compares against the canonical *directory*.
|
||||
return candidate.directoryRealpath &&
|
||||
skillPathsEqual(candidate.directoryRealpath, canonicalDirectoryPath, context.semantics)
|
||||
? placement('alias-dir')
|
||||
: null
|
||||
}
|
||||
if (
|
||||
!candidate.skillFileRealpath ||
|
||||
!skillPathsEqual(candidate.skillFileRealpath, canonicalSkillFilePath, context.semantics)
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return placement(candidate.skillFileKind === 'symlink' ? 'alias-file' : 'canonical')
|
||||
}
|
||||
@@ -0,0 +1,240 @@
|
||||
import { mkdir, mkdtemp, rm, stat, symlink, utimes, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import type { SkillDeleteRequest } from '../../shared/skill-delete-contract'
|
||||
import { buildSkillDeletePlan } from './skill-delete-plan'
|
||||
import { nativeSkillInstallFilesystem } from './skill-install-filesystem'
|
||||
|
||||
const roots: string[] = []
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })))
|
||||
})
|
||||
|
||||
async function fixture(): Promise<{ home: string }> {
|
||||
const home = await mkdtemp(join(tmpdir(), 'orca-skill-delete-plan-'))
|
||||
roots.push(home)
|
||||
return { home }
|
||||
}
|
||||
|
||||
async function writeSkill(directory: string, name: string): Promise<string> {
|
||||
await mkdir(directory, { recursive: true })
|
||||
const file = join(directory, 'SKILL.md')
|
||||
await writeFile(file, `---\nname: ${name}\ndescription: ${name}\n---\n\n# ${name}\n`)
|
||||
return file
|
||||
}
|
||||
|
||||
async function request(
|
||||
skillFilePath: string,
|
||||
overrides: Partial<SkillDeleteRequest['skills'][number]> = {}
|
||||
): Promise<SkillDeleteRequest> {
|
||||
const mtime = (await stat(skillFilePath).catch(() => null))?.mtimeMs ?? null
|
||||
return {
|
||||
operationId: 'op',
|
||||
skills: [
|
||||
{
|
||||
id: 'skill-id',
|
||||
directoryPath: join(skillFilePath, '..'),
|
||||
skillFilePath,
|
||||
name: 'demo',
|
||||
updatedAt: mtime,
|
||||
...overrides
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
function plan(home: string, deleteRequest: SkillDeleteRequest) {
|
||||
return buildSkillDeletePlan({
|
||||
request: deleteRequest,
|
||||
target: { kind: 'native-host', cwd: undefined },
|
||||
repos: [],
|
||||
filesystem: nativeSkillInstallFilesystem,
|
||||
homeDir: home
|
||||
})
|
||||
}
|
||||
|
||||
describe('buildSkillDeletePlan placement enumeration', () => {
|
||||
it('finds the canonical directory and every alias that resolves to it', async () => {
|
||||
const { home } = await fixture()
|
||||
const canonicalDirectory = join(home, '.agents', 'skills', 'demo')
|
||||
const file = await writeSkill(canonicalDirectory, 'demo')
|
||||
// An alias-dir: a symlinked directory in another agent's root.
|
||||
await mkdir(join(home, '.claude', 'skills'), { recursive: true })
|
||||
await symlink(canonicalDirectory, join(home, '.claude', 'skills', 'demo'), 'dir')
|
||||
// An alias-file: a real directory whose SKILL.md is a symlink.
|
||||
const aliasFileDirectory = join(home, '.codex', 'skills', 'demo')
|
||||
await mkdir(aliasFileDirectory, { recursive: true })
|
||||
await symlink(file, join(aliasFileDirectory, 'SKILL.md'))
|
||||
|
||||
const resolved = await plan(home, await request(file))
|
||||
const entry = resolved.plan.skills[0]
|
||||
expect(entry.blocked).toBeUndefined()
|
||||
expect(new Set(entry.placements.map((placement) => placement.kind))).toEqual(
|
||||
new Set(['canonical', 'alias-dir', 'alias-file'])
|
||||
)
|
||||
expect(entry.placements.map((placement) => placement.path)).toContain(canonicalDirectory)
|
||||
})
|
||||
|
||||
it('classifies a directory whose SKILL.md is a symlink as alias-file, not canonical', async () => {
|
||||
const { home } = await fixture()
|
||||
const canonicalDirectory = join(home, '.agents', 'skills', 'demo')
|
||||
const file = await writeSkill(canonicalDirectory, 'demo')
|
||||
const aliasDirectory = join(home, '.claude', 'skills', 'demo')
|
||||
await mkdir(aliasDirectory, { recursive: true })
|
||||
await symlink(file, join(aliasDirectory, 'SKILL.md'))
|
||||
|
||||
const resolved = await plan(home, await request(file))
|
||||
const alias = resolved.plan.skills[0].placements.find(
|
||||
(placement) => placement.path === aliasDirectory
|
||||
)
|
||||
expect(alias?.kind).toBe('alias-file')
|
||||
})
|
||||
|
||||
it('removes a home-root link to content outside every root, and only the link', async () => {
|
||||
const { home } = await fixture()
|
||||
const outside = join(home, 'elsewhere', 'demo')
|
||||
await writeSkill(outside, 'demo')
|
||||
await mkdir(join(home, '.claude', 'skills'), { recursive: true })
|
||||
await symlink(outside, join(home, '.claude', 'skills', 'demo'), 'dir')
|
||||
|
||||
const resolved = await plan(
|
||||
home,
|
||||
await request(join(home, '.claude', 'skills', 'demo', 'SKILL.md'))
|
||||
)
|
||||
expect(resolved.plan.skills[0].blocked).toBeUndefined()
|
||||
expect(resolved.plan.skills[0].placements).toEqual([
|
||||
expect.objectContaining({ path: join(home, '.claude', 'skills', 'demo'), kind: 'alias-dir' })
|
||||
])
|
||||
})
|
||||
|
||||
it('deletes the links of a repo skill whose content lives outside every root', async () => {
|
||||
// The shape that bit a real user: `<repo>/.agents/skills/<name>` is a link
|
||||
// to a tool-managed directory elsewhere on disk. The links are Orca's to
|
||||
// remove; the content behind them is not.
|
||||
const { home } = await fixture()
|
||||
const repo = join(home, 'projects', 'app')
|
||||
const managedElsewhere = join(home, '.local', 'share', 'devex', 'skills', 'demo')
|
||||
await writeSkill(managedElsewhere, 'demo')
|
||||
await mkdir(join(repo, '.agents', 'skills'), { recursive: true })
|
||||
await symlink(managedElsewhere, join(repo, '.agents', 'skills', 'demo'), 'dir')
|
||||
|
||||
const linked = join(repo, '.agents', 'skills', 'demo', 'SKILL.md')
|
||||
const resolved = await buildSkillDeletePlan({
|
||||
request: await request(linked),
|
||||
target: { kind: 'native-host', cwd: undefined },
|
||||
repos: [{ id: 'repo-1', name: 'app', path: repo } as never],
|
||||
filesystem: nativeSkillInstallFilesystem,
|
||||
homeDir: home
|
||||
})
|
||||
const entry = resolved.plan.skills[0]
|
||||
expect(entry.blocked).toBeUndefined()
|
||||
expect(entry.placements).toEqual([
|
||||
expect.objectContaining({ path: join(repo, '.agents', 'skills', 'demo'), kind: 'alias-dir' })
|
||||
])
|
||||
// Nothing outside the roots is ever offered for removal.
|
||||
expect(entry.placements.some((placement) => placement.path.includes('devex'))).toBe(false)
|
||||
})
|
||||
|
||||
it('still refuses when the content is outside and no link sits in a root', async () => {
|
||||
const { home } = await fixture()
|
||||
const outside = join(home, 'elsewhere', 'demo')
|
||||
const file = await writeSkill(outside, 'demo')
|
||||
const resolved = await plan(home, await request(file))
|
||||
expect(resolved.plan.skills[0].blocked).toBe('unowned')
|
||||
expect(resolved.plan.skills[0].placements).toEqual([])
|
||||
})
|
||||
|
||||
it('refuses a bundled skill even when the row claimed a home source', async () => {
|
||||
const { home } = await fixture()
|
||||
const bundled = join(home, '.claude', 'skills', '.system', 'demo')
|
||||
const file = await writeSkill(bundled, 'demo')
|
||||
// Symlinked into a plain home root, which is what makes the row read `home`.
|
||||
await mkdir(join(home, '.agents', 'skills'), { recursive: true })
|
||||
await symlink(bundled, join(home, '.agents', 'skills', 'demo'), 'dir')
|
||||
|
||||
const resolved = await plan(home, await request(file))
|
||||
expect(resolved.plan.skills[0].blocked).toBe('bundled')
|
||||
})
|
||||
|
||||
it('refuses a plugin-cache skill symlinked into a plain home root', async () => {
|
||||
const { home } = await fixture()
|
||||
const cached = join(home, '.codex', 'plugins', 'cache', 'pack', 'demo')
|
||||
const file = await writeSkill(cached, 'demo')
|
||||
await mkdir(join(home, '.agents', 'skills'), { recursive: true })
|
||||
await symlink(cached, join(home, '.agents', 'skills', 'demo'), 'dir')
|
||||
|
||||
const resolved = await plan(home, await request(file))
|
||||
expect(resolved.plan.skills[0].blocked).toBe('plugin')
|
||||
})
|
||||
|
||||
it('lists an overlapping root\u2019s directory once, not twice', async () => {
|
||||
const { home } = await fixture()
|
||||
// `~/.claude/skills` is both a home root and a repo root when the home
|
||||
// directory is also the workspace; staging that path twice would fail the
|
||||
// second rename and roll the whole skill back.
|
||||
const canonicalDirectory = join(home, '.claude', 'skills', 'demo')
|
||||
const file = await writeSkill(canonicalDirectory, 'demo')
|
||||
const resolved = await buildSkillDeletePlan({
|
||||
request: await request(file),
|
||||
target: { kind: 'native-host', cwd: home },
|
||||
repos: [],
|
||||
filesystem: nativeSkillInstallFilesystem,
|
||||
homeDir: home
|
||||
})
|
||||
expect(
|
||||
resolved.plan.skills[0].placements.filter(
|
||||
(placement) => placement.path === canonicalDirectory
|
||||
)
|
||||
).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('never treats a root itself as a placement', async () => {
|
||||
const { home } = await fixture()
|
||||
const root = join(home, '.agents', 'skills')
|
||||
// A SKILL.md directly in the root would otherwise look like a placement.
|
||||
const file = await writeSkill(root, 'demo')
|
||||
const resolved = await plan(home, await request(file))
|
||||
expect(resolved.plan.skills[0].placements).toEqual([])
|
||||
expect(resolved.plan.skills[0].blocked).toBe('unowned')
|
||||
})
|
||||
})
|
||||
|
||||
describe('buildSkillDeletePlan freshness guard', () => {
|
||||
it('reports stale when the file changed beyond the tolerance', async () => {
|
||||
const { home } = await fixture()
|
||||
const file = await writeSkill(join(home, '.agents', 'skills', 'demo'), 'demo')
|
||||
const displayed = (await stat(file)).mtimeMs
|
||||
const shifted = new Date(displayed + 60_000)
|
||||
await utimes(file, shifted, shifted)
|
||||
|
||||
const resolved = await plan(home, await request(file, { updatedAt: displayed }))
|
||||
expect(resolved.plan.skills[0].blocked).toBe('stale')
|
||||
})
|
||||
|
||||
it('fails closed on a null updatedAt rather than skipping the check', async () => {
|
||||
const { home } = await fixture()
|
||||
const file = await writeSkill(join(home, '.agents', 'skills', 'demo'), 'demo')
|
||||
const resolved = await plan(home, await request(file, { updatedAt: null }))
|
||||
expect(resolved.plan.skills[0].blocked).toBe('stale')
|
||||
})
|
||||
|
||||
it('accepts a second-granularity updatedAt, as WSL discovery reports it', async () => {
|
||||
const { home } = await fixture()
|
||||
const file = await writeSkill(join(home, '.agents', 'skills', 'demo'), 'demo')
|
||||
const seconds = Math.floor((await stat(file)).mtimeMs / 1000) * 1000
|
||||
const resolved = await plan(home, await request(file, { updatedAt: seconds }))
|
||||
expect(resolved.plan.skills[0].blocked).toBeUndefined()
|
||||
})
|
||||
|
||||
it('reports missing when the skill file is gone', async () => {
|
||||
const { home } = await fixture()
|
||||
const directory = join(home, '.agents', 'skills', 'demo')
|
||||
const file = await writeSkill(directory, 'demo')
|
||||
const deleteRequest = await request(file)
|
||||
await rm(file)
|
||||
const resolved = await plan(home, deleteRequest)
|
||||
expect(resolved.plan.skills[0].blocked).toBe('missing')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,211 @@
|
||||
import { posix as pathPosix, win32 as pathWin32 } from 'node:path'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import type {
|
||||
SkillDeleteBlockReason,
|
||||
SkillDeletePlan,
|
||||
SkillDeletePlanEntry,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteTargetSkill
|
||||
} from '../../shared/skill-delete-contract'
|
||||
import { normalizedSkillPath, type SkillPathSemantics } from '../../shared/skill-path-containment'
|
||||
import {
|
||||
enumerateSkillPlacementCandidates,
|
||||
requireEnumerableFilesystem
|
||||
} from './skill-delete-enumeration'
|
||||
import {
|
||||
blockedCanonicalReason,
|
||||
classifySkillPlacement,
|
||||
isSkillDeleteFresh,
|
||||
owningSkillRoot,
|
||||
type ClassifiedPlacement,
|
||||
type SkillDeleteGuardContext
|
||||
} from './skill-delete-guards'
|
||||
import { buildSkillDeleteRootSet } from './skill-delete-roots'
|
||||
import type { ResolvedSkillDiscoveryTarget } from './skill-discovery-target'
|
||||
import type { SkillInstallFilesystem } from './skill-install-filesystem'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
import type { SkillScanRoot } from './skill-discovery-sources'
|
||||
|
||||
/** Overlapping roots can reach one directory twice. Staging the same path twice
|
||||
* would fail the second rename and roll the whole skill back. */
|
||||
function dedupePlacements(
|
||||
placements: readonly ClassifiedPlacement[],
|
||||
semantics: SkillPathSemantics
|
||||
): ClassifiedPlacement[] {
|
||||
const seen = new Set<string>()
|
||||
return placements.filter((placement) => {
|
||||
const key = normalizedSkillPath(placement.path, semantics)
|
||||
if (seen.has(key)) {
|
||||
return false
|
||||
}
|
||||
seen.add(key)
|
||||
return true
|
||||
})
|
||||
}
|
||||
|
||||
function blockedEntry(
|
||||
skill: SkillDeleteTargetSkill,
|
||||
canonicalPath: string,
|
||||
blocked: SkillDeleteBlockReason
|
||||
): SkillDeletePlanEntry {
|
||||
return { id: skill.id, name: skill.name, canonicalPath, placements: [], blocked }
|
||||
}
|
||||
|
||||
export type SkillDeletePlanInput = {
|
||||
request: SkillDeleteRequest
|
||||
target: ResolvedSkillDiscoveryTarget
|
||||
repos: readonly Repo[]
|
||||
providerRootOverrides?: SkillProviderRootOverrides
|
||||
filesystem: SkillInstallFilesystem
|
||||
homeDir?: string
|
||||
}
|
||||
|
||||
/** The plan plus what execution needs but the wire contract must not carry. */
|
||||
export type ResolvedSkillDeletePlan = {
|
||||
plan: SkillDeletePlan
|
||||
semantics: SkillPathSemantics
|
||||
toFilesystemPath: (path: string) => string
|
||||
/** Root paths the filesystem must be authorized for before any operation. */
|
||||
rootPaths: string[]
|
||||
placementRoots: Map<string, ClassifiedPlacement[]>
|
||||
}
|
||||
|
||||
/**
|
||||
* One enumeration serves the whole batch, and `previewDelete` and `delete` both
|
||||
* come through here. There is deliberately no second enumeration path: a
|
||||
* preview promising one set of removals and a delete performing another is the
|
||||
* failure this design exists to avoid.
|
||||
*/
|
||||
export async function buildSkillDeletePlan(
|
||||
input: SkillDeletePlanInput
|
||||
): Promise<ResolvedSkillDeletePlan> {
|
||||
const { roots, semantics, toFilesystemPath } = await buildSkillDeleteRootSet({
|
||||
target: input.target,
|
||||
repos: input.repos,
|
||||
...(input.providerRootOverrides ? { providerRootOverrides: input.providerRootOverrides } : {}),
|
||||
...(input.homeDir ? { homeDir: input.homeDir } : {})
|
||||
})
|
||||
const rootPaths = roots.map((root) => root.path)
|
||||
// Widen the guest allow-list before anything touches it: the WSL filesystem's
|
||||
// constructor list covers installable providers only, a strict subset of the
|
||||
// discovery roots, so nearly every placement would throw before a safety rule
|
||||
// ever ran. This is a second, independent containment check with its own
|
||||
// semantics; it does not replace the shared path guard.
|
||||
input.filesystem.authorizeRoots?.(rootPaths.map(toFilesystemPath))
|
||||
|
||||
const api = semantics.sep === '\\' ? pathWin32 : pathPosix
|
||||
// Placements are literal paths inside literal roots; the canonical path is a
|
||||
// realpath. Comparing the two spellings makes every skill under a symlinked
|
||||
// component (a symlinked home, macOS `/tmp` -> `/private/tmp`) read `unowned`,
|
||||
// so canonical classification gets roots resolved the same way it is.
|
||||
const context: SkillDeleteGuardContext = { semantics, roots }
|
||||
const canonicalContext: SkillDeleteGuardContext = {
|
||||
semantics,
|
||||
roots: await resolvedRoots(roots, input.filesystem, toFilesystemPath)
|
||||
}
|
||||
const requested = input.request.skills
|
||||
// Inspect only paths a root already owns. A path outside every root is
|
||||
// `unowned` by definition, and asking the guest filesystem about it would
|
||||
// throw its own containment error before any of this reported a reason.
|
||||
const inspectable = requested.filter(
|
||||
(skill) => owningSkillRoot(api.dirname(skill.skillFilePath), context) !== null
|
||||
)
|
||||
const inspections = await inspectRequestedSkillFiles(
|
||||
inspectable,
|
||||
input.filesystem,
|
||||
toFilesystemPath
|
||||
)
|
||||
const candidates = await enumerateSkillPlacementCandidates({
|
||||
roots,
|
||||
filesystem: input.filesystem,
|
||||
semantics,
|
||||
toFilesystemPath
|
||||
})
|
||||
|
||||
const placementRoots = new Map<string, ClassifiedPlacement[]>()
|
||||
const skills = requested.map((skill): SkillDeletePlanEntry => {
|
||||
const inspection = inspections.get(skill.skillFilePath)
|
||||
if (!inspection) {
|
||||
return blockedEntry(skill, skill.skillFilePath, 'unowned')
|
||||
}
|
||||
const canonicalPath = inspection.realpath
|
||||
if (!canonicalPath || inspection.kind === 'missing') {
|
||||
return blockedEntry(skill, skill.skillFilePath, 'missing')
|
||||
}
|
||||
if (!isSkillDeleteFresh(skill.updatedAt, inspection.mtimeMs)) {
|
||||
return blockedEntry(skill, canonicalPath, 'stale')
|
||||
}
|
||||
const blocked = blockedCanonicalReason(api.dirname(canonicalPath), canonicalContext)
|
||||
if (blocked) {
|
||||
return blockedEntry(skill, canonicalPath, blocked)
|
||||
}
|
||||
const placements = dedupePlacements(
|
||||
candidates
|
||||
.map((candidate) => classifySkillPlacement(candidate, canonicalPath, context))
|
||||
.filter((placement): placement is ClassifiedPlacement => placement !== null),
|
||||
semantics
|
||||
)
|
||||
if (placements.length === 0) {
|
||||
return blockedEntry(skill, canonicalPath, 'unowned')
|
||||
}
|
||||
placementRoots.set(skill.id, placements)
|
||||
return {
|
||||
id: skill.id,
|
||||
name: skill.name,
|
||||
canonicalPath,
|
||||
placements: placements.map(({ path, kind, rootLabel }) => ({ path, kind, rootLabel }))
|
||||
}
|
||||
})
|
||||
|
||||
return {
|
||||
plan: { operationId: input.request.operationId, skills },
|
||||
semantics,
|
||||
toFilesystemPath,
|
||||
rootPaths,
|
||||
placementRoots
|
||||
}
|
||||
}
|
||||
|
||||
async function resolvedRoots(
|
||||
roots: readonly SkillScanRoot[],
|
||||
filesystem: SkillInstallFilesystem,
|
||||
toFilesystemPath: (path: string) => string
|
||||
): Promise<SkillScanRoot[]> {
|
||||
const inspections = await filesystem
|
||||
.inspectPaths?.(roots.map((root) => toFilesystemPath(root.path)))
|
||||
.catch(() => null)
|
||||
return roots.map((root) => {
|
||||
const realpath = inspections?.get(toFilesystemPath(root.path))?.realpath
|
||||
return realpath
|
||||
? { ...root, path: fromFilesystemPath(realpath, root.path, toFilesystemPath) }
|
||||
: root
|
||||
})
|
||||
}
|
||||
|
||||
/** Guest realpaths already come back in host-owned spelling; native ones are
|
||||
* the same string space. Falls back to the literal root if they ever diverge. */
|
||||
function fromFilesystemPath(
|
||||
realpath: string,
|
||||
literalRoot: string,
|
||||
toFilesystemPath: (path: string) => string
|
||||
): string {
|
||||
return toFilesystemPath(realpath) === realpath || toFilesystemPath(literalRoot) !== literalRoot
|
||||
? realpath
|
||||
: literalRoot
|
||||
}
|
||||
|
||||
async function inspectRequestedSkillFiles(
|
||||
skills: readonly SkillDeleteTargetSkill[],
|
||||
filesystem: SkillInstallFilesystem,
|
||||
toFilesystemPath: (path: string) => string
|
||||
): ReturnType<NonNullable<SkillInstallFilesystem['inspectPaths']>> {
|
||||
const { inspectPaths } = requireEnumerableFilesystem(filesystem)
|
||||
const paths = [...new Set(skills.map((skill) => skill.skillFilePath))]
|
||||
const raw = await inspectPaths(paths.map(toFilesystemPath))
|
||||
return new Map(
|
||||
paths.flatMap((path) => {
|
||||
const inspection = raw.get(toFilesystemPath(path))
|
||||
return inspection ? [[path, inspection] as const] : []
|
||||
})
|
||||
)
|
||||
}
|
||||
@@ -0,0 +1,191 @@
|
||||
import { lstat, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import {
|
||||
readSkillDeleteRecoveryJournal,
|
||||
recoverSkillDeleteTransaction,
|
||||
skillDeleteJournalPath,
|
||||
writeSkillDeleteJournal,
|
||||
type SkillDeleteJournalV1
|
||||
} from './skill-delete-recovery'
|
||||
import { writeSkillStateFile } from './skill-install-provenance'
|
||||
import { skillDeleteStagedName } from './skill-staging-names'
|
||||
|
||||
const roots: string[] = []
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })))
|
||||
})
|
||||
|
||||
async function fixture(): Promise<{ home: string; stateDirectory: string; root: string }> {
|
||||
const home = await mkdtemp(join(tmpdir(), 'orca-skill-delete-recovery-'))
|
||||
roots.push(home)
|
||||
const root = join(home, '.agents', 'skills')
|
||||
await mkdir(root, { recursive: true })
|
||||
return { home, stateDirectory: join(home, 'state'), root }
|
||||
}
|
||||
|
||||
function journal(
|
||||
root: string,
|
||||
canonicalPath: string,
|
||||
overrides: Partial<SkillDeleteJournalV1> = {}
|
||||
): SkillDeleteJournalV1 {
|
||||
return {
|
||||
schemaVersion: 1,
|
||||
operation: 'delete',
|
||||
phase: 'staged',
|
||||
canonicalPath,
|
||||
wslDistro: null,
|
||||
allowedRoots: [root],
|
||||
movedCount: 1,
|
||||
moves: [
|
||||
{
|
||||
sourcePath: canonicalPath,
|
||||
stagedPath: join(root, skillDeleteStagedName('demo', 'id-1')),
|
||||
kind: 'canonical'
|
||||
}
|
||||
],
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
async function exists(path: string): Promise<boolean> {
|
||||
return Boolean(await lstat(path).catch(() => null))
|
||||
}
|
||||
|
||||
describe('skill delete journal validation', () => {
|
||||
it('round-trips a valid journal', async () => {
|
||||
const { stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
await writeSkillDeleteJournal(stateDirectory, journal(root, canonicalPath))
|
||||
expect(await readSkillDeleteRecoveryJournal(stateDirectory, canonicalPath)).toMatchObject({
|
||||
operation: 'delete',
|
||||
phase: 'staged'
|
||||
})
|
||||
})
|
||||
|
||||
it('returns null when there is no journal', async () => {
|
||||
const { stateDirectory, root } = await fixture()
|
||||
expect(await readSkillDeleteRecoveryJournal(stateDirectory, join(root, 'demo'))).toBeNull()
|
||||
})
|
||||
|
||||
it('refuses a move whose source escapes every allowed root', async () => {
|
||||
const { home, stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
const escaped = join(home, 'elsewhere', 'demo')
|
||||
await writeSkillStateFile(
|
||||
skillDeleteJournalPath(stateDirectory, canonicalPath),
|
||||
journal(root, canonicalPath, {
|
||||
moves: [
|
||||
{
|
||||
sourcePath: escaped,
|
||||
stagedPath: join(home, 'elsewhere', skillDeleteStagedName('demo', 'id-1')),
|
||||
kind: 'canonical'
|
||||
}
|
||||
]
|
||||
})
|
||||
)
|
||||
await expect(readSkillDeleteRecoveryJournal(stateDirectory, canonicalPath)).rejects.toThrow(
|
||||
'skill-delete-journal-invalid'
|
||||
)
|
||||
})
|
||||
|
||||
it('refuses a staged path that is not our sibling naming convention', async () => {
|
||||
const { stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
await writeSkillStateFile(
|
||||
skillDeleteJournalPath(stateDirectory, canonicalPath),
|
||||
journal(root, canonicalPath, {
|
||||
moves: [
|
||||
{ sourcePath: canonicalPath, stagedPath: join(root, 'demo.bak'), kind: 'canonical' }
|
||||
]
|
||||
})
|
||||
)
|
||||
await expect(readSkillDeleteRecoveryJournal(stateDirectory, canonicalPath)).rejects.toThrow(
|
||||
'skill-delete-journal-invalid'
|
||||
)
|
||||
})
|
||||
|
||||
it('refuses a movedCount larger than the move list', async () => {
|
||||
const { stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
await writeSkillStateFile(
|
||||
skillDeleteJournalPath(stateDirectory, canonicalPath),
|
||||
journal(root, canonicalPath, { movedCount: 4 })
|
||||
)
|
||||
await expect(readSkillDeleteRecoveryJournal(stateDirectory, canonicalPath)).rejects.toThrow(
|
||||
'skill-delete-journal-invalid'
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('recoverSkillDeleteTransaction', () => {
|
||||
it('rolls a staged phase forward by removing the staged path', async () => {
|
||||
const { stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
const entry = journal(root, canonicalPath)
|
||||
await mkdir(entry.moves[0].stagedPath, { recursive: true })
|
||||
await writeSkillDeleteJournal(stateDirectory, entry)
|
||||
|
||||
await recoverSkillDeleteTransaction(stateDirectory, canonicalPath)
|
||||
expect(await exists(entry.moves[0].stagedPath)).toBe(false)
|
||||
expect(await exists(skillDeleteJournalPath(stateDirectory, canonicalPath))).toBe(false)
|
||||
})
|
||||
|
||||
it('rolls an interrupted staging back so the skill survives whole', async () => {
|
||||
const { stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
const entry = journal(root, canonicalPath, { phase: 'staging' })
|
||||
await mkdir(entry.moves[0].stagedPath, { recursive: true })
|
||||
await writeFile(join(entry.moves[0].stagedPath, 'SKILL.md'), 'content')
|
||||
await writeSkillDeleteJournal(stateDirectory, entry)
|
||||
|
||||
await recoverSkillDeleteTransaction(stateDirectory, canonicalPath)
|
||||
expect(await exists(canonicalPath)).toBe(true)
|
||||
expect(await exists(entry.moves[0].stagedPath)).toBe(false)
|
||||
expect(await exists(skillDeleteJournalPath(stateDirectory, canonicalPath))).toBe(false)
|
||||
})
|
||||
|
||||
it('restores canonical before alias-file, so no alias dangles mid-recovery', async () => {
|
||||
const { home, stateDirectory, root } = await fixture()
|
||||
const canonicalPath = join(root, 'demo')
|
||||
const aliasRoot = join(home, '.claude', 'skills', 'demo')
|
||||
await mkdir(aliasRoot, { recursive: true })
|
||||
const entry = journal(root, canonicalPath, {
|
||||
phase: 'staging',
|
||||
movedCount: 2,
|
||||
allowedRoots: [root, join(home, '.claude', 'skills')],
|
||||
// Staging order is alias-file first, then canonical, so plain reversal
|
||||
// gives canonical-first restore.
|
||||
moves: [
|
||||
{
|
||||
sourcePath: join(aliasRoot, 'SKILL.md'),
|
||||
stagedPath: join(aliasRoot, skillDeleteStagedName('SKILL.md', 'id-2')),
|
||||
kind: 'alias-file'
|
||||
},
|
||||
{
|
||||
sourcePath: canonicalPath,
|
||||
stagedPath: join(root, skillDeleteStagedName('demo', 'id-1')),
|
||||
kind: 'canonical'
|
||||
}
|
||||
]
|
||||
})
|
||||
await mkdir(entry.moves[1].stagedPath, { recursive: true })
|
||||
await writeFile(entry.moves[0].stagedPath, 'link-stand-in')
|
||||
await writeSkillDeleteJournal(stateDirectory, entry)
|
||||
|
||||
const order: string[] = []
|
||||
await recoverSkillDeleteTransaction(stateDirectory, canonicalPath, {
|
||||
prepareExtractedSkill: async () => undefined,
|
||||
observeSkill: async () => {
|
||||
throw new Error('unused')
|
||||
},
|
||||
rename: async (_source, target) => {
|
||||
order.push(target)
|
||||
},
|
||||
remove: async () => undefined
|
||||
})
|
||||
expect(order).toEqual([canonicalPath, join(aliasRoot, 'SKILL.md')])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,167 @@
|
||||
import { rm } from 'node:fs/promises'
|
||||
import { join, posix as pathPosix, win32 as pathWin32 } from 'node:path'
|
||||
import { readNodeFileWithinLimit } from '../../shared/node-bounded-file-reader'
|
||||
import {
|
||||
SKILL_DELETE_PLACEMENT_KINDS,
|
||||
type SkillDeletePlacementKind
|
||||
} from '../../shared/skill-delete-contract'
|
||||
import { nativeSkillPathSemantics, skillPathInside } from '../../shared/skill-path-containment'
|
||||
import {
|
||||
nativeSkillInstallFilesystem,
|
||||
type SkillInstallFilesystem
|
||||
} from './skill-install-filesystem'
|
||||
import { skillInstallStateKey, writeSkillStateFile } from './skill-install-provenance'
|
||||
import { isSkillDeleteStagedName } from './skill-staging-names'
|
||||
|
||||
const JOURNAL_MAX_BYTES = 4 * 1024 * 1024
|
||||
|
||||
export type SkillDeleteJournalMove = {
|
||||
sourcePath: string
|
||||
stagedPath: string
|
||||
kind: SkillDeletePlacementKind
|
||||
}
|
||||
|
||||
/**
|
||||
* A path-based delete has no install receipt — that absence is the reason this
|
||||
* feature exists — so replay is validated by containment and placement shape
|
||||
* instead of a receipt digest.
|
||||
*
|
||||
* `wslDistro` and `allowedRoots` are not extras: at startup there is no target,
|
||||
* no workspace, and no repo list to resolve either from, so a journal that
|
||||
* omits them cannot be replayed on WSL and cannot be validated at all.
|
||||
*/
|
||||
export type SkillDeleteJournalV1 = {
|
||||
schemaVersion: 1
|
||||
operation: 'delete'
|
||||
phase: 'prepared' | 'staging' | 'staged'
|
||||
/**
|
||||
* The `canonical` placement's own literal, pre-realpath directory path — the
|
||||
* same value the lock key is derived from, and the name the shared journal
|
||||
* scanner cross-checks against the file name.
|
||||
*/
|
||||
canonicalPath: string
|
||||
/** Set when the files live in a WSL distro, so startup recovery can rebuild
|
||||
* the guest filesystem with no target, workspace, or repo list to resolve
|
||||
* from. Every path above is already in the spelling that filesystem takes. */
|
||||
wslDistro: string | null
|
||||
allowedRoots: string[]
|
||||
movedCount: number
|
||||
moves: SkillDeleteJournalMove[]
|
||||
}
|
||||
|
||||
export function skillDeleteJournalPath(stateDirectory: string, canonicalPath: string): string {
|
||||
return join(stateDirectory, 'delete-journals', `${skillInstallStateKey(canonicalPath)}.json`)
|
||||
}
|
||||
|
||||
function validMove(move: unknown, journal: SkillDeleteJournalV1): move is SkillDeleteJournalMove {
|
||||
if (typeof move !== 'object' || move === null) {
|
||||
return false
|
||||
}
|
||||
const candidate: Partial<SkillDeleteJournalMove> = move
|
||||
if (
|
||||
typeof candidate.sourcePath !== 'string' ||
|
||||
typeof candidate.stagedPath !== 'string' ||
|
||||
!SKILL_DELETE_PLACEMENT_KINDS.includes(candidate.kind as SkillDeletePlacementKind)
|
||||
) {
|
||||
return false
|
||||
}
|
||||
const semantics = nativeSkillPathSemantics()
|
||||
const api = semantics.sep === '\\' ? pathWin32 : pathPosix
|
||||
// A sibling rename by construction: same parent, hidden name, our marker.
|
||||
return (
|
||||
api.dirname(candidate.sourcePath) === api.dirname(candidate.stagedPath) &&
|
||||
isSkillDeleteStagedName(
|
||||
api.basename(candidate.sourcePath),
|
||||
api.basename(candidate.stagedPath)
|
||||
) &&
|
||||
journal.allowedRoots.some((root) => skillPathInside(root, candidate.sourcePath!, semantics))
|
||||
)
|
||||
}
|
||||
|
||||
function isDeleteJournal(value: unknown, canonicalPath: string): value is SkillDeleteJournalV1 {
|
||||
if (typeof value !== 'object' || value === null) {
|
||||
return false
|
||||
}
|
||||
const journal: Partial<SkillDeleteJournalV1> = value
|
||||
if (
|
||||
journal.schemaVersion !== 1 ||
|
||||
journal.operation !== 'delete' ||
|
||||
journal.canonicalPath !== canonicalPath ||
|
||||
(journal.phase !== 'prepared' && journal.phase !== 'staging' && journal.phase !== 'staged') ||
|
||||
(typeof journal.wslDistro !== 'string' && journal.wslDistro !== null) ||
|
||||
!Array.isArray(journal.allowedRoots) ||
|
||||
!journal.allowedRoots.every((root) => typeof root === 'string') ||
|
||||
!Array.isArray(journal.moves) ||
|
||||
!Number.isInteger(journal.movedCount)
|
||||
) {
|
||||
return false
|
||||
}
|
||||
const candidate = journal as SkillDeleteJournalV1
|
||||
if (candidate.movedCount < 0 || candidate.movedCount > candidate.moves.length) {
|
||||
return false
|
||||
}
|
||||
const sources = new Set(candidate.moves.map((move: SkillDeleteJournalMove) => move?.sourcePath))
|
||||
return (
|
||||
sources.size === candidate.moves.length &&
|
||||
candidate.moves.every((move) => validMove(move, candidate))
|
||||
)
|
||||
}
|
||||
|
||||
export async function writeSkillDeleteJournal(
|
||||
stateDirectory: string,
|
||||
journal: SkillDeleteJournalV1
|
||||
): Promise<void> {
|
||||
await writeSkillStateFile(skillDeleteJournalPath(stateDirectory, journal.canonicalPath), journal)
|
||||
}
|
||||
|
||||
export async function readSkillDeleteRecoveryJournal(
|
||||
stateDirectory: string,
|
||||
canonicalPath: string
|
||||
): Promise<SkillDeleteJournalV1 | null> {
|
||||
try {
|
||||
const parsed: unknown = JSON.parse(
|
||||
(
|
||||
await readNodeFileWithinLimit(
|
||||
skillDeleteJournalPath(stateDirectory, canonicalPath),
|
||||
JOURNAL_MAX_BYTES
|
||||
)
|
||||
).buffer.toString('utf8')
|
||||
)
|
||||
if (!isDeleteJournal(parsed, canonicalPath)) {
|
||||
throw new Error('skill-delete-journal-invalid')
|
||||
}
|
||||
return parsed
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
|
||||
return null
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
export async function recoverSkillDeleteTransaction(
|
||||
stateDirectory: string,
|
||||
canonicalPath: string,
|
||||
filesystem: SkillInstallFilesystem = nativeSkillInstallFilesystem
|
||||
): Promise<void> {
|
||||
const journal = await readSkillDeleteRecoveryJournal(stateDirectory, canonicalPath)
|
||||
if (!journal) {
|
||||
return
|
||||
}
|
||||
const staged = journal.moves.slice(0, journal.movedCount)
|
||||
if (journal.phase === 'staged') {
|
||||
// Roll forward: the skill is already gone from every discovered location.
|
||||
for (const move of staged) {
|
||||
await filesystem.remove(move.stagedPath)
|
||||
}
|
||||
} else {
|
||||
// Roll back in reverse of staging order, so a restored alias never points at
|
||||
// a canonical directory that does not exist yet. Indexed rather than
|
||||
// `toReversed()`: this module reaches the Node 18 relay bundle.
|
||||
for (let index = staged.length - 1; index >= 0; index -= 1) {
|
||||
const move = staged[index]
|
||||
await filesystem.rename(move.stagedPath, move.sourcePath).catch(() => undefined)
|
||||
}
|
||||
}
|
||||
await rm(skillDeleteJournalPath(stateDirectory, canonicalPath), { force: true })
|
||||
}
|
||||
@@ -0,0 +1,66 @@
|
||||
import { join } from 'node:path'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import type {
|
||||
SkillDeletePlan,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteResult
|
||||
} from '../../shared/skill-delete-contract'
|
||||
import {
|
||||
deleteSkills,
|
||||
previewSkillDeletion,
|
||||
type SkillDeleteServiceInput
|
||||
} from './skill-delete-service'
|
||||
import type { ResolvedSkillDiscoveryTarget } from './skill-discovery-target'
|
||||
import { nativeSkillInstallFilesystem } from './skill-install-filesystem'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
import { WslSkillInstallFilesystem } from './skill-wsl-install-filesystem'
|
||||
|
||||
/** Assembly shared by the local IPC channel and the runtime RPC method, so both
|
||||
* transports resolve the same host, roots, and filesystem. */
|
||||
export type SkillDeleteRequestDependencies = {
|
||||
repos: () => readonly Repo[]
|
||||
resolveProviderRootOverrides?: (
|
||||
target: ResolvedSkillDiscoveryTarget
|
||||
) => Promise<SkillProviderRootOverrides | undefined>
|
||||
/** `app.getPath('userData')`; the skill-install state root hangs off it. */
|
||||
userDataPath: string
|
||||
}
|
||||
|
||||
async function serviceInput(
|
||||
request: SkillDeleteRequest,
|
||||
target: ResolvedSkillDiscoveryTarget,
|
||||
dependencies: SkillDeleteRequestDependencies
|
||||
): Promise<SkillDeleteServiceInput> {
|
||||
const providerRootOverrides = await dependencies.resolveProviderRootOverrides?.(target)
|
||||
return {
|
||||
request,
|
||||
target,
|
||||
repos: dependencies.repos(),
|
||||
...(providerRootOverrides ? { providerRootOverrides } : {}),
|
||||
// The allow-list starts empty on purpose: the plan authorizes the rebuilt
|
||||
// discovery root set, which is wider than the installable-provider roots the
|
||||
// factory would seed.
|
||||
filesystem:
|
||||
target.kind === 'wsl'
|
||||
? new WslSkillInstallFilesystem(target.distro, [])
|
||||
: nativeSkillInstallFilesystem,
|
||||
stateDirectory: join(dependencies.userDataPath, 'skill-installs'),
|
||||
wslDistro: target.kind === 'wsl' ? target.distro : null
|
||||
}
|
||||
}
|
||||
|
||||
export async function previewSkillDeleteRequest(
|
||||
request: SkillDeleteRequest,
|
||||
target: ResolvedSkillDiscoveryTarget,
|
||||
dependencies: SkillDeleteRequestDependencies
|
||||
): Promise<SkillDeletePlan> {
|
||||
return previewSkillDeletion(await serviceInput(request, target, dependencies))
|
||||
}
|
||||
|
||||
export async function runSkillDeleteRequest(
|
||||
request: SkillDeleteRequest,
|
||||
target: ResolvedSkillDiscoveryTarget,
|
||||
dependencies: SkillDeleteRequestDependencies
|
||||
): Promise<SkillDeleteResult> {
|
||||
return deleteSkills(await serviceInput(request, target, dependencies))
|
||||
}
|
||||
@@ -0,0 +1,74 @@
|
||||
import { homedir } from 'node:os'
|
||||
import { posix as pathPosix } from 'node:path'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import {
|
||||
POSIX_SKILL_PATH_SEMANTICS,
|
||||
nativeSkillPathSemantics,
|
||||
type SkillPathSemantics
|
||||
} from '../../shared/skill-path-containment'
|
||||
import { toWindowsWslPath } from '../../shared/wsl-paths'
|
||||
import { discoverClaudePluginSkillSources } from './claude-plugin-skill-sources'
|
||||
import { discoverClaudePluginSkillSourcesInWsl } from './claude-plugin-skill-sources-wsl'
|
||||
import { buildSkillDiscoverySources, type SkillScanRoot } from './skill-discovery-sources'
|
||||
import type { ResolvedSkillDiscoveryTarget } from './skill-discovery-target'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
|
||||
/**
|
||||
* The delete host owns everything about where skills may live, so it rebuilds
|
||||
* the discovery root set from the same three inputs `discoverSkillsOnTarget`
|
||||
* uses — target, repo list, provider-root overrides. Rebuilding beats trusting
|
||||
* the client: the worst case is a refusal, never a delete outside a root.
|
||||
*/
|
||||
export type SkillDeleteRootSet = {
|
||||
roots: SkillScanRoot[]
|
||||
semantics: SkillPathSemantics
|
||||
/** Translates a host-owned path into the spelling the filesystem accepts. */
|
||||
toFilesystemPath: (path: string) => string
|
||||
}
|
||||
|
||||
export async function buildSkillDeleteRootSet(input: {
|
||||
target: ResolvedSkillDiscoveryTarget
|
||||
repos: readonly Repo[]
|
||||
providerRootOverrides?: SkillProviderRootOverrides
|
||||
homeDir?: string
|
||||
}): Promise<SkillDeleteRootSet> {
|
||||
if (input.target.kind === 'wsl') {
|
||||
const { distro, homeDir, cwd } = input.target
|
||||
return {
|
||||
roots: [
|
||||
...buildSkillDiscoverySources({
|
||||
homeDir,
|
||||
cwd,
|
||||
repos: [],
|
||||
pathApi: pathPosix,
|
||||
providerRootOverrides: input.providerRootOverrides
|
||||
}),
|
||||
// Why tolerated: plugin metadata is enrichment. A failed read must not
|
||||
// turn every skill on the host into `unowned`.
|
||||
...(await discoverClaudePluginSkillSourcesInWsl({ distro, homeDir, cwd }).catch(() => []))
|
||||
],
|
||||
// A WSL runtime is POSIX and case-sensitive even though the process is win32.
|
||||
semantics: POSIX_SKILL_PATH_SEMANTICS,
|
||||
toFilesystemPath: (path) => toWindowsWslPath(path, distro)
|
||||
}
|
||||
}
|
||||
|
||||
const home = input.homeDir ?? homedir()
|
||||
const cwd = input.target.cwd
|
||||
return {
|
||||
roots: [
|
||||
// Mirrors `discoverSkillsOnTarget`: a cwd-scoped scan drops the repo list,
|
||||
// and `buildSkillDiscoverySources` adds `cwd ?? process.cwd()` on top
|
||||
// either way, so `repos: []` is not the whole exclusivity rule by itself.
|
||||
...buildSkillDiscoverySources({
|
||||
homeDir: home,
|
||||
...(cwd ? { cwd } : {}),
|
||||
repos: cwd ? [] : [...input.repos],
|
||||
providerRootOverrides: input.providerRootOverrides
|
||||
}),
|
||||
...(cwd ? await discoverClaudePluginSkillSources({ homeDir: home, cwd }).catch(() => []) : [])
|
||||
],
|
||||
semantics: nativeSkillPathSemantics(),
|
||||
toFilesystemPath: (path) => path
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,261 @@
|
||||
import { lstat, mkdir, mkdtemp, readdir, rm, stat, symlink, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { SkillDeleteRequest } from '../../shared/skill-delete-contract'
|
||||
import { deleteSkills } from './skill-delete-service'
|
||||
import { acquireSkillInstallLock, skillInstallLockPath } from './skill-install-lock'
|
||||
import {
|
||||
nativeSkillInstallFilesystem,
|
||||
type SkillInstallFilesystem
|
||||
} from './skill-install-filesystem'
|
||||
import { readSkillInstallReceipt, writeSkillInstallReceipt } from './skill-install-provenance'
|
||||
import { skillDeleteJournalPath } from './skill-delete-recovery'
|
||||
|
||||
const roots: string[] = []
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })))
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
async function fixture(): Promise<{
|
||||
home: string
|
||||
stateDirectory: string
|
||||
canonicalDirectory: string
|
||||
file: string
|
||||
}> {
|
||||
const home = await mkdtemp(join(tmpdir(), 'orca-skill-delete-service-'))
|
||||
roots.push(home)
|
||||
const canonicalDirectory = join(home, '.agents', 'skills', 'demo')
|
||||
await mkdir(canonicalDirectory, { recursive: true })
|
||||
const file = join(canonicalDirectory, 'SKILL.md')
|
||||
await writeFile(file, '---\nname: demo\ndescription: Demo\n---\n\n# Demo\n')
|
||||
return { home, stateDirectory: join(home, 'state'), canonicalDirectory, file }
|
||||
}
|
||||
|
||||
async function request(file: string): Promise<SkillDeleteRequest> {
|
||||
return {
|
||||
operationId: 'op',
|
||||
skills: [
|
||||
{
|
||||
id: 'skill-id',
|
||||
directoryPath: join(file, '..'),
|
||||
skillFilePath: file,
|
||||
name: 'demo',
|
||||
updatedAt: (await stat(file)).mtimeMs
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
function run(
|
||||
home: string,
|
||||
stateDirectory: string,
|
||||
deleteRequest: SkillDeleteRequest,
|
||||
filesystem: SkillInstallFilesystem = nativeSkillInstallFilesystem
|
||||
) {
|
||||
return deleteSkills({
|
||||
request: deleteRequest,
|
||||
target: { kind: 'native-host', cwd: undefined },
|
||||
repos: [],
|
||||
filesystem,
|
||||
stateDirectory,
|
||||
homeDir: home,
|
||||
lockTimeoutMs: 50
|
||||
})
|
||||
}
|
||||
|
||||
async function exists(path: string): Promise<boolean> {
|
||||
return Boolean(await lstat(path).catch(() => null))
|
||||
}
|
||||
|
||||
describe('deleteSkills', () => {
|
||||
it('removes every placement and leaves no staged residue', async () => {
|
||||
const { home, stateDirectory, canonicalDirectory, file } = await fixture()
|
||||
const aliasDirectory = join(home, '.claude', 'skills', 'demo')
|
||||
await mkdir(join(home, '.claude', 'skills'), { recursive: true })
|
||||
await symlink(canonicalDirectory, aliasDirectory, 'dir')
|
||||
|
||||
const result = await run(home, stateDirectory, await request(file))
|
||||
expect(result.skills[0].status).toBe('deleted')
|
||||
expect(await exists(canonicalDirectory)).toBe(false)
|
||||
expect(await exists(aliasDirectory)).toBe(false)
|
||||
expect(await readdir(join(home, '.agents', 'skills'))).toEqual([])
|
||||
expect(await exists(skillDeleteJournalPath(stateDirectory, canonicalDirectory))).toBe(false)
|
||||
})
|
||||
|
||||
it('removes an alias-file symlink but keeps a directory that holds anything else', async () => {
|
||||
const { home, stateDirectory, file } = await fixture()
|
||||
const aliasDirectory = join(home, '.codex', 'skills', 'demo')
|
||||
await mkdir(aliasDirectory, { recursive: true })
|
||||
await symlink(file, join(aliasDirectory, 'SKILL.md'))
|
||||
await writeFile(join(aliasDirectory, 'notes.md'), 'kept')
|
||||
|
||||
const result = await run(home, stateDirectory, await request(file))
|
||||
expect(result.skills[0].status).toBe('deleted')
|
||||
expect(await exists(join(aliasDirectory, 'SKILL.md'))).toBe(false)
|
||||
// "Not shared with another placement" is not "contains nothing else".
|
||||
expect(await exists(join(aliasDirectory, 'notes.md'))).toBe(true)
|
||||
})
|
||||
|
||||
it('removes an emptied alias-file directory', async () => {
|
||||
const { home, stateDirectory, file } = await fixture()
|
||||
const aliasDirectory = join(home, '.codex', 'skills', 'demo')
|
||||
await mkdir(aliasDirectory, { recursive: true })
|
||||
await symlink(file, join(aliasDirectory, 'SKILL.md'))
|
||||
|
||||
await run(home, stateDirectory, await request(file))
|
||||
expect(await exists(aliasDirectory)).toBe(false)
|
||||
})
|
||||
|
||||
it('takes the lock an install of the same destination directory would take', async () => {
|
||||
const { home, stateDirectory, canonicalDirectory, file } = await fixture()
|
||||
// Keyed on the placement's literal directory path, which is what install
|
||||
// hashes — not the realpath'd file identity's dirname.
|
||||
const release = await acquireSkillInstallLock({
|
||||
path: skillInstallLockPath(stateDirectory, canonicalDirectory)
|
||||
})
|
||||
try {
|
||||
const result = await run(home, stateDirectory, await request(file))
|
||||
expect(result.skills[0].status).toBe('busy')
|
||||
expect(await exists(canonicalDirectory)).toBe(true)
|
||||
} finally {
|
||||
await release()
|
||||
}
|
||||
})
|
||||
|
||||
it('continues the batch when one skill is locked by another transaction', async () => {
|
||||
const { home, stateDirectory, canonicalDirectory, file } = await fixture()
|
||||
const otherDirectory = join(home, '.agents', 'skills', 'other')
|
||||
await mkdir(otherDirectory, { recursive: true })
|
||||
const otherFile = join(otherDirectory, 'SKILL.md')
|
||||
await writeFile(otherFile, '---\nname: other\ndescription: Other\n---\n')
|
||||
|
||||
const release = await acquireSkillInstallLock({
|
||||
path: skillInstallLockPath(stateDirectory, canonicalDirectory)
|
||||
})
|
||||
try {
|
||||
const [first, second] = await Promise.all([request(file), request(otherFile)])
|
||||
const result = await run(home, stateDirectory, {
|
||||
operationId: 'batch',
|
||||
skills: [first.skills[0], { ...second.skills[0], id: 'other-id', name: 'other' }]
|
||||
})
|
||||
expect(result.skills.map((skill) => skill.status)).toEqual(['busy', 'deleted'])
|
||||
expect(await exists(otherDirectory)).toBe(false)
|
||||
} finally {
|
||||
await release()
|
||||
}
|
||||
})
|
||||
|
||||
it('restores every staged placement when a later rename fails', async () => {
|
||||
const { home, stateDirectory, canonicalDirectory, file } = await fixture()
|
||||
const aliasDirectory = join(home, '.claude', 'skills', 'demo')
|
||||
await mkdir(join(home, '.claude', 'skills'), { recursive: true })
|
||||
await symlink(canonicalDirectory, aliasDirectory, 'dir')
|
||||
|
||||
let renames = 0
|
||||
const filesystem: SkillInstallFilesystem = {
|
||||
...nativeSkillInstallFilesystem,
|
||||
rename: async (source, target) => {
|
||||
renames += 1
|
||||
// Fail the canonical stage, which runs last.
|
||||
if (renames === 2) {
|
||||
throw new Error('EBUSY')
|
||||
}
|
||||
return nativeSkillInstallFilesystem.rename(source, target)
|
||||
}
|
||||
}
|
||||
|
||||
const result = await run(home, stateDirectory, await request(file), filesystem)
|
||||
expect(result.skills[0].status).toBe('failed')
|
||||
expect(await exists(canonicalDirectory)).toBe(true)
|
||||
expect(await exists(aliasDirectory)).toBe(true)
|
||||
})
|
||||
|
||||
it('reports partial with the staged paths when the rollback itself fails', async () => {
|
||||
const { home, stateDirectory, canonicalDirectory, file } = await fixture()
|
||||
await mkdir(join(home, '.claude', 'skills'), { recursive: true })
|
||||
await symlink(canonicalDirectory, join(home, '.claude', 'skills', 'demo'), 'dir')
|
||||
|
||||
let renames = 0
|
||||
const filesystem: SkillInstallFilesystem = {
|
||||
...nativeSkillInstallFilesystem,
|
||||
rename: async (source, target) => {
|
||||
renames += 1
|
||||
if (renames === 1) {
|
||||
return nativeSkillInstallFilesystem.rename(source, target)
|
||||
}
|
||||
throw new Error('EACCES')
|
||||
}
|
||||
}
|
||||
|
||||
const result = await run(home, stateDirectory, await request(file), filesystem)
|
||||
// Neither restored nor removed — reported as `partial`, never folded into
|
||||
// `failed`, which would wrongly imply nothing changed.
|
||||
expect(result.skills[0].status).toBe('partial')
|
||||
expect(result.skills[0].stagedPaths?.length).toBe(1)
|
||||
})
|
||||
|
||||
it('reports partial rather than deleted when a staged removal fails', async () => {
|
||||
const { home, stateDirectory, file } = await fixture()
|
||||
const filesystem: SkillInstallFilesystem = {
|
||||
...nativeSkillInstallFilesystem,
|
||||
remove: async () => {
|
||||
throw new Error('EACCES')
|
||||
}
|
||||
}
|
||||
const result = await run(home, stateDirectory, await request(file), filesystem)
|
||||
expect(result.skills[0].status).toBe('partial')
|
||||
expect(result.skills[0].stagedPaths?.length).toBe(1)
|
||||
})
|
||||
|
||||
it('deletes the install receipt so no phantom managed install survives', async () => {
|
||||
const { home, stateDirectory, canonicalDirectory, file } = await fixture()
|
||||
await writeSkillInstallReceipt(stateDirectory, {
|
||||
schemaVersion: 1,
|
||||
packageId: 'package_1',
|
||||
versionId: 'version_1',
|
||||
packageDigest: 'a'.repeat(64),
|
||||
archiveSha256: 'b'.repeat(64),
|
||||
scope: 'global',
|
||||
destinationIdentity: 'identity',
|
||||
canonicalPath: canonicalDirectory,
|
||||
placements: [],
|
||||
installedAt: new Date(0).toISOString(),
|
||||
hostIdentity: 'host'
|
||||
})
|
||||
await run(home, stateDirectory, await request(file))
|
||||
expect(await readSkillInstallReceipt(stateDirectory, canonicalDirectory)).toBeNull()
|
||||
})
|
||||
|
||||
it('removes a link to outside content and leaves the content itself intact', async () => {
|
||||
const { home, stateDirectory } = await fixture()
|
||||
const managedElsewhere = join(home, '.local', 'share', 'devex', 'skills', 'linked')
|
||||
await mkdir(managedElsewhere, { recursive: true })
|
||||
await writeFile(
|
||||
join(managedElsewhere, 'SKILL.md'),
|
||||
'---\nname: linked\ndescription: Linked\n---\n'
|
||||
)
|
||||
const link = join(home, '.claude', 'skills', 'linked')
|
||||
await mkdir(join(home, '.claude', 'skills'), { recursive: true })
|
||||
await symlink(managedElsewhere, link, 'dir')
|
||||
|
||||
const result = await run(home, stateDirectory, await request(join(link, 'SKILL.md')))
|
||||
expect(result.skills[0].status).toBe('deleted')
|
||||
expect(await exists(link)).toBe(false)
|
||||
// The whole point: Orca removed its own pointer, not the tool's content.
|
||||
expect(await exists(join(managedElsewhere, 'SKILL.md'))).toBe(true)
|
||||
})
|
||||
|
||||
it('skips a blocked skill without touching disk', async () => {
|
||||
const { home, stateDirectory, file } = await fixture()
|
||||
const deleteRequest = await request(file)
|
||||
const result = await run(home, stateDirectory, {
|
||||
...deleteRequest,
|
||||
skills: [{ ...deleteRequest.skills[0], updatedAt: 1 }]
|
||||
})
|
||||
expect(result.skills[0]).toMatchObject({ status: 'skipped', blocked: 'stale' })
|
||||
expect(await exists(file)).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,202 @@
|
||||
import { rm } from 'node:fs/promises'
|
||||
import { posix as pathPosix, win32 as pathWin32 } from 'node:path'
|
||||
import type { Repo } from '../../shared/repo-types'
|
||||
import type {
|
||||
SkillDeletePlan,
|
||||
SkillDeletePlanEntry,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteResult,
|
||||
SkillDeleteResultEntry
|
||||
} from '../../shared/skill-delete-contract'
|
||||
import { nativeSkillPathSemantics } from '../../shared/skill-path-containment'
|
||||
import { buildSkillDeletePlan, type ResolvedSkillDeletePlan } from './skill-delete-plan'
|
||||
import {
|
||||
recoverSkillDeleteTransaction,
|
||||
skillDeleteJournalPath,
|
||||
writeSkillDeleteJournal,
|
||||
type SkillDeleteJournalMove,
|
||||
type SkillDeleteJournalV1
|
||||
} from './skill-delete-recovery'
|
||||
import {
|
||||
planSkillDeleteMoves,
|
||||
removeStagedSkillDeleteMoves,
|
||||
stageSkillDeleteMoves
|
||||
} from './skill-delete-staging'
|
||||
import {
|
||||
clearSkillDiscoveryCaches,
|
||||
type ResolvedSkillDiscoveryTarget
|
||||
} from './skill-discovery-target'
|
||||
import { acquireSkillInstallLock, skillInstallLockPath } from './skill-install-lock'
|
||||
import { removeSkillInstallReceipt } from './skill-install-provenance'
|
||||
import type { SkillInstallFilesystem } from './skill-install-filesystem'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
import { recordSkillDeleteOperation } from './skill-operation-observability'
|
||||
|
||||
export type SkillDeleteServiceInput = {
|
||||
request: SkillDeleteRequest
|
||||
target: ResolvedSkillDiscoveryTarget
|
||||
repos: readonly Repo[]
|
||||
providerRootOverrides?: SkillProviderRootOverrides
|
||||
filesystem: SkillInstallFilesystem
|
||||
/** `<userData>/skill-installs` — the same state root install and remove use. */
|
||||
stateDirectory: string
|
||||
wslDistro?: string | null
|
||||
homeDir?: string
|
||||
/** Mirrors install's own knob; the shared 5s default applies when unset. */
|
||||
lockTimeoutMs?: number
|
||||
}
|
||||
|
||||
export async function previewSkillDeletion(
|
||||
input: SkillDeleteServiceInput
|
||||
): Promise<SkillDeletePlan> {
|
||||
return (await buildSkillDeletePlan(input)).plan
|
||||
}
|
||||
|
||||
/**
|
||||
* Recomputes the plan rather than trusting a client-supplied one, and executes
|
||||
* it per skill: one lock, one journal, stage → remove → receipt cleanup. A
|
||||
* failure is confined to its own skill; the rest of the batch continues.
|
||||
*/
|
||||
export async function deleteSkills(input: SkillDeleteServiceInput): Promise<SkillDeleteResult> {
|
||||
const resolved = await buildSkillDeletePlan(input)
|
||||
const skills: SkillDeleteResultEntry[] = []
|
||||
for (const entry of resolved.plan.skills) {
|
||||
skills.push(
|
||||
entry.blocked
|
||||
? {
|
||||
id: entry.id,
|
||||
name: entry.name,
|
||||
status: 'skipped',
|
||||
blocked: entry.blocked,
|
||||
removedPaths: []
|
||||
}
|
||||
: await deleteOneSkill(entry, resolved, input)
|
||||
)
|
||||
}
|
||||
// Once per batch, on the executing host: the target coalescer, the per-root
|
||||
// cache, and the last-known-good retention would otherwise serve a pre-delete
|
||||
// answer and the row visibly comes back.
|
||||
clearSkillDiscoveryCaches()
|
||||
recordSkillDeleteOperation({
|
||||
operationId: resolved.plan.operationId,
|
||||
hostKind: input.target.kind,
|
||||
skills,
|
||||
// Root *ids* only. A repo or plugin id is already a hash, where a label
|
||||
// carries the repo name and a path carries the user's directory names.
|
||||
rootIds: [
|
||||
...new Set(
|
||||
[...resolved.placementRoots.values()].flatMap((placements) =>
|
||||
placements.map((placement) => placement.root.id)
|
||||
)
|
||||
)
|
||||
]
|
||||
})
|
||||
return { operationId: resolved.plan.operationId, skills }
|
||||
}
|
||||
|
||||
async function deleteOneSkill(
|
||||
entry: SkillDeletePlanEntry,
|
||||
resolved: ResolvedSkillDeletePlan,
|
||||
input: SkillDeleteServiceInput
|
||||
): Promise<SkillDeleteResultEntry> {
|
||||
const api = nativeSkillPathSemantics().sep === '\\' ? pathWin32 : pathPosix
|
||||
const canonical = entry.placements.find((placement) => placement.kind === 'canonical')
|
||||
// The lock key is the canonical placement's own LITERAL directory path, in the
|
||||
// filesystem's spelling — not `canonicalPath`'s dirname. Install hashes the
|
||||
// literal destination without realpath'ing it, so a symlinked component (a
|
||||
// symlinked home, macOS `/tmp`) would hash to a different, uncontended lock.
|
||||
//
|
||||
// With no canonical placement (a tool-managed skill linked in from outside the
|
||||
// roots) no install could have created one, so any stable key excludes
|
||||
// correctly; prefer a path Orca owns over the out-of-root canonical.
|
||||
const lockKeyPath = resolved.toFilesystemPath(
|
||||
canonical?.path ?? entry.placements[0]?.path ?? api.dirname(entry.canonicalPath)
|
||||
)
|
||||
let releaseLock: (() => Promise<void>) | undefined
|
||||
try {
|
||||
releaseLock = await acquireSkillInstallLock({
|
||||
path: skillInstallLockPath(input.stateDirectory, lockKeyPath),
|
||||
...(input.lockTimeoutMs === undefined ? {} : { timeoutMs: input.lockTimeoutMs })
|
||||
})
|
||||
} catch {
|
||||
// Someone else is mid-transaction on this exact skill. The rest of the batch
|
||||
// is unaffected.
|
||||
return { id: entry.id, name: entry.name, status: 'busy', removedPaths: [] }
|
||||
}
|
||||
try {
|
||||
return await runStagedDeletion(entry, resolved, input, lockKeyPath)
|
||||
} catch {
|
||||
await recoverSkillDeleteTransaction(input.stateDirectory, lockKeyPath, input.filesystem).catch(
|
||||
() => undefined
|
||||
)
|
||||
return { id: entry.id, name: entry.name, status: 'failed', removedPaths: [] }
|
||||
} finally {
|
||||
await releaseLock()
|
||||
}
|
||||
}
|
||||
|
||||
async function runStagedDeletion(
|
||||
entry: SkillDeletePlanEntry,
|
||||
resolved: ResolvedSkillDeletePlan,
|
||||
input: SkillDeleteServiceInput,
|
||||
lockKeyPath: string
|
||||
): Promise<SkillDeleteResultEntry> {
|
||||
const moves = planSkillDeleteMoves(entry.placements, resolved.toFilesystemPath)
|
||||
const journal = (
|
||||
phase: SkillDeleteJournalV1['phase'],
|
||||
movedCount: number
|
||||
): SkillDeleteJournalV1 => ({
|
||||
schemaVersion: 1,
|
||||
operation: 'delete',
|
||||
phase,
|
||||
canonicalPath: lockKeyPath,
|
||||
wslDistro: input.wslDistro ?? null,
|
||||
allowedRoots: resolved.rootPaths.map(resolved.toFilesystemPath),
|
||||
movedCount,
|
||||
moves
|
||||
})
|
||||
// Written before anything is renamed: a crash between staging and cleanup must
|
||||
// not orphan a directory nothing will ever revisit.
|
||||
await writeSkillDeleteJournal(input.stateDirectory, journal('prepared', 0))
|
||||
|
||||
const staging = await stageSkillDeleteMoves({
|
||||
moves,
|
||||
filesystem: input.filesystem,
|
||||
onProgress: (movedCount) =>
|
||||
writeSkillDeleteJournal(input.stateDirectory, journal('staging', movedCount))
|
||||
})
|
||||
if (staging.status !== 'staged') {
|
||||
await rm(skillDeleteJournalPath(input.stateDirectory, lockKeyPath), { force: true })
|
||||
return staging.status === 'partial'
|
||||
? {
|
||||
id: entry.id,
|
||||
name: entry.name,
|
||||
status: 'partial',
|
||||
removedPaths: [],
|
||||
stagedPaths: staging.stagedPaths
|
||||
}
|
||||
: { id: entry.id, name: entry.name, status: 'failed', removedPaths: [] }
|
||||
}
|
||||
|
||||
await writeSkillDeleteJournal(input.stateDirectory, journal('staged', moves.length))
|
||||
const removal = await removeStagedSkillDeleteMoves(moves, input.filesystem)
|
||||
await removeSkillInstallReceipt(input.stateDirectory, lockKeyPath).catch(() => undefined)
|
||||
await rm(skillDeleteJournalPath(input.stateDirectory, lockKeyPath), { force: true })
|
||||
return {
|
||||
id: entry.id,
|
||||
name: entry.name,
|
||||
// Staging fully succeeded, so the skill is gone from every discovered
|
||||
// location — but an unremoved staged path still occupies disk.
|
||||
status: removal.unremoved.length > 0 ? 'partial' : 'deleted',
|
||||
removedPaths: sourcePathsFor(moves, removal.removedPaths),
|
||||
...(removal.unremoved.length > 0 ? { stagedPaths: removal.unremoved } : {})
|
||||
}
|
||||
}
|
||||
|
||||
function sourcePathsFor(
|
||||
moves: readonly SkillDeleteJournalMove[],
|
||||
removedSourcePaths: readonly string[]
|
||||
): string[] {
|
||||
const removed = new Set(removedSourcePaths)
|
||||
return moves.filter((move) => removed.has(move.sourcePath)).map((move) => move.sourcePath)
|
||||
}
|
||||
@@ -0,0 +1,145 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { posix as pathPosix, win32 as pathWin32 } from 'node:path'
|
||||
import type { SkillDeletePlacement } from '../../shared/skill-delete-contract'
|
||||
import { nativeSkillPathSemantics } from '../../shared/skill-path-containment'
|
||||
import type { SkillDeleteJournalMove } from './skill-delete-recovery'
|
||||
import type { SkillInstallFilesystem } from './skill-install-filesystem'
|
||||
import { SKILL_FILE_NAME } from './skill-root-file-walk'
|
||||
import { skillDeleteStagedName } from './skill-staging-names'
|
||||
|
||||
function api(): typeof pathPosix {
|
||||
return nativeSkillPathSemantics().sep === '\\' ? pathWin32 : pathPosix
|
||||
}
|
||||
|
||||
/**
|
||||
* Aliases stage before the canonical directory, so plain reversal of the move
|
||||
* list is exactly the rollback order the design requires: canonical restored
|
||||
* first, then `alias-dir`, then `alias-file` symlinks last — a restored alias
|
||||
* never points at a canonical directory that does not exist yet.
|
||||
*/
|
||||
const STAGING_ORDER: Record<SkillDeletePlacement['kind'], number> = {
|
||||
'alias-file': 0,
|
||||
'alias-dir': 1,
|
||||
canonical: 2
|
||||
}
|
||||
|
||||
/**
|
||||
* A staged sibling shares one parent directory with its source by construction,
|
||||
* so the rename cannot fail with `EXDEV` — there is no volume to cross. The one
|
||||
* failure that survives is staging a placement that is itself a mount point,
|
||||
* which fails with `EBUSY` and is reported per placement.
|
||||
*/
|
||||
export function planSkillDeleteMoves(
|
||||
placements: readonly SkillDeletePlacement[],
|
||||
toFilesystemPath: (path: string) => string
|
||||
): SkillDeleteJournalMove[] {
|
||||
const path = api()
|
||||
return [...placements]
|
||||
.sort((left, right) => STAGING_ORDER[left.kind] - STAGING_ORDER[right.kind])
|
||||
.map((placement) => {
|
||||
// An `alias-file` stages only the symlinked SKILL.md, in place. Its
|
||||
// directory is never staged, because it has to survive when it holds
|
||||
// anything besides that symlink.
|
||||
const source =
|
||||
placement.kind === 'alias-file'
|
||||
? path.join(toFilesystemPath(placement.path), SKILL_FILE_NAME)
|
||||
: toFilesystemPath(placement.path)
|
||||
return {
|
||||
sourcePath: source,
|
||||
stagedPath: path.join(
|
||||
path.dirname(source),
|
||||
skillDeleteStagedName(path.basename(source), randomUUID())
|
||||
),
|
||||
kind: placement.kind
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
export type SkillDeleteStagingOutcome =
|
||||
| { status: 'staged' }
|
||||
| { status: 'rolled-back'; error: unknown }
|
||||
| { status: 'partial'; error: unknown; stagedPaths: string[] }
|
||||
|
||||
export async function stageSkillDeleteMoves(input: {
|
||||
moves: readonly SkillDeleteJournalMove[]
|
||||
filesystem: SkillInstallFilesystem
|
||||
onProgress: (movedCount: number) => Promise<void>
|
||||
}): Promise<SkillDeleteStagingOutcome> {
|
||||
const staged: SkillDeleteJournalMove[] = []
|
||||
for (const move of input.moves) {
|
||||
// Record the intent before the rename, so a crash in between leaves a move
|
||||
// recovery treats as possibly-done rather than orphaning the directory.
|
||||
await input.onProgress(staged.length + 1)
|
||||
try {
|
||||
await input.filesystem.rename(move.sourcePath, move.stagedPath)
|
||||
staged.push(move)
|
||||
} catch (error) {
|
||||
const unrestored = await rollbackSkillDeleteMoves(staged, input.filesystem)
|
||||
return unrestored.length > 0
|
||||
? { status: 'partial', error, stagedPaths: unrestored }
|
||||
: { status: 'rolled-back', error }
|
||||
}
|
||||
}
|
||||
return { status: 'staged' }
|
||||
}
|
||||
|
||||
/** Returns the staged paths that could not be put back. */
|
||||
export async function rollbackSkillDeleteMoves(
|
||||
staged: readonly SkillDeleteJournalMove[],
|
||||
filesystem: SkillInstallFilesystem
|
||||
): Promise<string[]> {
|
||||
const unrestored: string[] = []
|
||||
// Indexed rather than `toReversed()`: the delete modules reach the Node 18
|
||||
// relay bundle, where the ES2023 array-copy methods do not exist.
|
||||
for (let index = staged.length - 1; index >= 0; index -= 1) {
|
||||
const move = staged[index]
|
||||
try {
|
||||
await filesystem.rename(move.stagedPath, move.sourcePath)
|
||||
} catch {
|
||||
// Neither fully restored nor fully removed. Reported as `partial` with the
|
||||
// staged path so the result band can say where the files actually are,
|
||||
// rather than `failed`, which would wrongly imply nothing changed.
|
||||
unrestored.push(move.stagedPath)
|
||||
}
|
||||
}
|
||||
return unrestored
|
||||
}
|
||||
|
||||
export type SkillDeleteRemovalOutcome = { removedPaths: string[]; unremoved: string[] }
|
||||
|
||||
export async function removeStagedSkillDeleteMoves(
|
||||
moves: readonly SkillDeleteJournalMove[],
|
||||
filesystem: SkillInstallFilesystem
|
||||
): Promise<SkillDeleteRemovalOutcome> {
|
||||
const path = api()
|
||||
const removedPaths: string[] = []
|
||||
const unremoved: string[] = []
|
||||
for (const move of moves) {
|
||||
try {
|
||||
await filesystem.remove(move.stagedPath)
|
||||
removedPaths.push(move.sourcePath)
|
||||
if (move.kind === 'alias-file') {
|
||||
await removeDirectoryIfEmpty(path.dirname(move.sourcePath), filesystem)
|
||||
}
|
||||
} catch {
|
||||
unremoved.push(move.stagedPath)
|
||||
}
|
||||
}
|
||||
return { removedPaths, unremoved }
|
||||
}
|
||||
|
||||
/** Matches the model's guarantee: a directory holding anything besides the
|
||||
* removed symlink is left in place, because "not shared with another
|
||||
* placement" is not the same guarantee as "contains nothing else". */
|
||||
async function removeDirectoryIfEmpty(
|
||||
directory: string,
|
||||
filesystem: SkillInstallFilesystem
|
||||
): Promise<void> {
|
||||
if (!filesystem.listEntries) {
|
||||
return
|
||||
}
|
||||
const listing = await filesystem.listEntries([directory]).catch(() => null)
|
||||
if (listing?.get(directory)?.length === 0) {
|
||||
await filesystem.remove(directory).catch(() => undefined)
|
||||
}
|
||||
}
|
||||
@@ -17,6 +17,8 @@ import {
|
||||
} from './skill-discovery-sources'
|
||||
import { discoverClaudePluginSkillSourcesInWsl } from './claude-plugin-skill-sources-wsl'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
import { SKILL_STAGING_GLOB } from './skill-staging-names'
|
||||
import { skillFileMaxDepth } from '../../shared/skill-discovery-depth'
|
||||
|
||||
const MAX_MARKDOWN_BYTES = 256 * 1024
|
||||
const WSL_SCAN_TIMEOUT_MS = 10_000
|
||||
@@ -42,11 +44,11 @@ export function buildWslSkillDiscoveryCommand(roots: readonly SkillScanRoot[]):
|
||||
` printf '%s\\0%s\\0%s\\0%s\\0%s\\0' S "$root_index" "$skill_file" "$canonical_path" "$updated_at"`,
|
||||
` printf '%s' "$encoded_markdown"`,
|
||||
` printf '\\0'`,
|
||||
` done < <(find -L "$root_path" -mindepth 1 -maxdepth "$max_depth" -type f -name 'SKILL.md' -print0 2>/dev/null)`,
|
||||
` done < <(find -L "$root_path" -mindepth 1 -maxdepth "$max_depth" \\( -name '${SKILL_STAGING_GLOB}' -prune \\) -o \\( -type f -name 'SKILL.md' -print0 \\) 2>/dev/null)`,
|
||||
'}'
|
||||
]
|
||||
roots.forEach((root, index) => {
|
||||
const maxDepth = root.sourceKind === 'plugin' ? 10 : 5
|
||||
const maxDepth = skillFileMaxDepth(root.sourceKind)
|
||||
lines.push(`scan_root ${index} ${quoteBashString(root.path)} ${maxDepth}`)
|
||||
})
|
||||
return lines.join('\n')
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { rm } from 'node:fs/promises'
|
||||
import type { Dirent, Stats } from 'node:fs'
|
||||
import { lstat, readdir, realpath, rm, stat } from 'node:fs/promises'
|
||||
import type { SkillPackageManifestV1 } from '../../shared/skill-package-manifest'
|
||||
import { renameSkillPathWithWindowsRetry } from './skill-filesystem-retry'
|
||||
import { runSkillCandidateTasks } from './skill-candidate-concurrency'
|
||||
import {
|
||||
SKILL_PACKAGE_OBSERVATION_LIMITS,
|
||||
observeSkillPackage,
|
||||
@@ -9,8 +11,36 @@ import {
|
||||
|
||||
export type SkillInstalledFileMode = { path: string; executable: boolean }
|
||||
|
||||
/** `lstat` semantics: a symlink is `symlink` whatever it points at. */
|
||||
export type SkillFilesystemEntryKind = 'directory' | 'file' | 'symlink' | 'other' | 'missing'
|
||||
|
||||
export type SkillDirectoryEntry = { name: string; kind: SkillFilesystemEntryKind }
|
||||
|
||||
export type SkillPathInspection = {
|
||||
kind: SkillFilesystemEntryKind
|
||||
/** Fully resolved path; null when it cannot be resolved (broken link, gone). */
|
||||
realpath: string | null
|
||||
/**
|
||||
* Modification time in ms, read the same way the host's own discovery read it
|
||||
* so a freshness comparison stays self-consistent: native `stat` (follows a
|
||||
* symlink), WSL's `stat -c %Y` (does not). Null when it cannot be read.
|
||||
*/
|
||||
mtimeMs: number | null
|
||||
}
|
||||
|
||||
export type SkillInstallFilesystem = {
|
||||
authorizeRoots?(paths: readonly string[]): void
|
||||
/**
|
||||
* Enumeration primitives for path-based deletion. Both take arrays because a
|
||||
* guest or remote filesystem must answer a whole delete plan in one round
|
||||
* trip — the WSL home alone contributes ~16 discovery roots, and one
|
||||
* `wsl.exe` boot per root is the cost this shape exists to avoid.
|
||||
*
|
||||
* Optional: only implementations that can enumerate provide them, and the
|
||||
* delete service refuses rather than guessing when they are absent.
|
||||
*/
|
||||
listEntries?(directories: readonly string[]): Promise<Map<string, SkillDirectoryEntry[]>>
|
||||
inspectPaths?(paths: readonly string[]): Promise<Map<string, SkillPathInspection>>
|
||||
prepareExtractedSkill(path: string, manifest: SkillPackageManifestV1): Promise<void>
|
||||
observeSkill(
|
||||
path: string,
|
||||
@@ -33,5 +63,58 @@ export const nativeSkillInstallFilesystem: SkillInstallFilesystem = {
|
||||
: undefined
|
||||
),
|
||||
rename: renameSkillPathWithWindowsRetry,
|
||||
remove: (path) => rm(path, { recursive: true, force: true })
|
||||
remove: (path) => rm(path, { recursive: true, force: true }),
|
||||
// Why the bounded pool rather than `Promise.all`: a delete plan enumerates
|
||||
// every discovery root, and unbounded fan-out here is the same burst of
|
||||
// filesystem-metadata work discovery already learned to cap.
|
||||
listEntries: async (directories) =>
|
||||
new Map(
|
||||
await runSkillCandidateTasks(
|
||||
directories.map((directory) => async (): Promise<[string, SkillDirectoryEntry[]]> => {
|
||||
const entries = await readdir(directory, { withFileTypes: true }).catch(() => null)
|
||||
return [
|
||||
directory,
|
||||
(entries ?? []).map((entry) => ({ name: entry.name, kind: direntKind(entry) }))
|
||||
]
|
||||
})
|
||||
)
|
||||
),
|
||||
inspectPaths: async (paths) =>
|
||||
new Map(
|
||||
await runSkillCandidateTasks(
|
||||
paths.map(
|
||||
(path) => async (): Promise<[string, SkillPathInspection]> => [
|
||||
path,
|
||||
{
|
||||
kind: statKind(await lstat(path).catch(() => null)),
|
||||
realpath: await realpath(path).catch(() => null),
|
||||
mtimeMs: (await stat(path).catch(() => null))?.mtimeMs ?? null
|
||||
}
|
||||
]
|
||||
)
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
function direntKind(entry: Dirent): SkillFilesystemEntryKind {
|
||||
if (entry.isSymbolicLink()) {
|
||||
return 'symlink'
|
||||
}
|
||||
if (entry.isDirectory()) {
|
||||
return 'directory'
|
||||
}
|
||||
return entry.isFile() ? 'file' : 'other'
|
||||
}
|
||||
|
||||
function statKind(value: Stats | null): SkillFilesystemEntryKind {
|
||||
if (!value) {
|
||||
return 'missing'
|
||||
}
|
||||
if (value.isSymbolicLink()) {
|
||||
return 'symlink'
|
||||
}
|
||||
if (value.isDirectory()) {
|
||||
return 'directory'
|
||||
}
|
||||
return value.isFile() ? 'file' : 'other'
|
||||
}
|
||||
|
||||
@@ -15,6 +15,9 @@ import { skillInstallFailureFromError } from './skill-install-operation-error'
|
||||
|
||||
const SAFE_LABEL = /^[a-z0-9][a-z0-9._-]{0,63}$/i
|
||||
const MAX_OBSERVED_COUNT = 1_000_000
|
||||
// Roots grow with the repo count, so an uncapped id list makes one log line
|
||||
// grow with the install.
|
||||
const MAX_LOGGED_DELETE_ROOT_IDS = 12
|
||||
const MAX_OBSERVED_BYTES = 64 * 1024 * 1024
|
||||
|
||||
type SkillOperationPhase =
|
||||
@@ -279,3 +282,36 @@ export function startSkillBundleInstallOperation(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* One summary line per delete batch. Root *ids* are safe to log where labels
|
||||
* and paths are not — a repo or plugin id is already a hash — matching
|
||||
* discovery's own logging rule.
|
||||
*/
|
||||
export function recordSkillDeleteOperation(input: {
|
||||
operationId: string
|
||||
hostKind: 'native-host' | 'wsl'
|
||||
skills: readonly { status: string }[]
|
||||
rootIds: readonly string[]
|
||||
}): void {
|
||||
const counts = new Map<string, number>()
|
||||
for (const skill of input.skills) {
|
||||
counts.set(skill.status, (counts.get(skill.status) ?? 0) + 1)
|
||||
}
|
||||
const span = startSpan('skill.delete', {
|
||||
attributes: {
|
||||
phase: 'delete',
|
||||
platform: platformLabel(),
|
||||
host: safeLabel(input.hostKind, 'unknown'),
|
||||
skillCount: boundedNumber(input.skills.length),
|
||||
...Object.fromEntries(
|
||||
[...counts].map(([status, count]) => [
|
||||
`${safeLabel(status, 'unknown')}Count`,
|
||||
boundedNumber(count)
|
||||
])
|
||||
),
|
||||
rootIds: input.rootIds.slice(0, MAX_LOGGED_DELETE_ROOT_IDS).join(',')
|
||||
}
|
||||
})
|
||||
span.end()
|
||||
}
|
||||
|
||||
@@ -1,22 +1,21 @@
|
||||
import { lstat, readlink } from 'node:fs/promises'
|
||||
import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'
|
||||
import { basename, dirname, join, resolve } from 'node:path'
|
||||
import {
|
||||
nativeSkillPathSemantics,
|
||||
normalizedSkillPath,
|
||||
skillPathInside
|
||||
} from '../../shared/skill-path-containment'
|
||||
import type { SkillPlacementResult } from '../../shared/skill-install-contract'
|
||||
import type { SkillInstallFilesystem } from './skill-install-filesystem'
|
||||
import type { SkillInstallReceiptV1 } from './skill-install-provenance'
|
||||
|
||||
// Native-only call site: these placements are always the process's own paths.
|
||||
function normalizedPath(path: string): string {
|
||||
const normalized = resolve(path)
|
||||
return process.platform === 'win32' ? normalized.toLocaleLowerCase('en-US') : normalized
|
||||
return normalizedSkillPath(path, nativeSkillPathSemantics())
|
||||
}
|
||||
|
||||
function pathInside(root: string, path: string): boolean {
|
||||
const child = relative(resolve(root), resolve(path))
|
||||
return (
|
||||
child !== '' &&
|
||||
child !== '..' &&
|
||||
!child.startsWith(`..${process.platform === 'win32' ? '\\' : '/'}`) &&
|
||||
!isAbsolute(child)
|
||||
)
|
||||
return skillPathInside(root, path, nativeSkillPathSemantics())
|
||||
}
|
||||
|
||||
async function aliasTargetsCanonical(path: string, canonicalPath: string): Promise<boolean> {
|
||||
|
||||
@@ -1,5 +1,10 @@
|
||||
import { lstat, readlink, rm } from 'node:fs/promises'
|
||||
import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'
|
||||
import { basename, dirname, join, resolve } from 'node:path'
|
||||
import {
|
||||
nativeSkillPathSemantics,
|
||||
normalizedSkillPath,
|
||||
skillPathInside
|
||||
} from '../../shared/skill-path-containment'
|
||||
import { readNodeFileWithinLimit } from '../../shared/node-bounded-file-reader'
|
||||
import type { SkillPlacementResult } from '../../shared/skill-install-contract'
|
||||
import type { SkillInstallFilesystem } from './skill-install-filesystem'
|
||||
@@ -47,19 +52,13 @@ function validBackup(move: RemovalMove): boolean {
|
||||
)
|
||||
}
|
||||
|
||||
// Native-only call site: a journal's paths are always the process's own.
|
||||
function normalizedPath(path: string): string {
|
||||
const normalized = resolve(path)
|
||||
return process.platform === 'win32' ? normalized.toLocaleLowerCase('en-US') : normalized
|
||||
return normalizedSkillPath(path, nativeSkillPathSemantics())
|
||||
}
|
||||
|
||||
function pathInside(root: string, path: string): boolean {
|
||||
const child = relative(resolve(root), resolve(path))
|
||||
return (
|
||||
child !== '' &&
|
||||
child !== '..' &&
|
||||
!child.startsWith(`..${process.platform === 'win32' ? '\\' : '/'}`) &&
|
||||
!isAbsolute(child)
|
||||
)
|
||||
return skillPathInside(root, path, nativeSkillPathSemantics())
|
||||
}
|
||||
|
||||
function moveOwnedByJournal(journal: Partial<SkillRemovalJournalV1>, move: RemovalMove): boolean {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import type { Dirent } from 'node:fs'
|
||||
import { readdir, realpath, stat } from 'node:fs/promises'
|
||||
import { isAbsolute, join, relative, sep } from 'node:path'
|
||||
import { isSkillStagingEntryName } from './skill-staging-names'
|
||||
|
||||
export const SKILL_FILE_NAME = 'SKILL.md'
|
||||
|
||||
@@ -62,6 +63,11 @@ export async function findSkillFiles(
|
||||
}
|
||||
for (const entry of entries) {
|
||||
signal?.throwIfAborted()
|
||||
// Why: a staged sibling sits directly in a scanned root, so without this a
|
||||
// skill mid-transaction surfaces as a second, separately actionable row.
|
||||
if (isSkillStagingEntryName(entry.name)) {
|
||||
continue
|
||||
}
|
||||
const entryPath = join(dirPath, entry.name)
|
||||
if (entry.name === SKILL_FILE_NAME) {
|
||||
if (entry.isFile()) {
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* Every skill transaction stages a path as a hidden sibling before touching it,
|
||||
* and those siblings land directly in a scanned discovery root. Discovery must
|
||||
* not surface them, or a skill mid-transaction shows up as a second row (and,
|
||||
* for delete, as a row the user can delete again).
|
||||
*
|
||||
* The four conventions in the tree today:
|
||||
* `.<name>.orca-skill-delete-<uuid>` (skill-delete-service)
|
||||
* `.<name>.orca-remove-backup-<uuid>` (skill-remove-transaction)
|
||||
* `.<name>.orca-placement-backup-<id>` (skill-placement-transaction-controller)
|
||||
* `.<name>.orca-placement-staging-<id>` (skill-placement-transaction-controller)
|
||||
*
|
||||
* Matched by shape rather than by an enumerated list so the native walker and
|
||||
* WSL's `find -prune` (which can only express a glob) cannot drift apart.
|
||||
*/
|
||||
export const SKILL_DELETE_STAGING_MARKER = '.orca-skill-delete-'
|
||||
|
||||
/** The `find -name` glob the WSL guest script prunes on. */
|
||||
export const SKILL_STAGING_GLOB = '.*.orca-*'
|
||||
|
||||
const SKILL_STAGING_NAME = /^\..+\.orca-/u
|
||||
|
||||
export function isSkillStagingEntryName(name: string): boolean {
|
||||
return SKILL_STAGING_NAME.test(name)
|
||||
}
|
||||
|
||||
export function skillDeleteStagedName(basename: string, id: string): string {
|
||||
return `.${basename}${SKILL_DELETE_STAGING_MARKER}${id}`
|
||||
}
|
||||
|
||||
export function isSkillDeleteStagedName(basename: string, staged: string): boolean {
|
||||
return staged.startsWith(`.${basename}${SKILL_DELETE_STAGING_MARKER}`)
|
||||
}
|
||||
@@ -0,0 +1,82 @@
|
||||
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { buildWslSkillDiscoveryCommand } from './skill-discovery-wsl'
|
||||
import { findSkillFiles } from './skill-root-file-walk'
|
||||
import {
|
||||
isSkillStagingEntryName,
|
||||
skillDeleteStagedName,
|
||||
SKILL_STAGING_GLOB
|
||||
} from './skill-staging-names'
|
||||
|
||||
const roots: string[] = []
|
||||
|
||||
afterEach(async () => {
|
||||
await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true })))
|
||||
})
|
||||
|
||||
async function stagedRoot(stagedName: string): Promise<string> {
|
||||
const root = await mkdtemp(join(tmpdir(), 'orca-skill-staging-visibility-'))
|
||||
roots.push(root)
|
||||
const staged = join(root, stagedName)
|
||||
await mkdir(staged, { recursive: true })
|
||||
await writeFile(join(staged, 'SKILL.md'), '---\nname: demo\n---\n')
|
||||
return root
|
||||
}
|
||||
|
||||
describe('isSkillStagingEntryName', () => {
|
||||
it.each([
|
||||
'.demo.orca-skill-delete-1234',
|
||||
'.demo.orca-remove-backup-1234',
|
||||
'.demo.orca-placement-backup-1234',
|
||||
'.demo.orca-placement-staging-1234'
|
||||
])('matches the %s convention', (name) => {
|
||||
expect(isSkillStagingEntryName(name)).toBe(true)
|
||||
})
|
||||
|
||||
it.each(['demo', '.hidden-skill', '..cache', '.orca'])('leaves %s alone', (name) => {
|
||||
expect(isSkillStagingEntryName(name)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('native walker', () => {
|
||||
it.each([
|
||||
skillDeleteStagedName('demo', 'id'),
|
||||
'.demo.orca-remove-backup-id',
|
||||
'.demo.orca-placement-backup-id'
|
||||
])('does not surface a %s staged directly in a scanned root', async (stagedName) => {
|
||||
const root = await stagedRoot(stagedName)
|
||||
expect(await findSkillFiles(root, 4)).toEqual([])
|
||||
})
|
||||
|
||||
it('still finds an ordinary skill beside a staged one', async () => {
|
||||
const root = await stagedRoot(skillDeleteStagedName('demo', 'id'))
|
||||
const real = join(root, 'demo')
|
||||
await mkdir(real)
|
||||
await writeFile(join(real, 'SKILL.md'), '---\nname: demo\n---\n')
|
||||
expect(await findSkillFiles(root, 4)).toEqual([join(real, 'SKILL.md')])
|
||||
})
|
||||
})
|
||||
|
||||
describe('WSL guest scan', () => {
|
||||
it('prunes the same staging shapes the native walker skips', () => {
|
||||
// The command is base64-wrapped, so assert on the decoded script.
|
||||
const command = buildWslSkillDiscoveryCommand([
|
||||
{
|
||||
id: 'home-agents',
|
||||
label: 'Agent skills home',
|
||||
path: '/home/u/.agents/skills',
|
||||
sourceKind: 'home',
|
||||
providers: ['agent-skills'],
|
||||
owner: null
|
||||
}
|
||||
])
|
||||
const encoded = command.match(/printf %s '([^']+)'/)?.[1]
|
||||
expect(encoded).toBeTruthy()
|
||||
const script = Buffer.from(encoded ?? '', 'base64').toString('utf8')
|
||||
expect(script).toContain(`-name '${SKILL_STAGING_GLOB}' -prune`)
|
||||
// The prune must gate the print, not sit beside it.
|
||||
expect(script).toContain("-prune \\) -o \\( -type f -name 'SKILL.md' -print0 \\)")
|
||||
})
|
||||
})
|
||||
@@ -19,6 +19,10 @@ import {
|
||||
import { startSkillPhaseOperation } from './skill-operation-observability'
|
||||
import { WslSkillInstallFilesystem } from './skill-wsl-install-filesystem'
|
||||
import { readSkillPlacementRecoveryJournal } from './skill-placement-recovery-journal'
|
||||
import {
|
||||
readSkillDeleteRecoveryJournal,
|
||||
recoverSkillDeleteTransaction
|
||||
} from './skill-delete-recovery'
|
||||
import { recoverSkillPlacementTransaction } from './skill-placement-transaction'
|
||||
|
||||
const MAX_PENDING_TRANSACTION_JOURNALS = 64
|
||||
@@ -30,6 +34,7 @@ type PendingTransaction = {
|
||||
install: boolean
|
||||
removal: boolean
|
||||
placement: boolean
|
||||
delete: boolean
|
||||
}
|
||||
|
||||
export type SkillTransactionStartupRecoveryReport = {
|
||||
@@ -49,7 +54,7 @@ function failureCode(error: unknown): string {
|
||||
|
||||
async function scanJournalDirectory(
|
||||
stateDirectory: string,
|
||||
directoryName: 'journals' | 'removal-journals' | 'placement-journals'
|
||||
directoryName: 'journals' | 'removal-journals' | 'placement-journals' | 'delete-journals'
|
||||
): Promise<{
|
||||
candidates: { canonicalPath: string; journalKey: string }[]
|
||||
failures: { journalKey: string; code: string }[]
|
||||
@@ -109,14 +114,15 @@ function pendingTransactions(
|
||||
const pending = new Map<string, PendingTransaction>()
|
||||
const add = (
|
||||
candidate: { canonicalPath: string; journalKey: string },
|
||||
kind: 'install' | 'removal' | 'placement'
|
||||
kind: 'install' | 'removal'
|
||||
): void => {
|
||||
const current = pending.get(candidate.canonicalPath) ?? {
|
||||
canonicalPath: candidate.canonicalPath,
|
||||
journalKey: candidate.journalKey,
|
||||
install: false,
|
||||
removal: false,
|
||||
placement: false
|
||||
placement: false,
|
||||
delete: false
|
||||
}
|
||||
current[kind] = true
|
||||
pending.set(candidate.canonicalPath, current)
|
||||
@@ -126,13 +132,38 @@ function pendingTransactions(
|
||||
return [...pending.values()]
|
||||
}
|
||||
|
||||
/** Merged after the map-based dedupe so a kind that shares a canonical path with
|
||||
* an install or removal joins that transaction rather than racing it. */
|
||||
function mergeJournalCandidates(
|
||||
pending: PendingTransaction[],
|
||||
candidates: readonly { canonicalPath: string; journalKey: string }[],
|
||||
kind: 'placement' | 'delete'
|
||||
): void {
|
||||
for (const candidate of candidates) {
|
||||
const current = pending.find((entry) => entry.canonicalPath === candidate.canonicalPath)
|
||||
if (current) {
|
||||
current[kind] = true
|
||||
continue
|
||||
}
|
||||
pending.push({
|
||||
...candidate,
|
||||
install: false,
|
||||
removal: false,
|
||||
placement: false,
|
||||
delete: false,
|
||||
[kind]: true
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
async function recoverPendingSkillTransactionsUnobserved(
|
||||
stateDirectory: string
|
||||
): Promise<SkillTransactionStartupRecoveryReport> {
|
||||
const [installs, removals, placements, extractions, locks] = await Promise.all([
|
||||
const [installs, removals, placements, deletes, extractions, locks] = await Promise.all([
|
||||
scanJournalDirectory(stateDirectory, 'journals'),
|
||||
scanJournalDirectory(stateDirectory, 'removal-journals'),
|
||||
scanJournalDirectory(stateDirectory, 'placement-journals'),
|
||||
scanJournalDirectory(stateDirectory, 'delete-journals'),
|
||||
recoverPendingSkillExtractions(stateDirectory),
|
||||
reclaimDeadSkillInstallLocks(stateDirectory)
|
||||
])
|
||||
@@ -141,6 +172,7 @@ async function recoverPendingSkillTransactionsUnobserved(
|
||||
installs.candidates.length +
|
||||
removals.candidates.length +
|
||||
placements.candidates.length +
|
||||
deletes.candidates.length +
|
||||
extractions.scanned,
|
||||
recovered: extractions.recovered,
|
||||
orphanedExtractionsRecovered: extractions.recovered,
|
||||
@@ -149,24 +181,20 @@ async function recoverPendingSkillTransactionsUnobserved(
|
||||
...installs.failures,
|
||||
...removals.failures,
|
||||
...placements.failures,
|
||||
...deletes.failures,
|
||||
...extractions.failures
|
||||
],
|
||||
truncated:
|
||||
installs.truncated ||
|
||||
removals.truncated ||
|
||||
placements.truncated ||
|
||||
deletes.truncated ||
|
||||
extractions.truncated ||
|
||||
locks.truncated
|
||||
}
|
||||
const pending = pendingTransactions(installs.candidates, removals.candidates)
|
||||
for (const placement of placements.candidates) {
|
||||
const current = pending.find((candidate) => candidate.canonicalPath === placement.canonicalPath)
|
||||
if (current) {
|
||||
current.placement = true
|
||||
} else {
|
||||
pending.push({ ...placement, install: false, removal: false, placement: true })
|
||||
}
|
||||
}
|
||||
mergeJournalCandidates(pending, placements.candidates, 'placement')
|
||||
mergeJournalCandidates(pending, deletes.candidates, 'delete')
|
||||
for (const pendingTransaction of pending) {
|
||||
let releaseLock: (() => Promise<void>) | null = null
|
||||
try {
|
||||
@@ -182,11 +210,15 @@ async function recoverPendingSkillTransactionsUnobserved(
|
||||
const placementJournal = pendingTransaction.placement
|
||||
? await readSkillPlacementRecoveryJournal(stateDirectory, pendingTransaction.canonicalPath)
|
||||
: null
|
||||
const deleteJournal = pendingTransaction.delete
|
||||
? await readSkillDeleteRecoveryJournal(stateDirectory, pendingTransaction.canonicalPath)
|
||||
: null
|
||||
const distros = new Set(
|
||||
[
|
||||
installJournal?.receipt.wslDistro,
|
||||
removalJournal?.receipt.wslDistro,
|
||||
placementJournal?.wslDistro
|
||||
placementJournal?.wslDistro,
|
||||
deleteJournal?.wslDistro
|
||||
].filter((distro): distro is string => Boolean(distro))
|
||||
)
|
||||
if (distros.size > 1 || (distros.size && process.platform !== 'win32')) {
|
||||
@@ -200,6 +232,12 @@ async function recoverPendingSkillTransactionsUnobserved(
|
||||
...(placementJournal?.actions.map((action) => action.rootPath) ?? [])
|
||||
])
|
||||
: undefined
|
||||
// Why explicit rather than only widening the constructor list: the delete
|
||||
// journal is the sole record of which roots its moves are allowed to
|
||||
// touch, and replay must be authorized for them before it operates.
|
||||
if (deleteJournal) {
|
||||
filesystem?.authorizeRoots(deleteJournal.allowedRoots)
|
||||
}
|
||||
if (removalJournal) {
|
||||
await recoverSkillRemovalTransaction(
|
||||
stateDirectory,
|
||||
@@ -224,6 +262,14 @@ async function recoverPendingSkillTransactionsUnobserved(
|
||||
)
|
||||
report.recovered += 1
|
||||
}
|
||||
if (deleteJournal) {
|
||||
await recoverSkillDeleteTransaction(
|
||||
stateDirectory,
|
||||
pendingTransaction.canonicalPath,
|
||||
filesystem
|
||||
)
|
||||
report.recovered += 1
|
||||
}
|
||||
} catch (error) {
|
||||
report.failures.push({
|
||||
journalKey: pendingTransaction.journalKey,
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
import { execFile } from 'node:child_process'
|
||||
import { promisify } from 'node:util'
|
||||
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
||||
import { WslSkillInstallFilesystem } from './skill-wsl-install-filesystem'
|
||||
|
||||
const execFileAsync = promisify(execFile)
|
||||
const DISTRO = process.env.ORCA_REAL_WSL_SKILL_DISTRO ?? 'Ubuntu-24.04'
|
||||
const RUN_REAL_WSL = process.platform === 'win32' && process.env.ORCA_REAL_WSL_SKILL_TEST === '1'
|
||||
|
||||
async function runWsl(...args: string[]): Promise<string> {
|
||||
const { stdout } = await execFileAsync('wsl.exe', ['-d', DISTRO, '--exec', ...args], {
|
||||
encoding: 'utf8',
|
||||
timeout: 30_000,
|
||||
windowsHide: true
|
||||
})
|
||||
return stdout.trim()
|
||||
}
|
||||
|
||||
function uncPath(guestPath: string): string {
|
||||
return `\\\\wsl.localhost\\${DISTRO}${guestPath.replaceAll('/', '\\')}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Enumeration and deletion stay server-side in the distro for the same reason
|
||||
* install does: node's `fs` over a `\\wsl$` path applies Windows semantics to
|
||||
* POSIX symlinks and permissions.
|
||||
*/
|
||||
describe.runIf(RUN_REAL_WSL)('real WSL skill deletion primitives', () => {
|
||||
let guestRoot = ''
|
||||
let skillsRoot = ''
|
||||
|
||||
beforeAll(async () => {
|
||||
guestRoot = await runWsl('mktemp', '-d', '/tmp/orca-skill-delete.XXXXXX')
|
||||
if (!guestRoot.startsWith('/tmp/orca-skill-delete.')) {
|
||||
throw new Error('unexpected-wsl-integration-root')
|
||||
}
|
||||
skillsRoot = `${guestRoot}/home/.agents/skills`
|
||||
await runWsl('mkdir', '-p', `${skillsRoot}/demo`, `${guestRoot}/home/.codex/skills/demo`)
|
||||
await runWsl('sh', '-c', `printf '%s' 'x' > ${skillsRoot}/demo/SKILL.md`)
|
||||
// An alias-dir in another provider root, and an alias-file inside a real one.
|
||||
await runWsl('mkdir', '-p', `${guestRoot}/home/.claude/skills`)
|
||||
await runWsl('ln', '-sT', `${skillsRoot}/demo`, `${guestRoot}/home/.claude/skills/demo`)
|
||||
await runWsl(
|
||||
'ln',
|
||||
'-sT',
|
||||
`${skillsRoot}/demo/SKILL.md`,
|
||||
`${guestRoot}/home/.codex/skills/demo/SKILL.md`
|
||||
)
|
||||
// Deliberately differs only by case, to prove POSIX case-sensitivity.
|
||||
await runWsl('mkdir', '-p', `${guestRoot}/home/.agents/Skills`)
|
||||
})
|
||||
|
||||
afterAll(async () => {
|
||||
if (guestRoot.startsWith('/tmp/orca-skill-delete.')) {
|
||||
await runWsl('rm', '-rf', '--', guestRoot)
|
||||
}
|
||||
})
|
||||
|
||||
it('refuses an unauthorized root before authorizeRoots, and permits it after', async () => {
|
||||
const filesystem = new WslSkillInstallFilesystem(DISTRO, [])
|
||||
await expect(filesystem.listEntries([uncPath(skillsRoot)])).rejects.toThrow(
|
||||
'skill-install-wsl-path-outside-root'
|
||||
)
|
||||
filesystem.authorizeRoots([uncPath(skillsRoot)])
|
||||
const listing = await filesystem.listEntries([uncPath(skillsRoot)])
|
||||
expect(listing.get(uncPath(skillsRoot))).toContainEqual({ name: 'demo', kind: 'directory' })
|
||||
})
|
||||
|
||||
it('keeps containment case-sensitive, as the guest filesystem is', async () => {
|
||||
const filesystem = new WslSkillInstallFilesystem(DISTRO, [
|
||||
uncPath(`${guestRoot}/home/.agents/Skills`)
|
||||
])
|
||||
await expect(filesystem.remove(uncPath(`${skillsRoot}/demo`))).rejects.toThrow(
|
||||
'skill-install-wsl-path-outside-root'
|
||||
)
|
||||
})
|
||||
|
||||
it('classifies alias-dir and alias-file from one batched inspection', async () => {
|
||||
const filesystem = new WslSkillInstallFilesystem(DISTRO, [uncPath(`${guestRoot}/home`)])
|
||||
const aliasDir = uncPath(`${guestRoot}/home/.claude/skills/demo`)
|
||||
const aliasFile = uncPath(`${guestRoot}/home/.codex/skills/demo/SKILL.md`)
|
||||
const canonicalFile = uncPath(`${skillsRoot}/demo/SKILL.md`)
|
||||
const inspections = await filesystem.inspectPaths([aliasDir, aliasFile, canonicalFile])
|
||||
|
||||
expect(inspections.get(aliasDir)).toMatchObject({
|
||||
kind: 'symlink',
|
||||
realpath: `${skillsRoot}/demo`
|
||||
})
|
||||
expect(inspections.get(aliasFile)).toMatchObject({
|
||||
kind: 'symlink',
|
||||
realpath: `${skillsRoot}/demo/SKILL.md`
|
||||
})
|
||||
expect(inspections.get(canonicalFile)).toMatchObject({
|
||||
kind: 'file',
|
||||
realpath: `${skillsRoot}/demo/SKILL.md`
|
||||
})
|
||||
})
|
||||
|
||||
it('reads mtime without dereferencing, so an alias-file records the link itself', async () => {
|
||||
const filesystem = new WslSkillInstallFilesystem(DISTRO, [uncPath(`${guestRoot}/home`)])
|
||||
const aliasFile = uncPath(`${guestRoot}/home/.codex/skills/demo/SKILL.md`)
|
||||
const inspection = (await filesystem.inspectPaths([aliasFile])).get(aliasFile)
|
||||
const linkSeconds = Number.parseInt(
|
||||
await runWsl('stat', '-c', '%Y', '--', `${guestRoot}/home/.codex/skills/demo/SKILL.md`),
|
||||
10
|
||||
)
|
||||
expect(inspection?.mtimeMs).toBe(linkSeconds * 1000)
|
||||
})
|
||||
|
||||
it('renames a placement to a hidden sibling and removes it', async () => {
|
||||
const filesystem = new WslSkillInstallFilesystem(DISTRO, [uncPath(`${guestRoot}/home`)])
|
||||
const source = `${skillsRoot}/demo`
|
||||
const staged = `${skillsRoot}/.demo.orca-skill-delete-integration`
|
||||
await filesystem.rename(uncPath(source), uncPath(staged))
|
||||
expect(await runWsl('sh', '-c', `[ -d '${staged}' ] && printf yes`)).toBe('yes')
|
||||
await filesystem.remove(uncPath(staged))
|
||||
expect(await runWsl('sh', '-c', `[ -e '${staged}' ] || printf gone`)).toBe('gone')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,92 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
parseWslInspectPathsOutput,
|
||||
parseWslListEntriesOutput,
|
||||
WslEnumerationProtocolError,
|
||||
WSL_INSPECT_PATHS_SCRIPT,
|
||||
WSL_LIST_ENTRIES_SCRIPT
|
||||
} from './skill-wsl-enumeration-protocol'
|
||||
|
||||
const NUL = '\0'
|
||||
|
||||
function records(...fields: string[]): string {
|
||||
return fields.map((field) => `${field}${NUL}`).join('')
|
||||
}
|
||||
|
||||
describe('parseWslListEntriesOutput', () => {
|
||||
it('keys listings by the caller’s own strings, not the guest spelling', () => {
|
||||
const directories = [
|
||||
'\\\\wsl.localhost\\Ubuntu\\home\\u\\.agents\\skills',
|
||||
'\\\\wsl.localhost\\Ubuntu\\home\\u\\.claude\\skills'
|
||||
]
|
||||
const output = records('D', '0', 'E', 'demo', 'directory', 'E', 'link', 'symlink', 'D', '1')
|
||||
const listings = parseWslListEntriesOutput(output, directories)
|
||||
expect(listings.get(directories[0])).toEqual([
|
||||
{ name: 'demo', kind: 'directory' },
|
||||
{ name: 'link', kind: 'symlink' }
|
||||
])
|
||||
expect(listings.get(directories[1])).toEqual([])
|
||||
})
|
||||
|
||||
it('reports an absent directory as an empty listing rather than a missing key', () => {
|
||||
const listings = parseWslListEntriesOutput('', ['/a', '/b'])
|
||||
expect([...listings.keys()]).toEqual(['/a', '/b'])
|
||||
expect(listings.get('/a')).toEqual([])
|
||||
})
|
||||
|
||||
it('refuses an unknown record kind instead of guessing', () => {
|
||||
expect(() => parseWslListEntriesOutput(records('X', '0'), ['/a'])).toThrow(
|
||||
WslEnumerationProtocolError
|
||||
)
|
||||
})
|
||||
|
||||
it('refuses an index no argument produced', () => {
|
||||
expect(() => parseWslListEntriesOutput(records('D', '7'), ['/a'])).toThrow(
|
||||
WslEnumerationProtocolError
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
describe('parseWslInspectPathsOutput', () => {
|
||||
it('scales the guest’s whole-second mtime the way WSL discovery does', () => {
|
||||
const output = records('P', '0', 'file', '/home/u/.agents/skills/demo/SKILL.md', '1700000000')
|
||||
const inspections = parseWslInspectPathsOutput(output, ['/host/path'])
|
||||
expect(inspections.get('/host/path')).toEqual({
|
||||
kind: 'file',
|
||||
realpath: '/home/u/.agents/skills/demo/SKILL.md',
|
||||
mtimeMs: 1_700_000_000_000
|
||||
})
|
||||
})
|
||||
|
||||
it('reports an unresolvable realpath and an unreadable mtime as null', () => {
|
||||
const inspections = parseWslInspectPathsOutput(records('P', '0', 'symlink', '', ''), ['/p'])
|
||||
expect(inspections.get('/p')).toEqual({ kind: 'symlink', realpath: null, mtimeMs: null })
|
||||
})
|
||||
|
||||
it('refuses an unknown entry kind', () => {
|
||||
expect(() =>
|
||||
parseWslInspectPathsOutput(records('P', '0', 'socket', '/x', '1'), ['/p'])
|
||||
).toThrow(WslEnumerationProtocolError)
|
||||
})
|
||||
})
|
||||
|
||||
describe('guest scripts', () => {
|
||||
it('classify a symlink before following it, so a broken link is not "missing"', () => {
|
||||
expect(WSL_INSPECT_PATHS_SCRIPT.indexOf('-L "$path"')).toBeLessThan(
|
||||
WSL_INSPECT_PATHS_SCRIPT.indexOf('-d "$path"')
|
||||
)
|
||||
expect(WSL_LIST_ENTRIES_SCRIPT.indexOf('-L "$entry"')).toBeLessThan(
|
||||
WSL_LIST_ENTRIES_SCRIPT.indexOf('-d "$entry"')
|
||||
)
|
||||
})
|
||||
|
||||
it('reads mtime without dereferencing, matching WSL discovery', () => {
|
||||
expect(WSL_INSPECT_PATHS_SCRIPT).toContain(`stat -c '%Y' -- "$path"`)
|
||||
expect(WSL_INSPECT_PATHS_SCRIPT).not.toContain('stat -L')
|
||||
})
|
||||
|
||||
it('takes every path positionally rather than interpolating it into the script', () => {
|
||||
expect(WSL_LIST_ENTRIES_SCRIPT).toContain('for dir in "$@"; do')
|
||||
expect(WSL_INSPECT_PATHS_SCRIPT).toContain('for path in "$@"; do')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,139 @@
|
||||
import type {
|
||||
SkillDirectoryEntry,
|
||||
SkillFilesystemEntryKind,
|
||||
SkillPathInspection
|
||||
} from './skill-install-filesystem'
|
||||
|
||||
/**
|
||||
* Guest-side enumeration for path-based skill deletion.
|
||||
*
|
||||
* Both scripts take every path as a positional argument and emit one
|
||||
* NUL-delimited stream, because the delete plan needs the whole discovery root
|
||||
* set at once and one `wsl.exe` boot per root is the cost this shape avoids.
|
||||
* Records are keyed by argument index rather than by path so the caller rebuilds
|
||||
* its map under the exact strings it passed in, with no path round-tripping.
|
||||
*/
|
||||
export const WSL_LIST_ENTRIES_SCRIPT = [
|
||||
'set -u',
|
||||
'index=0',
|
||||
'for dir in "$@"; do',
|
||||
` printf 'D\\0%s\\0' "$index"`,
|
||||
' index=$((index + 1))',
|
||||
' [ -d "$dir" ] || continue',
|
||||
' for entry in "$dir"/* "$dir"/.*; do',
|
||||
' name=${entry##*/}',
|
||||
' case "$name" in "*"|".*"|"."|"..") continue ;; esac',
|
||||
' if [ -L "$entry" ]; then kind=symlink',
|
||||
' elif [ -d "$entry" ]; then kind=directory',
|
||||
' elif [ -f "$entry" ]; then kind=file',
|
||||
' elif [ -e "$entry" ]; then kind=other',
|
||||
' else continue; fi',
|
||||
` printf 'E\\0%s\\0%s\\0' "$name" "$kind"`,
|
||||
' done',
|
||||
'done'
|
||||
].join('\n')
|
||||
|
||||
export const WSL_INSPECT_PATHS_SCRIPT = [
|
||||
'set -u',
|
||||
'index=0',
|
||||
'for path in "$@"; do',
|
||||
' if [ -L "$path" ]; then kind=symlink',
|
||||
' elif [ -d "$path" ]; then kind=directory',
|
||||
' elif [ -f "$path" ]; then kind=file',
|
||||
' elif [ -e "$path" ]; then kind=other',
|
||||
' else kind=missing; fi',
|
||||
` resolved=$(realpath -- "$path" 2>/dev/null || printf '')`,
|
||||
// Deliberately no `-L`: WSL discovery reads mtime the same way, so the
|
||||
// freshness guard compares like with like on this host.
|
||||
` updated=$(stat -c '%Y' -- "$path" 2>/dev/null || printf '')`,
|
||||
` printf 'P\\0%s\\0%s\\0%s\\0%s\\0' "$index" "$kind" "$resolved" "$updated"`,
|
||||
' index=$((index + 1))',
|
||||
'done'
|
||||
].join('\n')
|
||||
|
||||
export class WslEnumerationProtocolError extends Error {
|
||||
constructor() {
|
||||
super('skill-install-wsl-guest-operation-failed')
|
||||
this.name = 'WslEnumerationProtocolError'
|
||||
}
|
||||
}
|
||||
|
||||
function splitRecords(output: string): string[] {
|
||||
const fields = output.split('\0')
|
||||
// A trailing delimiter always leaves one empty field; anything else is data.
|
||||
if (fields.at(-1) === '') {
|
||||
fields.pop()
|
||||
}
|
||||
return fields
|
||||
}
|
||||
|
||||
function isEntryKind(value: string | undefined): value is SkillFilesystemEntryKind {
|
||||
return (
|
||||
value === 'directory' ||
|
||||
value === 'file' ||
|
||||
value === 'symlink' ||
|
||||
value === 'other' ||
|
||||
value === 'missing'
|
||||
)
|
||||
}
|
||||
|
||||
export function parseWslListEntriesOutput(
|
||||
output: string,
|
||||
directories: readonly string[]
|
||||
): Map<string, SkillDirectoryEntry[]> {
|
||||
const listings = new Map<string, SkillDirectoryEntry[]>(
|
||||
directories.map((directory) => [directory, []])
|
||||
)
|
||||
const fields = splitRecords(output)
|
||||
let current: SkillDirectoryEntry[] | undefined
|
||||
let index = 0
|
||||
while (index < fields.length) {
|
||||
const record = fields[index++]
|
||||
if (record === 'D') {
|
||||
const directory = directories[Number.parseInt(fields[index++] ?? '', 10)]
|
||||
if (directory === undefined) {
|
||||
throw new WslEnumerationProtocolError()
|
||||
}
|
||||
current = listings.get(directory)
|
||||
continue
|
||||
}
|
||||
if (record !== 'E') {
|
||||
throw new WslEnumerationProtocolError()
|
||||
}
|
||||
const name = fields[index++]
|
||||
const kind = fields[index++]
|
||||
if (name === undefined || !isEntryKind(kind)) {
|
||||
throw new WslEnumerationProtocolError()
|
||||
}
|
||||
current?.push({ name, kind })
|
||||
}
|
||||
return listings
|
||||
}
|
||||
|
||||
export function parseWslInspectPathsOutput(
|
||||
output: string,
|
||||
paths: readonly string[]
|
||||
): Map<string, SkillPathInspection> {
|
||||
const inspections = new Map<string, SkillPathInspection>()
|
||||
const fields = splitRecords(output)
|
||||
let index = 0
|
||||
while (index < fields.length) {
|
||||
if (fields[index++] !== 'P') {
|
||||
throw new WslEnumerationProtocolError()
|
||||
}
|
||||
const path = paths[Number.parseInt(fields[index++] ?? '', 10)]
|
||||
const kind = fields[index++]
|
||||
const resolved = fields[index++]
|
||||
const updated = Number.parseInt(fields[index++] ?? '', 10)
|
||||
if (path === undefined || !isEntryKind(kind) || resolved === undefined) {
|
||||
throw new WslEnumerationProtocolError()
|
||||
}
|
||||
inspections.set(path, {
|
||||
kind,
|
||||
realpath: resolved === '' ? null : resolved,
|
||||
// `stat -c %Y` is whole seconds, matching how WSL discovery scales it.
|
||||
mtimeMs: Number.isFinite(updated) ? updated * 1000 : null
|
||||
})
|
||||
}
|
||||
return inspections
|
||||
}
|
||||
@@ -6,7 +6,18 @@ import {
|
||||
observeSkillPackage,
|
||||
type ObservedSkillPackage
|
||||
} from './skill-package-identity'
|
||||
import type { SkillInstallFilesystem, SkillInstalledFileMode } from './skill-install-filesystem'
|
||||
import type {
|
||||
SkillDirectoryEntry,
|
||||
SkillInstallFilesystem,
|
||||
SkillInstalledFileMode,
|
||||
SkillPathInspection
|
||||
} from './skill-install-filesystem'
|
||||
import {
|
||||
parseWslInspectPathsOutput,
|
||||
parseWslListEntriesOutput,
|
||||
WSL_INSPECT_PATHS_SCRIPT,
|
||||
WSL_LIST_ENTRIES_SCRIPT
|
||||
} from './skill-wsl-enumeration-protocol'
|
||||
import { SKILL_INSTALL_PROVIDERS } from '../../shared/skill-install-providers'
|
||||
import type { SkillProviderRootOverrides } from './skill-provider-destinations'
|
||||
import { runWslProcess } from '../wsl/wsl-runner'
|
||||
@@ -14,6 +25,10 @@ import { runWslProcess } from '../wsl/wsl-runner'
|
||||
const BATCH_FILE_COUNT = 8
|
||||
const GUEST_COMMAND_TIMEOUT_MS = 30_000
|
||||
const GUEST_COMMAND_MAX_OUTPUT_BYTES = 64 * 1024
|
||||
// Why far above the default: one batched `listEntries` covers every discovery
|
||||
// root of a WSL home at once, which is orders of magnitude more output than any
|
||||
// install call. Matched to the discovery scanner's own bound.
|
||||
const ENUMERATION_MAX_BUFFER_BYTES = 128 * 1024 * 1024
|
||||
|
||||
function isWindowsBackedGuestPath(path: string): boolean {
|
||||
return /^\/mnt\/[a-z](?:\/|$)/iu.test(path)
|
||||
@@ -149,6 +164,34 @@ export class WslSkillInstallFilesystem implements SkillInstallFilesystem {
|
||||
)
|
||||
}
|
||||
|
||||
async listEntries(directories: readonly string[]): Promise<Map<string, SkillDirectoryEntry[]>> {
|
||||
if (directories.length === 0) {
|
||||
return new Map()
|
||||
}
|
||||
return parseWslListEntriesOutput(
|
||||
await this.runOutput(
|
||||
WSL_LIST_ENTRIES_SCRIPT,
|
||||
directories.map((directory) => this.requireReadable(directory)),
|
||||
ENUMERATION_MAX_BUFFER_BYTES
|
||||
),
|
||||
directories
|
||||
)
|
||||
}
|
||||
|
||||
async inspectPaths(paths: readonly string[]): Promise<Map<string, SkillPathInspection>> {
|
||||
if (paths.length === 0) {
|
||||
return new Map()
|
||||
}
|
||||
return parseWslInspectPathsOutput(
|
||||
await this.runOutput(
|
||||
WSL_INSPECT_PATHS_SCRIPT,
|
||||
paths.map((path) => this.requireReadable(path)),
|
||||
ENUMERATION_MAX_BUFFER_BYTES
|
||||
),
|
||||
paths
|
||||
)
|
||||
}
|
||||
|
||||
private async applyModes(entries: { path: string; mode: string }[]): Promise<void> {
|
||||
for (let offset = 0; offset < entries.length; offset += BATCH_FILE_COUNT) {
|
||||
await this.run(
|
||||
@@ -206,11 +249,30 @@ export class WslSkillInstallFilesystem implements SkillInstallFilesystem {
|
||||
return guestPath
|
||||
}
|
||||
|
||||
/** Enumeration also has to reach an authorized root *itself*, which
|
||||
* `requireAllowed` refuses because it demands strict containment. Read-only,
|
||||
* so admitting the root adds no mutation surface. */
|
||||
private requireReadable(path: string): string {
|
||||
const guestPath = this.toGuestPath(path)
|
||||
const allowed = this.guestAllowedRoots.some((root) => {
|
||||
const child = posix.relative(root, guestPath)
|
||||
return child !== '..' && !child.startsWith('../') && !posix.isAbsolute(child)
|
||||
})
|
||||
if (!allowed) {
|
||||
throw new Error('skill-install-wsl-path-outside-root')
|
||||
}
|
||||
return guestPath
|
||||
}
|
||||
|
||||
private async run(script: string, args: string[]): Promise<void> {
|
||||
await this.runOutput(script, args)
|
||||
}
|
||||
|
||||
private async runOutput(script: string, args: string[]): Promise<string> {
|
||||
private async runOutput(
|
||||
script: string,
|
||||
args: string[],
|
||||
maxOutputBytes = GUEST_COMMAND_MAX_OUTPUT_BYTES
|
||||
): Promise<string> {
|
||||
const result = await runWslProcess({
|
||||
distro: this.distro,
|
||||
loginPath: 'none',
|
||||
@@ -219,7 +281,7 @@ export class WslSkillInstallFilesystem implements SkillInstallFilesystem {
|
||||
shell: 'sh',
|
||||
args,
|
||||
timeoutMs: GUEST_COMMAND_TIMEOUT_MS,
|
||||
maxOutputBytes: GUEST_COMMAND_MAX_OUTPUT_BYTES
|
||||
maxOutputBytes
|
||||
})
|
||||
if (result.code !== 0 || result.timedOut) {
|
||||
throw Object.assign(new Error('skill-install-wsl-guest-operation-failed'), {
|
||||
|
||||
@@ -32,6 +32,12 @@ import type {
|
||||
SkillUpdateStartResult
|
||||
} from '../../shared/skill-freshness'
|
||||
|
||||
import type {
|
||||
SkillDeletePlan,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteResult
|
||||
} from '../../shared/skill-delete-contract'
|
||||
|
||||
export type SkillsApi = {
|
||||
discover: (target?: SkillDiscoveryTarget) => Promise<SkillDiscoveryResult>
|
||||
freshnessInventory: () => Promise<SkillFreshnessInventory>
|
||||
@@ -65,6 +71,8 @@ export type SkillsApi = {
|
||||
input: SkillBundleInstallPreviewInput
|
||||
) => Promise<SkillBundleInstallPreviewOperation>
|
||||
removeInstall: (input: SkillRemoveInput) => Promise<SkillRemoveOperation>
|
||||
previewDelete: (request: SkillDeleteRequest) => Promise<SkillDeletePlan>
|
||||
delete: (request: SkillDeleteRequest) => Promise<SkillDeleteResult>
|
||||
listManagedInstalls: (environmentId?: string) => Promise<ManagedSkillInstallListOperation>
|
||||
getPackage: (packageId: string) => Promise<SkillCloudOperation<SkillCloudPackageDetails>>
|
||||
listOwnedShares: () => Promise<SkillCloudOperation<SkillCloudOwnedShare[]>>
|
||||
|
||||
@@ -3,6 +3,11 @@ import { contextBridge, ipcRenderer, webFrame, webUtils } from 'electron'
|
||||
import { electronAPI } from '@electron-toolkit/preload'
|
||||
import { preloadE2EConfig } from './e2e-config'
|
||||
import { glApi } from './gitlab'
|
||||
import type {
|
||||
SkillDeletePlan,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteResult
|
||||
} from '../shared/skill-delete-contract'
|
||||
import type { AppIdentity } from '../shared/app-identity'
|
||||
import type { MacCapturedDigitRowChord } from '../shared/macos-symbolic-hotkeys'
|
||||
import type { ComputerAwakeStatus } from '../shared/computer-awake-mode'
|
||||
@@ -2628,6 +2633,10 @@ const api = {
|
||||
ipcRenderer.invoke('skills:previewBundleInstall', input),
|
||||
removeInstall: (input: SkillRemoveInput): Promise<SkillRemoveOperation> =>
|
||||
ipcRenderer.invoke('skills:removeInstall', input),
|
||||
previewDelete: (request: SkillDeleteRequest): Promise<SkillDeletePlan> =>
|
||||
ipcRenderer.invoke('skills:previewDelete', request),
|
||||
delete: (request: SkillDeleteRequest): Promise<SkillDeleteResult> =>
|
||||
ipcRenderer.invoke('skills:delete', request),
|
||||
listManagedInstalls: (environmentId?: string): Promise<ManagedSkillInstallListOperation> =>
|
||||
ipcRenderer.invoke('skills:listManagedInstalls', environmentId),
|
||||
getPackage: (packageId: string): Promise<SkillCloudOperation<SkillCloudPackageDetails>> =>
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
import { X } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type { SkillDeleteResult } from '../../../../shared/skill-delete-contract'
|
||||
import { skillDeleteResultLines } from './skill-delete-copy'
|
||||
import { SKILLS_PAGE_COLUMN } from './skills-page-column'
|
||||
|
||||
/**
|
||||
* Persistent rather than a toast: a toast disappears before a user can act on
|
||||
* "3 failed", and the lines are grouped per reason, which the flat four-string
|
||||
* install status component cannot render.
|
||||
*/
|
||||
export function SkillDeleteResultBand({
|
||||
result,
|
||||
onDismiss
|
||||
}: {
|
||||
result: SkillDeleteResult
|
||||
onDismiss: () => void
|
||||
}): React.JSX.Element | null {
|
||||
const lines = skillDeleteResultLines(result.skills)
|
||||
if (lines.length === 0) {
|
||||
return null
|
||||
}
|
||||
return (
|
||||
<div className="shrink-0 border-b border-border bg-muted/40">
|
||||
<div className={cn(SKILLS_PAGE_COLUMN, 'flex items-start gap-2 py-2')} role="status">
|
||||
<ul className="min-w-0 flex-1 space-y-0.5 text-xs text-muted-foreground">
|
||||
{lines.map((line) => (
|
||||
<li key={line.key} className="truncate">
|
||||
{line.label}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
onClick={onDismiss}
|
||||
aria-label={translate('auto.components.skills.SkillDelete.dismissResults', 'Dismiss')}
|
||||
>
|
||||
<X />
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
@@ -1,4 +1,4 @@
|
||||
import { ClipboardCopy, FolderOpen, Share2 } from 'lucide-react'
|
||||
import { ClipboardCopy, FolderOpen, Share2, Trash2 } from 'lucide-react'
|
||||
import { toast } from 'sonner'
|
||||
import { Badge } from '@/components/ui/badge'
|
||||
import { Button } from '@/components/ui/button'
|
||||
@@ -10,6 +10,7 @@ import {
|
||||
DialogHeader,
|
||||
DialogTitle
|
||||
} from '@/components/ui/dialog'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type { DiscoveredSkill } from '../../../../shared/skills'
|
||||
import { sourceKindLabel } from './skill-display-labels'
|
||||
@@ -54,14 +55,22 @@ export function SkillDetailDialog({
|
||||
skill,
|
||||
agentByRootPath,
|
||||
shareable,
|
||||
deletable,
|
||||
deleteDisabledReason,
|
||||
onOpenChange,
|
||||
onShare
|
||||
onShare,
|
||||
onDelete
|
||||
}: {
|
||||
skill: DiscoveredSkill | null
|
||||
agentByRootPath: ReadonlyMap<string, string>
|
||||
shareable: boolean
|
||||
deletable: boolean
|
||||
/** Why a reason string rather than share's bare boolean: delete is valid on a
|
||||
* remote host, so "can't" always has a specific cause worth showing. */
|
||||
deleteDisabledReason: string | null
|
||||
onOpenChange: (open: boolean) => void
|
||||
onShare: () => void
|
||||
onDelete: () => void
|
||||
}): React.JSX.Element | null {
|
||||
if (!skill) {
|
||||
return null
|
||||
@@ -148,6 +157,28 @@ export function SkillDetailDialog({
|
||||
<FolderOpen className="size-3.5" />
|
||||
{translate('auto.components.skills.SkillsPage.dc4c3328ee', 'Reveal file')}
|
||||
</Button>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="pointer-events-auto mr-auto inline-flex">
|
||||
<Button
|
||||
type="button"
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="text-destructive hover:text-destructive"
|
||||
disabled={!deletable}
|
||||
onClick={onDelete}
|
||||
>
|
||||
<Trash2 className="size-3.5" />
|
||||
{translate('auto.components.skills.SkillRow.deleteSkill', 'Delete…')}
|
||||
</Button>
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
{deleteDisabledReason && !deletable ? (
|
||||
<TooltipContent side="top" sideOffset={4}>
|
||||
{deleteDisabledReason}
|
||||
</TooltipContent>
|
||||
) : null}
|
||||
</Tooltip>
|
||||
<Button type="button" size="sm" disabled={!shareable} onClick={onShare}>
|
||||
<Share2 className="size-3.5" />
|
||||
{translate('auto.components.skills.SkillCard.d25a1b8ae6', 'Share skill')}
|
||||
|
||||
@@ -1,19 +1,13 @@
|
||||
import { useId, useRef } from 'react'
|
||||
import { ClipboardCopy, FolderOpen, Info, MoreHorizontal, Share2 } from 'lucide-react'
|
||||
import { ClipboardCopy, FolderOpen, Info, MoreHorizontal, Share2, Trash2 } 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 {
|
||||
ContextMenu,
|
||||
ContextMenuContent,
|
||||
ContextMenuItem,
|
||||
ContextMenuTrigger
|
||||
} from '@/components/ui/context-menu'
|
||||
import { ContextMenu, ContextMenuContent, ContextMenuTrigger } from '@/components/ui/context-menu'
|
||||
import {
|
||||
DropdownMenu,
|
||||
DropdownMenuContent,
|
||||
DropdownMenuItem,
|
||||
DropdownMenuTrigger
|
||||
} from '@/components/ui/dropdown-menu'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
@@ -21,6 +15,11 @@ import { cn } from '@/lib/utils'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type { DiscoveredSkill, SkillProvider } from '../../../../shared/skills'
|
||||
import { sourceKindLabel } from './skill-display-labels'
|
||||
import {
|
||||
SkillRowContextActions,
|
||||
SkillRowDropdownActions,
|
||||
type SkillRowAction
|
||||
} from './SkillRowActions'
|
||||
|
||||
const providerLabels: Record<SkillProvider, string> = {
|
||||
codex: 'Codex',
|
||||
@@ -39,25 +38,21 @@ function formatUpdatedAt(value: number | null): string {
|
||||
: translate('auto.components.skills.SkillRow.updatedUnknown', 'No date')
|
||||
}
|
||||
|
||||
type SkillRowAction = {
|
||||
key: string
|
||||
label: string
|
||||
icon: React.JSX.Element
|
||||
disabled?: boolean
|
||||
onSelect: () => void
|
||||
}
|
||||
|
||||
export function SkillRow({
|
||||
skill,
|
||||
selectionMode,
|
||||
selected,
|
||||
selectable,
|
||||
shareable,
|
||||
deletable,
|
||||
deleteDisabledReason,
|
||||
disabledLabel,
|
||||
disabledReason,
|
||||
focusable,
|
||||
onOpenDetail,
|
||||
onSelectionChange,
|
||||
onShare,
|
||||
onDelete,
|
||||
onFocus,
|
||||
onKeyDown
|
||||
}: {
|
||||
@@ -66,6 +61,12 @@ export function SkillRow({
|
||||
selected: boolean
|
||||
selectable: boolean
|
||||
shareable: boolean
|
||||
deletable: boolean
|
||||
/** Shown on the `Delete…` item itself, so it reads outside selection mode too. */
|
||||
deleteDisabledReason: string | null
|
||||
/** Mode-dependent: a delete-ineligible row and a share-ineligible row need
|
||||
* different copy in the same visual slot. */
|
||||
disabledLabel: string
|
||||
/** Row-specific explanation only; page-wide causes are stated once above the
|
||||
* list instead of once per row. */
|
||||
disabledReason: string | null
|
||||
@@ -73,6 +74,7 @@ export function SkillRow({
|
||||
onOpenDetail: () => void
|
||||
onSelectionChange: (selected: boolean, range: boolean) => void
|
||||
onShare: () => void
|
||||
onDelete: () => void
|
||||
onFocus: () => void
|
||||
onKeyDown: (event: React.KeyboardEvent<HTMLDivElement>) => void
|
||||
}): React.JSX.Element {
|
||||
@@ -122,6 +124,15 @@ export function SkillRow({
|
||||
label: translate('auto.components.skills.SkillRow.copyPath', 'Copy path'),
|
||||
icon: <ClipboardCopy />,
|
||||
onSelect: () => void copyPath()
|
||||
},
|
||||
{
|
||||
key: 'delete',
|
||||
label: translate('auto.components.skills.SkillRow.deleteSkill', 'Delete…'),
|
||||
icon: <Trash2 />,
|
||||
disabled: !deletable,
|
||||
destructive: true,
|
||||
...(deleteDisabledReason ? { disabledReason: deleteDisabledReason } : {}),
|
||||
onSelect: onDelete
|
||||
}
|
||||
]
|
||||
|
||||
@@ -197,7 +208,7 @@ export function SkillRow({
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="shrink-0 text-[11px] text-muted-foreground">
|
||||
{translate('auto.components.skills.SkillRow.notShareable', 'Not shareable')}
|
||||
{disabledLabel}
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="top" sideOffset={4}>
|
||||
@@ -247,28 +258,14 @@ export function SkillRow({
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
<DropdownMenuContent align="end">
|
||||
{actions.map((action) => (
|
||||
<DropdownMenuItem
|
||||
key={action.key}
|
||||
disabled={action.disabled}
|
||||
onSelect={action.onSelect}
|
||||
>
|
||||
{action.icon}
|
||||
{action.label}
|
||||
</DropdownMenuItem>
|
||||
))}
|
||||
<SkillRowDropdownActions actions={actions} />
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
</div>
|
||||
</div>
|
||||
</ContextMenuTrigger>
|
||||
<ContextMenuContent>
|
||||
{actions.map((action) => (
|
||||
<ContextMenuItem key={action.key} disabled={action.disabled} onSelect={action.onSelect}>
|
||||
{action.icon}
|
||||
{action.label}
|
||||
</ContextMenuItem>
|
||||
))}
|
||||
<SkillRowContextActions actions={actions} />
|
||||
</ContextMenuContent>
|
||||
</ContextMenu>
|
||||
)
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
import React from 'react'
|
||||
import { ContextMenuItem } from '@/components/ui/context-menu'
|
||||
import { DropdownMenuItem } from '@/components/ui/dropdown-menu'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
|
||||
export type SkillRowAction = {
|
||||
key: string
|
||||
label: string
|
||||
icon: React.JSX.Element
|
||||
disabled?: boolean
|
||||
/** Why an item-level reason: the existing row badge is shown only in selection
|
||||
* mode, and a disabled `Delete…` has to explain itself outside it too. */
|
||||
disabledReason?: string
|
||||
destructive?: boolean
|
||||
onSelect: () => void
|
||||
}
|
||||
|
||||
/**
|
||||
* Radix disables pointer events on a disabled item, so the tooltip has to fire
|
||||
* on a wrapper that keeps them.
|
||||
*/
|
||||
function withReason(action: SkillRowAction, item: React.JSX.Element): React.JSX.Element {
|
||||
if (!action.disabled || !action.disabledReason) {
|
||||
return item
|
||||
}
|
||||
return (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="pointer-events-auto block">{item}</span>
|
||||
</TooltipTrigger>
|
||||
<TooltipContent side="left" sideOffset={4}>
|
||||
{action.disabledReason}
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
)
|
||||
}
|
||||
|
||||
export function SkillRowDropdownActions({
|
||||
actions
|
||||
}: {
|
||||
actions: readonly SkillRowAction[]
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<>
|
||||
{actions.map((action) => (
|
||||
<React.Fragment key={action.key}>
|
||||
{withReason(
|
||||
action,
|
||||
<DropdownMenuItem
|
||||
disabled={action.disabled}
|
||||
variant={action.destructive ? 'destructive' : 'default'}
|
||||
onSelect={action.onSelect}
|
||||
>
|
||||
{action.icon}
|
||||
{action.label}
|
||||
</DropdownMenuItem>
|
||||
)}
|
||||
</React.Fragment>
|
||||
))}
|
||||
</>
|
||||
)
|
||||
}
|
||||
|
||||
export function SkillRowContextActions({
|
||||
actions
|
||||
}: {
|
||||
actions: readonly SkillRowAction[]
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<>
|
||||
{actions.map((action) => (
|
||||
<React.Fragment key={action.key}>
|
||||
{withReason(
|
||||
action,
|
||||
<ContextMenuItem
|
||||
disabled={action.disabled}
|
||||
variant={action.destructive ? 'destructive' : 'default'}
|
||||
onSelect={action.onSelect}
|
||||
>
|
||||
{action.icon}
|
||||
{action.label}
|
||||
</ContextMenuItem>
|
||||
)}
|
||||
</React.Fragment>
|
||||
))}
|
||||
</>
|
||||
)
|
||||
}
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
selectedShareSkillNameKeys,
|
||||
skillShareEligibilityReason
|
||||
} from './skill-share-selection'
|
||||
import { isSkillDeleteEligible, skillDeleteEligibilityReason } from './skill-delete-selection'
|
||||
|
||||
const OPTION_SELECTOR = '[role="option"]'
|
||||
|
||||
@@ -28,19 +29,28 @@ export function SkillsList({
|
||||
agentByRootPath,
|
||||
selectedIds,
|
||||
selectionMode,
|
||||
deleteSupported,
|
||||
deleteUnsupportedReason,
|
||||
onSelectedChange,
|
||||
onSelectResults,
|
||||
onShare
|
||||
onShare,
|
||||
onDelete
|
||||
}: {
|
||||
skills: readonly DiscoveredSkill[]
|
||||
allSkills: readonly DiscoveredSkill[]
|
||||
local: boolean
|
||||
agentByRootPath: ReadonlyMap<string, string>
|
||||
selectedIds: ReadonlySet<string>
|
||||
selectionMode: boolean
|
||||
selectionMode: 'share' | 'delete' | null
|
||||
/** False while the runtime target is unresolved or the host predates
|
||||
* `skills.delete.v1`; the action disables with a reason rather than issuing
|
||||
* an RPC nothing answers. */
|
||||
deleteSupported: boolean
|
||||
deleteUnsupportedReason: string | null
|
||||
onSelectedChange: (skillId: string, selected: boolean) => void
|
||||
onSelectResults: (results: readonly DiscoveredSkill[]) => void
|
||||
onShare: (skill: DiscoveredSkill) => void
|
||||
onDelete: (skill: DiscoveredSkill) => void
|
||||
}): React.JSX.Element {
|
||||
const listRef = useRef<HTMLDivElement>(null)
|
||||
// Why: an id survives filtering; an index would silently anchor a shift-click
|
||||
@@ -53,6 +63,9 @@ export function SkillsList({
|
||||
|
||||
const isMac = navigator.userAgent.includes('Mac')
|
||||
|
||||
const deleteReasonFor = (skill: DiscoveredSkill): string | null =>
|
||||
deleteSupported ? skillDeleteEligibilityReason(skill) : deleteUnsupportedReason
|
||||
|
||||
const handleSelection = (index: number, selected: boolean, range: boolean): void => {
|
||||
const skill = skills[index]
|
||||
const anchorIndex = skills.findIndex((candidate) => candidate.id === anchorIdRef.current)
|
||||
@@ -95,6 +108,8 @@ export function SkillsList({
|
||||
skill={detailSkill}
|
||||
agentByRootPath={agentByRootPath}
|
||||
shareable={detailSkill ? isSkillShareEligible(detailSkill, local) : false}
|
||||
deletable={detailSkill ? deleteSupported && isSkillDeleteEligible(detailSkill) : false}
|
||||
deleteDisabledReason={detailSkill ? deleteReasonFor(detailSkill) : null}
|
||||
onOpenChange={(open) => {
|
||||
if (!open) {
|
||||
setDetailSkill(null)
|
||||
@@ -106,36 +121,57 @@ export function SkillsList({
|
||||
setDetailSkill(null)
|
||||
}
|
||||
}}
|
||||
onDelete={() => {
|
||||
if (detailSkill) {
|
||||
onDelete(detailSkill)
|
||||
setDetailSkill(null)
|
||||
}
|
||||
}}
|
||||
/>
|
||||
<div
|
||||
ref={listRef}
|
||||
role="listbox"
|
||||
aria-multiselectable={selectionMode || undefined}
|
||||
aria-multiselectable={selectionMode !== null || undefined}
|
||||
aria-label={translate('auto.components.skills.SkillsList.listLabel', 'Skills')}
|
||||
aria-orientation="vertical"
|
||||
>
|
||||
{skills.map((skill, index) => {
|
||||
const duplicateNameSelected =
|
||||
!selectedIds.has(skill.id) && selectedNames.has(skill.name.toLocaleLowerCase('en-US'))
|
||||
const eligible = isSkillShareEligible(skill, local)
|
||||
const shareEligible = isSkillShareEligible(skill, local)
|
||||
const deleteEligible = isSkillDeleteEligible(skill)
|
||||
const deleting = selectionMode === 'delete'
|
||||
return (
|
||||
<SkillRow
|
||||
key={skill.id}
|
||||
skill={skill}
|
||||
selectionMode={selectionMode}
|
||||
selectionMode={selectionMode !== null}
|
||||
selected={selectedIds.has(skill.id)}
|
||||
selectable={eligible && !duplicateNameSelected}
|
||||
shareable={eligible}
|
||||
// Why: on a remote runtime every row shares one cause, so the page
|
||||
// states it once above the list instead of on all 114 rows.
|
||||
selectable={deleting ? deleteEligible : shareEligible && !duplicateNameSelected}
|
||||
shareable={shareEligible}
|
||||
deletable={deleteSupported && deleteEligible}
|
||||
deleteDisabledReason={deleteReasonFor(skill)}
|
||||
disabledLabel={
|
||||
deleting
|
||||
? translate('auto.components.skills.SkillRow.notDeletable', 'Not deletable')
|
||||
: translate('auto.components.skills.SkillRow.notShareable', 'Not shareable')
|
||||
}
|
||||
// Why: on a remote runtime every row shares one *share* cause, so
|
||||
// the page states it once above the list instead of on all 114
|
||||
// rows. Delete is valid on a remote host, so it keeps its own.
|
||||
disabledReason={
|
||||
local ? skillShareEligibilityReason(skill, local, duplicateNameSelected) : null
|
||||
deleting
|
||||
? skillDeleteEligibilityReason(skill)
|
||||
: local
|
||||
? skillShareEligibilityReason(skill, local, duplicateNameSelected)
|
||||
: null
|
||||
}
|
||||
focusable={focusTargetId === skill.id}
|
||||
onFocus={() => setFocusedId(skill.id)}
|
||||
onOpenDetail={() => setDetailSkill(skill)}
|
||||
onSelectionChange={(selected, range) => handleSelection(index, selected, range)}
|
||||
onShare={() => onShare(skill)}
|
||||
onDelete={() => onDelete(skill)}
|
||||
onKeyDown={onListKeyDown}
|
||||
/>
|
||||
)
|
||||
|
||||
@@ -9,6 +9,7 @@ import type { DiscoveredSkill, SkillDiscoveryResult } from '../../../../shared/s
|
||||
import { createCompatibleRuntimeStatusResponseIfNeeded } from '@/runtime/runtime-compatibility-test-fixture'
|
||||
import { clearRuntimeCompatibilityCacheForTests } from '@/runtime/runtime-rpc-client'
|
||||
import { TooltipProvider } from '@/components/ui/tooltip'
|
||||
import { ConfirmationDialogProvider } from '@/components/confirmation-dialog'
|
||||
import { useAppStore } from '@/store'
|
||||
import SkillsPage from './SkillsPage'
|
||||
|
||||
@@ -65,7 +66,9 @@ async function renderPage(): Promise<void> {
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
<TooltipProvider>
|
||||
<SkillsPage />
|
||||
<ConfirmationDialogProvider>
|
||||
<SkillsPage />
|
||||
</ConfirmationDialogProvider>
|
||||
</TooltipProvider>
|
||||
)
|
||||
})
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
|
||||
import { Share2, Trash2 } from 'lucide-react'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { useAppStore } from '@/store'
|
||||
import { discoverSkillsForRuntimeTarget } from '@/runtime/runtime-skills-client'
|
||||
@@ -28,13 +29,25 @@ import { SkillSharedLinksView } from './SkillSharedLinksView'
|
||||
import { useOwnedSkillShares } from './use-owned-skill-shares'
|
||||
import type { SkillsPageView } from './skills-page-view'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { INSTALLED_AGENT_SKILLS_CHANGED_EVENT } from '@/hooks/installed-agent-skills-change-event'
|
||||
import {
|
||||
INSTALLED_AGENT_SKILLS_CHANGED_EVENT,
|
||||
INSTALLED_AGENT_SKILLS_REFRESHED_EVENT
|
||||
} from '@/hooks/installed-agent-skills-change-event'
|
||||
import {
|
||||
addShareableSkillResults,
|
||||
eligibleShareSkillCount,
|
||||
retainedShareableSkillSelection,
|
||||
updatedSkillSelection
|
||||
} from './skill-share-selection'
|
||||
import {
|
||||
addDeletableSkillResults,
|
||||
eligibleDeleteSkillCount,
|
||||
retainedDeletableSkillSelection
|
||||
} from './skill-delete-selection'
|
||||
import { skillDeleteActionLabel } from './skill-delete-copy'
|
||||
import { shareSelectionActionLabel } from './skill-display-labels'
|
||||
import { SkillDeleteResultBand } from './SkillDeleteResultBand'
|
||||
import { useSkillDeleteFlow } from './use-skill-delete-flow'
|
||||
|
||||
const EMPTY_SKILLS: DiscoveredSkill[] = []
|
||||
const NO_FILTERS: SkillsFilterState = {
|
||||
@@ -55,7 +68,7 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
const [loading, setLoading] = useState(true)
|
||||
const [scanError, setScanError] = useState<string | null>(null)
|
||||
const [shareSkills, setShareSkills] = useState<DiscoveredSkill[]>([])
|
||||
const [selectingShare, setSelectingShare] = useState(false)
|
||||
const [selectionMode, setSelectionMode] = useState<'share' | 'delete' | null>(null)
|
||||
const [selectedSkillIds, setSelectedSkillIds] = useState<Set<string>>(() => new Set())
|
||||
const [installOpen, setInstallOpen] = useState(false)
|
||||
const [installLink, setInstallLink] = useState('')
|
||||
@@ -65,44 +78,59 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
const ownedShares = useOwnedSkillShares()
|
||||
const mountedRef = useMountedRef()
|
||||
const scanGenerationRef = useRef(0)
|
||||
// Why a ref: `loadSkills` must not re-identify (and re-scan) when the user
|
||||
// merely switches selection mode. Switching modes clears the selection anyway,
|
||||
// so a one-render lag here cannot retain the wrong rows.
|
||||
const selectionModeRef = useRef(selectionMode)
|
||||
useEffect(() => {
|
||||
selectionModeRef.current = selectionMode
|
||||
}, [selectionMode])
|
||||
|
||||
const loadSkills = useCallback(async (): Promise<void> => {
|
||||
setLoading(true)
|
||||
// Why: a cold local scan walks every skill root, so switching runtimes can
|
||||
// land a stale result after a newer one. Only the newest scan may write.
|
||||
const scanGeneration = ++scanGenerationRef.current
|
||||
const isCurrentScan = (): boolean =>
|
||||
mountedRef.current && scanGeneration === scanGenerationRef.current
|
||||
if (!runtimeTarget) {
|
||||
// Why: keep scanning until the owning runtime is known, rather than
|
||||
// showing the client's skills to someone whose skills live remotely.
|
||||
return
|
||||
}
|
||||
try {
|
||||
const nextResult = await discoverSkillsForRuntimeTarget(runtimeTarget)
|
||||
const local = runtimeTarget.kind === 'local'
|
||||
if (isCurrentScan()) {
|
||||
setResult(nextResult)
|
||||
setScanError(null)
|
||||
setSelectedSkillIds((current) =>
|
||||
retainedShareableSkillSelection(current, nextResult.skills, local)
|
||||
const loadSkills = useCallback(
|
||||
async (refresh = false): Promise<void> => {
|
||||
setLoading(true)
|
||||
// Why: a cold local scan walks every skill root, so switching runtimes can
|
||||
// land a stale result after a newer one. Only the newest scan may write.
|
||||
const scanGeneration = ++scanGenerationRef.current
|
||||
const isCurrentScan = (): boolean =>
|
||||
mountedRef.current && scanGeneration === scanGenerationRef.current
|
||||
if (!runtimeTarget) {
|
||||
// Why: keep scanning until the owning runtime is known, rather than
|
||||
// showing the client's skills to someone whose skills live remotely.
|
||||
return
|
||||
}
|
||||
try {
|
||||
const nextResult = await discoverSkillsForRuntimeTarget(
|
||||
runtimeTarget,
|
||||
refresh ? { refresh: true } : undefined
|
||||
)
|
||||
const local = runtimeTarget.kind === 'local'
|
||||
if (isCurrentScan()) {
|
||||
setResult(nextResult)
|
||||
setScanError(null)
|
||||
setSelectedSkillIds((current) =>
|
||||
selectionModeRef.current === 'delete'
|
||||
? retainedDeletableSkillSelection(current, nextResult.skills)
|
||||
: retainedShareableSkillSelection(current, nextResult.skills, local)
|
||||
)
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Failed to discover skills:', error)
|
||||
if (isCurrentScan()) {
|
||||
// Why: a failed scan needs to stay on screen with a retry — a toast
|
||||
// disappears before the user can act on it.
|
||||
setScanError(
|
||||
translate('auto.components.skills.SkillsPage.ea72d6185b', 'Could not scan skills')
|
||||
)
|
||||
}
|
||||
} finally {
|
||||
if (isCurrentScan()) {
|
||||
setLoading(false)
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Failed to discover skills:', error)
|
||||
if (isCurrentScan()) {
|
||||
// Why: a failed scan needs to stay on screen with a retry — a toast
|
||||
// disappears before the user can act on it.
|
||||
setScanError(
|
||||
translate('auto.components.skills.SkillsPage.ea72d6185b', 'Could not scan skills')
|
||||
)
|
||||
}
|
||||
} finally {
|
||||
if (isCurrentScan()) {
|
||||
setLoading(false)
|
||||
}
|
||||
}
|
||||
}, [mountedRef, runtimeTarget])
|
||||
},
|
||||
[mountedRef, runtimeTarget]
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
void loadSkills()
|
||||
@@ -132,10 +160,19 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
}, [clearPendingSkillShare, pendingSkillShareId])
|
||||
|
||||
const exitSelection = useCallback((): void => {
|
||||
setSelectingShare(false)
|
||||
setSelectionMode(null)
|
||||
setSelectedSkillIds(new Set())
|
||||
}, [])
|
||||
|
||||
const deleteFlow = useSkillDeleteFlow(runtimeTarget, hostLabel, () => {
|
||||
exitSelection()
|
||||
// Why an explicit refresh instead of firing the change event: this page
|
||||
// would then run a second, non-refresh scan of the host it just refreshed.
|
||||
// Other subscribers (settings badges, pickers) still hear the event.
|
||||
void loadSkills(true)
|
||||
window.dispatchEvent(new Event(INSTALLED_AGENT_SKILLS_REFRESHED_EVENT))
|
||||
})
|
||||
|
||||
// Why: the search box is shared between the two lists, but a link query means
|
||||
// nothing against skill names and vice versa.
|
||||
const exitSharedLinks = useCallback((): void => {
|
||||
@@ -188,7 +225,7 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
event.preventDefault()
|
||||
// Why: leaving the page would silently discard a selection that can hold
|
||||
// dozens of skills, so Escape backs out of the mode first.
|
||||
if (selectingShare) {
|
||||
if (selectionMode) {
|
||||
exitSelection()
|
||||
return
|
||||
}
|
||||
@@ -205,7 +242,7 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
// keeps page-level back navigation reliable when no overlay is active.
|
||||
window.addEventListener('keydown', handleKeyDown, { capture: true })
|
||||
return () => window.removeEventListener('keydown', handleKeyDown, { capture: true })
|
||||
}, [closeSkillsPage, exitSelection, exitSharedLinks, selectingShare, view])
|
||||
}, [closeSkillsPage, exitSelection, exitSharedLinks, selectionMode, view])
|
||||
|
||||
const skills = result?.skills ?? EMPTY_SKILLS
|
||||
const local = runtimeTarget?.kind === 'local'
|
||||
@@ -217,7 +254,18 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
)
|
||||
const sourceCounts = useMemo(() => countSkillsBySource(skills), [skills])
|
||||
const sourceEntries = useMemo(() => summarizeSkillSources(result), [result])
|
||||
const eligibleCount = eligibleShareSkillCount(visibleSkills, local)
|
||||
const eligibleCount =
|
||||
selectionMode === 'delete'
|
||||
? eligibleDeleteSkillCount(visibleSkills)
|
||||
: eligibleShareSkillCount(visibleSkills, local)
|
||||
const deleting = selectionMode === 'delete'
|
||||
const addSelected = (
|
||||
current: ReadonlySet<string>,
|
||||
results: readonly DiscoveredSkill[]
|
||||
): Set<string> =>
|
||||
deleting
|
||||
? addDeletableSkillResults(current, skills, results)
|
||||
: addShareableSkillResults(current, skills, results, local)
|
||||
const openInstallDialog = (): void => {
|
||||
setInstallLink('')
|
||||
setInstallOpen(true)
|
||||
@@ -225,18 +273,47 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
|
||||
return (
|
||||
<main className="flex min-h-0 flex-1 flex-col bg-background">
|
||||
{selectingShare ? (
|
||||
{selectionMode ? (
|
||||
<SkillsSelectionHeader
|
||||
selectedCount={selectedSkillIds.size}
|
||||
eligibleCount={eligibleCount}
|
||||
onSelectAll={() =>
|
||||
setSelectedSkillIds((current) =>
|
||||
addShareableSkillResults(current, skills, visibleSkills, local)
|
||||
title={
|
||||
deleting
|
||||
? translate(
|
||||
'auto.components.skills.SkillsSelectionHeader.deleteTitle',
|
||||
'Select skills to delete'
|
||||
)
|
||||
: translate(
|
||||
'auto.components.skills.SkillsSelectionHeader.title',
|
||||
'Select skills to share'
|
||||
)
|
||||
}
|
||||
icon={
|
||||
deleting ? (
|
||||
<Trash2 className="size-4 shrink-0 text-muted-foreground" />
|
||||
) : (
|
||||
<Share2 className="size-4 shrink-0 text-muted-foreground" />
|
||||
)
|
||||
}
|
||||
actionIcon={deleting ? <Trash2 className="size-3.5" /> : <Share2 className="size-3.5" />}
|
||||
actionLabel={
|
||||
deleting
|
||||
? skillDeleteActionLabel(selectedSkillIds.size)
|
||||
: shareSelectionActionLabel(selectedSkillIds.size)
|
||||
}
|
||||
destructive={deleting}
|
||||
busy={deleting && deleteFlow.running}
|
||||
selectedCount={selectedSkillIds.size}
|
||||
eligibleCount={eligibleCount}
|
||||
onSelectAll={() => setSelectedSkillIds((current) => addSelected(current, visibleSkills))}
|
||||
onClear={() => setSelectedSkillIds(new Set())}
|
||||
onCancel={exitSelection}
|
||||
onShare={() => setShareSkills(skills.filter((skill) => selectedSkillIds.has(skill.id)))}
|
||||
onSubmit={() => {
|
||||
const selected = skills.filter((skill) => selectedSkillIds.has(skill.id))
|
||||
if (deleting) {
|
||||
void deleteFlow.requestDelete(selected)
|
||||
return
|
||||
}
|
||||
setShareSkills(selected)
|
||||
}}
|
||||
/>
|
||||
) : (
|
||||
<SkillsPageHeader
|
||||
@@ -246,7 +323,13 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
hostLabel={hostLabel}
|
||||
onClose={closeSkillsPage}
|
||||
onStartShare={() => {
|
||||
setSelectingShare(true)
|
||||
setSelectionMode('share')
|
||||
setSelectedSkillIds(new Set())
|
||||
}}
|
||||
deleteSupported={deleteFlow.supported}
|
||||
deleteUnsupportedReason={deleteFlow.unsupportedReason}
|
||||
onStartDelete={() => {
|
||||
setSelectionMode('delete')
|
||||
setSelectedSkillIds(new Set())
|
||||
}}
|
||||
onInstallFromLink={openInstallDialog}
|
||||
@@ -274,6 +357,9 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
onRetry={() => void loadSkills()}
|
||||
/>
|
||||
) : null}
|
||||
{deleteFlow.result ? (
|
||||
<SkillDeleteResultBand result={deleteFlow.result} onDismiss={deleteFlow.dismissResult} />
|
||||
) : null}
|
||||
|
||||
<section className="scrollbar-sleek min-h-0 flex-1 overflow-y-auto">
|
||||
<div className={cn(SKILLS_PAGE_COLUMN, 'py-2')} data-skills-page-list="true">
|
||||
@@ -291,18 +377,19 @@ export default function SkillsPage(): React.JSX.Element {
|
||||
local={local}
|
||||
agentByRootPath={agentByRootPath}
|
||||
selectedIds={selectedSkillIds}
|
||||
selectionMode={selectingShare}
|
||||
selectionMode={selectionMode}
|
||||
deleteSupported={deleteFlow.supported}
|
||||
deleteUnsupportedReason={deleteFlow.unsupportedReason}
|
||||
onSelectedChange={(skillId, selected) =>
|
||||
setSelectedSkillIds((current) =>
|
||||
updatedSkillSelection(current, skillId, selected)
|
||||
)
|
||||
}
|
||||
onSelectResults={(results) =>
|
||||
setSelectedSkillIds((current) =>
|
||||
addShareableSkillResults(current, skills, results, local)
|
||||
)
|
||||
setSelectedSkillIds((current) => addSelected(current, results))
|
||||
}
|
||||
onShare={(skill) => setShareSkills([skill])}
|
||||
onDelete={(skill) => void deleteFlow.requestDelete([skill])}
|
||||
/>
|
||||
) : skills.length > 0 ? (
|
||||
<SkillsNoMatchesState onClearFilters={() => setFilters(NO_FILTERS)} />
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { BookOpen, Download, History, Link2, MoreHorizontal, Share2, X } from 'lucide-react'
|
||||
import { BookOpen, Download, History, Link2, MoreHorizontal, Share2, Trash2, X } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import {
|
||||
DropdownMenu,
|
||||
@@ -21,6 +21,9 @@ export function SkillsPageHeader({
|
||||
hostLabel,
|
||||
onClose,
|
||||
onStartShare,
|
||||
deleteSupported,
|
||||
deleteUnsupportedReason,
|
||||
onStartDelete,
|
||||
onInstallFromLink,
|
||||
onManageInstalls,
|
||||
onOpenSharedLinks
|
||||
@@ -31,6 +34,12 @@ export function SkillsPageHeader({
|
||||
hostLabel: string | null
|
||||
onClose: () => void
|
||||
onStartShare: () => void
|
||||
/** False while the target is unresolved or the host predates the delete
|
||||
* capability, so the entry disables with a reason rather than routing a
|
||||
* request nothing on that host answers. */
|
||||
deleteSupported: boolean
|
||||
deleteUnsupportedReason: string | null
|
||||
onStartDelete: () => void
|
||||
onInstallFromLink: () => void
|
||||
onManageInstalls: () => void
|
||||
onOpenSharedLinks: () => void
|
||||
@@ -113,6 +122,25 @@ export function SkillsPageHeader({
|
||||
<Link2 />
|
||||
{translate('auto.components.skills.SkillsPage.sharedLinks', 'Shared links')}
|
||||
</DropdownMenuItem>
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<span className="pointer-events-auto block">
|
||||
<DropdownMenuItem
|
||||
variant="destructive"
|
||||
disabled={!deleteSupported}
|
||||
onSelect={onStartDelete}
|
||||
>
|
||||
<Trash2 />
|
||||
{translate('auto.components.skills.SkillsPage.deleteSkills', 'Delete skills…')}
|
||||
</DropdownMenuItem>
|
||||
</span>
|
||||
</TooltipTrigger>
|
||||
{deleteSupported || !deleteUnsupportedReason ? null : (
|
||||
<TooltipContent side="left" sideOffset={4}>
|
||||
{deleteUnsupportedReason}
|
||||
</TooltipContent>
|
||||
)}
|
||||
</Tooltip>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
</div>
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
import { Share2, X } from 'lucide-react'
|
||||
import { X } from 'lucide-react'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { selectedCountLabel, shareSelectionActionLabel } from './skill-display-labels'
|
||||
import { selectedCountLabel } from './skill-display-labels'
|
||||
import { SKILLS_PAGE_COLUMN } from './skills-page-column'
|
||||
import { SKILLS_SUBTITLE_ACTION_CLASS } from './skills-subtitle-action'
|
||||
|
||||
@@ -13,19 +13,34 @@ import { SKILLS_SUBTITLE_ACTION_CLASS } from './skills-subtitle-action'
|
||||
* Esc means — leave the mode, not the page.
|
||||
*/
|
||||
export function SkillsSelectionHeader({
|
||||
title,
|
||||
icon,
|
||||
actionIcon,
|
||||
actionLabel,
|
||||
destructive = false,
|
||||
busy = false,
|
||||
selectedCount,
|
||||
eligibleCount,
|
||||
onSelectAll,
|
||||
onClear,
|
||||
onCancel,
|
||||
onShare
|
||||
onSubmit
|
||||
}: {
|
||||
/** Copy is passed in already translated: this header's own key namespace
|
||||
* cannot own strings for two unrelated modes. */
|
||||
title: string
|
||||
icon: React.JSX.Element
|
||||
actionIcon: React.JSX.Element
|
||||
actionLabel: string
|
||||
destructive?: boolean
|
||||
/** Keeps a slow remote submit from being fired twice. */
|
||||
busy?: boolean
|
||||
selectedCount: number
|
||||
eligibleCount: number
|
||||
onSelectAll: () => void
|
||||
onClear: () => void
|
||||
onCancel: () => void
|
||||
onShare: () => void
|
||||
onSubmit: () => void
|
||||
}): React.JSX.Element {
|
||||
return (
|
||||
<header className="shrink-0 border-b border-border">
|
||||
@@ -53,14 +68,9 @@ export function SkillsSelectionHeader({
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
<div className="mx-1 h-5 w-px bg-border/50" aria-hidden />
|
||||
<Share2 className="size-4 shrink-0 text-muted-foreground" />
|
||||
{icon}
|
||||
<div className="min-w-0 flex-1">
|
||||
<h1 className="truncate text-sm font-semibold">
|
||||
{translate(
|
||||
'auto.components.skills.SkillsSelectionHeader.title',
|
||||
'Select skills to share'
|
||||
)}
|
||||
</h1>
|
||||
<h1 className="truncate text-sm font-semibold">{title}</h1>
|
||||
<div className="flex min-w-0 items-center gap-1.5 text-xs text-muted-foreground">
|
||||
<span className="shrink-0" role="status" aria-live="polite">
|
||||
{selectedCountLabel(selectedCount)}
|
||||
@@ -91,9 +101,15 @@ export function SkillsSelectionHeader({
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
<Button type="button" size="sm" disabled={selectedCount === 0} onClick={onShare}>
|
||||
<Share2 className="size-3.5" />
|
||||
{shareSelectionActionLabel(selectedCount)}
|
||||
<Button
|
||||
type="button"
|
||||
size="sm"
|
||||
variant={destructive ? 'destructive' : 'default'}
|
||||
disabled={selectedCount === 0 || busy}
|
||||
onClick={onSubmit}
|
||||
>
|
||||
{actionIcon}
|
||||
{actionLabel}
|
||||
</Button>
|
||||
</div>
|
||||
</header>
|
||||
|
||||
@@ -0,0 +1,146 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { SkillDeletePlan } from '../../../../shared/skill-delete-contract'
|
||||
import {
|
||||
skillDeleteBlockedLines,
|
||||
skillDeletePlacementSummary,
|
||||
skillDeleteResultLines,
|
||||
skillDeleteRetainedSourceLines
|
||||
} from './skill-delete-copy'
|
||||
|
||||
function plan(skills: SkillDeletePlan['skills']): SkillDeletePlan {
|
||||
return { operationId: 'op', skills }
|
||||
}
|
||||
|
||||
describe('skillDeletePlacementSummary', () => {
|
||||
it('summarizes folders, links, and the roots they sit in', () => {
|
||||
const summary = skillDeletePlacementSummary(
|
||||
plan([
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
canonicalPath: '/root/demo/SKILL.md',
|
||||
placements: [
|
||||
{
|
||||
path: '/home/.agents/skills/demo',
|
||||
kind: 'canonical',
|
||||
rootLabel: 'Agent skills home'
|
||||
},
|
||||
{ path: '/home/.claude/skills/demo', kind: 'alias-dir', rootLabel: 'Claude home' }
|
||||
]
|
||||
}
|
||||
])
|
||||
)
|
||||
expect(summary).toContain('1')
|
||||
expect(summary).toContain('Agent skills home')
|
||||
expect(summary).toContain('Claude home')
|
||||
})
|
||||
|
||||
it('drops the zero half so a link-only delete never reads "0 folders"', () => {
|
||||
const summary = skillDeletePlacementSummary(
|
||||
plan([
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
canonicalPath: '/home/.local/share/devex/skills/demo/SKILL.md',
|
||||
placements: [
|
||||
{ path: '/repo/.agents/skills/demo', kind: 'alias-dir', rootLabel: 'Repo app .agents' }
|
||||
]
|
||||
}
|
||||
])
|
||||
)
|
||||
expect(summary).not.toContain('0')
|
||||
expect(summary).toContain('link')
|
||||
expect(summary).not.toContain('folder')
|
||||
})
|
||||
|
||||
it('is null when the plan removes nothing', () => {
|
||||
// Otherwise the dialog read "0 folders and 0 links across " with an empty
|
||||
// tail, beside a red button that could not do anything.
|
||||
expect(
|
||||
skillDeletePlacementSummary(
|
||||
plan([
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
canonicalPath: '/elsewhere/demo/SKILL.md',
|
||||
placements: [],
|
||||
blocked: 'unowned'
|
||||
}
|
||||
])
|
||||
)
|
||||
).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('skillDeleteRetainedSourceLines', () => {
|
||||
it('says where the content stays when only links are removed', () => {
|
||||
const lines = skillDeleteRetainedSourceLines(
|
||||
plan([
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
canonicalPath: '/home/.local/share/devex/skills/demo/SKILL.md',
|
||||
placements: [
|
||||
{ path: '/repo/.agents/skills/demo', kind: 'alias-dir', rootLabel: 'Repo app .agents' }
|
||||
]
|
||||
}
|
||||
])
|
||||
)
|
||||
expect(lines).toHaveLength(1)
|
||||
expect(lines[0]).toContain('/home/.local/share/devex/skills/demo')
|
||||
})
|
||||
|
||||
it('stays silent when a real folder is being removed', () => {
|
||||
expect(
|
||||
skillDeleteRetainedSourceLines(
|
||||
plan([
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
canonicalPath: '/home/.agents/skills/demo/SKILL.md',
|
||||
placements: [
|
||||
{
|
||||
path: '/home/.agents/skills/demo',
|
||||
kind: 'canonical',
|
||||
rootLabel: 'Agent skills home'
|
||||
}
|
||||
]
|
||||
}
|
||||
])
|
||||
)
|
||||
).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('skillDeleteBlockedLines', () => {
|
||||
it('groups by typed reason rather than one merged skipped line', () => {
|
||||
const lines = skillDeleteBlockedLines(
|
||||
plan([
|
||||
{ id: 'a', name: 'a', canonicalPath: '/p', placements: [], blocked: 'bundled' },
|
||||
{ id: 'b', name: 'b', canonicalPath: '/p', placements: [], blocked: 'bundled' },
|
||||
{ id: 'c', name: 'c', canonicalPath: '/p', placements: [], blocked: 'plugin' }
|
||||
])
|
||||
)
|
||||
expect(lines).toHaveLength(2)
|
||||
expect(lines.some((line) => line.includes('2') && line.includes('Bundled'))).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('skillDeleteResultLines', () => {
|
||||
it('omits deleted rows and keeps one line per distinct outcome', () => {
|
||||
const lines = skillDeleteResultLines([
|
||||
{ id: 'a', name: 'a', status: 'deleted', removedPaths: ['/x'] },
|
||||
{ id: 'b', name: 'b', status: 'skipped', blocked: 'unowned', removedPaths: [] },
|
||||
{ id: 'c', name: 'c', status: 'skipped', blocked: 'unowned', removedPaths: [] },
|
||||
{ id: 'd', name: 'd', status: 'busy', removedPaths: [] }
|
||||
])
|
||||
expect(lines.map((line) => line.key)).toEqual(['skipped:unowned', 'busy'])
|
||||
expect(lines[0].label).toContain('2')
|
||||
})
|
||||
|
||||
it('renders nothing when every skill was deleted', () => {
|
||||
expect(
|
||||
skillDeleteResultLines([{ id: 'a', name: 'a', status: 'deleted', removedPaths: ['/x'] }])
|
||||
).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,203 @@
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import type {
|
||||
SkillDeleteBlockReason,
|
||||
SkillDeletePlan,
|
||||
SkillDeleteResultEntry
|
||||
} from '../../../../shared/skill-delete-contract'
|
||||
|
||||
export function skillDeleteBlockReasonLabel(reason: SkillDeleteBlockReason): string {
|
||||
switch (reason) {
|
||||
case 'bundled': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.reasonBundled',
|
||||
'Bundled with Orca — it would be restored'
|
||||
)
|
||||
}
|
||||
case 'plugin': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.reasonPlugin',
|
||||
'Installed by a plugin — remove the plugin instead'
|
||||
)
|
||||
}
|
||||
case 'unowned': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.reasonUnowned',
|
||||
'This skill lives outside Orca’s skill folders — delete it where it is stored'
|
||||
)
|
||||
}
|
||||
case 'missing': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.reasonMissing',
|
||||
'This skill is no longer on disk'
|
||||
)
|
||||
}
|
||||
case 'stale': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.reasonStale',
|
||||
'This skill changed since the list was loaded — refresh and try again'
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function skillDeleteActionLabel(count: number): string {
|
||||
return count === 1
|
||||
? translate('auto.components.skills.count.deleteOne', 'Delete {{count}} skill', { count })
|
||||
: translate('auto.components.skills.count.deleteOther', 'Delete {{count}} skills', { count })
|
||||
}
|
||||
|
||||
function foldersLabel(count: number): string {
|
||||
return count === 1
|
||||
? translate('auto.components.skills.count.deleteFolderOne', '{{count}} folder', { count })
|
||||
: translate('auto.components.skills.count.deleteFolderOther', '{{count}} folders', { count })
|
||||
}
|
||||
|
||||
function linksLabel(count: number): string {
|
||||
return count === 1
|
||||
? translate('auto.components.skills.count.deleteLinkOne', '{{count}} link', { count })
|
||||
: translate('auto.components.skills.count.deleteLinkOther', '{{count}} links', { count })
|
||||
}
|
||||
|
||||
/** "Removes 12 folders and 5 links across Claude, Codex, Agent Skills."
|
||||
* Null when the plan removes nothing, so the caller states that instead of
|
||||
* rendering "0 folders and 0 links across " with an empty tail. Zero-valued
|
||||
* halves are dropped: a link-only delete must not read "0 folders and 1 links". */
|
||||
export function skillDeletePlacementSummary(plan: SkillDeletePlan): string | null {
|
||||
const placements = plan.skills.flatMap((skill) => skill.placements)
|
||||
if (placements.length === 0) {
|
||||
return null
|
||||
}
|
||||
const folders = placements.filter((placement) => placement.kind === 'canonical').length
|
||||
const links = placements.length - folders
|
||||
const parts = [
|
||||
folders > 0 ? foldersLabel(folders) : null,
|
||||
links > 0 ? linksLabel(links) : null
|
||||
].filter((part): part is string => part !== null)
|
||||
const roots = [...new Set(placements.map((placement) => placement.rootLabel))].sort((a, b) =>
|
||||
a.localeCompare(b, undefined, { sensitivity: 'base' })
|
||||
)
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.placementSummaryParts',
|
||||
'Removes {{parts}} across {{roots}}.',
|
||||
{
|
||||
parts: parts.join(translate('auto.components.skills.SkillDelete.placementJoin', ' and ')),
|
||||
roots: roots.join(', ')
|
||||
}
|
||||
)
|
||||
}
|
||||
|
||||
function parentDirectory(path: string): string {
|
||||
const index = Math.max(path.lastIndexOf('/'), path.lastIndexOf('\\'))
|
||||
return index > 0 ? path.slice(0, index) : path
|
||||
}
|
||||
|
||||
/**
|
||||
* A skill whose content lives outside every Orca root is removed by its links
|
||||
* only. Saying so is the difference between "deleted" and "gone" — the content
|
||||
* survives at its source, and the user has to be told which one happened.
|
||||
*/
|
||||
export function skillDeleteRetainedSourceLines(plan: SkillDeletePlan): string[] {
|
||||
return plan.skills
|
||||
.filter(
|
||||
(skill) =>
|
||||
skill.placements.length > 0 &&
|
||||
!skill.placements.some((placement) => placement.kind === 'canonical')
|
||||
)
|
||||
.map((skill) =>
|
||||
translate(
|
||||
'auto.components.skills.SkillDelete.retainedSource',
|
||||
'The skill itself stays at {{path}}.',
|
||||
{ path: parentDirectory(skill.canonicalPath) }
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
export function skillDeleteNothingToDoLabel(count: number): string {
|
||||
return count === 1
|
||||
? translate(
|
||||
'auto.components.skills.SkillDelete.nothingOne',
|
||||
'That skill cannot be deleted from here.'
|
||||
)
|
||||
: translate(
|
||||
'auto.components.skills.SkillDelete.nothingOther',
|
||||
'None of the {{count}} selected skills can be deleted from here.',
|
||||
{ count }
|
||||
)
|
||||
}
|
||||
|
||||
/** One line per skip reason rather than one merged "skipped" line: the plan
|
||||
* already carries a typed reason per skill, so the dialog groups by it. */
|
||||
export function skillDeleteBlockedLines(plan: SkillDeletePlan): string[] {
|
||||
const counts = new Map<SkillDeleteBlockReason, number>()
|
||||
for (const skill of plan.skills) {
|
||||
if (skill.blocked) {
|
||||
counts.set(skill.blocked, (counts.get(skill.blocked) ?? 0) + 1)
|
||||
}
|
||||
}
|
||||
return [...counts].map(([reason, count]) =>
|
||||
translate('auto.components.skills.SkillDelete.blockedLine', '{{count}} × {{reason}}', {
|
||||
count,
|
||||
reason: skillDeleteBlockReasonLabel(reason)
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
export type SkillDeleteResultLine = { key: string; label: string }
|
||||
|
||||
/** A toast disappears before a user can act on "3 failed", so anything that is
|
||||
* not `deleted` gets a persistent, per-reason line instead. */
|
||||
export function skillDeleteResultLines(
|
||||
skills: readonly SkillDeleteResultEntry[]
|
||||
): SkillDeleteResultLine[] {
|
||||
const grouped = new Map<string, { count: number; label: string }>()
|
||||
for (const skill of skills) {
|
||||
if (skill.status === 'deleted') {
|
||||
continue
|
||||
}
|
||||
const key =
|
||||
skill.status === 'skipped' && skill.blocked ? `skipped:${skill.blocked}` : skill.status
|
||||
const existing = grouped.get(key)
|
||||
grouped.set(key, {
|
||||
count: (existing?.count ?? 0) + 1,
|
||||
label: existing?.label ?? statusLabel(skill)
|
||||
})
|
||||
}
|
||||
return [...grouped].map(([key, value]) => ({
|
||||
key,
|
||||
label: translate('auto.components.skills.SkillDelete.resultLine', '{{count}} × {{label}}', {
|
||||
count: value.count,
|
||||
label: value.label
|
||||
})
|
||||
}))
|
||||
}
|
||||
|
||||
function statusLabel(skill: SkillDeleteResultEntry): string {
|
||||
switch (skill.status) {
|
||||
case 'skipped': {
|
||||
return skill.blocked
|
||||
? skillDeleteBlockReasonLabel(skill.blocked)
|
||||
: translate('auto.components.skills.SkillDelete.statusSkipped', 'Skipped')
|
||||
}
|
||||
case 'busy': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.statusBusy',
|
||||
'Busy — another skill operation is in progress'
|
||||
)
|
||||
}
|
||||
case 'partial': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.statusPartial',
|
||||
'Partly removed — some files are still on disk under a hidden name'
|
||||
)
|
||||
}
|
||||
case 'failed': {
|
||||
return translate(
|
||||
'auto.components.skills.SkillDelete.statusFailed',
|
||||
'Failed — nothing changed'
|
||||
)
|
||||
}
|
||||
case 'deleted': {
|
||||
return translate('auto.components.skills.SkillDelete.statusDeleted', 'Deleted')
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,80 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { DiscoveredSkill } from '../../../../shared/skills'
|
||||
import {
|
||||
addDeletableSkillResults,
|
||||
eligibleDeleteSkillCount,
|
||||
isSkillDeleteEligible,
|
||||
retainedDeletableSkillSelection,
|
||||
skillDeleteEligibilityReason
|
||||
} from './skill-delete-selection'
|
||||
|
||||
function skill(overrides: Partial<DiscoveredSkill> & { id: string }): DiscoveredSkill {
|
||||
return {
|
||||
name: 'demo',
|
||||
description: null,
|
||||
providers: ['agent-skills'],
|
||||
sourceKind: 'home',
|
||||
sourceLabel: 'Agent skills home',
|
||||
rootPath: '/root',
|
||||
directoryPath: '/root/demo',
|
||||
skillFilePath: '/root/demo/SKILL.md',
|
||||
installed: true,
|
||||
updatedAt: 1,
|
||||
...overrides
|
||||
}
|
||||
}
|
||||
|
||||
describe('delete selection', () => {
|
||||
it('keeps two rows sharing a name independently selectable', () => {
|
||||
// Share collapses duplicate names because two skills cannot publish under
|
||||
// one name. Delete has no such constraint — these are two distinct files.
|
||||
const skills = [
|
||||
skill({ id: 'a', rootPath: '/home/.agents/skills' }),
|
||||
skill({ id: 'b', rootPath: '/repo/.claude/skills' })
|
||||
]
|
||||
expect(eligibleDeleteSkillCount(skills)).toBe(2)
|
||||
expect([...addDeletableSkillResults(new Set(), skills, skills)]).toEqual(['a', 'b'])
|
||||
})
|
||||
|
||||
it('excludes bundled and plugin rows from select-all', () => {
|
||||
const skills = [
|
||||
skill({ id: 'a' }),
|
||||
skill({ id: 'b', sourceKind: 'bundled' }),
|
||||
skill({ id: 'c', sourceKind: 'plugin' }),
|
||||
skill({ id: 'd', sourceKind: 'repo' })
|
||||
]
|
||||
expect(eligibleDeleteSkillCount(skills)).toBe(2)
|
||||
expect([...addDeletableSkillResults(new Set(), skills, skills)]).toEqual(['a', 'd'])
|
||||
})
|
||||
|
||||
it('drops a selection whose row no longer exists after a rescan', () => {
|
||||
const current = new Set(['a', 'gone'])
|
||||
const retained = retainedDeletableSkillSelection(current, [skill({ id: 'a' })])
|
||||
expect([...retained]).toEqual(['a'])
|
||||
})
|
||||
|
||||
it('returns the same set reference when a rescan changes nothing', () => {
|
||||
const skills = [skill({ id: 'a' }), skill({ id: 'b' })]
|
||||
const current = new Set(['a', 'b'])
|
||||
expect(retainedDeletableSkillSelection(current, skills)).toBe(current)
|
||||
})
|
||||
|
||||
it('drops a row that became ineligible between scans', () => {
|
||||
const current = new Set(['a'])
|
||||
const retained = retainedDeletableSkillSelection(current, [
|
||||
skill({ id: 'a', sourceKind: 'plugin' })
|
||||
])
|
||||
expect([...retained]).toEqual([])
|
||||
})
|
||||
|
||||
it('gives a reason only when the row is not deletable', () => {
|
||||
expect(isSkillDeleteEligible(skill({ id: 'a' }))).toBe(true)
|
||||
expect(skillDeleteEligibilityReason(skill({ id: 'a' }))).toBeNull()
|
||||
expect(skillDeleteEligibilityReason(skill({ id: 'b', sourceKind: 'bundled' }))).toContain(
|
||||
'Bundled'
|
||||
)
|
||||
expect(skillDeleteEligibilityReason(skill({ id: 'c', sourceKind: 'plugin' }))).toContain(
|
||||
'plugin'
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,46 @@
|
||||
import type { DiscoveredSkill } from '../../../../shared/skills'
|
||||
import { skillDeletionEligibility } from '../../../../shared/skill-deletion-eligibility'
|
||||
import { skillDeleteBlockReasonLabel } from './skill-delete-copy'
|
||||
import {
|
||||
addSelectableSkillResults,
|
||||
eligibleSkillSelectionCount,
|
||||
retainedSkillSelection,
|
||||
type SkillSelectionPolicy
|
||||
} from './skill-selection'
|
||||
|
||||
/** No collision key: two rows named `foo` from different roots are two distinct
|
||||
* skills, and both must be independently selectable and reportable. */
|
||||
const DELETE_SELECTION_POLICY: SkillSelectionPolicy = {
|
||||
isEligible: (skill) => skillDeletionEligibility(skill).deletable,
|
||||
collisionKey: () => null
|
||||
}
|
||||
|
||||
export function isSkillDeleteEligible(skill: DiscoveredSkill): boolean {
|
||||
return DELETE_SELECTION_POLICY.isEligible(skill)
|
||||
}
|
||||
|
||||
/** Advisory client-side reason. The host re-derives the verdict from the
|
||||
* placement set and can refuse a row this reports as deletable. */
|
||||
export function skillDeleteEligibilityReason(skill: DiscoveredSkill): string | null {
|
||||
const eligibility = skillDeletionEligibility(skill)
|
||||
return eligibility.deletable ? null : skillDeleteBlockReasonLabel(eligibility.reason)
|
||||
}
|
||||
|
||||
export function eligibleDeleteSkillCount(results: readonly DiscoveredSkill[]): number {
|
||||
return eligibleSkillSelectionCount(results, DELETE_SELECTION_POLICY)
|
||||
}
|
||||
|
||||
export function addDeletableSkillResults(
|
||||
current: ReadonlySet<string>,
|
||||
skills: readonly DiscoveredSkill[],
|
||||
results: readonly DiscoveredSkill[]
|
||||
): Set<string> {
|
||||
return addSelectableSkillResults(current, skills, results, DELETE_SELECTION_POLICY)
|
||||
}
|
||||
|
||||
export function retainedDeletableSkillSelection(
|
||||
current: Set<string>,
|
||||
skills: readonly DiscoveredSkill[]
|
||||
): Set<string> {
|
||||
return retainedSkillSelection(current, skills, DELETE_SELECTION_POLICY)
|
||||
}
|
||||
@@ -0,0 +1,97 @@
|
||||
import type { DiscoveredSkill } from '../../../../shared/skills'
|
||||
|
||||
/**
|
||||
* The range/select-all/retain-across-rescan logic both selection modes share.
|
||||
*
|
||||
* `collisionKey` is what they do *not* share. Share collapses selections by
|
||||
* lowercase name, because two skills cannot publish under one name. Delete has
|
||||
* no such constraint — two rows named `foo` from different roots must both be
|
||||
* selectable — so it passes `null` and nothing collapses.
|
||||
*/
|
||||
export type SkillSelectionPolicy = {
|
||||
isEligible: (skill: DiscoveredSkill) => boolean
|
||||
collisionKey: (skill: DiscoveredSkill) => string | null
|
||||
}
|
||||
|
||||
function collisionKeys(
|
||||
skills: readonly DiscoveredSkill[],
|
||||
selectedIds: ReadonlySet<string>,
|
||||
policy: SkillSelectionPolicy
|
||||
): Set<string> {
|
||||
const keys = new Set<string>()
|
||||
for (const skill of skills) {
|
||||
const key = selectedIds.has(skill.id) ? policy.collisionKey(skill) : null
|
||||
if (key !== null) {
|
||||
keys.add(key)
|
||||
}
|
||||
}
|
||||
return keys
|
||||
}
|
||||
|
||||
/** How many rows "Select all" would end up holding. */
|
||||
export function eligibleSkillSelectionCount(
|
||||
results: readonly DiscoveredSkill[],
|
||||
policy: SkillSelectionPolicy
|
||||
): number {
|
||||
const keys = new Set<string>()
|
||||
let uncollapsed = 0
|
||||
for (const skill of results) {
|
||||
if (!policy.isEligible(skill)) {
|
||||
continue
|
||||
}
|
||||
const key = policy.collisionKey(skill)
|
||||
if (key === null) {
|
||||
uncollapsed += 1
|
||||
continue
|
||||
}
|
||||
keys.add(key)
|
||||
}
|
||||
return keys.size + uncollapsed
|
||||
}
|
||||
|
||||
/** Used by both select-all and shift-range. */
|
||||
export function addSelectableSkillResults(
|
||||
current: ReadonlySet<string>,
|
||||
skills: readonly DiscoveredSkill[],
|
||||
results: readonly DiscoveredSkill[],
|
||||
policy: SkillSelectionPolicy
|
||||
): Set<string> {
|
||||
const next = new Set(current)
|
||||
const taken = collisionKeys(skills, current, policy)
|
||||
for (const skill of results) {
|
||||
if (!policy.isEligible(skill)) {
|
||||
continue
|
||||
}
|
||||
const key = policy.collisionKey(skill)
|
||||
if (key !== null && taken.has(key)) {
|
||||
continue
|
||||
}
|
||||
next.add(skill.id)
|
||||
if (key !== null) {
|
||||
taken.add(key)
|
||||
}
|
||||
}
|
||||
return next
|
||||
}
|
||||
|
||||
/** Rebuilds the selection from a fresh scan, dropping rows that vanished or
|
||||
* became ineligible. Returns the same reference when nothing changed. */
|
||||
export function retainedSkillSelection(
|
||||
current: Set<string>,
|
||||
skills: readonly DiscoveredSkill[],
|
||||
policy: SkillSelectionPolicy
|
||||
): Set<string> {
|
||||
const next = new Set<string>()
|
||||
const taken = new Set<string>()
|
||||
for (const skill of skills) {
|
||||
const key = policy.collisionKey(skill)
|
||||
if (!current.has(skill.id) || !policy.isEligible(skill) || (key !== null && taken.has(key))) {
|
||||
continue
|
||||
}
|
||||
next.add(skill.id)
|
||||
if (key !== null) {
|
||||
taken.add(key)
|
||||
}
|
||||
}
|
||||
return next.size === current.size ? current : next
|
||||
}
|
||||
@@ -1,5 +1,11 @@
|
||||
import type { DiscoveredSkill } from '../../../../shared/skills'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import {
|
||||
addSelectableSkillResults,
|
||||
eligibleSkillSelectionCount,
|
||||
retainedSkillSelection,
|
||||
type SkillSelectionPolicy
|
||||
} from './skill-selection'
|
||||
|
||||
export function isSkillShareEligible(skill: DiscoveredSkill, local: boolean): boolean {
|
||||
return local && skill.installed && (skill.sourceKind === 'home' || skill.sourceKind === 'repo')
|
||||
@@ -49,19 +55,25 @@ export function selectedShareSkillNameKeys(
|
||||
)
|
||||
}
|
||||
|
||||
/** How many rows "Select all" would end up holding — duplicate names collapse to
|
||||
* one, so the count has to dedup the same way the selection itself does. */
|
||||
const SHARE_SELECTION_POLICY_LOCAL: SkillSelectionPolicy = {
|
||||
isEligible: (skill) => isSkillShareEligible(skill, true),
|
||||
collisionKey: shareSkillNameKey
|
||||
}
|
||||
|
||||
const SHARE_SELECTION_POLICY_REMOTE: SkillSelectionPolicy = {
|
||||
isEligible: () => false,
|
||||
collisionKey: shareSkillNameKey
|
||||
}
|
||||
|
||||
function sharePolicy(local: boolean): SkillSelectionPolicy {
|
||||
return local ? SHARE_SELECTION_POLICY_LOCAL : SHARE_SELECTION_POLICY_REMOTE
|
||||
}
|
||||
|
||||
export function eligibleShareSkillCount(
|
||||
results: readonly DiscoveredSkill[],
|
||||
local: boolean
|
||||
): number {
|
||||
const names = new Set<string>()
|
||||
for (const skill of results) {
|
||||
if (isSkillShareEligible(skill, local)) {
|
||||
names.add(shareSkillNameKey(skill))
|
||||
}
|
||||
}
|
||||
return names.size
|
||||
return eligibleSkillSelectionCount(results, sharePolicy(local))
|
||||
}
|
||||
|
||||
export function addShareableSkillResults(
|
||||
@@ -70,17 +82,7 @@ export function addShareableSkillResults(
|
||||
results: readonly DiscoveredSkill[],
|
||||
local: boolean
|
||||
): Set<string> {
|
||||
const next = new Set(current)
|
||||
const selectedNames = selectedShareSkillNameKeys(skills, current)
|
||||
for (const skill of results) {
|
||||
const name = shareSkillNameKey(skill)
|
||||
if (!isSkillShareEligible(skill, local) || selectedNames.has(name)) {
|
||||
continue
|
||||
}
|
||||
next.add(skill.id)
|
||||
selectedNames.add(name)
|
||||
}
|
||||
return next
|
||||
return addSelectableSkillResults(current, skills, results, sharePolicy(local))
|
||||
}
|
||||
|
||||
export function retainedShareableSkillSelection(
|
||||
@@ -88,17 +90,7 @@ export function retainedShareableSkillSelection(
|
||||
skills: readonly DiscoveredSkill[],
|
||||
local: boolean
|
||||
): Set<string> {
|
||||
const next = new Set<string>()
|
||||
const names = new Set<string>()
|
||||
for (const skill of skills) {
|
||||
const name = shareSkillNameKey(skill)
|
||||
if (!current.has(skill.id) || !isSkillShareEligible(skill, local) || names.has(name)) {
|
||||
continue
|
||||
}
|
||||
next.add(skill.id)
|
||||
names.add(name)
|
||||
}
|
||||
return next.size === current.size ? current : next
|
||||
return retainedSkillSelection(current, skills, sharePolicy(local))
|
||||
}
|
||||
|
||||
export function updatedSkillSelection(
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
import { useCallback, useEffect, useState } from 'react'
|
||||
import { toast } from 'sonner'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { useConfirmationDialog } from '@/components/confirmation-dialog-context'
|
||||
import { useMountedRef } from '@/hooks/useMountedRef'
|
||||
import type {
|
||||
SkillDeletePlan,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteResult
|
||||
} from '../../../../shared/skill-delete-contract'
|
||||
import { SKILL_DELETE_UPDATE_REQUIRED_MESSAGE } from '../../../../shared/skill-install-capability'
|
||||
import type { DiscoveredSkill } from '../../../../shared/skills'
|
||||
import {
|
||||
deleteSkillsOnRuntimeTarget,
|
||||
previewSkillDeletionOnRuntimeTarget,
|
||||
runtimeTargetSupportsSkillDelete
|
||||
} from '../../runtime/runtime-skills-client'
|
||||
import type { RuntimeClientTarget } from '../../runtime/runtime-rpc-client'
|
||||
import {
|
||||
skillDeleteActionLabel,
|
||||
skillDeleteBlockedLines,
|
||||
skillDeleteNothingToDoLabel,
|
||||
skillDeletePlacementSummary,
|
||||
skillDeleteRetainedSourceLines
|
||||
} from './skill-delete-copy'
|
||||
|
||||
export type SkillDeleteFlow = {
|
||||
/** False while the target is unresolved or the host predates the capability. */
|
||||
supported: boolean
|
||||
/** Why it is unsupported — the two causes need different copy. Null when
|
||||
* delete is available. */
|
||||
unsupportedReason: string | null
|
||||
running: boolean
|
||||
result: SkillDeleteResult | null
|
||||
dismissResult: () => void
|
||||
requestDelete: (skills: readonly DiscoveredSkill[]) => Promise<boolean>
|
||||
}
|
||||
|
||||
function toRequest(skills: readonly DiscoveredSkill[]): SkillDeleteRequest {
|
||||
return {
|
||||
operationId: crypto.randomUUID(),
|
||||
skills: skills.map((skill) => ({
|
||||
id: skill.id,
|
||||
directoryPath: skill.directoryPath,
|
||||
skillFilePath: skill.skillFilePath,
|
||||
name: skill.name,
|
||||
updatedAt: skill.updatedAt
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
export function useSkillDeleteFlow(
|
||||
runtimeTarget: RuntimeClientTarget | null,
|
||||
hostLabel: string | null,
|
||||
onDeleted: (result: SkillDeleteResult) => void
|
||||
): SkillDeleteFlow {
|
||||
const confirm = useConfirmationDialog()
|
||||
const mountedRef = useMountedRef()
|
||||
const [supported, setSupported] = useState(false)
|
||||
const [running, setRunning] = useState(false)
|
||||
const [result, setResult] = useState<SkillDeleteResult | null>(null)
|
||||
|
||||
useEffect(() => {
|
||||
let current = true
|
||||
setSupported(false)
|
||||
void runtimeTargetSupportsSkillDelete(runtimeTarget).then((value) => {
|
||||
if (current && mountedRef.current) {
|
||||
setSupported(value)
|
||||
}
|
||||
})
|
||||
return () => {
|
||||
current = false
|
||||
}
|
||||
}, [mountedRef, runtimeTarget])
|
||||
|
||||
const requestDelete = useCallback(
|
||||
async (skills: readonly DiscoveredSkill[]): Promise<boolean> => {
|
||||
if (!runtimeTarget || skills.length === 0) {
|
||||
return false
|
||||
}
|
||||
const request = toRequest(skills)
|
||||
setRunning(true)
|
||||
try {
|
||||
const plan = await previewSkillDeletionOnRuntimeTarget(runtimeTarget, request)
|
||||
const actionable = plan.skills.filter((skill) => !skill.blocked)
|
||||
if (actionable.length === 0) {
|
||||
// The preview already knows this would remove nothing. Offering a
|
||||
// destructive confirm here is what let a blocked row be "deleted"
|
||||
// over and over with no effect; say why instead, in the band, where
|
||||
// it persists long enough to act on.
|
||||
setResult(blockedOnlyResult(plan))
|
||||
toast.error(skillDeleteNothingToDoLabel(plan.skills.length))
|
||||
return false
|
||||
}
|
||||
const confirmed = await confirm({
|
||||
title: skillDeleteActionLabel(actionable.length),
|
||||
description: [
|
||||
hostLabel
|
||||
? translate('auto.components.skills.SkillDelete.confirmHost', 'On {{host}}.', {
|
||||
host: hostLabel
|
||||
})
|
||||
: null,
|
||||
skillDeletePlacementSummary(plan),
|
||||
// Permanent, so a single-skill delete names the paths themselves.
|
||||
actionable.length === 1
|
||||
? actionable.flatMap((skill) => skill.placements.map((p) => p.path)).join('\n')
|
||||
: null,
|
||||
...skillDeleteRetainedSourceLines(plan),
|
||||
...skillDeleteBlockedLines(plan),
|
||||
translate(
|
||||
'auto.components.skills.SkillDelete.confirmPermanent',
|
||||
'This cannot be undone.'
|
||||
)
|
||||
]
|
||||
.filter((line): line is string => Boolean(line))
|
||||
.join('\n'),
|
||||
confirmLabel: skillDeleteActionLabel(actionable.length),
|
||||
confirmVariant: 'destructive'
|
||||
})
|
||||
if (!confirmed) {
|
||||
return false
|
||||
}
|
||||
const outcome = await deleteSkillsOnRuntimeTarget(runtimeTarget, request)
|
||||
if (!mountedRef.current) {
|
||||
return true
|
||||
}
|
||||
const deleted = outcome.skills.filter((skill) => skill.status === 'deleted').length
|
||||
if (deleted > 0) {
|
||||
toast.success(skillDeletedToast(deleted))
|
||||
}
|
||||
setResult(outcome)
|
||||
onDeleted(outcome)
|
||||
return true
|
||||
} catch (error) {
|
||||
toast.error(
|
||||
error instanceof Error
|
||||
? error.message
|
||||
: translate('auto.components.skills.SkillDelete.failed', 'Could not delete skills')
|
||||
)
|
||||
return false
|
||||
} finally {
|
||||
if (mountedRef.current) {
|
||||
setRunning(false)
|
||||
}
|
||||
}
|
||||
},
|
||||
[confirm, hostLabel, mountedRef, onDeleted, runtimeTarget]
|
||||
)
|
||||
|
||||
return {
|
||||
supported,
|
||||
unsupportedReason: supported ? null : unsupportedReason(runtimeTarget),
|
||||
running,
|
||||
result,
|
||||
dismissResult: useCallback(() => setResult(null), []),
|
||||
requestDelete
|
||||
}
|
||||
}
|
||||
|
||||
/** Turns a plan in which nothing is actionable into the same result shape the
|
||||
* band already groups, so one component renders every skip reason. */
|
||||
function blockedOnlyResult(plan: SkillDeletePlan): SkillDeleteResult {
|
||||
return {
|
||||
operationId: plan.operationId,
|
||||
skills: plan.skills.map((skill) => ({
|
||||
id: skill.id,
|
||||
name: skill.name,
|
||||
status: 'skipped' as const,
|
||||
...(skill.blocked ? { blocked: skill.blocked } : {}),
|
||||
removedPaths: []
|
||||
}))
|
||||
}
|
||||
}
|
||||
|
||||
function unsupportedReason(runtimeTarget: RuntimeClientTarget | null): string {
|
||||
return runtimeTarget
|
||||
? SKILL_DELETE_UPDATE_REQUIRED_MESSAGE
|
||||
: translate(
|
||||
'auto.components.skills.SkillDelete.hostUnresolved',
|
||||
'Still finding the machine that owns these skills.'
|
||||
)
|
||||
}
|
||||
|
||||
function skillDeletedToast(count: number): string {
|
||||
return count === 1
|
||||
? translate('auto.components.skills.count.deletedOne', 'Deleted {{count}} skill', { count })
|
||||
: translate('auto.components.skills.count.deletedOther', 'Deleted {{count}} skills', { count })
|
||||
}
|
||||
@@ -4148,7 +4148,8 @@
|
||||
"retry": "Retry",
|
||||
"remoteShareNotice": "These skills live on {{host}}. Open Skills on that machine to share them.",
|
||||
"viewSwitch": "Show",
|
||||
"searchLinks": "Search links"
|
||||
"searchLinks": "Search links",
|
||||
"deleteSkills": "Delete skills…"
|
||||
},
|
||||
"SkillFreshnessNudge": {
|
||||
"titleOne": "An installed Orca skill is out of date",
|
||||
@@ -4476,7 +4477,9 @@
|
||||
"detailSource": "Source",
|
||||
"detailContents": "Contents",
|
||||
"skillActions": "Actions for {{value0}}",
|
||||
"viewDetails": "View details"
|
||||
"viewDetails": "View details",
|
||||
"deleteSkill": "Delete…",
|
||||
"notDeletable": "Not deletable"
|
||||
},
|
||||
"SkillSelectionBar": {
|
||||
"selectAll": "Select all {{count}} eligible",
|
||||
@@ -4516,7 +4519,15 @@
|
||||
"retryOne": "Retry {{count}} skill",
|
||||
"retryOther": "Retry {{count}} skills",
|
||||
"linkOne": "{{count}} link",
|
||||
"linkOther": "{{count}} links"
|
||||
"linkOther": "{{count}} links",
|
||||
"deleteOne": "Delete {{count}} skill",
|
||||
"deleteOther": "Delete {{count}} skills",
|
||||
"deletedOne": "Deleted {{count}} skill",
|
||||
"deletedOther": "Deleted {{count}} skills",
|
||||
"deleteFolderOne": "{{count}} folder",
|
||||
"deleteFolderOther": "{{count}} folders",
|
||||
"deleteLinkOne": "{{count}} link",
|
||||
"deleteLinkOther": "{{count}} links"
|
||||
},
|
||||
"host": {
|
||||
"local": "This machine",
|
||||
@@ -4585,7 +4596,8 @@
|
||||
"exitTooltip": "Exit selection · Esc",
|
||||
"title": "Select skills to share",
|
||||
"selectAll": "Select all {{count}} eligible",
|
||||
"clear": "Clear"
|
||||
"clear": "Clear",
|
||||
"deleteTitle": "Select skills to delete"
|
||||
},
|
||||
"SkillDetailDialog": {
|
||||
"agents": "Agents",
|
||||
@@ -4639,6 +4651,30 @@
|
||||
"supportingFilesDescription": "Includes readable reference files beyond the main instructions.",
|
||||
"runnableFilesDescription": "Includes scripts that an agent may run with your access.",
|
||||
"binaryFilesDescription": "Includes opaque assets and an executable binary."
|
||||
},
|
||||
"SkillDelete": {
|
||||
"dismissResults": "Dismiss",
|
||||
"reasonBundled": "Bundled with Orca — it would be restored",
|
||||
"reasonPlugin": "Installed by a plugin — remove the plugin instead",
|
||||
"reasonUnowned": "This skill lives outside Orca’s skill folders — delete it where it is stored",
|
||||
"reasonMissing": "This skill is no longer on disk",
|
||||
"reasonStale": "This skill changed since the list was loaded — refresh and try again",
|
||||
"blockedLine": "{{count}} × {{reason}}",
|
||||
"resultLine": "{{count}} × {{label}}",
|
||||
"statusSkipped": "Skipped",
|
||||
"statusBusy": "Busy — another skill operation is in progress",
|
||||
"statusPartial": "Partly removed — some files are still on disk under a hidden name",
|
||||
"statusFailed": "Failed — nothing changed",
|
||||
"statusDeleted": "Deleted",
|
||||
"confirmHost": "On {{host}}.",
|
||||
"confirmPermanent": "This cannot be undone.",
|
||||
"failed": "Could not delete skills",
|
||||
"hostUnresolved": "Still finding the machine that owns these skills.",
|
||||
"nothingOne": "That skill cannot be deleted from here.",
|
||||
"nothingOther": "None of the {{count}} selected skills can be deleted from here.",
|
||||
"placementSummaryParts": "Removes {{parts}} across {{roots}}.",
|
||||
"placementJoin": " and ",
|
||||
"retainedSource": "The skill itself stays at {{path}}."
|
||||
}
|
||||
},
|
||||
"sidebar": {
|
||||
|
||||
@@ -3960,6 +3960,10 @@
|
||||
"SkillUpdateRow": {
|
||||
"oneLocation": "1 location",
|
||||
"manyLocations": "{{value0}} locations"
|
||||
},
|
||||
"host": {
|
||||
"local": "Esta máquina",
|
||||
"remote": "Entorno conectado"
|
||||
}
|
||||
},
|
||||
"sidebar": {
|
||||
|
||||
@@ -3960,6 +3960,10 @@
|
||||
"SkillUpdateRow": {
|
||||
"oneLocation": "1 件の場所",
|
||||
"manyLocations": "{{value0}} 件の場所"
|
||||
},
|
||||
"host": {
|
||||
"local": "このマシン",
|
||||
"remote": "接続済みランタイム"
|
||||
}
|
||||
},
|
||||
"sidebar": {
|
||||
|
||||
@@ -3962,6 +3962,10 @@
|
||||
"SkillUpdateRow": {
|
||||
"oneLocation": "1 location",
|
||||
"manyLocations": "{{value0}} locations"
|
||||
},
|
||||
"host": {
|
||||
"local": "이 컴퓨터",
|
||||
"remote": "연결된 런타임"
|
||||
}
|
||||
},
|
||||
"sidebar": {
|
||||
|
||||
@@ -3972,6 +3972,36 @@
|
||||
"SkillUpdateRow": {
|
||||
"oneLocation": "1 location",
|
||||
"manyLocations": "{{value0}} locations"
|
||||
},
|
||||
"count": {
|
||||
"deleteOne": "删除 {{count}} 个技能",
|
||||
"deleteOther": "删除 {{count}} 个技能",
|
||||
"deletedOne": "已删除 {{count}} 个技能",
|
||||
"deletedOther": "已删除 {{count}} 个技能"
|
||||
},
|
||||
"host": {
|
||||
"local": "此机器",
|
||||
"remote": "已连接的运行时"
|
||||
},
|
||||
"SkillDelete": {
|
||||
"dismissResults": "关闭",
|
||||
"reasonBundled": "Orca 内置技能 — 删除后会恢复",
|
||||
"reasonPlugin": "由插件安装 — 请改为移除插件",
|
||||
"reasonUnowned": "此技能位于 Orca 技能文件夹之外 — 请在其存储位置删除",
|
||||
"reasonMissing": "此技能已不在磁盘上",
|
||||
"reasonStale": "此技能在列表加载后发生了变化 — 请刷新后重试",
|
||||
"placementSummary": "将在 {{roots}} 中删除 {{folders}} 个文件夹和 {{links}} 个链接。",
|
||||
"blockedLine": "{{count}} × {{reason}}",
|
||||
"resultLine": "{{count}} × {{label}}",
|
||||
"statusSkipped": "已跳过",
|
||||
"statusBusy": "忙碌 — 另一项技能操作正在进行",
|
||||
"statusPartial": "部分移除 — 某些文件仍以隐藏名称保留在磁盘上",
|
||||
"statusFailed": "失败 — 未进行任何更改",
|
||||
"statusDeleted": "已删除",
|
||||
"confirmHost": "位于 {{host}}。",
|
||||
"confirmPermanent": "此操作无法撤销。",
|
||||
"failed": "无法删除技能",
|
||||
"hostUnresolved": "仍在查找存储这些技能的计算机。"
|
||||
}
|
||||
},
|
||||
"sidebar": {
|
||||
|
||||
@@ -1,5 +1,19 @@
|
||||
import type {
|
||||
SkillDeletePlan,
|
||||
SkillDeleteRequest,
|
||||
SkillDeleteResult
|
||||
} from '../../../shared/skill-delete-contract'
|
||||
import {
|
||||
SKILL_DELETE_CAPABILITY,
|
||||
SKILL_DELETE_UPDATE_REQUIRED_MESSAGE
|
||||
} from '../../../shared/skill-install-capability'
|
||||
import type { SkillDiscoveryResult, SkillDiscoveryTarget } from '../../../shared/skills'
|
||||
import { callRuntimeRpc, type RuntimeClientTarget } from './runtime-rpc-client'
|
||||
import {
|
||||
assertRuntimeEnvironmentCapability,
|
||||
callRuntimeRpc,
|
||||
runtimeEnvironmentSupportsCapability,
|
||||
type RuntimeClientTarget
|
||||
} from './runtime-rpc-client'
|
||||
|
||||
const SKILL_DISCOVERY_TIMEOUT_MS = 15_000
|
||||
|
||||
@@ -35,3 +49,69 @@ export async function discoverSkillsForRuntimeTarget(
|
||||
{ timeoutMs: SKILL_DISCOVERY_TIMEOUT_MS }
|
||||
)
|
||||
}
|
||||
|
||||
const SKILL_DELETE_PREVIEW_TIMEOUT_MS = 60_000
|
||||
const SKILL_DELETE_TIMEOUT_MS = 5 * 60_000
|
||||
|
||||
/**
|
||||
* Whether the delete affordance may be offered at all. The gate lives here,
|
||||
* beside where `callRuntimeRpc` is actually invoked — the main-process
|
||||
* `callRuntimeEnvironment` path install and remove use is a different,
|
||||
* non-overlapping transport, so a check there would never run for this.
|
||||
*/
|
||||
export async function runtimeTargetSupportsSkillDelete(
|
||||
runtimeTarget: RuntimeClientTarget | null
|
||||
): Promise<boolean> {
|
||||
if (!runtimeTarget) {
|
||||
return false
|
||||
}
|
||||
if (runtimeTarget.kind === 'local') {
|
||||
return true
|
||||
}
|
||||
return runtimeEnvironmentSupportsCapability(runtimeTarget.environmentId, SKILL_DELETE_CAPABILITY)
|
||||
}
|
||||
|
||||
async function assertSkillDeleteSupported(runtimeTarget: RuntimeClientTarget): Promise<void> {
|
||||
if (runtimeTarget.kind === 'local') {
|
||||
return
|
||||
}
|
||||
try {
|
||||
await assertRuntimeEnvironmentCapability(
|
||||
runtimeTarget.environmentId,
|
||||
SKILL_DELETE_CAPABILITY,
|
||||
SKILL_DELETE_UPDATE_REQUIRED_MESSAGE
|
||||
)
|
||||
} catch (error) {
|
||||
// A capability change racing the gate has no main-process hook to reuse:
|
||||
// `recordSkillCapabilityAbsence` imports the main tracer and its capability
|
||||
// parameter is a closed union over main-side capabilities.
|
||||
console.warn('[skills] delete capability absent at call time', error)
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
export async function previewSkillDeletionOnRuntimeTarget(
|
||||
runtimeTarget: RuntimeClientTarget,
|
||||
request: SkillDeleteRequest
|
||||
): Promise<SkillDeletePlan> {
|
||||
await assertSkillDeleteSupported(runtimeTarget)
|
||||
if (runtimeTarget.kind === 'local') {
|
||||
return window.api.skills.previewDelete(request)
|
||||
}
|
||||
return callRuntimeRpc<SkillDeletePlan>(runtimeTarget, 'skills.previewDelete', request, {
|
||||
timeoutMs: SKILL_DELETE_PREVIEW_TIMEOUT_MS
|
||||
})
|
||||
}
|
||||
|
||||
export async function deleteSkillsOnRuntimeTarget(
|
||||
runtimeTarget: RuntimeClientTarget,
|
||||
request: SkillDeleteRequest
|
||||
): Promise<SkillDeleteResult> {
|
||||
await assertSkillDeleteSupported(runtimeTarget)
|
||||
if (runtimeTarget.kind === 'local') {
|
||||
return window.api.skills.delete(request)
|
||||
}
|
||||
return callRuntimeRpc<SkillDeleteResult>(runtimeTarget, 'skills.delete', request, {
|
||||
timeoutMs: SKILL_DELETE_TIMEOUT_MS
|
||||
})
|
||||
}
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { SKILL_DELETE_UPDATE_REQUIRED_MESSAGE } from '../../../shared/skill-install-capability'
|
||||
import type { SkillDeleteRequest } from '../../../shared/skill-delete-contract'
|
||||
|
||||
const callRuntimeRpc = vi.fn()
|
||||
const runtimeEnvironmentSupportsCapability = vi.fn()
|
||||
const assertRuntimeEnvironmentCapability = vi.fn()
|
||||
|
||||
vi.mock('./runtime-rpc-client', () => ({
|
||||
callRuntimeRpc: (...args: unknown[]) => callRuntimeRpc(...args),
|
||||
runtimeEnvironmentSupportsCapability: (...args: unknown[]) =>
|
||||
runtimeEnvironmentSupportsCapability(...args),
|
||||
assertRuntimeEnvironmentCapability: (...args: unknown[]) =>
|
||||
assertRuntimeEnvironmentCapability(...args)
|
||||
}))
|
||||
|
||||
const {
|
||||
deleteSkillsOnRuntimeTarget,
|
||||
previewSkillDeletionOnRuntimeTarget,
|
||||
runtimeTargetSupportsSkillDelete
|
||||
} = await import('./runtime-skills-client')
|
||||
|
||||
const REQUEST: SkillDeleteRequest = {
|
||||
operationId: 'op',
|
||||
skills: [
|
||||
{ id: 'a', directoryPath: '/d', skillFilePath: '/d/SKILL.md', name: 'demo', updatedAt: 1 }
|
||||
]
|
||||
}
|
||||
|
||||
const localDelete = vi.fn()
|
||||
const localPreview = vi.fn()
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
vi.stubGlobal('window', {
|
||||
api: { skills: { delete: localDelete, previewDelete: localPreview } }
|
||||
})
|
||||
})
|
||||
|
||||
describe('runtimeTargetSupportsSkillDelete', () => {
|
||||
it('never probes the local target', async () => {
|
||||
expect(await runtimeTargetSupportsSkillDelete({ kind: 'local' })).toBe(true)
|
||||
expect(runtimeEnvironmentSupportsCapability).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('is unsupported while the runtime target is still unresolved', async () => {
|
||||
expect(await runtimeTargetSupportsSkillDelete(null)).toBe(false)
|
||||
expect(runtimeEnvironmentSupportsCapability).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('asks the environment for the capability', async () => {
|
||||
runtimeEnvironmentSupportsCapability.mockResolvedValue(false)
|
||||
expect(
|
||||
await runtimeTargetSupportsSkillDelete({ kind: 'environment', environmentId: 'env-1' })
|
||||
).toBe(false)
|
||||
expect(runtimeEnvironmentSupportsCapability).toHaveBeenCalledWith('env-1', 'skills.delete.v1')
|
||||
})
|
||||
})
|
||||
|
||||
describe('delete routing', () => {
|
||||
it('issues no RPC against a host that lacks the capability', async () => {
|
||||
assertRuntimeEnvironmentCapability.mockRejectedValue(
|
||||
new Error(SKILL_DELETE_UPDATE_REQUIRED_MESSAGE)
|
||||
)
|
||||
await expect(
|
||||
deleteSkillsOnRuntimeTarget({ kind: 'environment', environmentId: 'env-1' }, REQUEST)
|
||||
).rejects.toThrow(SKILL_DELETE_UPDATE_REQUIRED_MESSAGE)
|
||||
expect(callRuntimeRpc).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('routes a local delete through IPC rather than RPC', async () => {
|
||||
localDelete.mockResolvedValue({ operationId: 'op', skills: [] })
|
||||
await deleteSkillsOnRuntimeTarget({ kind: 'local' }, REQUEST)
|
||||
expect(localDelete).toHaveBeenCalledWith(REQUEST)
|
||||
expect(callRuntimeRpc).not.toHaveBeenCalled()
|
||||
expect(assertRuntimeEnvironmentCapability).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('sends the whole request to a capable remote host', async () => {
|
||||
assertRuntimeEnvironmentCapability.mockResolvedValue(undefined)
|
||||
callRuntimeRpc.mockResolvedValue({ operationId: 'op', skills: [] })
|
||||
await deleteSkillsOnRuntimeTarget({ kind: 'environment', environmentId: 'env-1' }, REQUEST)
|
||||
expect(callRuntimeRpc).toHaveBeenCalledWith(
|
||||
{ kind: 'environment', environmentId: 'env-1' },
|
||||
'skills.delete',
|
||||
REQUEST,
|
||||
expect.objectContaining({ timeoutMs: expect.any(Number) })
|
||||
)
|
||||
})
|
||||
|
||||
it('gates the preview on the same capability', async () => {
|
||||
assertRuntimeEnvironmentCapability.mockRejectedValue(new Error('nope'))
|
||||
await expect(
|
||||
previewSkillDeletionOnRuntimeTarget({ kind: 'environment', environmentId: 'env-1' }, REQUEST)
|
||||
).rejects.toThrow('nope')
|
||||
expect(callRuntimeRpc).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -49,6 +49,7 @@ import type {
|
||||
} from '../../../shared/worktree/create-types'
|
||||
import type { WorkspaceLineage, WorktreeLineage } from '../../../shared/worktree/lineage-types'
|
||||
import type { DetectedWorktreeListResult, Worktree } from '../../../shared/worktree/types'
|
||||
import type { SkillDeletePlan, SkillDeleteResult } from '../../../shared/skill-delete-contract'
|
||||
import type { SkillDiscoveryResult } from '../../../shared/skills'
|
||||
import type { SkillFreshnessInventory } from '../../../shared/skill-freshness'
|
||||
import type { SshConnectionState, SshTarget } from '../../../shared/ssh-types'
|
||||
@@ -3159,6 +3160,11 @@ function createSkillsApi(): NonNullable<Partial<PreloadApi>['skills']> {
|
||||
previewBundleInstall: () =>
|
||||
Promise.reject(new Error('Skill installation requires the desktop app.')),
|
||||
removeInstall: () => Promise.reject(new Error('Skill installation requires the desktop app.')),
|
||||
// Why routed rather than refused: on web the "local" host IS the runtime, so
|
||||
// delete rides the same RPC the scan already does.
|
||||
previewDelete: (request) =>
|
||||
callRuntimeResult<SkillDeletePlan>('skills.previewDelete', request, 60_000),
|
||||
delete: (request) => callRuntimeResult<SkillDeleteResult>('skills.delete', request, 5 * 60_000),
|
||||
listManagedInstalls: () =>
|
||||
Promise.reject(new Error('Skill installation requires the desktop app.')),
|
||||
getPackage: () => Promise.reject(new Error('Skill installation requires the desktop app.')),
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { REMOTE_SERVER_UPDATE_CAPABILITY } from './remote-server-update'
|
||||
import {
|
||||
SKILL_BUNDLE_INSTALL_CAPABILITY,
|
||||
SKILL_DELETE_CAPABILITY,
|
||||
SKILL_INSTALL_CAPABILITY,
|
||||
SKILL_INSTALL_CANCEL_CAPABILITY,
|
||||
SKILL_INSTALL_PROGRESS_CAPABILITY,
|
||||
@@ -161,7 +162,8 @@ export const RUNTIME_CAPABILITIES = [
|
||||
SKILL_INSTALL_RESULT_V2_CAPABILITY,
|
||||
SKILL_UPLOAD_CAPABILITY,
|
||||
SKILL_MANAGEMENT_CAPABILITY,
|
||||
SKILL_INSTALL_PROVIDERS_CAPABILITY
|
||||
SKILL_INSTALL_PROVIDERS_CAPABILITY,
|
||||
SKILL_DELETE_CAPABILITY
|
||||
] as const
|
||||
|
||||
export type RuntimeCapability = (typeof RUNTIME_CAPABILITIES)[number] | (string & {})
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
MAX_SKILL_DELETE_BATCH,
|
||||
SkillDeletePlanSchema,
|
||||
SkillDeleteRequestSchema,
|
||||
SkillDeleteResultSchema
|
||||
} from './skill-delete-contract'
|
||||
|
||||
const SKILL = {
|
||||
id: 'skill-id',
|
||||
directoryPath: '/root/demo',
|
||||
skillFilePath: '/root/demo/SKILL.md',
|
||||
name: 'demo',
|
||||
updatedAt: 1
|
||||
}
|
||||
|
||||
describe('SkillDeleteRequestSchema', () => {
|
||||
it('accepts a request with no target, which is what the page sends', () => {
|
||||
expect(SkillDeleteRequestSchema.parse({ operationId: 'op', skills: [SKILL] })).toEqual({
|
||||
operationId: 'op',
|
||||
skills: [SKILL]
|
||||
})
|
||||
})
|
||||
|
||||
it('rejects an unknown top-level field rather than ignoring it', () => {
|
||||
// Deliberately strict: an old host must refuse a field that would change
|
||||
// what gets deleted, not silently drop it.
|
||||
expect(() =>
|
||||
SkillDeleteRequestSchema.parse({ operationId: 'op', skills: [SKILL], force: true })
|
||||
).toThrow()
|
||||
})
|
||||
|
||||
it('strips unknown keys inside the nested discovery target', () => {
|
||||
const parsed = SkillDeleteRequestSchema.parse({
|
||||
operationId: 'op',
|
||||
target: { runtime: 'host', somethingNew: 1 },
|
||||
skills: [SKILL]
|
||||
})
|
||||
expect(parsed.target).toEqual({ runtime: 'host' })
|
||||
})
|
||||
|
||||
it('keeps a null updatedAt, which the host fails closed on', () => {
|
||||
const parsed = SkillDeleteRequestSchema.parse({
|
||||
operationId: 'op',
|
||||
skills: [{ ...SKILL, updatedAt: null }]
|
||||
})
|
||||
expect(parsed.skills[0].updatedAt).toBeNull()
|
||||
})
|
||||
|
||||
it('refuses an empty or oversized batch', () => {
|
||||
expect(() => SkillDeleteRequestSchema.parse({ operationId: 'op', skills: [] })).toThrow()
|
||||
expect(() =>
|
||||
SkillDeleteRequestSchema.parse({
|
||||
operationId: 'op',
|
||||
skills: Array.from({ length: MAX_SKILL_DELETE_BATCH + 1 }, () => SKILL)
|
||||
})
|
||||
).toThrow()
|
||||
})
|
||||
})
|
||||
|
||||
describe('plan and result schemas', () => {
|
||||
it('carries a typed block reason so the band can group without string matching', () => {
|
||||
const plan = SkillDeletePlanSchema.parse({
|
||||
operationId: 'op',
|
||||
skills: [
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
canonicalPath: '/root/demo/SKILL.md',
|
||||
placements: [],
|
||||
blocked: 'bundled'
|
||||
}
|
||||
]
|
||||
})
|
||||
expect(plan.skills[0].blocked).toBe('bundled')
|
||||
})
|
||||
|
||||
it('rejects a block reason outside the shared vocabulary', () => {
|
||||
expect(() =>
|
||||
SkillDeletePlanSchema.parse({
|
||||
operationId: 'op',
|
||||
skills: [{ id: 'a', name: 'demo', canonicalPath: '/p', placements: [], blocked: 'because' }]
|
||||
})
|
||||
).toThrow()
|
||||
})
|
||||
|
||||
it('has no `unsupported` status, because such a delete is never issued', () => {
|
||||
expect(() =>
|
||||
SkillDeleteResultSchema.parse({
|
||||
operationId: 'op',
|
||||
skills: [{ id: 'a', name: 'demo', status: 'unsupported', removedPaths: [] }]
|
||||
})
|
||||
).toThrow()
|
||||
})
|
||||
|
||||
it('accepts a partial result carrying the staged paths', () => {
|
||||
const result = SkillDeleteResultSchema.parse({
|
||||
operationId: 'op',
|
||||
skills: [
|
||||
{
|
||||
id: 'a',
|
||||
name: 'demo',
|
||||
status: 'partial',
|
||||
removedPaths: [],
|
||||
stagedPaths: ['/root/.demo.orca-skill-delete-1']
|
||||
}
|
||||
]
|
||||
})
|
||||
expect(result.skills[0].stagedPaths).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,120 @@
|
||||
import { z } from 'zod'
|
||||
import { SkillDiscoveryTargetSchema } from './skills'
|
||||
|
||||
/** Why a closed vocabulary: the result band groups skips by reason, and a remote
|
||||
* host cannot localize free-text prose for the client that renders it. */
|
||||
export const SKILL_DELETE_BLOCK_REASONS = [
|
||||
'bundled',
|
||||
'plugin',
|
||||
'unowned',
|
||||
'missing',
|
||||
'stale'
|
||||
] as const
|
||||
export type SkillDeleteBlockReason = (typeof SKILL_DELETE_BLOCK_REASONS)[number]
|
||||
|
||||
export const SKILL_DELETE_PLACEMENT_KINDS = ['canonical', 'alias-dir', 'alias-file'] as const
|
||||
export type SkillDeletePlacementKind = (typeof SKILL_DELETE_PLACEMENT_KINDS)[number]
|
||||
|
||||
export const SKILL_DELETE_STATUSES = ['deleted', 'skipped', 'partial', 'failed', 'busy'] as const
|
||||
export type SkillDeleteStatus = (typeof SKILL_DELETE_STATUSES)[number]
|
||||
|
||||
/** One page of rows; a selection larger than this is not a thing the UI offers. */
|
||||
export const MAX_SKILL_DELETE_BATCH = 512
|
||||
|
||||
const SkillPathSchema = z.string().min(1).max(4096)
|
||||
|
||||
const SkillDeleteTargetSkillSchema = z
|
||||
.object({
|
||||
/** `DiscoveredSkill.id` — identity, so host and client agree without
|
||||
* re-deriving it from a path that may have just been renamed. */
|
||||
id: z.string().min(1).max(128),
|
||||
directoryPath: SkillPathSchema,
|
||||
skillFilePath: SkillPathSchema,
|
||||
name: z.string().min(1).max(256),
|
||||
/** `stat(skillFilePath).mtimeMs` as displayed; null fails the guard closed. */
|
||||
updatedAt: z.number().nullable()
|
||||
})
|
||||
.strict()
|
||||
|
||||
/** Strict on purpose (matching `SkillInstallRequestSchema`): an old host must
|
||||
* reject a field that would change what gets deleted, not ignore it. */
|
||||
export const SkillDeleteRequestSchema = z
|
||||
.object({
|
||||
operationId: z.string().min(1).max(128),
|
||||
// Send exactly what the scan sent — usually nothing.
|
||||
target: SkillDiscoveryTargetSchema.optional(),
|
||||
skills: z.array(SkillDeleteTargetSkillSchema).min(1).max(MAX_SKILL_DELETE_BATCH)
|
||||
})
|
||||
.strict()
|
||||
|
||||
export type SkillDeleteRequest = z.infer<typeof SkillDeleteRequestSchema>
|
||||
export type SkillDeleteTargetSkill = z.infer<typeof SkillDeleteTargetSkillSchema>
|
||||
|
||||
export type SkillDeletePlacement = {
|
||||
path: string
|
||||
kind: SkillDeletePlacementKind
|
||||
rootLabel: string
|
||||
}
|
||||
|
||||
export type SkillDeletePlanEntry = {
|
||||
id: string
|
||||
name: string
|
||||
/** realpath of `skillFilePath`. NOT the lock key — see the delete service. */
|
||||
canonicalPath: string
|
||||
placements: SkillDeletePlacement[]
|
||||
blocked?: SkillDeleteBlockReason
|
||||
}
|
||||
|
||||
export type SkillDeletePlan = {
|
||||
operationId: string
|
||||
skills: SkillDeletePlanEntry[]
|
||||
}
|
||||
|
||||
export type SkillDeleteResultEntry = {
|
||||
id: string
|
||||
name: string
|
||||
status: SkillDeleteStatus
|
||||
/** Present when status is 'skipped'; same vocabulary as the plan. */
|
||||
blocked?: SkillDeleteBlockReason
|
||||
removedPaths: string[]
|
||||
/** Present when status is 'partial'. */
|
||||
stagedPaths?: string[]
|
||||
}
|
||||
|
||||
export type SkillDeleteResult = {
|
||||
operationId: string
|
||||
skills: SkillDeleteResultEntry[]
|
||||
}
|
||||
|
||||
const SkillDeletePlacementSchema = z.object({
|
||||
path: SkillPathSchema,
|
||||
kind: z.enum(SKILL_DELETE_PLACEMENT_KINDS),
|
||||
rootLabel: z.string().max(256)
|
||||
})
|
||||
|
||||
export const SkillDeletePlanSchema: z.ZodType<SkillDeletePlan> = z.object({
|
||||
operationId: z.string().min(1).max(128),
|
||||
skills: z.array(
|
||||
z.object({
|
||||
id: z.string().min(1).max(128),
|
||||
name: z.string().max(256),
|
||||
canonicalPath: SkillPathSchema,
|
||||
placements: z.array(SkillDeletePlacementSchema).max(64),
|
||||
blocked: z.enum(SKILL_DELETE_BLOCK_REASONS).optional()
|
||||
})
|
||||
)
|
||||
})
|
||||
|
||||
export const SkillDeleteResultSchema: z.ZodType<SkillDeleteResult> = z.object({
|
||||
operationId: z.string().min(1).max(128),
|
||||
skills: z.array(
|
||||
z.object({
|
||||
id: z.string().min(1).max(128),
|
||||
name: z.string().max(256),
|
||||
status: z.enum(SKILL_DELETE_STATUSES),
|
||||
blocked: z.enum(SKILL_DELETE_BLOCK_REASONS).optional(),
|
||||
removedPaths: z.array(SkillPathSchema).max(64),
|
||||
stagedPaths: z.array(SkillPathSchema).max(64).optional()
|
||||
})
|
||||
)
|
||||
})
|
||||
@@ -0,0 +1,41 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
isSkillDeletable,
|
||||
skillDeletionEligibility,
|
||||
SKILL_DELETE_BLOCK_MESSAGES
|
||||
} from './skill-deletion-eligibility'
|
||||
import type { SkillSourceKind } from './skills'
|
||||
|
||||
describe('skillDeletionEligibility', () => {
|
||||
it.each<[SkillSourceKind, boolean]>([
|
||||
['home', true],
|
||||
['repo', true],
|
||||
['bundled', false],
|
||||
['plugin', false]
|
||||
])('decides %s', (sourceKind, deletable) => {
|
||||
expect(skillDeletionEligibility({ sourceKind }).deletable).toBe(deletable)
|
||||
expect(isSkillDeletable({ sourceKind })).toBe(deletable)
|
||||
})
|
||||
|
||||
it('names the reason a bundled skill would come back', () => {
|
||||
const eligibility = skillDeletionEligibility({ sourceKind: 'bundled' })
|
||||
expect(eligibility).toEqual({
|
||||
deletable: false,
|
||||
reason: 'bundled',
|
||||
message: SKILL_DELETE_BLOCK_MESSAGES.bundled
|
||||
})
|
||||
})
|
||||
|
||||
it('points a plugin skill at the plugin rather than the file', () => {
|
||||
const eligibility = skillDeletionEligibility({ sourceKind: 'plugin' })
|
||||
expect(eligibility).toMatchObject({ deletable: false, reason: 'plugin' })
|
||||
})
|
||||
|
||||
it('has a message for every host-only reason the client never predicts', () => {
|
||||
// The client cannot know these; they arrive on the result, and the band
|
||||
// still has to say something specific for each.
|
||||
expect(SKILL_DELETE_BLOCK_MESSAGES.unowned).toContain('outside')
|
||||
expect(SKILL_DELETE_BLOCK_MESSAGES.missing).toBeTruthy()
|
||||
expect(SKILL_DELETE_BLOCK_MESSAGES.stale).toBeTruthy()
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,54 @@
|
||||
import type { SkillDeleteBlockReason } from './skill-delete-contract'
|
||||
import type { DiscoveredSkill, SkillSourceKind } from './skills'
|
||||
|
||||
export type SkillDeletionEligibility =
|
||||
| { deletable: true }
|
||||
| { deletable: false; reason: SkillDeleteBlockReason; message: string }
|
||||
|
||||
/** English defaults. The renderer translates by reason code; logs and the CLI
|
||||
* use these directly. */
|
||||
export const SKILL_DELETE_BLOCK_MESSAGES: Record<SkillDeleteBlockReason, string> = {
|
||||
bundled: 'Bundled with Orca — it would be restored',
|
||||
plugin: 'Installed by a plugin — remove the plugin instead',
|
||||
unowned: 'This skill lives outside Orca’s skill folders — delete it where it is stored',
|
||||
missing: 'This skill is no longer on disk',
|
||||
stale: 'This skill changed since the list was loaded — refresh and try again'
|
||||
}
|
||||
|
||||
function blocked(reason: SkillDeleteBlockReason): SkillDeletionEligibility {
|
||||
return { deletable: false, reason, message: SKILL_DELETE_BLOCK_MESSAGES[reason] }
|
||||
}
|
||||
|
||||
/**
|
||||
* Client-side predicate over the displayed row.
|
||||
*
|
||||
* Advisory for `bundled`/`plugin`: `sourceKind` is first-scanned-root-wins, so a
|
||||
* plugin-cache skill symlinked into a plain home root reports `home` here. The
|
||||
* host re-derives the verdict from the placement set and can be stricter.
|
||||
*/
|
||||
export function skillDeletionEligibility(
|
||||
skill: Pick<DiscoveredSkill, 'sourceKind'>
|
||||
): SkillDeletionEligibility {
|
||||
return skillSourceKindDeletionEligibility(skill.sourceKind)
|
||||
}
|
||||
|
||||
export function skillSourceKindDeletionEligibility(
|
||||
sourceKind: SkillSourceKind
|
||||
): SkillDeletionEligibility {
|
||||
switch (sourceKind) {
|
||||
case 'bundled': {
|
||||
return blocked('bundled')
|
||||
}
|
||||
case 'plugin': {
|
||||
return blocked('plugin')
|
||||
}
|
||||
case 'home':
|
||||
case 'repo': {
|
||||
return { deletable: true }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function isSkillDeletable(skill: Pick<DiscoveredSkill, 'sourceKind'>): boolean {
|
||||
return skillDeletionEligibility(skill).deletable
|
||||
}
|
||||
@@ -0,0 +1,19 @@
|
||||
/**
|
||||
* One source for how deep a skill file may sit below a discovery root.
|
||||
*
|
||||
* Why one constant for two numbers: the native walker bounds *directories* and
|
||||
* WSL's `find -maxdepth` bounds the *file*, so the two spellings drifted apart
|
||||
* historically. Expressing the limit on the `SKILL.md` path and deriving the
|
||||
* directory bound from it keeps native, WSL, and the delete guard on one number.
|
||||
*/
|
||||
export const SKILL_FILE_MAX_DEPTH = 5
|
||||
export const PLUGIN_SKILL_FILE_MAX_DEPTH = 10
|
||||
|
||||
export function skillFileMaxDepth(sourceKind: 'home' | 'repo' | 'bundled' | 'plugin'): number {
|
||||
return sourceKind === 'plugin' ? PLUGIN_SKILL_FILE_MAX_DEPTH : SKILL_FILE_MAX_DEPTH
|
||||
}
|
||||
|
||||
/** The directory the walker may still descend into — one level above the file. */
|
||||
export function skillDirectoryMaxDepth(sourceKind: 'home' | 'repo' | 'bundled' | 'plugin'): number {
|
||||
return skillFileMaxDepth(sourceKind) - 1
|
||||
}
|
||||
@@ -12,3 +12,11 @@ export const SKILL_INSTALL_PROVIDERS_CAPABILITY = 'skills.install-providers.v1'
|
||||
|
||||
export const SKILL_INSTALL_UPDATE_REQUIRED_MESSAGE =
|
||||
'Update Orca on the selected machine to install shared skills.'
|
||||
|
||||
/** Why capability-gated rather than Rule 1: `skills.delete` is a new RPC method,
|
||||
* and an old host answers method-not-found, which surfaces as an unexplained
|
||||
* failure instead of a disabled action with a reason. */
|
||||
export const SKILL_DELETE_CAPABILITY = 'skills.delete.v1' as const
|
||||
|
||||
export const SKILL_DELETE_UPDATE_REQUIRED_MESSAGE =
|
||||
'Update Orca on the selected machine to delete skills.'
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
nativeSkillPathSemantics,
|
||||
normalizedSkillPath,
|
||||
POSIX_SKILL_PATH_SEMANTICS,
|
||||
skillPathDepthBelow,
|
||||
skillPathInside,
|
||||
skillPathsEqual,
|
||||
WINDOWS_SKILL_PATH_SEMANTICS
|
||||
} from './skill-path-containment'
|
||||
|
||||
describe('skillPathInside', () => {
|
||||
it('refuses the root itself, so a delete can never target a discovery root', () => {
|
||||
expect(
|
||||
skillPathInside(
|
||||
'/home/u/.claude/skills',
|
||||
'/home/u/.claude/skills',
|
||||
POSIX_SKILL_PATH_SEMANTICS
|
||||
)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('refuses a `..` escape', () => {
|
||||
expect(
|
||||
skillPathInside(
|
||||
'/home/u/.claude/skills',
|
||||
'/home/u/.claude/skills/../other',
|
||||
POSIX_SKILL_PATH_SEMANTICS
|
||||
)
|
||||
).toBe(false)
|
||||
expect(
|
||||
skillPathInside('/home/u/.claude/skills', '/home/u/other', POSIX_SKILL_PATH_SEMANTICS)
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('accepts a child', () => {
|
||||
expect(
|
||||
skillPathInside(
|
||||
'/home/u/.claude/skills',
|
||||
'/home/u/.claude/skills/foo',
|
||||
POSIX_SKILL_PATH_SEMANTICS
|
||||
)
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('keeps POSIX case-sensitive even when the process is win32', () => {
|
||||
// A WSL runtime is POSIX whatever `process.platform` says.
|
||||
expect(
|
||||
skillPathInside('/home/u/Skills', '/home/u/skills/foo', POSIX_SKILL_PATH_SEMANTICS)
|
||||
).toBe(false)
|
||||
expect(skillPathsEqual('/a/B', '/a/b', POSIX_SKILL_PATH_SEMANTICS)).toBe(false)
|
||||
})
|
||||
|
||||
it('folds case for Windows semantics', () => {
|
||||
expect(
|
||||
skillPathInside(
|
||||
'C:\\Users\\u\\.claude\\skills',
|
||||
'c:\\users\\U\\.claude\\skills\\foo',
|
||||
WINDOWS_SKILL_PATH_SEMANTICS
|
||||
)
|
||||
).toBe(true)
|
||||
expect(skillPathsEqual('C:\\A', 'c:\\a', WINDOWS_SKILL_PATH_SEMANTICS)).toBe(true)
|
||||
})
|
||||
|
||||
it('does not treat a `..`-prefixed name as a traversal', () => {
|
||||
expect(skillPathInside('/root', '/root/..cache', POSIX_SKILL_PATH_SEMANTICS)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('skillPathDepthBelow', () => {
|
||||
it('counts segments and returns null outside the root', () => {
|
||||
expect(skillPathDepthBelow('/r', '/r/a/SKILL.md', POSIX_SKILL_PATH_SEMANTICS)).toBe(2)
|
||||
expect(skillPathDepthBelow('/r', '/r/a/b/c/d/SKILL.md', POSIX_SKILL_PATH_SEMANTICS)).toBe(5)
|
||||
expect(skillPathDepthBelow('/r', '/other/SKILL.md', POSIX_SKILL_PATH_SEMANTICS)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('nativeSkillPathSemantics', () => {
|
||||
it('reads the platform it is given rather than the process it runs in', () => {
|
||||
expect(nativeSkillPathSemantics('win32')).toEqual(WINDOWS_SKILL_PATH_SEMANTICS)
|
||||
expect(nativeSkillPathSemantics('linux')).toEqual(POSIX_SKILL_PATH_SEMANTICS)
|
||||
expect(nativeSkillPathSemantics('darwin')).toEqual(POSIX_SKILL_PATH_SEMANTICS)
|
||||
})
|
||||
})
|
||||
|
||||
describe('normalizedSkillPath', () => {
|
||||
it('resolves before comparing', () => {
|
||||
expect(normalizedSkillPath('/a/b/../c', POSIX_SKILL_PATH_SEMANTICS)).toBe('/a/c')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,72 @@
|
||||
import { posix, win32 } from 'node:path'
|
||||
|
||||
/**
|
||||
* Path semantics of the host that owns the files, which is not always the host
|
||||
* running this process: a WSL runtime is POSIX and case-sensitive while the
|
||||
* process platform is `win32`, and a remote Orca runtime has its own platform.
|
||||
*/
|
||||
export type SkillPathSemantics = { caseSensitive: boolean; sep: '/' | '\\' }
|
||||
|
||||
export const POSIX_SKILL_PATH_SEMANTICS: SkillPathSemantics = { caseSensitive: true, sep: '/' }
|
||||
export const WINDOWS_SKILL_PATH_SEMANTICS: SkillPathSemantics = { caseSensitive: false, sep: '\\' }
|
||||
|
||||
/** Semantics of the process's own filesystem. */
|
||||
export function nativeSkillPathSemantics(
|
||||
platform: NodeJS.Platform = process.platform
|
||||
): SkillPathSemantics {
|
||||
return platform === 'win32' ? WINDOWS_SKILL_PATH_SEMANTICS : POSIX_SKILL_PATH_SEMANTICS
|
||||
}
|
||||
|
||||
function pathApi(semantics: SkillPathSemantics): typeof posix {
|
||||
return semantics.sep === '\\' ? win32 : posix
|
||||
}
|
||||
|
||||
function comparable(path: string, semantics: SkillPathSemantics): string {
|
||||
const resolved = pathApi(semantics).resolve(path)
|
||||
return semantics.caseSensitive ? resolved : resolved.toLocaleLowerCase('en-US')
|
||||
}
|
||||
|
||||
export function normalizedSkillPath(path: string, semantics: SkillPathSemantics): string {
|
||||
return comparable(path, semantics)
|
||||
}
|
||||
|
||||
export function skillPathsEqual(
|
||||
left: string,
|
||||
right: string,
|
||||
semantics: SkillPathSemantics
|
||||
): boolean {
|
||||
return comparable(left, semantics) === comparable(right, semantics)
|
||||
}
|
||||
|
||||
/** True when `path` sits strictly below `root`; the root itself is not inside. */
|
||||
export function skillPathInside(
|
||||
root: string,
|
||||
path: string,
|
||||
semantics: SkillPathSemantics
|
||||
): boolean {
|
||||
return relativeInside(root, path, semantics) !== null
|
||||
}
|
||||
|
||||
/** Segment count of `path` below `root`; null when `path` is not inside `root`. */
|
||||
export function skillPathDepthBelow(
|
||||
root: string,
|
||||
path: string,
|
||||
semantics: SkillPathSemantics
|
||||
): number | null {
|
||||
const child = relativeInside(root, path, semantics)
|
||||
return child === null ? null : child.split(semantics.sep).length
|
||||
}
|
||||
|
||||
function relativeInside(root: string, path: string, semantics: SkillPathSemantics): string | null {
|
||||
const api = pathApi(semantics)
|
||||
const child = api.relative(comparable(root, semantics), comparable(path, semantics))
|
||||
if (
|
||||
child === '' ||
|
||||
child === '..' ||
|
||||
child.startsWith(`..${semantics.sep}`) ||
|
||||
api.isAbsolute(child)
|
||||
) {
|
||||
return null
|
||||
}
|
||||
return child
|
||||
}
|
||||
Reference in New Issue
Block a user