From 2e14806fe7a67d5be53018bb95a7eb6f10e46d47 Mon Sep 17 00:00:00 2001 From: brennanb2025 Date: Fri, 3 Jul 2026 13:51:12 -0700 Subject: [PATCH] Route Windows skill updates through repo-source add everywhere The update->add conversion for native Windows (skills update is unreliable there, PR #6307) silently stopped matching when update commands gained the non-interactive npx --yes ... --yes shape, and the new managed-skill flow bypassed it entirely: - Widen the renderer normalizer regex to match both command shapes, so all settings panels convert again on win32 - Build update-kind manual commands host-platform-aware in the main process (the modal command executes on the host, and web clients can't know the host platform from the browser) - Make the background auto-update runner reinstall from the repo source on win32 instead of spawning the unreliable skills update - Pin the normalizer tests to the real shared command constants so a future shape change can't silently strand the regex again Co-authored-by: Orca --- .../skills/managed-skill-update-contract.ts | 10 ++++- .../managed-skill-update-runner.test.ts | 32 +++++++++++++ .../skills/managed-skill-update-runner.ts | 18 +++++++- .../managed-skill-updates-safety.test.ts | 13 +++++- src/main/skills/managed-skill-updates.test.ts | 12 +++++ .../settings/CliSkillRuntimeSetup.test.tsx | 45 ++++++++++++++----- .../settings/CliSkillRuntimeSetup.tsx | 7 ++- .../settings/OrchestrationPane.test.tsx | 3 +- .../ManagedAgentSkillSetupDialogHost.tsx | 15 +++++-- 9 files changed, 134 insertions(+), 21 deletions(-) diff --git a/src/main/skills/managed-skill-update-contract.ts b/src/main/skills/managed-skill-update-contract.ts index 1ae914e9d57..7f0318da0b0 100644 --- a/src/main/skills/managed-skill-update-contract.ts +++ b/src/main/skills/managed-skill-update-contract.ts @@ -30,6 +30,14 @@ export function buildManagedSkillManualCommand( command: kind === 'install' ? buildAgentFeatureSkillInstallCommand([skillName]) - : buildAgentFeatureSkillUpdateCommand(skillName) + : buildManagedSkillHostUpdateCommand(skillName) } } + +function buildManagedSkillHostUpdateCommand(skillName: ManagedAgentSkillName): string { + // Why: `skills update` is unreliable on native Windows; reinstalling from the + // same repo source is idempotent and keeps the manual action working. + return process.platform === 'win32' + ? buildAgentFeatureSkillInstallCommand([skillName]) + : buildAgentFeatureSkillUpdateCommand(skillName) +} diff --git a/src/main/skills/managed-skill-update-runner.test.ts b/src/main/skills/managed-skill-update-runner.test.ts index 8996d762e26..626f76d00b2 100644 --- a/src/main/skills/managed-skill-update-runner.test.ts +++ b/src/main/skills/managed-skill-update-runner.test.ts @@ -90,6 +90,38 @@ describe('createManagedSkillUpdateRunner', () => { ) }) + it('reinstalls from the repo source instead of skills update on native Windows', async () => { + const child = new FakeChildProcess() + vi.mocked(spawn).mockReturnValue(child as never) + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + const runner = createManagedSkillUpdateRunner({ cwd: TEST_ALICE_HOME, timeoutMs: 1_000 }) + + const resultPromise = runner(ORCHESTRATION_SKILL_NAME) + child.emit('close', 0) + + await expect(resultPromise).resolves.toEqual({ status: 'success' }) + expect(spawn).toHaveBeenCalledWith( + 'npx.cmd', + [ + '--yes', + 'skills', + 'add', + 'https://github.com/stablyai/orca', + '--skill', + 'orchestration', + '--global', + '--yes' + ], + { + cwd: TEST_ALICE_HOME, + shell: false, + stdio: 'ignore', + windowsHide: true + } + ) + platform.mockRestore() + }) + it('uses taskkill for Windows process-tree cleanup on timeout', async () => { vi.useFakeTimers() const child = new FakeChildProcess(123) diff --git a/src/main/skills/managed-skill-update-runner.ts b/src/main/skills/managed-skill-update-runner.ts index ce47f4d271a..09e3af7110e 100644 --- a/src/main/skills/managed-skill-update-runner.ts +++ b/src/main/skills/managed-skill-update-runner.ts @@ -1,5 +1,6 @@ import { spawn, type ChildProcess } from 'node:child_process' import { homedir } from 'node:os' +import { ORCA_SKILLS_REPOSITORY_URL } from '../../shared/agent-feature-install-commands' import type { ManagedAgentSkillName } from '../../shared/skills' export type ManagedSkillUpdateRunnerResult = @@ -17,9 +18,24 @@ export function buildManagedSkillUpdateCommand(skillName: ManagedAgentSkillName) executable: string args: string[] } { + // Why: `skills update` is unreliable on native Windows; reinstalling from the + // same repo source is idempotent and passes the same post-update verification. + const args = + process.platform === 'win32' + ? [ + '--yes', + 'skills', + 'add', + ORCA_SKILLS_REPOSITORY_URL, + '--skill', + skillName, + '--global', + '--yes' + ] + : ['--yes', 'skills', 'update', skillName, '--global', '--yes'] return { executable: process.platform === 'win32' ? 'npx.cmd' : 'npx', - args: ['--yes', 'skills', 'update', skillName, '--global', '--yes'] + args } } diff --git a/src/main/skills/managed-skill-updates-safety.test.ts b/src/main/skills/managed-skill-updates-safety.test.ts index 4f3ec5c70d0..7f86b991e0d 100644 --- a/src/main/skills/managed-skill-updates-safety.test.ts +++ b/src/main/skills/managed-skill-updates-safety.test.ts @@ -277,9 +277,20 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => { args: ['--yes', 'skills', 'update', 'orchestration', '--global', '--yes'] }) const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + // Why: `skills update` is unreliable on native Windows, so the runner + // reinstalls from the repo source there instead. expect(buildManagedSkillUpdateCommand(ORCHESTRATION_SKILL_NAME)).toEqual({ executable: 'npx.cmd', - args: ['--yes', 'skills', 'update', 'orchestration', '--global', '--yes'] + args: [ + '--yes', + 'skills', + 'add', + 'https://github.com/stablyai/orca', + '--skill', + 'orchestration', + '--global', + '--yes' + ] }) platform.mockRestore() }) diff --git a/src/main/skills/managed-skill-updates.test.ts b/src/main/skills/managed-skill-updates.test.ts index 16cb17187a3..82ebf385c40 100644 --- a/src/main/skills/managed-skill-updates.test.ts +++ b/src/main/skills/managed-skill-updates.test.ts @@ -6,6 +6,7 @@ import { lockfile, orchestrationRequest } from './managed-skill-test-fixtures' +import { buildManagedSkillManualCommand } from './managed-skill-update-contract' import { ManagedSkillUpdateCoordinator } from './managed-skill-updates' describe('ManagedSkillUpdateCoordinator', () => { @@ -198,6 +199,17 @@ describe('ManagedSkillUpdateCoordinator', () => { expect(readTextFile).toHaveBeenCalledTimes(1) }) + it('serves the repo-source add command for manual updates on native Windows', () => { + const platform = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + try { + expect(buildManagedSkillManualCommand('update', ORCHESTRATION_SKILL_NAME).command).toBe( + 'npx --yes skills add https://github.com/stablyai/orca --skill orchestration --global --yes' + ) + } finally { + platform.mockRestore() + } + }) + it('lets explicit re-checks bypass automatic update cooldown', async () => { let now = 100 const updateRunner = vi.fn(async () => ({ status: 'failure' as const, exitCode: 1 })) diff --git a/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx b/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx index 64b7bb3dd1c..c76f75fb36f 100644 --- a/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx +++ b/src/renderer/src/components/settings/CliSkillRuntimeSetup.test.tsx @@ -1,6 +1,10 @@ import { describe, expect, it } from 'vitest' import { getDefaultSettings } from '../../../../shared/constants' -import { buildAgentFeatureSkillInstallCommand } from '../../../../shared/agent-feature-install-commands' +import { + buildAgentFeatureSkillInstallCommand, + ORCA_CLI_SKILL_UPDATE_COMMAND, + ORCHESTRATION_SKILL_UPDATE_COMMAND +} from '../../../../shared/agent-feature-install-commands' import { buildSkillCommandForRuntime, buildSkillInstallCommandForRuntime, @@ -22,18 +26,35 @@ describe('CliSkillRuntimeSetup runtime helpers', () => { }) it('wraps WSL skill updates with the same selected distro login shell', () => { - const command = buildSkillCommandForRuntime('npx skills update orchestration --global', { - runtime: 'wsl', - wslDistro: 'Fedora Remix', - label: 'WSL Fedora Remix' - }) + const command = buildSkillCommandForRuntime( + ORCHESTRATION_SKILL_UPDATE_COMMAND, + { + runtime: 'wsl', + wslDistro: 'Fedora Remix', + label: 'WSL Fedora Remix' + }, + 'win32' + ) expect(command).toContain("wsl.exe -d 'Fedora Remix' -- sh -c") expect(command).toContain('getent passwd') - expect(command).toContain('npx skills update orchestration --global') + expect(command).toContain(ORCHESTRATION_SKILL_UPDATE_COMMAND) }) it('reinstalls Windows-host skill updates through the add path', () => { + expect( + buildSkillCommandForRuntime( + ORCHESTRATION_SKILL_UPDATE_COMMAND, + { + runtime: 'host', + label: 'Windows' + }, + 'win32' + ) + ).toBe(buildAgentFeatureSkillInstallCommand(['orchestration'])) + }) + + it('reinstalls legacy-shape Windows-host skill updates through the add path', () => { expect( buildSkillCommandForRuntime( 'npx skills update orchestration --global', @@ -47,22 +68,22 @@ describe('CliSkillRuntimeSetup runtime helpers', () => { }) it('treats missing runtime as a Windows host fallback for skill updates', () => { - expect( - buildSkillCommandForRuntime('npx skills update orca-cli --global', undefined, 'win32') - ).toBe(buildAgentFeatureSkillInstallCommand(['orca-cli'])) + expect(buildSkillCommandForRuntime(ORCA_CLI_SKILL_UPDATE_COMMAND, undefined, 'win32')).toBe( + buildAgentFeatureSkillInstallCommand(['orca-cli']) + ) }) it('keeps non-Windows host skill updates on the update path', () => { expect( buildSkillCommandForRuntime( - 'npx skills update orchestration --global', + ORCHESTRATION_SKILL_UPDATE_COMMAND, { runtime: 'host', label: 'This device' }, 'linux' ) - ).toBe('npx skills update orchestration --global') + ).toBe(ORCHESTRATION_SKILL_UPDATE_COMMAND) }) it('preserves the selected WSL distro for skill discovery', () => { diff --git a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx index 895caa1b80a..b309af1cab3 100644 --- a/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx +++ b/src/renderer/src/components/settings/CliSkillRuntimeSetup.tsx @@ -100,7 +100,12 @@ function normalizeWindowsSkillUpdateCommand( } const trimmedCommand = command.trim() - const updateMatch = /^npx\s+skills\s+update\s+([A-Za-z0-9_-]+)\s+--global$/i.exec(trimmedCommand) + // Why: match both the legacy interactive shape and the non-interactive + // `npx --yes … --yes` shape emitted by buildAgentFeatureSkillUpdateCommand. + const updateMatch = + /^npx\s+(?:--yes\s+)?skills\s+update\s+([A-Za-z0-9_-]+)\s+--global(?:\s+--yes)?$/i.exec( + trimmedCommand + ) if (!updateMatch) { return command } diff --git a/src/renderer/src/components/settings/OrchestrationPane.test.tsx b/src/renderer/src/components/settings/OrchestrationPane.test.tsx index 208ea23cf22..be3b775c4aa 100644 --- a/src/renderer/src/components/settings/OrchestrationPane.test.tsx +++ b/src/renderer/src/components/settings/OrchestrationPane.test.tsx @@ -164,7 +164,8 @@ describe('OrchestrationPane', () => { expect(mocks.panelProps.at(-1)).toEqual( expect.objectContaining({ command: INSTALL_COMMAND, - installedCommand: UPDATE_COMMAND + // Why: this suite mocks a win32 host, where updates reinstall via add. + installedCommand: INSTALL_COMMAND }) ) diff --git a/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx b/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx index ca2a89e212b..cd49a6bb16c 100644 --- a/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx +++ b/src/renderer/src/components/skills/ManagedAgentSkillSetupDialogHost.tsx @@ -21,6 +21,7 @@ import { type ManagedAgentSkillDialogState } from './managed-agent-skill-dialog-state' import { AgentSkillSetupPanel } from '@/components/settings/AgentSkillSetupPanel' +import { buildSkillCommandForRuntime } from '@/components/settings/CliSkillRuntimeSetup' import { Button } from '@/components/ui/button' import { Dialog, @@ -126,10 +127,16 @@ export function ManagedAgentSkillSetupDialogHost(): React.JSX.Element | null { ? translate('auto.components.skills.ManagedAgentSkillSetupDialogHost.update', 'Update') : translate('auto.components.skills.ManagedAgentSkillSetupDialogHost.review', 'Review') const contextCopy = active ? getManagedSkillContextCopy(active.context, contextActionLabel) : '' - const installedCommand = useMemo( - () => (active ? buildAgentFeatureSkillUpdateCommand(active.skillName) : ''), - [active] - ) + const installedCommand = useMemo(() => { + if (!active) { + return '' + } + // Why: main builds update-kind manual commands host-platform-aware; only + // renderer-built fallbacks need the local Windows update->add conversion. + return active.manualCommand?.kind === 'update' + ? active.manualCommand.command + : buildSkillCommandForRuntime(buildAgentFeatureSkillUpdateCommand(active.skillName)) + }, [active]) const recheck = useCallback(async (): Promise => { if (!active || rechecking) {