fix(skills): correct the headless update run's verdict, cancel path, and stopping copy

Review fixes for the headless skill-update runner.

Main process:
- Judge per-skill outcomes on a positive signal. "Absent from
  eligibleUpdateNames" is not success: a deleted, half-written, or unreadable
  skill also leaves that list, so a corrupt update reported a green check.
  skillUpdateFailedNames now requires every convergent placement to come back
  current or newer-known.
- Retire a child's handlers with a per-run token. A failed spawn emits error
  *and* close, so the second settle clobbered the real spawn ENOENT; a
  cancelled child could also settle, or write output into, the run that
  replaced it. The token guards the rescan's finish closure too.
- Hold the run `running` until the killed process tree is actually dead.
  Releasing on the synchronous path let an immediate re-Update spawn a second
  npx writing the same bundles, with a watchdog so a sweep that never settles
  cannot wedge the run.
- Kill the tree, not just the npx wrapper, via killWithDescendantSweep.
- Publish an error instead of a silent `started: false` when the cmd.exe rail
  rejects the resolved npx path, which a profile directory containing & or %
  is enough to trigger on Windows.
- Coalesce captured output into one push per tick instead of structured-cloning
  the whole buffer to every window on each progress frame.

Renderer:
- Keep rows on screen while the settling re-scan runs. Refreshing the inventory
  nulls it synchronously, so every row vanished at the moment the result
  appeared. Rows render off the last good scan; eligibility stays on the live
  snapshot so nothing is authorized off stale bytes.
- Retry the names that failed, not the eligibility list that same re-scan has
  just emptied.
- Add Stop, restoring the escape hatch the embedded terminal used to provide,
  and say "stopping" on every surface rather than claiming the update keeps
  running in the background.
- Show a skipped skill's reason outside the disclosure, so it no longer depends
  on a mount-time defaultOpen a later re-scan can never re-fire.
- Drop the summary line telling users to open "Update details", a control this
  PR removes; it was translated into four languages.
- Keep the success linger from retiring a result the open dialog is showing.
- Delete skill-location-chip-copy.tsx: an unreferenced copy of the old row
  component, colliding on basename with the module that is actually imported.
