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. */