Files
orca/src/shared/skills.ts
T
Neil a275f5ad85 fix(skills): cancel and bound abandoned skill discovery scans (#14670)
* fix(skills): cancel and bound abandoned skill discovery scans

A root on a stalled network mount never settles its readdir, so after 30s
the coalescer starts a replacement walk. The abandoned walk kept running
with no cancellation, and nothing counted it, so live filesystem work
accumulated for the life of the process.

Superseding a scan for age now aborts it, and `findSkillFiles` plus the
candidate tasks bail on the signal — that cannot unblock a syscall already
in the kernel, but it stops an abandoned walk issuing more of them. A
budget caps how many abandoned scans may be live at once; past it a
replacement is shed rather than started, and the stalled entry is left in
place so the root recovers on its own once the mount answers.

The budget counts abandoned scans rather than live ones on purpose: one
discovery legitimately walks a dozen-plus roots at once, so a cap on live
scans would shed healthy roots and empty the picker.

A `refresh` still does not abort what it supersedes — on a healthy root
that scan is about to finish and its callers want an answer, and the
existing publish fence already stops it writing a pre-mutation result.

Shed roots report the new `unavailable` skipped reason so they stay
distinct from roots that genuinely are not there.

* fix(skills): propagate a walk abort thrown through a symlinked directory

The broken-link `catch` around the symlink branch also wrapped the nested
`visit`, so an abort thrown from inside a symlinked subtree was swallowed.
When that link was the last entry, nothing afterwards re-checked the
signal and the walk returned a truncated listing as success — the exact
outcome the abort path exists to prevent.

Only the `stat` is guarded now. The test pins the narrow window by
aborting inside the symlink's stat, after the entry loop's check and
before the nested visit's.

* fix(skills): degrade an aborted root instead of failing the whole discovery

Aborting a scan abandoned for age gave the coalescer a way to reject that
it did not have before: every error inside the walk and the candidate
tasks was caught locally, so the task effectively could not fail. Callers
already waiting on that scan now see it reject, and `scanRootShared`
re-threw anything that was not a shed — so one slow root failed the
entire discovery and emptied the picker for every healthy root beside it,
which is exactly what the shed path was written to avoid.

Both endings now mean the same thing to a caller that can degrade a
single root, behind one predicate: shed before the walk began, or aborted
after it was abandoned.

`discoverSkillsOnTarget` runs a second coalescer over the whole target,
where there is no partial answer to degrade to, so it converts both into
a retryable error rather than leaking the internal class to IPC/RPC. It
must not answer with an empty result — zero skills reads as "nothing
installed" and re-offers installs for skills that are present.

Also drops the scan key from the shed message, which carried an absolute
workspace path to a paired client and the renderer's error string, and
corrects the budget comment: it is global and per-abandonment, so one
wedged root can spend it alone, one replacement per 30s window.

* fix(skills): keep the original error as the cause of a stalled-target error

The target layer replaces the internal error with a user-facing one, which
discarded what actually went wrong. Attaching it as `cause` keeps that in
logs while the message stays free of the host path.

Also records why name-matching is narrow rather than broad: the walk and
the candidate tasks catch every filesystem error locally, so the only
AbortError that can escape a scan is the one its own signal raised.

* test(skills): pin that a real abort produces the name the predicate matches

`isSkillRootUnavailableError` decides on `error.name`, and the discovery
and target tests fabricate that shape rather than driving a real abort. So
nothing pinned the linkage: if `throwIfAborted()` stopped producing an
`AbortError` that is an `instanceof Error`, every test would still pass
while a stalled root began failing whole discoveries again.

The walk test already drives a genuine abort, so it asserts the name, the
Error subclassing DOMException relies on, and the predicate itself.
2026-08-14 21:46:09 -07:00

114 lines
3.7 KiB
TypeScript

import { z } from 'zod'
import type { AgentType } from './agent-status-types'
import type { ProjectExecutionRuntimeResolution } from './project-execution-runtime'
export type SkillProvider = 'codex' | 'claude' | 'agent-skills'
export type SkillSourceKind = 'home' | 'repo' | 'bundled' | 'plugin'
export type DiscoveredSkill = {
id: string
name: string
description: string | null
providers: SkillProvider[]
sourceKind: SkillSourceKind
sourceLabel: string
rootPath: string
/** Every root that reached this file. Canonical-path dedup keeps one row but
* must not erase co-owning roots, or shared symlinked skills lose agents. */
rootPaths?: string[]
directoryPath: string
skillFilePath: string
installed: boolean
updatedAt: number | null
}
export type SkillDiscoverySource = {
id: string
label: string
path: string
sourceKind: SkillSourceKind
providers: SkillProvider[]
/** Agent that owns this root; null is the explicit shared-skills scope. */
owner: AgentType | null
exists: boolean
/** `unavailable`: the root did not answer in time, so its skills are unknown rather than absent. */
skippedReason?: 'missing' | 'remote-repo' | 'unavailable'
}
export type SkillDiscoveryResult = {
skills: DiscoveredSkill[]
sources: SkillDiscoverySource[]
scannedAt: number
}
export type SkillDiscoveryTarget = {
runtime?: 'host' | 'wsl'
wslDistro?: string | null
/** Workspace path whose local .agents/.claude skill roots should be scanned. */
cwd?: string | null
/** Lets the owning runtime resolve the project runtime from its own store
* when the caller (e.g. a remote client) cannot supply `projectRuntime`. */
worktreeId?: string | null
projectRuntime?: ProjectExecutionRuntimeResolution
/** Bypass the host's shared scans because the caller knows disk just changed.
* Optional so an older host simply ignores it and scans as it always did. */
refresh?: boolean
}
const ResolvedProjectRuntimeSchema = z.object({
status: z.literal('resolved'),
runtime: z.discriminatedUnion('kind', [
z.object({
kind: z.literal('local-host'),
hostPlatform: z.string(),
projectId: z.string(),
reason: z.literal('non-windows'),
cacheKey: z.string()
}),
z.object({
kind: z.literal('windows-host'),
hostPlatform: z.literal('win32'),
projectId: z.string(),
reason: z.enum(['project-override', 'global-default', 'migration-fallback']),
cacheKey: z.string()
}),
z.object({
kind: z.literal('wsl'),
hostPlatform: z.literal('wsl'),
projectId: z.string(),
distro: z.string(),
reason: z.enum(['project-override', 'global-default']),
cacheKey: z.string()
})
])
})
const RepairProjectRuntimeSchema = z.object({
status: z.literal('repair-required'),
repair: z.object({
projectId: z.string(),
preferredRuntime: z.object({ kind: z.literal('wsl'), distro: z.string().nullable() }),
reason: z.enum(['wsl-unavailable', 'wsl-distro-required', 'wsl-distro-missing']),
source: z.enum(['project-override', 'global-default']),
cacheKey: z.string()
})
})
/** Both desktop IPC and runtime RPC parse the complete discovery target here. */
export const SkillDiscoveryTargetSchema: z.ZodType<SkillDiscoveryTarget> = z.object({
runtime: z.enum(['host', 'wsl']).optional(),
wslDistro: z.string().nullable().optional(),
cwd: z.string().nullable().optional(),
worktreeId: z.string().nullable().optional(),
projectRuntime: z
.discriminatedUnion('status', [ResolvedProjectRuntimeSchema, RepairProjectRuntimeSchema])
.optional(),
refresh: z.boolean().optional()
})
export type SkillFrontmatterSummary = {
name: string | null
description: string | null
}