From 9f5c32b0d100ea703b64dbfc061359c5135fad23 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 27 Oct 2026 04:08:12 -0700 Subject: [PATCH] fix(kiro): manage status hooks in agent configs kiro-cli agent upgrade rewrote (#34385) kiro-cli agent upgrade (and Kiro CLI 3.0) turns an agent's hooks into a list of {name, trigger, matcher, action, timeout}. Orca rejected that shape, so status reported an error and on/off left the files untouched. Orca now refreshes its list entries in place, appends missing ones, removes only its own on off, and writes the list form into upgraded agents that have no hooks yet so they stay in the universal format. The remote installer shares the same path. Fixes #34194 --- src/main/kiro/hook-service.test.ts | 66 +++++++++++++++- src/main/kiro/hook-settings.ts | 91 +++++++++++++++++++++-- src/main/kiro/standalone-hook-settings.ts | 22 ++---- 3 files changed, 159 insertions(+), 20 deletions(-) diff --git a/src/main/kiro/hook-service.test.ts b/src/main/kiro/hook-service.test.ts index d2c83c85a41..2039d2c9bb0 100644 --- a/src/main/kiro/hook-service.test.ts +++ b/src/main/kiro/hook-service.test.ts @@ -141,7 +141,71 @@ describe('KiroHookService', () => { }) }) - it.each([{ hooks: [] }, { hooks: 'keep this' }, { hooks: { stop: { command: 'say done' } } }])( + // Shape `kiro-cli agent upgrade` (2.29+) writes; edits to `hooks` keep the file "universal". + it('converges list-form hooks in place and removes only its entries, locally and remotely', async () => { + const orcaHook = (trigger: string, command: string) => ({ + name: `${trigger}-0`, + trigger, + ...(trigger === 'preToolUse' ? { matcher: '*' } : {}), + action: { type: 'command', command }, + timeout: 10 + }) + const userHook = { + name: 'preToolUse-0', + trigger: 'preToolUse', + matcher: 'execute_bash', + action: { type: 'command', command: 'audit.sh' }, + timeout: 10 + } + const stale = `/bin/sh '${join(home, '.orca', 'agent-hooks', 'kiro-hook.sh')}'` + const extra = { + permissions: { rules: [{ capability: 'shell', effect: 'allow' }] }, + kiroUpgrade: { notes: [] } + } + writeAgent('reviewer', { + name: 'reviewer', + hooks: [orcaHook('agentSpawn', stale), userHook, orcaHook('preToolUse', stale)], + ...extra + }) + writeAgent('upgraded', { name: 'upgraded', permissions: extra.permissions }) + + const service = new KiroHookService() + expect(service.getStatus()).toMatchObject({ state: 'partial', managedHooksPresent: true }) + expect(service.install().state).toBe('installed') + const installed = readAgent('reviewer') + const hooks = installed.hooks + if (!Array.isArray(hooks)) { + throw new Error('Expected hooks list') + } + expect(hooks.map((hook) => hook.trigger)).toEqual([ + 'agentSpawn', + 'preToolUse', + 'preToolUse', + 'userPromptSubmit', + 'postToolUse', + 'stop' + ]) + expect(hooks[1]).toEqual(userHook) + expect(hooks[2]).toMatchObject({ matcher: '*', timeout: 10, action: { type: 'command' } }) + expect(hooks.filter((hook) => hook.action.command === stale)).toEqual([]) + expect(installed).toMatchObject(extra) + expect(readAgent('upgraded').hooks).toHaveLength(KIRO_HOOK_EVENTS.length) + + expect(service.remove().state).toBe('not_installed') + expect(readAgent('reviewer')).toEqual({ name: 'reviewer', hooks: [userHook], ...extra }) + expect(readAgent('upgraded')).toEqual({ name: 'upgraded', permissions: extra.permissions }) + + expect(await service.installRemote(createManagedHookLocalFilesystem(), home)).toMatchObject({ + state: 'installed' + }) + expect(readAgent('reviewer').hooks).toHaveLength(KIRO_HOOK_EVENTS.length + 1) + }) + + it.each([ + { hooks: ['say done'] }, + { hooks: 'keep this' }, + { hooks: { stop: { command: 'say done' } } } + ])( 'preserves unsupported user hook structures and reports them locally and remotely: %j', async ({ hooks }) => { writeAgent('unsupported', { name: 'unsupported', hooks }) diff --git a/src/main/kiro/hook-settings.ts b/src/main/kiro/hook-settings.ts index 2e066df4fa9..8741ce87548 100644 --- a/src/main/kiro/hook-settings.ts +++ b/src/main/kiro/hook-settings.ts @@ -6,6 +6,7 @@ import { hookDefinitionHasManagedCommand, isPlainObject, MANAGED_HOOK_TIMEOUT_MILLISECONDS, + MANAGED_HOOK_TIMEOUT_SECONDS, removeManagedCommands, wrapPosixHookCommand, type HookDefinition @@ -29,6 +30,9 @@ export function isKiroHooksConfigSupported(config: Record): boo if (config.hooks === undefined) { return true } + if (Array.isArray(config.hooks)) { + return config.hooks.every(isPlainObject) + } if (!isPlainObject(config.hooks)) { return false } @@ -85,6 +89,72 @@ function buildManagedDefinition(event: string, command: string): HookDefinition } } +/** List-form entry, as in V3 hook files and agents `kiro-cli agent upgrade` rewrote. */ +export function buildKiroListHook(trigger: string, command: string): Record { + return { + name: `orca-agent-status-${trigger}`, + trigger, + ...(KIRO_TOOL_EVENTS.has(trigger) ? { matcher: '*' } : {}), + action: { type: 'command', command }, + timeout: MANAGED_HOOK_TIMEOUT_SECONDS + } +} + +export function kiroListHookRunsManagedCommand( + entry: unknown, + isManagedCommand: (command: string | undefined) => boolean +): boolean { + const action = isPlainObject(entry) ? entry.action : undefined + return ( + isPlainObject(action) && typeof action.command === 'string' && isManagedCommand(action.command) + ) +} + +function managedListHookEvent( + entry: unknown, + isManagedCommand: (command: string | undefined) => boolean +): string | null { + if (!isPlainObject(entry) || !kiroListHookRunsManagedCommand(entry, isManagedCommand)) { + return null + } + return KIRO_HOOK_EVENTS.find((event) => event === entry.trigger) ?? null +} + +// An upgraded agent carries `permissions`; object-form hooks would knock it out of the universal format. +function usesListHooks(config: Record): boolean { + return ( + Array.isArray(config.hooks) || (config.hooks === undefined && isPlainObject(config.permissions)) + ) +} + +function applyManagedKiroListHooks( + entries: unknown[], + command: string, + isManagedCommand: (command: string | undefined) => boolean +): unknown[] { + const missing = new Set(KIRO_HOOK_EVENTS) + const kept = entries.flatMap((entry) => { + const event = managedListHookEvent(entry, isManagedCommand) + if (event === null) { + return [entry] + } + // Refresh the first Orca entry per event where it sits; drop duplicates. + return missing.delete(event) ? [buildKiroListHook(event, command)] : [] + }) + return [...kept, ...[...missing].map((event) => buildKiroListHook(event, command))] +} + +function withHooks( + config: Record, + hooks: unknown[] | Record +): Record { + if ((Array.isArray(hooks) ? hooks.length : Object.keys(hooks).length) > 0) { + return { ...config, hooks } + } + const { hooks: _emptied, ...rest } = config + return rest +} + function splitDefinitions(value: unknown): { definitions: HookDefinition[]; other: unknown[] } { const entries = Array.isArray(value) ? value : [] return { @@ -108,6 +178,10 @@ export function applyManagedKiroHooks( command: string, isManagedCommand: (command: string | undefined) => boolean ): Record { + if (usesListHooks(config)) { + const entries = Array.isArray(config.hooks) ? config.hooks : [] + return { ...config, hooks: applyManagedKiroListHooks(entries, command, isManagedCommand) } + } const hooks: Record = isPlainObject(config.hooks) ? { ...config.hooks } : {} for (const event of KIRO_HOOK_EVENTS) { const { definitions, other } = splitDefinitions(hooks[event]) @@ -125,6 +199,12 @@ export function removeManagedKiroHooks( isManagedCommand: (command: string | undefined) => boolean ): Record { const original = config.hooks + if (Array.isArray(original)) { + const hooks = original.filter( + (entry) => !kiroListHookRunsManagedCommand(entry, isManagedCommand) + ) + return hooks.length === original.length ? config : withHooks(config, hooks) + } if ( !isPlainObject(original) || !Object.values(original).some((value) => hasManagedDefinition(value, isManagedCommand)) @@ -144,17 +224,18 @@ export function removeManagedKiroHooks( delete hooks[event] } } - if (Object.keys(hooks).length > 0) { - return { ...config, hooks } - } - const { hooks: _emptied, ...rest } = config - return rest + return withHooks(config, hooks) } export function readManagedKiroHookEvents( config: unknown, isManagedCommand: (command: string | undefined) => boolean ): Set { + if (isPlainObject(config) && Array.isArray(config.hooks)) { + return new Set( + config.hooks.flatMap((entry) => managedListHookEvent(entry, isManagedCommand) ?? []) + ) + } const hooks = isPlainObject(config) && isPlainObject(config.hooks) ? config.hooks : {} return new Set( KIRO_HOOK_EVENTS.filter((event) => hasManagedDefinition(hooks[event], isManagedCommand)) diff --git a/src/main/kiro/standalone-hook-settings.ts b/src/main/kiro/standalone-hook-settings.ts index 306d496d621..a4e6daa9f9e 100644 --- a/src/main/kiro/standalone-hook-settings.ts +++ b/src/main/kiro/standalone-hook-settings.ts @@ -1,7 +1,11 @@ import { join, posix as pathPosix } from 'node:path' import { resolveKiroHomeDir } from '../../shared/kiro-home' -import { isPlainObject, MANAGED_HOOK_TIMEOUT_SECONDS } from '../agent-hooks/installer-utils' -import { getKiroManagedCommandMatcher } from './hook-settings' +import { isPlainObject } from '../agent-hooks/installer-utils' +import { + buildKiroListHook, + getKiroManagedCommandMatcher, + kiroListHookRunsManagedCommand +} from './hook-settings' const KIRO_STANDALONE_HOOKS_FILE_NAME = 'orca-agent-status.json' @@ -31,12 +35,7 @@ export function getKiroRemoteStandaloneHooksFilePath( export function buildKiroStandaloneHooksFile(command: string): Record { return { version: 'v1', - hooks: KIRO_STANDALONE_HOOK_TRIGGERS.map((trigger) => ({ - name: `orca-agent-status-${trigger}`, - trigger, - action: { type: 'command', command }, - timeout: MANAGED_HOOK_TIMEOUT_SECONDS - })) + hooks: KIRO_STANDALONE_HOOK_TRIGGERS.map((trigger) => buildKiroListHook(trigger, command)) } } @@ -45,12 +44,7 @@ function hookEntries(file: Record): unknown[] { } function runsManagedKiroScript(entry: unknown): boolean { - const action = isPlainObject(entry) ? entry.action : undefined - return ( - isPlainObject(action) && - typeof action.command === 'string' && - getKiroManagedCommandMatcher()(action.command) - ) + return kiroListHookRunsManagedCommand(entry, getKiroManagedCommandMatcher()) } /** `activeOnly` drops `enabled: false` entries, which Kiro skips. */