diff --git a/src/main/providers/ssh-skill-discovery-dispatch.ts b/src/main/providers/ssh-skill-discovery-dispatch.ts new file mode 100644 index 00000000000..90686b51045 --- /dev/null +++ b/src/main/providers/ssh-skill-discovery-dispatch.ts @@ -0,0 +1,31 @@ +import type { SshSkillDiscoveryProvider } from './ssh-skill-discovery-provider' + +const sshProviders = new Map() + +export const SSH_SKILL_DISCOVERY_PROVIDER_UNAVAILABLE_MESSAGE = + 'Remote connection dropped. Click Reconnect on the SSH target before retrying.' + +export function registerSshSkillDiscoveryProvider( + connectionId: string, + provider: SshSkillDiscoveryProvider +): void { + sshProviders.set(connectionId, provider) +} + +export function unregisterSshSkillDiscoveryProvider(connectionId: string): void { + sshProviders.delete(connectionId) +} + +export function getSshSkillDiscoveryProvider( + connectionId: string +): SshSkillDiscoveryProvider | undefined { + return sshProviders.get(connectionId) +} + +export function requireSshSkillDiscoveryProvider(connectionId: string): SshSkillDiscoveryProvider { + const provider = getSshSkillDiscoveryProvider(connectionId) + if (!provider) { + throw new Error(SSH_SKILL_DISCOVERY_PROVIDER_UNAVAILABLE_MESSAGE) + } + return provider +} diff --git a/src/main/providers/ssh-skill-discovery-provider.test.ts b/src/main/providers/ssh-skill-discovery-provider.test.ts new file mode 100644 index 00000000000..4bf5ec7155d --- /dev/null +++ b/src/main/providers/ssh-skill-discovery-provider.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it, vi } from 'vitest' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { + SshSkillDiscoveryProvider, + SshSkillDiscoveryUnsupportedError +} from './ssh-skill-discovery-provider' + +const VALID_RESULT = { + skills: [ + { + id: 'docs', + name: 'docs', + description: null, + providers: ['agent-skills'], + sourceKind: 'repo', + sourceLabel: 'Repo skills', + rootPath: '/remote/repo/.agents/skills', + directoryPath: '/remote/repo/.agents/skills/docs', + skillFilePath: '/remote/repo/.agents/skills/docs/SKILL.md', + installed: true, + fileCount: 1, + updatedAt: null + } + ], + sources: [ + { + id: 'repo-agents', + label: 'Repo skills', + path: '/remote/repo/.agents/skills', + sourceKind: 'repo', + providers: ['agent-skills'], + owner: null, + exists: true + } + ], + scannedAt: 12345 +} + +function providerWith(request: ReturnType): SshSkillDiscoveryProvider { + return new SshSkillDiscoveryProvider('conn-1', { request } as unknown as SshChannelMultiplexer) +} + +describe('SshSkillDiscoveryProvider', () => { + it('sends one skills.discover request with cwd, signal, and a bounded timeout', async () => { + const request = vi.fn().mockResolvedValue(VALID_RESULT) + const controller = new AbortController() + + const result = await providerWith(request).discover('/remote/repo', { + signal: controller.signal + }) + + expect(request).toHaveBeenCalledTimes(1) + expect(request).toHaveBeenCalledWith( + 'skills.discover', + { cwd: '/remote/repo' }, + { signal: controller.signal, timeoutMs: 9_000 } + ) + expect(result.skills.map((skill) => skill.name)).toEqual(['docs']) + }) + + it('maps JSON-RPC method-not-found to the relay upgrade error', async () => { + const request = vi + .fn() + .mockRejectedValue( + Object.assign(new Error('Method not found: skills.discover'), { code: -32601 }) + ) + + await expect(providerWith(request).discover('/remote/repo')).rejects.toBeInstanceOf( + SshSkillDiscoveryUnsupportedError + ) + }) + + it('rejects malformed relay responses instead of trusting the frame', async () => { + const malformed = { + ...VALID_RESULT, + skills: [{ ...VALID_RESULT.skills[0], fileCount: Number.POSITIVE_INFINITY }] + } + const request = vi.fn().mockResolvedValue(malformed) + + await expect(providerWith(request).discover('/remote/repo')).rejects.toThrow() + }) + + it('rejects oversized strings from the relay', async () => { + const malformed = { + ...VALID_RESULT, + skills: [{ ...VALID_RESULT.skills[0], name: 'x'.repeat(10_000) }] + } + const request = vi.fn().mockResolvedValue(malformed) + + await expect(providerWith(request).discover('/remote/repo')).rejects.toThrow() + }) + + it('passes through other transport errors unchanged', async () => { + const request = vi.fn().mockRejectedValue(new Error('Request "skills.discover" timed out')) + + await expect(providerWith(request).discover('/remote/repo')).rejects.toThrow( + 'Request "skills.discover" timed out' + ) + }) +}) diff --git a/src/main/providers/ssh-skill-discovery-provider.ts b/src/main/providers/ssh-skill-discovery-provider.ts new file mode 100644 index 00000000000..1cbccff9a58 --- /dev/null +++ b/src/main/providers/ssh-skill-discovery-provider.ts @@ -0,0 +1,55 @@ +import type { SkillDiscoveryResult } from '../../shared/skills' +import { parseSkillDiscoveryResult } from '../../shared/skills' +import type { SshChannelMultiplexer } from '../ssh/ssh-channel-multiplexer' +import { JsonRpcErrorCode } from '../ssh/relay-protocol' + +/** Below the renderer's 10s runtime-RPC timeout and above the relay's 8s scan + * budget so each layer's classified error outlives the outer deadline. */ +const SKILL_DISCOVERY_REQUEST_TIMEOUT_MS = 9_000 + +/** Old relay without skills.discover; the runtime maps this to the typed + * relay-upgrade-required pane response instead of a generic host error. */ +export class SshSkillDiscoveryUnsupportedError extends Error { + constructor() { + super( + 'This SSH host is running an older Orca relay without skill discovery. Reconnect to deploy the latest relay, then try again.' + ) + this.name = 'SshSkillDiscoveryUnsupportedError' + } +} + +function isJsonRpcMethodNotFoundError(error: unknown): boolean { + if (!error || typeof error !== 'object') { + return false + } + return (error as { code?: unknown }).code === JsonRpcErrorCode.MethodNotFound +} + +export class SshSkillDiscoveryProvider { + constructor( + private readonly connectionId: string, + private readonly mux: SshChannelMultiplexer + ) {} + + getConnectionId(): string { + return this.connectionId + } + + async discover(cwd: string, options?: { signal?: AbortSignal }): Promise { + let raw: unknown + try { + raw = await this.mux.request( + 'skills.discover', + { cwd }, + { signal: options?.signal, timeoutMs: SKILL_DISCOVERY_REQUEST_TIMEOUT_MS } + ) + } catch (error) { + if (isJsonRpcMethodNotFoundError(error)) { + throw new SshSkillDiscoveryUnsupportedError() + } + throw error + } + // Why: the relay frame is remote input; validate before it reaches renderer state. + return parseSkillDiscoveryResult(raw) + } +} diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 1a2aed7a74b..d33bf4aec94 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -666,6 +666,8 @@ import { } from '../project-runtime-git-options' import { resolveLocalProjectRuntimeForWorktreeId } from '../local-project-runtime-resolution' import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' +import type { SkillDiscoveryForPaneResponse } from '../../shared/skills' +import { discoverPaneSkills } from '../skills/pane-skill-discovery' import { resolveTerminalOrchestrationCliCommand } from './orchestration/cli-command' import { getLocalWorktreePathAccess, @@ -12272,6 +12274,37 @@ export class OrcaRuntimeService { : undefined } + // Why: the pane request carries identity only; this runtime derives the scan + // directory and execution host from its own state, so a renderer-claimed path + // can never reach a remote scan. + async discoverSkillsForPane( + args: { worktreeId: string; terminalTabId?: string }, + signal?: AbortSignal + ): Promise { + const target = await this.resolveRuntimeFileTarget(args.worktreeId) + const session = this.getWorkspaceSessionForWorktree(args.worktreeId) + const sessionWorktreeId = session + ? resolveTerminalSessionWorktreeId(session, args.worktreeId) + : null + const tab = + args.terminalTabId && session && sessionWorktreeId + ? session.tabsByWorktree[sessionWorktreeId]?.find( + (candidate) => candidate.id === args.terminalTabId + ) + : undefined + return discoverPaneSkills({ + worktreeId: args.worktreeId, + // A missing/unpersisted tab still scans a runtime-owned path: the workspace root. + cwd: tab?.startupCwd?.trim() || target.worktree.path, + connectionId: target.connectionId, + projectRuntime: target.connectionId + ? undefined + : this.resolveProjectRuntimeForWorktree(args.worktreeId), + repos: this.listRepos(), + signal + }) + } + getTerminalOrchestrationCliCommand(handle: string): 'orca' | 'orca-ide' { let pty: RuntimePtyWorktreeRecord | null = null try { diff --git a/src/main/runtime/rpc/methods/skills.ts b/src/main/runtime/rpc/methods/skills.ts index 346198aeffc..bd2fb7376e2 100644 --- a/src/main/runtime/rpc/methods/skills.ts +++ b/src/main/runtime/rpc/methods/skills.ts @@ -1,5 +1,8 @@ import { defineMethod, type RpcMethod } from '../core' -import { SkillDiscoveryTargetSchema } from '../../../../shared/skills' +import { + PaneSkillDiscoveryTargetSchema, + SkillDiscoveryTargetSchema +} from '../../../../shared/skills' import { discoverSkillsOnTarget, resolveSkillDiscoveryTarget @@ -21,5 +24,13 @@ export const SKILL_METHODS: RpcMethod[] = [ } return discoverSkillsOnTarget(resolveSkillDiscoveryTarget(target), runtime.listRepos()) } + }), + // Why: a separate method, not a field on skills.discover — old runtimes strip + // unknown fields and would answer with a wrong-host native scan; method_not_found + // lets clients classify the skew instead. + defineMethod({ + name: 'skills.discoverForPane', + params: PaneSkillDiscoveryTargetSchema, + handler: async (params, { runtime, signal }) => runtime.discoverSkillsForPane(params, signal) }) ] diff --git a/src/main/skills/discovery.test.ts b/src/main/skills/discovery.test.ts index 9f94e9f8347..efd05aae5a9 100644 --- a/src/main/skills/discovery.test.ts +++ b/src/main/skills/discovery.test.ts @@ -389,3 +389,55 @@ describe('skill discovery', () => { expect(result.skills.map((skill) => skill.name)).not.toContain('Too Deep') }) }) + +describe('skill discovery cancellation', () => { + async function makeScanFixture(): Promise<{ home: string; repo: string }> { + const root = await mkdtemp(join(tmpdir(), 'orca-skills-abort-')) + const home = join(root, 'home') + const repo = join(root, 'repo') + const repoSkill = join(repo, '.agents', 'skills', 'docs') + await mkdir(join(home, '.codex', 'skills', 'review'), { recursive: true }) + await mkdir(repoSkill, { recursive: true }) + await writeFile(join(home, '.codex', 'skills', 'review', 'SKILL.md'), '# Review\n') + await writeFile(join(repoSkill, 'SKILL.md'), '# Docs\n') + return { home, repo } + } + + it('rejects an already-aborted scan instead of resolving with empty results', async () => { + const { home, repo } = await makeScanFixture() + const controller = new AbortController() + controller.abort() + await expect( + discoverSkills({ homeDir: home, cwd: repo, repos: [], signal: controller.signal }) + ).rejects.toMatchObject({ name: 'AbortError' }) + }) + + it('rejects when aborted mid-scan and never publishes partial data', async () => { + const { home, repo } = await makeScanFixture() + const controller = new AbortController() + const pending = discoverSkills({ + homeDir: home, + cwd: repo, + repos: [], + signal: controller.signal + }) + controller.abort() + await expect(pending).rejects.toMatchObject({ name: 'AbortError' }) + }) + + it('returns identical successful results when a live signal is provided', async () => { + const { home, repo } = await makeScanFixture() + const controller = new AbortController() + const withSignal = await discoverSkills({ + homeDir: home, + cwd: repo, + repos: [], + signal: controller.signal + }) + const withoutSignal = await discoverSkills({ homeDir: home, cwd: repo, repos: [] }) + expect(withSignal.skills.map((skill) => skill.name).sort()).toEqual( + withoutSignal.skills.map((skill) => skill.name).sort() + ) + expect(withSignal.skills.length).toBeGreaterThan(0) + }) +}) diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index 00edfc8e705..0d19c7320b3 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -25,6 +25,16 @@ const SKILL_FILE_NAME = 'SKILL.md' const MAX_MARKDOWN_BYTES = 256 * 1024 const MAX_SKILL_FILES = 200 +/** Node-version-safe alternative to signal.throwIfAborted(); an aborted scan + * must reject rather than resolve with partial results. */ +function throwIfDiscoveryAborted(signal: AbortSignal | undefined): void { + if (signal?.aborted) { + const error = new Error('Skill discovery aborted') + error.name = 'AbortError' + throw error + } +} + async function pathExists(pathValue: string): Promise { try { await stat(pathValue) @@ -46,10 +56,15 @@ function isWithinDepth(rootPath: string, childPath: string, maxDepth: number): b return rel.split(sep).length <= maxDepth } -async function findSkillFiles(rootPath: string, maxDepth: number): Promise { +async function findSkillFiles( + rootPath: string, + maxDepth: number, + signal?: AbortSignal +): Promise { const out: string[] = [] const visitedDirectoryPaths = new Set() async function visit(dirPath: string): Promise { + throwIfDiscoveryAborted(signal) if (!isWithinDepth(rootPath, dirPath, maxDepth)) { return } @@ -71,6 +86,7 @@ async function findSkillFiles(rootPath: string, maxDepth: number): Promise { +async function countFiles(dirPath: string, signal?: AbortSignal): Promise { let count = 0 const visitedDirectoryPaths = new Set() async function visit(currentPath: string): Promise { + throwIfDiscoveryAborted(signal) if (count >= MAX_SKILL_FILES) { return } @@ -184,11 +201,12 @@ async function readSkillSummary(skillFilePath: string): Promise<{ type ScannedSkill = DiscoveredSkill & { canonicalSkillFilePath: string } -async function scanRoot(root: SkillScanRoot): Promise { +async function scanRoot(root: SkillScanRoot, signal?: AbortSignal): Promise { const maxDepth = root.sourceKind === 'plugin' ? 9 : 4 - const skillFiles = await findSkillFiles(root.path, maxDepth) + const skillFiles = await findSkillFiles(root.path, maxDepth, signal) const skills = await Promise.all( skillFiles.map(async (skillFilePath): Promise => { + throwIfDiscoveryAborted(signal) // Why: path identity belongs to the scanning host; canonicalizing before // returning prevents symlinked roots from becoming duplicate picker rows. const canonicalSkillFilePath = await realpath(skillFilePath).catch(() => skillFilePath) @@ -211,7 +229,7 @@ async function scanRoot(root: SkillScanRoot): Promise { directoryPath, skillFilePath, installed: true, - fileCount: await countFiles(directoryPath), + fileCount: await countFiles(directoryPath, signal), updatedAt: summary.updatedAt, canonicalSkillFilePath } satisfies ScannedSkill @@ -225,7 +243,10 @@ export async function discoverSkills(args: { homeDir?: string cwd?: string includeCwd?: boolean + signal?: AbortSignal }): Promise { + const signal = args.signal + throwIfDiscoveryAborted(signal) const homeDir = args.homeDir ?? homedir() const roots = [ ...buildSkillDiscoverySources({ ...args, homeDir }), @@ -238,6 +259,7 @@ export async function discoverSkills(args: { const sources: SkillDiscoverySource[] = [] const skillGroups = await Promise.all( roots.map(async (root) => { + throwIfDiscoveryAborted(signal) const exists = await pathExists(root.path) sources.push({ ...root, @@ -248,9 +270,10 @@ export async function discoverSkills(args: { if (!exists) { return [] } - return scanRoot(root) + return scanRoot(root, signal) }) ) + throwIfDiscoveryAborted(signal) const seen = new Map() for (const skill of skillGroups.flat()) { // Why: overlapping repo/cwd roots and symlinked provider homes can reach diff --git a/src/main/skills/pane-skill-discovery.test.ts b/src/main/skills/pane-skill-discovery.test.ts new file mode 100644 index 00000000000..7bd9c098f0a --- /dev/null +++ b/src/main/skills/pane-skill-discovery.test.ts @@ -0,0 +1,100 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { SkillDiscoveryResult } from '../../shared/skills' +import type { SshSkillDiscoveryProvider } from '../providers/ssh-skill-discovery-provider' +import { SshSkillDiscoveryUnsupportedError } from '../providers/ssh-skill-discovery-provider' +import { + registerSshSkillDiscoveryProvider, + unregisterSshSkillDiscoveryProvider +} from '../providers/ssh-skill-discovery-dispatch' + +const mocks = vi.hoisted(() => ({ + discoverSkillsOnTarget: vi.fn(), + resolveSkillDiscoveryTarget: vi.fn() +})) + +vi.mock('./skill-discovery-target', () => ({ + discoverSkillsOnTarget: mocks.discoverSkillsOnTarget, + resolveSkillDiscoveryTarget: mocks.resolveSkillDiscoveryTarget +})) + +import { discoverPaneSkills } from './pane-skill-discovery' + +const EMPTY_RESULT: SkillDiscoveryResult = { skills: [], sources: [], scannedAt: 1 } + +function fakeProvider(discover: ReturnType): SshSkillDiscoveryProvider { + return { getConnectionId: () => 'conn-1', discover } as unknown as SshSkillDiscoveryProvider +} + +describe('discoverPaneSkills', () => { + afterEach(() => { + unregisterSshSkillDiscoveryProvider('conn-1') + mocks.discoverSkillsOnTarget.mockReset() + mocks.resolveSkillDiscoveryTarget.mockReset() + }) + + it('routes SSH panes to the connection-scoped provider with the derived cwd', async () => { + const discover = vi.fn().mockResolvedValue(EMPTY_RESULT) + registerSshSkillDiscoveryProvider('conn-1', fakeProvider(discover)) + const signal = new AbortController().signal + + const response = await discoverPaneSkills({ + worktreeId: 'wt-1', + cwd: '/remote/repo', + connectionId: 'conn-1', + repos: [], + signal + }) + + expect(response).toEqual({ status: 'ok', result: EMPTY_RESULT }) + expect(discover).toHaveBeenCalledWith('/remote/repo', { signal }) + expect(mocks.discoverSkillsOnTarget).not.toHaveBeenCalled() + }) + + it('reports an old relay as relay-upgrade-required, never an empty success', async () => { + const discover = vi.fn().mockRejectedValue(new SshSkillDiscoveryUnsupportedError()) + registerSshSkillDiscoveryProvider('conn-1', fakeProvider(discover)) + + await expect( + discoverPaneSkills({ + worktreeId: 'wt-1', + cwd: '/remote/repo', + connectionId: 'conn-1', + repos: [] + }) + ).resolves.toEqual({ status: 'relay-upgrade-required' }) + }) + + it('fails a disconnected SSH pane instead of scanning local disk', async () => { + await expect( + discoverPaneSkills({ + worktreeId: 'wt-1', + cwd: '/remote/repo', + connectionId: 'conn-1', + repos: [] + }) + ).rejects.toThrow('Remote connection dropped') + expect(mocks.discoverSkillsOnTarget).not.toHaveBeenCalled() + }) + + it('keeps non-SSH panes on the existing local/WSL target resolution', async () => { + const resolved = { kind: 'native-host', cwd: '/local/repo' } + mocks.resolveSkillDiscoveryTarget.mockReturnValue(resolved) + mocks.discoverSkillsOnTarget.mockResolvedValue(EMPTY_RESULT) + const projectRuntime = { status: 'resolved' } as never + + const response = await discoverPaneSkills({ + worktreeId: 'wt-1', + cwd: '/local/repo', + projectRuntime, + repos: [] + }) + + expect(response).toEqual({ status: 'ok', result: EMPTY_RESULT }) + expect(mocks.resolveSkillDiscoveryTarget).toHaveBeenCalledWith({ + cwd: '/local/repo', + worktreeId: 'wt-1', + projectRuntime + }) + expect(mocks.discoverSkillsOnTarget).toHaveBeenCalledWith(resolved, []) + }) +}) diff --git a/src/main/skills/pane-skill-discovery.ts b/src/main/skills/pane-skill-discovery.ts new file mode 100644 index 00000000000..847e900e0d2 --- /dev/null +++ b/src/main/skills/pane-skill-discovery.ts @@ -0,0 +1,41 @@ +import type { Repo } from '../../shared/types' +import type { ProjectExecutionRuntimeResolution } from '../../shared/project-execution-runtime' +import type { SkillDiscoveryForPaneResponse } from '../../shared/skills' +import { + getSshSkillDiscoveryProvider, + SSH_SKILL_DISCOVERY_PROVIDER_UNAVAILABLE_MESSAGE +} from '../providers/ssh-skill-discovery-dispatch' +import { SshSkillDiscoveryUnsupportedError } from '../providers/ssh-skill-discovery-provider' +import { discoverSkillsOnTarget, resolveSkillDiscoveryTarget } from './skill-discovery-target' + +/** Executes a pane-scoped scan on the workspace's execution host. `cwd` and + * `connectionId` are runtime-resolved, never renderer input. */ +export async function discoverPaneSkills(args: { + worktreeId: string + cwd: string + connectionId?: string + projectRuntime?: ProjectExecutionRuntimeResolution + repos: readonly Repo[] + signal?: AbortSignal +}): Promise { + if (args.connectionId) { + const provider = getSshSkillDiscoveryProvider(args.connectionId) + if (!provider) { + throw new Error(SSH_SKILL_DISCOVERY_PROVIDER_UNAVAILABLE_MESSAGE) + } + try { + return { status: 'ok', result: await provider.discover(args.cwd, { signal: args.signal }) } + } catch (error) { + if (error instanceof SshSkillDiscoveryUnsupportedError) { + return { status: 'relay-upgrade-required' } + } + throw error + } + } + const target = resolveSkillDiscoveryTarget({ + cwd: args.cwd, + worktreeId: args.worktreeId, + ...(args.projectRuntime ? { projectRuntime: args.projectRuntime } : {}) + }) + return { status: 'ok', result: await discoverSkillsOnTarget(target, args.repos) } +} diff --git a/src/main/skills/relay-scanner-import-boundary.test.ts b/src/main/skills/relay-scanner-import-boundary.test.ts new file mode 100644 index 00000000000..3f5b3ccc99e --- /dev/null +++ b/src/main/skills/relay-scanner-import-boundary.test.ts @@ -0,0 +1,93 @@ +import { existsSync, readFileSync } from 'node:fs' +import { dirname, join, resolve } from 'node:path' +// TypeScript 7 is a native CLI; AST tests still need the legacy JavaScript API. +import ts from 'typescript-api' +import { describe, expect, it } from 'vitest' + +/** The relay bundle imports this scanner directly; an Electron import anywhere + * in its transitive graph would break every deployed SSH relay. */ +const RELAY_SCANNER_ENTRY = 'src/main/skills/discovery.ts' + +describe('relay-bundled skill scanner boundary', () => { + it('keeps the scanner import graph free of Electron', () => { + const visited = new Set() + const offenders: string[] = [] + const queue = [resolve(RELAY_SCANNER_ENTRY)] + while (queue.length > 0) { + const file = queue.pop() as string + if (visited.has(file)) { + continue + } + visited.add(file) + const source = readFileSync(file, 'utf8') + for (const specifier of collectModuleSpecifiers(file, source)) { + if (specifier === 'electron' || specifier.startsWith('electron/')) { + offenders.push(`${file} -> ${specifier}`) + continue + } + if (!specifier.startsWith('.')) { + continue + } + const resolved = resolveLocalModule(dirname(file), specifier) + if (resolved) { + queue.push(resolved) + } + } + } + expect(offenders).toEqual([]) + expect(visited.size).toBeGreaterThan(1) + }) +}) + +function resolveLocalModule(fromDir: string, specifier: string): string | null { + const base = resolve(fromDir, specifier) + for (const candidate of [base, `${base}.ts`, `${base}.tsx`, join(base, 'index.ts')]) { + if (candidate.endsWith('.ts') || candidate.endsWith('.tsx')) { + if (existsSync(candidate)) { + return candidate + } + } + } + return null +} + +function collectModuleSpecifiers(fileName: string, source: string): string[] { + const file = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true) + const specifiers: string[] = [] + + const visit = (node: ts.Node): void => { + if ( + (ts.isImportDeclaration(node) || ts.isExportDeclaration(node)) && + node.moduleSpecifier && + ts.isStringLiteral(node.moduleSpecifier) + ) { + specifiers.push(node.moduleSpecifier.text) + } + if ( + ts.isImportTypeNode(node) && + ts.isLiteralTypeNode(node.argument) && + ts.isStringLiteral(node.argument.literal) + ) { + specifiers.push(node.argument.literal.text) + } + if ( + ts.isCallExpression(node) && + isModuleLoader(node.expression) && + node.arguments.length === 1 && + ts.isStringLiteral(node.arguments[0]) + ) { + specifiers.push(node.arguments[0].text) + } + ts.forEachChild(node, visit) + } + + visit(file) + return specifiers +} + +function isModuleLoader(expression: ts.Expression): boolean { + return ( + expression.kind === ts.SyntaxKind.ImportKeyword || + (ts.isIdentifier(expression) && expression.text === 'require') + ) +} diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index f44d2a77dbd..fac155456fb 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -16,6 +16,7 @@ import { toAppSshPtyId, toRelaySshPtyId } from '../providers/ssh-pty-id' import { SshFilesystemProvider } from '../providers/ssh-filesystem-provider' import { isMethodNotFoundError } from './ssh-filesystem-stream-reader' import { SshGitProvider } from '../providers/ssh-git-provider' +import { SshSkillDiscoveryProvider } from '../providers/ssh-skill-discovery-provider' import { agentHookServer } from '../agent-hooks/server' import { isAgentStatusHooksEnabled } from '../agent-hooks/managed-agent-hook-controls' import { @@ -61,6 +62,10 @@ import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' import { registerSshGitProvider, unregisterSshGitProvider } from '../providers/ssh-git-dispatch' +import { + registerSshSkillDiscoveryProvider, + unregisterSshSkillDiscoveryProvider +} from '../providers/ssh-skill-discovery-dispatch' import { notifyRemoteWorkspaceHandlers } from '../ipc/remote-workspace-events' import { PortScanner } from './ssh-port-scanner' import { isMainWindowVisible, onMainWindowBecameVisible } from '../window/main-window-visibility' @@ -1065,6 +1070,11 @@ export class SshRelaySession { ) registerSshGitProvider(this.targetId, gitProvider) + registerSshSkillDiscoveryProvider( + this.targetId, + new SshSkillDiscoveryProvider(this.targetId, mux) + ) + this.wireUpPtyEvents(ptyProvider, mux, providerGeneration) this.wireUpAgentHookEvents(mux) this.wireUpRemoteWorkspaceEvents(mux) @@ -1572,6 +1582,7 @@ export class SshRelaySession { unregisterSshPtyProvider(this.targetId) unregisterSshFilesystemProvider(this.targetId) unregisterSshGitProvider(this.targetId) + unregisterSshSkillDiscoveryProvider(this.targetId) this.sourceIdentityByRelayPtyId.clear() this.retiredSourceDeliveries.clear() this.rejectedPtyRecoveryAttempts.clear() diff --git a/src/relay/relay.ts b/src/relay/relay.ts index 1da1dd70b5d..5aab2dc01e3 100644 --- a/src/relay/relay.ts +++ b/src/relay/relay.ts @@ -41,6 +41,7 @@ import { PortScanHandler } from './port-scan-handler' import { AgentExecHandler } from './agent-exec-handler' import { WorkspaceSessionHandler } from './workspace-session-handler' import { AiVaultHandler } from './ai-vault-handler' +import { SkillDiscoveryHandler } from './skill-discovery-handler' import { endpointDirForRelaySocket, RelayAgentHookServer } from './agent-hook-server' import { PluginOverlayManager } from './plugin-overlay' import { @@ -700,6 +701,9 @@ async function main(): Promise { const _aiVaultHandler = new AiVaultHandler(dispatcher) void _aiVaultHandler + const _skillDiscoveryHandler = new SkillDiscoveryHandler(dispatcher) + void _skillDiscoveryHandler + // Why: relay-hosted plugin provisioning is a later phase. Register the // enforcement boundary now with no consented identities or runtime services. registerRelayPluginHostCallHandlers( diff --git a/src/relay/skill-discovery-handler.test.ts b/src/relay/skill-discovery-handler.test.ts new file mode 100644 index 00000000000..d66ff1a79c7 --- /dev/null +++ b/src/relay/skill-discovery-handler.test.ts @@ -0,0 +1,111 @@ +import { mkdir, mkdtemp, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type * as NodeOs from 'node:os' +import type { MethodHandler, RequestContext } from './dispatcher' + +const mocks = vi.hoisted(() => ({ homedir: vi.fn(() => '/nonexistent-home') })) + +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, homedir: () => mocks.homedir() } +}) + +import { SkillDiscoveryHandler } from './skill-discovery-handler' + +function createHandler(): MethodHandler { + const handlers = new Map() + new SkillDiscoveryHandler({ + onRequest: (method: string, handler: MethodHandler): void => { + handlers.set(method, handler) + } + } as never) + const handler = handlers.get('skills.discover') + if (!handler) { + throw new Error('skills.discover handler not registered') + } + return handler +} + +function requestContext(signal?: AbortSignal): RequestContext { + return { clientId: 1, isStale: () => false, signal } +} + +describe('SkillDiscoveryHandler', () => { + beforeEach(() => { + mocks.homedir.mockReturnValue('/nonexistent-home') + }) + + it('discovers remote home and repository skills from the relay host', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-relay-skills-')) + const home = join(root, 'home') + const repo = join(root, 'repo') + await mkdir(join(home, '.codex', 'skills', 'review'), { recursive: true }) + await mkdir(join(repo, '.agents', 'skills', 'docs'), { recursive: true }) + await mkdir(join(repo, '.claude', 'skills', 'notes'), { recursive: true }) + await writeFile( + join(home, '.codex', 'skills', 'review', 'SKILL.md'), + ['---', 'name: code-review', 'description: Review changes.', '---', ''].join('\n') + ) + await writeFile(join(repo, '.agents', 'skills', 'docs', 'SKILL.md'), '# Docs\n') + await writeFile(join(repo, '.claude', 'skills', 'notes', 'SKILL.md'), '# Notes\n') + mocks.homedir.mockReturnValue(home) + + const result = (await createHandler()({ cwd: repo }, requestContext())) as { + skills: { name: string; providers: string[] }[] + sources: { exists: boolean }[] + } + + expect(result.skills.map((skill) => skill.name).sort()).toEqual([ + 'Docs', + 'Notes', + 'code-review' + ]) + expect(result.skills.find((skill) => skill.name === 'Docs')?.providers).toEqual([ + 'agent-skills' + ]) + expect(result.skills.find((skill) => skill.name === 'Notes')?.providers).toEqual(['claude']) + // Missing roots are reported exactly as local targeted discovery reports them. + expect(result.sources.some((source) => !source.exists)).toBe(true) + }) + + it('rejects a missing, empty, or oversized cwd', async () => { + const handler = createHandler() + await expect(handler({}, requestContext())).rejects.toThrow('Invalid skill discovery cwd') + await expect(handler({ cwd: ' ' }, requestContext())).rejects.toThrow( + 'Invalid skill discovery cwd' + ) + await expect(handler({ cwd: 'x'.repeat(5000) }, requestContext())).rejects.toThrow( + 'Invalid skill discovery cwd' + ) + }) + + it('rejects a cancelled request instead of publishing an empty success', async () => { + const controller = new AbortController() + controller.abort() + await expect( + createHandler()({ cwd: '/tmp/anywhere' }, requestContext(controller.signal)) + ).rejects.toMatchObject({ name: 'AbortError' }) + }) + + it('never includes full SKILL.md contents in the response', async () => { + const root = await mkdtemp(join(tmpdir(), 'orca-relay-skills-')) + const repo = join(root, 'repo') + await mkdir(join(repo, '.agents', 'skills', 'docs'), { recursive: true }) + // Name/description metadata may cross; body text past them must not. + const body = [ + '---', + 'name: docs', + 'description: Write docs.', + '---', + '', + 'SECRET-BODY-MARKER should never cross the relay boundary.' + ].join('\n') + await writeFile(join(repo, '.agents', 'skills', 'docs', 'SKILL.md'), body) + + const result = await createHandler()({ cwd: repo }, requestContext()) + + expect(JSON.stringify(result)).not.toContain('SECRET-BODY-MARKER') + }) +}) diff --git a/src/relay/skill-discovery-handler.ts b/src/relay/skill-discovery-handler.ts new file mode 100644 index 00000000000..637cf20047f --- /dev/null +++ b/src/relay/skill-discovery-handler.ts @@ -0,0 +1,57 @@ +import { homedir } from 'node:os' +import { discoverSkills } from '../main/skills/discovery' +import type { RelayDispatcher, RequestContext } from './dispatcher' +import type { SkillDiscoveryResult } from '../shared/skills' + +/** Below the desktop provider's 9s request timeout so the classified timeout + * error, not a generic transport timeout, is what reaches the picker. */ +const SCAN_BUDGET_MS = 8_000 +const MAX_CWD_LENGTH = 4096 + +export class SkillDiscoveryHandler { + constructor(dispatcher: RelayDispatcher) { + // Why: new method name — pre-skill relays answer -32601 so the desktop can + // prompt a reconnect instead of showing an empty skill list. + dispatcher.onRequest('skills.discover', (params, context) => this.discover(params, context)) + } + + private async discover( + params: Record, + context: RequestContext + ): Promise { + const cwd = typeof params.cwd === 'string' ? params.cwd.trim() : '' + if (!cwd || cwd.length > MAX_CWD_LENGTH) { + throw new Error('Invalid skill discovery cwd') + } + const budget = new AbortController() + let timedOut = false + const timer = setTimeout(() => { + timedOut = true + budget.abort() + }, SCAN_BUDGET_MS) + const onClientAbort = (): void => budget.abort() + if (context.signal?.aborted) { + budget.abort() + } else { + context.signal?.addEventListener('abort', onClientAbort, { once: true }) + } + try { + // Why: repos [] + relay-derived home keeps the scan targeted to this cwd's + // known skill roots; the client never supplies remote path semantics. + return await discoverSkills({ + repos: [], + cwd, + homeDir: homedir(), + signal: budget.signal + }) + } catch (error) { + if (timedOut) { + throw new Error('Skill discovery timed out on the SSH host') + } + throw error + } finally { + clearTimeout(timer) + context.signal?.removeEventListener('abort', onClientAbort) + } + } +} diff --git a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx index 8357931fb49..6e73d574852 100644 --- a/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx +++ b/src/renderer/src/components/native-chat/NativeChatAutocompleteMenus.tsx @@ -4,6 +4,7 @@ import { translate } from '@/i18n/i18n' import { cn } from '@/lib/utils' import type { SkillSourceKind } from '../../../../shared/skills' import type { ComposerAutocomplete, NativeChatPickerItem } from './native-chat-composer-state' +import type { NativeChatSkillDiscoveryErrorKind } from './native-chat-picker-items' export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ autocomplete, @@ -72,28 +73,16 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ ) : null} {autocomplete.skillStatus === 'error' ? ( - - {autocomplete.skillErrorKind === 'unavailable' - ? translate( - 'components.native-chat.composer.skillsUnavailableHost', - 'Skills are unavailable for this host' - ) - : translate( - 'components.native-chat.composer.skillsLoadFailed', - 'Could not load skills from this host' - )} - - {autocomplete.skillErrorKind !== 'unavailable' ? ( - - ) : null} + {getSkillErrorText(autocomplete.skillErrorKind)} + ) : null} {skills.map((item) => { @@ -116,10 +105,7 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ {autocomplete.skillStatus === 'loading' ? translate('components.native-chat.composer.loadingSkills', 'Loading skills...') : autocomplete.skillStatus === 'error' - ? translate( - 'components.native-chat.composer.skillsLoadFailed', - 'Could not load skills from this host' - ) + ? getSkillErrorText(autocomplete.skillErrorKind) : emptyText ? emptyText : autocomplete.skillsEnabled @@ -139,6 +125,25 @@ export const NativeChatPickerMenu = memo(function NativeChatPickerMenu({ ) }) +function getSkillErrorText(errorKind: NativeChatSkillDiscoveryErrorKind | undefined): string { + if (errorKind === 'relay-upgrade-required') { + return translate( + 'components.native-chat.composer.skillsSshRelayUpgrade', + 'Reconnect this SSH host to enable skills.' + ) + } + if (errorKind === 'runtime-upgrade-required') { + return translate( + 'components.native-chat.composer.skillsRuntimeUpgrade', + 'Update the remote runtime to enable skills.' + ) + } + return translate( + 'components.native-chat.composer.skillsLoadFailed', + 'Could not load skills from this host' + ) +} + function getPickerEmptyText( autocomplete: Extract ): string { diff --git a/src/renderer/src/components/native-chat/native-chat-picker-items.ts b/src/renderer/src/components/native-chat/native-chat-picker-items.ts index 2592ca0acab..478cf696af8 100644 --- a/src/renderer/src/components/native-chat/native-chat-picker-items.ts +++ b/src/renderer/src/components/native-chat/native-chat-picker-items.ts @@ -28,10 +28,17 @@ export type NativeChatPickerItem = sources: { sourceKind: SkillSourceKind; skillFilePath: string }[] } +export type NativeChatSkillDiscoveryErrorKind = + | 'timeout' + | 'host' + | 'unknown' + | 'relay-upgrade-required' + | 'runtime-upgrade-required' + export type NativeChatSkillDiscoverySnapshot = { status: 'idle' | 'loading' | 'ready' | 'error' skills: readonly DiscoveredSkill[] - errorKind?: 'unavailable' | 'timeout' | 'host' | 'unknown' + errorKind?: NativeChatSkillDiscoveryErrorKind } const PICKER_RESULT_LIMIT = 50 diff --git a/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts b/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts index 17e05af6a5c..8f3ecbee8e6 100644 --- a/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts +++ b/src/renderer/src/components/native-chat/native-chat-skill-discovery-context.ts @@ -1,6 +1,6 @@ import type { AppState } from '../../store/types' -import type { SkillDiscoveryTarget } from '../../../../shared/skills' -import { parseExecutionHostId } from '../../../../shared/execution-host' +import type { PaneSkillDiscoveryTarget, SkillDiscoveryTarget } from '../../../../shared/skills' +import { isRuntimeOwnedSshTargetId, parseExecutionHostId } from '../../../../shared/execution-host' import type { RuntimeClientTarget } from '@/runtime/runtime-rpc-client' import { getExplicitRuntimeEnvironmentIdForWorktree, @@ -19,6 +19,8 @@ export type NativeChatSkillStateInputs = Pick< | 'repos' | 'restoredRuntimeHostIdByWorkspaceSessionKey' | 'settings' + | 'sshConnectionStates' + | 'sshStateByEnvironment' | 'tabsByWorktree' | 'worktreesByRepo' > @@ -30,13 +32,24 @@ type NativeChatSkillWorktreeState = { worktreesByRepo: Record } -export type NativeChatSkillDiscoveryContext = { - key: string - cwd: string - executionHostKind: 'local' | 'runtime' | 'ssh' - runtimeTarget: RuntimeClientTarget - discoveryTarget: SkillDiscoveryTarget -} +export type NativeChatSkillDiscoveryContext = + | { + key: string + cwd: string + executionHostKind: 'local' | 'runtime' + runtimeTarget: RuntimeClientTarget + discoveryTarget: SkillDiscoveryTarget + } + | { + key: string + cwd: string + executionHostKind: 'ssh' + runtimeTarget: RuntimeClientTarget + /** Identity only; the owning runtime derives the scanned directory. */ + paneTarget: PaneSkillDiscoveryTarget + /** Known-disconnected host: skip the doomed RPC and show the host error. */ + sshDisconnected: boolean + } export function selectNativeChatSkillStateInputs(state: AppState): NativeChatSkillStateInputs { return { @@ -48,6 +61,8 @@ export function selectNativeChatSkillStateInputs(state: AppState): NativeChatSki repos: state.repos, restoredRuntimeHostIdByWorkspaceSessionKey: state.restoredRuntimeHostIdByWorkspaceSessionKey, settings: state.settings, + sshConnectionStates: state.sshConnectionStates, + sshStateByEnvironment: state.sshStateByEnvironment, tabsByWorktree: state.tabsByWorktree, worktreesByRepo: state.worktreesByRepo } @@ -99,12 +114,35 @@ export function resolveNativeChatSkillDiscoveryContext( const hostId = getExecutionHostIdForWorktree(state, worktreeId) const parsedHost = parseExecutionHostId(hostId) if (parsedHost?.kind === 'ssh') { + const runtimeEnvironmentId = getExplicitRuntimeEnvironmentIdForWorktree(state, worktreeId) + // Why: a runtime-owned SSH target without an explicit owner stamp is + // ambiguous; failing closed beats resolving it against local providers. + if (isRuntimeOwnedSshTargetId(parsedHost.targetId) && !runtimeEnvironmentId) { + return null + } + const connectionState = runtimeEnvironmentId + ? state.sshStateByEnvironment + .get(runtimeEnvironmentId) + ?.connectionStates.get(parsedHost.targetId) + : state.sshConnectionStates.get(parsedHost.targetId) + // Why: an unhydrated environment bucket is unknown, not disconnected — the + // owning runtime answers authoritatively. A missing local entry is offline. + const status = connectionState?.status ?? (runtimeEnvironmentId ? null : 'disconnected') return { - key: JSON.stringify(['ssh', hostId, cwd]), + key: JSON.stringify([ + 'ssh', + runtimeEnvironmentId ?? null, + hostId, + connectionState?.connectionGeneration ?? 0, + cwd + ]), cwd, executionHostKind: 'ssh', - runtimeTarget: { kind: 'local' }, - discoveryTarget: { cwd, worktreeId } + runtimeTarget: runtimeEnvironmentId + ? { kind: 'environment', environmentId: runtimeEnvironmentId } + : { kind: 'local' }, + paneTarget: { worktreeId, terminalTabId }, + sshDisconnected: status !== null && status !== 'connected' } } diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx index 647a07b2f4f..368bfc35df6 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.react.test.tsx @@ -25,6 +25,7 @@ import { resetNativeChatSkillDiscoveryCacheForTests, useNativeChatSkills } from './use-native-chat-skills' +import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' function stateForHost(hostId: string) { return { @@ -43,6 +44,8 @@ function stateForHost(hostId: string) { ], restoredRuntimeHostIdByWorkspaceSessionKey: {}, settings: { activeRuntimeEnvironmentId: null }, + sshConnectionStates: new Map(), + sshStateByEnvironment: new Map(), tabsByWorktree: { 'worktree-1': [{ id: 'tab-1' }] }, worktreesByRepo: { 'repo-1': [{ id: 'worktree-1', repoId: 'repo-1', path: '/repo/worktree', hostId }] @@ -50,46 +53,66 @@ function stateForHost(hostId: string) { } } +function connectedSshState() { + return { + ...stateForHost('ssh:target-1'), + sshConnectionStates: new Map([ + [ + 'target-1', + { + targetId: 'target-1', + status: 'connected', + error: null, + reconnectAttempt: 0, + connectionGeneration: 3 + } + ] + ]) + } +} + function Probe({ enabled }: { enabled: boolean }): null { mocks.snapshots.push(useNativeChatSkills('codex', 'tab-1', enabled)) return null } +const DISCOVERY_RESULT = { + skills: [ + { + id: 'browser', + name: 'browser', + description: null, + providers: ['agent-skills'], + sourceKind: 'home', + sourceLabel: 'Agent skills home', + rootPath: '/home/test/.agents/skills', + directoryPath: '/home/test/.agents/skills/browser', + skillFilePath: '/home/test/.agents/skills/browser/SKILL.md', + installed: true, + fileCount: 1, + updatedAt: null + } + ], + sources: [ + { + id: 'home-agents', + label: 'Agent skills home', + path: '/home/test/.agents/skills', + sourceKind: 'home', + providers: ['agent-skills'], + owner: null, + exists: true + } + ], + scannedAt: 1 +} + describe('useNativeChatSkills', () => { beforeEach(() => { mocks.state = stateForHost('local') mocks.snapshots = [] mocks.callRuntimeRpc.mockReset() - mocks.callRuntimeRpc.mockResolvedValue({ - skills: [ - { - id: 'browser', - name: 'browser', - description: null, - providers: ['agent-skills'], - sourceKind: 'home', - sourceLabel: 'Agent skills home', - rootPath: '/home/test/.agents/skills', - directoryPath: '/home/test/.agents/skills/browser', - skillFilePath: '/home/test/.agents/skills/browser/SKILL.md', - installed: true, - fileCount: 1, - updatedAt: null - } - ], - sources: [ - { - id: 'home-agents', - label: 'Agent skills home', - path: '/home/test/.agents/skills', - sourceKind: 'home', - providers: ['agent-skills'], - owner: null, - exists: true - } - ], - scannedAt: 1 - }) + mocks.callRuntimeRpc.mockResolvedValue(DISCOVERY_RESULT) resetNativeChatSkillDiscoveryCacheForTests() }) @@ -123,10 +146,44 @@ describe('useNativeChatSkills', () => { expect(mocks.callRuntimeRpc).toHaveBeenCalledTimes(1) }) - it('marks SSH discovery unavailable without scanning another host', async () => { - mocks.state = stateForHost('ssh:connection-1') + it('discovers SSH pane skills through the identity-only pane method', async () => { + mocks.state = connectedSshState() + mocks.callRuntimeRpc.mockResolvedValue({ status: 'ok', result: DISCOVERY_RESULT }) render() - await waitFor(() => expect(mocks.snapshots.at(-1)?.errorKind).toBe('unavailable')) + await waitFor(() => expect(mocks.snapshots.at(-1)?.status).toBe('ready')) + expect(mocks.snapshots.at(-1)?.skills.map((skill) => skill.name)).toEqual(['browser']) + expect(mocks.callRuntimeRpc).toHaveBeenCalledWith( + { kind: 'local' }, + 'skills.discoverForPane', + { worktreeId: 'worktree-1', terminalTabId: 'tab-1' }, + { timeoutMs: 10_000 } + ) + }) + + it('maps an old relay to relay-upgrade-required with Retry intact', async () => { + mocks.state = connectedSshState() + mocks.callRuntimeRpc.mockResolvedValue({ status: 'relay-upgrade-required' }) + render() + await waitFor(() => expect(mocks.snapshots.at(-1)?.errorKind).toBe('relay-upgrade-required')) + expect(mocks.snapshots.at(-1)?.status).toBe('error') + expect(typeof mocks.snapshots.at(-1)?.retry).toBe('function') + }) + + it('maps an old paired runtime (method_not_found) to runtime-upgrade-required', async () => { + mocks.state = connectedSshState() + mocks.callRuntimeRpc.mockRejectedValue( + new RuntimeRpcCallError({ + error: { code: 'method_not_found', message: 'Unknown method: skills.discoverForPane' } + } as never) + ) + render() + await waitFor(() => expect(mocks.snapshots.at(-1)?.errorKind).toBe('runtime-upgrade-required')) + }) + + it('shows a host error for a disconnected SSH host without issuing the RPC', async () => { + mocks.state = stateForHost('ssh:target-1') + render() + await waitFor(() => expect(mocks.snapshots.at(-1)?.errorKind).toBe('host')) expect(mocks.callRuntimeRpc).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts b/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts index 7154abb1043..e6b329ea266 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.test.ts @@ -2,8 +2,10 @@ import { describe, expect, it } from 'vitest' import type { DiscoveredSkill, SkillDiscoveryResult } from '../../../../shared/skills' import { isNativeChatSkillForAgent, + resolveNativeChatSkillDiscoveryContext, resolveNativeChatSkillDiscoveryCwd } from './use-native-chat-skills' +import type { NativeChatSkillStateInputs } from './native-chat-skill-discovery-context' function skill(overrides: Partial): DiscoveredSkill { return { @@ -158,3 +160,185 @@ describe('resolveNativeChatSkillDiscoveryCwd', () => { ).toBe('/repo/worktree/packages/app') }) }) + +function connectionState(overrides: Record = {}): Record { + return { + targetId: 'target-1', + status: 'connected', + error: null, + reconnectAttempt: 0, + connectionGeneration: 7, + ...overrides + } +} + +function sshInputs(overrides: Record = {}): NativeChatSkillStateInputs { + return { + activeRepoId: 'repo-1', + activeWorktreeId: 'worktree-1', + folderWorkspaces: [], + projectGroups: [], + projects: [], + repos: [{ id: 'repo-1', path: '/repo', connectionId: null, executionHostId: 'ssh:target-1' }], + restoredRuntimeHostIdByWorkspaceSessionKey: {}, + settings: { activeRuntimeEnvironmentId: null }, + sshConnectionStates: new Map([['target-1', connectionState()]]), + sshStateByEnvironment: new Map(), + tabsByWorktree: { 'worktree-1': [{ id: 'tab-1' }] }, + worktreesByRepo: { + 'repo-1': [ + { id: 'worktree-1', repoId: 'repo-1', path: '/repo/worktree', hostId: 'ssh:target-1' } + ] + }, + ...overrides + } as unknown as NativeChatSkillStateInputs +} + +describe('resolveNativeChatSkillDiscoveryContext for SSH panes', () => { + it('builds a pane-bound SSH context keyed on the connection generation', () => { + const context = resolveNativeChatSkillDiscoveryContext(sshInputs(), 'tab-1') + expect(context?.executionHostKind).toBe('ssh') + if (context?.executionHostKind !== 'ssh') { + throw new Error('expected ssh context') + } + expect(context.runtimeTarget).toEqual({ kind: 'local' }) + expect(context.paneTarget).toEqual({ worktreeId: 'worktree-1', terminalTabId: 'tab-1' }) + expect(context.sshDisconnected).toBe(false) + expect(context.key).toContain('7') + + const regenerated = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + sshConnectionStates: new Map([['target-1', connectionState({ connectionGeneration: 8 })]]) + }), + 'tab-1' + ) + // Reconnect bumps the generation, which must invalidate the pane cache key. + expect(regenerated?.key).not.toBe(context.key) + }) + + it('marks a missing or non-connected local SSH state as disconnected', () => { + const missing = resolveNativeChatSkillDiscoveryContext( + sshInputs({ sshConnectionStates: new Map() }), + 'tab-1' + ) + const reconnecting = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + sshConnectionStates: new Map([['target-1', connectionState({ status: 'reconnecting' })]]) + }), + 'tab-1' + ) + expect(missing?.executionHostKind === 'ssh' && missing.sshDisconnected).toBe(true) + expect(reconnecting?.executionHostKind === 'ssh' && reconnecting.sshDisconnected).toBe(true) + }) + + it('fails closed for a runtime-owned SSH target without an explicit owner', () => { + const context = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + repos: [ + { id: 'repo-1', path: '/repo', connectionId: null, executionHostId: 'ssh:runtime-ssh-t1' } + ], + worktreesByRepo: { + 'repo-1': [ + { + id: 'worktree-1', + repoId: 'repo-1', + path: '/repo/worktree', + hostId: 'ssh:runtime-ssh-t1' + } + ] + } + }), + 'tab-1' + ) + expect(context).toBeNull() + }) + + it('routes an owner-stamped runtime SSH target through its environment', () => { + const context = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + repos: [ + { id: 'repo-1', path: '/repo', connectionId: null, executionHostId: 'ssh:runtime-ssh-t1' } + ], + worktreesByRepo: { + 'repo-1': [ + { + id: 'worktree-1', + repoId: 'repo-1', + path: '/repo/worktree', + hostId: 'ssh:runtime-ssh-t1', + runtimeOwnerEnvironmentId: 'env-1' + } + ] + }, + sshStateByEnvironment: new Map([ + [ + 'env-1', + { + connectionStates: new Map([ + [ + 'runtime-ssh-t1', + connectionState({ targetId: 'runtime-ssh-t1', connectionGeneration: 4 }) + ] + ]), + targetLabels: new Map(), + removedTargetLabels: new Map(), + targetsHydrated: true + } + ] + ]) + }), + 'tab-1' + ) + expect(context?.executionHostKind).toBe('ssh') + if (context?.executionHostKind !== 'ssh') { + throw new Error('expected ssh context') + } + expect(context.runtimeTarget).toEqual({ kind: 'environment', environmentId: 'env-1' }) + expect(context.key).toContain('env-1') + expect(context.key).toContain('4') + expect(context.sshDisconnected).toBe(false) + }) + + it('treats an unhydrated environment bucket as unknown rather than disconnected', () => { + const context = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + repos: [ + { id: 'repo-1', path: '/repo', connectionId: null, executionHostId: 'ssh:runtime-ssh-t1' } + ], + worktreesByRepo: { + 'repo-1': [ + { + id: 'worktree-1', + repoId: 'repo-1', + path: '/repo/worktree', + hostId: 'ssh:runtime-ssh-t1', + runtimeOwnerEnvironmentId: 'env-1' + } + ] + } + }), + 'tab-1' + ) + expect(context?.executionHostKind === 'ssh' && context.sshDisconnected).toBe(false) + }) + + it('keeps the same remote path on two SSH hosts cache-isolated', () => { + const onTargetTwo = resolveNativeChatSkillDiscoveryContext( + sshInputs({ + repos: [ + { id: 'repo-1', path: '/repo', connectionId: null, executionHostId: 'ssh:target-2' } + ], + worktreesByRepo: { + 'repo-1': [ + { id: 'worktree-1', repoId: 'repo-1', path: '/repo/worktree', hostId: 'ssh:target-2' } + ] + }, + sshConnectionStates: new Map([['target-2', connectionState({ targetId: 'target-2' })]]) + }), + 'tab-1' + ) + const onTargetOne = resolveNativeChatSkillDiscoveryContext(sshInputs(), 'tab-1') + expect(onTargetOne?.cwd).toBe(onTargetTwo?.cwd) + expect(onTargetOne?.key).not.toBe(onTargetTwo?.key) + }) +}) diff --git a/src/renderer/src/components/native-chat/use-native-chat-skills.ts b/src/renderer/src/components/native-chat/use-native-chat-skills.ts index 42c7702d017..a461501630d 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-skills.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-skills.ts @@ -2,15 +2,22 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useShallow } from 'zustand/react/shallow' import { useAppStore } from '../../store' import type { AgentType } from '../../../../shared/agent-status-types' -import type { DiscoveredSkill, SkillDiscoveryResult } from '../../../../shared/skills' +import type { + DiscoveredSkill, + SkillDiscoveryForPaneResponse, + SkillDiscoveryResult +} from '../../../../shared/skills' import { getNativeChatAgentProfile } from '../../../../shared/native-chat-agent-profiles' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' +import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' +import { isRuntimeCompatBlockError } from '@/runtime/runtime-protocol-compat' import { emitNativeChatSkillDiscovery } from '@/lib/native-chat-telemetry' import { resolveNativeChatSkillDiscoveryContext, selectNativeChatSkillStateInputs, type NativeChatSkillDiscoveryContext } from './native-chat-skill-discovery-context' +import type { NativeChatSkillDiscoveryErrorKind } from './native-chat-picker-items' export { resolveNativeChatSkillDiscoveryContext, @@ -29,10 +36,18 @@ export type NativeChatSkillDiscovery = { status: 'idle' | 'loading' | 'ready' | 'error' skills: DiscoveredSkill[] error: Error | null - errorKind?: 'unavailable' | 'timeout' | 'host' | 'unknown' + errorKind?: NativeChatSkillDiscoveryErrorKind retry: () => void } +/** Old relay behind a current runtime; reconnecting the SSH host deploys it. */ +class SshRelaySkillUpgradeRequiredError extends Error { + constructor() { + super('This SSH host is running an older Orca relay without skill discovery.') + this.name = 'SshRelaySkillUpgradeRequiredError' + } +} + type StoredDiscoveryState = Omit & { contextKey: string | null } @@ -91,17 +106,17 @@ export function useNativeChatSkills( setState(IDLE_STATE) return } - if (context.executionHostKind === 'ssh') { + if (context.executionHostKind === 'ssh' && context.sshDisconnected) { emitNativeChatSkillDiscovery({ agent, - outcome: 'unavailable', + outcome: 'error', executionHostKind: 'ssh' }) setState({ status: 'error', skills: [], - error: new Error('Skill discovery is unavailable for SSH hosts.'), - errorKind: 'unavailable', + error: new Error('The SSH host is disconnected.'), + errorKind: 'host', contextKey: context.key }) return @@ -138,21 +153,24 @@ export function useNativeChatSkills( return } const error = reason instanceof Error ? reason : new Error(String(reason)) - const timedOut = /timed?\s*out|timeout/i.test(error.message) + const upgradeKind = classifyUpgradeRequired(context, error) + const timedOut = !upgradeKind && /timed?\s*out|timeout/i.test(error.message) emitNativeChatSkillDiscovery({ agent, - outcome: timedOut ? 'timeout' : 'error', + outcome: upgradeKind ? 'upgrade-required' : timedOut ? 'timeout' : 'error', executionHostKind: context.executionHostKind }) setState({ status: 'error', skills: [], error, - errorKind: timedOut - ? 'timeout' - : context.executionHostKind === 'runtime' - ? 'host' - : 'unknown', + errorKind: + upgradeKind ?? + (timedOut + ? 'timeout' + : context.executionHostKind === 'runtime' || context.executionHostKind === 'ssh' + ? 'host' + : 'unknown'), contextKey: paneCacheKey }) } @@ -210,14 +228,7 @@ function getOrStartDiscovery( // Why: the local runtime.call branch ignores timeoutMs, so the renderer must // enforce the design's scan timeout itself or a stalled local scan loads forever. const request = withDiscoveryTimeout( - callRuntimeRpc( - context.runtimeTarget, - 'skills.discover', - context.discoveryTarget, - { - timeoutMs: DISCOVERY_TIMEOUT_MS - } - ), + startDiscoveryRequest(context), DISCOVERY_BACKSTOP_TIMEOUT_MS ).finally(() => { if (inFlightDiscovery.get(context.key) === request) { @@ -228,6 +239,54 @@ function getOrStartDiscovery( return request } +async function startDiscoveryRequest( + context: NativeChatSkillDiscoveryContext +): Promise { + if (context.executionHostKind === 'ssh') { + const response = await callRuntimeRpc( + context.runtimeTarget, + 'skills.discoverForPane', + context.paneTarget, + { timeoutMs: DISCOVERY_TIMEOUT_MS } + ) + if (response.status === 'relay-upgrade-required') { + throw new SshRelaySkillUpgradeRequiredError() + } + return response.result + } + return callRuntimeRpc( + context.runtimeTarget, + 'skills.discover', + context.discoveryTarget, + { timeoutMs: DISCOVERY_TIMEOUT_MS } + ) +} + +function classifyUpgradeRequired( + context: NativeChatSkillDiscoveryContext, + error: Error +): Extract< + NativeChatSkillDiscoveryErrorKind, + 'relay-upgrade-required' | 'runtime-upgrade-required' +> | null { + if (context.executionHostKind !== 'ssh') { + return null + } + if (error instanceof SshRelaySkillUpgradeRequiredError) { + return 'relay-upgrade-required' + } + // Why: a runtime predating skills.discoverForPane would strip pane identity + // from the legacy method and scan its own disk; the missing method is the + // detectable version-skew signal (same mapping as native chat history). + if (error instanceof RuntimeRpcCallError && error.code === 'method_not_found') { + return 'runtime-upgrade-required' + } + if (isRuntimeCompatBlockError(error)) { + return 'runtime-upgrade-required' + } + return null +} + function withDiscoveryTimeout(promise: Promise, timeoutMs: number): Promise { return new Promise((resolve, reject) => { const timer = setTimeout(() => reject(new Error('Skill discovery timed out.')), timeoutMs) diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 3f940076736..431ca2b65b7 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -14773,7 +14773,8 @@ "skills": "Skills", "loadingSkills": "Loading skills...", "skillsLoaded": "Skills loaded", - "skillsUnavailableHost": "Skills are unavailable for this host", + "skillsSshRelayUpgrade": "Reconnect this SSH host to enable skills.", + "skillsRuntimeUpgrade": "Update the remote runtime to enable skills.", "skillsLoadFailed": "Could not load skills from this host", "retrySkills": "Retry", "skillCommandCollision": "Also a skill name - agent decides", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 33b14ec6fa7..fba5fce7c73 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -14458,7 +14458,8 @@ "commands": "Comandos", "skills": "Habilidades", "loadingSkills": "Cargando habilidades…", - "skillsUnavailableHost": "Las habilidades no están disponibles para este host", + "skillsSshRelayUpgrade": "Reconecta este host SSH para habilitar las habilidades.", + "skillsRuntimeUpgrade": "Actualiza el runtime remoto para habilitar las habilidades.", "skillsLoadFailed": "No se pudieron cargar las habilidades de este host", "retrySkills": "Reintentar", "skillCommandCollision": "También es un nombre de habilidad - el agente decide", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 24c94d5c823..9f0e3946851 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -14458,7 +14458,8 @@ "commands": "コマンド", "skills": "スキル", "loadingSkills": "スキルを読み込み中…", - "skillsUnavailableHost": "このホストではスキルが利用できません", + "skillsSshRelayUpgrade": "スキルを有効にするには、このSSHホストに再接続してください。", + "skillsRuntimeUpgrade": "スキルを有効にするには、リモートランタイムを更新してください。", "skillsLoadFailed": "このホストからスキルを読み込めませんでした", "retrySkills": "再試行", "skillCommandCollision": "スキル名でもあります - エージェントが判断します", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index ead7ea624f4..c1ff49fbb1b 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -14469,7 +14469,8 @@ "commands": "명령어", "skills": "스킬", "loadingSkills": "스킬 로딩 중…", - "skillsUnavailableHost": "이 호스트에서는 스킬을 사용할 수 없습니다", + "skillsSshRelayUpgrade": "스킬을 사용하려면 이 SSH 호스트를 다시 연결하세요.", + "skillsRuntimeUpgrade": "스킬을 사용하려면 원격 런타임을 업데이트하세요.", "skillsLoadFailed": "이 호스트에서 스킬을 불러올 수 없습니다", "retrySkills": "재시도", "skillCommandCollision": "스킬 이름이기도 합니다 - 에이전트가 결정합니다", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index b64faff2472..bb3a94502d6 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -14478,7 +14478,8 @@ "commands": "命令", "skills": "技能", "loadingSkills": "正在加载技能…", - "skillsUnavailableHost": "此主机不可用技能", + "skillsSshRelayUpgrade": "重新连接此 SSH 主机以启用技能。", + "skillsRuntimeUpgrade": "更新远程运行时以启用技能。", "skillsLoadFailed": "无法从此主机加载技能", "retrySkills": "重试", "skillCommandCollision": "也是技能名称 - 由代理决定", diff --git a/src/renderer/src/lib/native-chat-telemetry.ts b/src/renderer/src/lib/native-chat-telemetry.ts index 0d6f96584d2..7017de52fbd 100644 --- a/src/renderer/src/lib/native-chat-telemetry.ts +++ b/src/renderer/src/lib/native-chat-telemetry.ts @@ -91,7 +91,7 @@ export function emitNativeChatSendClassified(args: { export function emitNativeChatSkillDiscovery(args: { agent: NativeChatTelemetryAgent - outcome: 'ready' | 'error' | 'timeout' | 'unavailable' + outcome: 'ready' | 'error' | 'timeout' | 'upgrade-required' executionHostKind: 'local' | 'runtime' | 'ssh' }): void { track('native_chat_skill_discovery', { diff --git a/src/shared/skills.ts b/src/shared/skills.ts index c908954d3a9..fb60c53e60c 100644 --- a/src/shared/skills.ts +++ b/src/shared/skills.ts @@ -103,6 +103,77 @@ export const SkillDiscoveryTargetSchema: z.ZodType = z.obj .optional() }) +export type PaneSkillDiscoveryTarget = { + worktreeId: string + /** Pane whose runtime-persisted startup directory should be scanned. */ + terminalTabId?: string +} + +/** Pane discovery carries workspace/pane identity only — never a path or SSH + * connection id. The owning runtime derives the scan directory from its own + * persisted state. */ +export const PaneSkillDiscoveryTargetSchema: z.ZodType = z.object({ + worktreeId: z.string().min(1).max(512), + terminalTabId: z.string().min(1).max(512).optional() +}) + +/** Relay upgrade is a typed payload, not an error: old runtimes strip unknown + * request fields and old relays answer -32601, so clients must never have to + * string-match error messages to detect version skew. */ +export type SkillDiscoveryForPaneResponse = + | { status: 'ok'; result: SkillDiscoveryResult } + | { status: 'relay-upgrade-required' } + +const SKILL_PROVIDER_VALUES = ['codex', 'claude', 'agent-skills'] as const +const SKILL_SOURCE_KIND_VALUES = ['home', 'repo', 'bundled', 'plugin'] as const + +const boundedString = (max: number): z.ZodString => z.string().max(max) + +/** Validates untrusted skill metadata at the SSH relay boundary before it can + * enter renderer state: bounded strings, known enums, finite numbers. */ +export const SkillDiscoveryResultSchema = z.object({ + skills: z + .array( + z.object({ + id: boundedString(512), + name: boundedString(512), + description: boundedString(8192).nullable(), + providers: z.array(z.enum(SKILL_PROVIDER_VALUES)).max(8), + sourceKind: z.enum(SKILL_SOURCE_KIND_VALUES), + sourceLabel: boundedString(1024), + rootPath: boundedString(4096), + rootPaths: z.array(boundedString(4096)).max(64).optional(), + directoryPath: boundedString(4096), + skillFilePath: boundedString(4096), + installed: z.boolean(), + fileCount: z.number().int().min(0).max(1_000_000), + updatedAt: z.number().finite().nullable() + }) + ) + .max(5000), + sources: z + .array( + z.object({ + id: boundedString(512), + label: boundedString(1024), + path: boundedString(4096), + sourceKind: z.enum(SKILL_SOURCE_KIND_VALUES), + providers: z.array(z.enum(SKILL_PROVIDER_VALUES)).max(8), + owner: boundedString(64).nullable(), + exists: z.boolean(), + skippedReason: z.enum(['missing', 'remote-repo']).optional() + }) + ) + .max(1000), + scannedAt: z.number().finite() +}) + +export function parseSkillDiscoveryResult(value: unknown): SkillDiscoveryResult { + // Why: an unrecognized source owner only fails provider filtering, so the + // schema keeps owner as a bounded string rather than pinning AgentType. + return SkillDiscoveryResultSchema.parse(value) as SkillDiscoveryResult +} + export type SkillFrontmatterSummary = { name: string | null description: string | null diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 490bb162af6..7bf63a10722 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -529,7 +529,7 @@ const nativeChatSendClassifiedSchema = z const nativeChatSkillDiscoverySchema = z .object({ agent_kind: agentKindSchema, - outcome: z.enum(['ready', 'error', 'timeout', 'unavailable']), + outcome: z.enum(['ready', 'error', 'timeout', 'upgrade-required']), execution_host_kind: z.enum(['local', 'runtime', 'ssh']) }) .strict()