This commit is contained in:
Brennan Benson
2026-07-27 16:19:34 -07:00
parent d8b3a03375
commit af01e09baf
18 changed files with 886 additions and 248 deletions
+3 -3
View File
@@ -12,6 +12,7 @@ import type {
} from '../../shared/skill-freshness'
import { inventorySkillFreshness } from '../skills/skill-freshness-inventory'
import { SkillUpdateRunner } from '../skills/skill-update-run'
import { skillUpdateFailedNames } from '../skills/skill-update-outcome'
import {
discoverSkillsOnTarget,
resolveSkillDiscoveryTarget
@@ -28,11 +29,10 @@ export function registerSkillsHandlers(store: Store): void {
const runner = new SkillUpdateRunner({
// Why: per-skill outcomes come from re-hashing what is actually on disk, not
// from scraping stdout. Anything still eligible after the run did not land.
// from scraping stdout.
rescanOutdatedNames: async (names) => {
const inventory = await scanInventory()
const stillEligible = new Set(inventory.eligibleUpdateNames)
return names.filter((name) => stillEligible.has(name))
return skillUpdateFailedNames(names, inventory.installations)
},
onState: (run: SkillUpdateRun) => {
for (const window of BrowserWindow.getAllWindows()) {
@@ -0,0 +1,96 @@
import { describe, expect, it } from 'vitest'
import type {
SkillFreshnessInstallation,
SkillFreshnessStatus,
SkillInstallationTopology
} from '../../shared/skill-freshness'
import { skillUpdateFailedNames } from './skill-update-outcome'
function placement(
name: string,
status: SkillFreshnessStatus,
topology: SkillInstallationTopology = 'canonical-copy'
): SkillFreshnessInstallation {
return {
id: `${name}-${topology}-${status}`,
name,
rootId: 'home-agents',
providers: ['agent-skills'],
sourceKind: 'home',
sourceLabel: 'Agent skills home',
unresolvedPath: `/home/.agents/skills/${name}`,
resolvedPath: `/home/.agents/skills/${name}`,
physicalIdentity: `physical-${name}`,
topology,
status,
installedReleaseRevision: 2,
installedAppVersion: '2.0.0',
currentReleaseRevision: 2,
currentPackageDigest: 'current',
currentAppVersion: '2.0.0',
observedPackageDigest: 'current',
errorCategory: null
}
}
describe('skillUpdateFailedNames', () => {
it('treats a convergent copy that is now current as landed', () => {
expect(skillUpdateFailedNames(['orca-cli'], [placement('orca-cli', 'current')])).toEqual([])
})
it('reports a copy the run left outdated', () => {
expect(skillUpdateFailedNames(['orca-cli'], [placement('orca-cli', 'outdated')])).toEqual([
'orca-cli'
])
})
it('reports a half-written bundle instead of reading it as success', () => {
// The old "still eligible?" test passed here: an unrecognized copy is not
// eligible either, so a corrupt write looked identical to a clean update.
expect(skillUpdateFailedNames(['orca-cli'], [placement('orca-cli', 'unrecognized')])).toEqual([
'orca-cli'
])
})
it('reports an unreadable copy', () => {
expect(skillUpdateFailedNames(['orca-cli'], [placement('orca-cli', 'inaccessible')])).toEqual([
'orca-cli'
])
})
it('reports a skill the run removed outright', () => {
expect(skillUpdateFailedNames(['orca-cli'], [])).toEqual(['orca-cli'])
})
it('accepts a revision newer than this build ships', () => {
// The CLI pulls from the source repo, which runs ahead of the bundled manifest.
expect(skillUpdateFailedNames(['orca-cli'], [placement('orca-cli', 'newer-known')])).toEqual([])
})
it('ignores placements the update command never writes to', () => {
expect(
skillUpdateFailedNames(
['orca-cli'],
[placement('orca-cli', 'current'), placement('orca-cli', 'outdated', 'plugin-cache')]
)
).toEqual([])
})
it('fails the name when any convergent alias was left behind', () => {
expect(
skillUpdateFailedNames(
['orca-cli'],
[placement('orca-cli', 'current'), placement('orca-cli', 'outdated', 'provider-alias')]
)
).toEqual(['orca-cli'])
})
it('judges each requested name independently', () => {
expect(
skillUpdateFailedNames(
['orca-cli', 'orchestration'],
[placement('orca-cli', 'current'), placement('orchestration', 'outdated')]
)
).toEqual(['orchestration'])
})
})
+39
View File
@@ -0,0 +1,39 @@
import {
SUPPORTED_GLOBAL_SKILL_TOPOLOGIES,
type SkillFreshnessInstallation
} from '../../shared/skill-freshness'
/**
* Names that did not land, judged from the post-run inventory.
*
* Why not "whatever is still in `eligibleUpdateNames`": a name leaves that list
* for reasons that are not success. A half-written bundle hashes to
* `unrecognized`, a failed write can leave the directory unreadable, and a
* removed skill has no convergent placement at all — each would read as
* "updated" and show a green check over a broken skill. So demand a positive
* signal instead: every placement the command writes to has to come back as a
* revision we recognise.
*
* Known limit: the topology filter runs before the status check, so a placement
* that degrades OUT of the convergent set is dropped from the judgment rather
* than failing it. A run that rewrites the canonical copy but leaves a
* provider-alias a broken symlink still reports success. Catching that needs the
* pre-run inventory to compare convergent-then against convergent-now; every
* wholly-degraded and removed-entirely case is already reported as a failure.
*/
export function skillUpdateFailedNames(
names: readonly string[],
installations: readonly SkillFreshnessInstallation[]
): string[] {
return names.filter((name) => {
const convergent = installations.filter(
(entry) => entry.name === name && SUPPORTED_GLOBAL_SKILL_TOPOLOGIES.has(entry.topology)
)
if (convergent.length === 0) {
return true
}
// `newer-known` counts as landed: the CLI pulls from the source repo, which
// can be ahead of the revision this build ships in its manifest.
return convergent.some((entry) => entry.status !== 'current' && entry.status !== 'newer-known')
})
}
+173 -1
View File
@@ -1,11 +1,12 @@
import { EventEmitter } from 'node:events'
import { describe, expect, it, vi } from 'vitest'
import type { SkillUpdateRun } from '../../shared/skill-freshness'
import { SkillUpdateRunner } from './skill-update-run'
import { CANCEL_RELEASE_TIMEOUT_MS, SkillUpdateRunner } from './skill-update-run'
class FakeChild extends EventEmitter {
stdout = new EventEmitter()
stderr = new EventEmitter()
pid: number | undefined = 1234
kill = vi.fn()
}
@@ -13,6 +14,8 @@ function makeRunner(
overrides: {
rescanOutdatedNames?: (names: string[]) => Promise<string[]>
resolveCommand?: (name: string) => string
killTree?: (pid: number, killRoot: () => void) => Promise<void>
buildSpawnArgs?: (command: string, args: string[]) => { spawnCmd: string; spawnArgs: string[] }
} = {}
) {
const child = new FakeChild()
@@ -22,6 +25,9 @@ function makeRunner(
now: () => 1000,
resolveCommand: overrides.resolveCommand ?? (() => '/usr/local/bin/npx'),
rescanOutdatedNames: overrides.rescanOutdatedNames,
// Default to a no-op sweep so tests never signal a real PID.
killTree: overrides.killTree ?? (async (_pid, killRoot) => killRoot()),
buildSpawnArgs: overrides.buildSpawnArgs,
onState: (run) => states.push(run),
spawnProcess: ((command: string, args: string[], options: Record<string, unknown>) => {
spawnCalls.push({ command, args, options })
@@ -129,6 +135,102 @@ describe('SkillUpdateRunner', () => {
expect(run.state === 'error' && run.message).toBe('spawn ENOENT')
})
it('keeps the spawn error when the failed child also emits close', async () => {
// A spawn failure emits `error` *then* `close`. Without a latch the second
// settle overwrites `spawn ENOENT` with a useless "exited with code null".
const rescan = vi.fn(async () => ['orca-cli'])
const { runner, child } = makeRunner({ rescanOutdatedNames: rescan })
runner.start(['orca-cli'])
child.emit('error', new Error('spawn ENOENT'))
child.emit('close', null)
await flush()
const run = runner.getState()
expect(run.state === 'error' && run.message).toBe('spawn ENOENT')
expect(rescan).toHaveBeenCalledTimes(1)
})
it('refuses a new run until the cancelled process tree is actually dead', async () => {
// The sweep waits for a descendant snapshot before it signals anything, so
// releasing the UI synchronously would let a second npx write the same
// bundles as the one still being killed.
let finishKill = (): void => {}
const { runner } = makeRunner({
killTree: (_pid, killRoot) =>
new Promise<void>((resolve) => {
finishKill = () => {
killRoot()
resolve()
}
})
})
runner.start(['orca-cli'])
runner.cancel()
await flush()
expect(runner.getState().state).toBe('running')
expect(runner.start(['orchestration'])).toEqual({ started: false, reason: 'already-running' })
finishKill()
await flush()
expect(runner.getState()).toEqual({ state: 'idle' })
expect(runner.start(['orchestration'])).toEqual({ started: true })
})
it('releases the run even if the kill sweep never settles', async () => {
vi.useFakeTimers()
try {
// Stop is already spent by this point, so a sweep that hangs would leave
// the run wedged in `running` with no way out.
const { runner } = makeRunner({ killTree: () => new Promise<void>(() => {}) })
runner.start(['orca-cli'])
runner.cancel()
const stopping = runner.getState()
expect(stopping.state).toBe('running')
// Surfaced so the dialog can retire the Stop affordance it already spent.
expect(stopping.state === 'running' && stopping.stopping).toBe(true)
await vi.advanceTimersByTimeAsync(CANCEL_RELEASE_TIMEOUT_MS)
expect(runner.getState()).toEqual({ state: 'idle' })
expect(runner.start(['orchestration'])).toEqual({ started: true })
} finally {
vi.useRealTimers()
}
})
it('does not let a cancelled child settle the run that replaced it', async () => {
const children: FakeChild[] = []
const states: SkillUpdateRun[] = []
const runner = new SkillUpdateRunner({
now: () => 1000,
resolveCommand: () => '/usr/local/bin/npx',
rescanOutdatedNames: async () => [],
// Never let a test reach the real sweep — it would signal live PIDs.
killTree: async (_pid, killRoot) => killRoot(),
onState: (run) => states.push(run),
spawnProcess: (() => {
const child = new FakeChild()
children.push(child)
return child as never
}) as never
})
runner.start(['orca-cli'])
runner.cancel()
await flush()
runner.start(['orchestration'])
// The killed child's exit lands after the replacement is already in flight.
children[0].stdout.emit('data', Buffer.from('output from the dead run'))
children[0].emit('close', 1)
await flush()
const run = runner.getState()
expect(run.state).toBe('running')
expect(run.state === 'running' && run.names).toEqual(['orchestration'])
expect(run.state === 'running' && run.output).toBe('')
expect(states.some((state) => state.state === 'error')).toBe(false)
})
it('returns to idle on cancel and stops reporting output', async () => {
const { runner, child, states } = makeRunner({ rescanOutdatedNames: async () => [] })
runner.start(['orca-cli'])
@@ -141,6 +243,76 @@ describe('SkillUpdateRunner', () => {
expect(states.at(-1)).toEqual({ state: 'idle' })
})
it('ignores a re-scan that resolves after the run was cancelled', async () => {
let releaseRescan = (): void => {}
const { runner, child } = makeRunner({
rescanOutdatedNames: () =>
new Promise<string[]>((resolve) => {
releaseRescan = () => resolve([])
})
})
runner.start(['orca-cli'])
child.emit('close', 0)
await flush()
// The re-scan re-hashes every package, so a cancel lands well inside it.
runner.cancel()
releaseRescan()
await flush()
expect(runner.getState()).toEqual({ state: 'idle' })
})
it('kills the whole npx tree on cancel, not just the wrapper', async () => {
const killTree = vi.fn(async (_pid: number, killRoot: () => void) => {
killRoot()
})
const { runner, child } = makeRunner({ killTree })
child.pid = 4242
runner.start(['orca-cli'])
runner.cancel()
await flush()
expect(killTree).toHaveBeenCalledWith(4242, expect.any(Function))
expect(child.kill).toHaveBeenCalled()
})
it('surfaces an unspawnable command path instead of silently doing nothing', async () => {
// A Windows profile directory containing `&` makes the cmd.exe rail reject
// the resolved npx path; the names are already canonical by this point.
const { runner, states } = makeRunner({
resolveCommand: () => 'C:\\Users\\A&B\\AppData\\Roaming\\npm\\npx.cmd',
buildSpawnArgs: () => {
throw new Error('unsafe batch arguments')
}
})
const result = runner.start(['orca-cli'])
expect(result.started).toBe(false)
const run = runner.getState()
expect(run.state).toBe('error')
expect(run.state === 'error' && run.failedNames).toEqual(['orca-cli'])
expect(states.at(-1)?.state).toBe('error')
})
it('coalesces progress frames into one push instead of one per chunk', async () => {
const { runner, child, states } = makeRunner({ rescanOutdatedNames: async () => [] })
runner.start(['orca-cli'])
const pushesAfterStart = states.length
for (let frame = 0; frame < 25; frame += 1) {
child.stdout.emit('data', Buffer.from(`\rfetching ${frame}%`))
}
expect(states.length).toBe(pushesAfterStart)
child.emit('close', 0)
await flush()
const run = runner.getState()
expect(run.state).toBe('success')
// Nothing is dropped — the tail is drained before the run settles.
expect(run.state === 'success' && run.output).toContain('fetching 24%')
})
it('acknowledge clears a settled run but leaves a live one alone', async () => {
const { runner, child } = makeRunner({ rescanOutdatedNames: async () => [] })
runner.start(['orca-cli'])
+153 -17
View File
@@ -5,6 +5,7 @@ import {
type SkillUpdateStartResult
} from '../../shared/skill-freshness'
import { resolveCliCommand } from '../codex-cli/command'
import { killWithDescendantSweep } from '../pty-descendant-termination'
import { getSpawnArgsForWindows } from '../win32-utils'
// Why: `skills update` prints ANSI colour and \r + erase-line progress. We show
@@ -16,11 +17,23 @@ const ANSI_RE = /\x1b\[[0-9;?]*[A-Za-z]/g // eslint-disable-line no-control-rege
// however much the CLI decides to print.
const MAX_OUTPUT_CHARS = 32_000
// Long enough to swallow a burst of progress frames, short enough that the log
// still reads as live when the user has it expanded.
const OUTPUT_FLUSH_MS = 100
// Strictly above the sweep's own transitive worst case (~1s on POSIX; 3s identity
// query + 5s taskkill on Windows). A backstop that ties its own bound would fire
// while a slow-but-healthy sweep is still working.
export const CANCEL_RELEASE_TIMEOUT_MS = 12_000
export type SkillUpdateRunnerDeps = {
spawnProcess?: typeof spawn
resolveCommand?: (commandName: string) => string
/** Returns the subset of `names` still outdated after the run. */
/** Returns the subset of `names` that did not land, re-read from disk. */
rescanOutdatedNames?: (names: string[]) => Promise<string[]>
killTree?: (pid: number, killRoot: () => void) => Promise<void>
/** Injected so the Windows cmd.exe rail is reachable off Windows. */
buildSpawnArgs?: typeof getSpawnArgsForWindows
now?: () => number
onState?: (run: SkillUpdateRun) => void
}
@@ -45,6 +58,13 @@ function clampOutput(value: string): string {
export class SkillUpdateRunner {
private run: SkillUpdateRun = { state: 'idle' }
private child: ChildProcess | null = null
// Why: a failed spawn emits `error` *and* `close`, and a cancelled child still
// emits `close` after `kill()`. The token retires a child's handlers so a dead
// run can never settle or write output into the run that replaced it; the latch
// keeps the first verdict of a live run while its re-scan is still in flight.
private runToken = 0
private settling = false
private killing = false
private readonly deps: Required<Pick<SkillUpdateRunnerDeps, 'now'>> & SkillUpdateRunnerDeps
constructor(deps: SkillUpdateRunnerDeps = {}) {
@@ -77,12 +97,29 @@ export class SkillUpdateRunner {
let spawnCmd: string
let spawnArgs: string[]
try {
;({ spawnCmd, spawnArgs } = getSpawnArgsForWindows(npxCommand, npxArgs))
const buildSpawnArgs = this.deps.buildSpawnArgs ?? getSpawnArgsForWindows
;({ spawnCmd, spawnArgs } = buildSpawnArgs(npxCommand, npxArgs))
} catch {
return { started: false, reason: 'invalid-names' }
// Why: the names are already canonical here, so this is the cmd.exe rail
// rejecting the resolved npx *path* — a profile directory containing `&`,
// `%` or `!` is enough. Publishing the failure keeps the dialog honest;
// returning a bare `started: false` would leave the button dead and silent.
this.runToken += 1
this.settling = false
this.publish({
state: 'error',
names: canonicalNames,
finishedAt: this.deps.now(),
output: '',
failedNames: canonicalNames,
message: `Could not run ${npxCommand} safely from this location.`
})
return { started: false, reason: 'unsafe-command-path' }
}
const startedAt = this.deps.now()
const token = ++this.runToken
this.settling = false
this.publish({ state: 'running', names: canonicalNames, startedAt, output: '' })
const child = spawnProcess(spawnCmd, spawnArgs, {
@@ -94,32 +131,62 @@ export class SkillUpdateRunner {
})
this.child = child
const append = (chunk: Buffer): void => {
if (this.run.state !== 'running') {
// Why: `stripAnsi` turns each \r progress frame into its own line, so an npm
// install emits many chunks a second — and every publish structured-clones
// the whole buffer to every window. Coalesce into one push per tick.
let flushTimer: ReturnType<typeof setTimeout> | null = null
let pendingOutput = ''
const flush = (): void => {
flushTimer = null
if (token !== this.runToken || this.run.state !== 'running' || !pendingOutput) {
return
}
this.publish({
...this.run,
output: clampOutput(this.run.output + stripAnsi(chunk.toString('utf8')))
})
const appended = pendingOutput
pendingOutput = ''
this.publish({ ...this.run, output: clampOutput(this.run.output + appended) })
}
const append = (chunk: Buffer): void => {
if (token !== this.runToken || this.run.state !== 'running') {
return
}
pendingOutput = clampOutput(pendingOutput + stripAnsi(chunk.toString('utf8')))
if (!flushTimer) {
flushTimer = setTimeout(flush, OUTPUT_FLUSH_MS)
flushTimer.unref?.()
}
}
child.stdout?.on('data', append)
child.stderr?.on('data', append)
// The tail matters most on failure, so never let a pending chunk die with the
// process — drain it before the exit handlers settle the run.
const drain = (): void => {
if (flushTimer) {
clearTimeout(flushTimer)
}
flush()
}
child.on('error', (error) => {
this.settle(canonicalNames, error.message)
drain()
this.settle(token, canonicalNames, error.message)
})
child.on('close', (code) => {
this.settle(canonicalNames, code === 0 ? null : `skills update exited with code ${code}`)
drain()
this.settle(
token,
canonicalNames,
code === 0 ? null : `skills update exited with code ${code}`
)
})
return { started: true }
}
private settle(names: string[], spawnError: string | null): void {
if (this.run.state !== 'running') {
private settle(token: number, names: string[], spawnError: string | null): void {
if (token !== this.runToken || this.settling || this.run.state !== 'running') {
return
}
this.settling = true
this.child = null
const output = this.run.output
const finishedAt = this.deps.now()
@@ -130,6 +197,12 @@ export class SkillUpdateRunner {
// code only decides the outcome when no verdict is available, because
// `skills update` reports nothing else we can trust.
const finish = (failedNames: string[] | null): void => {
// The re-scan is slow enough that a cancel — or a whole replacement run —
// can land while it is still in flight; its verdict is about a run that no
// longer exists.
if (token !== this.runToken) {
return
}
const failed = failedNames ?? (spawnError ? names : [])
if (failed.length === 0) {
this.publish({ state: 'success', names, finishedAt, output })
@@ -156,11 +229,74 @@ export class SkillUpdateRunner {
}
cancel(): void {
this.child?.kill()
this.child = null
if (this.run.state === 'running') {
this.publish({ state: 'idle' })
if (this.killing) {
return
}
// Retire the child's handlers now so its exit settles nothing.
this.runToken += 1
this.settling = false
const child = this.child
this.child = null
if (!child) {
if (this.run.state === 'running') {
this.publish({ state: 'idle' })
}
return
}
// Why: `npx` is a wrapper — on POSIX the `skills` process it execs is a
// child, and on Windows the shim runs under cmd.exe. Killing only the direct
// child leaves the process that is actually writing to the global skill
// homes alive.
this.killing = true
let hasReleased = false
let releaseTimer: ReturnType<typeof setTimeout> | null = null
const release = (): void => {
if (hasReleased) {
return
}
hasReleased = true
if (releaseTimer) {
clearTimeout(releaseTimer)
}
this.killing = false
// Why: stay `running` until the tree is actually dead. The sweep waits for
// a descendant snapshot before it signals anything, so releasing on the
// synchronous path would let an immediate re-Update spawn a second npx
// writing the same bundles — the corruption the post-run verdict exists to
// catch. `start()` already refuses while running, so holding the state is
// the whole guard.
if (this.run.state === 'running') {
this.publish({ state: 'idle' })
}
}
// Every layer of the sweep is individually bounded, but this is the recovery
// path: if one ever fails to settle, the run would be stuck `running` with
// Stop already spent. Cap it rather than depend on that transitively.
releaseTimer = setTimeout(release, CANCEL_RELEASE_TIMEOUT_MS)
releaseTimer.unref?.()
if (this.run.state === 'running') {
this.publish({ ...this.run, stopping: true })
}
const kill = this.deps.killTree ?? killWithDescendantSweep
const pid = child.pid
if (typeof pid !== 'number') {
// Same contract as the sweep path below: a throwing kill must not escape
// and leave `killing` latched with the run stuck `running`.
try {
child.kill()
} catch {
/* already gone, or not ours to signal */
}
release()
return
}
// Why `release` on both paths and no retry: the sweep runs `killRoot()` in its
// own `finally`, so the only way it rejects is that kill throwing (EPERM) —
// calling it again would throw straight back out of the rejection handler,
// leaving an unhandled rejection and no release at all.
void kill(pid, () => child.kill()).then(release, release)
}
/** Clears a settled run so the status-bar segment can retire itself. */
@@ -13,7 +13,7 @@ import {
consumeSkillFreshnessUpdateDialogRequest,
requestSkillFreshnessUpdateDialog
} from './skill-freshness-update-dialog'
import { _resetSkillUpdateRunStore } from './skill-update-run-store'
import { _resetSkillUpdateRunStore, SKILL_UPDATE_SUCCESS_LINGER_MS } from './skill-update-run-store'
const mocks = vi.hoisted(() => ({
inventory: null as SkillFreshnessInventory | null,
@@ -67,7 +67,11 @@ vi.mock('@/components/ui/collapsible', () => ({
)
},
CollapsibleTrigger: ({ children }: { children?: ReactNode }) => <div>{children}</div>,
CollapsibleContent: ({ children }: { children?: ReactNode }) => <div>{children}</div>
// Tagged so tests can prove WHERE content sits: this mock renders it whether
// or not the disclosure is open, so presence in the DOM alone proves nothing.
CollapsibleContent: ({ children }: { children?: ReactNode }) => (
<div data-collapsible-content>{children}</div>
)
}))
const skillsApi = {
@@ -333,6 +337,32 @@ describe('SkillFreshnessUpdateDialog', () => {
expect(container?.textContent).toContain('2 locations')
})
it('does not let the status-bar linger retire the result being read here', async () => {
vi.useFakeTimers()
try {
await renderDialog()
await openViaRequest()
await emitRun({ state: 'success', names: ['orca-cli'], finishedAt: 2, output: 'done' })
await act(async () => {
vi.advanceTimersByTime(SKILL_UPDATE_SUCCESS_LINGER_MS * 3)
})
expect(skillsApi.acknowledgeUpdateRun).not.toHaveBeenCalled()
expect(container?.textContent).toContain('Updated 1 skill')
expect(findButton('Done')).toBeDefined()
expect(
container?.querySelector('[data-skill-row="orca-cli"]')?.getAttribute('data-state-label')
).toBe('done')
// Closing is what hands the run back — it must not be left stuck.
await clickButton('Done')
expect(skillsApi.acknowledgeUpdateRun).toHaveBeenCalledTimes(1)
} finally {
vi.useRealTimers()
}
})
it('shows the captured log verbatim without parsing it', async () => {
await renderDialog()
await openViaRequest()
@@ -362,7 +392,7 @@ describe('SkillFreshnessUpdateDialog', () => {
expect(findButton('Update 1 skill')).toBeUndefined()
})
it('keeps skills it cannot update visible with their reason', async () => {
it('shows why a skill was skipped without needing the disclosure', async () => {
mocks.inventory = {
schemaVersion: 1,
installations: [placement('computer-use', { topology: 'repo-scope' })],
@@ -372,18 +402,151 @@ describe('SkillFreshnessUpdateDialog', () => {
await renderDialog()
await openViaRequest()
const row = container?.querySelector('[data-skill-row="computer-use"]')
expect(container?.textContent).toContain('computer-use')
expect(container?.textContent).toContain('Skipped')
// The reason is the one thing a skipped row exists to say, so it lives
// outside the disclosure — visible with the row still collapsed, and not
// dependent on a mount-time `defaultOpen` a later re-scan could never re-fire.
expect(row?.getAttribute('data-collapsible-open')).toBe('false')
expect(container?.textContent).toContain('This is a project skill, not a global one')
// Assert the placement, not just the presence: moving it back inside the
// disclosure would hide it in production but still satisfy `textContent`.
expect(row?.querySelector('[data-collapsible-content]')?.textContent).not.toContain(
'This is a project skill, not a global one'
)
})
it('stays coherent while an idle re-scan is in flight', async () => {
await renderDialog()
await openViaRequest()
// Re-check publishes {inventory: null, loading: true} synchronously. The rows
// are retained, so the headline and the count must not be read off the live
// snapshot and the retained one at the same time — that pairs a "0 updates
// available" headline with rows badged "Update available".
mocks.inventory = null
mocks.loading = true
await rerender()
expect(container?.textContent).not.toContain('0 updates available')
expect(container?.textContent).toContain('Checking installed Orca skills…')
// The action keeps its place rather than reflowing the footer, but cannot
// fire against bytes that are being re-read.
const update = findButton('Update 1 skill')
expect(update).toBeDefined()
expect(update?.disabled).toBe(true)
expect(
container?.querySelector('[data-skill-row="orca-cli"]')?.getAttribute('data-state-label')
).toBe('available')
})
it('says it is stopping while the process tree is still being killed', async () => {
await renderDialog()
await openViaRequest()
await emitRun({ state: 'running', names: ['orca-cli'], startedAt: 1, output: '' })
await clickButton('Stop')
// Main holds the run `running` until the kill lands — that is what blocks a
// second writer — so the button must not sit enabled and inert meanwhile.
await emitRun({
state: 'running',
names: ['orca-cli'],
startedAt: 1,
output: '',
stopping: true
})
const stopping = findButton('Stopping…')
expect(stopping).toBeDefined()
expect(stopping?.disabled).toBe(true)
expect(findButton('Stop')).toBeUndefined()
// The headline must not contradict the button — telling someone the update
// "keeps running in the background" is the opposite of what Stop just did.
expect(container?.textContent).toContain('Stopping the update…')
expect(container?.textContent).not.toContain('keeps running in the background')
expect(container?.textContent).not.toContain('Updating 1 skill…')
// The primary button sits right next to "Stopping…" — it must not still be
// announcing "Updating…", visually or to a screen reader.
expect(findButton('Updating…')).toBeUndefined()
expect(container?.querySelector('[role="progressbar"]')?.getAttribute('aria-label')).toBe(
'Stopping the update…'
)
})
it('re-reads the inventory after a run is stopped', async () => {
await renderDialog()
await openViaRequest()
await emitRun({ state: 'running', names: ['orca-cli'], startedAt: 1, output: '' })
mocks.notifyChanged.mockClear()
// A killed run may already have written several skills; leaving the pre-run
// scan on screen would re-offer skills that are now current.
await emitRun({ state: 'idle' })
expect(mocks.notifyChanged).toHaveBeenCalled()
})
it('keeps the rows on screen while the settling re-scan is in flight', async () => {
await renderDialog()
await openViaRequest()
await emitRun({ state: 'success', names: ['orca-cli'], finishedAt: 2, output: 'done' })
// Settling notifies every skills surface, and that refresh nulls the
// inventory synchronously while it re-hashes every package on disk.
mocks.inventory = null
mocks.loading = true
await rerender()
expect(
container?.querySelector('[data-skill-row="orca-cli"]')?.getAttribute('data-state-label')
).toBe('done')
expect(container?.textContent).toContain('Updated 1 skill')
})
it('retries what failed rather than the emptied eligibility list', async () => {
await renderDialog()
await openViaRequest()
await emitRun({
state: 'error',
names: ['orca-cli'],
failedNames: ['orca-cli'],
finishedAt: 3,
output: '',
message: 'skills update exited with code 1'
})
// The settling re-scan has emptied `eligibleUpdateNames` by the time Retry
// is on screen; retrying that list would spawn nothing at all.
mocks.inventory = null
mocks.loading = true
await rerender()
await clickButton('Retry')
expect(skillsApi.startUpdateRun).toHaveBeenCalledWith(['orca-cli'])
})
it('offers a way out of a run that never finishes', async () => {
await renderDialog()
await openViaRequest()
await emitRun({ state: 'running', names: ['orca-cli'], startedAt: 1, output: '' })
await clickButton('Stop')
expect(skillsApi.cancelUpdateRun).toHaveBeenCalledTimes(1)
})
it('surfaces a scan error instead of a stale summary', async () => {
mocks.inventory = null
mocks.error = 'Missing canonical agent skills root'
await renderDialog()
await openViaRequest()
// Prime a good scan first: the retained inventory must not survive into the
// error state. That invariant lives in useSkillFreshness (every publish that
// sets `error` also clears `loading`), so pin it from the side that depends
// on it — otherwise a later "keep spinning while retrying" change would
// silently start showing stale rows under an error.
expect(container?.querySelector('[data-skill-row="orca-cli"]')).not.toBeNull()
mocks.inventory = null
mocks.error = 'Missing canonical agent skills root'
await rerender()
expect(container?.textContent).toContain('Missing canonical agent skills root')
expect(container?.querySelector('[data-skill-row="orca-cli"]')).toBeNull()
expect(findButton('Update 1 skill')).toBeUndefined()
})
})
@@ -1,6 +1,9 @@
import { useMemo, useState, useSyncExternalStore } from 'react'
import { useMemo, useRef, useState, useSyncExternalStore } from 'react'
import { AlertTriangle, CheckCircle2, ChevronDown, Copy, Loader2, RefreshCw } from 'lucide-react'
import { buildTargetedSkillUpdateCommand } from '../../../../shared/skill-freshness'
import {
buildTargetedSkillUpdateCommand,
type SkillFreshnessInventory
} from '../../../../shared/skill-freshness'
import { useSkillFreshness } from '@/hooks/useSkillFreshness'
import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills'
import { translate } from '@/i18n/i18n'
@@ -19,6 +22,7 @@ import { SkillUpdateRow } from './SkillUpdateRow'
import { SummaryHeadline, summarizeInventory } from './skill-freshness-summary-headline'
import {
acknowledgeSkillUpdateRun,
cancelSkillUpdateRun,
startSkillUpdateRun,
useSkillUpdateRun
} from './skill-update-run-store'
@@ -64,11 +68,31 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
getSkillFreshnessUpdateDialogRequest
)
const [copied, setCopied] = useState(false)
const inventory = state.inventory
const eligibleNames = useMemo(() => inventory?.eligibleUpdateNames ?? [], [inventory])
// Why: settling a run notifies every skills surface, and that refresh nulls the
// inventory *synchronously* while it re-hashes each package on disk. Rendering
// rows off the last good scan keeps them on screen through that window instead
// of blanking the dialog at the exact moment the result appears. Eligibility
// below still reads the live snapshot, so nothing is authorized off stale bytes.
const lastInventoryRef = useRef<SkillFreshnessInventory | null>(null)
if (state.inventory) {
lastInventoryRef.current = state.inventory
}
const inventory = state.inventory ?? (state.loading ? lastInventoryRef.current : null)
const eligibleNames = useMemo(() => state.inventory?.eligibleUpdateNames ?? [], [state.inventory])
// Display only. The action still fires `eligibleNames`, so a re-scan in flight
// can never authorize work — but the button keeps its place and its label
// instead of vanishing and reflowing the footer every time one runs.
const displayEligibleCount = inventory?.eligibleUpdateNames.length ?? 0
const isRunning = run.state === 'running'
// The kill sweep can take seconds; without this the Stop button sits enabled
// and inert, which reads as broken.
const isStopping = run.state === 'running' && run.stopping === true
const showResult = run.state === 'success' || run.state === 'error'
const runNames = useMemo(() => (run.state === 'idle' ? [] : run.names), [run])
// Keyed on the names themselves: every captured output chunk republishes the
// run, and regrouping the whole inventory per chunk would re-render each row.
const runNamesKey = run.state === 'idle' ? '' : run.names.join('\n')
const runNames = useMemo(() => (runNamesKey ? runNamesKey.split('\n') : []), [runNamesKey])
const groups = useMemo(
() =>
inventory
@@ -78,13 +102,18 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
)
const hasBlockedGroup = groups.some((group) => group.status === 'cannot-update')
const blockedCount = groups.filter((group) => group.status === 'cannot-update').length
const summaryKind = summarizeInventory(inventory, hasBlockedGroup)
// Why: the headline reads the LIVE snapshot, not the retained one — the two
// disagree for the whole loading window, and pairing a retained "eligible" with
// a live count of 0 renders "0 updates available" over rows badged "Update
// available". Live means it says "Checking…" over the rows it kept on screen.
const summaryKind = summarizeInventory(state.inventory, hasBlockedGroup)
// Why: one row list for every state. The rows are identical objects across the
// transition, so pressing Update changes each row's leading icon in place
// instead of swapping the dialog's body for a different component.
const failedNamesKey = run.state === 'error' ? run.failedNames.join('\n') : ''
const rows = useMemo(() => {
const failed = new Set(run.state === 'error' ? run.failedNames : [])
const failed = new Set(failedNamesKey ? failedNamesKey.split('\n') : [])
const inRun = new Set(runNames)
return groups.map((group) => {
if (inRun.has(group.name)) {
@@ -98,7 +127,7 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
state: group.status === 'cannot-update' ? ('blocked' as const) : ('available' as const)
}
})
}, [groups, isRunning, run, runNames])
}, [groups, isRunning, failedNamesKey, runNames])
const handleOpenChange = (next: boolean): void => {
if (next) {
@@ -108,14 +137,20 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
// status-bar segment carries it from here.
consumeSkillFreshnessUpdateDialogRequest()
setCopied(false)
// Don't carry a finished session's rows into the next open — but a live run
// keeps its own, or reopening from the status segment mid-run would land on
// an empty list while the close's own re-scan is still reading disk.
if (run.state === 'idle') {
lastInventoryRef.current = null
}
if (showResult) {
void acknowledgeSkillUpdateRun()
}
notifyInstalledAgentSkillsChanged()
}
const handleUpdate = (): void => {
void startSkillUpdateRun(eligibleNames)
const handleUpdate = (names: readonly string[]): void => {
void startSkillUpdateRun(names)
}
const handleCopyCommand = (): void => {
@@ -125,13 +160,33 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
if (!command) {
return
}
void navigator.clipboard.writeText(command).then(() => {
setCopied(true)
setTimeout(() => setCopied(false), 2000)
})
// Clipboard writes reject on a denied permission or an unfocused document;
// without this the button just never flips to "Copied".
void navigator.clipboard
.writeText(command)
.then(() => {
setCopied(true)
setTimeout(() => setCopied(false), 2000)
})
.catch((error: unknown) => {
console.error('Failed to copy skill update command', error)
})
}
const headline = ((): React.JSX.Element => {
if (isStopping) {
// Why: no "keeps running in the background" line here — after Stop that is
// the opposite of what is happening.
return (
<div className="flex items-center gap-2 text-sm font-medium text-foreground">
<Loader2 className="size-4 animate-spin text-muted-foreground" />
{translate(
'auto.components.skills.SkillFreshnessUpdateDialog.stoppingHeadline',
'Stopping the update…'
)}
</div>
)
}
if (isRunning) {
return (
<div className="space-y-1">
@@ -218,10 +273,17 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
// Indeterminate on purpose: the CLI reports no parseable progress.
<div
role="progressbar"
aria-label={translate(
'auto.components.skills.SkillFreshnessUpdateDialog.progressAria',
'Updating skills'
)}
aria-label={
isStopping
? translate(
'auto.components.skills.SkillFreshnessUpdateDialog.stoppingHeadline',
'Stopping the update…'
)
: translate(
'auto.components.skills.SkillFreshnessUpdateDialog.progressAria',
'Updating skills'
)
}
className="h-1 overflow-hidden rounded-full bg-secondary"
>
<div className="h-full w-2/5 animate-[skill-update-slide_1.35s_ease-in-out_infinite] rounded-full bg-primary motion-reduce:w-full motion-reduce:animate-none motion-reduce:opacity-40" />
@@ -250,7 +312,15 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
{run.message}
</p>
<div className="flex flex-wrap gap-1.5">
<Button type="button" variant="outline" size="xs" onClick={handleUpdate}>
{/* Retry what actually failed, not the live eligibility list —
the settling re-scan empties that for the whole window this
button is on screen, so retrying it would be a silent no-op. */}
<Button
type="button"
variant="outline"
size="xs"
onClick={() => handleUpdate(run.failedNames)}
>
{translate('auto.components.skills.SkillFreshnessUpdateDialog.retry', 'Retry')}
</Button>
<Button
@@ -275,7 +345,24 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
{isRunning || showResult ? <RunLog output={run.output} /> : null}
<DialogFooter className="sm:justify-between">
{isRunning || showResult ? (
{isRunning ? (
// The terminal used to be the escape hatch for a stalled update;
// without it a wedged npx would leave restarting Orca as the only way out.
<Button
type="button"
variant="ghost"
size="sm"
disabled={isStopping}
onClick={() => void cancelSkillUpdateRun()}
>
{isStopping
? translate(
'auto.components.skills.SkillFreshnessUpdateDialog.stopping',
'Stopping…'
)
: translate('auto.components.skills.SkillFreshnessUpdateDialog.stop', 'Stop')}
</Button>
) : showResult ? (
<span />
) : (
<Button
@@ -295,14 +382,21 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
? translate('auto.components.skills.SkillFreshnessUpdateDialog.done', 'Done')
: translate('auto.components.skills.SkillFreshnessUpdateDialog.close', 'Close')}
</Button>
{!showResult && eligibleNames.length > 0 ? (
<Button type="button" size="sm" disabled={isRunning} onClick={handleUpdate}>
{isRunning
{!showResult && displayEligibleCount > 0 ? (
<Button
type="button"
size="sm"
disabled={isRunning || eligibleNames.length === 0}
onClick={() => handleUpdate(eligibleNames)}
>
{/* Not during a stop: the Stop button already carries the status,
and "Updating…" beside "Stopping…" says both at once. */}
{isRunning && !isStopping
? translate(
'auto.components.skills.SkillFreshnessUpdateDialog.updating',
'Updating…'
)
: eligibleNames.length === 1
: displayEligibleCount === 1
? translate(
'auto.components.skills.SkillFreshnessUpdateDialog.updateActionOne',
'Update 1 skill'
@@ -310,7 +404,7 @@ export function SkillFreshnessUpdateDialog(): React.JSX.Element {
: translate(
'auto.components.skills.SkillFreshnessUpdateDialog.updateActionMany',
'Update {{value0}} skills',
{ value0: eligibleNames.length }
{ value0: displayEligibleCount }
)}
</Button>
) : null}
@@ -113,13 +113,17 @@ export function SkillUpdateRow({
</p>
) : null}
{/* Why: outside the disclosure, alongside the failure line. The reason is
the whole point of a skipped row, so it must not depend on a click —
nor on a mount-time `defaultOpen` that a re-scan can't re-fire. */}
{state === 'blocked' ? (
<p className="px-1.5 pb-1.5 text-xs leading-5 text-muted-foreground">
{skippedReason(group.locations)}
</p>
) : null}
<CollapsibleContent className="overflow-hidden data-[state=closed]:animate-collapsible-up data-[state=open]:animate-collapsible-down">
<div className="flex flex-col gap-2 px-1.5 pb-2 pt-0.5">
{state === 'blocked' ? (
<p className="text-xs leading-5 text-muted-foreground">
{skippedReason(group.locations)}
</p>
) : null}
{group.locations.map((location) => (
<div key={location.id} className="flex min-w-0 items-center gap-2">
{/* Why: plugin-cache paths nest arbitrarily deep. Without an explicit
@@ -66,21 +66,15 @@ export function SummaryHeadline({
)
}
if (kind === 'attention') {
// No follow-up sentence: skipped rows open themselves, so the reason is
// already on screen directly under this headline.
return (
<div className="space-y-1">
<div className="flex items-center gap-2 text-sm font-medium text-foreground">
<AlertTriangle className="size-4 text-amber-600 dark:text-amber-400" />
{translate(
'auto.components.skills.SkillFreshnessUpdateDialog.attention',
'Some installed Orca skills were left out of the update.'
)}
</div>
<p className="text-xs text-muted-foreground">
{translate(
'auto.components.skills.SkillFreshnessUpdateDialog.attentionDescription',
'Open Update details to see why each one was skipped.'
)}
</p>
<div className="flex items-center gap-2 text-sm font-medium text-foreground">
<AlertTriangle className="size-4 text-amber-600 dark:text-amber-400" />
{translate(
'auto.components.skills.SkillFreshnessUpdateDialog.attention',
'Some installed Orca skills were left out of the update.'
)}
</div>
)
}
@@ -1,143 +0,0 @@
import type { SkillFreshnessGroupModel, SkillLocationChip } from './skill-freshness-grouping'
import { translate } from '@/i18n/i18n'
import { Badge } from '@/components/ui/badge'
import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
import { skippedReason } from './skill-freshness-skipped-reason'
function chipLabel(chip: SkillLocationChip): string {
switch (chip) {
case 'current':
return translate('auto.components.skills.SkillFreshnessRow.chipCurrent', 'Current')
case 'unrecognized':
return translate('auto.components.skills.SkillFreshnessRow.chipUnrecognized', 'Unrecognized')
case 'inaccessible':
return translate('auto.components.skills.SkillFreshnessRow.chipInaccessible', 'Inaccessible')
case 'duplicate':
return translate('auto.components.skills.SkillFreshnessRow.chipDuplicate', 'Duplicate')
case 'external-link':
return translate('auto.components.skills.SkillFreshnessRow.chipExternalLink', 'External link')
case 'broken-link':
return translate('auto.components.skills.SkillFreshnessRow.chipBrokenLink', 'Broken link')
case 'read-only':
return translate('auto.components.skills.SkillFreshnessRow.chipReadOnly', 'Read only')
case 'in-a-repo':
return translate('auto.components.skills.SkillFreshnessRow.chipInRepo', 'In a repo')
case 'plugin-cache':
return translate('auto.components.skills.SkillFreshnessRow.chipPluginCache', 'Plugin cache')
}
}
// Why: chips describe only what a location *is*; the effect on the update
// command lives in the per-skill sentence, so the two never say it twice.
function chipTooltip(chip: SkillLocationChip): string {
switch (chip) {
case 'current':
return translate(
'auto.components.skills.SkillFreshnessRow.tipCurrent',
'This copy matches the current official version.'
)
case 'unrecognized':
return translate(
'auto.components.skills.SkillFreshnessRow.tipUnrecognized',
'This copy doesn’t match any official version — it may be modified, or a different skill with the same name.'
)
case 'inaccessible':
return translate(
'auto.components.skills.SkillFreshnessRow.tipInaccessible',
'Orca couldn’t read this copy (a permissions or file error).'
)
case 'duplicate':
return translate(
'auto.components.skills.SkillFreshnessRow.tipDuplicate',
'A separate copy of this skill, installed apart from the main one.'
)
case 'external-link':
return translate(
'auto.components.skills.SkillFreshnessRow.tipExternalLink',
'A shortcut pointing outside Orca’s skill folders.'
)
case 'broken-link':
return translate(
'auto.components.skills.SkillFreshnessRow.tipBrokenLink',
'A shortcut to something that no longer exists.'
)
case 'read-only':
return translate(
'auto.components.skills.SkillFreshnessRow.tipReadOnly',
'This copy is in a read-only location.'
)
case 'in-a-repo':
return translate(
'auto.components.skills.SkillFreshnessRow.tipInRepo',
'This copy lives inside a project, not your global skills.'
)
case 'plugin-cache':
return translate(
'auto.components.skills.SkillFreshnessRow.tipPluginCache',
'This copy is managed by a plugin.'
)
}
}
export function SkillFreshnessGroup({
group
}: {
group: SkillFreshnessGroupModel
}): React.JSX.Element {
const isBlocked = group.status === 'cannot-update'
return (
<div className="space-y-2 py-3 first:pt-0 last:pb-0">
<div className="flex min-w-0 flex-wrap items-center gap-2">
<span className="text-sm font-medium text-foreground">{group.name}</span>
{isBlocked ? (
<Badge
variant="outline"
className="border-amber-600/50 text-amber-700 dark:border-amber-400/40 dark:text-amber-400"
>
{translate('auto.components.skills.SkillFreshnessRow.statusCantUpdate', 'Skipped')}
</Badge>
) : (
<Badge variant="secondary">
{translate(
'auto.components.skills.SkillFreshnessRow.statusUpdateAvailable',
'Update available'
)}
</Badge>
)}
</div>
{isBlocked ? (
<p className="text-xs leading-5 text-muted-foreground">{skippedReason(group.locations)}</p>
) : null}
<div className="flex flex-col gap-2">
{group.locations.map((location) => (
<div
key={location.id}
className="flex min-w-0 items-center gap-2 border-l-2 border-border/60 pl-3"
>
{/* Why: plugin-cache paths nest arbitrarily deep. Without an explicit
shrink basis the unbreakable string sets the dialog's width and
pushes the footer actions off-screen. */}
<span
className="min-w-0 flex-1 truncate font-mono text-[11px] text-muted-foreground"
title={location.path}
>
{location.path}
</span>
{location.chip ? (
<Tooltip>
<TooltipTrigger asChild>
<Badge variant="outline" className="shrink-0 cursor-help border-dashed">
{chipLabel(location.chip)}
</Badge>
</TooltipTrigger>
<TooltipContent className="max-w-xs text-pretty">
{chipTooltip(location.chip)}
</TooltipContent>
</Tooltip>
) : null}
</div>
))}
</div>
</div>
)
}
@@ -1,6 +1,10 @@
import { useSyncExternalStore } from 'react'
import type { SkillUpdateRun } from '../../../../shared/skill-freshness'
import { notifyInstalledAgentSkillsChanged } from '@/hooks/useInstalledAgentSkills'
import {
getSkillFreshnessUpdateDialogRequest,
subscribeSkillFreshnessUpdateDialog
} from './skill-freshness-update-dialog'
// Why: the run outlives the dialog — closing the window must not cancel it, and
// the status-bar segment needs the same snapshot. Keeping it outside React means
@@ -19,22 +23,42 @@ function emit(): void {
}
}
function setRun(next: SkillUpdateRun): void {
run = next
function clearSuccessTimer(): void {
if (successTimer) {
clearTimeout(successTimer)
successTimer = null
}
if (next.state === 'success') {
// Why: a success needs to be *seen*, then get out of the way. Errors stay
// until the user acts on them.
successTimer = setTimeout(() => {
successTimer = null
void window.api.skills.acknowledgeUpdateRun()
}, SKILL_UPDATE_SUCCESS_LINGER_MS)
}
/**
* Why: a success needs to be *seen*, then get out of the way. Errors stay until
* the user acts on them. The dialog renders this same run, so retiring it while
* the dialog is open would yank the result rows out from under someone reading
* them — there, closing the dialog is what acknowledges the run.
*/
function scheduleSuccessLinger(): void {
clearSuccessTimer()
if (run.state !== 'success' || getSkillFreshnessUpdateDialogRequest()) {
return
}
if (next.state === 'success' || next.state === 'error') {
// A finished run changes what's on disk; let every skills surface re-read.
successTimer = setTimeout(() => {
successTimer = null
void acknowledgeSkillUpdateRun()
}, SKILL_UPDATE_SUCCESS_LINGER_MS)
}
// Opening the dialog on a lingering success hands ownership back to it.
subscribeSkillFreshnessUpdateDialog(scheduleSuccessLinger)
function setRun(next: SkillUpdateRun): void {
const wasRunning = run.state === 'running'
run = next
scheduleSuccessLinger()
// A run that stopped changes what's on disk — including a cancelled one, which
// may have written several skills before the kill landed. Without this a Stop
// leaves the rows and the count describing the pre-run scan, so the Update
// button re-offers skills that already updated.
if (next.state === 'success' || next.state === 'error' || (wasRunning && next.state === 'idle')) {
notifyInstalledAgentSkillsChanged()
}
emit()
@@ -68,26 +92,38 @@ export function useSkillUpdateRun(): SkillUpdateRun {
return useSyncExternalStore(subscribeSkillUpdateRun, getSkillUpdateRun, getSkillUpdateRun)
}
// Why: every caller fires these from an event handler with `void`. Swallowing
// here rather than at each call site keeps a dropped IPC from surfacing as an
// unhandled rejection; the run state itself is pushed from main either way.
export async function startSkillUpdateRun(names: readonly string[]): Promise<void> {
ensureSubscribed()
await window.api.skills.startUpdateRun([...names])
try {
await window.api.skills.startUpdateRun([...names])
} catch (error) {
console.error('Failed to start skill update run', error)
}
}
export async function cancelSkillUpdateRun(): Promise<void> {
await window.api.skills.cancelUpdateRun()
try {
await window.api.skills.cancelUpdateRun()
} catch (error) {
console.error('Failed to cancel skill update run', error)
}
}
export async function acknowledgeSkillUpdateRun(): Promise<void> {
await window.api.skills.acknowledgeUpdateRun()
try {
await window.api.skills.acknowledgeUpdateRun()
} catch (error) {
console.error('Failed to acknowledge skill update run', error)
}
}
/** @internal - tests need a clean module between cases. */
export function _resetSkillUpdateRunStore(): void {
run = { state: 'idle' }
subscribed = false
if (successTimer) {
clearTimeout(successTimer)
successTimer = null
}
clearSuccessTimer()
listeners.clear()
}
@@ -18,6 +18,25 @@ export function SkillUpdateStatusSegment({
}
const segment = (() => {
if (run.state === 'running' && run.stopping) {
// The dialog can be closed while a Stop is still killing the tree; this is
// then the only surface reporting it, and "Updating skills" would be wrong.
return {
icon: <Loader2 className="size-3 animate-spin text-muted-foreground" />,
label: translate(
'auto.components.status.bar.SkillUpdateStatusSegment.stoppingLabel',
'Stopping update'
),
tooltip: translate(
'auto.components.status.bar.SkillUpdateStatusSegment.stoppingTooltip',
'Stopping the skill update…'
),
ariaLabel: translate(
'auto.components.status.bar.SkillUpdateStatusSegment.stoppingAria',
'Stopping the skill update. Click to open details.'
)
}
}
if (run.state === 'running') {
return {
icon: <Loader2 className="size-3 animate-spin text-muted-foreground" />,
+8 -3
View File
@@ -3177,7 +3177,10 @@
"successAria": "Skills updated. Click to open details.",
"errorLabel": "Update failed",
"errorTooltip": "Skill update failed — click to see details",
"errorAria": "Skill update failed. Click to open details."
"errorAria": "Skill update failed. Click to open details.",
"stoppingLabel": "Stopping update",
"stoppingTooltip": "Stopping the skill update…",
"stoppingAria": "Stopping the skill update. Click to open details."
},
"UpdateStatusSegment": {
"5cd13105a3": "Update failed. Click to expand.",
@@ -3792,7 +3795,6 @@
"blockedMany": "{{value0}} skills can't be updated automatically.",
"success": "All installed Orca skills are up to date.",
"attention": "Some installed Orca skills were left out of the update.",
"attentionDescription": "Open Update details to see why each one was skipped.",
"runningOne": "Updating 1 skill…",
"runningMany": "Updating {{value0}} skills…",
"runningDescription": "You can close this window — it keeps running in the background.",
@@ -3810,7 +3812,10 @@
"updateActionMany": "Update {{value0}} skills",
"checkNow": "Re-check",
"done": "Done",
"close": "Close"
"close": "Close",
"stop": "Stop",
"stopping": "Stopping…",
"stoppingHeadline": "Stopping the update…"
},
"SkillUpdateResultRows": {
"stillOutdated": "Still out of date after the update ran."
+8 -3
View File
@@ -3356,7 +3356,10 @@
"successAria": "Skills updated. Click to open details.",
"errorLabel": "Update failed",
"errorTooltip": "Skill update failed — click to see details",
"errorAria": "Skill update failed. Click to open details."
"errorAria": "Skill update failed. Click to open details.",
"stoppingLabel": "Stopping update",
"stoppingTooltip": "Stopping the skill update…",
"stoppingAria": "Stopping the skill update. Click to open details."
}
}
},
@@ -3767,7 +3770,6 @@
"updateMany": "Se pueden actualizar {{value0}} skills de forma segura",
"success": "Todas las skills de Orca instaladas están actualizadas.",
"attention": "Algunas skills de Orca instaladas no se pueden actualizar automáticamente.",
"attentionDescription": "Abre Detalles de la actualización para ver por qué no se puede actualizar cada una.",
"checkNow": "Comprobar ahora",
"close": "Cerrar",
"blockedOne": "1 skill can't be updated automatically.",
@@ -3787,7 +3789,10 @@
"updating": "Updating…",
"updateActionOne": "Update 1 skill",
"updateActionMany": "Update {{value0}} skills",
"done": "Done"
"done": "Done",
"stop": "Stop",
"stopping": "Stopping…",
"stoppingHeadline": "Stopping the update…"
},
"SkillFreshnessStatusPill": {
"updateAvailable": "Actualización disponible",
+8 -3
View File
@@ -3356,7 +3356,10 @@
"successAria": "Skills updated. Click to open details.",
"errorLabel": "Update failed",
"errorTooltip": "Skill update failed — click to see details",
"errorAria": "Skill update failed. Click to open details."
"errorAria": "Skill update failed. Click to open details.",
"stoppingLabel": "Stopping update",
"stoppingTooltip": "Stopping the skill update…",
"stoppingAria": "Stopping the skill update. Click to open details."
}
}
},
@@ -3767,7 +3770,6 @@
"updateMany": "{{value0}} 件のスキルを安全に更新できます",
"success": "インストール済みの Orca スキルはすべて最新です。",
"attention": "一部のインストール済み Orca スキルは自動的に更新できません。",
"attentionDescription": "更新できない理由は、更新の詳細を開いて確認してください。",
"checkNow": "今すぐ確認",
"close": "閉じる",
"blockedOne": "1 skill can't be updated automatically.",
@@ -3787,7 +3789,10 @@
"updating": "Updating…",
"updateActionOne": "Update 1 skill",
"updateActionMany": "Update {{value0}} skills",
"done": "Done"
"done": "Done",
"stop": "Stop",
"stopping": "Stopping…",
"stoppingHeadline": "Stopping the update…"
},
"SkillFreshnessStatusPill": {
"updateAvailable": "更新があります",
+8 -3
View File
@@ -3356,7 +3356,10 @@
"successAria": "Skills updated. Click to open details.",
"errorLabel": "Update failed",
"errorTooltip": "Skill update failed — click to see details",
"errorAria": "Skill update failed. Click to open details."
"errorAria": "Skill update failed. Click to open details.",
"stoppingLabel": "Stopping update",
"stoppingTooltip": "Stopping the skill update…",
"stoppingAria": "Stopping the skill update. Click to open details."
}
}
},
@@ -3767,7 +3770,6 @@
"updateMany": "스킬 {{value0}}개를 안전하게 업데이트할 수 있습니다",
"success": "설치된 Orca 스킬이 모두 최신 상태입니다.",
"attention": "일부 설치된 Orca 스킬은 자동으로 업데이트할 수 없습니다.",
"attentionDescription": "각 항목을 업데이트할 수 없는 이유는 업데이트 세부 정보를 열어 확인하세요.",
"checkNow": "지금 확인",
"close": "닫기",
"blockedOne": "1 skill can't be updated automatically.",
@@ -3787,7 +3789,10 @@
"updating": "Updating…",
"updateActionOne": "Update 1 skill",
"updateActionMany": "Update {{value0}} skills",
"done": "Done"
"done": "Done",
"stop": "Stop",
"stopping": "Stopping…",
"stoppingHeadline": "Stopping the update…"
},
"SkillFreshnessStatusPill": {
"updateAvailable": "업데이트 가능",
+8 -3
View File
@@ -3356,7 +3356,10 @@
"successAria": "Skills updated. Click to open details.",
"errorLabel": "Update failed",
"errorTooltip": "Skill update failed — click to see details",
"errorAria": "Skill update failed. Click to open details."
"errorAria": "Skill update failed. Click to open details.",
"stoppingLabel": "Stopping update",
"stoppingTooltip": "Stopping the skill update…",
"stoppingAria": "Stopping the skill update. Click to open details."
}
}
},
@@ -3767,7 +3770,6 @@
"updateMany": "可以安全更新 {{value0}} 个技能",
"success": "所有已安装的 Orca 技能均为最新。",
"attention": "部分已安装的 Orca 技能无法自动更新。",
"attentionDescription": "打开“更新详情”查看为何每一项都无法更新。",
"checkNow": "立即检查",
"close": "关闭",
"blockedOne": "1 skill can't be updated automatically.",
@@ -3787,7 +3789,10 @@
"updating": "Updating…",
"updateActionOne": "Update 1 skill",
"updateActionMany": "Update {{value0}} skills",
"done": "Done"
"done": "Done",
"stop": "Stop",
"stopping": "Stopping…",
"stoppingHeadline": "Stopping the update…"
},
"SkillFreshnessStatusPill": {
"updateAvailable": "有可用更新",
+5 -2
View File
@@ -111,7 +111,10 @@ export function buildTargetedSkillUpdateCommand(names: readonly string[]): strin
// the inventory after exit, never from parsing stdout.
export type SkillUpdateRun =
| { state: 'idle' }
| { state: 'running'; names: string[]; startedAt: number; output: string }
// `stopping` covers the window between Stop and the process tree actually
// dying — the run is still `running` (that is what blocks a second writer),
// but the Stop affordance has already been spent.
| { state: 'running'; names: string[]; startedAt: number; output: string; stopping?: boolean }
| { state: 'success'; names: string[]; finishedAt: number; output: string }
| {
state: 'error'
@@ -125,4 +128,4 @@ export type SkillUpdateRun =
export type SkillUpdateStartResult =
| { started: true }
| { started: false; reason: 'already-running' | 'invalid-names' }
| { started: false; reason: 'already-running' | 'invalid-names' | 'unsafe-command-path' }